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=-0.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no 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 0B41AC433DF for ; Wed, 10 Jun 2020 19:15:19 +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 D354E20656 for ; Wed, 10 Jun 2020 19:15:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="aNq/T4LF"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=vanguardiasur-com-ar.20150623.gappssmtp.com header.i=@vanguardiasur-com-ar.20150623.gappssmtp.com header.b="aJfwS8QK" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D354E20656 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=vanguardiasur.com.ar 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:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=kr1825sCIiiWSefSQusNNLNjPrybtjuRkr4AyHdKRug=; b=aNq/T4LFA0dydq aAjXM1sLYObBms838TD5sATw6If/DpaZeM4KKna76OVIRW3RQYWrEQmyi09k5yMQxlnNTZHQuNkgM kYbOkjWIMx2TAV1OH5cccSZNGWUIhZkMuUDMOC8v/QIk/NXVtsCtQ2HSyMmXUscbImXG12jw3gKNg zggvBNdL9lcV63Ytx54S/I09qKy6DeQUVKiyuVbqAzoHdL1jpwgMZN6JLmuIDE51irQfQmhrL8ZsZ 4ZxWdQBdpCSPN316Itd3aZIG9xt4yE4hHRU7s+Inffl4EhdMRAiDsJvhicW9uY0B7xbUXCfsy/rrt cXZoBaYBnINi+QV2UBRg==; 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 1jj6Bl-0001eg-8e; Wed, 10 Jun 2020 19:15:05 +0000 Received: from mail-ed1-x541.google.com ([2a00:1450:4864:20::541]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jj6BZ-0001Lq-7l for linux-mediatek@lists.infradead.org; Wed, 10 Jun 2020 19:14:54 +0000 Received: by mail-ed1-x541.google.com with SMTP id d15so2192270edm.10 for ; Wed, 10 Jun 2020 12:14:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vanguardiasur-com-ar.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=hY1EqCjTHPxqFfI2Ha7phQq/vyBFFXs9xfoG48pRam8=; b=aJfwS8QKSiJ6H8efGbgr8JxwnYO1ycqGIy32HcO1eG3MGZPBt66ZyhwXGNS+ghpCA8 MlCtqoDfJ8M3mbV1AgWLpGWf+BrKi4CjjSE3kapuXpX/wW+K7yKW/Wl7WT8jowM/Ie+q dLhJivuS+r93N3eMejYqC6BL/icw9TbscMS/1zgtaCagnljzPRliDuw0nNJm6Nu2lNR5 wxcYpKvMkX70bEiohd4mCcUK6HDV5+JG8XRRb9tMfhGLnF/QWmnmCU+QP4L3ZZLa4KnY tJsgeBZGfHZQjF0s4mZCD+HuR/3RzUm1NynteTQZhsux6044q+WAmCrBTpi8exknwZ9N RJ9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=hY1EqCjTHPxqFfI2Ha7phQq/vyBFFXs9xfoG48pRam8=; b=afFnMUa4A848Ruxv1ehI1HevD1l/vBEBtpNDA6icdD8n4sN7a4L5v2V4b7IXQUOswr eCcyxEIIaYGmkj3hFao22bwDlpIqvlnUHxThDy1hm/jY9BzlouLYVH1D3pnCkcCNH+Us DTcy9RBWn2pUM0yry/rkwKciEBcMwtzCLG/5VP63lyN9DxnKIEpOXNZGu3eNYstcLA4i Yt4DmawbxzghH8UKaeOqSmUaWhBAN/c/ohbDAPgQTRmdYELgG7U9h8aHbgC4vvbqjkZZ XJEe+A8aUMGl0S2rZTpXhlt8yWkfy0GtPSf8VjNxo+RDpb8bfTMpJRIUfkN9gEC59S1u xxlQ== X-Gm-Message-State: AOAM532KfGTxf5MU/QwVgE9glXHH/BIiIt86ai/JfkntvKDjXS6l5kj0 qaGgY7KLlvwQnOa9OXGwlgLbFOl9NqoEs1K4tjW9kA== X-Google-Smtp-Source: ABdhPJz9i/RGn/VGDboPOGapRKfIYiBLxFTRAkrJWXFnfcy4J6pqbtf/64Ut6cbDRPpojftZ8tGKiCLnT+y4+PGXC7s= X-Received: by 2002:a05:6402:1153:: with SMTP id g19mr3693132edw.127.1591816481715; Wed, 10 Jun 2020 12:14:41 -0700 (PDT) MIME-Version: 1.0 References: <20191204124732.10932-1-Jerry-Ch.chen@mediatek.com> <20191204124732.10932-2-Jerry-Ch.chen@mediatek.com> <20200521171101.GA243874@chromium.org> <20200610190356.GJ201868@chromium.org> In-Reply-To: <20200610190356.GJ201868@chromium.org> From: Ezequiel Garcia Date: Wed, 10 Jun 2020 16:14:30 -0300 Message-ID: Subject: Re: [RFC PATCH V4 1/4] media: v4l2-mem2mem: add v4l2_m2m_suspend, v4l2_m2m_resume To: Tomasz Figa X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200610_121453_283444_DA03439D X-CRM114-Status: GOOD ( 28.93 ) 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: linux-devicetree , =?UTF-8?B?U2VhbiBDaGVuZyAo6YSt5piH5byYKQ==?= , Laurent Pinchart , =?UTF-8?B?UnlubiBXdSAo5ZCz6IKy5oGpKQ==?= , zwisler@chromium.org, srv_heupstream , Jerry-ch Chen , Jerry-ch Chen , Hans Verkuil , =?UTF-8?B?SnVuZ28gTGluICjmnpfmmI7kv4op?= , Sj Huang , yuzhao@chromium.org, "moderated list:ARM/Mediatek SoC support" , Pi-Hsun Shih , =?UTF-8?B?RnJlZGVyaWMgQ2hlbiAo6Zmz5L+K5YWDKQ==?= , Matthias Brugger , =?UTF-8?B?Q2hyaXN0aWUgWXUgKOa4uOmbheaDoCk=?= , Mauro Carvalho Chehab , "list@263.net:IOMMU DRIVERS , Joerg Roedel , " , Linux Media Mailing List 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 On Wed, 10 Jun 2020 at 16:03, Tomasz Figa wrote: > > On Wed, Jun 10, 2020 at 03:52:39PM -0300, Ezequiel Garcia wrote: > > Hi everyone, > > > > Thanks for the patch. > > > > On Wed, 10 Jun 2020 at 07:33, Tomasz Figa wrote: > > > > > > On Wed, Jun 10, 2020 at 12:29 PM Hans Verkuil wrote: > > > > > > > > On 21/05/2020 19:11, Tomasz Figa wrote: > > > > > Hi Jerry, > > > > > > > > > > On Wed, Dec 04, 2019 at 08:47:29PM +0800, Jerry-ch Chen wrote: > > > > >> From: Pi-Hsun Shih > > > > >> > > > > >> Add two functions that can be used to stop new jobs from being queued / > > > > >> continue running queued job. This can be used while a driver using m2m > > > > >> helper is going to suspend / wake up from resume, and can ensure that > > > > >> there's no job running in suspend process. > [snip] > > > > > > > > I assume this will be part of a future patch series that calls these new functions? > > > > > > The mtk-jpeg encoder series depends on this patch as well, so I guess > > > it would go together with whichever is ready first. > > > > > > I would also envision someone changing the other existing drivers to > > > use the helpers, as I'm pretty much sure some of them don't handle > > > suspend/resume correctly. > > > > > > > This indeed looks very good. If I understood the issue properly, > > the change would be useful for both stateless (e.g. hantro, et al) > > and stateful (e.g. coda) codecs. > > > > Hantro uses pm_runtime_force_suspend, and I believe that > > could is enough for proper suspend/resume operation. > > Unfortunately, no. :( > > If the decoder is already decoding a frame, that would forcefully power > off the hardware and possibly even cause a system lockup if we are > unlucky to gate a clock in the middle of a bus transaction. > pm_runtime_force_suspend calls pm_runtime_disable, which says: """ Increment power.disable_depth for the device and if it was zero previously, cancel all pending runtime PM requests for the device and wait for all operations in progress to complete. """ Doesn't this mean it waits for the current job (if there is one) and prevents any new jobs to be issued? > I just inspected the code now and actually found one more bug in its > power management handling. device_run() calls clk_bulk_enable() before > pm_runtime_get_sync(), but only the latter is guaranteed to actually > power on the relevant power domains, so we end up clocking unpowered > hardware. > How about we just move clk_enable/disable to runtime PM? Since we use autosuspend delay, it theoretically has some impact, which is why I was refraining from doing so. I can't decide if this impact would be marginal or significant. > > > > I'm not seeing any code in CODA to handle this, so not sure > > how it's handling suspend/resume. > > > > Maybe we can have CODA as the first user, given it's a well-maintained > > driver and should be fairly easy to test. > > I remember checking a number of drivers using the m2m helpers randomly > and none of them implemented suspend/resume correctly. I suppose that > was not discovered because normally the userspace itself would stop the > operation before the system is suspended, although it's not an API > guarantee. > Indeed. Do you have any recomendations for how we could test this case to make sure we are handling it correctly? > Best regards, > Tomasz _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek