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

[Sakari Ailus]

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@xxxxxxxxxxxxxxx>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> ---
>  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