linux-kernel - Re: [PATCH v3 16/24] media: Add i.MX media core driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1485257846.3600.106.camel@pengutronix.de>
Date:   Tue, 24 Jan 2017 12:37:26 +0100
From:   Philipp Zabel <p.zabel@...gutronix.de>
To:     Steve Longerbeam <steve_longerbeam@...tor.com>
Cc:     Steve Longerbeam <slongerbeam@...il.com>, robh+dt@...nel.org,
        mark.rutland@....com, shawnguo@...nel.org, kernel@...gutronix.de,
        fabio.estevam@....com, linux@...linux.org.uk, mchehab@...nel.org,
        hverkuil@...all.nl, nick@...anahar.org, markus.heiser@...marIT.de,
        laurent.pinchart+renesas@...asonboard.com, bparrot@...com,
        geert@...ux-m68k.org, arnd@...db.de, sudipm.mukherjee@...il.com,
        minghsiu.tsai@...iatek.com, tiffany.lin@...iatek.com,
        jean-christophe.trotin@...com, horms+renesas@...ge.net.au,
        niklas.soderlund+renesas@...natech.se, robert.jarzmik@...e.fr,
        songjun.wu@...rochip.com, andrew-ct.chen@...iatek.com,
        gregkh@...uxfoundation.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-media@...r.kernel.org, devel@...verdev.osuosl.org
Subject: Re: [PATCH v3 16/24] media: Add i.MX media core driver

Hi Steve,

On Mon, 2017-01-23 at 17:45 -0800, Steve Longerbeam wrote:
> 
> On 01/23/2017 05:38 PM, Steve Longerbeam wrote:
> >
> >>
> >>> Second, ignoring the above locking issue for a moment,
> >>> v4l2_pipeline_pm_use()
> >>> will call s_power on the sensor _first_, then the mipi csi-2 s_power,
> >>> when executing
> >>> media-ctl -l '"ov5640 1-003c":0 -> "imx6-mipi-csi2":0[1]'. Which is the
> >>> wrong order.
> >>> In my version which enforces the correct power on order, the mipi csi-2
> >>> s_power
> >>> is called first in that link setup, followed by the sensor.
> >> I don't understand why you want to power up subdevs as soon as the links
> >> are established.
> >
> > Because that is the precedence, all other media drivers do pipeline
> > power on/off at link_notify. And v4l2_pipeline_link_notify() was written
> > as a link_notify method.
> >
> >>   Shouldn't that rather be done for all subdevices in the
> >> pipeline when the corresponding capture device is opened?
> >
> > that won't work. There's no guarantee the links will be established
> > at capture device open time.

If the device is opened before the links are established, it won't be
usable anyway. And I think the connected pipeline should be locked in
place while the video device is opened. Is there any reason to ever open
the video device and only then start linking entities?

> ugh, maybe v4l2_pipeline_pm_use() would work at open/release. If there are
> no links yet, it would basically be a no-op. And stream on requires 
> opening the
> device, and the pipeline links should be established by then, so this 
> might be
> fine, looking into this too.

Thanks for looking into it, at least I had that working for the
TC358743->MIPI-CSI2 link in my driver.

> >> It seems to me that powering up the pipeline should be the last step
> >> before userspace actually starts the capture.
> >
> > Well, I'm ok with moving pipeline power on/off to start/stop streaming.
> > I would actually prefer to do it then, I only chose at link_notify 
> > because of precedence. I'll look into it.

That might be too late, though. I would expect STREAMON/STREAMOFF to be
a rather fast operation as all the slow preparation could be at open /
REQBUFS time. Also, there might be sensors that need to be powered on to
handle the v4l2_ctrl passthrough?

regards
Philipp