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

[Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

Hi Daniel,

Am Freitag, den 02.10.2015, 21:47 +0800 schrieb Daniel Kurtz:
> On Fri, Oct 2, 2015 at 3:40 PM, Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> > Am Donnerstag, den 01.10.2015, 22:29 +0800 schrieb Daniel Kurtz:
> >> On Thu, Oct 1, 2015 at 8:58 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> > 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";
> >
> > Yes, the problem with the ovl nodes is that there are two equivalent
> > ones. Having two equivalent ipu nodes on i.MX6 was the reason to
> > introduce the display-subsystem node in the first place, but I'd very
> > much prefer to avoid it, if possible.
> 
> Oh, interesting. How is it a problem that there are two equivalent ovl nodes?

Which one of the two should create the drm device?

> Why do you prefer to avoid the display-subsystem node?

Because then we don't have to argue about what it should look like ;)

Mostly because nodes like this don't describe an actual hardware
devices, but rather how the hardware designers intended other, actually
existing hardware devices to work together (in the best case), or just
how the Linux driver developers wanted to use the components together
(in the worst case).

If the display-subsystem node is necessary or useful, I won't argue
against it. If we can do without, I'll be happier.

> BTW, what I really meant was for my example was that the
> 'display-subsystem' 'ports' node lists phandles representing the
> 'root' nodes for the for the display-subsystem graph.
> This matches what we did for rk3288.dtsi:
>
> display-subsystem {
>   compatible = "mediatek,display-subsystem";
>   ports = <&ovl0_out>, <&ovl1_out>;
> };

Oh, ok then. I'm worried again that we would be describing what the
driver should load, not what the hardware looks like. Note that the
ovl_out ports have no significance outside of the display subsystem. In
the RK3288 and i.MX6 cases, the 'external' ports of the display core are
listed, to which the encoders are connected. On MT8173 this would
correspond to:

display-subsystem {
    compatible = "mediatek,display-subsystem";
    ports = <&dsi0_out>, <&dsi1_out>, <&dpi0_out>;
};

[...]
> > I think the ovl node has no business linking to mmsys_config.
> > It's the drm driver code that sets up the pipeline, including the crtc
> > -> encoder connections
> 
> My original thinking is that the "ovl" node should link to the
> mmsys_config since the mmsys_config has the ovl output select
> registers.

I see. If each function block takes care of the attached SEL input
selectors and SOUT/MOUT output selectors, wouldn't all of them have to
link to the mmsys_config node, then?
That's certainly a possibility, but in this case I feel like having a
single, central instance to handle building the paths and arbitrating
register access with help of the disp_mutex would be easier to
understand.

regards
Philipp

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html