| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e4119fa6-a4b7-f59e-7115-044fa83c9063@linaro.org>
Date: Tue, 12 Sep 2023 10:19:54 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Moudy Ho <moudy.ho@...iatek.com>,
Chun-Kuang Hu <chunkuang.hu@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Matthias Brugger <matthias.bgg@...il.com>,
Hans Verkuil <hverkuil-cisco@...all.nl>
Cc: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
dri-devel@...ts.freedesktop.org,
linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v5 3/3] dt-binding: mediatek: add MediaTek mt8195 MDP3
components
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@...iatek.com>
> ---
> .../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@...il.com>
> + - Moudy Ho <moudy.ho@...iatek.com>
> +
> +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@...02000 {
> + 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@...il.com>
> + - Moudy Ho <moudy.ho@...iatek.com>
> +
> +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@...04000 {
> + 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@...il.com>
> + - Moudy Ho <moudy.ho@...iatek.com>
> +
> +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@...0a000 {
> + 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
Powered by blists - more mailing lists