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

[Rob Herring]

On Mon, Dec 19, 2016 at 10:54 AM, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> 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 at kernel.org>