From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Ezequiel Garcia <ezequiel@collabora.com>
Cc: linuxarm@huawei.com, mauro.chehab@huawei.com,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Hans Verkuil <hverkuil-cisco@xs4all.nl>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Philipp Zabel <p.zabel@pengutronix.de>,
linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
linux-rockchip@lists.infradead.org,
linux-staging@lists.linux.dev
Subject: Re: [PATCH 09/25] media: hantro: do a PM resume earlier
Date: Wed, 5 May 2021 16:15:32 +0200 [thread overview]
Message-ID: <20210505161532.6d9da768@coco.lan> (raw)
In-Reply-To: <735925557d7fde1c11affeea703f8486febc825d.camel@collabora.com>
Em Wed, 05 May 2021 11:01:35 -0300
Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> On Wed, 2021-05-05 at 15:46 +0200, Mauro Carvalho Chehab wrote:
> > Em Wed, 05 May 2021 10:22:03 -0300
> > Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> >
> > > 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@collabora.com>
> > > > Fixes: 775fec69008d ("media: add Rockchip VPU JPEG encoder driver")
> > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > >
> > > It seems this is wrong now, as this series doesn't have
> > >
> > > https://lore.kernel.org/linux-media/803c39fafdd62efc6f9e4d99a372af2c6955143b.1619621413.git.mchehab+huawei@kernel.org/
> > >
> > > 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).
> >
> > There was a request to break the original /79 series into smaller ones,
> > to make easier for reviewers. So, I opted to split it into (probably)
> > 3 series:
> >
> > 1. Fixes (this series);
> > 2. "use pm_runtime_resume_and_get" for the I2C drivers;
> > 3. "use pm_runtime_resume_and_get" for remaining ones.
> >
> > Before flooding everybody's email's with series (2) and (3), better
> > to focus at the fixes first. I'll probably send the other two series
> > by tomorrow.
> >
> > > 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
> >
> > The problem is that pm_runtime_resume_and_get() was added only
> > recently (Kernel v5.10).
> >
> > So, I opted to place the fix patches before the changes, as this
> > way, most (all?) patches can be easily be backported to legacy Kernels
> > as needed.
> >
>
> Got it.
>
> Maybe the better fix would be the squash of [PATCH v4 78/79] media: hantro: use pm_runtime_resume_and_get()
> and [PATCH v4 79/79] media: hantro: do a PM resume earlier but keeping pm_runtime_get_sync.
>
> And then you can replace the pm_runtime_get_sync with pm_runtime_resume_and_get.
Works for me. So, the fixes patch will be the enclosed one, right?
Btw, I agree with Jonathan that the best would be to also move this:
clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
out of hantro_job_finish_no_pm(), as, when an error happens at
device_run(), the clock lines won't be enabled at the first place.
> Thanks,
> Ezequiel
Thanks,
Mauro
[PATCH] media: hantro: do a PM resume earlier
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@collabora.com>
Fixes: 775fec69008d ("media: add Rockchip VPU JPEG encoder driver")
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 595e82a82728..bdb57fb56f47 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -56,14 +56,12 @@ dma_addr_t hantro_get_ref(struct hantro_ctx *ctx, u64 ts)
return hantro_get_dec_buf_addr(ctx, buf);
}
-static void hantro_job_finish(struct hantro_dev *vpu,
- struct hantro_ctx *ctx,
- enum vb2_buffer_state result)
+static void hantro_job_finish_no_pm(struct hantro_dev *vpu,
+ struct hantro_ctx *ctx,
+ enum vb2_buffer_state result)
{
struct vb2_v4l2_buffer *src, *dst;
- pm_runtime_mark_last_busy(vpu->dev);
- pm_runtime_put_autosuspend(vpu->dev);
clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
@@ -81,6 +79,16 @@ static void hantro_job_finish(struct hantro_dev *vpu,
result);
}
+static void hantro_job_finish(struct hantro_dev *vpu,
+ struct hantro_ctx *ctx,
+ enum vb2_buffer_state result)
+{
+ pm_runtime_mark_last_busy(vpu->dev);
+ pm_runtime_put_autosuspend(vpu->dev);
+
+ hantro_job_finish_no_pm(vpu, ctx, result);
+}
+
void hantro_irq_done(struct hantro_dev *vpu,
enum vb2_buffer_state result)
{
@@ -152,12 +160,15 @@ static void device_run(void *priv)
src = hantro_get_src_buf(ctx);
dst = hantro_get_dst_buf(ctx);
+ ret = pm_runtime_get_sync(ctx->dev->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(ctx->dev->dev);
+ goto err_cancel_job;
+ }
+
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;
v4l2_m2m_buf_copy_metadata(src, dst, true);
@@ -165,7 +176,7 @@ static void device_run(void *priv)
return;
err_cancel_job:
- hantro_job_finish(ctx->dev, ctx, VB2_BUF_STATE_ERROR);
+ hantro_job_finish_no_pm(ctx->dev, ctx, VB2_BUF_STATE_ERROR);
}
static struct v4l2_m2m_ops vpu_m2m_ops = {
next prev parent reply other threads:[~2021-05-05 14:15 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-05 9:41 [PATCH 00/25] Fix some PM runtime issues at the media subsystem Mauro Carvalho Chehab
2021-05-05 9:41 ` [PATCH 01/25] staging: media: rkvdec: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-05-05 12:23 ` Jonathan Cameron
2021-05-05 9:41 ` [PATCH 02/25] staging: media: imx7-mipi-csis: " Mauro Carvalho Chehab
2021-05-05 11:06 ` Jonathan Cameron
2021-05-05 11:17 ` Mauro Carvalho Chehab
2021-05-05 13:56 ` Rui Miguel Silva
2021-05-05 14:23 ` Mauro Carvalho Chehab
2021-05-05 9:41 ` [PATCH 03/25] media: venus: Rework error fail recover logic Mauro Carvalho Chehab
2021-05-05 11:05 ` Jonathan Cameron
2021-05-05 9:41 ` [PATCH 04/25] media: s5p_cec: decrement usage count if disabled Mauro Carvalho Chehab
2021-05-05 12:24 ` Jonathan Cameron
2021-05-05 9:41 ` [PATCH 05/25] media: i2c: ccs-core: return the right error code at suspend Mauro Carvalho Chehab
2021-05-05 12:24 ` Jonathan Cameron
2021-05-05 12:51 ` Sakari Ailus
2021-05-05 9:41 ` [PATCH 06/25] media: i2c: imx334: fix the pm runtime get logic Mauro Carvalho Chehab
2021-05-05 11:10 ` Jonathan Cameron
2021-05-05 11:24 ` Mauro Carvalho Chehab
2021-05-05 12:26 ` Jonathan Cameron
2021-05-05 9:41 ` [PATCH 07/25] media: exynos-gsc: don't resume at remove time Mauro Carvalho Chehab
2021-05-05 12:27 ` Jonathan Cameron
2021-05-05 9:41 ` [PATCH 08/25] media: atmel: properly get pm_runtime Mauro Carvalho Chehab
2021-05-05 12:08 ` Jonathan Cameron
2021-06-10 9:04 ` Eugen.Hristev
2021-06-10 9:38 ` Mauro Carvalho Chehab
2021-06-10 12:00 ` Eugen.Hristev
2021-06-16 8:03 ` Mauro Carvalho Chehab
2021-09-06 8:03 ` Mauro Carvalho Chehab
2021-09-06 8:13 ` Eugen.Hristev
2021-09-13 10:26 ` Eugen.Hristev
2021-05-05 9:41 ` [PATCH 09/25] media: hantro: do a PM resume earlier Mauro Carvalho Chehab
2021-05-05 11:34 ` Jonathan Cameron
2021-05-05 13:22 ` Ezequiel Garcia
2021-05-05 13:46 ` Mauro Carvalho Chehab
2021-05-05 14:01 ` Ezequiel Garcia
2021-05-05 14:15 ` Mauro Carvalho Chehab [this message]
2021-05-05 9:42 ` [PATCH 10/25] media: marvel-ccic: fix some issues when getting pm_runtime Mauro Carvalho Chehab
2021-05-05 9:42 ` [PATCH 11/25] media: mdk-mdp: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-05-05 9:42 ` [PATCH 12/25] media: rcar_fdp1: simplify error check logic at fdp_open() Mauro Carvalho Chehab
2021-05-05 9:48 ` Sergei Shtylyov
2021-05-05 9:42 ` [PATCH 13/25] media: rcar_fdp1: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-05-05 12:31 ` Jonathan Cameron
2021-05-05 9:42 ` [PATCH 14/25] media: renesas-ceu: Properly check for PM errors Mauro Carvalho Chehab
2021-05-05 9:56 ` Jacopo Mondi
2021-05-05 9:42 ` [PATCH 15/25] media: s5p: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-05-05 12:31 ` Jonathan Cameron
2021-05-05 9:42 ` [PATCH 16/25] media: am437x: " Mauro Carvalho Chehab
2021-05-05 12:32 ` Jonathan Cameron
2021-05-08 13:01 ` Lad, Prabhakar
2021-05-05 9:42 ` [PATCH 17/25] media: sh_vou: " Mauro Carvalho Chehab
2021-05-05 9:42 ` [PATCH 18/25] media: mtk-vcodec: fix PM runtime get logic Mauro Carvalho Chehab
2021-05-05 12:32 ` Jonathan Cameron
2021-05-05 9:42 ` [PATCH 19/25] media: s5p-jpeg: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-05-05 12:33 ` Jonathan Cameron
2021-05-05 9:42 ` [PATCH 20/25] media: sti/delta: use pm_runtime_resume_and_get() Mauro Carvalho Chehab
2021-05-05 12:01 ` Jonathan Cameron
2021-05-05 12:33 ` Jonathan Cameron
2021-05-05 9:42 ` [PATCH 21/25] media: sunxi: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-05-05 9:42 ` [PATCH 22/25] media: sti/bdisp: " Mauro Carvalho Chehab
2021-05-05 12:34 ` Jonathan Cameron
2021-05-05 9:42 ` [PATCH 23/25] media: exynos4-is: " Mauro Carvalho Chehab
2021-05-05 12:20 ` Jonathan Cameron
2021-05-05 9:42 ` [PATCH 24/25] media: exynos-gsc: " Mauro Carvalho Chehab
2021-05-05 12:34 ` Jonathan Cameron
2021-05-05 9:42 ` [PATCH 25/25] media: i2c: ccs-core: " Mauro Carvalho Chehab
2021-05-05 10:34 ` Sakari Ailus
2021-05-05 10:57 ` Mauro Carvalho Chehab
2021-05-05 10:58 ` Mauro Carvalho Chehab
2021-05-05 12:35 ` Jonathan Cameron
2021-05-05 14:06 ` Mauro Carvalho Chehab
2021-05-05 16:36 ` Jonathan Cameron
2021-05-05 11:02 ` Sakari Ailus
2021-05-06 15:11 ` [PATCH 00/25] Fix some PM runtime issues at the media subsystem Mauro Carvalho Chehab
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210505161532.6d9da768@coco.lan \
--to=mchehab+huawei@kernel.org \
--cc=ezequiel@collabora.com \
--cc=gregkh@linuxfoundation.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-staging@lists.linux.dev \
--cc=linuxarm@huawei.com \
--cc=mauro.chehab@huawei.com \
--cc=mchehab@kernel.org \
--cc=p.zabel@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).