2022-11-04 14:21:38

by Guillaume Ranquet

[permalink] [raw]
Subject: [PATCH v3 02/12] dt-bindings: display: mediatek: add MT8195 hdmi bindings

Add mt8195 SoC bindings for hdmi and hdmi-ddc

On mt8195 the ddc i2c controller is part of the hdmi IP block and thus has no
specific register range, power domain or interrupt, making it simpler
than its the legacy "mediatek,hdmi-ddc" binding.

Signed-off-by: Guillaume Ranquet <[email protected]>
---
.../bindings/display/mediatek/mediatek,hdmi.yaml | 61 ++++++++++++++++++----
.../display/mediatek/mediatek,mt8195-hdmi-ddc.yaml | 51 ++++++++++++++++++
2 files changed, 101 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
index bdaf0b51e68c..9710b7b6e9bf 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
@@ -21,6 +21,7 @@ properties:
- mediatek,mt7623-hdmi
- mediatek,mt8167-hdmi
- mediatek,mt8173-hdmi
+ - mediatek,mt8195-hdmi

reg:
maxItems: 1
@@ -29,18 +30,12 @@ properties:
maxItems: 1

clocks:
- items:
- - description: Pixel Clock
- - description: HDMI PLL
- - description: Bit Clock
- - description: S/PDIF Clock
+ minItems: 4
+ maxItems: 4

clock-names:
- items:
- - const: pixel
- - const: pll
- - const: bclk
- - const: spdif
+ minItems: 4
+ maxItems: 4

phys:
maxItems: 1
@@ -58,6 +53,9 @@ properties:
description: |
phandle link and register offset to the system configuration registers.

+ power-domains:
+ maxItems: 1
+
ports:
$ref: /schemas/graph.yaml#/properties/ports

@@ -86,9 +84,50 @@ required:
- clock-names
- phys
- phy-names
- - mediatek,syscon-hdmi
- ports

+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mediatek,mt8195-hdmi
+ then:
+ properties:
+ clocks:
+ items:
+ - description: APB
+ - description: HDCP
+ - description: HDCP 24M
+ - description: Split HDMI
+ clock-names:
+ items:
+ - const: hdmi_apb_sel
+ - const: hdcp_sel
+ - const: hdcp24_sel
+ - const: split_hdmi
+
+ required:
+ - power-domains
+ else:
+ properties:
+ clocks:
+ items:
+ - description: Pixel Clock
+ - description: HDMI PLL
+ - description: Bit Clock
+ - description: S/PDIF Clock
+
+ clock-names:
+ items:
+ - const: pixel
+ - const: pll
+ - const: bclk
+ - const: spdif
+
+ required:
+ - mediatek,syscon-hdmi
+
additionalProperties: false

examples:
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml
new file mode 100644
index 000000000000..2dc273689584
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek HDMI DDC for mt8195
+
+maintainers:
+ - CK Hu <[email protected]>
+ - Jitao shi <[email protected]>
+
+description: |
+ The HDMI DDC i2c controller is used to interface with the HDMI DDC pins.
+
+properties:
+ compatible:
+ enum:
+ - mediatek,mt8195-hdmi-ddc
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: ddc
+
+ mediatek,hdmi:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ A phandle to the mt8195 hdmi controller
+
+required:
+ - compatible
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ hdmiddc0: i2c {
+ compatible = "mediatek,mt8195-hdmi-ddc";
+ mediatek,hdmi = <&hdmi0>;
+ clocks = <&clk26m>;
+ clock-names = "ddc";
+ };
+
+...

--
b4 0.11.0-dev


2022-11-07 10:24:34

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 02/12] dt-bindings: display: mediatek: add MT8195 hdmi bindings

On 04/11/2022 15:09, Guillaume Ranquet wrote:
> Add mt8195 SoC bindings for hdmi and hdmi-ddc
>
> On mt8195 the ddc i2c controller is part of the hdmi IP block and thus has no
> specific register range, power domain or interrupt, making it simpler
> than its the legacy "mediatek,hdmi-ddc" binding.
>
> Signed-off-by: Guillaume Ranquet <[email protected]>
> ---
> .../bindings/display/mediatek/mediatek,hdmi.yaml | 61 ++++++++++++++++++----
> .../display/mediatek/mediatek,mt8195-hdmi-ddc.yaml | 51 ++++++++++++++++++
> 2 files changed, 101 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
> index bdaf0b51e68c..9710b7b6e9bf 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
> @@ -21,6 +21,7 @@ properties:
> - mediatek,mt7623-hdmi
> - mediatek,mt8167-hdmi
> - mediatek,mt8173-hdmi
> + - mediatek,mt8195-hdmi
>
> reg:
> maxItems: 1
> @@ -29,18 +30,12 @@ properties:
> maxItems: 1
>
> clocks:
> - items:
> - - description: Pixel Clock
> - - description: HDMI PLL
> - - description: Bit Clock
> - - description: S/PDIF Clock
> + minItems: 4

Drop minItems, it's not needed when equal to maxItems.

> + maxItems: 4
>
> clock-names:
> - items:
> - - const: pixel
> - - const: pll
> - - const: bclk
> - - const: spdif
> + minItems: 4

Drop minItems, it's not needed when equal to maxItems.


> + maxItems: 4
>
> phys:
> maxItems: 1
> @@ -58,6 +53,9 @@ properties:
> description: |
> phandle link and register offset to the system configuration registers.
>
> + power-domains:
> + maxItems: 1
> +
> ports:
> $ref: /schemas/graph.yaml#/properties/ports
>
> @@ -86,9 +84,50 @@ required:
> - clock-names
> - phys
> - phy-names
> - - mediatek,syscon-hdmi
> - ports
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mediatek,mt8195-hdmi
> + then:
> + properties:
> + clocks:
> + items:
> + - description: APB
> + - description: HDCP
> + - description: HDCP 24M
> + - description: Split HDMI
> + clock-names:
> + items:
> + - const: hdmi_apb_sel
> + - const: hdcp_sel
> + - const: hdcp24_sel
> + - const: split_hdmi
> +
> + required:
> + - power-domains
> + else:
> + properties:
> + clocks:
> + items:
> + - description: Pixel Clock
> + - description: HDMI PLL
> + - description: Bit Clock
> + - description: S/PDIF Clock
> +
> + clock-names:
> + items:
> + - const: pixel
> + - const: pll
> + - const: bclk
> + - const: spdif
> +
> + required:
> + - mediatek,syscon-hdmi
> +
> additionalProperties: false
>
> examples:
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml
> new file mode 100644
> index 000000000000..2dc273689584
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mediatek HDMI DDC for mt8195
> +
> +maintainers:
> + - CK Hu <[email protected]>
> + - Jitao shi <[email protected]>
> +
> +description: |
> + The HDMI DDC i2c controller is used to interface with the HDMI DDC pins.
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt8195-hdmi-ddc
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: ddc

Unless you expect it to grow, I propose to drop clock-names. It's not
useful when the name is the same as name of hardware.

Best regards,
Krzysztof


2022-11-07 15:58:03

by Guillaume Ranquet

[permalink] [raw]
Subject: Re: [PATCH v3 02/12] dt-bindings: display: mediatek: add MT8195 hdmi bindings

On Mon, 07 Nov 2022 11:02, Krzysztof Kozlowski
<[email protected]> wrote:
>On 04/11/2022 15:09, Guillaume Ranquet wrote:
>> Add mt8195 SoC bindings for hdmi and hdmi-ddc
>>
>> On mt8195 the ddc i2c controller is part of the hdmi IP block and thus has no
>> specific register range, power domain or interrupt, making it simpler
>> than its the legacy "mediatek,hdmi-ddc" binding.
>>
>> Signed-off-by: Guillaume Ranquet <[email protected]>
>> ---
>> .../bindings/display/mediatek/mediatek,hdmi.yaml | 61 ++++++++++++++++++----
>> .../display/mediatek/mediatek,mt8195-hdmi-ddc.yaml | 51 ++++++++++++++++++
>> 2 files changed, 101 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
>> index bdaf0b51e68c..9710b7b6e9bf 100644
>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
>> @@ -21,6 +21,7 @@ properties:
>> - mediatek,mt7623-hdmi
>> - mediatek,mt8167-hdmi
>> - mediatek,mt8173-hdmi
>> + - mediatek,mt8195-hdmi
>>
>> reg:
>> maxItems: 1
>> @@ -29,18 +30,12 @@ properties:
>> maxItems: 1
>>
>> clocks:
>> - items:
>> - - description: Pixel Clock
>> - - description: HDMI PLL
>> - - description: Bit Clock
>> - - description: S/PDIF Clock
>> + minItems: 4
>
>Drop minItems, it's not needed when equal to maxItems.
>
>> + maxItems: 4
>>
>> clock-names:
>> - items:
>> - - const: pixel
>> - - const: pll
>> - - const: bclk
>> - - const: spdif
>> + minItems: 4
>
>Drop minItems, it's not needed when equal to maxItems.
>
>
>> + maxItems: 4
>>
>> phys:
>> maxItems: 1
>> @@ -58,6 +53,9 @@ properties:
>> description: |
>> phandle link and register offset to the system configuration registers.
>>
>> + power-domains:
>> + maxItems: 1
>> +
>> ports:
>> $ref: /schemas/graph.yaml#/properties/ports
>>
>> @@ -86,9 +84,50 @@ required:
>> - clock-names
>> - phys
>> - phy-names
>> - - mediatek,syscon-hdmi
>> - ports
>>
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: mediatek,mt8195-hdmi
>> + then:
>> + properties:
>> + clocks:
>> + items:
>> + - description: APB
>> + - description: HDCP
>> + - description: HDCP 24M
>> + - description: Split HDMI
>> + clock-names:
>> + items:
>> + - const: hdmi_apb_sel
>> + - const: hdcp_sel
>> + - const: hdcp24_sel
>> + - const: split_hdmi
>> +
>> + required:
>> + - power-domains
>> + else:
>> + properties:
>> + clocks:
>> + items:
>> + - description: Pixel Clock
>> + - description: HDMI PLL
>> + - description: Bit Clock
>> + - description: S/PDIF Clock
>> +
>> + clock-names:
>> + items:
>> + - const: pixel
>> + - const: pll
>> + - const: bclk
>> + - const: spdif
>> +
>> + required:
>> + - mediatek,syscon-hdmi
>> +
>> additionalProperties: false
>>
>> examples:
>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml
>> new file mode 100644
>> index 000000000000..2dc273689584
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Mediatek HDMI DDC for mt8195
>> +
>> +maintainers:
>> + - CK Hu <[email protected]>
>> + - Jitao shi <[email protected]>
>> +
>> +description: |
>> + The HDMI DDC i2c controller is used to interface with the HDMI DDC pins.
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - mediatek,mt8195-hdmi-ddc
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + clock-names:
>> + items:
>> + - const: ddc
>
>Unless you expect it to grow, I propose to drop clock-names. It's not
>useful when the name is the same as name of hardware.
>
>Best regards,
>Krzysztof
>

Thx for your suggestions, I'll apply them in v4.

Thx,
Guillaume.

2022-12-26 05:26:20

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [PATCH v3 02/12] dt-bindings: display: mediatek: add MT8195 hdmi bindings

Hi, Guillaume:

On Fri, 2022-11-04 at 15:09 +0100, Guillaume Ranquet wrote:
> Add mt8195 SoC bindings for hdmi and hdmi-ddc
>
> On mt8195 the ddc i2c controller is part of the hdmi IP block and
> thus has no
> specific register range, power domain or interrupt, making it simpler
> than its the legacy "mediatek,hdmi-ddc" binding.
>
> Signed-off-by: Guillaume Ranquet <[email protected]>
> ---
>

[snip]

> a/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
> hdmi-ddc.yaml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
> hdmi-ddc.yaml
> new file mode 100644
> index 000000000000..2dc273689584
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
> hdmi-ddc.yaml
> @@ -0,0 +1,51 @@
> +# 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,mt8195-hdmi-ddc.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WfpOlq57w$
>
> +$schema:
> https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WdSGOSxzw$
>
> +
> +title: Mediatek HDMI DDC for mt8195
> +
> +maintainers:
> + - CK Hu <[email protected]>
> + - Jitao shi <[email protected]>
> +
> +description: |
> + The HDMI DDC i2c controller is used to interface with the HDMI DDC
> pins.
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt8195-hdmi-ddc
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: ddc
> +
> + mediatek,hdmi:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + A phandle to the mt8195 hdmi controller
> +
> +required:
> + - compatible
> + - clocks
> + - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + hdmiddc0: i2c {
> + compatible = "mediatek,mt8195-hdmi-ddc";
> + mediatek,hdmi = <&hdmi0>;
> + clocks = <&clk26m>;
> + clock-names = "ddc";
> + };

I think we should not have a virtual device. This ddc is part of
mt8195-hdmi device, so just keep mt8195-hdmi, and let mt8195-hdmi
driver to probe the sub driver of ddc driver.

Regards,
CK

> +
> +...
>

2023-01-02 13:41:49

by Guillaume Ranquet

[permalink] [raw]
Subject: Re: [PATCH v3 02/12] dt-bindings: display: mediatek: add MT8195 hdmi bindings

On Mon, 26 Dec 2022 06:18, CK Hu (胡俊光) <[email protected]> wrote:
>Hi, Guillaume:
>
>On Fri, 2022-11-04 at 15:09 +0100, Guillaume Ranquet wrote:
>> Add mt8195 SoC bindings for hdmi and hdmi-ddc
>>
>> On mt8195 the ddc i2c controller is part of the hdmi IP block and
>> thus has no
>> specific register range, power domain or interrupt, making it simpler
>> than its the legacy "mediatek,hdmi-ddc" binding.
>>
>> Signed-off-by: Guillaume Ranquet <[email protected]>
>> ---
>>
>
>[snip]
>
>> a/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>> hdmi-ddc.yaml
>> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>> hdmi-ddc.yaml
>> new file mode 100644
>> index 000000000000..2dc273689584
>> --- /dev/null
>> +++
>> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>> hdmi-ddc.yaml
>> @@ -0,0 +1,51 @@
>> +# 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,mt8195-hdmi-ddc.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WfpOlq57w$
>>
>> +$schema:
>> https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WdSGOSxzw$
>>
>> +
>> +title: Mediatek HDMI DDC for mt8195
>> +
>> +maintainers:
>> + - CK Hu <[email protected]>
>> + - Jitao shi <[email protected]>
>> +
>> +description: |
>> + The HDMI DDC i2c controller is used to interface with the HDMI DDC
>> pins.
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - mediatek,mt8195-hdmi-ddc
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + clock-names:
>> + items:
>> + - const: ddc
>> +
>> + mediatek,hdmi:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + A phandle to the mt8195 hdmi controller
>> +
>> +required:
>> + - compatible
>> + - clocks
>> + - clock-names
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> + hdmiddc0: i2c {
>> + compatible = "mediatek,mt8195-hdmi-ddc";
>> + mediatek,hdmi = <&hdmi0>;
>> + clocks = <&clk26m>;
>> + clock-names = "ddc";
>> + };
>
>I think we should not have a virtual device. This ddc is part of
>mt8195-hdmi device, so just keep mt8195-hdmi, and let mt8195-hdmi
>driver to probe the sub driver of ddc driver.
>
>Regards,
>CK

Hi CK,

Thx for your input.
Though I would strongly prefer to keep the ddc as a separate "virtual device".

It aligns better with the goal of reusing as much code as possible
from the HDMI V1 IP,
which is something you have been advocating since V1 of this patch
quite some time ago
and has shaped this patch.

To me we are in a state that is clean and avoids branching in the hdmi
common code.
Would you reconsider and allow the use of that virtual device?

Thx,
Guillaume.

>
>> +
>> +...
>>

Subject: Re: [PATCH v3 02/12] dt-bindings: display: mediatek: add MT8195 hdmi bindings

Il 02/01/23 14:38, Guillaume Ranquet ha scritto:
> On Mon, 26 Dec 2022 06:18, CK Hu (胡俊光) <[email protected]> wrote:
>> Hi, Guillaume:
>>
>> On Fri, 2022-11-04 at 15:09 +0100, Guillaume Ranquet wrote:
>>> Add mt8195 SoC bindings for hdmi and hdmi-ddc
>>>
>>> On mt8195 the ddc i2c controller is part of the hdmi IP block and
>>> thus has no
>>> specific register range, power domain or interrupt, making it simpler
>>> than its the legacy "mediatek,hdmi-ddc" binding.
>>>
>>> Signed-off-by: Guillaume Ranquet <[email protected]>
>>> ---
>>>
>>
>> [snip]
>>
>>> a/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>>> hdmi-ddc.yaml
>>> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>>> hdmi-ddc.yaml
>>> new file mode 100644
>>> index 000000000000..2dc273689584
>>> --- /dev/null
>>> +++
>>> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>>> hdmi-ddc.yaml
>>> @@ -0,0 +1,51 @@
>>> +# 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,mt8195-hdmi-ddc.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WfpOlq57w$
>>>
>>> +$schema:
>>> https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WdSGOSxzw$
>>>
>>> +
>>> +title: Mediatek HDMI DDC for mt8195
>>> +
>>> +maintainers:
>>> + - CK Hu <[email protected]>
>>> + - Jitao shi <[email protected]>
>>> +
>>> +description: |
>>> + The HDMI DDC i2c controller is used to interface with the HDMI DDC
>>> pins.
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - mediatek,mt8195-hdmi-ddc
>>> +
>>> + clocks:
>>> + maxItems: 1
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: ddc
>>> +
>>> + mediatek,hdmi:
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>> + description:
>>> + A phandle to the mt8195 hdmi controller
>>> +
>>> +required:
>>> + - compatible
>>> + - clocks
>>> + - clock-names
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>> + hdmiddc0: i2c {
>>> + compatible = "mediatek,mt8195-hdmi-ddc";
>>> + mediatek,hdmi = <&hdmi0>;
>>> + clocks = <&clk26m>;
>>> + clock-names = "ddc";
>>> + };
>>
>> I think we should not have a virtual device. This ddc is part of
>> mt8195-hdmi device, so just keep mt8195-hdmi, and let mt8195-hdmi
>> driver to probe the sub driver of ddc driver.
>>
>> Regards,
>> CK
>
> Hi CK,
>
> Thx for your input.
> Though I would strongly prefer to keep the ddc as a separate "virtual device".
>
> It aligns better with the goal of reusing as much code as possible
> from the HDMI V1 IP,
> which is something you have been advocating since V1 of this patch
> quite some time ago
> and has shaped this patch.
>
> To me we are in a state that is clean and avoids branching in the hdmi
> common code.
> Would you reconsider and allow the use of that virtual device?
>
> Thx,
> Guillaume.
>

You can as well keep the DDC as a separated driver, but register in the HDMI v1 and
v2 driver at probe time.

Doing that, you won't need any devicetree node specific to any virtual device :-)

Cheers,
Angelo


2023-01-02 16:08:38

by Guillaume Ranquet

[permalink] [raw]
Subject: Re: [PATCH v3 02/12] dt-bindings: display: mediatek: add MT8195 hdmi bindings

On Mon, 02 Jan 2023 15:14, AngeloGioacchino Del Regno
<[email protected]> wrote:
>Il 02/01/23 14:38, Guillaume Ranquet ha scritto:
>> On Mon, 26 Dec 2022 06:18, CK Hu (胡俊光) <[email protected]> wrote:
>>> Hi, Guillaume:
>>>
>>> On Fri, 2022-11-04 at 15:09 +0100, Guillaume Ranquet wrote:
>>>> Add mt8195 SoC bindings for hdmi and hdmi-ddc
>>>>
>>>> On mt8195 the ddc i2c controller is part of the hdmi IP block and
>>>> thus has no
>>>> specific register range, power domain or interrupt, making it simpler
>>>> than its the legacy "mediatek,hdmi-ddc" binding.
>>>>
>>>> Signed-off-by: Guillaume Ranquet <[email protected]>
>>>> ---
>>>>
>>>
>>> [snip]
>>>
>>>> a/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>>>> hdmi-ddc.yaml
>>>> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>>>> hdmi-ddc.yaml
>>>> new file mode 100644
>>>> index 000000000000..2dc273689584
>>>> --- /dev/null
>>>> +++
>>>> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>>>> hdmi-ddc.yaml
>>>> @@ -0,0 +1,51 @@
>>>> +# 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,mt8195-hdmi-ddc.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WfpOlq57w$
>>>>
>>>> +$schema:
>>>> https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WdSGOSxzw$
>>>>
>>>> +
>>>> +title: Mediatek HDMI DDC for mt8195
>>>> +
>>>> +maintainers:
>>>> + - CK Hu <[email protected]>
>>>> + - Jitao shi <[email protected]>
>>>> +
>>>> +description: |
>>>> + The HDMI DDC i2c controller is used to interface with the HDMI DDC
>>>> pins.
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + enum:
>>>> + - mediatek,mt8195-hdmi-ddc
>>>> +
>>>> + clocks:
>>>> + maxItems: 1
>>>> +
>>>> + clock-names:
>>>> + items:
>>>> + - const: ddc
>>>> +
>>>> + mediatek,hdmi:
>>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>>> + description:
>>>> + A phandle to the mt8195 hdmi controller
>>>> +
>>>> +required:
>>>> + - compatible
>>>> + - clocks
>>>> + - clock-names
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> + - |
>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>>> + hdmiddc0: i2c {
>>>> + compatible = "mediatek,mt8195-hdmi-ddc";
>>>> + mediatek,hdmi = <&hdmi0>;
>>>> + clocks = <&clk26m>;
>>>> + clock-names = "ddc";
>>>> + };
>>>
>>> I think we should not have a virtual device. This ddc is part of
>>> mt8195-hdmi device, so just keep mt8195-hdmi, and let mt8195-hdmi
>>> driver to probe the sub driver of ddc driver.
>>>
>>> Regards,
>>> CK
>>
>> Hi CK,
>>
>> Thx for your input.
>> Though I would strongly prefer to keep the ddc as a separate "virtual device".
>>
>> It aligns better with the goal of reusing as much code as possible
>> from the HDMI V1 IP,
>> which is something you have been advocating since V1 of this patch
>> quite some time ago
>> and has shaped this patch.
>>
>> To me we are in a state that is clean and avoids branching in the hdmi
>> common code.
>> Would you reconsider and allow the use of that virtual device?
>>
>> Thx,
>> Guillaume.
>>
>
>You can as well keep the DDC as a separated driver, but register in the HDMI v1 and
>v2 driver at probe time.
>
>Doing that, you won't need any devicetree node specific to any virtual device :-)
>
>Cheers,
>Angelo
>
>

Sure, but does it make any sense for HDMI v1?
As the ddc in v1 is a real device which (in theory) can be reused by other IPs.

I would see either v1 and v2 ddc exposed as a devicetree node (which
is what I favor).
Or v1 as a devicetree node and v2 probed directly from the hdmi code base.

Thx,
Guillaume.

Subject: Re: [PATCH v3 02/12] dt-bindings: display: mediatek: add MT8195 hdmi bindings

Il 02/01/23 16:19, Guillaume Ranquet ha scritto:
> On Mon, 02 Jan 2023 15:14, AngeloGioacchino Del Regno
> <[email protected]> wrote:
>> Il 02/01/23 14:38, Guillaume Ranquet ha scritto:
>>> On Mon, 26 Dec 2022 06:18, CK Hu (胡俊光) <[email protected]> wrote:
>>>> Hi, Guillaume:
>>>>
>>>> On Fri, 2022-11-04 at 15:09 +0100, Guillaume Ranquet wrote:
>>>>> Add mt8195 SoC bindings for hdmi and hdmi-ddc
>>>>>
>>>>> On mt8195 the ddc i2c controller is part of the hdmi IP block and
>>>>> thus has no
>>>>> specific register range, power domain or interrupt, making it simpler
>>>>> than its the legacy "mediatek,hdmi-ddc" binding.
>>>>>
>>>>> Signed-off-by: Guillaume Ranquet <[email protected]>
>>>>> ---
>>>>>
>>>>
>>>> [snip]
>>>>
>>>>> a/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>>>>> hdmi-ddc.yaml
>>>>> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>>>>> hdmi-ddc.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..2dc273689584
>>>>> --- /dev/null
>>>>> +++
>>>>> b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-
>>>>> hdmi-ddc.yaml
>>>>> @@ -0,0 +1,51 @@
>>>>> +# 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,mt8195-hdmi-ddc.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WfpOlq57w$
>>>>>
>>>>> +$schema:
>>>>> https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WdSGOSxzw$
>>>>>
>>>>> +
>>>>> +title: Mediatek HDMI DDC for mt8195
>>>>> +
>>>>> +maintainers:
>>>>> + - CK Hu <[email protected]>
>>>>> + - Jitao shi <[email protected]>
>>>>> +
>>>>> +description: |
>>>>> + The HDMI DDC i2c controller is used to interface with the HDMI DDC
>>>>> pins.
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + enum:
>>>>> + - mediatek,mt8195-hdmi-ddc
>>>>> +
>>>>> + clocks:
>>>>> + maxItems: 1
>>>>> +
>>>>> + clock-names:
>>>>> + items:
>>>>> + - const: ddc
>>>>> +
>>>>> + mediatek,hdmi:
>>>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>>>> + description:
>>>>> + A phandle to the mt8195 hdmi controller
>>>>> +
>>>>> +required:
>>>>> + - compatible
>>>>> + - clocks
>>>>> + - clock-names
>>>>> +
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> + - |
>>>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>>>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>>>> + hdmiddc0: i2c {
>>>>> + compatible = "mediatek,mt8195-hdmi-ddc";
>>>>> + mediatek,hdmi = <&hdmi0>;
>>>>> + clocks = <&clk26m>;
>>>>> + clock-names = "ddc";
>>>>> + };
>>>>
>>>> I think we should not have a virtual device. This ddc is part of
>>>> mt8195-hdmi device, so just keep mt8195-hdmi, and let mt8195-hdmi
>>>> driver to probe the sub driver of ddc driver.
>>>>
>>>> Regards,
>>>> CK
>>>
>>> Hi CK,
>>>
>>> Thx for your input.
>>> Though I would strongly prefer to keep the ddc as a separate "virtual device".
>>>
>>> It aligns better with the goal of reusing as much code as possible
>>> from the HDMI V1 IP,
>>> which is something you have been advocating since V1 of this patch
>>> quite some time ago
>>> and has shaped this patch.
>>>
>>> To me we are in a state that is clean and avoids branching in the hdmi
>>> common code.
>>> Would you reconsider and allow the use of that virtual device?
>>>
>>> Thx,
>>> Guillaume.
>>>
>>
>> You can as well keep the DDC as a separated driver, but register in the HDMI v1 and
>> v2 driver at probe time.
>>
>> Doing that, you won't need any devicetree node specific to any virtual device :-)
>>
>> Cheers,
>> Angelo
>>
>>
>
> Sure, but does it make any sense for HDMI v1?
> As the ddc in v1 is a real device which (in theory) can be reused by other IPs.
>
> I would see either v1 and v2 ddc exposed as a devicetree node (which
> is what I favor).
> Or v1 as a devicetree node and v2 probed directly from the hdmi code base.
>

The last option looks sensible, since v1 has an external ddc, but on v2 it's
integrated.

v1 -> dt probe
v2 -> driver probe

> Thx,
> Guillaume.