Re: [PATCH 09/25] media: hantro: do a PM resume earlier

[Ezequiel Garcia]

Hi Mauro,

Thanks for working on this.

On Wed, 2021-05-05 at 11:41 +0200, Mauro Carvalho Chehab wrote:
> The device_run() first enables the clock and then
> tries to resume PM runtime, checking for errors.
> 
> Well, if for some reason the pm_runtime can not resume,
> it would be better to detect it beforehand.
> 
> So, change the order inside device_run().
> 
> Reviewed-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> Fixes: 775fec69008d ("media: add Rockchip VPU JPEG encoder driver")
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>

It seems this is wrong now, as this series doesn't have

https://lore.kernel.org/linux-media/803c39fafdd62efc6f9e4d99a372af2c6955143b.1619621413.git.mchehab+huawei@xxxxxxxxxx/

I don't fully understand why all the back and forth
happening on this series, but the former Hantro patches
looked good (despite perhaps unclear commit messages).

Any issues just squashing these two commits from "[PATCH v4 00/79] Address some issues with PM runtime at media subsystem":

  media: hantro: use pm_runtime_resume_and_get()
  media: hantro: do a PM resume earlier

?

Thanks,
Ezequiel

>  drivers/staging/media/hantro/hantro_drv.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index 595e82a82728..4387edaa1d0d 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -152,13 +152,14 @@ static void device_run(void *priv)
>         src = hantro_get_src_buf(ctx);
>         dst = hantro_get_dst_buf(ctx);
>  
> -       ret = clk_bulk_enable(ctx->dev->variant->num_clocks, ctx->dev->clocks);
> -       if (ret)
> -               goto err_cancel_job;
>         ret = pm_runtime_get_sync(ctx->dev->dev);
>         if (ret < 0)
>                 goto err_cancel_job;
>  
> +       ret = clk_bulk_enable(ctx->dev->variant->num_clocks, ctx->dev->clocks);
> +       if (ret)
> +               goto err_cancel_job;
> +
>         v4l2_m2m_buf_copy_metadata(src, dst, true);
>  
>         ctx->codec_ops->run(ctx);