Re: [PATCH v5 3/3] dt-binding: mediatek: add MediaTek mt8195 MDP3 components

From: Krzysztof Kozlowski
Date: Tue Sep 12 2023 - 04:20:01 EST

Next message: Huang Ying: "[PATCH -V3 0/4] memory tiering: calculate abstract distance based on ACPI HMAT"
Previous message: Geert Uytterhoeven: "Re: [PATCH] drm: fix up fbdev Kconfig defaults"
In reply to: Moudy Ho: "[PATCH v5 3/3] dt-binding: mediatek: add MediaTek mt8195 MDP3 components"
Next in thread: Moudy Ho (何宗原): "Re: [PATCH v5 3/3] dt-binding: mediatek: add MediaTek mt8195 MDP3 components"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 12/09/2023 09:56, Moudy Ho wrote:
> Introduce more MDP3 components present in MT8195.

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.

>
> Signed-off-by: Moudy Ho <moudy.ho@xxxxxxxxxxxx>
> ---
> .../display/mediatek/mediatek,aal.yaml | 2 +-
> .../display/mediatek/mediatek,color.yaml | 2 +-
> .../display/mediatek/mediatek,merge.yaml | 1 +
> .../display/mediatek/mediatek,ovl.yaml | 2 +-
> .../display/mediatek/mediatek,split.yaml | 1 +
> .../bindings/media/mediatek,mdp3-fg.yaml | 61 +++++++++++++++++++
> .../bindings/media/mediatek,mdp3-hdr.yaml | 60 ++++++++++++++++++
> .../bindings/media/mediatek,mdp3-pad.yaml | 61 +++++++++++++++++++
> .../bindings/media/mediatek,mdp3-rdma.yaml | 16 ++---
> .../bindings/media/mediatek,mdp3-stitch.yaml | 61 +++++++++++++++++++
> .../bindings/media/mediatek,mdp3-tcc.yaml | 60 ++++++++++++++++++
> .../bindings/media/mediatek,mdp3-tdshp.yaml | 61 +++++++++++++++++++
> 12 files changed, 378 insertions(+), 10 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-fg.yaml
> create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-hdr.yaml
> create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-pad.yaml
> create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-stitch.yaml
> create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-tcc.yaml
> create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-tdshp.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml
> index 7fd42c8fdc32..04b1314d00f2 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml
> @@ -24,6 +24,7 @@ properties:
> - enum:
> - mediatek,mt8173-disp-aal
> - mediatek,mt8183-disp-aal
> + - mediatek,mt8195-mdp3-aal
> - items:
> - enum:
> - mediatek,mt2712-disp-aal
> @@ -63,7 +64,6 @@ properties:
> required:
> - compatible
> - reg
> - - interrupts

Why? commit msg tells nothing about it. Why interrupt is not erquired in
mt8173? How dropping such requirement is anyhow related to mt8195?

> - power-domains
> - clocks
>
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> index f21e44092043..8e97b0a6a7b3 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml
> @@ -26,6 +26,7 @@ properties:
> - mediatek,mt2701-disp-color
> - mediatek,mt8167-disp-color
> - mediatek,mt8173-disp-color
> + - mediatek,mt8195-mdp3-color
> - items:
> - enum:
> - mediatek,mt7623-disp-color
> @@ -66,7 +67,6 @@ properties:
> required:
> - compatible
> - reg
> - - interrupts

Why?

> - power-domains
> - clocks
>
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
> index eead5cb8636e..401498523404 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml
> @@ -24,6 +24,7 @@ properties:
> - enum:
> - mediatek,mt8173-disp-merge
> - mediatek,mt8195-disp-merge
> + - mediatek,mt8195-mdp3-merge
> - items:
> - const: mediatek,mt6795-disp-merge
> - const: mediatek,mt8173-disp-merge
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml
> index 3e1069b00b56..10d4d4f64e09 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml
> @@ -26,6 +26,7 @@ properties:
> - mediatek,mt8173-disp-ovl
> - mediatek,mt8183-disp-ovl
> - mediatek,mt8192-disp-ovl
> + - mediatek,mt8195-mdp3-ovl
> - items:
> - enum:
> - mediatek,mt7623-disp-ovl
> @@ -76,7 +77,6 @@ properties:
> required:
> - compatible
> - reg
> - - interrupts

Why?

> - power-domains
> - clocks
> - iommus
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> index a8a5c9608598..a96b271e3240 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> @@ -23,6 +23,7 @@ properties:
> oneOf:
> - enum:
> - mediatek,mt8173-disp-split
> + - mediatek,mt8195-mdp3-split
> - items:
> - const: mediatek,mt6795-disp-split
> - const: mediatek,mt8173-disp-split
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-fg.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-fg.yaml
> new file mode 100644
> index 000000000000..71fd449de8b4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-fg.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/mediatek,mdp3-fg.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Media Data Path 3 FG
> +
> +maintainers:
> + - Matthias Brugger <matthias.bgg@xxxxxxxxx>
> + - Moudy Ho <moudy.ho@xxxxxxxxxxxx>
> +
> +description:
> + One of Media Data Path 3 (MDP3) components used to add film grain
> + according to AV1 spec.
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt8195-mdp3-fg
> +
> + reg:
> + maxItems: 1
> +
> + mediatek,gce-client-reg:
> + description:
> + The register of display function block to be set by gce. There are 4 arguments,
> + such as gce node, subsys id, offset and register size. The subsys id that is
> + mapping to the register of display function blocks is defined in the gce header
> + include/dt-bindings/gce/<chip>-gce.h of each chips.
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + items:
> + items:
> + - description: phandle of GCE
> + - description: GCE subsys id
> + - description: register offset
> + - description: register size
> + maxItems: 1
> +
> + clocks:
> + minItems: 1

This must be maxItems. Use existing code as an example, do not re-invent it.

> +
> +required:
> + - compatible
> + - reg
> + - mediatek,gce-client-reg
> + - clocks
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/mt8195-clk.h>
> + #include <dt-bindings/gce/mt8195-gce.h>
> +
> + display@14002000 {
> + compatible = "mediatek,mt8195-mdp3-fg";
> + reg = <0x14002000 0x1000>;
> + mediatek,gce-client-reg = <&gce1 SUBSYS_1400XXXX 0x2000 0x1000>;
> + clocks = <&vppsys0 CLK_VPP0_MDP_FG>;
> + };
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-hdr.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-hdr.yaml
> new file mode 100644
> index 000000000000..fb1bb5a9e57f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-hdr.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/mediatek,mdp3-hdr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Media Data Path 3 HDR
> +
> +maintainers:
> + - Matthias Brugger <matthias.bgg@xxxxxxxxx>
> + - Moudy Ho <moudy.ho@xxxxxxxxxxxx>
> +
> +description:
> + One of Media Data Path 3 (MDP3) components used to perform HDR to SDR
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt8195-mdp3-hdr
> +
> + reg:
> + maxItems: 1
> +
> + mediatek,gce-client-reg:
> + description:
> + The register of display function block to be set by gce. There are 4 arguments,
> + such as gce node, subsys id, offset and register size. The subsys id that is
> + mapping to the register of display function blocks is defined in the gce header
> + include/dt-bindings/gce/<chip>-gce.h of each chips.
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + items:
> + items:
> + - description: phandle of GCE
> + - description: GCE subsys id
> + - description: register offset
> + - description: register size
> + maxItems: 1
> +
> + clocks:
> + minItems: 1

Here as well. Why is this minitems?

> +
> +required:
> + - compatible
> + - reg
> + - mediatek,gce-client-reg
> + - clocks
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/mt8195-clk.h>
> + #include <dt-bindings/gce/mt8195-gce.h>
> +
> + display@14004000 {
> + compatible = "mediatek,mt8195-mdp3-hdr";
> + reg = <0x14004000 0x1000>;
> + mediatek,gce-client-reg = <&gce1 SUBSYS_1400XXXX 0x4000 0x1000>;
> + clocks = <&vppsys0 CLK_VPP0_MDP_HDR>;
> + };
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-pad.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-pad.yaml
> new file mode 100644
> index 000000000000..13b66c5985fe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-pad.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/mediatek,mdp3-pad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Media Data Path 3 PADDING
> +
> +maintainers:
> + - Matthias Brugger <matthias.bgg@xxxxxxxxx>
> + - Moudy Ho <moudy.ho@xxxxxxxxxxxx>
> +
> +description:
> + One of Media Data Path 3 (MDP3) components used to insert
> + pre-defined color or alpha value to arbitrary side of image.
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt8195-mdp3-pad

And you cannot add it to existing padding because?

> +
> + reg:
> + maxItems: 1
> +
> + mediatek,gce-client-reg:
> + description:
> + The register of display function block to be set by gce. There are 4 arguments,
> + such as gce node, subsys id, offset and register size. The subsys id that is
> + mapping to the register of display function blocks is defined in the gce header
> + include/dt-bindings/gce/<chip>-gce.h of each chips.
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + items:
> + items:
> + - description: phandle of GCE
> + - description: GCE subsys id
> + - description: register offset
> + - description: register size
> + maxItems: 1
> +
> + clocks:
> + minItems: 1

Nope

> +
> +required:
> + - compatible
> + - reg
> + - mediatek,gce-client-reg
> + - clocks
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/mt8195-clk.h>
> + #include <dt-bindings/gce/mt8195-gce.h>
> +
> + display@1400a000 {
> + compatible = "mediatek,mt8195-mdp3-pad";
> + reg = <0x1400a000 0x1000>;
> + mediatek,gce-client-reg = <&gce1 SUBSYS_1400XXXX 0xa000 0x1000>;
> + clocks = <&vppsys0 CLK_VPP0_PADDING>;
> + };
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> index 0c22571d8c22..17cd5b587e23 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> @@ -23,6 +23,7 @@ properties:
> enum:
> - mediatek,mt8183-mdp3-rdma
> - mediatek,mt8195-vdo1-rdma
> + - mediatek,mt8195-mdp3-rdma

m is before v

>
> reg:
> maxItems: 1
> @@ -50,17 +51,19 @@ properties:
> maxItems: 1
>
> clocks:
> - items:
> - - description: RDMA clock
> - - description: RSZ clock
> + oneOf:
> + - items:
> + - description: RDMA clock
> + - description: SRAM shared component clock
> + - items:
> + - description: RDMA clock

Why now mt8183 can have SRAM clock optional? How changing mt8183 is
related to this patch?

I'll finish the review, sorry fix basics here.

Best regards,
Krzysztof

Next message: Huang Ying: "[PATCH -V3 0/4] memory tiering: calculate abstract distance based on ACPI HMAT"
Previous message: Geert Uytterhoeven: "Re: [PATCH] drm: fix up fbdev Kconfig defaults"
In reply to: Moudy Ho: "[PATCH v5 3/3] dt-binding: mediatek: add MediaTek mt8195 MDP3 components"
Next in thread: Moudy Ho (何宗原): "Re: [PATCH v5 3/3] dt-binding: mediatek: add MediaTek mt8195 MDP3 components"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]