media: chips-media: wave5: Fix Reports from Kernel Lock Validator

handle_dynamic_resolution change requires that the state_lock be acquired
based on the lockdep_assert_held. However, the
handle_dynamic_resolution_change call in initialize_sequence does not
properly obtain the lock before calling.

Since the v4l2_ctrl_find and s_ctrl can sleep, they should not be called
while a lock is already held. Store off the fbc_buf_count then properly
update control once lock has been freed.

Signed-off-by: Brandon Brnich <b-brnich@ti.com>
Tested-by: Jackson Lee <jackson.lee@chipsnmedia.com>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>

diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index d419076d7052c61ec50a40df3c1049031de07764..bb2ba9204a8365c8df2fc27f3cd43bb593b4c8a1 100644 (file)
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -283,10 +283,23 @@ static void send_eos_event(struct vpu_instance *inst)
        inst->sent_eos = true;
 }
 
+static void wave5_update_min_bufs_ctrl(struct vpu_instance *inst, u32 fbc_buf_count)
+{
+       struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
+       struct v4l2_ctrl *ctrl;
+
+       if (!fbc_buf_count || fbc_buf_count == v4l2_m2m_num_dst_bufs_ready(m2m_ctx))
+               return;
+
+       ctrl = v4l2_ctrl_find(&inst->v4l2_ctrl_hdl,
+                             V4L2_CID_MIN_BUFFERS_FOR_CAPTURE);
+       if (ctrl)
+               v4l2_ctrl_s_ctrl(ctrl, fbc_buf_count);
+}
+
 static int handle_dynamic_resolution_change(struct vpu_instance *inst)
 {
        struct v4l2_fh *fh = &inst->v4l2_fh;
-       struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
 
        static const struct v4l2_event vpu_event_src_ch = {
                .type = V4L2_EVENT_SOURCE_CHANGE,
@@ -305,14 +318,6 @@ static int handle_dynamic_resolution_change(struct vpu_instance *inst)
 
        inst->needs_reallocation = true;
        inst->fbc_buf_count = initial_info->min_frame_buffer_count + 1;
-       if (inst->fbc_buf_count != v4l2_m2m_num_dst_bufs_ready(m2m_ctx)) {
-               struct v4l2_ctrl *ctrl;
-
-               ctrl = v4l2_ctrl_find(&inst->v4l2_ctrl_hdl,
-                                     V4L2_CID_MIN_BUFFERS_FOR_CAPTURE);
-               if (ctrl)
-                       v4l2_ctrl_s_ctrl(ctrl, inst->fbc_buf_count);
-       }
 
        if (p_dec_info->initial_info_obtained) {
                const struct vpu_format *vpu_fmt;
@@ -439,19 +444,24 @@ static void wave5_vpu_dec_finish_decode(struct vpu_instance *inst)
        if ((dec_info.index_frame_display == DISPLAY_IDX_FLAG_SEQ_END ||
             dec_info.sequence_changed)) {
                unsigned long flags;
+               u32 fbc_buf_count = 0;
 
                spin_lock_irqsave(&inst->state_spinlock, flags);
                if (!v4l2_m2m_has_stopped(m2m_ctx)) {
                        switch_state(inst, VPU_INST_STATE_STOP);
 
-                       if (dec_info.sequence_changed)
+                       if (dec_info.sequence_changed) {
                                handle_dynamic_resolution_change(inst);
-                       else
+                               fbc_buf_count = inst->fbc_buf_count;
+                       } else {
                                send_eos_event(inst);
+                       }
 
                        flag_last_buffer_done(inst);
                }
                spin_unlock_irqrestore(&inst->state_spinlock, flags);
+
+               wave5_update_min_bufs_ctrl(inst, fbc_buf_count);
        }
 
        if (inst->sent_eos &&
@@ -1592,8 +1602,9 @@ static const struct vpu_instance_ops wave5_vpu_dec_inst_ops = {
 static int initialize_sequence(struct vpu_instance *inst)
 {
        struct dec_initial_info initial_info;
-       int ret = 0;
        unsigned long flags;
+       u32 fbc_buf_count;
+       int ret = 0;
 
        memset(&initial_info, 0, sizeof(struct dec_initial_info));
 
@@ -1617,8 +1628,11 @@ static int initialize_sequence(struct vpu_instance *inst)
 
        spin_lock_irqsave(&inst->state_spinlock, flags);
        handle_dynamic_resolution_change(inst);
+       fbc_buf_count = inst->fbc_buf_count;
        spin_unlock_irqrestore(&inst->state_spinlock, flags);
 
+       wave5_update_min_bufs_ctrl(inst, fbc_buf_count);
+
        return 0;
 }
 
@@ -1659,6 +1673,7 @@ static void wave5_vpu_dec_device_run(void *priv)
                ret = initialize_sequence(inst);
                if (ret) {
                        unsigned long flags;
+                       u32 fbc_buf_count = 0;
 
                        spin_lock_irqsave(&inst->state_spinlock, flags);
                        if (wave5_is_draining_or_eos(inst) &&
@@ -1667,14 +1682,18 @@ static void wave5_vpu_dec_device_run(void *priv)
 
                                switch_state(inst, VPU_INST_STATE_STOP);
 
-                               if (vb2_is_streaming(dst_vq))
+                               if (vb2_is_streaming(dst_vq)) {
                                        send_eos_event(inst);
-                               else
+                               } else {
                                        handle_dynamic_resolution_change(inst);
+                                       fbc_buf_count = inst->fbc_buf_count;
+                               }
 
                                flag_last_buffer_done(inst);
                        }
                        spin_unlock_irqrestore(&inst->state_spinlock, flags);
+
+                       wave5_update_min_bufs_ctrl(inst, fbc_buf_count);
                } else {
                        set_instance_state(inst, VPU_INST_STATE_INIT_SEQ);
                }