msm: camera: sync: Enhance synx/dma fence drivers

The change addresses race conditions stemming from releasing
internal row lock prior to invoking synx APIs. The change
also deregisters dma/synx callbacks if the corresponding
fences are signaled when the associated sync object is signaled.
The change also cleans up & reorganizes functions.

CRs-Fixed: 3403341
Change-Id: I32995900e31ba73eac6b44d2238808f8e6c23f9a
Signed-off-by: Karthik Anantha Ram <[email protected]>

drivers/cam_sync/cam_sync.c

@@ -400,9 +400,9 @@ int cam_sync_signal(int32_t sync_obj, uint32_t status, uint32_t event_cause)
 {
 	struct sync_table_row *row = NULL;
 	struct list_head parents_list;
-	int rc = 0, synx_row_idx = 0;
-	uint32_t synx_obj = 0;
+	int rc = 0;
 #if IS_ENABLED(CONFIG_TARGET_SYNX_ENABLE)
+	uint32_t synx_row_idx;
 	struct cam_synx_obj_signal signal_synx_obj;
 #endif
 
@@ -441,11 +441,25 @@ int cam_sync_signal(int32_t sync_obj, uint32_t status, uint32_t event_cause)
 				row->dma_fence_info.dma_fence_fd, row->name, sync_obj);
 	}
 
-	/* Obtain associated synx hdl if any with the row lock held */
+#if IS_ENABLED(CONFIG_TARGET_SYNX_ENABLE)
+	/*
+	 * Signal associated synx obj prior to sync
+	 */
 	if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ, &row->ext_fence_mask)) {
-		synx_obj = row->synx_obj_info.synx_obj;
+		signal_synx_obj.status = status;
+		signal_synx_obj.synx_obj = row->synx_obj_info.synx_obj;
 		synx_row_idx = row->synx_obj_info.synx_obj_row_idx;
+
+		/* Release & obtain the row lock after synx signal */
+		spin_unlock_bh(&sync_dev->row_spinlocks[sync_obj]);
+		rc = cam_synx_obj_internal_signal(synx_row_idx, &signal_synx_obj);
+		spin_lock_bh(&sync_dev->row_spinlocks[sync_obj]);
+		if (rc)
+			CAM_ERR(CAM_SYNC,
+				"Error: Failed to signal associated synx obj = %d for sync_obj = %d",
+				signal_synx_obj.synx_obj, sync_obj);
 	}
+#endif
 
 	cam_sync_util_dispatch_signaled_cb(sync_obj, status, event_cause);
 
@@ -459,22 +473,6 @@ int cam_sync_signal(int32_t sync_obj, uint32_t status, uint32_t event_cause)
 			CAM_FENCE_OP_SIGNAL);
 
 	spin_unlock_bh(&sync_dev->row_spinlocks[sync_obj]);
-
-#if IS_ENABLED(CONFIG_TARGET_SYNX_ENABLE)
-	/*
-	 * Signal associated synx obj after unlock
-	 */
-	if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ, &row->ext_fence_mask)) {
-		signal_synx_obj.status = status;
-		signal_synx_obj.synx_obj = synx_obj;
-		rc = cam_synx_obj_internal_signal(synx_row_idx, &signal_synx_obj);
-		if (rc)
-			CAM_ERR(CAM_SYNC,
-				"Error: Failed to signal associated synx obj = %d for sync_obj = %d",
-				synx_obj, sync_obj);
-	}
-#endif
-
 	if (list_empty(&parents_list))
 		return 0;
 

drivers/cam_sync/cam_sync_dma_fence.c

@@ -54,7 +54,8 @@ const char *__cam_dma_fence_get_driver_name(
 void __cam_dma_fence_free(struct dma_fence *fence)
 {
 	CAM_DBG(CAM_DMA_FENCE,
-		"Free memory for dma fence seqno: %llu", fence->seqno);
+		"Free memory for dma fence context: %llu seqno: %llu",
+		fence->context, fence->seqno);
 	kfree(fence->lock);
 	kfree(fence);
 }
@@ -109,17 +110,18 @@ static void __cam_dma_fence_print_table(void)
 
 static int __cam_dma_fence_find_free_idx(uint32_t *idx)
 {
-	int rc = -ENOMEM;
+	int rc = 0;
+	bool bit;
 
-	/* Hold lock to obtain free index */
-	mutex_lock(&g_cam_dma_fence_dev->dev_lock);
+	do {
+		*idx = find_first_zero_bit(g_cam_dma_fence_dev->bitmap, CAM_DMA_FENCE_MAX_FENCES);
+		if (*idx >=  CAM_DMA_FENCE_MAX_FENCES) {
+			rc = -ENOMEM;
+			break;
+		}
 
-	*idx = find_first_zero_bit(g_cam_dma_fence_dev->bitmap, CAM_DMA_FENCE_MAX_FENCES);
-	if (*idx < CAM_DMA_FENCE_MAX_FENCES) {
-		set_bit(*idx, g_cam_dma_fence_dev->bitmap);
-		rc = 0;
-	}
-	mutex_unlock(&g_cam_dma_fence_dev->dev_lock);
+		bit = test_and_set_bit(*idx, g_cam_dma_fence_dev->bitmap);
+	} while (bit);
 
 	if (rc) {
 		CAM_ERR(CAM_DMA_FENCE, "No free idx, printing dma fence table......");
@@ -440,20 +442,28 @@ static int __cam_dma_fence_signal_fence(
 	struct dma_fence *dma_fence,
 	int32_t status)
 {
+	int rc;
+	unsigned long flags;
 	bool fence_signaled = false;
 
-	fence_signaled = dma_fence_is_signaled(dma_fence);
+	spin_lock_irqsave(dma_fence->lock, flags);
+	fence_signaled = dma_fence_is_signaled_locked(dma_fence);
 	if (fence_signaled) {
-		CAM_WARN(CAM_DMA_FENCE,
-			"dma fence seqno: %llu is already signaled",
-			dma_fence->seqno);
-		return 0;
+		rc = -EPERM;
+		goto end;
 	}
 
 	if (status)
 		dma_fence_set_error(dma_fence, status);
 
-	return dma_fence_signal(dma_fence);
+	rc = dma_fence_signal_locked(dma_fence);
+
+end:
+	spin_unlock_irqrestore(dma_fence->lock, flags);
+	if (fence_signaled)
+		CAM_DBG(CAM_DMA_FENCE, "dma fence seqno: %llu is already signaled",
+			dma_fence->seqno);
+	return rc;
 }
 
 int cam_dma_fence_internal_signal(
@@ -498,6 +508,16 @@ int cam_dma_fence_internal_signal(
 			&g_cam_dma_fence_dev->dev_lock, g_cam_dma_fence_dev->monitor_data,
 			CAM_FENCE_OP_SIGNAL);
 
+	if (row->cb_registered_for_sync) {
+		if (!dma_fence_remove_callback(row->fence, &row->fence_cb)) {
+			CAM_ERR(CAM_DMA_FENCE,
+				"Failed to remove cb for dma fence seqno: %llu fd: %d",
+				dma_fence->seqno, row->fd);
+			rc = -EINVAL;
+			goto monitor_dump;
+		}
+	}
+
 	rc = __cam_dma_fence_signal_fence(dma_fence, signal_dma_fence->status);
 	if (rc)
 		CAM_WARN(CAM_DMA_FENCE,
@@ -522,10 +542,8 @@ monitor_dump:
 
 int cam_dma_fence_signal_fd(struct cam_dma_fence_signal *signal_dma_fence)
 {
-	int rc;
 	uint32_t idx;
 	struct dma_fence *dma_fence = NULL;
-	struct cam_dma_fence_row *row = NULL;
 
 	dma_fence = __cam_dma_fence_find_fence_in_table(
 		signal_dma_fence->dma_fence_fd, &idx);
@@ -536,53 +554,7 @@ int cam_dma_fence_signal_fd(struct cam_dma_fence_signal *signal_dma_fence)
 		return -EINVAL;
 	}
 
-	spin_lock_bh(&g_cam_dma_fence_dev->row_spinlocks[idx]);
-	row = &g_cam_dma_fence_dev->rows[idx];
-	/*
-	 * Check for invalid state again, there could be a contention
-	 * between signal and release
-	 */
-	if (row->state == CAM_DMA_FENCE_STATE_INVALID) {
-		CAM_ERR(CAM_DMA_FENCE,
-			"dma fence fd: %d is invalid row_idx: %u, failed to signal",
-			signal_dma_fence->dma_fence_fd, idx);
-		rc = -EINVAL;
-		goto monitor_dump;
-	}
-
-	if (row->state == CAM_DMA_FENCE_STATE_SIGNALED) {
-		spin_unlock_bh(&g_cam_dma_fence_dev->row_spinlocks[idx]);
-		CAM_WARN(CAM_DMA_FENCE,
-			"dma fence fd: %d[seqno: %llu] already in signaled state",
-			signal_dma_fence->dma_fence_fd, dma_fence->seqno);
-		return 0;
-	}
-
-	if (test_bit(CAM_GENERIC_FENCE_TYPE_DMA_FENCE, &cam_sync_monitor_mask))
-		cam_generic_fence_update_monitor_array(idx,
-			&g_cam_dma_fence_dev->dev_lock, g_cam_dma_fence_dev->monitor_data,
-			CAM_FENCE_OP_SIGNAL);
-
-	rc = __cam_dma_fence_signal_fence(dma_fence, signal_dma_fence->status);
-	if (rc)
-		CAM_WARN(CAM_DMA_FENCE,
-			"dma fence seqno: %llu fd: %d already signaled rc: %d",
-			dma_fence->seqno, row->fd, rc);
-
-	row->state = CAM_DMA_FENCE_STATE_SIGNALED;
-	spin_unlock_bh(&g_cam_dma_fence_dev->row_spinlocks[idx]);
-
-	CAM_DBG(CAM_DMA_FENCE,
-		"dma fence fd: %d[seqno: %llu] signaled with status: %d rc: %d",
-		signal_dma_fence->dma_fence_fd, dma_fence->seqno,
-		signal_dma_fence->status, rc);
-
-	return rc;
-
-monitor_dump:
-	__cam_dma_fence_dump_monitor_array(idx);
-	spin_unlock_bh(&g_cam_dma_fence_dev->row_spinlocks[idx]);
-	return rc;
+	return cam_dma_fence_internal_signal(idx, signal_dma_fence);
 }
 
 static int __cam_dma_fence_get_fd(int32_t *row_idx,
@@ -872,6 +844,10 @@ int cam_dma_fence_driver_init(void)
 		spin_lock_init(&g_cam_dma_fence_dev->row_spinlocks[i]);
 
 	bitmap_zero(g_cam_dma_fence_dev->bitmap, CAM_DMA_FENCE_MAX_FENCES);
+
+	/* zero will be considered an invalid slot */
+	set_bit(0, g_cam_dma_fence_dev->bitmap);
+
 	g_cam_dma_fence_dev->dma_fence_context = dma_fence_context_alloc(1);
 
 	CAM_DBG(CAM_DMA_FENCE, "Camera DMA fence driver initialized");

drivers/cam_sync/cam_sync_synx.c

@@ -132,24 +132,156 @@ static void __cam_synx_obj_dump_monitor_array(int32_t row_idx)
 	cam_generic_fence_dump_monitor_array(&obj_info);
 }
 
+static void __cam_synx_obj_signal_cb(u32 h_synx, int status, void *data)
+{
+	struct cam_synx_obj_signal_sync_obj signal_sync_obj;
+	struct cam_synx_obj_row *synx_obj_row = NULL;
+	int32_t idx;
+
+	if (!data) {
+		CAM_ERR(CAM_SYNX,
+			"Invalid data passed to synx obj : %d callback function.",
+			synx_obj_row->synx_obj);
+		return;
+	}
+
+	synx_obj_row = (struct cam_synx_obj_row *)data;
+
+	/* If this synx obj is signaled by sync obj, skip cb */
+	if (synx_obj_row->sync_signal_synx)
+		return;
+
+	if (synx_obj_row->synx_obj != h_synx) {
+		CAM_ERR(CAM_SYNX,
+			"Synx obj: %d callback does not match synx obj: %d in sync table.",
+			h_synx, synx_obj_row->synx_obj);
+		return;
+	}
+
+	if (synx_obj_row->state == CAM_SYNX_OBJ_STATE_INVALID) {
+		CAM_ERR(CAM_SYNX,
+			"Synx obj :%d is in invalid state: %d",
+			synx_obj_row->synx_obj, synx_obj_row->state);
+		return;
+	}
+
+	CAM_DBG(CAM_SYNX, "Synx obj: %d signaled, signal sync obj: %d",
+		 synx_obj_row->synx_obj, synx_obj_row->sync_obj);
+
+	if ((synx_obj_row->cb_registered_for_sync) && (synx_obj_row->sync_cb)) {
+		signal_sync_obj.synx_obj = synx_obj_row->synx_obj;
+		switch (status) {
+		case SYNX_STATE_SIGNALED_SUCCESS:
+			signal_sync_obj.status = CAM_SYNC_STATE_SIGNALED_SUCCESS;
+			break;
+		case SYNX_STATE_SIGNALED_CANCEL:
+			signal_sync_obj.status = CAM_SYNC_STATE_SIGNALED_CANCEL;
+			break;
+		default:
+			CAM_WARN(CAM_SYNX,
+				"Synx signal status %d is neither SUCCESS nor CANCEL, custom code?",
+				status);
+			signal_sync_obj.status = CAM_SYNC_STATE_SIGNALED_ERROR;
+			break;
+		}
+		synx_obj_row->state = CAM_SYNX_OBJ_STATE_SIGNALED;
+		synx_obj_row->sync_cb(synx_obj_row->sync_obj, &signal_sync_obj);
+		if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ,
+			&cam_sync_monitor_mask)) {
+			cam_synx_obj_find_obj_in_table(synx_obj_row->synx_obj, &idx);
+			cam_generic_fence_update_monitor_array(idx,
+				&g_cam_synx_obj_dev->dev_lock, g_cam_synx_obj_dev->monitor_data,
+				CAM_FENCE_OP_UNREGISTER_ON_SIGNAL);
+		}
+	}
+
+}
+
+/*
+ * Synx APIs need to be invoked in non atomic context,
+ * all these utils invoke synx driver
+ */
+static inline int __cam_synx_signal_util(
+	uint32_t synx_hdl, uint32_t signal_status)
+{
+	return synx_signal(g_cam_synx_obj_dev->session_handle, synx_hdl, signal_status);
+}
+
+static inline int __cam_synx_deregister_cb_util(
+	uint32_t synx_hdl, void *data)
+{
+	struct synx_callback_params cb_params;
+
+	cb_params.userdata = data;
+	cb_params.cancel_cb_func = NULL;
+	cb_params.h_synx = synx_hdl;
+	cb_params.cb_func = __cam_synx_obj_signal_cb;
+
+	return synx_cancel_async_wait(g_cam_synx_obj_dev->session_handle, &cb_params);
+}
+
+static inline int __cam_synx_create_hdl_util(
+	struct synx_create_params *params)
+{
+	return synx_create(g_cam_synx_obj_dev->session_handle, params);
+}
+
+static inline int __cam_synx_release_hdl_util(uint32_t synx_hdl)
+{
+	return synx_release(g_cam_synx_obj_dev->session_handle, synx_hdl);
+}
+
+static inline int __cam_synx_import_hdl_util(
+	struct synx_import_params *params)
+{
+	return synx_import(g_cam_synx_obj_dev->session_handle, params);
+}
+
+static inline int __cam_synx_register_cb_util(
+	struct synx_callback_params *cb_params)
+{
+	return synx_async_wait(g_cam_synx_obj_dev->session_handle, cb_params);
+}
+
 static int __cam_synx_obj_release(int32_t row_idx)
 {
+	int rc;
+	bool deregister_cb = false;
+	uint32_t synx_hdl = 0;
 	struct cam_synx_obj_row *row = NULL;
 
 	spin_lock_bh(&g_cam_synx_obj_dev->row_spinlocks[row_idx]);
 	row = &g_cam_synx_obj_dev->rows[row_idx];
+	synx_hdl = row->synx_obj;
 
 	if (row->state == CAM_SYNX_OBJ_STATE_ACTIVE) {
-		CAM_WARN(CAM_SYNX,
+		CAM_DBG(CAM_SYNX,
 			"Unsignaled synx obj being released name: %s synx_obj:%d",
 			row->name, row->synx_obj);
-		if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ,
-			&cam_sync_monitor_mask))
+
+		if (row->cb_registered_for_sync) {
+			if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ, &cam_sync_monitor_mask))
+				cam_generic_fence_update_monitor_array(row_idx,
+					&g_cam_synx_obj_dev->dev_lock,
+					g_cam_synx_obj_dev->monitor_data,
+					CAM_FENCE_OP_UNREGISTER_ON_SIGNAL);
+			deregister_cb = true;
+		}
+
+		if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ, &cam_sync_monitor_mask))
 			cam_generic_fence_update_monitor_array(row_idx,
 				&g_cam_synx_obj_dev->dev_lock, g_cam_synx_obj_dev->monitor_data,
 				CAM_FENCE_OP_SIGNAL);
-		synx_signal(g_cam_synx_obj_dev->session_handle, row->synx_obj,
-			SYNX_STATE_SIGNALED_CANCEL);
+
+		if (deregister_cb) {
+			spin_unlock_bh(&g_cam_synx_obj_dev->row_spinlocks[row_idx]);
+			rc = __cam_synx_deregister_cb_util(synx_hdl, row);
+			spin_lock_bh(&g_cam_synx_obj_dev->row_spinlocks[row_idx]);
+			if (rc)
+				CAM_DBG(CAM_SYNX,
+					"Failed to deregister cb for synx hdl: %u rc: %d",
+					synx_hdl, rc);
+		}
 	}
 
 	if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ, &cam_sync_monitor_mask)) {
@@ -164,34 +296,32 @@ static int __cam_synx_obj_release(int32_t row_idx)
 	}
 
 	CAM_DBG(CAM_SYNX,
-		"Releasing synx_obj: %d[%s] row_idx: %u",
-		row->synx_obj, row->name, row_idx);
-
-	synx_release(g_cam_synx_obj_dev->session_handle, row->synx_obj);
+		"Releasing synx_obj: %d[%s] row_idx: %u", row->synx_obj, row->name, row_idx);
 
 	/* deinit row */
 	memset(row, 0, sizeof(struct cam_synx_obj_row));
 	clear_bit(row_idx, g_cam_synx_obj_dev->bitmap);
 	spin_unlock_bh(&g_cam_synx_obj_dev->row_spinlocks[row_idx]);
-	return 0;
+
+	return __cam_synx_release_hdl_util(synx_hdl);
 }
 
 static int __cam_synx_obj_find_free_idx(uint32_t *idx)
 {
-	int rc = -ENOMEM;
-
-	/* Hold lock to obtain free index */
-	mutex_lock(&g_cam_synx_obj_dev->dev_lock);
-
-	*idx = find_first_zero_bit(g_cam_synx_obj_dev->bitmap, CAM_SYNX_MAX_OBJS);
-	if (*idx < CAM_SYNX_MAX_OBJS) {
-		set_bit(*idx, g_cam_synx_obj_dev->bitmap);
-		rc = 0;
-	}
-	mutex_unlock(&g_cam_synx_obj_dev->dev_lock);
+	int rc = 0;
+	bool bit;
+
+	do {
+		*idx = find_first_zero_bit(g_cam_synx_obj_dev->bitmap, CAM_SYNX_MAX_OBJS);
+		if (*idx >= CAM_SYNX_MAX_OBJS) {
+			CAM_ERR(CAM_SYNC,
+				"Error: Unable to create synx, no free index");
+			rc = -ENOMEM;
+			break;
+		}
 
-	if (rc)
-		CAM_ERR(CAM_SYNX, "No free synx idx");
+		bit = test_and_set_bit(*idx, g_cam_synx_obj_dev->bitmap);
+	} while (bit);
 
 	return rc;
 }
@@ -225,71 +355,6 @@ static int __cam_synx_obj_release_row(int32_t row_idx)
 	return __cam_synx_obj_release(row_idx);
 }
 
-static void __cam_synx_obj_signal_cb(u32 h_synx, int status, void *data)
-{
-	struct cam_synx_obj_signal_sync_obj signal_sync_obj;
-	struct cam_synx_obj_row *synx_obj_row = NULL;
-	int32_t idx;
-
-	if (!data) {
-		CAM_ERR(CAM_SYNX,
-			"Invalid data passed to synx obj : %d callback function.",
-			synx_obj_row->synx_obj);
-		return;
-	}
-
-	synx_obj_row = (struct cam_synx_obj_row *)data;
-
-	/* If this synx obj is signaled by sync obj, skip cb */
-	if (synx_obj_row->sync_signal_synx)
-		return;
-
-	if (synx_obj_row->synx_obj != h_synx) {
-		CAM_ERR(CAM_SYNX,
-			"Synx obj: %d callback does not match synx obj: %d in sync table.",
-			h_synx, synx_obj_row->synx_obj);
-		return;
-	}
-
-	if (synx_obj_row->state == CAM_SYNX_OBJ_STATE_INVALID) {
-		CAM_ERR(CAM_SYNX,
-			"Synx obj :%d is in invalid state: %d",
-			synx_obj_row->synx_obj, synx_obj_row->state);
-		return;
-	}
-
-	CAM_DBG(CAM_SYNX, "Synx obj: %d signaled, signal sync obj: %d",
-		 synx_obj_row->synx_obj, synx_obj_row->sync_obj);
-
-	if ((synx_obj_row->cb_registered_for_sync) && (synx_obj_row->sync_cb)) {
-		signal_sync_obj.synx_obj = synx_obj_row->synx_obj;
-		switch (status) {
-		case SYNX_STATE_SIGNALED_SUCCESS:
-			signal_sync_obj.status = CAM_SYNC_STATE_SIGNALED_SUCCESS;
-			break;
-		case SYNX_STATE_SIGNALED_CANCEL:
-			signal_sync_obj.status = CAM_SYNC_STATE_SIGNALED_CANCEL;
-			break;
-		default:
-			CAM_WARN(CAM_SYNX,
-				"Synx signal status %d is neither SUCCESS nor CANCEL, custom code?",
-				status);
-			signal_sync_obj.status = CAM_SYNC_STATE_SIGNALED_ERROR;
-			break;
-		}
-		synx_obj_row->state = CAM_SYNX_OBJ_STATE_SIGNALED;
-		synx_obj_row->sync_cb(synx_obj_row->sync_obj, &signal_sync_obj);
-		if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ,
-			&cam_sync_monitor_mask)) {
-			cam_synx_obj_find_obj_in_table(synx_obj_row->synx_obj, &idx);
-			cam_generic_fence_update_monitor_array(idx,
-				&g_cam_synx_obj_dev->dev_lock, g_cam_synx_obj_dev->monitor_data,
-				CAM_FENCE_OP_UNREGISTER_ON_SIGNAL);
-		}
-	}
-
-}
-
 int cam_synx_obj_find_obj_in_table(uint32_t synx_obj, int32_t *idx)
 {
 	int i, rc = -EINVAL;
@@ -330,7 +395,7 @@ static int __cam_synx_obj_import(const char *name,
 	if (__cam_synx_obj_find_free_idx(&idx))
 		goto end;
 
-	rc = synx_import(g_cam_synx_obj_dev->session_handle, params);
+	rc = __cam_synx_import_hdl_util(params);
 	if (rc) {
 		CAM_ERR(CAM_SYNX, "Synx import failed for fence : %p",
 			params->indv.fence);
@@ -406,9 +471,9 @@ int cam_synx_obj_create(const char *name, uint32_t flags, uint32_t *synx_obj,
 		goto free_idx;
 	}
 
-	rc = synx_create(g_cam_synx_obj_dev->session_handle, &params);
+	rc = __cam_synx_create_hdl_util(&params);
 	if (rc) {
-		CAM_ERR(CAM_SYNX, "Failed to create synx obj");
+		CAM_ERR(CAM_SYNX, "Failed to create new synx handle rc: %d", rc);
 		goto free_idx;
 	}
 
@@ -456,7 +521,8 @@ int cam_synx_obj_internal_signal(int32_t row_idx,
 	struct cam_synx_obj_signal *signal_synx_obj)
 {
 	int rc;
-	uint32_t signal_status, synx_obj = 0;
+	uint32_t signal_status;
+	bool deregister_cb = false;
 	struct cam_synx_obj_row *row = NULL;
 
 	if ((row_idx < 0) || (row_idx >= CAM_SYNX_MAX_OBJS)) {
@@ -470,37 +536,59 @@ int cam_synx_obj_internal_signal(int32_t row_idx,
 
 	/* Ensures sync obj cb is not invoked */
 	row->sync_signal_synx = true;
-	synx_obj = row->synx_obj;
 
-	if (row->state == CAM_SYNX_OBJ_STATE_SIGNALED) {
+	if (row->state != CAM_SYNX_OBJ_STATE_ACTIVE) {
+		CAM_ERR(CAM_SYNX, "synx obj: %u not in right state: %d to signal",
+			signal_synx_obj->synx_obj, row->state);
+		__cam_synx_obj_dump_monitor_array(row_idx);
+		spin_unlock_bh(&g_cam_synx_obj_dev->row_spinlocks[row_idx]);
+		return -EINVAL;
+	}
+
+	if (row->synx_obj != signal_synx_obj->synx_obj) {
+		CAM_WARN(CAM_SYNX,
+			"Trying to signal synx obj: %u in row: %u having a different synx obj: %u",
+			signal_synx_obj->synx_obj, row_idx, row->synx_obj);
+		__cam_synx_obj_dump_monitor_array(row_idx);
 		spin_unlock_bh(&g_cam_synx_obj_dev->row_spinlocks[row_idx]);
-		CAM_WARN(CAM_SYNX, "synx obj fd: %d already in signaled state",
-			signal_synx_obj->synx_obj);
 		return 0;
 	}
 
-	rc = __cam_synx_obj_map_sync_status_util(signal_synx_obj->status,
-		&signal_status);
+	rc = __cam_synx_obj_map_sync_status_util(signal_synx_obj->status, &signal_status);
 	if (rc) {
 		CAM_WARN(CAM_SYNX,
 			"Signaling undefined status: %d for synx obj: %d",
-			signal_synx_obj->status,
-			signal_synx_obj->synx_obj);
+			signal_synx_obj->status, signal_synx_obj->synx_obj);
 	}
 
-	row->state = CAM_SYNX_OBJ_STATE_SIGNALED;
+	if (row->cb_registered_for_sync) {
+		if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ, &cam_sync_monitor_mask))
+			cam_generic_fence_update_monitor_array(row_idx,
+				&g_cam_synx_obj_dev->dev_lock, g_cam_synx_obj_dev->monitor_data,
+				CAM_FENCE_OP_UNREGISTER_ON_SIGNAL);
+		deregister_cb = true;
+	}
 
 	if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ, &cam_sync_monitor_mask))
 		cam_generic_fence_update_monitor_array(row_idx,
 			&g_cam_synx_obj_dev->dev_lock, g_cam_synx_obj_dev->monitor_data,
 			CAM_FENCE_OP_SIGNAL);
+
 	spin_unlock_bh(&g_cam_synx_obj_dev->row_spinlocks[row_idx]);
 
-	rc = synx_signal(g_cam_synx_obj_dev->session_handle,
-		signal_synx_obj->synx_obj, signal_status);
+	if (deregister_cb) {
+		rc = __cam_synx_deregister_cb_util(signal_synx_obj->synx_obj, row);
+		if (rc) {
+			CAM_ERR(CAM_SYNX, "Failed to deregister cb for synx: %u rc: %d",
+				signal_synx_obj->synx_obj, rc);
+			goto end;
+		}
+	}
+
+	rc = __cam_synx_signal_util(signal_synx_obj->synx_obj, signal_status);
 	if (rc) {
-		CAM_WARN(CAM_SYNX, "synx obj: %d already signaled rc: %d",
-			synx_obj, rc);
+		CAM_ERR(CAM_SYNX, "Failed to signal synx hdl: %u with status: %u rc: %d",
+			signal_synx_obj->synx_obj, signal_status, rc);
 		goto end;
 	}
 
@@ -531,57 +619,15 @@ int cam_synx_obj_release(struct cam_synx_obj_release_params *release_params)
 int cam_synx_obj_signal_obj(struct cam_synx_obj_signal *signal_synx_obj)
 {
 	int rc;
-	uint32_t idx, signal_status = 0, synx_obj = 0;
-	struct cam_synx_obj_row *row = NULL;
-
-	rc = cam_synx_obj_find_obj_in_table(
-		signal_synx_obj->synx_obj, &idx);
+	uint32_t idx;
 
+	rc = cam_synx_obj_find_obj_in_table(signal_synx_obj->synx_obj, &idx);
 	if (rc) {
-		CAM_ERR(CAM_SYNX, "Failed to find synx obj: %d",
-			signal_synx_obj->synx_obj);
+		CAM_ERR(CAM_SYNX, "Failed to find synx obj: %u", signal_synx_obj->synx_obj);
 		return -EINVAL;
 	}
 
-	spin_lock_bh(&g_cam_synx_obj_dev->row_spinlocks[idx]);
-	row = &g_cam_synx_obj_dev->rows[idx];
-	synx_obj = row->synx_obj;
-	if (row->state == CAM_SYNX_OBJ_STATE_SIGNALED) {
-		spin_unlock_bh(&g_cam_synx_obj_dev->row_spinlocks[idx]);
-		CAM_WARN(CAM_SYNX, "synx obj: %d already in signaled state",
-			signal_synx_obj->synx_obj);
-		return 0;
-	}
-
-	rc = __cam_synx_obj_map_sync_status_util(signal_synx_obj->status,
-		&signal_status);
-	if (rc) {
-		CAM_WARN(CAM_SYNX,
-			"Signaling undefined sync status: %d for synx obj: %d",
-			signal_synx_obj->status,
-			signal_synx_obj->synx_obj);
-	}
-	row->state = CAM_SYNX_OBJ_STATE_SIGNALED;
-
-	if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ, &cam_sync_monitor_mask))
-		cam_generic_fence_update_monitor_array(idx,
-			&g_cam_synx_obj_dev->dev_lock, g_cam_synx_obj_dev->monitor_data,
-			CAM_FENCE_OP_SIGNAL);
-	spin_unlock_bh(&g_cam_synx_obj_dev->row_spinlocks[idx]);
-
-	rc = synx_signal(g_cam_synx_obj_dev->session_handle,
-		signal_synx_obj->synx_obj, signal_status);
-	if (rc) {
-		CAM_WARN(CAM_SYNX, "synx obj: %d already signaled rc: %d",
-			synx_obj, rc);
-		goto end;
-	}
-
-	CAM_DBG(CAM_SYNX, "synx obj: %d signaled with status: %d rc: %d",
-		signal_synx_obj->synx_obj, signal_status, rc);
-
-end:
-	return rc;
+	return cam_synx_obj_internal_signal(idx, signal_synx_obj);
 }
 
 int cam_synx_obj_register_cb(int32_t *sync_obj, int32_t row_idx,
@@ -649,13 +695,13 @@ int cam_synx_obj_register_cb(int32_t *sync_obj, int32_t row_idx,
 		cam_generic_fence_update_monitor_array(row_idx,
 			&g_cam_synx_obj_dev->dev_lock, g_cam_synx_obj_dev->monitor_data,
 			CAM_FENCE_OP_REGISTER_CB);
+
 	spin_unlock_bh(&g_cam_synx_obj_dev->row_spinlocks[row_idx]);
 
-	rc = synx_async_wait(g_cam_synx_obj_dev->session_handle, &cb_params);
+	rc = __cam_synx_register_cb_util(&cb_params);
 	if (rc) {
 		CAM_ERR(CAM_SYNX,
-			"Failed to register cb for synx obj: %d rc: %d",
-			synx_obj, rc);
+			"Failed to register cb for synx obj: %d rc: %d", synx_obj, rc);
 		return rc;
 	}
 
@@ -712,9 +758,8 @@ void cam_synx_obj_open(void)
 
 void cam_synx_obj_close(void)
 {
-	int i, rc;
+	int i;
 	struct cam_synx_obj_row *row = NULL;
-	struct synx_callback_params cb_params;
 
 	mutex_lock(&g_cam_synx_obj_dev->dev_lock);
 	for (i = 0; i < CAM_SYNX_MAX_OBJS; i++) {
@@ -728,25 +773,14 @@ void cam_synx_obj_close(void)
 
 		/* If registered for cb, remove cb */
 		if (row->cb_registered_for_sync) {
-			cb_params.userdata = row;
-			cb_params.cancel_cb_func = NULL;
-			cb_params.h_synx = row->synx_obj;
-			cb_params.cb_func = __cam_synx_obj_signal_cb;
-
 			if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ,
 				&cam_sync_monitor_mask))
 				cam_generic_fence_update_monitor_array(i,
 					&g_cam_synx_obj_dev->dev_lock,
 					g_cam_synx_obj_dev->monitor_data,
 					CAM_FENCE_OP_UNREGISTER_CB);
-			rc = synx_cancel_async_wait(
-				g_cam_synx_obj_dev->session_handle,
-				&cb_params);
-			if (rc) {
-				CAM_WARN(CAM_SYNX,
-				"Registered callback could not be canceled for synx obj : %d",
-				cb_params.h_synx);
-			}
+
+			__cam_synx_deregister_cb_util(row->synx_obj, row);
 		}
 
 		/* Signal and release the synx obj */
@@ -757,8 +791,8 @@ void cam_synx_obj_close(void)
 					&g_cam_synx_obj_dev->dev_lock,
 					g_cam_synx_obj_dev->monitor_data,
 					CAM_FENCE_OP_SIGNAL);
-			synx_signal(g_cam_synx_obj_dev->session_handle,
-				row->synx_obj, SYNX_STATE_SIGNALED_CANCEL);
+
+			__cam_synx_signal_util(row->synx_obj, SYNX_STATE_SIGNALED_CANCEL);
 		}
 
 		if (test_bit(CAM_GENERIC_FENCE_TYPE_SYNX_OBJ,
@@ -767,9 +801,8 @@ void cam_synx_obj_close(void)
 				&g_cam_synx_obj_dev->dev_lock,
 				g_cam_synx_obj_dev->monitor_data,
 				CAM_FENCE_OP_DESTROY);
-		synx_release(g_cam_synx_obj_dev->session_handle,
-			row->synx_obj);
 
+		__cam_synx_release_hdl_util(row->synx_obj);
 		memset(row, 0, sizeof(struct cam_synx_obj_row));
 		clear_bit(i, g_cam_synx_obj_dev->bitmap);
 	}
@@ -803,6 +836,9 @@ int cam_synx_obj_driver_init(void)
 	memset(&g_cam_synx_obj_dev->bitmap, 0, sizeof(g_cam_synx_obj_dev->bitmap));
 	bitmap_zero(g_cam_synx_obj_dev->bitmap, CAM_SYNX_MAX_OBJS);
 
+	/* zero will be considered an invalid slot */
+	set_bit(0, g_cam_synx_obj_dev->bitmap);
+
 	CAM_DBG(CAM_SYNX, "Camera synx obj driver initialized");
 	return 0;