Received-SPF: pass (google.com: domain of linux-kernel+bounces-203688-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240521075717.50330-1-angelogioacchino.delregno@collabora.com>
 <20240521075717.50330-3-angelogioacchino.delregno@collabora.com>
 <e7845300fa822413f6308cb6297222cde89c39e0.camel@mediatek.com> <0e0fe86c-92da-43f5-89d7-8084274a908a@collabora.com>
In-Reply-To: <0e0fe86c-92da-43f5-89d7-8084274a908a@collabora.com>
From: Chen-Yu Tsai <wenst@chromium.org>
Date: Thu, 6 Jun 2024 14:46:51 +0800
Message-ID: <CAGXv+5FgVk9z3DhAC5oYoGXSJ+wJf+sa6wFSyJ_Nhy3JrKkCng@mail.gmail.com>
Subject: Re: [PATCH v5 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph
 support for board path
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: =?UTF-8?B?Q0sgSHUgKOiDoeS/iuWFiSk=?= <ck.hu@mediatek.com>, 
	"chunkuang.hu@kernel.org" <chunkuang.hu@kernel.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
	"linux-mediatek@lists.infradead.org" <linux-mediatek@lists.infradead.org>, 
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, "tzimmermann@suse.de" <tzimmermann@suse.de>, 
	=?UTF-8?B?U2hhd24gU3VuZyAo5a6L5a2d6KyZKQ==?= <Shawn.Sung@mediatek.com>, 
	"mripard@kernel.org" <mripard@kernel.org>, =?UTF-8?B?Sml0YW8gU2hpICjnn7PorrDmtpsp?= <jitao.shi@mediatek.com>, 
	"daniel@ffwll.ch" <daniel@ffwll.ch>, "p.zabel@pengutronix.de" <p.zabel@pengutronix.de>, 
	"conor+dt@kernel.org" <conor+dt@kernel.org>, 
	"maarten.lankhorst@linux.intel.com" <maarten.lankhorst@linux.intel.com>, "robh@kernel.org" <robh@kernel.org>, 
	"dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, "airlied@gmail.com" <airlied@gmail.com>, 
	"krzysztof.kozlowski+dt@linaro.org" <krzysztof.kozlowski+dt@linaro.org>, 
	"kernel@collabora.com" <kernel@collabora.com>, "matthias.bgg@gmail.com" <matthias.bgg@gmail.com>, 
	=?UTF-8?B?WXUtY2hhbmcgTGVlICjmnY7nprnnkosp?= <Yu-chang.Lee@mediatek.com>, 
	"linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, 
	"amergnat@baylibre.com" <amergnat@baylibre.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jun 5, 2024 at 7:15=E2=80=AFPM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 05/06/24 03:38, CK Hu (=E8=83=A1=E4=BF=8A=E5=85=89) ha scritto:
> > Hi, Angelo:
> >
> > On Tue, 2024-05-21 at 09:57 +0200, AngeloGioacchino Del Regno wrote:
> >> Document OF graph on MMSYS/VDOSYS: this supports up to three DDP paths
> >> per HW instance (so potentially up to six displays for multi-vdo SoCs)=
.
> >>
> >> The MMSYS or VDOSYS is always the first component in the DDP pipeline,
> >> so it only supports an output port with multiple endpoints - where eac=
h
> >> endpoint defines the starting point for one of the (currently three)
> >> possible hardware paths.
> >>
> >> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> >> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
> >> Tested-by: Alexandre Mergnat <amergnat@baylibre.com>
> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@c=
ollabora.com>
> >> ---
> >>   .../bindings/arm/mediatek/mediatek,mmsys.yaml | 28 +++++++++++++++++=
++
> >>   1 file changed, 28 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,m=
msys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.y=
aml
> >> index b3c6888c1457..0ef67ca4122b 100644
> >> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.ya=
ml
> >> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.ya=
ml
> >> @@ -93,6 +93,34 @@ properties:
> >>     '#reset-cells':
> >>       const: 1
> >>
> >> +  port:
> >> +    $ref: /schemas/graph.yaml#/properties/port
> >> +    description:
> >> +      Output port node. This port connects the MMSYS/VDOSYS output to
> >> +      the first component of one display pipeline, for example one of
> >> +      the available OVL or RDMA blocks.
> >> +      Some MediaTek SoCs support multiple display outputs per MMSYS.
> >
> > This patch looks good to me. Just want to share another information for=
 you.
> > Here is an example that mmsys/vdosys could point to the display interfa=
ce node.
> >
> > vdosys0: syscon@1c01a000 {
> >            mmsys-display-interface =3D <&dsi0>, <&dsi1>, <&dp_intf0>;
> > };
> >
> > vdosys1: syscon@1c100000 {
> >            mmsys-display-interface =3D <&dp_intf1>;
> > };
> >
> > There is no conflict that mmsys/vdosys point to first component of one =
display pipeline or point to display interface.
> > Both could co-exist.
> >
>
> Hey CK,
>
> yes, this could be an alternative to the OF graphs, and I'm sure that it'=
d work,
> even though this kind of solution would still require partial hardcoding =
of the
> display paths up until mmsys-display-interface (so, up until DSI0, or DSI=
1, etc).

I think you might be misunderstanding CK's proposal? He's simply saying tha=
t
instead of pointing to the start of the pipeline, point to the end instead.
You can still use the OF graph and work backwards from the output.

> The problem with a solution like this is that, well, even though it would=
 work,
> even if we ignore the suboptimal partial hardcoding, OF graphs are someth=
ing
> generic, while the mmsys-display-interface would be a MediaTek specific/c=
ustom
> property.
>
> In the end, reusing generic kernel apis/interfaces/etc is always preferre=
d
> compared to custom solutions, especially in this case, in which the gener=
ic
> stuff is on-par (or actually, depending purely on personal opinions, supe=
rior).

Here you are mixing hardware descriptions and kernel implementation details=
.

I think this goes back to whether the mmsys/vdosys is actually part of the
graph or not. It certainly controls the muxes within the graph. But that
doesn't mean it has to be within the graph itself. It can just have pointer=
s
to entry points of the graph (for which you would have a couple lines of
custom code [1]). If the data doesn't flow through the mmsys/vdosys, then
I would argue that it is not part of the graph.

I would also argue that the data path should be fully described in the
device tree, not hardcoding paths based on board usage. The latter is
a policy / design decision, not a hardware capability.


ChenYu

> As for the two to co-exist, I'm not sure that this is actually needed, as=
 the
> OF graphs are already (at the end of the graph) pointing to the display i=
nterface.
>
> In any case, just as a reminder: if there will be any need to add any cus=
tom
> MediaTek specific properties later, it's ok and we can do that at any tim=
e.
>
> Cheers!
> Angelo
>
> > Regards,
> > CK
> >
> >> +    properties:
> >> +      endpoint@0:
> >> +        $ref: /schemas/graph.yaml#/properties/endpoint
> >> +        description: Output to the primary display pipeline
> >> +
> >> +      endpoint@1:
> >> +        $ref: /schemas/graph.yaml#/properties/endpoint
> >> +        description: Output to the secondary display pipeline
> >> +
> >> +      endpoint@2:
> >> +        $ref: /schemas/graph.yaml#/properties/endpoint
> >> +        description: Output to the tertiary display pipeline
> >> +
> >> +    anyOf:
> >> +      - required:
> >> +          - endpoint@0
> >> +      - required:
> >> +          - endpoint@1
> >> +      - required:
> >> +          - endpoint@2
> >> +
> >>   required:
> >>     - compatible
> >>     - reg
>
>