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@xxxxxxxxxx> > > --- > > 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:
signature.asc
Description: PGP signature
