[RFC v2 1/4] dt-bindings: drm/mediatek: Add Mediatek display subsystem dts binding

[Daniel Kurtz]

On Thu, Oct 1, 2015 at 8:58 PM, Rob Herring <robh at kernel.org> wrote:
> On Thu, Oct 1, 2015 at 3:59 AM, Philipp Zabel <p.zabel at pengutronix.de> wrote:
>> Am Mittwoch, den 30.09.2015, 12:13 -0500 schrieb Rob Herring:
>>> On Mon, Sep 21, 2015 at 3:11 AM, Philipp Zabel <p.zabel at pengutronix.de> wrote:
>>> > 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?
>>
>> There is the mmsys clock-controller node at the top of the MMSYS address
>> space (0x14000000-0x14ffffff):
>>
>>         mmsys: clock-controller at 14000000 {
>>                 compatible = "mediatek,mt8173-mmsys", "syscon";
>>                 reg = <0 0x14000000 0 0x1000>;
>>                 #clock-cells = <1>;
>>         };
>>
>> Its register space also contains the MMSYS_CONFIG region that controls
>> the multiplexers between the display function blocks, so that would be a
>> good candidate.
>> No driver binds to this node yet, the clocks are registered with
>> CLK_OF_DECLARE.
>>
>> I'll try to bind to this node and have the driver find sibling nodes
>> using their compatible strings.
>
> That doesn't seem like a good choice since there are other functions
> in the block. I was thinking one of the display related blocks like
> whatever block provides the main crtc functions.

The two "OVL" nodes correspond to the "crtc" functions of the two
display pipes in this SoC, we would setup the display-subsystem node
like this:

display-subsystem {
  compatible = "mediatek,display-subsystem";
  ports = <&ovl0>, <&ovl1>;
};

And, any node that needs to poke around in the mmsys_config area, like
an ovl, could access it using a phandle <&mmsys> to the common
"mediatek,mt8173-mmsys" device, like this:

ovl0: ovl at 1400c000 {
       compatible = "mediatek,mt8173-ovl";
       reg = <0 0x1400c000 0 0x1000>;
       interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_LOW>;
       clocks = <&mmsys CLK_MM_DISP_OVL0>;
       iommus = <&iommu M4U_LARB0_ID M4U_PORT_DISP_OVL0>;
       mediatek,mmsys = <&mmsys>;
       status = "disabled";
};

One idea to reduce the size of the of_graph would be to just model the
entrance/exit points from the MM subsystem in dt.  So, instead of:
  ovl0 -> color0 -> aal -> od -> ufoe -> dsi0 -> dsi-edp-bridge
  ovl1 -> color1 -> gamma0 -> dpi0 -> hdmi

We can do something like this:
  ovl0 -> dsi0 -> dsi-edp-bridge
  ovl1 -> dpi0 -> hdmi

But ... this might be too limiting for some applications.
So, I think we should just live with the big graph.
I would, however, recommend that you move the graph to its own
mt8173-display-graph.dtsi file which would be #included into a board
specific .dts file.

If you let the board .dts include the graph, then we can possibly
shrink the resulting board specific device tree -
At first we would make the full graph in one .dtsi as above
"mt8173-display-graph.dtsi".
This is the full reference graph that will always work.

Then, if a particular board used a fixed configuration, the system
integrator could create a smaller graph with just the graph nodes that
they care about.

For example, if mt8173-evb only supports a single HDMI output, then
perhaps you would just
set up the endpoints for this single fixed function display pipe in
mt8173-evb-display-graph.dtsi:

&ovl1 {
    status = "okay";
    port {
        ovl1_to_color1: endpoint at 0 {
            remote-endpoint = <&color1_from_ovl1>;
        };
    };
};

&color1 {
    status = "okay";
    port at 0 {
        color1_from_ovl1: endpoint at 1 {
            remote-endpoint = <&ovl1_to_color1>;
        };
    };

    port at 1 {
        color1_to_gamma: endpoint at 0 {
            remote-endpoint = <&gamma_from_color1>;
        };
    };
};

&gamma {
    status = "okay";
    port at 0 {
        gamma_from_color1: endpoint {
            remote-endpoint = <&color1_to_gamma>;
        };
    };

    port at 1 {
        gamma_to_dpi0: endpoint at 3 {
            remote-endpoint = <&dpi0_from_gamma>;
        };
    };
};

&dpi0 {
    status = "okay";
    port at 0 {
        dpi0_from_gamma: endpoint at 2 {
            remote-endpoint = <&gamma_to_dpi0>;
        };
    };

    port at 1 {
        dpi0_to_hdmi: endpoint {
            remote-endpoint = <&hdmi_from_dpi0>;
        };
    };
};

Notice that this also only enables the nodes that we care about (all
of the other nodes would be disabled by default).

I this would allow full flexibility but also drastically cut down on
the resulting .dts size.

The individual component drivers would then march down their graphs
ensuring that all of the corresponding components have been bound, and
then themselves appropriately based on their set of endpoints that are
available.

What do you think?

-Dan


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