msm: camera: fd: Fix race condition in resetting ready_to_process

Do not unlock frame_req_mutex until the request is added
into processing list after submitting the frame to hw.
This can cause a race condition where Flush comes before
adding the frame into processing list and thinks there
is no frame with HW. And there is a possibility that
FrameDone IRQ thread can get the mutex and thinks no frame
in processing list - causing never to reset the flag
ready_to_process. This will cause all subsequent camera
usage to fail as requests will never be submitted to hw.

CRs-Fixed: 2600443
Change-Id: Icd12e155192fccd9adbc7b774b073ca623f1d7ba
Signed-off-by: Pavan Kumar Chilamkurthi <[email protected]>

drivers/cam_fd/fd_hw_mgr/cam_fd_hw_mgr.c

@@ -240,6 +240,15 @@ static int cam_fd_mgr_util_release_device(struct cam_fd_hw_mgr *hw_mgr,
 
 	mutex_lock(&hw_mgr->hw_mgr_mutex);
 	hw_device->num_ctxts--;
+
+	if (!hw_device->num_ctxts) {
+		mutex_lock(&hw_device->lock);
+		hw_device->ready_to_process = true;
+		hw_device->req_id = -1;
+		hw_device->cur_hw_ctx = NULL;
+		mutex_unlock(&hw_device->lock);
+	}
+
 	mutex_unlock(&hw_mgr->hw_mgr_mutex);
 
 	hw_ctx->device_index = -1;
@@ -277,6 +286,11 @@ static int cam_fd_mgr_util_select_device(struct cam_fd_hw_mgr *hw_mgr,
 			(!fd_acquire_args->get_raw_results ||
 			hw_device->hw_caps.raw_results_available)) {
 			CAM_DBG(CAM_FD, "Found dedicated HW Index=%d", i);
+			mutex_lock(&hw_device->lock);
+			hw_device->ready_to_process = true;
+			hw_device->req_id = -1;
+			hw_device->cur_hw_ctx = NULL;
+			mutex_unlock(&hw_device->lock);
 			hw_device->num_ctxts++;
 			break;
 		}
@@ -294,7 +308,9 @@ static int cam_fd_mgr_util_select_device(struct cam_fd_hw_mgr *hw_mgr,
 				(!fd_acquire_args->get_raw_results ||
 				hw_device->hw_caps.raw_results_available)) {
 				hw_device->num_ctxts++;
-				CAM_DBG(CAM_FD, "Found sharing HW Index=%d", i);
+				CAM_DBG(CAM_FD,
+					"Found sharing HW Index=%d, num_ctxts=%d",
+					i, hw_device->num_ctxts);
 				break;
 			}
 		}
@@ -846,6 +862,11 @@ static int cam_fd_mgr_util_submit_frame(void *priv, void *data)
 
 	mutex_lock(&hw_device->lock);
 	if (hw_device->ready_to_process == false) {
+		if (hw_mgr->num_pending_frames > 6) {
+			CAM_WARN(CAM_FD,
+				"Device busy for longer time with cur_hw_ctx=%pK, ReqId=%lld",
+				hw_device->cur_hw_ctx, hw_device->req_id);
+		}
 		mutex_unlock(&hw_device->lock);
 		mutex_unlock(&hw_mgr->frame_req_mutex);
 		return -EBUSY;
@@ -854,7 +875,8 @@ static int cam_fd_mgr_util_submit_frame(void *priv, void *data)
 	trace_cam_submit_to_hw("FD", frame_req->request_id);
 
 	list_del_init(&frame_req->list);
-	mutex_unlock(&hw_mgr->frame_req_mutex);
+	hw_mgr->num_pending_frames--;
+	list_add_tail(&frame_req->list, &hw_mgr->frame_processing_list);
 
 	if (hw_device->hw_intf->hw_ops.start) {
 		start_args.hw_ctx = hw_ctx;
@@ -869,12 +891,16 @@ static int cam_fd_mgr_util_submit_frame(void *priv, void *data)
 			sizeof(start_args));
 		if (rc) {
 			CAM_ERR(CAM_FD, "Failed in HW Start %d", rc);
+			list_del_init(&frame_req->list);
 			mutex_unlock(&hw_device->lock);
+			mutex_unlock(&hw_mgr->frame_req_mutex);
 			goto put_req_into_free_list;
 		}
 	} else {
 		CAM_ERR(CAM_FD, "Invalid hw_ops.start");
+		list_del_init(&frame_req->list);
 		mutex_unlock(&hw_device->lock);
+		mutex_unlock(&hw_mgr->frame_req_mutex);
 		rc = -EPERM;
 		goto put_req_into_free_list;
 	}
@@ -883,30 +909,9 @@ static int cam_fd_mgr_util_submit_frame(void *priv, void *data)
 	hw_device->cur_hw_ctx = hw_ctx;
 	hw_device->req_id = frame_req->request_id;
 	mutex_unlock(&hw_device->lock);
-	frame_req->submit_timestamp = ktime_get();
-
-	rc = cam_fd_mgr_util_put_frame_req(
-		&hw_mgr->frame_processing_list, &frame_req);
-	if (rc) {
-		CAM_ERR(CAM_FD,
-			"Failed in putting frame req in processing list");
-		goto stop_unlock;
-	}
+	mutex_unlock(&hw_mgr->frame_req_mutex);
 
 	return rc;
-
-stop_unlock:
-	if (hw_device->hw_intf->hw_ops.stop) {
-		struct cam_fd_hw_stop_args stop_args;
-
-		stop_args.hw_ctx = hw_ctx;
-		stop_args.ctx_hw_private = hw_ctx->ctx_hw_private;
-		stop_args.hw_req_private = &frame_req->hw_req_private;
-		if (hw_device->hw_intf->hw_ops.stop(
-			hw_device->hw_intf->hw_priv, &stop_args,
-			sizeof(stop_args)))
-			CAM_ERR(CAM_FD, "Failed in HW Stop %d", rc);
-	}
 put_req_into_free_list:
 	cam_fd_mgr_util_put_frame_req(&hw_mgr->frame_free_list, &frame_req);
 
@@ -1274,6 +1279,7 @@ static int cam_fd_mgr_hw_start(void *hw_mgr_priv, void *mgr_start_args)
 	if (hw_device->hw_intf->hw_ops.init) {
 		hw_init_args.hw_ctx = hw_ctx;
 		hw_init_args.ctx_hw_private = hw_ctx->ctx_hw_private;
+		hw_init_args.is_hw_reset = false;
 		if (fd_core->hw_static_info->enable_errata_wa.skip_reset)
 			hw_init_args.reset_required = false;
 		else
@@ -1286,6 +1292,14 @@ static int cam_fd_mgr_hw_start(void *hw_mgr_priv, void *mgr_start_args)
 			CAM_ERR(CAM_FD, "Failed in HW Init %d", rc);
 			return rc;
 		}
+
+		if (hw_init_args.is_hw_reset) {
+			mutex_lock(&hw_device->lock);
+			hw_device->ready_to_process = true;
+			hw_device->req_id = -1;
+			hw_device->cur_hw_ctx = NULL;
+			mutex_unlock(&hw_device->lock);
+		}
 	} else {
 		CAM_ERR(CAM_FD, "Invalid init function");
 		return -EINVAL;
@@ -1333,6 +1347,7 @@ static int cam_fd_mgr_hw_flush_req(void *hw_mgr_priv,
 			if (frame_req->request_id != flush_req->request_id)
 				continue;
 
+			hw_mgr->num_pending_frames--;
 			list_del_init(&frame_req->list);
 			break;
 		}
@@ -1345,6 +1360,7 @@ static int cam_fd_mgr_hw_flush_req(void *hw_mgr_priv,
 			if (frame_req->request_id != flush_req->request_id)
 				continue;
 
+			hw_mgr->num_pending_frames--;
 			list_del_init(&frame_req->list);
 			break;
 		}
@@ -1427,6 +1443,7 @@ static int cam_fd_mgr_hw_flush_ctx(void *hw_mgr_priv,
 		if (frame_req->hw_ctx != hw_ctx)
 			continue;
 
+		hw_mgr->num_pending_frames--;
 		list_del_init(&frame_req->list);
 	}
 
@@ -1435,6 +1452,7 @@ static int cam_fd_mgr_hw_flush_ctx(void *hw_mgr_priv,
 		if (frame_req->hw_ctx != hw_ctx)
 			continue;
 
+		hw_mgr->num_pending_frames--;
 		list_del_init(&frame_req->list);
 	}
 
@@ -1816,6 +1834,7 @@ static int cam_fd_mgr_hw_config(void *hw_mgr_priv, void *hw_config_args)
 	struct cam_fd_mgr_frame_request *frame_req;
 	int rc;
 	int i;
+	uint64_t req_id;
 
 	if (!hw_mgr || !config) {
 		CAM_ERR(CAM_FD, "Invalid arguments %pK %pK", hw_mgr, config);
@@ -1834,6 +1853,7 @@ static int cam_fd_mgr_hw_config(void *hw_mgr_priv, void *hw_config_args)
 	}
 
 	frame_req = config->priv;
+	req_id = frame_req->request_id;
 
 	trace_cam_apply_req("FD", frame_req->request_id);
 	CAM_DBG(CAM_FD, "FrameHWConfig : Frame[%lld]", frame_req->request_id);
@@ -1863,6 +1883,13 @@ static int cam_fd_mgr_hw_config(void *hw_mgr_priv, void *hw_config_args)
 		goto put_free_list;
 	}
 
+	mutex_lock(&g_fd_hw_mgr.frame_req_mutex);
+	hw_mgr->num_pending_frames++;
+	CAM_DBG(CAM_FD,
+		"Adding ctx[%pK] Req[%llu] : Total number of pending frames %d",
+		hw_ctx, req_id, hw_mgr->num_pending_frames);
+	mutex_unlock(&g_fd_hw_mgr.frame_req_mutex);
+
 	rc = cam_fd_mgr_util_schedule_frame_worker_task(hw_mgr);
 	if (rc) {
 		CAM_ERR(CAM_FD, "Worker task scheduling failed %d", rc);
@@ -1882,6 +1909,10 @@ remove_and_put_free_list:
 		cam_fd_mgr_util_get_frame_req(
 			&hw_mgr->frame_pending_list_normal, &frame_req);
 	}
+
+	mutex_lock(&g_fd_hw_mgr.frame_req_mutex);
+	hw_mgr->num_pending_frames--;
+	mutex_unlock(&g_fd_hw_mgr.frame_req_mutex);
 put_free_list:
 	cam_fd_mgr_util_put_frame_req(&hw_mgr->frame_free_list,
 		&frame_req);
@@ -1952,6 +1983,8 @@ int cam_fd_hw_mgr_init(struct device_node *of_node,
 		hw_device->valid = true;
 		hw_device->hw_intf = hw_intf;
 		hw_device->ready_to_process = true;
+		hw_device->req_id = -1;
+		hw_device->cur_hw_ctx = NULL;
 
 		if (hw_device->hw_intf->hw_ops.process_cmd) {
 			struct cam_fd_hw_cmd_set_irq_cb irq_cb_args;
@@ -2004,6 +2037,7 @@ int cam_fd_hw_mgr_init(struct device_node *of_node,
 	g_fd_hw_mgr.device_iommu.secure = -1;
 	g_fd_hw_mgr.cdm_iommu.non_secure = -1;
 	g_fd_hw_mgr.cdm_iommu.secure = -1;
+	g_fd_hw_mgr.num_pending_frames = 0;
 
 	rc = cam_smmu_get_handle("fd",
 		&g_fd_hw_mgr.device_iommu.non_secure);

drivers/cam_fd/fd_hw_mgr/cam_fd_hw_mgr.h

@@ -159,6 +159,8 @@ struct cam_fd_mgr_work_data {
  * @work                      : Worker handle
  * @work_data                 : Worker data
  * @fd_caps                   : FD driver capabilities
+ * @num_pending_frames        : Number of total frames pending for processing
+ *                              across contexts
  */
 struct cam_fd_hw_mgr {
 	struct list_head                   free_ctx_list;
@@ -182,6 +184,7 @@ struct cam_fd_hw_mgr {
 	struct cam_req_mgr_core_workq     *work;
 	struct cam_fd_mgr_work_data        *work_data;
 	struct cam_fd_query_cap_cmd        fd_caps;
+	uint32_t                           num_pending_frames;
 };
 
 #endif /* _CAM_FD_HW_MGR_H_ */

drivers/cam_fd/fd_hw_mgr/fd_hw/cam_fd_hw_core.c

@@ -753,6 +753,7 @@ int cam_fd_hw_init(void *hw_priv, void *init_hw_args, uint32_t arg_size)
 			CAM_ERR(CAM_FD, "Reset Failed, rc=%d", rc);
 			goto disable_soc;
 		}
+		init_args->is_hw_reset = true;
 	}
 
 	cam_fd_hw_util_enable_power_on_settings(fd_hw);

drivers/cam_fd/fd_hw_mgr/fd_hw/cam_fd_hw_intf.h

@@ -163,11 +163,13 @@ struct cam_fd_hw_release_args {
  * @hw_ctx         : HW context for which init is requested
  * @ctx_hw_private : HW layer's private information specific to this hw context
  * @reset_required : Indicates if the reset is required during init or not
+ * @is_hw_reset    : Output from hw layer, whether hw is reset on this init
  */
 struct cam_fd_hw_init_args {
 	void    *hw_ctx;
 	void    *ctx_hw_private;
 	bool     reset_required;
+	bool     is_hw_reset;
 };
 
 /**