Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.

[Philipp Zabel <p.zabel@pengutronix.de>]

On Wed, 2017-09-20 at 13:24 +0200, Hans Verkuil wrote:
> On 09/20/17 13:00, Dave Stevenson wrote:
[...]
> > It is communicated over the subdevice API - tc358743_g_mbus_config
> > reports back the appropriate number of lanes to the receiver
> > subdevice.
> > A suitable v4l2_subdev_has_op(dev->sensor, video, g_mbus_config) call
> > as you're starting streaming therefore gives you the correct
> > information. That's what I've just done for the BCM283x Unicam
> > driver[1], but admittedly I'm not using the media controller API which
> > i.MX6 is.
> 
> Shouldn't this information come from the device tree? The g_mbus_config
> op is close to being deprecated or even removed. There are currently only
> two obscure V4L2 bridge drivers that call it. It dates from pre-DT times
> I rather not see it used in a new bridge driver.
> 
> The problem is that contains data that belongs to the DT (hardware
> capabilities). Things that can actually change dynamically should be
> communicated via another op. We don't have that, so that should be
> created.
> 
> I've CC-ed Sakari, he is the specialist for such things.

The total number of MIPI CSI-2 lanes (as well as their order) and the
list of allowed link frequencies are static and come from the device
tree.

But the possible combinations of link frequency and number of active
lanes out of those for a given resolution and format can vary depending
on both transmitter and receiver capabilities.

For example, if the DT specifies 4 lanes and both 148.5 MHz and 297 MHz
link frequencies, the Toshiba chip could send 720p60 YUYV via 4 lanes at
148.5 MHz, via 2 lanes at 297 MHz, or even via 4 lanes at 297 MHz, with
the longer FIFO delay.

> > 
> > [1] http://www.spinics.net/lists/linux-media/msg121813.html, as part
> > of the unicam_start_streaming function.
> > 
> > > The i.MX6 MIPI CSI-2 receiver driver can't cope with that, as it always
> > > activates all four lanes that are configured in the device tree. I can
> > > work around that with the following patch:
> > 
> > It can't cope running at less than 4 lanes, or it can't cope with a
> > change?

The hardware can cope with both, although I don't know if there are
receivers that can not.
In my case this is just about not knowing how many lanes to activate
(see below) and which link frequency to choose (currently fixed to the
first).

[...]
> > > [...] 300 is giving a fair safety margin.
> > > 
> > > It does not work for 720p60 on 4 lanes at 594 Mbit/s, as the spreadsheet
> > > warns, and testing shows.
> > 
> > If it doesn't work with 720p60, then I guess it has no hope with many
> > other resolutions.

That would have to be checked on a case by case basis, unfortunately.
I have a usecase that only supports 1080p50/60 and 720p50/60.

> > It sounds like confirming whether g_mbus_config is a potential
> > solution for i.MX6 (sorry I'm not familiar enough with that code to do
> > my own quick search), but otherwise cranking it up to 320 is
> > reasonable, and I'll see what other numbers fall out of the
> > spreadsheet.

It seems we need a replacement for g_mbus_config that only returns the
dynamic parameters.

regards
Philipp