Re: [PATCH 1/2] dt-bindings: media: renesas,vin: Document renesas-vin-ycbcr-8b-g property

[Geert Uytterhoeven]

Hi Niklas,

On Sat, Aug 1, 2020 at 11:18 AM Niklas <niklas.soderlund@xxxxxxxxxxxx> wrote:
> On 2020-07-25 23:23:13 +0100, Lad, Prabhakar wrote:
> > On Sat, Jul 25, 2020 at 9:11 AM Niklas <niklas.soderlund@xxxxxxxxxxxx> wrote:
> > > On 2020-07-24 22:11:31 +0100, Lad, Prabhakar wrote:
> > > > On Fri, Jul 24, 2020 at 8:37 PM Niklas <niklas.soderlund@xxxxxxxxxxxx> wrote:
> > > > > On 2020-07-24 15:58:51 +0100, Lad Prabhakar wrote:
> > > > > > Add a DT property "renesas-vin-ycbcr-8b-g" to select YCbCr422 8-bit data
> > > > > > input pins.

> > > > > This nit apart I'm not sure a property is the right way here. Could it
> > > > > not be possible on some designs to have two different sensors one wired
> > > > > to DATA[7:0] and the other to DATA[15:8] and by controlling the
> > > > > VNDRM2_YDS register at runtime switch between the two? If so adding a DT
> > > > > property to hard-code one of the two options would prevent this. I fear
> > > > > we need to think of a runtime way to deal with this.
> > > > >
> > > > Aha Gen2 and Gen3 hardware manuals have a bit different description
> > > > about the YDS field. (I was working R8a7742 SoC so I referred Gen2
> > > > manual)
> > >
> > > Ahh, I think we should use the Gen3 names as I find them overall an
> > > improvement over the Gen2 ones.
> > >
> > Agreed.
> >
> > > >
> > > > > The best way to do that I think is to extend the port@0 node to allow
> > > > > for two endpoints, one for each of the two possible parallel sensors.
> > > > > This would then have to be expressed in the media graph and selection if
> > > > > YDS should be set or not depend on which media links are enabled.
> > > > >
> > > > In that case how do we handle endpoint matching each would have two
> > > > subdevs to be matched.
> > >
> > > It would be handle in the same was as the multiple endpoints in port@1.
> > >
> > > > And in case non media-ctl cases we cannot
> > > > switch between subdevs.
> > >
> > > For the Gen2 none media graph enabled mode this could be handled with
> > > the S_INPUT ioctl. For this feature to be merged however I it needs to
> > > be possible to select input both in Gen2 and Gen3 I'm afraid.
> > Ohh yes S_INPUT could be used to switch inputs. But  how do we decide
> > YDS needs to be enabled, for example with the below dts say vin3 is
> > parallel bus split into 2x 8-bit bus one connected to a ov5640 sensor
> > and other connected to ov7725 sensor. Should we use data-shift
> > property for the second vin endpoint (vin3ep1) to enable YDS ?
>
> Using data-shift is a great idea! If I understand your use-case you
> currently only have one sensor attached on the parallel bus right? If so
> we can postpone the multi sensor part until it's needed and just learn
> the VIN driver about data-shift. From the documentation,
>
>   - data-shift: on the parallel data busses, if bus-width is used to
>     specify the number of data lines, data-shift can be used to specify
>     which data lines are used, e.g. "bus-width=<8>; data-shift=<2>;"
>     means, that lines 9:2 are used.
>
> So in this case would not specifying data-shift=<8> solve the DT
> description problem? The VIN driver still needs to learn about this tho.

And the PFC drivers need to be extended with the data8_sft8 groups
(present in the BSP), right?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds