diff mbox series

[v29,1/7] dt-bindings: mediatek: add ethdr definition for mt8195

Message ID 20221227081011.6426-2-nancy.lin@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Add MediaTek SoC DRM (vdosys1) support for mt8195 | expand

Commit Message

Nancy Lin (林欣螢) Dec. 27, 2022, 8:10 a.m. UTC
Add vdosys1 ETHDR definition.

Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com>
Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../display/mediatek/mediatek,ethdr.yaml      | 188 ++++++++++++++++++
 1 file changed, 188 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml

Comments

Nancy Lin (林欣螢) March 15, 2023, 3:45 a.m. UTC | #1
On Tue, 2022-12-27 at 16:10 +0800, Nancy.Lin wrote:
> Add vdosys1 ETHDR definition.
> 
> Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com>
> Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Tested-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  .../display/mediatek/mediatek,ethdr.yaml      | 188
> ++++++++++++++++++
>  1 file changed, 188 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yam
> l
> 
> diff --git
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y
> aml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y
> aml
> new file mode 100644
> index 000000000000..3b11e47a8834
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y
> aml
> @@ -0,0 +1,188 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: 
> http://devicetree.org/schemas/display/mediatek/mediatek,ethdr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Ethdr Device
> +
> +maintainers:
> +  - Chun-Kuang Hu <chunkuang.hu@kernel.org>
> +  - Philipp Zabel <p.zabel@pengutronix.de>
> +
> +description:
> +  ETHDR (ET High Dynamic Range) is a MediaTek internal HDR engine
> and 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.
> +
> +properties:
> +  compatible:
> +    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:
> +    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/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
> +

Hi Rob and krzysztof,

I got the two messages when running dt_binding_check [1]. This binding
patch was sent previously in [2]. 

If I remove the following items/minItems/maxItems in the mediatek,gce-
client property, the two message disappear. I don't know what's wrong
with the original syntax. Do you have any suggestions for this?

-    items:
-      items:
-        - description: phandle of GCE
-        - description: GCE subsys id
-        - description: register offset
-        - description: register size
-    minItems: 7
-    maxItems: 7


[1].
Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.examp
le.dtb
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7, 16384,
4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
45056, 4096, 4294967295, 7, 49152, 4096] is too long
        From schema:
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.yaml
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7, 16384,
4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
45056, 4096, 4294967295, 7, 49152, 4096]] is too short
        From schema:
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.yaml

[2] [RESEND,v5,3/3] dt-bindings: mediatek: add ethdr definition for
mt8195

https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20220608043852.4980-4-rex-bc.chen@mediatek.com/


Regards,
Nancy















> +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>;
> +
> +        hdr-engine@1c114000 {
> +                compatible = "mediatek,mt8195-disp-ethdr";
> +                reg = <0 0x1c114000 0 0x1000>,
> +                      <0 0x1c115000 0 0x1000>,
> +                      <0 0x1c117000 0 0x1000>,
> +                      <0 0x1c119000 0 0x1000>,
> +                      <0 0x1c11a000 0 0x1000>,
> +                      <0 0x1c11b000 0 0x1000>,
> +                      <0 0x1c11c000 0 0x1000>;
> +                reg-names = "mixer", "vdo_fe0", "vdo_fe1",
> "gfx_fe0", "gfx_fe1",
> +                            "vdo_be", "adl_ds";
> +                mediatek,gce-client-reg = <&gce0 SUBSYS_1c11XXXX
> 0x4000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0x5000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0x7000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0x9000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0xa000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0xb000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0xc000 0x1000>;
> +                clocks = <&vdosys1 CLK_VDO1_DISP_MIXER>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE0>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE1>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE0>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE1>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_BE>,
> +                         <&vdosys1 CLK_VDO1_26M_SLOW>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE0_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE1_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE0_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE1_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_BE_DL_ASYNC>,
> +                         <&topckgen CLK_TOP_ETHDR>;
> +                clock-names = "mixer", "vdo_fe0", "vdo_fe1",
> "gfx_fe0", "gfx_fe1",
> +                              "vdo_be", "adl_ds", "vdo_fe0_async",
> "vdo_fe1_async",
> +                              "gfx_fe0_async",
> "gfx_fe1_async","vdo_be_async",
> +                              "ethdr_top";
> +                power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>;
> +                iommus = <&iommu_vpp M4U_PORT_L3_HDR_DS>,
> +                         <&iommu_vpp M4U_PORT_L3_HDR_ADL>;
> +                interrupts = <GIC_SPI 517 IRQ_TYPE_LEVEL_HIGH 0>; /*
> disp mixer */
> +                resets = <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE0_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE1_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE0_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE1_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_BE_DL_ASYNC>;
> +                reset-names = "vdo_fe0_async", "vdo_fe1_async",
> "gfx_fe0_async",
> +                              "gfx_fe1_async", "vdo_be_async";
> +        };
> +    };
> +...
Krzysztof Kozlowski March 15, 2023, 7:16 a.m. UTC | #2
On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote:

Trim the replies and remove unneeded context. You want to get the
attention of other people, not force them to read entire email.

>> +  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/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
>> +
> 
> Hi Rob and krzysztof,
> 
> I got the two messages when running dt_binding_check [1]. This binding
> patch was sent previously in [2]. 
> 
> If I remove the following items/minItems/maxItems in the mediatek,gce-
> client property, the two message disappear. I don't know what's wrong
> with the original syntax. Do you have any suggestions for this?
> 
> -    items:
> -      items:
> -        - description: phandle of GCE
> -        - description: GCE subsys id
> -        - description: register offset
> -        - description: register size
> -    minItems: 7
> -    maxItems: 7
> 
> 
> [1].
> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.examp
> le.dtb
> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
> hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7, 16384,
> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
> 45056, 4096, 4294967295, 7, 49152, 4096] is too long
>         From schema:

This looks like known issue with phandles with variable number of
arguments. Either we add it to the exceptions or just define it in
reduced way like in other cases - only maxItems: 1 without describing items.


Best regards,
Krzysztof
Nancy Lin (林欣螢) March 16, 2023, 6:19 a.m. UTC | #3
On Wed, 2023-03-15 at 08:16 +0100, Krzysztof Kozlowski wrote:
> On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote:
> 
> Trim the replies and remove unneeded context. You want to get the
> attention of other people, not force them to read entire email.
> 
> > > +  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/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
> > > +
> > 
> > Hi Rob and krzysztof,
> > 
> > I got the two messages when running dt_binding_check [1]. This
> > binding
> > patch was sent previously in [2]. 
> > 
> > If I remove the following items/minItems/maxItems in the
> > mediatek,gce-
> > client property, the two message disappear. I don't know what's
> > wrong
> > with the original syntax. Do you have any suggestions for this?
> > 
> > -    items:
> > -      items:
> > -        - description: phandle of GCE
> > -        - description: GCE subsys id
> > -        - description: register offset
> > -        - description: register size
> > -    minItems: 7
> > -    maxItems: 7
> > 
> > 
> > [1].
> > Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.e
> > xamp
> > le.dtb
> > /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
> > cetr
> > ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
> > hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7,
> > 16384,
> > 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
> > 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
> > 7,
> > 45056, 4096, 4294967295, 7, 49152, 4096] is too long
> >         From schema:
> 
> This looks like known issue with phandles with variable number of
> arguments. Either we add it to the exceptions or just define it in
> reduced way like in other cases - only maxItems: 1 without describing
> items.
> 
> 
> Best regards,
> Krzysztof


Hi Krzysztof,

Thanks for the comment.

But I have several items for this vendor property in the binding
example.
Can I remove maxItems? Change the mediatek,gce-client-reg as [1].

[1]
  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/dt-bindings/gce/<chip>-gce.h, mapping to the register of
display
      function block.

Regards,
Nancy

>
Krzysztof Kozlowski March 16, 2023, 6:31 a.m. UTC | #4
On 16/03/2023 07:19, Nancy Lin (林欣螢) wrote:
> On Wed, 2023-03-15 at 08:16 +0100, Krzysztof Kozlowski wrote:
>> On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote:
>>
>> Trim the replies and remove unneeded context. You want to get the
>> attention of other people, not force them to read entire email.
>>
>>>> +  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/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
>>>> +
>>>
>>> Hi Rob and krzysztof,
>>>
>>> I got the two messages when running dt_binding_check [1]. This
>>> binding
>>> patch was sent previously in [2]. 
>>>
>>> If I remove the following items/minItems/maxItems in the
>>> mediatek,gce-
>>> client property, the two message disappear. I don't know what's
>>> wrong
>>> with the original syntax. Do you have any suggestions for this?
>>>
>>> -    items:
>>> -      items:
>>> -        - description: phandle of GCE
>>> -        - description: GCE subsys id
>>> -        - description: register offset
>>> -        - description: register size
>>> -    minItems: 7
>>> -    maxItems: 7
>>>
>>>
>>> [1].
>>> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.e
>>> xamp
>>> le.dtb
>>> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
>>> cetr
>>> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
>>> hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7,
>>> 16384,
>>> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
>>> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
>>> 7,
>>> 45056, 4096, 4294967295, 7, 49152, 4096] is too long
>>>         From schema:
>>
>> This looks like known issue with phandles with variable number of
>> arguments. Either we add it to the exceptions or just define it in
>> reduced way like in other cases - only maxItems: 1 without describing
>> items.
>>
>>
>> Best regards,
>> Krzysztof
> 
> 
> Hi Krzysztof,
> 
> Thanks for the comment.
> 
> But I have several items for this vendor property in the binding
> example.

Do you? I thought you have one phandle?

> Can I remove maxItems? Change the mediatek,gce-client-reg as [1].
> 
> [1]
>   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/dt-bindings/gce/<chip>-gce.h, mapping to the register of
> display
>       function block.

No, this needs some constraints.

Best regards,
Krzysztof
AngeloGioacchino Del Regno March 16, 2023, 9:53 a.m. UTC | #5
Il 16/03/23 07:31, Krzysztof Kozlowski ha scritto:
> On 16/03/2023 07:19, Nancy Lin (林欣螢) wrote:
>> On Wed, 2023-03-15 at 08:16 +0100, Krzysztof Kozlowski wrote:
>>> On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote:
>>>

..snip..

>>>>
>>>>
>>>> [1].
>>>> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.e
>>>> xamp
>>>> le.dtb
>>>> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
>>>> cetr
>>>> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb:
>>>> hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7,
>>>> 16384,
>>>> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
>>>> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
>>>> 7,
>>>> 45056, 4096, 4294967295, 7, 49152, 4096] is too long
>>>>          From schema:
>>>
>>> This looks like known issue with phandles with variable number of
>>> arguments. Either we add it to the exceptions or just define it in
>>> reduced way like in other cases - only maxItems: 1 without describing
>>> items.
>>>

...

>>
>> But I have several items for this vendor property in the binding
>> example.
> 
> Do you? I thought you have one phandle?
> 
>> Can I remove maxItems? Change the mediatek,gce-client-reg as [1].
>>
>> [1]
>>    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/dt-bindings/gce/<chip>-gce.h, mapping to the register of
>> display
>>        function block.
> 
> No, this needs some constraints.

Hello Krzysztof, Nancy,

Since this series has reached v29, can we please reach an agreement on the bindings
to use here, so that we can get this finally upstreamed?

I will put some examples to try to get this issue resolved.

### Example 1: Constrain the number of GCE entries to *seven* array elements (7x4!)

   mediatek,gce-client-reg:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 1
     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/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
       function block.
     items:
       minItems: 28
       maxItems: 28
       items:                     <----- this block doesn't seem to get checked :\
         - description: phandle of GCE
         - description: GCE subsys id
         - description: register offset
         - description: register size


### Example 2: Don't care about constraining the number of arguments

   mediatek,gce-client-reg:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 1
     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/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
       function block.


Regards,
Angelo
Krzysztof Kozlowski March 16, 2023, 11:36 a.m. UTC | #6
On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:

> Hello Krzysztof, Nancy,
> 
> Since this series has reached v29, can we please reach an agreement on the bindings
> to use here, so that we can get this finally upstreamed?
> 
> I will put some examples to try to get this issue resolved.
> 
> ### Example 1: Constrain the number of GCE entries to *seven* array elements (7x4!)
> 
>    mediatek,gce-client-reg:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      maxItems: 1
>      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/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
>        function block.
>      items:
>        minItems: 28
>        maxItems: 28
>        items:                     <----- this block doesn't seem to get checked :\
>          - description: phandle of GCE
>          - description: GCE subsys id
>          - description: register offset
>          - description: register size

This is what we would like to have but it requires exception in
dtschema. Thus:

> 
> 
> ### Example 2: Don't care about constraining the number of arguments
> 
>    mediatek,gce-client-reg:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      maxItems: 1
>      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/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
>        function block.

use this.

Best regards,
Krzysztof
Nancy Lin (林欣螢) March 17, 2023, 7:55 a.m. UTC | #7
On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
> 
> > Hello Krzysztof, Nancy,
> > 
> > Since this series has reached v29, can we please reach an agreement
> > on the bindings
> > to use here, so that we can get this finally upstreamed?
> > 
> > I will put some examples to try to get this issue resolved.
> > 
> > ### Example 1: Constrain the number of GCE entries to *seven* array
> > elements (7x4!)
> > 
> >    mediatek,gce-client-reg:
> >      $ref: /schemas/types.yaml#/definitions/phandle-array
> >      maxItems: 1
> >      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/dt-bindings/gce/<chip>-gce.h, mapping to the
> > register of display
> >        function block.
> >      items:
> >        minItems: 28
> >        maxItems: 28
> >        items:                     <----- this block doesn't seem to
> > get checked :\
> >          - description: phandle of GCE
> >          - description: GCE subsys id
> >          - description: register offset
> >          - description: register size
> 
> This is what we would like to have but it requires exception in
> dtschema. Thus:
> 
> > 
> > 
> > ### Example 2: Don't care about constraining the number of
> > arguments
> > 
> >    mediatek,gce-client-reg:
> >      $ref: /schemas/types.yaml#/definitions/phandle-array
> >      maxItems: 1
> >      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/dt-bindings/gce/<chip>-gce.h, mapping to the
> > register of display
> >        function block.
> 
> use this.
> 
> Best regards,
> Krzysztof


Hi Krzysztof, Angelo,

Thanks for the comment.
The Example 2 can pass dt_binding_check. 

But the example in the binding has 7 items [1] and dts [2]. Does the
"maxItems: 1" affect any other schema or dts check? 

[1]
+    mediatek,gce-client-reg = <&gce0 SUBSYS_1c11XXXX 0x4000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0x5000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0x7000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0x9000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0xa000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0xb000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0xc000 0x1000>;
> 

[2] [v21,25/25] arm64: dts: mt8195: add display node for vdosys1

https://patchwork.kernel.org/project/linux-mediatek/patch/20220504091440.2052-26-nancy.lin@mediatek.com/

Regards,
Nancy
Krzysztof Kozlowski March 17, 2023, 9:03 a.m. UTC | #8
On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
> On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
>> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
>>
>>> Hello Krzysztof, Nancy,
>>>
>>> Since this series has reached v29, can we please reach an agreement
>>> on the bindings
>>> to use here, so that we can get this finally upstreamed?
>>>
>>> I will put some examples to try to get this issue resolved.
>>>
>>> ### Example 1: Constrain the number of GCE entries to *seven* array
>>> elements (7x4!)
>>>
>>>    mediatek,gce-client-reg:
>>>      $ref: /schemas/types.yaml#/definitions/phandle-array
>>>      maxItems: 1
>>>      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/dt-bindings/gce/<chip>-gce.h, mapping to the
>>> register of display
>>>        function block.
>>>      items:
>>>        minItems: 28
>>>        maxItems: 28
>>>        items:                     <----- this block doesn't seem to
>>> get checked :\
>>>          - description: phandle of GCE
>>>          - description: GCE subsys id
>>>          - description: register offset
>>>          - description: register size
>>
>> This is what we would like to have but it requires exception in
>> dtschema. Thus:
>>
>>>
>>>
>>> ### Example 2: Don't care about constraining the number of
>>> arguments
>>>
>>>    mediatek,gce-client-reg:
>>>      $ref: /schemas/types.yaml#/definitions/phandle-array
>>>      maxItems: 1
>>>      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/dt-bindings/gce/<chip>-gce.h, mapping to the
>>> register of display
>>>        function block.
>>
>> use this.
>>
>> Best regards,
>> Krzysztof
> 
> 
> Hi Krzysztof, Angelo,
> 
> Thanks for the comment.
> The Example 2 can pass dt_binding_check. 
> 
> But the example in the binding has 7 items [1] and dts [2]. Does the
> "maxItems: 1" affect any other schema or dts check? 

Ah, then it should be maxItems: 7, not 1.

Best regards,
Krzysztof
AngeloGioacchino Del Regno March 17, 2023, 9:37 a.m. UTC | #9
Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
> On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
>> On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
>>> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
>>>
>>>> Hello Krzysztof, Nancy,
>>>>
>>>> Since this series has reached v29, can we please reach an agreement
>>>> on the bindings
>>>> to use here, so that we can get this finally upstreamed?
>>>>
>>>> I will put some examples to try to get this issue resolved.
>>>>
>>>> ### Example 1: Constrain the number of GCE entries to *seven* array
>>>> elements (7x4!)
>>>>
>>>>     mediatek,gce-client-reg:
>>>>       $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>       maxItems: 1
>>>>       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/dt-bindings/gce/<chip>-gce.h, mapping to the
>>>> register of display
>>>>         function block.
>>>>       items:
>>>>         minItems: 28
>>>>         maxItems: 28
>>>>         items:                     <----- this block doesn't seem to
>>>> get checked :\
>>>>           - description: phandle of GCE
>>>>           - description: GCE subsys id
>>>>           - description: register offset
>>>>           - description: register size
>>>
>>> This is what we would like to have but it requires exception in
>>> dtschema. Thus:
>>>
>>>>
>>>>
>>>> ### Example 2: Don't care about constraining the number of
>>>> arguments
>>>>
>>>>     mediatek,gce-client-reg:
>>>>       $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>       maxItems: 1
>>>>       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/dt-bindings/gce/<chip>-gce.h, mapping to the
>>>> register of display
>>>>         function block.
>>>
>>> use this.
>>>
>>> Best regards,
>>> Krzysztof
>>
>>
>> Hi Krzysztof, Angelo,
>>
>> Thanks for the comment.
>> The Example 2 can pass dt_binding_check.
>>
>> But the example in the binding has 7 items [1] and dts [2]. Does the
>> "maxItems: 1" affect any other schema or dts check?
> 
> Ah, then it should be maxItems: 7, not 1.
> 

Keep in mind for your v30:

maxItems: 7 will pass - but only if minItems is *not* 7 :-)

-> (so, do not declare minItems, as default is 1) <-

Regards,
Angelo

> Best regards,
> Krzysztof
>
Nancy Lin (林欣螢) March 17, 2023, 9:52 a.m. UTC | #10
On Fri, 2023-03-17 at 10:37 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
> > On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
> > > On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
> > > > On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
> > > > 
> > > > > Hello Krzysztof, Nancy,
> > > > > 
> > > > > Since this series has reached v29, can we please reach an
> > > > > agreement
> > > > > on the bindings
> > > > > to use here, so that we can get this finally upstreamed?
> > > > > 
> > > > > I will put some examples to try to get this issue resolved.
> > > > > 
> > > > > ### Example 1: Constrain the number of GCE entries to *seven*
> > > > > array
> > > > > elements (7x4!)
> > > > > 
> > > > >     mediatek,gce-client-reg:
> > > > >       $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > >       maxItems: 1
> > > > >       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/dt-bindings/gce/<chip>-gce.h, mapping to the
> > > > > register of display
> > > > >         function block.
> > > > >       items:
> > > > >         minItems: 28
> > > > >         maxItems: 28
> > > > >         items:                     <----- this block doesn't
> > > > > seem to
> > > > > get checked :\
> > > > >           - description: phandle of GCE
> > > > >           - description: GCE subsys id
> > > > >           - description: register offset
> > > > >           - description: register size
> > > > 
> > > > This is what we would like to have but it requires exception in
> > > > dtschema. Thus:
> > > > 
> > > > > 
> > > > > 
> > > > > ### Example 2: Don't care about constraining the number of
> > > > > arguments
> > > > > 
> > > > >     mediatek,gce-client-reg:
> > > > >       $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > >       maxItems: 1
> > > > >       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/dt-bindings/gce/<chip>-gce.h, mapping to the
> > > > > register of display
> > > > >         function block.
> > > > 
> > > > use this.
> > > > 
> > > > Best regards,
> > > > Krzysztof
> > > 
> > > 
> > > Hi Krzysztof, Angelo,
> > > 
> > > Thanks for the comment.
> > > The Example 2 can pass dt_binding_check.
> > > 
> > > But the example in the binding has 7 items [1] and dts [2]. Does
> > > the
> > > "maxItems: 1" affect any other schema or dts check?
> > 
> > Ah, then it should be maxItems: 7, not 1.
> > 
> 
> Keep in mind for your v30:
> 
> maxItems: 7 will pass - but only if minItems is *not* 7 :-)
> 
> -> (so, do not declare minItems, as default is 1) <-
> 
> Regards,
> Angelo
> 
Hi Angelo,

I still have one message [1] when runing dt_binding_check for "example
2 + maxItems: 7" [2].

[1]
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7, 16384,
4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
45056, 4096, 4294967295, 7, 49152, 4096]] is too short


[2]
   mediatek,gce-client-reg:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 7
     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/dt-bindings/gce/<chip>-gce.h, mapping to the register of
display
       function block.

Regards,
Nancy


> > Best regards,
> > Krzysztof
> >
AngeloGioacchino Del Regno March 17, 2023, 9:58 a.m. UTC | #11
Il 17/03/23 10:52, Nancy Lin (林欣螢) ha scritto:
> On Fri, 2023-03-17 at 10:37 +0100, AngeloGioacchino Del Regno wrote:
>> Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
>>> On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
>>>> On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
>>>>> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
>>>>>
>>>>>> Hello Krzysztof, Nancy,
>>>>>>
>>>>>> Since this series has reached v29, can we please reach an
>>>>>> agreement
>>>>>> on the bindings
>>>>>> to use here, so that we can get this finally upstreamed?
>>>>>>
>>>>>> I will put some examples to try to get this issue resolved.
>>>>>>
>>>>>> ### Example 1: Constrain the number of GCE entries to *seven*
>>>>>> array
>>>>>> elements (7x4!)
>>>>>>
>>>>>>      mediatek,gce-client-reg:
>>>>>>        $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>>>        maxItems: 1
>>>>>>        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/dt-bindings/gce/<chip>-gce.h, mapping to the
>>>>>> register of display
>>>>>>          function block.
>>>>>>        items:
>>>>>>          minItems: 28
>>>>>>          maxItems: 28
>>>>>>          items:                     <----- this block doesn't
>>>>>> seem to
>>>>>> get checked :\
>>>>>>            - description: phandle of GCE
>>>>>>            - description: GCE subsys id
>>>>>>            - description: register offset
>>>>>>            - description: register size
>>>>>
>>>>> This is what we would like to have but it requires exception in
>>>>> dtschema. Thus:
>>>>>
>>>>>>
>>>>>>
>>>>>> ### Example 2: Don't care about constraining the number of
>>>>>> arguments
>>>>>>
>>>>>>      mediatek,gce-client-reg:
>>>>>>        $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>>>        maxItems: 1
>>>>>>        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/dt-bindings/gce/<chip>-gce.h, mapping to the
>>>>>> register of display
>>>>>>          function block.
>>>>>
>>>>> use this.
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>
>>>>
>>>> Hi Krzysztof, Angelo,
>>>>
>>>> Thanks for the comment.
>>>> The Example 2 can pass dt_binding_check.
>>>>
>>>> But the example in the binding has 7 items [1] and dts [2]. Does
>>>> the
>>>> "maxItems: 1" affect any other schema or dts check?
>>>
>>> Ah, then it should be maxItems: 7, not 1.
>>>
>>
>> Keep in mind for your v30:
>>
>> maxItems: 7 will pass - but only if minItems is *not* 7 :-)
>>
>> -> (so, do not declare minItems, as default is 1) <-
>>
>> Regards,
>> Angelo
>>
> Hi Angelo,
> 
> I still have one message [1] when runing dt_binding_check for "example
> 2 + maxItems: 7" [2].
> 
> [1]
> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb:
> hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7, 16384,
> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
> 45056, 4096, 4294967295, 7, 49152, 4096]] is too short
> 
> 
> [2]
>     mediatek,gce-client-reg:
>       $ref: /schemas/types.yaml#/definitions/phandle-array
>       maxItems: 7
>       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/dt-bindings/gce/<chip>-gce.h, mapping to the register of
> display
>         function block.
> 

Maybe I'm wrong about the "do not declare minItems"... try with

minItems: 1
maxItems: 7


...does it work now?

> Regards,
> Nancy
> 
> 
>>> Best regards,
>>> Krzysztof
>>>
Nancy Lin (林欣螢) March 21, 2023, 5:33 a.m. UTC | #12
Dear Angelo,

Sorry for late reply.

On Fri, 2023-03-17 at 10:58 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/03/23 10:52, Nancy Lin (林欣螢) ha scritto:
> > On Fri, 2023-03-17 at 10:37 +0100, AngeloGioacchino Del Regno
> > wrote:
> > > Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
> > > > On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
> > > > > On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
> > > > > > On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
> > > > > > 
> > > > > > > Hello Krzysztof, Nancy,
> > > > > > > 
> > > > > > > Since this series has reached v29, can we please reach an
> > > > > > > agreement
> > > > > > > on the bindings
> > > > > > > to use here, so that we can get this finally upstreamed?
> > > > > > > 
> > > > > > > I will put some examples to try to get this issue
> > > > > > > resolved.
> > > > > > > 
> > > > > > > ### Example 1: Constrain the number of GCE entries to
> > > > > > > *seven*
> > > > > > > array
> > > > > > > elements (7x4!)
> > > > > > > 
> > > > > > >      mediatek,gce-client-reg:
> > > > > > >        $ref: /schemas/types.yaml#/definitions/phandle-
> > > > > > > array
> > > > > > >        maxItems: 1
> > > > > > >        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/dt-bindings/gce/<chip>-gce.h, mapping to
> > > > > > > the
> > > > > > > register of display
> > > > > > >          function block.
> > > > > > >        items:
> > > > > > >          minItems: 28
> > > > > > >          maxItems: 28
> > > > > > >          items:                     <----- this block
> > > > > > > doesn't
> > > > > > > seem to
> > > > > > > get checked :\
> > > > > > >            - description: phandle of GCE
> > > > > > >            - description: GCE subsys id
> > > > > > >            - description: register offset
> > > > > > >            - description: register size
> > > > > > 
> > > > > > This is what we would like to have but it requires
> > > > > > exception in
> > > > > > dtschema. Thus:
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > ### Example 2: Don't care about constraining the number
> > > > > > > of
> > > > > > > arguments
> > > > > > > 
> > > > > > >      mediatek,gce-client-reg:
> > > > > > >        $ref: /schemas/types.yaml#/definitions/phandle-
> > > > > > > array
> > > > > > >        maxItems: 1
> > > > > > >        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/dt-bindings/gce/<chip>-gce.h, mapping to
> > > > > > > the
> > > > > > > register of display
> > > > > > >          function block.
> > > > > > 
> > > > > > use this.
> > > > > > 
> > > > > > Best regards,
> > > > > > Krzysztof
> > > > > 
> > > > > 
> > > > > Hi Krzysztof, Angelo,
> > > > > 
> > > > > Thanks for the comment.
> > > > > The Example 2 can pass dt_binding_check.
> > > > > 
> > > > > But the example in the binding has 7 items [1] and dts [2].
> > > > > Does
> > > > > the
> > > > > "maxItems: 1" affect any other schema or dts check?
> > > > 
> > > > Ah, then it should be maxItems: 7, not 1.
> > > > 
> > > 
> > > Keep in mind for your v30:
> > > 
> > > maxItems: 7 will pass - but only if minItems is *not* 7 :-)
> > > 
> > > -> (so, do not declare minItems, as default is 1) <-
> > > 
> > > Regards,
> > > Angelo
> > > 
> > 
> > Hi Angelo,
> > 
> > I still have one message [1] when runing dt_binding_check for
> > "example
> > 2 + maxItems: 7" [2].
> > 
> > [1]
> > /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
> > cetr
> > ee/bindings/display/mediatek/mediatek,ethdr.example.dtb:
> > hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7,
> > 16384,
> > 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
> > 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
> > 7,
> > 45056, 4096, 4294967295, 7, 49152, 4096]] is too short
> > 
> > 
> > [2]
> >     mediatek,gce-client-reg:
> >       $ref: /schemas/types.yaml#/definitions/phandle-array
> >       maxItems: 7
> >       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/dt-bindings/gce/<chip>-gce.h, mapping to the
> > register of
> > display
> >         function block.
> > 
> 
> Maybe I'm wrong about the "do not declare minItems"... try with
> 
> minItems: 1
> maxItems: 7
> 
> 
> ...does it work now?
> 

Yes, It works well with "example2 + minItems:1 + maxItems: 7" [1]

[1]
  mediatek,gce-client-reg:
    $ref: /schemas/types.yaml#/definitions/phandle-array
    minItems: 1
    maxItems: 7
    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/dt-bindings/gce/<chip>-gce.h, mapping to the register of
display
      function block.

Regards,
Nancy


> > Regards,
> > Nancy
> > 
> > 
> > > > Best regards,
> > > > Krzysztof
> > > > 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
AngeloGioacchino Del Regno March 21, 2023, 9:54 a.m. UTC | #13
Il 21/03/23 06:33, Nancy Lin (林欣螢) ha scritto:
> Dear Angelo,
> 
> Sorry for late reply.
> 
> On Fri, 2023-03-17 at 10:58 +0100, AngeloGioacchino Del Regno wrote:
>> Il 17/03/23 10:52, Nancy Lin (林欣螢) ha scritto:
>>> On Fri, 2023-03-17 at 10:37 +0100, AngeloGioacchino Del Regno
>>> wrote:
>>>> Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
>>>>> On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
>>>>>> On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
>>>>>>> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
>>>>>>>
>>>>>>>> Hello Krzysztof, Nancy,
>>>>>>>>
>>>>>>>> Since this series has reached v29, can we please reach an
>>>>>>>> agreement
>>>>>>>> on the bindings
>>>>>>>> to use here, so that we can get this finally upstreamed?
>>>>>>>>
>>>>>>>> I will put some examples to try to get this issue
>>>>>>>> resolved.
>>>>>>>>
>>>>>>>> ### Example 1: Constrain the number of GCE entries to
>>>>>>>> *seven*
>>>>>>>> array
>>>>>>>> elements (7x4!)
>>>>>>>>
>>>>>>>>       mediatek,gce-client-reg:
>>>>>>>>         $ref: /schemas/types.yaml#/definitions/phandle-
>>>>>>>> array
>>>>>>>>         maxItems: 1
>>>>>>>>         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/dt-bindings/gce/<chip>-gce.h, mapping to
>>>>>>>> the
>>>>>>>> register of display
>>>>>>>>           function block.
>>>>>>>>         items:
>>>>>>>>           minItems: 28
>>>>>>>>           maxItems: 28
>>>>>>>>           items:                     <----- this block
>>>>>>>> doesn't
>>>>>>>> seem to
>>>>>>>> get checked :\
>>>>>>>>             - description: phandle of GCE
>>>>>>>>             - description: GCE subsys id
>>>>>>>>             - description: register offset
>>>>>>>>             - description: register size
>>>>>>>
>>>>>>> This is what we would like to have but it requires
>>>>>>> exception in
>>>>>>> dtschema. Thus:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ### Example 2: Don't care about constraining the number
>>>>>>>> of
>>>>>>>> arguments
>>>>>>>>
>>>>>>>>       mediatek,gce-client-reg:
>>>>>>>>         $ref: /schemas/types.yaml#/definitions/phandle-
>>>>>>>> array
>>>>>>>>         maxItems: 1
>>>>>>>>         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/dt-bindings/gce/<chip>-gce.h, mapping to
>>>>>>>> the
>>>>>>>> register of display
>>>>>>>>           function block.
>>>>>>>
>>>>>>> use this.
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Krzysztof
>>>>>>
>>>>>>
>>>>>> Hi Krzysztof, Angelo,
>>>>>>
>>>>>> Thanks for the comment.
>>>>>> The Example 2 can pass dt_binding_check.
>>>>>>
>>>>>> But the example in the binding has 7 items [1] and dts [2].
>>>>>> Does
>>>>>> the
>>>>>> "maxItems: 1" affect any other schema or dts check?
>>>>>
>>>>> Ah, then it should be maxItems: 7, not 1.
>>>>>
>>>>
>>>> Keep in mind for your v30:
>>>>
>>>> maxItems: 7 will pass - but only if minItems is *not* 7 :-)
>>>>
>>>> -> (so, do not declare minItems, as default is 1) <-
>>>>
>>>> Regards,
>>>> Angelo
>>>>
>>>
>>> Hi Angelo,
>>>
>>> I still have one message [1] when runing dt_binding_check for
>>> "example
>>> 2 + maxItems: 7" [2].
>>>
>>> [1]
>>> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
>>> cetr
>>> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb:
>>> hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7,
>>> 16384,
>>> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
>>> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
>>> 7,
>>> 45056, 4096, 4294967295, 7, 49152, 4096]] is too short
>>>
>>>
>>> [2]
>>>      mediatek,gce-client-reg:
>>>        $ref: /schemas/types.yaml#/definitions/phandle-array
>>>        maxItems: 7
>>>        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/dt-bindings/gce/<chip>-gce.h, mapping to the
>>> register of
>>> display
>>>          function block.
>>>
>>
>> Maybe I'm wrong about the "do not declare minItems"... try with
>>
>> minItems: 1
>> maxItems: 7
>>
>>
>> ...does it work now?
>>
> 
> Yes, It works well with "example2 + minItems:1 + maxItems: 7" [1]
> 
> [1]
>    mediatek,gce-client-reg:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      minItems: 1
>      maxItems: 7
>      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/dt-bindings/gce/<chip>-gce.h, mapping to the register of
> display
>        function block.
> 

Please send a v30 with that solution ASAP then, so that we may perhaps *finally*
get it in for v6.4.

Regards,
Angelo
diff mbox series

Patch

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..3b11e47a8834
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml
@@ -0,0 +1,188 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/mediatek/mediatek,ethdr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Ethdr Device
+
+maintainers:
+  - Chun-Kuang Hu <chunkuang.hu@kernel.org>
+  - Philipp Zabel <p.zabel@pengutronix.de>
+
+description:
+  ETHDR (ET High Dynamic Range) is a MediaTek internal HDR engine and 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.
+
+properties:
+  compatible:
+    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:
+    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/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>;
+
+        hdr-engine@1c114000 {
+                compatible = "mediatek,mt8195-disp-ethdr";
+                reg = <0 0x1c114000 0 0x1000>,
+                      <0 0x1c115000 0 0x1000>,
+                      <0 0x1c117000 0 0x1000>,
+                      <0 0x1c119000 0 0x1000>,
+                      <0 0x1c11a000 0 0x1000>,
+                      <0 0x1c11b000 0 0x1000>,
+                      <0 0x1c11c000 0 0x1000>;
+                reg-names = "mixer", "vdo_fe0", "vdo_fe1", "gfx_fe0", "gfx_fe1",
+                            "vdo_be", "adl_ds";
+                mediatek,gce-client-reg = <&gce0 SUBSYS_1c11XXXX 0x4000 0x1000>,
+                                          <&gce0 SUBSYS_1c11XXXX 0x5000 0x1000>,
+                                          <&gce0 SUBSYS_1c11XXXX 0x7000 0x1000>,
+                                          <&gce0 SUBSYS_1c11XXXX 0x9000 0x1000>,
+                                          <&gce0 SUBSYS_1c11XXXX 0xa000 0x1000>,
+                                          <&gce0 SUBSYS_1c11XXXX 0xb000 0x1000>,
+                                          <&gce0 SUBSYS_1c11XXXX 0xc000 0x1000>;
+                clocks = <&vdosys1 CLK_VDO1_DISP_MIXER>,
+                         <&vdosys1 CLK_VDO1_HDR_VDO_FE0>,
+                         <&vdosys1 CLK_VDO1_HDR_VDO_FE1>,
+                         <&vdosys1 CLK_VDO1_HDR_GFX_FE0>,
+                         <&vdosys1 CLK_VDO1_HDR_GFX_FE1>,
+                         <&vdosys1 CLK_VDO1_HDR_VDO_BE>,
+                         <&vdosys1 CLK_VDO1_26M_SLOW>,
+                         <&vdosys1 CLK_VDO1_HDR_VDO_FE0_DL_ASYNC>,
+                         <&vdosys1 CLK_VDO1_HDR_VDO_FE1_DL_ASYNC>,
+                         <&vdosys1 CLK_VDO1_HDR_GFX_FE0_DL_ASYNC>,
+                         <&vdosys1 CLK_VDO1_HDR_GFX_FE1_DL_ASYNC>,
+                         <&vdosys1 CLK_VDO1_HDR_VDO_BE_DL_ASYNC>,
+                         <&topckgen CLK_TOP_ETHDR>;
+                clock-names = "mixer", "vdo_fe0", "vdo_fe1", "gfx_fe0", "gfx_fe1",
+                              "vdo_be", "adl_ds", "vdo_fe0_async", "vdo_fe1_async",
+                              "gfx_fe0_async", "gfx_fe1_async","vdo_be_async",
+                              "ethdr_top";
+                power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>;
+                iommus = <&iommu_vpp M4U_PORT_L3_HDR_DS>,
+                         <&iommu_vpp M4U_PORT_L3_HDR_ADL>;
+                interrupts = <GIC_SPI 517 IRQ_TYPE_LEVEL_HIGH 0>; /* disp mixer */
+                resets = <&vdosys1 MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE0_DL_ASYNC>,
+                         <&vdosys1 MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE1_DL_ASYNC>,
+                         <&vdosys1 MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE0_DL_ASYNC>,
+                         <&vdosys1 MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE1_DL_ASYNC>,
+                         <&vdosys1 MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_BE_DL_ASYNC>;
+                reset-names = "vdo_fe0_async", "vdo_fe1_async", "gfx_fe0_async",
+                              "gfx_fe1_async", "vdo_be_async";
+        };
+    };
+...