diff --git a/driver/platform/kalama/src/msm_vidc_kalama.c b/driver/platform/kalama/src/msm_vidc_kalama.c index 63789f49ad..a9003192dc 100644 --- a/driver/platform/kalama/src/msm_vidc_kalama.c +++ b/driver/platform/kalama/src/msm_vidc_kalama.c @@ -321,7 +321,7 @@ static struct msm_platform_core_capability core_data_kalama[] = { {DEVICE_CAPS, V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_META_CAPTURE | V4L2_CAP_STREAMING}, {SUPPORTS_SYNX_FENCE, 0}, - {SUPPORTS_REQUESTS, 1}, + {SUPPORTS_REQUESTS, 0}, }; static struct msm_platform_inst_capability instance_cap_data_kalama[] = { diff --git a/driver/platform/pineapple/src/msm_vidc_pineapple.c b/driver/platform/pineapple/src/msm_vidc_pineapple.c index 423b64fd8b..0d9c54bbd8 100644 --- a/driver/platform/pineapple/src/msm_vidc_pineapple.c +++ b/driver/platform/pineapple/src/msm_vidc_pineapple.c @@ -323,7 +323,7 @@ static struct msm_platform_core_capability core_data_pineapple[] = { {DEVICE_CAPS, V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_META_CAPTURE | V4L2_CAP_STREAMING}, {SUPPORTS_SYNX_FENCE, 1}, - {SUPPORTS_REQUESTS, 1}, + {SUPPORTS_REQUESTS, 0}, }; static int msm_vidc_set_ring_buffer_count_pineapple(void *instance, diff --git a/driver/vidc/src/msm_vidc_v4l2.c b/driver/vidc/src/msm_vidc_v4l2.c index e5ba233307..7691bf5fdd 100644 --- a/driver/vidc/src/msm_vidc_v4l2.c +++ b/driver/vidc/src/msm_vidc_v4l2.c @@ -505,18 +505,33 @@ int msm_v4l2_qbuf(struct file *filp, void *fh, } /* - * do not acquire inst lock here. acquire it in msm_vb2_buf_queue. - * for requests, msm_vb2_buf_queue() is not called from here. - * instead it's called as part of msm_v4l2_request_queue(). - * hence acquire the inst lock in common function i.e - * msm_vb2_buf_queue, to handle both requests and non-request - * scenarios. + * [1] If request_fd enabled, msm_vb2_buf_queue() is not called from here. + * instead it's called as part of msm_v4l2_request_queue(). + * Hence inst lock should be acquired in common function i.e + * msm_vb2_buf_queue, to handle both requests and non-request + * scenarios. + * [2] If request_fd is disabled, inst_lock can be acquired here. + * Acquiring inst_lock from here will ensure RO list insertion + * and deletion i.e. attach/map will happen under lock. + * Currently, request_fd is disabled. Therefore, acquire inst_lock + * from this function to ensure RO list insertion/updation is under + * lock to avoid stability usecase. */ + client_lock(inst, __func__); + inst_lock(inst, __func__); + if (is_session_error(inst)) { + i_vpr_e(inst, "%s: inst in error state\n", __func__); + rc = -EINVAL; + goto exit; + } + rc = msm_vidc_qbuf(inst, vdev->v4l2_dev->mdev, b); if (rc) goto exit; exit: + inst_unlock(inst, __func__); + client_unlock(inst, __func__); put_inst(inst); return rc; @@ -560,11 +575,21 @@ int msm_v4l2_streamon(struct file *filp, void *fh, return -EINVAL; } + client_lock(inst, __func__); + inst_lock(inst, __func__); + if (is_session_error(inst)) { + i_vpr_e(inst, "%s: inst in error state\n", __func__); + rc = -EBUSY; + goto exit; + } + rc = msm_vidc_streamon((void *)inst, i); if (rc) goto exit; exit: + inst_unlock(inst, __func__); + client_unlock(inst, __func__); put_inst(inst); return rc; diff --git a/driver/vidc/src/msm_vidc_vb2.c b/driver/vidc/src/msm_vidc_vb2.c index 3be0fdacf7..2f3ed993c1 100644 --- a/driver/vidc/src/msm_vidc_vb2.c +++ b/driver/vidc/src/msm_vidc_vb2.c @@ -369,32 +369,19 @@ int msm_vb2_start_streaming(struct vb2_queue *q, unsigned int count) return -EINVAL; } inst = q->drv_priv; - inst = get_inst_ref(g_core, inst); if (!inst || !inst->core) { d_vpr_e("%s: invalid params\n", __func__); return -EINVAL; } - client_lock(inst, __func__); - inst_lock(inst, __func__); - if (is_session_error(inst)) { - i_vpr_e(inst, "%s: inst in error state\n", __func__); - rc = -EBUSY; - goto unlock; - } - rc = inst->event_handle(inst, MSM_VIDC_STREAMON, q); if (rc) { i_vpr_e(inst, "Streamon: %s failed\n", v4l2_type_name(q->type)); msm_vidc_change_state(inst, MSM_VIDC_ERROR, __func__); - goto unlock; + goto exit; } -unlock: - inst_unlock(inst, __func__); - client_unlock(inst, __func__); - put_inst(inst); - +exit: return rc; } @@ -577,6 +564,7 @@ void msm_vb2_buf_queue(struct vb2_buffer *vb2) { int rc = 0; struct msm_vidc_inst *inst; + struct msm_vidc_core *core; u64 timestamp_us = 0; u64 ktime_ns = ktime_get_ns(); @@ -586,16 +574,11 @@ void msm_vb2_buf_queue(struct vb2_buffer *vb2) } inst = vb2_get_drv_priv(vb2->vb2_queue); - if (!inst) { + if (!inst || !inst->core) { d_vpr_e("%s: invalid params\n", __func__); return; } - - inst = get_inst_ref(g_core, inst); - if (!inst) { - d_vpr_e("%s: invalid instance\n", __func__); - return; - } + core = inst->core; /* * As part of every qbuf initalise request to true. @@ -605,29 +588,24 @@ void msm_vb2_buf_queue(struct vb2_buffer *vb2) * If the buffer does not have any requests with it, then * v4l2_ctrl_request_setup() will return 0. */ - inst->request = true; - rc = v4l2_ctrl_request_setup(vb2->req_obj.req, - &inst->ctrl_handler); - inst->request = false; - v4l2_ctrl_request_complete(vb2->req_obj.req, &inst->ctrl_handler); - /* - * call request_setup and request_complete without acquiring lock - * to avoid deadlock issues because request_setup or request_complete - * would call .s_ctrl and .g_volatile_ctrl respectively which acquire - * lock too. - */ - client_lock(inst, __func__); - inst_lock(inst, __func__); - if (rc) { - i_vpr_e(inst, "%s: request setup failed, error %d\n", - __func__, rc); - goto unlock; - } - - if (is_session_error(inst)) { - i_vpr_e(inst, "%s: inst in error state\n", __func__); - rc = -EINVAL; - goto unlock; + if (core->capabilities[SUPPORTS_REQUESTS].value) { + /* + * If Request API is enabled: + * Call request_setup and request_complete without acquiring lock + * to avoid deadlock issues because request_setup or request_complete + * would call .s_ctrl and .g_volatile_ctrl respectively which acquire + * lock too. + */ + inst->request = true; + rc = v4l2_ctrl_request_setup(vb2->req_obj.req, + &inst->ctrl_handler); + inst->request = false; + v4l2_ctrl_request_complete(vb2->req_obj.req, &inst->ctrl_handler); + if (rc) { + i_vpr_e(inst, "%s: request setup failed, error %d\n", + __func__, rc); + goto exit; + } } if (!vb2->planes[0].bytesused) { @@ -636,7 +614,7 @@ void msm_vb2_buf_queue(struct vb2_buffer *vb2) i_vpr_e(inst, "%s: zero bytesused input buffer not supported\n", __func__); rc = -EINVAL; - goto unlock; + goto exit; } if ((vb2->type == OUTPUT_META_PLANE && is_any_meta_tx_out_enabled(inst)) || (vb2->type == INPUT_META_PLANE && is_any_meta_tx_inp_enabled(inst))) { @@ -659,7 +637,7 @@ void msm_vb2_buf_queue(struct vb2_buffer *vb2) if (vb2->type == INPUT_MPLANE) { rc = msm_vidc_update_input_rate(inst, div_u64(ktime_ns, 1000)); if (rc) - goto unlock; + goto exit; } if (is_decode_session(inst)) @@ -670,17 +648,14 @@ void msm_vb2_buf_queue(struct vb2_buffer *vb2) rc = -EINVAL; if (rc) { print_vb2_buffer("failed vb2-qbuf", inst, vb2); - goto unlock; + goto exit; } -unlock: +exit: if (rc) { msm_vidc_change_state(inst, MSM_VIDC_ERROR, __func__); vb2_buffer_done(vb2, VB2_BUF_STATE_ERROR); } - inst_unlock(inst, __func__); - client_unlock(inst, __func__); - put_inst(inst); } int msm_vb2_buf_out_validate(struct vb2_buffer *vb)