diff mbox series

[03/12] dt-bindings: display: mediatek: add EXDMA yaml for MT8196

Message ID 20250110123835.2719824-4-paul-pl.chen@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Add Mediatek Soc DRM support for mt8196 | expand

Commit Message

Paul-pl Chen (陳柏霖) Jan. 10, 2025, 12:33 p.m. UTC
From: "Paul-pl.Chen" <paul-pl.chen@mediatek.com>

Add mediatek,exdma.yaml to support EXDMA for MT8196.

Signed-off-by: Paul-pl.Chen <paul-pl.chen@mediatek.com>
---
The header used in examples:
#include <dt-bindings/clock/mt8196-clk.h>
#include <dt-bindings/power/mt8196-power.h>
are not upstreamed yet.
It will be sent by related owner soon.
---
 .../display/mediatek/mediatek,exdma.yaml      | 77 +++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,exdma.yaml

Comments

Paul-pl Chen (陳柏霖) Jan. 17, 2025, 10:36 a.m. UTC | #1
On Fri, 2025-01-10 at 14:01 +0100, Krzysztof Kozlowski wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On 10/01/2025 13:33, paul-pl.chen wrote:
> > From: "Paul-pl.Chen" <paul-pl.chen@mediatek.com>
> > 
> > Add mediatek,exdma.yaml to support EXDMA for MT8196.
> > 
> > Signed-off-by: Paul-pl.Chen <paul-pl.chen@mediatek.com>
> > ---
> > The header used in examples:
> > #include <dt-bindings/clock/mt8196-clk.h>
> > #include <dt-bindings/power/mt8196-power.h>
> > are not upstreamed yet.
> 
> Which makes this untestable and unmergeable.
> 
> This cannot be accepted. Fix your dependencies or decouple from them.
> 
> > It will be sent by related owner soon.
> 
> Still this won't build and won't be possible to apply.
> 
> > ---
> >  .../display/mediatek/mediatek,exdma.yaml      | 77
> > +++++++++++++++++++
> >  1 file changed, 77 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/mediatek/mediatek,exdma.y
> > aml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/mediatek/mediatek,exdma
> > .yaml
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,exdma
> > .yaml
> > new file mode 100644
> > index 000000000000..385f5549dfaa
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/mediatek/mediatek,exdma
> > .yaml
> 
> Filename matching compatible.
> 
> Why is this in display? DMA goes to dma.

Hi Krzysztof ,

Regarding the issue of the EXDMA driver, we have conducted an internal
survey of drivers under the DMA subsystem. We found that EXDMA operates
differently from typical DMA drivers, and therefore we believe that the
EXDMA driver may not be suitable to be placed under the
driver/mediatek/drm directory. The main reasons are as follows:

(1)No Memory Allocation within EXDMA Engine:
The EXDMA engine does not perform memory allocation operations itself.
Instead, it relies on GEM (Graphics Execution Manager) to allocate
memory.Traditional DMA drivers often handle their own memory
allocations, but in the case of EXDMA, memory management is delegated
to GEM.

(2)Primary Task of EXDMA:
The main function of EXDMA is to transfer buffers allocated by GEM to
the subsequent display pipeline.
EXDMA serves as a bridge between memory allocated by GEM and the
display components, rather than acting as a general-purpose DMA engine.
Based on the points above, we have decided to place the EXDMA driver
under the DRM display subsystem rather than under the DMA subsystem.

> 
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > https://urldefense.com/v3/__http://devicetree.org/schemas/display/mediatek/mediatek,exdma.yaml*__;Iw!!CTRNKA9wMg0ARbw!ldlqohpAoMyTt24UKnssKOk5Qvmc_wlvQPCdjneKDCeshDPwI5Uuuy4A2sI2RlfYLIFDKZx_-GGDOlX48Q$
> > +$schema:
> > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!ldlqohpAoMyTt24UKnssKOk5Qvmc_wlvQPCdjneKDCeshDPwI5Uuuy4A2sI2RlfYLIFDKZx_-GHpxO9DFQ$
> > +
> > +title: MediaTek EXDMA
> > +
> > +maintainers:
> > +  - Chun-Kuang Hu <chunkuang.hu@kernel.org>
> > +  - Philipp Zabel <p.zabel@pengutronix.de>
> > +
> > +description:
> > +  The MediaTek display overlap extended DMA engine, namely
> > OVL_EXDMA or EXDMA,
> > +  primarily functions as a DMA engine for reading data from DRAM
> > with various
> > +  DRAM footprints and data formats. For input sources in certain
> > color formats
> > +  and color domains, OVL_EXDMA also includes a color transfer
> > function
> > +  to process pixels into a consistent color domain.
> > +
> 
> Missing ref to dma schemas.

allOf:
- $ref: dma-controller.yaml#
> 
> > +properties:
> > +  compatible:
> > +    const: mediatek,mt8196-exdma
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: EXDMA Clock
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  mediatek,larb:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> 
> Why array? And isn't the property named mediatek,larbs?
> 
Using "mediatek,larb" here is correct because the EXDMA hardware IP
will only have one mediatek,larb. In the next version, we will change
the phandle-array definition to a single phandle.
Please refer to this link:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/Documentation/devicetree/bindings?id=6d0990e6e844cfa045b1a7348f58964caceb4de4
Since MT8196 use SMMU, the SMMU can no longer internally determine the
connection relationship between smi-larb and the consumer's
pm_runtime_get(_sync). Therefore, we need to add this information back.
> > 
> > 
> > Best, Paul
Krzysztof Kozlowski Jan. 18, 2025, 8:37 a.m. UTC | #2
On 17/01/2025 11:36, Paul-pl Chen (陳柏霖) wrote:
>>> b/Documentation/devicetree/bindings/display/mediatek/mediatek,exdma
>>> .yaml
>>
>> Filename matching compatible.
>>
>> Why is this in display? DMA goes to dma.
> 
> Hi Krzysztof ,
> 
> Regarding the issue of the EXDMA driver, we have conducted an internal
> survey of drivers under the DMA subsystem. We found that EXDMA operates


I did not talk about driver. I talked about this patch. Look at patch
title - it starts with dt-bindings. Is here anything about driver? No.
Why do we talk about driver?

> differently from typical DMA drivers, and therefore we believe that the
> EXDMA driver may not be suitable to be placed under the
> driver/mediatek/drm directory. The main reasons are as follows:
> 
> (1)No Memory Allocation within EXDMA Engine:
> The EXDMA engine does not perform memory allocation operations itself.
> Instead, it relies on GEM (Graphics Execution Manager) to allocate
> memory.Traditional DMA drivers often handle their own memory
> allocations, but in the case of EXDMA, memory management is delegated
> to GEM.
> 
> (2)Primary Task of EXDMA:
> The main function of EXDMA is to transfer buffers allocated by GEM to
> the subsequent display pipeline.
> EXDMA serves as a bridge between memory allocated by GEM and the
> display components, rather than acting as a general-purpose DMA engine.
> Based on the points above, we have decided to place the EXDMA driver
> under the DRM display subsystem rather than under the DMA subsystem.


I don't care if it uses GEM or kernel allocator or even 3rd party
allocator. The question is: what is this device? If it is performing
DMA, then it should be placed in "dma" directory. The rdma was placed
differently but as you can easily check: it was never acked/reviewed, so
don't use it as an example.

Of course if it does not perform DMA, then it should not be in dma, but
then I don't agree on using dma-cells here and anything like that in the
driver.

Best regards,
Krzysztof
Paul-pl Chen (陳柏霖) Jan. 21, 2025, 5:07 p.m. UTC | #3
On Sat, 2025-01-18 at 09:37 +0100, Krzysztof Kozlowski wrote:
> //snip

> I did not talk about driver. I talked about this patch. Look at patch
> title - it starts with dt-bindings. Is here anything about driver?
> No.
> Why do we talk about driver?
> 
> > differently from typical DMA drivers, and therefore we believe that
> > the
> > EXDMA driver may not be suitable to be placed under the
> > driver/mediatek/drm directory. The main reasons are as follows:
> > 
> > (1)No Memory Allocation within EXDMA Engine:
> > The EXDMA engine does not perform memory allocation operations
> > itself.
> > Instead, it relies on GEM (Graphics Execution Manager) to allocate
> > memory.Traditional DMA drivers often handle their own memory
> > allocations, but in the case of EXDMA, memory management is
> > delegated
> > to GEM.
> > 
> > (2)Primary Task of EXDMA:
> > The main function of EXDMA is to transfer buffers allocated by GEM
> > to
> > the subsequent display pipeline.
> > EXDMA serves as a bridge between memory allocated by GEM and the
> > display components, rather than acting as a general-purpose DMA
> > engine.
> > Based on the points above, we have decided to place the EXDMA
> > driver
> > under the DRM display subsystem rather than under the DMA
> > subsystem.
> 
> 
> I don't care if it uses GEM or kernel allocator or even 3rd party
> allocator. The question is: what is this device? If it is performing
> DMA, then it should be placed in "dma" directory. The rdma was placed
> differently but as you can easily check: it was never acked/reviewed,
> so
> don't use it as an example.
> 
> Of course if it does not perform DMA, then it should not be in dma,
> but
> then I don't agree on using dma-cells here and anything like that in
> the
> driver.
> 
> Best regards,
> Krzysztof
> 


Hi Krzysztof,


The current placement of EXDMA under the display subsystem in
Mediatek's architecture is primarily due to its functional role as a
sub-device within the display pipeline.
 
In MT8196 hardware design, the sub-devices in display pipeline follow a
sequence of: EXDMA -> BLENDER -> OUTPROC -> PQ -> DVO.
 
In MT8195 hardware design, the sub-devices in display pipeline follow a
sequence of: OVL -> PQ ->DSI.
 
As we see, OVL has been divided into three new hardware IPs in MT8196.
OVL and EXDMA both have the ability to fetch data directly from DRAM
and can be regarded as DMA controller.
 
I also have confirmed with the hardware designer that EXDMA is a kind
of DMA, but it is specially designed to handle the graphical layer, and
has better performance than ordinary DMA.
 
Therefore, I think that moving EXDMA and OVL from the display folder to
the DMA folder, or only kepping them in the display folder is decided
by the two different views of DMA ability or display sub-device.
 
We will follow your instructions to put EXDMA on the place you decided.

Best regards,
Paul Chen
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,exdma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,exdma.yaml
new file mode 100644
index 000000000000..385f5549dfaa
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,exdma.yaml
@@ -0,0 +1,77 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/mediatek/mediatek,exdma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek EXDMA
+
+maintainers:
+  - Chun-Kuang Hu <chunkuang.hu@kernel.org>
+  - Philipp Zabel <p.zabel@pengutronix.de>
+
+description:
+  The MediaTek display overlap extended DMA engine, namely OVL_EXDMA or EXDMA,
+  primarily functions as a DMA engine for reading data from DRAM with various
+  DRAM footprints and data formats. For input sources in certain color formats
+  and color domains, OVL_EXDMA also includes a color transfer function
+  to process pixels into a consistent color domain.
+
+properties:
+  compatible:
+    const: mediatek,mt8196-exdma
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: EXDMA Clock
+
+  power-domains:
+    maxItems: 1
+
+  mediatek,larb:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    maxItems: 1
+    items:
+      maxItems: 1
+    description: |
+      A phandle to the local arbiters node in the current SoCs.
+      Refer to bindings/memory-controllers/mediatek,smi-larb.yaml.
+
+  iommus:
+    maxItems: 1
+
+  '#dma-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - power-domains
+  - mediatek,larb
+  - iommus
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8196-clk.h>
+    #include <dt-bindings/power/mt8196-power.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        disp_ovl0_exdma2: dma-controller@32850000 {
+            compatible = "mediatek,mt8196-exdma";
+            reg = <0 0x32850000 0 0x1000>;
+            clocks = <&ovlsys_config_clk CLK_OVL_EXDMA2_DISP>;
+            power-domains = <&hfrpsys MT8196_POWER_DOMAIN_OVL0_DORMANT>;
+            mediatek,larb = <&smi_larb0>;
+            iommus = <&mm_smmu 144>;
+            #dma-cells = <1>;
+        };
+    };