Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
From: Tomasz Figa <tfiga@chromium.org>
To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Cc: "Hans Verkuil" <hverkuil@xs4all.nl>,
	"Jerry-ch Chen" <Jerry-Ch.chen@mediatek.com>,
	"Laurent Pinchart" <laurent.pinchart+renesas@ideasonboard.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Pi-Hsun Shih" <pihsun@chromium.org>,
	yuzhao@chromium.org, zwisler@chromium.org,
	"moderated list:ARM/Mediatek SoC support"
	<linux-mediatek@lists.infradead.org>,
	"list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,"
	<linux-arm-kernel@lists.infradead.org>,
	"Sean Cheng (鄭昇弘)" <Sean.Cheng@mediatek.com>,
	"Sj Huang" <sj.huang@mediatek.com>,
	"Christie Yu (游雅惠)" <christie.yu@mediatek.com>,
	"Frederic Chen (陳俊元)" <frederic.chen@mediatek.com>,
	"Jungo Lin (林明俊)" <jungo.lin@mediatek.com>,
	"Rynn Wu (吳育恩)" <Rynn.Wu@mediatek.com>,
	"Linux Media Mailing List" <linux-media@vger.kernel.org>,
	srv_heupstream <srv_heupstream@mediatek.com>,
	linux-devicetree <devicetree@vger.kernel.org>,
	"Jerry-ch Chen" <jerry-ch.chen@mediatek.corp-partner.google.com>
Subject: Re: [RFC PATCH V4 1/4] media: v4l2-mem2mem: add v4l2_m2m_suspend, v4l2_m2m_resume
Date: Wed, 10 Jun 2020 19:03:56 +0000
Message-ID: <20200610190356.GJ201868@chromium.org> (raw)
In-Reply-To: <CAAEAJfDboyra5XcEiviJP_3Kk-mDNZH5nstDa_7P7XCV7bu7Pg@mail.gmail.com>

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 <tfiga@chromium.org> wrote:
> >
> > On Wed, Jun 10, 2020 at 12:29 PM Hans Verkuil <hverkuil@xs4all.nl> 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 <pihsun@chromium.org>
> > > >>
> > > >> 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.

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.

> 
> 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.

Best regards,
Tomasz

  reply index

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-04 12:47 [RFC PATCH V4 0/4] media: platform: Add support for Face Detection (FD) on mt8183 SoC Jerry-ch Chen
2019-12-04 12:47 ` [RFC PATCH V4 1/4] media: v4l2-mem2mem: add v4l2_m2m_suspend, v4l2_m2m_resume Jerry-ch Chen
2020-05-21 17:11   ` Tomasz Figa
2020-05-22  6:01     ` Jerry-ch Chen
2020-06-10 10:28     ` Hans Verkuil
2020-06-10 10:32       ` Tomasz Figa
2020-06-10 18:52         ` Ezequiel Garcia
2020-06-10 19:03           ` Tomasz Figa [this message]
2020-06-10 19:14             ` Ezequiel Garcia
2020-06-10 19:26               ` Tomasz Figa
2020-06-14 22:43                 ` Ezequiel Garcia
2019-12-04 12:47 ` [RFC PATCH V4 2/4] dt-bindings: mt8183: Added FD dt-bindings Jerry-ch Chen
2019-12-04 18:58   ` Rob Herring
2020-05-06  8:41     ` Jerry-ch Chen
2019-12-04 12:47 ` [RFC PATCH V4 3/4] dts: arm64: mt8183: Add FD nodes Jerry-ch Chen
2019-12-04 12:47 ` [RFC PATCH V4 4/4] platform: mtk-isp: Add Mediatek FD driver Jerry-ch Chen
2020-05-21 18:28   ` Tomasz Figa
2020-05-22 14:10     ` Jerry-ch Chen
2020-05-25 12:24       ` Tomasz Figa
2020-05-29 12:26         ` Jerry-ch Chen
2020-05-29 12:59           ` Tomasz Figa
2020-06-01 10:37             ` Jerry-ch Chen
2020-05-08  2:02 ` [RFC PATCH V4 0/4] media: platform: Add support for Face Detection (FD) on mt8183 SoC Jerry-ch Chen
2020-05-13 21:45   ` Tomasz Figa
2020-05-21 18:38     ` Tomasz Figa
2020-06-30 14:10       ` Jerry-ch Chen
2020-06-30 17:19         ` Tomasz Figa

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=20200610190356.GJ201868@chromium.org \
    --to=tfiga@chromium.org \
    --cc=Jerry-Ch.chen@mediatek.com \
    --cc=Rynn.Wu@mediatek.com \
    --cc=Sean.Cheng@mediatek.com \
    --cc=christie.yu@mediatek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=frederic.chen@mediatek.com \
    --cc=hverkuil@xs4all.nl \
    --cc=jerry-ch.chen@mediatek.corp-partner.google.com \
    --cc=jungo.lin@mediatek.com \
    --cc=laurent.pinchart+renesas@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=mchehab@kernel.org \
    --cc=pihsun@chromium.org \
    --cc=sj.huang@mediatek.com \
    --cc=srv_heupstream@mediatek.com \
    --cc=yuzhao@chromium.org \
    --cc=zwisler@chromium.org \
    /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

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git