Re: [RFC 2/9] dt-bindings: display, renesas,du: Document cmms property

[Jacopo Mondi <jacopo@jmondi.org>]

[-- Attachment #1: Type: text/plain, Size: 2626 bytes --]

Hi Laurent,
   thanks for the comments

On Sat, May 11, 2019 at 09:23:30PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, May 08, 2019 at 07:34:21PM +0200, Jacopo Mondi wrote:
> > Document the newly added 'cmms' property which accepts a list of phandle
> > and channel index pairs that point to the CMM units available for each
> > Display Unit output video channel.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  Documentation/devicetree/bindings/display/renesas,du.txt | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
> > index aedb22b4d161..2ccf78bf9a18 100644
> > --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> > +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> > @@ -44,6 +44,10 @@ Required Properties:
> >      instance that serves the DU channel, and the channel index identifies the
> >      LIF instance in that VSP.
> >
> > +  - cmms: A list of phandle and channel index tuples to the CMM modules
> > +    connected to DU channels. The phandle identifies the CMM instance that
> > +    serves the DU channel identified by the index.
>
> Do we need the index ?
>

Well, I struggled a bit at deciding if this was a good idea or not.
In the end I decided to use the index, as in this version, by just
providing the cmm phandle, the CMM gets enabled for the DU channel it
is associated to. It is true I could just enumerate them, and assign
the CMM corresponding to the first phandle to channel #0, the second
to channel #1 and so on, but in the (very unlikely?) case where a
developer what to enable CMM for, say, channel #0 and #2 but not #1,
this scheme would break, as I have then decided to have a mandatory
channel index to make the association stable. True that a CMM unit is
associated to a DU channel only, and I could derive this from the base
address or a custom property (like 'renesa,du-channel) in the CMM
device node, but this seems better handled here.

Now that I wrote this, I wonder if I actually need to know which
channel a CMM is associated to, or I could just go and enable the ones
listed in the 'cmms' property and that's it. Was this the idea behind
your question?

Thanks
   j
> It should also be noted that the property is optional for SoCs that
> don't have any CMM.
>
> > +
> >  Required nodes:
> >
> >  The connections to the DU output video ports are modeled using the OF graph
>
> Could you update the example ?
>
> --
> Regards,
>
> Laurent Pinchart

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]