Re: [PATCH v2 01/13] devicetree/bindings: display: Document common panel properties

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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