On Mon, 2022-05-09 at 15:35 +0800, Krzysztof Kozlowski wrote: > On 09/05/2022 06:43, Rex-BC Chen wrote: > > From: "Nancy.Lin" <nancy.lin@xxxxxxxxxxxx> > > > > Add vdosys1 ETHDR definition. > > > > Signed-off-by: Nancy.Lin <nancy.lin@xxxxxxxxxxxx> > > Reviewed-by: Chun-Kuang Hu <chunkuang.hu@xxxxxxxxxx> > > Reviewed-by: AngeloGioacchino Del Regno < > > angelogioacchino.delregno@xxxxxxxxxxxxx> > > --- > > .../display/mediatek/mediatek,ethdr.yaml | 191 > > ++++++++++++++++++ > > 1 file changed, 191 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y > > aml > > > > diff --git > > a/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr > > .yaml > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr > > .yaml > > new file mode 100644 > > index 000000000000..65f22fba9fed > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr > > .yaml > > @@ -0,0 +1,191 @@ > > +# 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,ethdr.yaml*__;Iw!!CTRNKA9wMg0ARbw!0l-uil4PAknmCtuDbYhilIpDVA7jzpoeImD2nUauP75Bx1wg3O7BVgM4gJL1ckoqSq7r0276AvMSgzFjlPAag3Pv4Q$ > > > > +$schema: > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!0l-uil4PAknmCtuDbYhilIpDVA7jzpoeImD2nUauP75Bx1wg3O7BVgM4gJL1ckoqSq7r0276AvMSgzFjlPCRT1R8yg$ > > > > + > > +title: MediaTek Ethdr Device Tree Bindings > > s/Device Tree Bindings// Hello Krzysztof, Thanks for your review. We will remove "Device Tree Bindings" in next version. > > You need to add some description of a device. What is a Ethdr? > Ethdr device is described in the description section. Do I need to add anything else? > > + > > +maintainers: > > + - Chun-Kuang Hu <chunkuang.hu@xxxxxxxxxx> > > + - Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > > + > > +description: > > + ETHDR is designed for HDR video and graphics conversion in the > > external display path. > > + It handles multiple HDR input types and performs tone mapping, > > color space/color > > + format conversion, and then combine different layers, output the > > required HDR or > > + SDR signal to the subsequent display path. This engine is > > composed of two video > > + frontends, two graphic frontends, one video backend and a mixer. > > ETHDR has two > > + DMA function blocks, DS and ADL. These two function blocks read > > the pre-programmed > > + registers from DRAM and set them to HW in the v-blanking period. > > Block does not look like wrapped at 80. > ok, we will fix this in next version. > > + > > +properties: > > + compatible: > > + items: > > One item, so no items. ok, we will modofy like this: properties: compatible: - const: mediatek,mt8195-disp-ethdr > > > + - const: mediatek,mt8195-disp-ethdr > > + > > + reg: > > + maxItems: 7 > > + > > + reg-names: > > + items: > > + - const: mixer > > + - const: vdo_fe0 > > + - const: vdo_fe1 > > + - const: gfx_fe0 > > + - const: gfx_fe1 > > + - const: vdo_be > > + - const: adl_ds > > + > > + interrupts: > > + maxItems: 1 > > + > > + iommus: > > + description: The compatible property is DMA function blocks. > > I don't understand this at all. > > > + Should point to the respective IOMMU block with master port > > as argument, > > + see > > Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for > > + details. > > Just skip the description, it's same everywhere. OK, we will remove the whole description. > > > + minItems: 1 > > + maxItems: 2 > > + > > + clocks: > > + items: > > + - description: mixer clock > > + - description: video frontend 0 clock > > + - description: video frontend 1 clock > > + - description: graphic frontend 0 clock > > + - description: graphic frontend 1 clock > > + - description: video backend clock > > + - description: autodownload and menuload clock > > + - description: video frontend 0 async clock > > + - description: video frontend 1 async clock > > + - description: graphic frontend 0 async clock > > + - description: graphic frontend 1 async clock > > + - description: video backend async clock > > + - description: ethdr top clock > > + > > + clock-names: > > + items: > > + - const: mixer > > + - const: vdo_fe0 > > + - const: vdo_fe1 > > + - const: gfx_fe0 > > + - const: gfx_fe1 > > + - const: vdo_be > > + - const: adl_ds > > + - const: vdo_fe0_async > > + - const: vdo_fe1_async > > + - const: gfx_fe0_async > > + - const: gfx_fe1_async > > + - const: vdo_be_async > > + - const: ethdr_top > > + > > + power-domains: > > + maxItems: 1 > > + > > + resets: > > + items: > > + - description: video frontend 0 async reset > > + - description: video frontend 1 async reset > > + - description: graphic frontend 0 async reset > > + - description: graphic frontend 1 async reset > > + - description: video backend async reset > > + > > + reset-names: > > + items: > > + - const: vdo_fe0_async > > + - const: vdo_fe1_async > > + - const: gfx_fe0_async > > + - const: gfx_fe1_async > > + - const: vdo_be_async > > + > > + mediatek,gce-client-reg: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + description: The register of display function block to be set > > by gce. > > + There are 4 arguments in this property, gce node, subsys id, > > offset and > > + register size. The subsys id is defined in the gce header of > > each chips > > + include/include/dt-bindings/gce/<chip>-gce.h, mapping to the > > register of > > + display function block. > > + items: > > + items: > > + - description: phandle of GCE > > + - description: GCE subsys id > > + - description: register offset > > + - description: register size > > + minItems: 7 > > + maxItems: 7 > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > + - interrupts > > + - power-domains > > + - resets > > + - mediatek,gce-client-reg > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/clock/mt8195-clk.h> > > + #include <dt-bindings/gce/mt8195-gce.h> > > + #include <dt-bindings/memory/mt8195-memory-port.h> > > + #include <dt-bindings/power/mt8195-power.h> > > + #include <dt-bindings/reset/mt8195-resets.h> > > + > > + soc { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + disp_ethdr@1c114000 { > > No underscores in node name. Generic node names, so display- > controller? > OK, we will change the node name to ethdr like in dts like this: ethdr0: ethdr@1c114000 { ... } https://patchwork.kernel.org/project/linux-mediatek/patch/20220504091440.2052-26-nancy.lin@xxxxxxxxxxxx/ BRs, Rex > > Best regards, > Krzysztof
