From: Tomasz Figa <tfiga@chromium.org>
To: Xia Jiang <xia.jiang@mediatek.com>
Cc: drinkcat@chromium.org, devicetree@vger.kernel.org,
mojahsu@chromium.org, srv_heupstream@mediatek.com,
Rick Chang <rick.chang@mediatek.com>,
senozhatsky@chromium.org, linux-kernel@vger.kernel.org,
maoguang.meng@mediatek.com,
Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
sj.huang@mediatek.com, Rob Herring <robh+dt@kernel.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
Hans Verkuil <hverkuil-cisco@xs4all.nl>,
linux-mediatek@lists.infradead.org,
Marek Szyprowski <m.szyprowski@samsung.com>,
linux-arm-kernel@lists.infradead.org,
linux-media@vger.kernel.org
Subject: Re: [PATCH v8 05/14] media: platform: Improve power on and power off flow
Date: Thu, 21 May 2020 15:22:53 +0000 [thread overview]
Message-ID: <20200521152253.GE209565@chromium.org> (raw)
In-Reply-To: <20200403094033.8288-6-xia.jiang@mediatek.com>
Hi Xia,
On Fri, Apr 03, 2020 at 05:40:24PM +0800, Xia Jiang wrote:
> Call pm_runtime_get_sync() before starting a frame and then
> pm_runtime_put() after completing it. This can save power for the time
> between processing two frames.
>
> Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> ---
> .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 27 +++++--------------
> 1 file changed, 6 insertions(+), 21 deletions(-)
>
Thank you for the patch. Please see my comments inline.
> diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> index a536fa95b3d6..dd5cadd101ef 100644
> --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> @@ -710,23 +710,6 @@ static struct vb2_v4l2_buffer *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
> return v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> }
>
> -static int mtk_jpeg_start_streaming(struct vb2_queue *q, unsigned int count)
> -{
> - struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> - struct vb2_v4l2_buffer *vb;
> - int ret = 0;
> -
> - ret = pm_runtime_get_sync(ctx->jpeg->dev);
> - if (ret < 0)
> - goto err;
> -
> - return 0;
> -err:
> - while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> - v4l2_m2m_buf_done(vb, VB2_BUF_STATE_QUEUED);
> - return ret;
> -}
> -
> static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> {
> struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> @@ -751,8 +734,6 @@ static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
>
> while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> -
> - pm_runtime_put_sync(ctx->jpeg->dev);
> }
>
> static const struct vb2_ops mtk_jpeg_qops = {
> @@ -761,7 +742,6 @@ static const struct vb2_ops mtk_jpeg_qops = {
> .buf_queue = mtk_jpeg_buf_queue,
> .wait_prepare = vb2_ops_wait_prepare,
> .wait_finish = vb2_ops_wait_finish,
> - .start_streaming = mtk_jpeg_start_streaming,
> .stop_streaming = mtk_jpeg_stop_streaming,
> };
>
> @@ -812,7 +792,7 @@ static void mtk_jpeg_device_run(void *priv)
> struct mtk_jpeg_src_buf *jpeg_src_buf;
> struct mtk_jpeg_bs bs;
> struct mtk_jpeg_fb fb;
> - int i;
> + int i, ret;
>
> src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> @@ -832,6 +812,10 @@ static void mtk_jpeg_device_run(void *priv)
> return;
> }
>
> + ret = pm_runtime_get_sync(jpeg->dev);
> + if (ret < 0)
> + goto dec_end;
> +
> mtk_jpeg_set_dec_src(ctx, &src_buf->vb2_buf, &bs);
> if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, &dst_buf->vb2_buf, &fb))
> goto dec_end;
> @@ -957,6 +941,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> v4l2_m2m_buf_done(src_buf, buf_state);
> v4l2_m2m_buf_done(dst_buf, buf_state);
> v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> + pm_runtime_put_sync(ctx->jpeg->dev);
The _sync variant explicitly waits until the asynchronous PM operation
completes. This is usually undesired, because the CPU stays blocked for
no good reason. In this context it is actually a bug, because this is an
interrupt handler and it's not allowed to sleep. I wonder why this
actually didn't crash in your testing. Please change to the regular
pm_runtime_put().
Best regards,
Tomasz
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
next prev parent reply other threads:[~2020-05-21 15:23 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-03 9:40 [PATCH v8 00/14] Add support for mt2701 JPEG ENC support Xia Jiang
2020-04-03 9:40 ` [PATCH v8 01/14] media: platform: Improve subscribe event flow for bug fixing Xia Jiang
2020-05-21 13:45 ` Tomasz Figa
2020-04-03 9:40 ` [PATCH v8 02/14] media: platform: Improve queue set up " Xia Jiang
2020-05-21 13:46 ` Tomasz Figa
2020-04-03 9:40 ` [PATCH v8 03/14] media: platform: Improve getting and requesting irq " Xia Jiang
2020-05-21 13:47 ` Tomasz Figa
2020-04-03 9:40 ` [PATCH v8 04/14] media: platform: Change the fixed device node number to unfixed value Xia Jiang
2020-05-11 8:39 ` Hans Verkuil
2020-05-21 13:59 ` Tomasz Figa
2020-06-05 6:02 ` Xia Jiang
2020-04-03 9:40 ` [PATCH v8 05/14] media: platform: Improve power on and power off flow Xia Jiang
2020-05-21 15:22 ` Tomasz Figa [this message]
2020-06-05 6:03 ` Xia Jiang
2020-04-03 9:40 ` [PATCH v8 06/14] media: platform: Improve the implementation of the system PM ops Xia Jiang
2020-05-21 15:32 ` Tomasz Figa
2020-05-27 1:52 ` Xia Jiang
2020-05-27 14:46 ` Tomasz Figa
2020-04-03 9:40 ` [PATCH v8 07/14] media: platform: Use kernel native functions for improving code quality Xia Jiang
2020-05-21 15:41 ` Tomasz Figa
2020-06-05 6:41 ` Xia Jiang
2020-04-03 9:40 ` [PATCH v8 08/14] media: platform: Change case " Xia Jiang
2020-05-11 8:37 ` Hans Verkuil
2020-06-05 8:04 ` Xia Jiang
2020-04-03 9:40 ` [PATCH v8 09/14] media: platform: Change MTK_JPEG_COMP_MAX macro definition location Xia Jiang
2020-05-21 15:44 ` Tomasz Figa
2020-04-03 9:40 ` [PATCH v8 10/14] media: platform: Delete redundant code for improving code quality Xia Jiang
2020-05-21 15:49 ` Tomasz Figa
2020-06-05 6:41 ` Xia Jiang
2020-04-03 9:40 ` [PATCH v8 11/14] media: dt-bindings: Add jpeg enc device tree node document Xia Jiang
2020-05-21 16:00 ` Tomasz Figa
2020-06-18 3:40 ` Xia Jiang
2020-06-18 12:34 ` Tomasz Figa
2020-04-03 9:40 ` [PATCH v8 12/14] arm: dts: Add jpeg enc device tree node Xia Jiang
2020-04-07 3:52 ` Yingjoe Chen
2020-04-03 9:40 ` [PATCH v8 13/14] media: platform: Rename jpeg dec file name Xia Jiang
2020-05-21 16:02 ` Tomasz Figa
2020-04-03 9:40 ` [PATCH v8 14/14] media: platform: Add jpeg dec/enc feature Xia Jiang
2020-05-11 9:04 ` Hans Verkuil
2020-05-21 16:08 ` Tomasz Figa
2020-06-05 8:07 ` Xia Jiang
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=20200521152253.GE209565@chromium.org \
--to=tfiga@chromium.org \
--cc=devicetree@vger.kernel.org \
--cc=drinkcat@chromium.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=m.szyprowski@samsung.com \
--cc=maoguang.meng@mediatek.com \
--cc=matthias.bgg@gmail.com \
--cc=mchehab+samsung@kernel.org \
--cc=mojahsu@chromium.org \
--cc=rick.chang@mediatek.com \
--cc=robh+dt@kernel.org \
--cc=senozhatsky@chromium.org \
--cc=sj.huang@mediatek.com \
--cc=srv_heupstream@mediatek.com \
--cc=xia.jiang@mediatek.com \
/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).