From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AD569C27C5E for ; Tue, 11 Jun 2024 06:54:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=LHO9/+aaT2pd8Io9rbdkeZgk0rv8OF/M30y0e2KQQjo=; b=tn9ABAJJDb+3MJpoC5OqqdnmRg kIsC5ho5vhSseDy5vAHMgplBm8TohyjndapOYkysXb6GsvSg0tvaXvd0F02KJDNvQoLTaMuJXLJLn uJjSXn8d1k06YRP/hfyxUD1b/uQXLzqzBdyIpKVsHRQfw9d1h4nykcCDWYXBRuy7Zgrwfllww9J1e jgDfbxaNxq8yZOifqoL5DhkYLWpJos4BGLMBQNSRhTjxTAJrCBy5Y4yS6TSRbXyWC2oEabe7OJrHd zY5+3J5T5FX3QjKvl730h1nxprAGsaB/Mpb1BatpxlBQ2k7uqkZ04lUy0EPq8s6d7KCp5NQLVmEIT /7bXZrYg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sGvOh-00000007lk9-0THn; Tue, 11 Jun 2024 06:54:23 +0000 Received: from madrid.collaboradmins.com ([2a00:1098:ed:100::25]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sGvOZ-00000007lgt-48HK; Tue, 11 Jun 2024 06:54:19 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1718088850; bh=L2glcEBJPqTLQO8Gdi0zVPEY8C63eL0dUxrZNc/ueBk=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=iiLcw0kA6mk7hFiX5Y1591cgeb/Ga9Ubf84v4VB0213cIV28cEdq4NBAvhLWFK73B aYC9owce1ydMJEuVlGqL0CObz/YJrGP3bO5N9r2c9SnNSJ9dQ/SITzarkyyslqv0BM mqv0JIN6AF7+7hwmh3Tn5ekQ5ZzRwiyAcnyMLXr3DiINpryoOz3/JpW8YN5SONFoPe YvibWtk5xpnjTCLamTVnnSEFfxiyZWqGa/bDNLP/XUPYcWECSNvKO9BkPblf8e8NdG oIJoGQNPcWTCuYU4Md5FoWCDXktCwmdliD7TCiRQFV9Xh5y1WmpXVyBODcJRIkxgPP NgnFa+18pgHHA== Received: from [100.113.186.2] (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by madrid.collaboradmins.com (Postfix) with ESMTPSA id 9070A3780EC6; Tue, 11 Jun 2024 06:54:09 +0000 (UTC) Message-ID: Date: Tue, 11 Jun 2024 08:54:08 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path To: =?UTF-8?B?Q0sgSHUgKOiDoeS/iuWFiSk=?= , "chunkuang.hu@kernel.org" Cc: "linux-kernel@vger.kernel.org" , "linux-mediatek@lists.infradead.org" , "wenst@chromium.org" , "devicetree@vger.kernel.org" , "tzimmermann@suse.de" , =?UTF-8?B?U2hhd24gU3VuZyAo5a6L5a2d6KyZKQ==?= , "mripard@kernel.org" , =?UTF-8?B?Sml0YW8gU2hpICjnn7PorrDmtpsp?= , "daniel@ffwll.ch" , "p.zabel@pengutronix.de" , "conor+dt@kernel.org" , "maarten.lankhorst@linux.intel.com" , "robh@kernel.org" , "dri-devel@lists.freedesktop.org" , "airlied@gmail.com" , "krzysztof.kozlowski+dt@linaro.org" , "kernel@collabora.com" , "matthias.bgg@gmail.com" , =?UTF-8?B?WXUtY2hhbmcgTGVlICjmnY7nprnnkosp?= , "linux-arm-kernel@lists.infradead.org" , "amergnat@baylibre.com" References: <20240521075717.50330-1-angelogioacchino.delregno@collabora.com> <20240521075717.50330-3-angelogioacchino.delregno@collabora.com> <0e0fe86c-92da-43f5-89d7-8084274a908a@collabora.com> <0f20214ab3a86f68669ad1392398b16228e699ee.camel@mediatek.com> <47f05439-815e-4ca1-b20d-8e427fef0a2a@collabora.com> From: AngeloGioacchino Del Regno Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240610_235418_042837_9C17B011 X-CRM114-Status: GOOD ( 34.06 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org Il 11/06/24 08:48, CK Hu (胡俊光) ha scritto: > On Mon, 2024-06-10 at 10:28 +0200, AngeloGioacchino Del Regno wrote: >> Il 06/06/24 07:29, CK Hu (胡俊光) ha scritto: >>> Hi, Angelo: >>> >>> On Wed, 2024-06-05 at 13:15 +0200, AngeloGioacchino Del Regno wrote: >>>> Il 05/06/24 03:38, CK Hu (胡俊光) 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 each >>>>>> endpoint defines the starting point for one of the (currently three) >>>>>> possible hardware paths. >>>>>> >>>>>> Reviewed-by: Rob Herring (Arm) >>>>>> Reviewed-by: Alexandre Mergnat >>>>>> Tested-by: Alexandre Mergnat >>>>>> Signed-off-by: AngeloGioacchino Del Regno >>>>>> --- >>>>>> .../bindings/arm/mediatek/mediatek,mmsys.yaml | 28 +++++++++++++++++++ >>>>>> 1 file changed, 28 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml >>>>>> index b3c6888c1457..0ef67ca4122b 100644 >>>>>> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml >>>>>> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml >>>>>> @@ -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 interface node. >>>>> >>>>> vdosys0: syscon@1c01a000 { >>>>> mmsys-display-interface = <&dsi0>, <&dsi1>, <&dp_intf0>; >>>>> }; >>>>> >>>>> vdosys1: syscon@1c100000 { >>>>> mmsys-display-interface = <&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 DSI1, etc). >>>> >>>> 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 something >>>> generic, while the mmsys-display-interface would be a MediaTek specific/custom >>>> property. >>>> >>>> In the end, reusing generic kernel apis/interfaces/etc is always preferred >>>> compared to custom solutions, especially in this case, in which the generic >>>> stuff is on-par (or actually, depending purely on personal opinions, superior). >>>> >>>> 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 interface. >>>> >>>> In any case, just as a reminder: if there will be any need to add any custom >>>> MediaTek specific properties later, it's ok and we can do that at any time. >>> >>> The alternative solution is using OF graphs to point display interface and use MediaTek specific property to first component: >>> >>> vdosys0: syscon@1c01a000 { >>> ports { >>> port@0 { >>> endpoint { >>> remote-endpoint = <&dsi0_endpoint>; >>> }; >>> }; >>> >>> port@1 { >>> endpoint { >>> remote-endpoint = <&dsi1_endpoint>; >>> }; >>> }; >>> >>> port@2 { >>> endpoint { >>> remote-endpoint = <&dp_intf0_endpoint>; >>> }; >>> }; >>> }; >>> >>> display-first-component = <&ovl0>; >>> }; >>> >>> And I agree to it's better to keep only OF graphs property, so it would be >>> >>> vdosys0: syscon@1c01a000 { >>> ports { >>> port@0 { >>> endpoint { >>> remote-endpoint = <&dsi0_endpoint>; >>> >>> }; >>> }; >>> >>> port@1 { >>> endpoint { >>> remote-endpoint = <&dsi1_endpoint>; >>> >>> }; >>> }; >>> >>> port@2 { >>> endpoint { >>> remote-endpoint = <&dp_intf0_endpoint>; >>> } >>> ; >>> }; >>> }; >>> }; >>> >>> Maybe we could use OF graphs for both first component and display interface and drop using MediaTek specific property. >>> >> >> We could, or we can simply walk through the OF Graph in the driver and get the >> display interface like that, as it's board-specific ;-) >> >> ...but anyway, let's see that later: after getting this series upstreamed, I will >> convert all MediaTek boards (including Chromebooks) to use the graphs instead, and >> you'll see that, at least for the currently supported boards, there's no need for >> any custom property. >> >> Also, setting the DSI0/1/dpintf endpoint to VDO0 is technically wrong, as that is >> supposed to be the last one, and a graph is conceptually supposed to go from the >> first to the last in sequence. >> >> *if* we will ever need (probably not) to get the VDO0 node to point directly to >> the last node for whatever reason, the right way would be the first one you said, >> so, mediatek,mmsys-display-interface = <&dsi0>, <&dsi1>, etc etc >> >> ...or mediatek,mmsys-possible-displays = < ... phandles > >> >> ...or anyway, many other solutions are possible - but again, I think this is not >> the right time to think about that. Knowing that there are eventual solutions for >> any need that might arise in the future is enough, IMO :-) > > This is one routing of display pipeline and the relation of VDOSYS0 with display pipeline. > > +-- VDOSYS0 ---------------------------------------------+ > | | > | | > DRAM -> IOMMU ---> OVL0 -> RDMA0 -> ... -> DSC0 -> MERGE0 -> DP_INTF0 ----> > | | > | | > +--------------------------------------------------------+ > > Video data is read by IOMMU from DRAM and send to display pipeline. Then video data travel through first component to display interface. > VDOSYS0 manage each component in the pipeline include first component and display interface. > The management include clock gating, reset, video data input/output routing. > The relationship of VDOSYTS0 with first component is the same as the relationship of VDOSYS0 with display interface. > If VDOSYS0 is not suitable using OF graph point to display interface, VDOSYS0 is also not suitable using OF graph point to first component. In the cases in which VDO goes directly to the display, it *is* possible to make it point directly to the display. In the cases in which the pipeline is larger, VDO still points to the display, but only later in the pipeline. > The job of the component in display pipeline is to process the video data, > but the job of VDOSYS0 is to manage (clock gating, reset, routing) the pipeline. > If the OF graph is to show the video data travel path, VDOSYS0 should not exist in the OF graph. > > Regards, > CK > >> >> Cheers, >> Angelo >> >>> Regards, >>> CK >>> >>>> >>>> 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 >>>> >>>> >> >>