lkml.org 
[lkml]   [2020]   [May]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v8 05/14] media: platform: Improve power on and power off flow
    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

    \
     
     \ /
      Last update: 2020-05-21 17:23    [W:4.166 / U:0.556 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site