On Fri, Sep 12, 2025 at 2:06 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > > On Thu, Sep 11, 2025 at 12:09:50PM -0300, Ariel D'Alessandro wrote: > > Convert the existing text-based DT bindings for MediaTek MT8173 Media Data > > Path to a DT schema. > > > > Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@xxxxxxxxxxxxx> > > --- > > .../bindings/media/mediatek,mt8173-mdp.yaml | 169 ++++++++++++++++++ > > .../bindings/media/mediatek-mdp.txt | 95 ---------- > > 2 files changed, 169 insertions(+), 95 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8173-mdp.yaml > > delete mode 100644 Documentation/devicetree/bindings/media/mediatek-mdp.txt > > > > diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8173-mdp.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8173-mdp.yaml > > new file mode 100644 > > index 0000000000000..8ca33a733c478 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/mediatek,mt8173-mdp.yaml > > @@ -0,0 +1,169 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/mediatek,mt8173-mdp.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: MediaTek MT8173 Media Data Path > > + > > +maintainers: > > + - Ariel D'Alessandro <ariel.dalessandro@xxxxxxxxxxxxx> > > + > > +description: > > + Media Data Path is used for scaling and color space conversion. > > + > > +properties: > > + compatible: > > + oneOf: > > + - enum: > > + - mediatek,mt8173-mdp-rdma > > + - mediatek,mt8173-mdp-rsz > > + - mediatek,mt8173-mdp-wdma > > + - mediatek,mt8173-mdp-wrot > > Why there is no mediatek,mt8173-mdp here? What does this compatible > represent? > > > + - items: > > + - const: mediatek,mt8173-mdp-rdma > > Still suspicious. Device cannot be simulatanously: compatible and not > compatible. This is not a well known cat that has superposition of two > states, whenenver you look the other way. > > Maybe the old binding was incorrect, maybe the in-tree DTS is incorrect. > Whichever the reason, this must be investigated and documented, because > by standard rules this is wrong. Each wrong code needs very clear > explanations (and "someone did it" is not a good enough explanation). My guess is that "mediatek,mt8173-mdp" is meant to serve as a single entry point for the implementation to bind the driver to. The MDP is a Data Pipeline and there could be multiple instances of the same IP block, as seen in the original example. The datasheet I have doesn't cover the "RDMA" block specifically, so I can't say whether there is an actual difference between the two RDMA blocks. ChenYu > > + - const: mediatek,mt8173-mdp > > + > > + reg: > > + maxItems: 1 > > + > > + clocks: > > + minItems: 1 > > + maxItems: 2 > > + > > + power-domains: > > + maxItems: 1 > > + > > + iommus: > > + maxItems: 1 > > + > > + mediatek,vpu: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: > > + phandle to Mediatek Video Processor Unit for HW Codec encode/decode and > > + image processing. > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - power-domains > > + > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: mediatek,mt8173-mdp-rdma > > + then: > > + properties: > > + clocks: > > + items: > > + - description: Main clock > > + - description: Mutex clock > > + else: > > + properties: > > + clocks: > > + items: > > + - description: Main clock > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - mediatek,mt8173-mdp-rdma > > + - mediatek,mt8173-mdp-wdma > > + - mediatek,mt8173-mdp-wrot > > + then: > > + required: > > + - iommus > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: mediatek,mt8173-mdp > > + then: > > + required: > > + - mediatek,vpu > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/mt8173-clk.h> > > + #include <dt-bindings/memory/mt8173-larb-port.h> > > + #include <dt-bindings/power/mt8173-power.h> > > + > > + soc { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + mdp_rdma0: rdma@14001000 { > > + compatible = "mediatek,mt8173-mdp-rdma", > > + "mediatek,mt8173-mdp"; > > + reg = <0 0x14001000 0 0x1000>; > > + clocks = <&mmsys CLK_MM_MDP_RDMA0>, > > + <&mmsys CLK_MM_MUTEX_32K>; > > + power-domains = <&spm MT8173_POWER_DOMAIN_MM>; > > + iommus = <&iommu M4U_PORT_MDP_RDMA0>; > > + mediatek,vpu = <&vpu>; > > + }; > > + > > + mdp_rdma1: rdma@14002000 { > > + compatible = "mediatek,mt8173-mdp-rdma"; > > + reg = <0 0x14002000 0 0x1000>; > > + clocks = <&mmsys CLK_MM_MDP_RDMA1>, > > + <&mmsys CLK_MM_MUTEX_32K>; > > + power-domains = <&spm MT8173_POWER_DOMAIN_MM>; > > + iommus = <&iommu M4U_PORT_MDP_RDMA1>; > > + }; > > My previous comment applies. > > Keep one or two examples. > > Best regards, > Krzysztof >