Hi Rob, On Monday 19 Dec 2016 09:38:49 Rob Herring wrote: > On Sun, Dec 18, 2016 at 2:54 PM, Laurent Pinchart wrote: > > On Tuesday 29 Nov 2016 20:23:41 Laurent Pinchart wrote: > >> On Tuesday 29 Nov 2016 09:14:09 Rob Herring wrote: > >>> On Tue, Nov 29, 2016 at 2:27 AM, Laurent Pinchart wrote: > >>>> On Tuesday 22 Nov 2016 11:36:55 Laurent Pinchart wrote: > >>>>> On Monday 21 Nov 2016 10:48:15 Rob Herring wrote: > >>>>>> On Sat, Nov 19, 2016 at 05:28:01AM +0200, Laurent Pinchart wrote: > >>>>>>> Document properties common to several display panels in a central > >>>>>>> location that can be referenced by the panel device tree bindings. > >>>>>> > >>>>>> Looks good. Just one comment... > >>>>>> > >>>>>> [...] > >>>>>> > >>>>>>> +Connectivity > >>>>>>> +------------ > >>>>>>> + > >>>>>>> +- ports: Panels receive video data through one or multiple > >>>>>>> connections. While > >>>>>>> + the nature of those connections is specific to the panel type, > >>>>>>> the > >>>>>>> + connectivity is expressed in a standard fashion using ports as > >>>>>>> specified in > >>>>>>> + the device graph bindings defined in > >>>>>>> + Documentation/devicetree/bindings/graph.txt. > >>>>>> > >>>>>> We allow panels to either use graph binding or be a child of the > >>>>>> display controller. > >>>>> > >>>>> I knew that some display controllers use a phandle to the panel (see > >>>>> the fsl,panel and nvidia,panel properties), but I didn't know we had > >>>>> panels as children of display controller nodes. I don't think we > >>>>> should allow that for anything but DSI panels, as the DT hierarchy is > >>>>> based on control buses. Are you sure we have other panels instantiated > >>>>> through that mechanism ? > >>> > >>> Some panels have no control bus, so were do we place them? > >> > >> I'd say under the root node, like all similar control-less devices. > >> > >>> I would say the hierarchy is based on buses with a preference for the > >>> control bus when there are multiple buses. I'm not a fan of just > >>> sticking things are the top level. > >> > >> OK, so much for my comment a few lines up :-) > >> > >> The problem with placing non-DSI panels as children of the display > >> controller and not using OF graph is that the panel bindings become > >> dependent of the display controller being used. A display controller > >> using OF graph would require the panel to do the same, while a display > >> controller expecting a panel child node (with a specific name) would > >> require DT properties for the panel node. > > Not sure I follow. Sorry, I meant "would not requite DT properties". > They become dependent on the controller driver to probe the panel, but the > contents of the panel node would not be controller dependent. If a display controller uses OF graph then the panel DT node has to declare ports. If the display controller doesn't use OF graph but instead expects the panel to be a direct subnode, or points to the panel using a property such as fsl,panel, then the panel DT node will not have ports. > >> I'm also not sure the complexity of OF graph is really that prohibitive > >> if you compare it to panels as child nodes. To get the panel driver to > >> bind to the panel DT node the display controller driver would need to > >> create a platform device for the panel and register it. That's not very > >> difficult, but parsing a single port and endpoint isn't either (and we > >> could even provide a helper function for that, a version of > >> of_drm_find_panel() that would take as an argument the display controller > >> device node instead of the panel device node). > > > > Ping ? > > > > I'd like to standardize on one model for panel DT bindings, but I'm not > > sure that can be achieved given that we already have multiple competing > > models. In any case, is that blocking to merge this patch ? I only > > describe one connectivity model here as that's what my panel driver > > needs, but I have no issue adding more models later when needed. I > > believe this patch is a good step forward already. > > It is an improvement which I appreciate, so yes I guess we can address > it later when needed. Thank you. Can I get your ack then ? :-) -- Regards, Laurent Pinchart