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

[Dave Stevenson]

Hi Sakari

On Tue, 14 Jul 2026 at 12:36, Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> 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@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.

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