From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4425C433DF for ; Thu, 11 Jun 2020 12:08:35 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7AA0C20760 for ; Thu, 11 Jun 2020 12:08:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="R5i1HG9E"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="kIE33h7L" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7AA0C20760 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=eN8uAIV/Z9/G2UJH6XT546+l5/D5KPqe025tn8CfNAU=; b=R5i1HG9EVx+G7Q x74Sk+9sTFar26jYFSJbm3ytT/B/N02kOTYOytwvL8Sv98N9FdLMFGcp8f0STHqudpNvQDudiKSzz 7ULytzhpBmkdq9s2ecHbqY9tYcxtOqI4fSlLeYSUg+s/8AoJ3IuYu8SpTVQDoTawWKhI732Hw5UFB XokgZdePQgNmmuqRUoyDhJAK0zRaTo/Rx0/9geUDg0TdAZavIfEX9U4Hy92VKz+rVOxCrLqKRjhpt QoofNdN5iNbqySRqpSyCA/ag2PkYh9oyHw4vHO2RVimHj7LxdQCtlgELg7dlsaBQbvE5YRemwqjAJ dDUnVqea7HXJKlbePQiQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jjM0R-0005ne-QZ; Thu, 11 Jun 2020 12:08:27 +0000 Received: from mail-wm1-x344.google.com ([2a00:1450:4864:20::344]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jjM0O-0005mt-KU for linux-mediatek@lists.infradead.org; Thu, 11 Jun 2020 12:08:26 +0000 Received: by mail-wm1-x344.google.com with SMTP id q25so4815933wmj.0 for ; Thu, 11 Jun 2020 05:08:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=l9/1IkVcWGgYEUiuZWnQIOE6uUjuPXFr0zUWFBzg324=; b=kIE33h7Lf2r+tx6hQ/ix/6+/19I3lB8Ldey0jlumYHalyyXnEmqjH3Ykw4mPIPgrcv 4dkNF36frta7vt0Et8JEAJhO/OWx50XwV4omuBKChHuTgqeA1dZ4TD8tQT9UZmcYYDBD iVtWJnbCi9wgpwY0FWz7KnMUpzhRVlUGS2xbU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=l9/1IkVcWGgYEUiuZWnQIOE6uUjuPXFr0zUWFBzg324=; b=Sc7WqQS0F7il/JKfwHJIqQUhnHY+UDLBC28iAq44/kh37rcAJtUPprwWPt0PQo7mgq 6milhWR7TQHqxzfvGF4sls4DDEQBdBqQRUWpwGIgbUNr7CFxf2zXXlnG2w7SKmh4Nab6 GBIp9vF8qRXElDDNyCE5BgnTmGWvkW72Bk57hKIlVXMsZWQZsl2nXSSSL4gNSUxstTWT 5ow47BNhL3ijSnFdKIZ1GOb5KR1bcoJXKvQaP7SHDU6RabmMdBG2OUUS67GcxGzDeAHS loZjBXGeWYmmcpneg/XYMIMwZIX3LU2lHuaZsUdGVmsHl6C+htV298HHYbnqGxd53pAA Vn8g== X-Gm-Message-State: AOAM532HdVsk9a5qMyH8oRRWqXX3veto1Bxra0i/tDqg9LnDtRtl3Raz iWoq/e7O5Sn8nlTVIoQGxhKjLA== X-Google-Smtp-Source: ABdhPJw/4tFHFEvAplryswUi9eYGwKBoFlfEALinuD5TCcI+/+xf1W81Wni9oObHj3ji4UEsPf8kKQ== X-Received: by 2002:a1c:7416:: with SMTP id p22mr7953394wmc.95.1591877303012; Thu, 11 Jun 2020 05:08:23 -0700 (PDT) Received: from chromium.org (205.215.190.35.bc.googleusercontent.com. [35.190.215.205]) by smtp.gmail.com with ESMTPSA id 5sm6618926wrr.5.2020.06.11.05.08.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Jun 2020 05:08:22 -0700 (PDT) Date: Thu, 11 Jun 2020 12:08:20 +0000 From: Tomasz Figa To: Xia Jiang Subject: Re: [PATCH RESEND v9 05/18] media: platform: Improve power on and power off flow Message-ID: <20200611120820.GC135826@chromium.org> References: <20200604090553.10861-1-xia.jiang@mediatek.com> <20200604090553.10861-7-xia.jiang@mediatek.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200604090553.10861-7-xia.jiang@mediatek.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200611_050824_690152_04C5BEC5 X-CRM114-Status: GOOD ( 18.61 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: drinkcat@chromium.org, devicetree@vger.kernel.org, mojahsu@chromium.org, srv_heupstream@mediatek.com, Rick Chang , senozhatsky@chromium.org, linux-kernel@vger.kernel.org, maoguang.meng@mediatek.com, Mauro Carvalho Chehab , sj.huang@mediatek.com, Rob Herring , Matthias Brugger , Hans Verkuil , linux-mediatek@lists.infradead.org, Marek Szyprowski , linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org Hi Xia, On Thu, Jun 04, 2020 at 05:05:40PM +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 > --- > v9: use pm_runtime_put() to replace pm_runtime_put_sync() > --- > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 27 +++++-------------- > 1 file changed, 6 insertions(+), 21 deletions(-) > > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c > index 12609ca46fd9..fb624385969e 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(ctx->jpeg->dev); This patch itself is correct and feel free to add my Reviewed-by: Tomasz Figa However, it looks like there might be one more problem with this driver. What happens if the hardware locks up? The driver doesn't seem to take any measures to detect that and recover the system. If you take a look at other drivers, e.g. the MTK FD driver, there is a delayed work scheduled before starting the hardware and canceled in the interrupt handler. If the delayed work is executed, it resets the hardware and reports the failure to V4L2, so that the execution can continue from next frames. This should be fixed in a separate patch, could be outside of this series. Best regards, Tomasz _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek