Video: Driver: Move inst_lock to v4l2 layer during q_buf

[1] If request_fd enabled, msm_vb2_buf_queue() is not called
  from v4l2_qbuf. 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 in
  v4l2_qbuf() call. Acquiring inst_lock from here will
  ensure RO list insertion and updation 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.

Change-Id: I3cc1d4b8b5547fd5e34ce5eb06480380cb9200cc
Signed-off-by: Vedang Nagar <quic_vnagar@quicinc.com>

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[] = {

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,

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;

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)