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().
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/staging/media/hantro/hantro_drv.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 25fa36e7e773..67de6b15236d 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -160,14 +160,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_resume_and_get(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);
--
2.30.2
Hi Mauro,
Thanks a lot for taking care of this.
On Wed, 2021-04-28 at 16:52 +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().
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Clocks could be behind power-domains, IIRC, so this change
is fixing that.
However, this has ever been a problem for this driver,
so I don't think it makes sense to bother with Fixes tag.
Reviewed-by: Ezequiel Garcia <[email protected]>
Thanks,
Ezequiel
> ---
> drivers/staging/media/hantro/hantro_drv.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index 25fa36e7e773..67de6b15236d 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -160,14 +160,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_resume_and_get(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);
Em Wed, 28 Apr 2021 14:17:50 -0300
Ezequiel Garcia <[email protected]> escreveu:
> Hi Mauro,
>
> Thanks a lot for taking care of this.
>
> On Wed, 2021-04-28 at 16:52 +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().
> >
> > Signed-off-by: Mauro Carvalho Chehab <[email protected]>
>
> Clocks could be behind power-domains, IIRC, so this change
> is fixing that.
>
> However, this has ever been a problem for this driver,
> so I don't think it makes sense to bother with Fixes tag.
I would prefer to move this patch to the first part of this
series, together with other fixes, rebasing it to apply cleanly
before the pm_runtime_resume_and_get() patch, with:
Fixes: 775fec69008d ("media: add Rockchip VPU JPEG encoder driver")
This way, people that could be interested on backporting it will be
capable to apply it as is to stable Kernel releases that came
with this driver.
>
> Reviewed-by: Ezequiel Garcia <[email protected]>
>
> Thanks,
> Ezequiel
>
> > ---
> > drivers/staging/media/hantro/hantro_drv.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> > index 25fa36e7e773..67de6b15236d 100644
> > --- a/drivers/staging/media/hantro/hantro_drv.c
> > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > @@ -160,14 +160,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_resume_and_get(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);
>
>
Thanks,
Mauro