[PATCH 0/2] Fix for CVS getting stuck on fast subsequent capture

[PATCH 0/2] Fix for CVS getting stuck on fast subsequent capture

[Antti Laakso]

The CVS may get stuck if sensor stops streaming mid-frame. To recover
CVS the CSI ownership need to be transferred to CVS and back to host.
This is done in CVS runtime pm suspend/resume, but it is prone to timing
related errors.

Fix is to change CSI link ownership in stream enable/disable.

The problem is reproduced and fix tested on Dell Pro13.

Antti Laakso (2):
  media: cvs: Hand CSI link to CVS on stream stop
  media: cvs: Remove runtime PM support

 drivers/media/i2c/cvs/core.c | 65 +++---------------------------------
 drivers/media/i2c/cvs/icvs.h |  2 +-
 drivers/media/i2c/cvs/v4l2.c | 40 +++++++++++-----------
 3 files changed, 24 insertions(+), 83 deletions(-)

-- 
2.55.0

[PATCH 1/2] media: cvs: Hand CSI link to CVS on stream stop

[Antti Laakso]

The CVS may get stuck if sensor is being stopped in the middle of a
frame. The CVS recovers by handing the CSI link to CVS and back to host.
This is done with runtime suspend/resume, but only after autosuspend
delay. Removing autosuspend delay is not enough, as suspend depends on
IPU which may still be active.

Fix the issue by moving CVS link handing to enable/disable streams.

Fixes: 8e2b43d2c10b ("media: i2c: cvs: Add driver of Intel Computer Vision Sensing Controller(CVS)")
Signed-off-by: Antti Laakso <antti.laakso@linux.intel.com>
---
 drivers/media/i2c/cvs/v4l2.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/cvs/v4l2.c b/drivers/media/i2c/cvs/v4l2.c
index 776fe9460168..5a9d7caaee3f 100644
--- a/drivers/media/i2c/cvs/v4l2.c
+++ b/drivers/media/i2c/cvs/v4l2.c
@@ -83,7 +83,13 @@ static int cvs_csi_enable_streams(struct v4l2_subdev *sd,
 	s64 link_freq;
 	int ret;
 
-	/* cvs_set_link_owner(ICVS_CSI_LINK_HOST) */
+	ret = cvs_set_link_owner(ctx, ICVS_CSI_LINK_HOST);
+	if (ret) {
+		dev_err(dev, "failed to hand over CSI link to host: %d\n",
+			ret);
+		return ret;
+	}
+
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		return ret;
@@ -125,7 +131,7 @@ static int cvs_csi_disable_streams(struct v4l2_subdev *sd,
 	struct v4l2_subdev *remote_sd =
 			    media_entity_to_v4l2_subdev(ctx->remote->entity);
 	struct device *dev = cvs_dev(ctx);
-	int ret;
+	int ret, link_ret;
 
 	ret = v4l2_subdev_disable_streams(remote_sd,
 					  ctx->remote->index,
@@ -133,10 +139,14 @@ static int cvs_csi_disable_streams(struct v4l2_subdev *sd,
 	if (ret)
 		dev_err(dev, "disable streams failed: %d\n", ret);
 
-	/* cvs_set_link_owner(ICVS_CSI_LINK_CVS) */
+	link_ret = cvs_set_link_owner(ctx, ICVS_CSI_LINK_CVS);
+	if (link_ret)
+		dev_err(dev, "failed to hand over CSI link to CVS: %d\n",
+			link_ret);
+
 	pm_runtime_put_autosuspend(dev);
 
-	return ret;
+	return ret ? : link_ret;
 }
 
 /*
-- 
2.55.0

[PATCH 2/2] media: cvs: Remove runtime PM support

[Antti Laakso]

The CSI link is handed to the host on stream enable and back to the
CVS on stream disable. The runtime PM is doing the same, and is now
unnecessary. Remove runtime PM.

Signed-off-by: Antti Laakso <antti.laakso@linux.intel.com>
---
 drivers/media/i2c/cvs/core.c | 65 +++---------------------------------
 drivers/media/i2c/cvs/icvs.h |  2 +-
 drivers/media/i2c/cvs/v4l2.c | 30 +++++------------
 3 files changed, 14 insertions(+), 83 deletions(-)

diff --git a/drivers/media/i2c/cvs/core.c b/drivers/media/i2c/cvs/core.c
index 489a5ce7bd1d..0cd5f747eaeb 100644
--- a/drivers/media/i2c/cvs/core.c
+++ b/drivers/media/i2c/cvs/core.c
@@ -15,7 +15,6 @@
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
-#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/time64.h>
 #include <linux/workqueue.h>
@@ -603,9 +602,6 @@ int cvs_send(struct icvs *ctx, struct icvs_cmd *cmd, size_t len)
  * @ctx: CVS device context
  * @owner: Desired owner (ICVS_CSI_LINK_HOST or ICVS_CSI_LINK_CVS)
  *
- * Called from runtime PM callbacks to claim or release the CSI-2 link.
- * Also callable directly for error recovery paths.
- *
  * Return: 0 on success or negative errno.
  */
 int cvs_set_link_owner(struct icvs *ctx, enum icvs_csi_link_owner owner)
@@ -800,21 +796,12 @@ static int cvs_core_probe(struct device *dev, struct i2c_client *i2c)
 	}
 
 	dev_set_drvdata(dev, ctx);
-	pm_runtime_set_autosuspend_delay(dev, 1000);
-	pm_runtime_use_autosuspend(dev);
-	pm_runtime_enable(dev);
-	pm_runtime_idle(dev);
 
 	/*
-	 * Create a PM runtime device link with IPU as consumer and CVS as
-	 * supplier. When the IPU runtime-resumes to start streaming, the PM
-	 * framework automatically resumes CVS first, triggering
-	 * cvs_runtime_resume() which hands CSI-2 link ownership to the host.
+	 * Create a device link with IPU as consumer and CVS as supplier so
+	 * that CVS suspends after and resumes before the IPU.
 	 */
-	ctx->ipu_link = device_link_add(&ipu->dev, dev,
-					DL_FLAG_PM_RUNTIME |
-					DL_FLAG_RPM_ACTIVE |
-					DL_FLAG_STATELESS);
+	ctx->ipu_link = device_link_add(&ipu->dev, dev, DL_FLAG_STATELESS);
 	if (!ctx->ipu_link) {
 		dev_err(dev, "IPU device link failed\n");
 		ret = -ENODEV;
@@ -829,12 +816,7 @@ static int cvs_core_probe(struct device *dev, struct i2c_client *i2c)
 	return 0;
 
 err_csi_remove:
-	if (ctx->ipu_link)
-		device_link_del(ctx->ipu_link);
 	cvs_csi_remove(ctx);
-	pm_runtime_dont_use_autosuspend(dev);
-	pm_runtime_disable(dev);
-	pm_runtime_set_suspended(dev);
 
 err_put_ipu:
 	put_device(&ipu->dev);
@@ -864,12 +846,7 @@ static void cvs_core_remove(struct device *dev)
 	cancel_delayed_work_sync(&ctx->work);
 	cvs_csi_remove(ctx);
 
-	if (ctx->ipu_link)
-		device_link_del(ctx->ipu_link);
-
-	pm_runtime_put_noidle(dev);
-	pm_runtime_disable(dev);
-	pm_runtime_set_suspended(dev);
+	device_link_del(ctx->ipu_link);
 
 	cvs_reset(ctx);
 }
@@ -921,42 +898,8 @@ static int __maybe_unused cvs_resume(struct device *dev)
 	return 0;
 }
 
-/**
- * cvs_runtime_resume - Runtime PM resume: claim CSI-2 link ownership
- * @dev: Device
- *
- * Triggered automatically when the IPU (consumer) runtime-resumes, because
- * a DL_FLAG_PM_RUNTIME device link makes CVS the supplier. Transfers CSI-2
- * link ownership to the host so the IPU can start receiving sensor frames.
- *
- * Return: 0 on success or negative errno.
- */
-static int __maybe_unused cvs_runtime_resume(struct device *dev)
-{
-	struct icvs *ctx = dev_get_drvdata(dev);
-
-	return cvs_set_link_owner(ctx, ICVS_CSI_LINK_HOST);
-}
-
-/**
- * cvs_runtime_suspend - Runtime PM suspend: release CSI-2 link ownership
- * @dev: Device
- *
- * Called when the streaming reference is dropped by cvs_csi_disable_streams
- * via pm_runtime_put_autosuspend. Returns CSI-2 link ownership to CVS firmware.
- *
- * Return: 0 on success or negative errno.
- */
-static int __maybe_unused cvs_runtime_suspend(struct device *dev)
-{
-	struct icvs *ctx = dev_get_drvdata(dev);
-
-	return cvs_set_link_owner(ctx, ICVS_CSI_LINK_CVS);
-}
-
 static const struct dev_pm_ops __maybe_unused cvs_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(cvs_suspend, cvs_resume)
-	SET_RUNTIME_PM_OPS(cvs_runtime_suspend, cvs_runtime_resume, NULL)
 };
 
 static const struct acpi_device_id intel_cvs_acpi_match[] = {
diff --git a/drivers/media/i2c/cvs/icvs.h b/drivers/media/i2c/cvs/icvs.h
index 17beb0920dd2..18ec76ce49a6 100644
--- a/drivers/media/i2c/cvs/icvs.h
+++ b/drivers/media/i2c/cvs/icvs.h
@@ -432,7 +432,7 @@ enum icvs_state {
  * @freq_ctrl: (future) frequency control pointer
  * @pads: Local media pads (sink/source)
  * @nr_of_lanes: Active CSI-2 lane count
- * @ipu_link: PM runtime device link (IPU consumer, CVS supplier)
+ * @ipu_link: Device link for suspend/resume ordering
  * @res: Resource capability (light/full)
  * @caps: Reported device protocol capabilities
  * @prefix: Firmware response prefix present
diff --git a/drivers/media/i2c/cvs/v4l2.c b/drivers/media/i2c/cvs/v4l2.c
index 5a9d7caaee3f..37cde9f115a0 100644
--- a/drivers/media/i2c/cvs/v4l2.c
+++ b/drivers/media/i2c/cvs/v4l2.c
@@ -7,7 +7,6 @@
 #include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/module.h>
-#include <linux/pm_runtime.h>
 #include <linux/property.h>
 
 #include <media/v4l2-async.h>
@@ -83,6 +82,10 @@ static int cvs_csi_enable_streams(struct v4l2_subdev *sd,
 	s64 link_freq;
 	int ret;
 
+	link_freq = v4l2_get_link_freq(ctx->remote, 0, 0);
+	if (link_freq < 0)
+		return link_freq;
+
 	ret = cvs_set_link_owner(ctx, ICVS_CSI_LINK_HOST);
 	if (ret) {
 		dev_err(dev, "failed to hand over CSI link to host: %d\n",
@@ -90,36 +93,23 @@ static int cvs_csi_enable_streams(struct v4l2_subdev *sd,
 		return ret;
 	}
 
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret < 0)
-		return ret;
-
-	link_freq = v4l2_get_link_freq(ctx->remote, 0, 0);
-	if (link_freq < 0) {
-		ret = link_freq;
-		goto err_rpm_put;
-	}
-
 	if (ctx->i2c_client) {
 		ret = csi_set_link_cfg(ctx, link_freq);
 		if (ret < 0)
-			goto err_rpm_put_sync;
+			goto err_release_link;
 	}
 
 	ret = v4l2_subdev_enable_streams(remote_sd,
 					 ctx->remote->index,
 					 streams_mask);
 	if (ret)
-		goto err_rpm_put_sync;
+		goto err_release_link;
 
 	return 0;
 
-err_rpm_put_sync:
-	/* Bypass autosuspend to immediately release ownership on error */
-	pm_runtime_put_sync(dev);
-	return ret;
-err_rpm_put:
-	pm_runtime_put_autosuspend(dev);
+err_release_link:
+	cvs_set_link_owner(ctx, ICVS_CSI_LINK_CVS);
+
 	return ret;
 }
 
@@ -144,8 +134,6 @@ static int cvs_csi_disable_streams(struct v4l2_subdev *sd,
 		dev_err(dev, "failed to hand over CSI link to CVS: %d\n",
 			link_ret);
 
-	pm_runtime_put_autosuspend(dev);
-
 	return ret ? : link_ret;
 }
 
-- 
2.55.0

Re: [PATCH 2/2] media: cvs: Remove runtime PM support

[Sakari Ailus]

Moi,

On Mon, Oct 12, 2026 at 10:25:25PM +0300, Antti Laakso wrote:
> The CSI link is handed to the host on stream enable and back to the
> CVS on stream disable. The runtime PM is doing the same, and is now
> unnecessary. Remove runtime PM.

The sensor driver may access sensor's registers when the device is powered
on, e.g. for setting controls when streaming is disabled. Not every driver
does that but that's why the Runtime PM flags are there.

-- 
Terveisin,

Sakari Ailus