On Sun, Dec 18, 2016 at 2:54 PM, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > Hi Rob, > > 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. They become dependent on the controller driver to probe the panel, but the contents of the panel node would not be controller dependent. >> 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. Rob
