[v2,04/15] dt-bindings: display: mediatek: padding: Add documentation for MT8188

Message ID 20230614073125.17958-5-shawn.sung@mediatek.com
State New
Headers
Series Add display driver for MT8188 VDOSYS1 |

Commit Message

Shawn Sung (宋孝謙) June 14, 2023, 7:31 a.m. UTC
  PADDING is a new hardware module on MediaTek MT8188,
Add device tree bindings documentation for it.

Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
---
 .../display/mediatek/mediatek,padding.yaml    | 81 +++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,padding.yaml

--
2.18.0
  

Comments

Krzysztof Kozlowski June 15, 2023, 8:32 a.m. UTC | #1
On 14/06/2023 09:31, Hsiao Chien Sung wrote:
> PADDING is a new hardware module on MediaTek MT8188,
> Add device tree bindings documentation for it.
> 

A nit, subject: drop second/last, redundant "documentation for". The
"dt-bindings" prefix is already stating that these are bindings and
documentation.

> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
>  .../display/mediatek/mediatek,padding.yaml    | 81 +++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,padding.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,padding.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,padding.yaml
> new file mode 100644
> index 000000000000..390a518fa2cf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,padding.yaml
> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/mediatek/mediatek,padding.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek PADDING

MediaTek Foo Bar Padding

Please explain what is this. PADDING does not look like acronym. If it
is, expand it.

> +
> +maintainers:
> +  - Chun-Kuang Hu <chunkuang.hu@kernel.org>
> +  - Philipp Zabel <p.zabel@pengutronix.de>
> +
> +description:
> +  MediaTek PADDING provides ability to VDOSYS1 to add pixels to width and height

Expand the acronym.

> +  of a layer with a specified color.
> +  Since MIXER in VDOSYS1 requires the width of a layer to be 2-pixel-align, or
> +  4-pixel-align when ETHDR is enabled, we need PADDING to deal with odd width.
> +  Please notice that even if the PADDING is in bypass mode, settings in the
> +  registers must be cleared to 0, or undefined behaviors could happen.
> +
> +properties:
> +  compatible:
> +    const: mediatek,mt8188-padding
> +
> +  reg:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: RDMA Clock
> +
> +  mediatek,gce-client-reg:
> +    description:
> +      GCE (Global Command Engine) is a multi-core micro processor that helps
> +      its clients to execute commands without interrupting CPU. This property
> +      describes GCE client's information that is composed by 4 fields.
> +      1. pHandle of the GCE (there may be several GCE processors)
> +      2. Sub-system ID defined in the dt-binding like a user ID
> +         (Please refer to include/dt-bindings/gce/<chip>-gce.h)
> +      3. Offset from base address of the subsys you are at
> +      4. Size of the register the client needs
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      items:
> +        - description: pHandle of the GCE

Phandle (if first in sentence) or phandle. It's not a pH unit. Fix it in
other places as well.


> +        - description: Subsys ID defined in the dt-binding
> +        - description: Offset from base address of the subsys
> +        - description: Size of register


Best regards,
Krzysztof
  
Shawn Sung (宋孝謙) June 16, 2023, 5:40 a.m. UTC | #2
Hi Krzysztof,

Got it, the new title will be “dt-bindings: display: mediatek: padding:
Add MT8188”.

Since “PADDING” is not an acronym but just padding, in this case is
used to pad or stuff pixels to layers, I’ll change all of them to
“Padding” in the next version.

For “MediaTek Foo Bar Padding” suggestion, I changed it to “MediaTek
Display Padding”, hope it could make more sense.

Thanks,
Hsiao Chien Sung

On Thu, 2023-06-15 at 10:32 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 14/06/2023 09:31, Hsiao Chien Sung wrote:
> > PADDING is a new hardware module on MediaTek MT8188,
> > Add device tree bindings documentation for it.
> > 
> 
> A nit, subject: drop second/last, redundant "documentation for". The
> "dt-bindings" prefix is already stating that these are bindings and
> documentation.
> 
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > ---
> >  .../display/mediatek/mediatek,padding.yaml    | 81
> +++++++++++++++++++
> >  1 file changed, 81 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/display/mediatek/mediatek,padding.y
> aml
> > 
> > diff --git
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,padding
> .yaml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,padding
> .yaml
> > new file mode 100644
> > index 000000000000..390a518fa2cf
> > --- /dev/null
> > +++
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,padding
> .yaml
> > @@ -0,0 +1,81 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: 
> http://devicetree.org/schemas/display/mediatek/mediatek,padding.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek PADDING
> 
> MediaTek Foo Bar Padding
> 
> Please explain what is this. PADDING does not look like acronym. If
> it
> is, expand it.
> 
> > +
> > +maintainers:
> > +  - Chun-Kuang Hu <chunkuang.hu@kernel.org>
> > +  - Philipp Zabel <p.zabel@pengutronix.de>
> > +
> > +description:
> > +  MediaTek PADDING provides ability to VDOSYS1 to add pixels to
> width and height
> 
> Expand the acronym.
> 
> > +  of a layer with a specified color.
> > +  Since MIXER in VDOSYS1 requires the width of a layer to be 2-
> pixel-align, or
> > +  4-pixel-align when ETHDR is enabled, we need PADDING to deal
> with odd width.
> > +  Please notice that even if the PADDING is in bypass mode,
> settings in the
> > +  registers must be cleared to 0, or undefined behaviors could
> happen.
> > +
> > +properties:
> > +  compatible:
> > +    const: mediatek,mt8188-padding
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: RDMA Clock
> > +
> > +  mediatek,gce-client-reg:
> > +    description:
> > +      GCE (Global Command Engine) is a multi-core micro processor
> that helps
> > +      its clients to execute commands without interrupting CPU.
> This property
> > +      describes GCE client's information that is composed by 4
> fields.
> > +      1. pHandle of the GCE (there may be several GCE processors)
> > +      2. Sub-system ID defined in the dt-binding like a user ID
> > +         (Please refer to include/dt-bindings/gce/<chip>-gce.h)
> > +      3. Offset from base address of the subsys you are at
> > +      4. Size of the register the client needs
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    items:
> > +      items:
> > +        - description: pHandle of the GCE
> 
> Phandle (if first in sentence) or phandle. It's not a pH unit. Fix it
> in
> other places as well.
> 
> 
> > +        - description: Subsys ID defined in the dt-binding
> > +        - description: Offset from base address of the subsys
> > +        - description: Size of register
> 
> 
> Best regards,
> Krzysztof
>
  

Patch

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,padding.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,padding.yaml
new file mode 100644
index 000000000000..390a518fa2cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,padding.yaml
@@ -0,0 +1,81 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/mediatek/mediatek,padding.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek PADDING
+
+maintainers:
+  - Chun-Kuang Hu <chunkuang.hu@kernel.org>
+  - Philipp Zabel <p.zabel@pengutronix.de>
+
+description:
+  MediaTek PADDING provides ability to VDOSYS1 to add pixels to width and height
+  of a layer with a specified color.
+  Since MIXER in VDOSYS1 requires the width of a layer to be 2-pixel-align, or
+  4-pixel-align when ETHDR is enabled, we need PADDING to deal with odd width.
+  Please notice that even if the PADDING is in bypass mode, settings in the
+  registers must be cleared to 0, or undefined behaviors could happen.
+
+properties:
+  compatible:
+    const: mediatek,mt8188-padding
+
+  reg:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: RDMA Clock
+
+  mediatek,gce-client-reg:
+    description:
+      GCE (Global Command Engine) is a multi-core micro processor that helps
+      its clients to execute commands without interrupting CPU. This property
+      describes GCE client's information that is composed by 4 fields.
+      1. pHandle of the GCE (there may be several GCE processors)
+      2. Sub-system ID defined in the dt-binding like a user ID
+         (Please refer to include/dt-bindings/gce/<chip>-gce.h)
+      3. Offset from base address of the subsys you are at
+      4. Size of the register the client needs
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: pHandle of the GCE
+        - description: Subsys ID defined in the dt-binding
+        - description: Offset from base address of the subsys
+        - description: Size of register
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - power-domains
+  - clocks
+  - mediatek,gce-client-reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/mediatek,mt8188-clk.h>
+    #include <dt-bindings/power/mediatek,mt8188-power.h>
+    #include <dt-bindings/gce/mt8195-gce.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        padding0: padding@1c11d000 {
+            compatible = "mediatek,mt8188-padding";
+            reg = <0 0x1c11d000 0 0x1000>;
+            clocks = <&vdosys1 CLK_VDO1_PADDING0>;
+            power-domains = <&spm MT8188_POWER_DOMAIN_VDOSYS1>;
+            mediatek,gce-client-reg = <&gce0 SUBSYS_1c11XXXX 0xd000 0x1000>;
+        };
+    };