Re: [PATCH v1.1 4/4] ti-vpe: Parse local endpoint for properties, not the remote one

[Benoit Parrot <bparrot@ti.com>]

Sakari Ailus <sakari.ailus@linux.intel.com> wrote on Tue [2019-Mar-05 18:32:40 +0200]:
> Hi Benoit,
> 
> On Tue, Mar 05, 2019 at 08:34:09AM -0600, Benoit Parrot wrote:
> > Sakari,
> > 
> > Thank you for the patch.
> > 
> > Sakari Ailus <sakari.ailus@linux.intel.com> wrote on Tue [2019-Mar-05 16:02:24 +0200]:
> > > ti-vpe driver parsed the remote endpoints for properties but ignored the
> > > local ones. Fix this by parsing the local endpoint properties instead.
> > 
> > I am not sure I understand the logic here.  For CSI2 sensor as far as I
> > understand the lane mapping (clock and data) is driven from the sensor
> > side. The bridge driver (in this case CAL) needs to setup the receiver side
> > based on what the sensor (aka remote endpoint) can provide.
> > 
> > I failed to see how this fixes things here.
> > 
> > Are you suggesting that sensor relevant properties be set (and effectively
> > duplicated) on the bridge/receiver side?
> 
> Yes. The endpoint configuration in general is local to the device and
> should not be accessed from other device drivers.
> 
> The lane mapping, for instance, is specific to a given device --- and may
> differ even between for two connected endpoints. It's used to reorder the
> PHY lanes (if the device supports that). Same goes for the clock lane.

I did not see omap3isp having lane reorder capability, but I guess it would
be possible for instance, that a sensor uses clock lane 0 and data lane 1
& 2 but the way it is wired on the board makes it that the receiver would see
sensor lane 0 on device lane 2 and so on... Not sure why you would wire it
up that way but who knows...

> 
> See e.g. arch/arm/boot/dts/omap3-n9.dts .
> 
> > 
> > Some sensor can and do handle multiple data lanes configuration so the
> > sensor driver also needs to use those properties at probe time, duplicating
> > the lane data is just asking for a mismatch to happen, no?
> 
> It's a different configuration on the sensor side. We currently have no
> checks in place to verify that the two would match. I haven't heard of this
> would have really been a problem though.

I had just never thought about this cases, to me a single source of
information is better than 2. But anyhow I guess I'll have to update all of
my relevant dts files in the near future.

Benoit

> 
> The frame descriptors should be used for runtime configuration. Niklas and
> more recently Jacopo have been working on that.
> 
> -- 
> Kind regards,
> 
> Sakari Ailus
> sakari.ailus@linux.intel.com