[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dri-devel
Subject:    Re: [RFC v2 1/4] dt-bindings: drm/mediatek: Add Mediatek display subsystem dts binding
From:       Rob Herring <robh () kernel ! org>
Date:       2015-09-30 17:13:41
Message-ID: CAL_JsqLcraiS+Yq48-m-xTksohU5wPSPrjtjV2GLzDTSDcUCUA () mail ! gmail ! com
[Download RAW message or body]

On Mon, Sep 21, 2015 at 3:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Hi Rob,
>
> thank you for the comments.
>
> Am Freitag, den 18.09.2015, 15:33 -0500 schrieb Rob Herring:
> [...]
>> > +The display-subsystem node binds together all individual device nodes that
>> > +comprise the DISP subsystem.
>> > +
>> > +Required properties:
>> > +
>> > +- compatible: "mediatek,<chip>-disp"
>>
>> > +- components: Should contain a list of phandles pointing to the DISP function
>> > +  block device nodes.
>> > +- component-names: Should contain the name of the function block pointed to
>> > +  by the components phandle of the same index.
>>
>> NAK. Group these nodes under a parent node, use of-graph or just don't
>> put this into DT. Don't invent a new way.
>
> Ok. The reason I haven't grouped all the display nodes together in the
> first place is that they aren't contiguous in the register space. So
> instead of:
>
>         ovl@1400c000 {
>                 compatible = "mediatek,mt8173-disp-ovl";
>         };
>         ...
>         ufoe@1401a000 {
>                 compatible = "mediatek,mt8173-disp-ufoe";
>         };
>
>         pwm@1401e000 {
>                 compatible = "mediatek,mt8173-disp-pwm";
>         };
>
>         larb@14021000 {
>                 compatible = "mediatek,mt8173-smi-larb";
>         };
>
>         od@14023000 {
>                 compatible = "mediatek,mt8173-disp-od";
>         };
>
> We'd have:
>
>         display-subsystem@1400c00 {
>                 compatible = "mediatek,mt8173-disp", "simple-bus";
>
>                 ovl@1400c000 {
>                         compatible = "mediatek,mt8173-disp-ovl";
>                 };
>                 ...
>                 ufoe@1401a000 {
>                         compatible = "mediatek,mt8173-disp-ufoe";
>                 };
>
>                 od@14023000 {
>                         compatible = "mediatek,mt8173-disp-od";
>                 };
>         };
>
>         pwm@1401e000 {
>                 compatible = "mediatek,mt8173-disp-pwm";
>         };
>
>         larb@14021000 {
>                 compatible = "mediatek,mt8173-smi-larb";
>         };
>
> Note how the display-subsystem node overlaps the larb node. Is that
> acceptable?

Given what the graph looks like, perhaps. However, do you really need
a container node? It only serves to provide a list of nodes (e.g. all
the children) to include as components. There are other ways to
determine this list. You could find all nodes just searching
compatible strings for each component. You just need to bind the drm
driver to some other DT node. Is there no node you can pick as the
master component?

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[prev in list] [next in list] [prev in thread] [next in thread]