[PATCH v3 12/20] media: imx355: Use pm_runtime autosuspend_delay

Dave Stevenson posted 20 patches 1 month ago
There is a newer version of this series
[PATCH v3 12/20] media: imx355: Use pm_runtime autosuspend_delay
Posted by Dave Stevenson 1 month ago
Avoid powering the sensor up and down unnecessarily by using
pm_runtime's autosuspend_delay feature.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 drivers/media/i2c/imx355.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
index ee1f41a66a0b..f01a1a5ae7c9 100644
--- a/drivers/media/i2c/imx355.c
+++ b/drivers/media/i2c/imx355.c
@@ -1073,7 +1073,7 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
 			goto err_rpm_put;
 	} else {
 		imx355_stop_streaming(imx355);
-		pm_runtime_put(imx355->dev);
+		pm_runtime_put_autosuspend(imx355->dev);
 	}
 
 	/* vflip and hflip cannot change during streaming */
@@ -1085,7 +1085,7 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 
 err_rpm_put:
-	pm_runtime_put(imx355->dev);
+	pm_runtime_put_autosuspend(imx355->dev);
 err_unlock:
 	mutex_unlock(&imx355->mutex);
 
@@ -1436,6 +1436,8 @@ static int imx355_probe(struct i2c_client *client)
 	pm_runtime_set_active(imx355->dev);
 	pm_runtime_enable(imx355->dev);
 	pm_runtime_idle(imx355->dev);
+	pm_runtime_set_autosuspend_delay(imx355->dev, 1000);
+	pm_runtime_use_autosuspend(imx355->dev);
 
 	ret = v4l2_async_register_subdev_sensor(&imx355->sd);
 	if (ret < 0)
@@ -1446,6 +1448,7 @@ static int imx355_probe(struct i2c_client *client)
 error_media_entity_runtime_pm:
 	pm_runtime_disable(imx355->dev);
 	pm_runtime_set_suspended(imx355->dev);
+	pm_runtime_dont_use_autosuspend(imx355->dev);
 	media_entity_cleanup(&imx355->sd.entity);
 
 error_handler_free:
@@ -1476,6 +1479,8 @@ static void imx355_remove(struct i2c_client *client)
 		pm_runtime_set_suspended(imx355->dev);
 	}
 
+	pm_runtime_dont_use_autosuspend(imx355->dev);
+
 	mutex_destroy(&imx355->mutex);
 }
 

-- 
2.34.1
Re: [PATCH v3 12/20] media: imx355: Use pm_runtime autosuspend_delay
Posted by Sakari Ailus 1 month ago
Hi Dave,

On Wed, Jul 08, 2026 at 03:48:48PM +0100, Dave Stevenson wrote:
> Avoid powering the sensor up and down unnecessarily by using
> pm_runtime's autosuspend_delay feature.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  drivers/media/i2c/imx355.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
> index ee1f41a66a0b..f01a1a5ae7c9 100644
> --- a/drivers/media/i2c/imx355.c
> +++ b/drivers/media/i2c/imx355.c
> @@ -1073,7 +1073,7 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
>  			goto err_rpm_put;
>  	} else {
>  		imx355_stop_streaming(imx355);
> -		pm_runtime_put(imx355->dev);
> +		pm_runtime_put_autosuspend(imx355->dev);
>  	}
>  
>  	/* vflip and hflip cannot change during streaming */
> @@ -1085,7 +1085,7 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
>  	return ret;
>  
>  err_rpm_put:
> -	pm_runtime_put(imx355->dev);
> +	pm_runtime_put_autosuspend(imx355->dev);
>  err_unlock:
>  	mutex_unlock(&imx355->mutex);
>  
> @@ -1436,6 +1436,8 @@ static int imx355_probe(struct i2c_client *client)
>  	pm_runtime_set_active(imx355->dev);
>  	pm_runtime_enable(imx355->dev);
>  	pm_runtime_idle(imx355->dev);
> +	pm_runtime_set_autosuspend_delay(imx355->dev, 1000);
> +	pm_runtime_use_autosuspend(imx355->dev);

I think it'd make sense to do this before calling pm_runtime_idle(),
wouldn't it?

There's also a pre-existing bug here: if pm_runtime_idle() powers the
sensor off before calling pm_runtime_disable(), imx355_power_off() will get
called twice. The easiest fix is to call pm_runtime_idle() only after
v4l2_async_register_subdev_sensor().

This should be fixed before adding autosuspend support.

>  
>  	ret = v4l2_async_register_subdev_sensor(&imx355->sd);
>  	if (ret < 0)
> @@ -1446,6 +1448,7 @@ static int imx355_probe(struct i2c_client *client)
>  error_media_entity_runtime_pm:
>  	pm_runtime_disable(imx355->dev);
>  	pm_runtime_set_suspended(imx355->dev);
> +	pm_runtime_dont_use_autosuspend(imx355->dev);
>  	media_entity_cleanup(&imx355->sd.entity);
>  
>  error_handler_free:
> @@ -1476,6 +1479,8 @@ static void imx355_remove(struct i2c_client *client)
>  		pm_runtime_set_suspended(imx355->dev);
>  	}
>  
> +	pm_runtime_dont_use_autosuspend(imx355->dev);
> +
>  	mutex_destroy(&imx355->mutex);
>  }
>  
> 

-- 
Kind regards,

Sakari Ailus
Re: [PATCH v3 12/20] media: imx355: Use pm_runtime autosuspend_delay
Posted by Dave Stevenson 4 weeks, 1 day ago
Hi Sakari

On Tue, 14 Jul 2026 at 12:36, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> Hi Dave,
>
> On Wed, Jul 08, 2026 at 03:48:48PM +0100, Dave Stevenson wrote:
> > Avoid powering the sensor up and down unnecessarily by using
> > pm_runtime's autosuspend_delay feature.
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  drivers/media/i2c/imx355.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
> > index ee1f41a66a0b..f01a1a5ae7c9 100644
> > --- a/drivers/media/i2c/imx355.c
> > +++ b/drivers/media/i2c/imx355.c
> > @@ -1073,7 +1073,7 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
> >                       goto err_rpm_put;
> >       } else {
> >               imx355_stop_streaming(imx355);
> > -             pm_runtime_put(imx355->dev);
> > +             pm_runtime_put_autosuspend(imx355->dev);
> >       }
> >
> >       /* vflip and hflip cannot change during streaming */
> > @@ -1085,7 +1085,7 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
> >       return ret;
> >
> >  err_rpm_put:
> > -     pm_runtime_put(imx355->dev);
> > +     pm_runtime_put_autosuspend(imx355->dev);
> >  err_unlock:
> >       mutex_unlock(&imx355->mutex);
> >
> > @@ -1436,6 +1436,8 @@ static int imx355_probe(struct i2c_client *client)
> >       pm_runtime_set_active(imx355->dev);
> >       pm_runtime_enable(imx355->dev);
> >       pm_runtime_idle(imx355->dev);
> > +     pm_runtime_set_autosuspend_delay(imx355->dev, 1000);
> > +     pm_runtime_use_autosuspend(imx355->dev);
>
> I think it'd make sense to do this before calling pm_runtime_idle(),
> wouldn't it?
>
> There's also a pre-existing bug here: if pm_runtime_idle() powers the
> sensor off before calling pm_runtime_disable(), imx355_power_off() will get
> called twice. The easiest fix is to call pm_runtime_idle() only after
> v4l2_async_register_subdev_sensor().
>
> This should be fixed before adding autosuspend support.

Ack, I'll do that.

I'm aiming to have a new revision out tomorrow to address the build
breakage in the middle of the set (I've updated my test frameworks to
catch this in future), and the Sashiko comments that the device-tree
list bot has flagged.

  Dave


> >
> >       ret = v4l2_async_register_subdev_sensor(&imx355->sd);
> >       if (ret < 0)
> > @@ -1446,6 +1448,7 @@ static int imx355_probe(struct i2c_client *client)
> >  error_media_entity_runtime_pm:
> >       pm_runtime_disable(imx355->dev);
> >       pm_runtime_set_suspended(imx355->dev);
> > +     pm_runtime_dont_use_autosuspend(imx355->dev);
> >       media_entity_cleanup(&imx355->sd.entity);
> >
> >  error_handler_free:
> > @@ -1476,6 +1479,8 @@ static void imx355_remove(struct i2c_client *client)
> >               pm_runtime_set_suspended(imx355->dev);
> >       }
> >
> > +     pm_runtime_dont_use_autosuspend(imx355->dev);
> > +
> >       mutex_destroy(&imx355->mutex);
> >  }
> >
> >
>
> --
> Kind regards,
>
> Sakari Ailus