[PATCH v4 0/4] E5010 Probe Cleanup and Adding autosuspend

[PATCH v4 0/4] E5010 Probe Cleanup and Adding autosuspend

[Brandon Brnich]

Hello all,

This series has changed a bit since v3 so thought it was appropriate to
add in a cover letter.

I noticed a report from the intel bot[0] which pointed out a valid issue
with the driver never calling e5010_init_device unless CONFIG_PM was
enabled. That has now been addressed in v4 by moving the call to probe.

While looking at probe though, I noticed that the clk_enable was never
properly called. That has now been fixed with a proper remove path as
well.

Lastly, I cleaned up the goto jumps in probe to fix an issue where the
m2m_dev wasn't being freed.

Changes since v3:

  - patches 1 and 2 are new in v4
  - patch 3 remains unchanged
  - patch 4
      > updated goto to match changes in patch 1
      > update PM calls in probe

Best,
Brandon

[0]: https://lore.kernel.org/oe-kbuild-all/202608140323.qyc8mKlp-lkp@intel.com/

Brandon Brnich (4):
  media: imagination: e5010: Properly Release m2m_dev if probe fails
  media: imagination: e5010: Fix clk never enabled without CONFIG_PM
  media: imagination: e5010: Move e5010_init_device to Runtime Resume
    Hook
  media: imagination: e5010: Enable autosuspend for runtime PM

 .../platform/imagination/e5010-jpeg-enc.c     | 76 +++++++++++--------
 1 file changed, 44 insertions(+), 32 deletions(-)

-- 
2.43.0

[PATCH v4 1/4] media: imagination: e5010: Properly Release m2m_dev if probe fails

[Brandon Brnich]

After the call to v4l2_m2m_init() is successful, the following
initialization functions should go to fail_after_video_register_device.

Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
Cc: stable@vger.kernel.org
Signed-off-by: Brandon Brnich <b-brnich@ti.com>
---
 drivers/media/platform/imagination/e5010-jpeg-enc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
index 42ad9ee3993b4..d3c11337c815d 100644
--- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
+++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
@@ -1072,14 +1072,14 @@ static int e5010_probe(struct platform_device *pdev)
 	if (IS_ERR(e5010->core_base)) {
 		ret = PTR_ERR(e5010->core_base);
 		dev_err_probe(dev, ret, "Missing 'core' resources area\n");
-		goto fail_after_v4l2_register;
+		goto fail_after_video_register_device;
 	}
 
 	e5010->mmu_base = devm_platform_ioremap_resource_byname(pdev, "mmu");
 	if (IS_ERR(e5010->mmu_base)) {
 		ret = PTR_ERR(e5010->mmu_base);
 		dev_err_probe(dev, ret, "Missing 'mmu' resources area\n");
-		goto fail_after_v4l2_register;
+		goto fail_after_video_register_device;
 	}
 
 	e5010->last_context_run = NULL;
@@ -1089,14 +1089,14 @@ static int e5010_probe(struct platform_device *pdev)
 			       E5010_MODULE_NAME, e5010);
 	if (ret) {
 		dev_err_probe(dev, ret, "failed to register IRQ %d\n", irq);
-		goto fail_after_v4l2_register;
+		goto fail_after_video_register_device;
 	}
 
 	e5010->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(e5010->clk)) {
 		ret = PTR_ERR(e5010->clk);
 		dev_err_probe(dev, ret, "failed to get clock\n");
-		goto fail_after_v4l2_register;
+		goto fail_after_video_register_device;
 	}
 
 	pm_runtime_enable(dev);
-- 
2.43.0

Re: [PATCH v4 1/4] media: imagination: e5010: Properly Release m2m_dev if probe fails

[Devarsh Thakkar]

Hi Brandon,

Thanks for the patch.

On 01/09/26 03:02, Brandon Brnich wrote:
> After the call to v4l2_m2m_init() is successful, the following
> initialization functions should go to fail_after_video_register_device.
> 
> Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Brandon Brnich <b-brnich@ti.com>
> ---
>   drivers/media/platform/imagination/e5010-jpeg-enc.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> index 42ad9ee3993b4..d3c11337c815d 100644
> --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
> +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> @@ -1072,14 +1072,14 @@ static int e5010_probe(struct platform_device *pdev)
>   	if (IS_ERR(e5010->core_base)) {
>   		ret = PTR_ERR(e5010->core_base);
>   		dev_err_probe(dev, ret, "Missing 'core' resources area\n");
> -		goto fail_after_v4l2_register;
> +		goto fail_after_video_register_device;

The label naming looks incorrect semantically since 
video_register_device happens after all this at last actually. Please 
change it to fail_after_v4l2_m2m_init instead.

>   	}
>   
>   	e5010->mmu_base = devm_platform_ioremap_resource_byname(pdev, "mmu");
>   	if (IS_ERR(e5010->mmu_base)) {
>   		ret = PTR_ERR(e5010->mmu_base);
>   		dev_err_probe(dev, ret, "Missing 'mmu' resources area\n");
> -		goto fail_after_v4l2_register;
> +		goto fail_after_video_register_device;
>   	}
>   

goto fail_after_v4l2_m2m_init

>   	e5010->last_context_run = NULL;
> @@ -1089,14 +1089,14 @@ static int e5010_probe(struct platform_device *pdev)
>   			       E5010_MODULE_NAME, e5010);
>   	if (ret) {
>   		dev_err_probe(dev, ret, "failed to register IRQ %d\n", irq);
> -		goto fail_after_v4l2_register;
> +		goto fail_after_video_register_device;
>   	}

goto fail_after_v4l2_m2m_init

>   
>   	e5010->clk = devm_clk_get(dev, NULL);
>   	if (IS_ERR(e5010->clk)) {
>   		ret = PTR_ERR(e5010->clk);
>   		dev_err_probe(dev, ret, "failed to get clock\n");
> -		goto fail_after_v4l2_register;
> +		goto fail_after_video_register_device;
>   	}

goto fail_after_v4l2_m2m_init
>   
>   	pm_runtime_enable(dev);

Regards
Devarsh

[PATCH v4 2/4] media: imagination: e5010: Fix clk never enabled without CONFIG_PM

[Brandon Brnich]

clk_prepare_enable is not called anywhere unless CONFIG_PM is set. This
means without CONFIG_PM, the JPEG Encoder will not function properly.

Add this call during the probe sequence to properly configure the device.
If video_registration fails after clk is enabled, clk would be left on.
Make a label to cover this case.

Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
Cc: stable@vger.kernel.org
Signed-off-by: Brandon Brnich <b-brnich@ti.com>
---
 .../media/platform/imagination/e5010-jpeg-enc.c    | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
index d3c11337c815d..e98fcd3fcd9ae 100644
--- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
+++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
@@ -1099,19 +1099,28 @@ static int e5010_probe(struct platform_device *pdev)
 		goto fail_after_video_register_device;
 	}
 
-	pm_runtime_enable(dev);
+	ret = clk_prepare_enable(e5010->clk);
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to enable clock\n");
+		goto fail_after_video_register_device;
+	}
+
 
 	ret = video_register_device(e5010->vdev, VFL_TYPE_VIDEO, 0);
 	if (ret) {
 		dev_err_probe(dev, ret, "failed to register video device\n");
-		goto fail_after_video_register_device;
+		goto fail_after_clock_enable;
 	}
 
+	pm_runtime_enable(dev);
+
 	v4l2_info(&e5010->v4l2_dev, "Device registered as /dev/video%d\n",
 		  e5010->vdev->num);
 
 	return 0;
 
+fail_after_clock_enable:
+	clk_disable_unprepare(e5010->clk);
 fail_after_video_register_device:
 	v4l2_m2m_release(e5010->m2m_dev);
 fail_after_v4l2_register:
@@ -1126,6 +1135,7 @@ static void e5010_remove(struct platform_device *pdev)
 	struct e5010_dev *e5010 = platform_get_drvdata(pdev);
 
 	pm_runtime_disable(e5010->dev);
+	clk_disable_unprepare(e5010->clk);
 	video_unregister_device(e5010->vdev);
 	v4l2_m2m_release(e5010->m2m_dev);
 	v4l2_device_unregister(&e5010->v4l2_dev);
-- 
2.43.0

Re: [PATCH v4 2/4] media: imagination: e5010: Fix clk never enabled without CONFIG_PM

[Devarsh Thakkar]

Hi Brandon,

Thanks for the patch.

On 01/09/26 03:02, Brandon Brnich wrote:
> clk_prepare_enable is not called anywhere unless CONFIG_PM is set. This
> means without CONFIG_PM, the JPEG Encoder will not function properly.
> 
> Add this call during the probe sequence to properly configure the device.
> If video_registration fails after clk is enabled, clk would be left on.
> Make a label to cover this case.
> 
> Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Brandon Brnich <b-brnich@ti.com>
> ---
>   .../media/platform/imagination/e5010-jpeg-enc.c    | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> index d3c11337c815d..e98fcd3fcd9ae 100644
> --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
> +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> @@ -1099,19 +1099,28 @@ static int e5010_probe(struct platform_device *pdev)
>   		goto fail_after_video_register_device;
>   	}
>   
> -	pm_runtime_enable(dev);
> +	ret = clk_prepare_enable(e5010->clk);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to enable clock\n");
> +		goto fail_after_video_register_device;
> +	}
> +
>   
>   	ret = video_register_device(e5010->vdev, VFL_TYPE_VIDEO, 0);
>   	if (ret) {
>   		dev_err_probe(dev, ret, "failed to register video device\n");
> -		goto fail_after_video_register_device;
> +		goto fail_after_clock_enable;
>   	}
>   
> +	pm_runtime_enable(dev);
> +
>   	v4l2_info(&e5010->v4l2_dev, "Device registered as /dev/video%d\n",
>   		  e5010->vdev->num);
>   
>   	return 0;
>   
> +fail_after_clock_enable:
> +	clk_disable_unprepare(e5010->clk);
>   fail_after_video_register_device:
>   	v4l2_m2m_release(e5010->m2m_dev);
>   fail_after_v4l2_register:
> @@ -1126,6 +1135,7 @@ static void e5010_remove(struct platform_device *pdev)
>   	struct e5010_dev *e5010 = platform_get_drvdata(pdev);
>   
>   	pm_runtime_disable(e5010->dev);
> +	clk_disable_unprepare(e5010->clk);

I think you should only disable if driver was not suspended as you 
already do a clk_disable in suspend path, i think it should be :

         if (!pm_runtime_status_suspended(&pdev->dev)) 

		clk_disable_unprepare(e5010->clk);

Regards
Devarsh
>   	video_unregister_device(e5010->vdev);
>   	v4l2_m2m_release(e5010->m2m_dev);
>   	v4l2_device_unregister(&e5010->v4l2_dev);

[PATCH v4 3/4] media: imagination: e5010: Move e5010_init_device to Runtime Resume Hook

[Brandon Brnich]

The PM framework skips the runtime resume hook if the device was never on
in the first place. This results in a crash in the system resume hook when
a call to e5010_init_device attempts a write to a powered down device.

Move e5010_init_device to the resume hook to ensure register write only
occurs when device is properly powered on.

Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
Cc: stable@vger.kernel.org
Signed-off-by: Brandon Brnich <b-brnich@ti.com>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
Reviewed-by: Kendall Willis <k-willis@ti.com>
---
 drivers/media/platform/imagination/e5010-jpeg-enc.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
index e98fcd3fcd9ae..488f5d7e4c9d5 100644
--- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
+++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
@@ -1511,6 +1511,13 @@ static int e5010_runtime_resume(struct device *dev)
 		return ret;
 	}
 
+	ret = e5010_init_device(e5010);
+	if (ret) {
+		dev_err(dev, "Failed to re-enable e5010 device\n");
+		clk_disable_unprepare(e5010->clk);
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -1543,12 +1550,6 @@ static int e5010_resume(struct device *dev)
 	if (ret < 0)
 		return ret;
 
-	ret = e5010_init_device(e5010);
-	if (ret) {
-		dev_err(dev, "Failed to re-enable e5010 device\n");
-		return ret;
-	}
-
 	v4l2_m2m_resume(e5010->m2m_dev);
 
 	return ret;
-- 
2.43.0

Re: [PATCH v4 3/4] media: imagination: e5010: Move e5010_init_device to Runtime Resume Hook

[Devarsh Thakkar]

On 01/09/26 03:02, Brandon Brnich wrote:
> The PM framework skips the runtime resume hook if the device was never on
> in the first place. This results in a crash in the system resume hook when
> a call to e5010_init_device attempts a write to a powered down device.
> 
> Move e5010_init_device to the resume hook to ensure register write only
> occurs when device is properly powered on.
> 
> Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Brandon Brnich <b-brnich@ti.com>
> Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
> Reviewed-by: Kendall Willis <k-willis@ti.com>

Reviewed-by: Devarsh Thakkar <devarsht@ti.com>

Regards
Devarsh

> ---
>   drivers/media/platform/imagination/e5010-jpeg-enc.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> index e98fcd3fcd9ae..488f5d7e4c9d5 100644
> --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
> +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> @@ -1511,6 +1511,13 @@ static int e5010_runtime_resume(struct device *dev)
>   		return ret;
>   	}
>   
> +	ret = e5010_init_device(e5010);
> +	if (ret) {
> +		dev_err(dev, "Failed to re-enable e5010 device\n");
> +		clk_disable_unprepare(e5010->clk);
> +		return ret;
> +	}
> +
>   	return 0;
>   }
>   
> @@ -1543,12 +1550,6 @@ static int e5010_resume(struct device *dev)
>   	if (ret < 0)
>   		return ret;
>   
> -	ret = e5010_init_device(e5010);
> -	if (ret) {
> -		dev_err(dev, "Failed to re-enable e5010 device\n");
> -		return ret;
> -	}
> -
>   	v4l2_m2m_resume(e5010->m2m_dev);
>   
>   	return ret;

[PATCH v4 4/4] media: imagination: e5010: Enable autosuspend for runtime PM

[Brandon Brnich]

Current implementation only has one call to turn the device on -
start_streaming. This can waste power of device as the device can be in
STREAM_ON state, but no buffers are actually being submitted.

Need to set last_context_run to NULL in resume hook. This will trigger
QP value updates during next call to device_run.

Signed-off-by: Brandon Brnich <b-brnich@ti.com>
---
 .../platform/imagination/e5010-jpeg-enc.c     | 41 ++++++++++---------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
index 488f5d7e4c9d5..79347014562bd 100644
--- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
+++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
@@ -976,6 +976,7 @@ static irqreturn_t e5010_irq(int irq, void *data)
 
 	v4l2_m2m_job_finish(e5010->m2m_dev, ctx->fh.m2m_ctx);
 	dprintk(e5010, 3, "ctx: 0x%p Finish job\n", ctx);
+	pm_runtime_put_autosuspend(e5010->dev);
 
 job_unlock:
 	spin_unlock(&e5010->hw_lock);
@@ -1105,6 +1106,11 @@ static int e5010_probe(struct platform_device *pdev)
 		goto fail_after_video_register_device;
 	}
 
+	ret = e5010_init_device(e5010);
+	if (ret) {
+		dev_err_probe(dev, ret, "failed to init device\n");
+		goto fail_after_clock_enable;
+	}
 
 	ret = video_register_device(e5010->vdev, VFL_TYPE_VIDEO, 0);
 	if (ret) {
@@ -1112,7 +1118,11 @@ static int e5010_probe(struct platform_device *pdev)
 		goto fail_after_clock_enable;
 	}
 
+	pm_runtime_set_autosuspend_delay(dev, 100);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
+	pm_runtime_idle(dev);
 
 	v4l2_info(&e5010->v4l2_dev, "Device registered as /dev/video%d\n",
 		  e5010->vdev->num);
@@ -1299,31 +1309,13 @@ static int e5010_encoder_cmd(struct file *file, void *priv,
 static int e5010_start_streaming(struct vb2_queue *q, unsigned int count)
 {
 	struct e5010_context *ctx = vb2_get_drv_priv(q);
-	int ret;
 
 	struct e5010_q_data *queue = get_queue(ctx, q->type);
 
 	v4l2_m2m_update_start_streaming_state(ctx->fh.m2m_ctx, q);
 	queue->sequence = 0;
 
-	ret = pm_runtime_resume_and_get(ctx->e5010->dev);
-	if (ret < 0) {
-		v4l2_err(&ctx->e5010->v4l2_dev, "failed to power up jpeg\n");
-		goto fail;
-	}
-
-	ret = e5010_init_device(ctx->e5010);
-	if (ret) {
-		v4l2_err(&ctx->e5010->v4l2_dev, "failed to Enable e5010 device\n");
-		goto fail;
-	}
-
 	return 0;
-
-fail:
-	e5010_vb2_buffers_return(q, VB2_BUF_STATE_QUEUED);
-
-	return ret;
 }
 
 static void e5010_stop_streaming(struct vb2_queue *q)
@@ -1339,8 +1331,6 @@ static void e5010_stop_streaming(struct vb2_queue *q)
 	    v4l2_m2m_has_stopped(ctx->fh.m2m_ctx)) {
 		v4l2_event_queue_fh(&ctx->fh, &e5010_eos_event);
 	}
-
-	pm_runtime_put_sync(ctx->e5010->dev);
 }
 
 static void e5010_device_run(void *priv)
@@ -1353,7 +1343,15 @@ static void e5010_device_run(void *priv)
 	unsigned long flags;
 	int num_planes = ctx->out_queue.fmt->num_planes;
 
+	ret = pm_runtime_resume_and_get(e5010->dev);
+	if (ret < 0) {
+		dev_err(e5010->dev, "Device failed to turn on\n");
+		v4l2_m2m_job_finish(e5010->m2m_dev, ctx->fh.m2m_ctx);
+		return;
+	}
+
 	spin_lock_irqsave(&e5010->hw_lock, flags);
+
 	s_vb = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	WARN_ON(!s_vb);
 	d_vb = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
@@ -1484,6 +1482,7 @@ static void e5010_device_run(void *priv)
 	e5010_reset(e5010->dev, e5010->core_base, e5010->mmu_base);
 
 no_ready_buf_err:
+	pm_runtime_put_autosuspend(e5010->dev);
 	if (s_vb) {
 		v4l2_m2m_src_buf_remove_by_buf(ctx->fh.m2m_ctx, s_vb);
 		v4l2_m2m_buf_done(s_vb, VB2_BUF_STATE_ERROR);
@@ -1518,6 +1517,8 @@ static int e5010_runtime_resume(struct device *dev)
 		return ret;
 	}
 
+	e5010->last_context_run = NULL;
+
 	return 0;
 }
 
-- 
2.43.0

Re: [PATCH v4 4/4] media: imagination: e5010: Enable autosuspend for runtime PM

[Devarsh Thakkar]

Hi Brandon,

Thanks for the patch.

On 01/09/26 03:02, Brandon Brnich wrote:
> Current implementation only has one call to turn the device on -
> start_streaming. This can waste power of device as the device can be in
> STREAM_ON state, but no buffers are actually being submitted.
> 
> Need to set last_context_run to NULL in resume hook. This will trigger
> QP value updates during next call to device_run.
> 
> Signed-off-by: Brandon Brnich <b-brnich@ti.com>
> ---
>   .../platform/imagination/e5010-jpeg-enc.c     | 41 ++++++++++---------
>   1 file changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> index 488f5d7e4c9d5..79347014562bd 100644
> --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
> +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> @@ -976,6 +976,7 @@ static irqreturn_t e5010_irq(int irq, void *data)
>   
>   	v4l2_m2m_job_finish(e5010->m2m_dev, ctx->fh.m2m_ctx);
>   	dprintk(e5010, 3, "ctx: 0x%p Finish job\n", ctx);
> +	pm_runtime_put_autosuspend(e5010->dev);
>   
>   job_unlock:
>   	spin_unlock(&e5010->hw_lock);
> @@ -1105,6 +1106,11 @@ static int e5010_probe(struct platform_device *pdev)
>   		goto fail_after_video_register_device;
>   	}
>   
> +	ret = e5010_init_device(e5010);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "failed to init device\n");
> +		goto fail_after_clock_enable;
> +	}
>   


I think we already do e5010_init_device(e5010) in runtime_resume,

I understand your goal here is to handle for scenario where PM is not 
enabled but it's kind of duplication for the scenario where PM is 
enabled already. I would suggest to wrap this (and also the clk_enable 
from previous patch) with a check :

I think in probe itself have a check :

if (!pm_runtime_enabled(dev)) {

// do clock enable
// do init device

}


>   	ret = video_register_device(e5010->vdev, VFL_TYPE_VIDEO, 0);
>   	if (ret) {
> @@ -1112,7 +1118,11 @@ static int e5010_probe(struct platform_device *pdev)
>   		goto fail_after_clock_enable;
>   	}
>   
> +	pm_runtime_set_autosuspend_delay(dev, 100);

Better to use a macro for the timeout delay.

> +	pm_runtime_use_autosuspend(dev);
> +	pm_runtime_set_active(dev);
>   	pm_runtime_enable(dev);
> +	pm_runtime_idle(dev);
>   
>   	v4l2_info(&e5010->v4l2_dev, "Device registered as /dev/video%d\n",
>   		  e5010->vdev->num);
> @@ -1299,31 +1309,13 @@ static int e5010_encoder_cmd(struct file *file, void *priv,
>   static int e5010_start_streaming(struct vb2_queue *q, unsigned int count)
>   {
>   	struct e5010_context *ctx = vb2_get_drv_priv(q);
> -	int ret;
>   
>   	struct e5010_q_data *queue = get_queue(ctx, q->type);
>   
>   	v4l2_m2m_update_start_streaming_state(ctx->fh.m2m_ctx, q);
>   	queue->sequence = 0;
>   
> -	ret = pm_runtime_resume_and_get(ctx->e5010->dev);
> -	if (ret < 0) {
> -		v4l2_err(&ctx->e5010->v4l2_dev, "failed to power up jpeg\n");
> -		goto fail;
> -	}
> -
> -	ret = e5010_init_device(ctx->e5010);
> -	if (ret) {
> -		v4l2_err(&ctx->e5010->v4l2_dev, "failed to Enable e5010 device\n");
> -		goto fail;
> -	}
> -
>   	return 0;
> -
> -fail:
> -	e5010_vb2_buffers_return(q, VB2_BUF_STATE_QUEUED);
> -
> -	return ret;
>   }
>   
>   static void e5010_stop_streaming(struct vb2_queue *q)
> @@ -1339,8 +1331,6 @@ static void e5010_stop_streaming(struct vb2_queue *q)
>   	    v4l2_m2m_has_stopped(ctx->fh.m2m_ctx)) {
>   		v4l2_event_queue_fh(&ctx->fh, &e5010_eos_event);
>   	}
> -
> -	pm_runtime_put_sync(ctx->e5010->dev);
>   }
>   
>   static void e5010_device_run(void *priv)
> @@ -1353,7 +1343,15 @@ static void e5010_device_run(void *priv)
>   	unsigned long flags;
>   	int num_planes = ctx->out_queue.fmt->num_planes;
>   
> +	ret = pm_runtime_resume_and_get(e5010->dev);

> +	if (ret < 0) {
> +		dev_err(e5010->dev, "Device failed to turn on\n");
> +		v4l2_m2m_job_finish(e5010->m2m_dev, ctx->fh.m2m_ctx);

You also need to remove buffers from queue and return back as 
VB2_BUF_ERROR I think you should do this after the initial check in 
driver, something like below should work :

	if (!s_vb || !d_vb) {
		/* Buffers aren't even ready; end the job immediately without touching 
power */
		v4l2_m2m_job_finish(e5010->m2m_dev, ctx->fh.m2m_ctx);
		return;
	}

	ret = pm_runtime_resume_and_get(e5010->dev);
	if (ret < 0) {
		dev_err(e5010->dev, "Device failed to turn on\n");
		/* Jump directly to the clean-up path that skips unlocking the unheld 
spinlock */
		goto device_turn_on_fail;
	}

	spin_lock_irqsave(&e5010->hw_lock, flags);

...
...

device_busy_err:
	e5010_reset(e5010->dev, e5010->core_base, e5010->mmu_base);
	spin_unlock_irqrestore(&e5010->hw_lock, flags);

device_turn_on_fail:
	pm_runtime_put_autosuspend(e5010->dev);
	if (s_vb) {
		v4l2_m2m_src_buf_remove_by_buf(ctx->fh.m2m_ctx, s_vb);
		v4l2_m2m_buf_done(s_vb, VB2_BUF_STATE_ERROR);
	}

	if (d_vb) {
		v4l2_m2m_dst_buf_remove_by_buf(ctx->fh.m2m_ctx, d_vb);
		/* Payload set to 1 since 0 payload can trigger EOS */
		vb2_set_plane_payload(&d_vb->vb2_buf, 0, 1);
		v4l2_m2m_buf_done(d_vb, VB2_BUF_STATE_ERROR);
	}
	v4l2_m2m_job_finish(e5010->m2m_dev, ctx->fh.m2m_ctx);

Regards
Devarsh


> +		return;
> +	}
> +
>   	spin_lock_irqsave(&e5010->hw_lock, flags);
> +
>   	s_vb = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>   	WARN_ON(!s_vb);
>   	d_vb = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> @@ -1484,6 +1482,7 @@ static void e5010_device_run(void *priv)
>   	e5010_reset(e5010->dev, e5010->core_base, e5010->mmu_base);
>   
>   no_ready_buf_err:
> +	pm_runtime_put_autosuspend(e5010->dev);
>   	if (s_vb) {
>   		v4l2_m2m_src_buf_remove_by_buf(ctx->fh.m2m_ctx, s_vb);
>   		v4l2_m2m_buf_done(s_vb, VB2_BUF_STATE_ERROR);
> @@ -1518,6 +1517,8 @@ static int e5010_runtime_resume(struct device *dev)
>   		return ret;
>   	}
>   
> +	e5010->last_context_run = NULL;
> +
>   	return 0;
>   }
>