On Mon, Dec 19, 2016 at 10:54 AM, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > 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. The controller should just ask for the panel via a common function. That function should then look for either a child node (called 'panel') or a graph port. Of course, for more complex cases, only OF graph may work. It really just the simple case of a controller with a single output and single panel that I'm talking about. >> >> 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 ? :-) Yes. Acked-by: Rob Herring <robh@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html