diff mbox series

[v5,1/4] dt-bindings: display: bridge: Add schema for Synopsys DW HDMI QP TX IP

Message ID 20240831-b4-rk3588-bridge-upstream-v5-1-9503bece0136@collabora.com (mailing list archive)
State New, archived
Headers show
Series Add initial support for the Rockchip RK3588 HDMI TX Controller | expand

Commit Message

Cristian Ciocaltea Aug. 30, 2024, 9:55 p.m. UTC
Add dt-binding schema containing the common properties for the Synopsys
DesignWare HDMI QP TX controller.

Note this is not a full dt-binding specification, but is meant to be
referenced by platform-specific bindings for this IP core.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 .../display/bridge/synopsys,dw-hdmi-qp.yaml        | 88 ++++++++++++++++++++++
 1 file changed, 88 insertions(+)

Comments

Krzysztof Kozlowski Aug. 31, 2024, 6:16 a.m. UTC | #1
On Sat, Aug 31, 2024 at 12:55:29AM +0300, Cristian Ciocaltea wrote:
> Add dt-binding schema containing the common properties for the Synopsys
> DesignWare HDMI QP TX controller.
> 
> Note this is not a full dt-binding specification, but is meant to be
> referenced by platform-specific bindings for this IP core.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  .../display/bridge/synopsys,dw-hdmi-qp.yaml        | 88 ++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml b/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml
> new file mode 100644
> index 000000000000..771f7fba6c50
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/synopsys,dw-hdmi-qp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common Properties for Synopsys DesignWare HDMI QP TX Controller IP
> +
> +maintainers:
> +  - Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> +
> +description: |
> +  The Synopsys DesignWare HDMI 2.1 Quad-Pixel (QP) TX Controller IP core
> +  supports the following features, among others:
> +
> +  * Fixed Rate Link (FRL)
> +  * Display Stream Compression (DSC)
> +  * 4K@120Hz and 8K@60Hz video modes
> +  * Variable Refresh Rate (VRR) including Quick Media Switching (QMS)
> +  * Fast Vactive (FVA)
> +  * SCDC I2C DDC access
> +  * Multi-stream audio
> +  * Enhanced Audio Return Channel (EARC)
> +
> +  Note this is not a full dt-binding specification, but is meant to be
> +  referenced by platform-specific bindings for this IP core.
> +
> +properties:
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 4
> +    maxItems: 6
> +    items:
> +      - description: Peripheral/APB bus clock
> +      - description: EARC RX biphase clock
> +      - description: Reference clock
> +      - description: Audio interface clock
> +    additionalItems: true

What is the usefulness of all this? How can you even be sure that each
implementation of this core will have exactly these clocks?


> +
> +  clock-names:
> +    minItems: 4
> +    maxItems: 6
> +    items:
> +      - const: pclk
> +      - const: earc
> +      - const: ref
> +      - const: aud
> +    additionalItems: true
> +
> +  interrupts:
> +    minItems: 4
> +    maxItems: 5
> +    items:
> +      - description: AVP Unit interrupt
> +      - description: CEC interrupt
> +      - description: eARC RX interrupt
> +      - description: Main Unit interrupt

If these are real pins, then this seems more possible, but
additionalItems does not make me happy.

I don't see much value in this schema and I am afraid even enforcing
clock and interrupt names won't work for the second or third user.

Best regards,
Krzysztof
Heiko Stübner Aug. 31, 2024, 1:58 p.m. UTC | #2
Hi,

Am Samstag, 31. August 2024, 08:16:26 CEST schrieb Krzysztof Kozlowski:
> On Sat, Aug 31, 2024 at 12:55:29AM +0300, Cristian Ciocaltea wrote:

> > +  clocks:
> > +    minItems: 4
> > +    maxItems: 6
> > +    items:
> > +      - description: Peripheral/APB bus clock
> > +      - description: EARC RX biphase clock
> > +      - description: Reference clock
> > +      - description: Audio interface clock
> > +    additionalItems: true
> 
> What is the usefulness of all this? How can you even be sure that each
> implementation of this core will have exactly these clocks?
>
> > +
> > +  clock-names:
> > +    minItems: 4
> > +    maxItems: 6
> > +    items:
> > +      - const: pclk
> > +      - const: earc
> > +      - const: ref
> > +      - const: aud
> > +    additionalItems: true
> > +
> > +  interrupts:
> > +    minItems: 4
> > +    maxItems: 5
> > +    items:
> > +      - description: AVP Unit interrupt
> > +      - description: CEC interrupt
> > +      - description: eARC RX interrupt
> > +      - description: Main Unit interrupt
> 
> If these are real pins, then this seems more possible, but
> additionalItems does not make me happy.

So while not "pins", the interrupts are separately specified in the
SoC's list of interrupts in the GIC:

RK3588 has:

201  irq_hdmitx0_oavp
202  irq_hdmitx0_ocec
203  irq_hdmitx0_oearcrx
204  irq_hdmitx0_omain
392  irq_hdmitx0_hpd

and another set of all of them for hdmitx1

and RK3576 using the same hdmi IP has:

370  irq_hdmitx_oavp
371  irq_hdmitx_ocec
372  irq_hdmitx_oearcrx
373  irq_hdmitx_omain
399  irq_hdmitx_hpd

so I guess the fifth interrupt is meant to be the hotplug?
Though I guess this should be specificed in the name-list too.

From the SoC's manual it looks like the controller is set up from
different modules.
Like AVP is the audio-video-packet-module, there is a Main and CEC Module
as well as a eARC RX controller inside. I'd guess it might be possible
other SoC vendors could leave out specific modules?


TL;DR I think those clocks and interrupts are dependent on how the
IP core was synthesized, so for now I'd think we can only guarantee
that they are true for rk3588 and rk3576.

So I guess they should move to the rockchip-specific part of the binding
until we have more hdmi-qp controllers in the field?

Heiko
Cristian Ciocaltea Aug. 31, 2024, 9:53 p.m. UTC | #3
On 8/31/24 4:58 PM, Heiko Stübner wrote:
> Hi,
> 
> Am Samstag, 31. August 2024, 08:16:26 CEST schrieb Krzysztof Kozlowski:
>> On Sat, Aug 31, 2024 at 12:55:29AM +0300, Cristian Ciocaltea wrote:
> 
>>> +  clocks:
>>> +    minItems: 4
>>> +    maxItems: 6
>>> +    items:
>>> +      - description: Peripheral/APB bus clock
>>> +      - description: EARC RX biphase clock
>>> +      - description: Reference clock
>>> +      - description: Audio interface clock
>>> +    additionalItems: true
>>
>> What is the usefulness of all this? How can you even be sure that each
>> implementation of this core will have exactly these clocks?
>>
>>> +
>>> +  clock-names:
>>> +    minItems: 4
>>> +    maxItems: 6
>>> +    items:
>>> +      - const: pclk
>>> +      - const: earc
>>> +      - const: ref
>>> +      - const: aud
>>> +    additionalItems: true
>>> +
>>> +  interrupts:
>>> +    minItems: 4
>>> +    maxItems: 5
>>> +    items:
>>> +      - description: AVP Unit interrupt
>>> +      - description: CEC interrupt
>>> +      - description: eARC RX interrupt
>>> +      - description: Main Unit interrupt
>>
>> If these are real pins, then this seems more possible, but
>> additionalItems does not make me happy.
> 
> So while not "pins", the interrupts are separately specified in the
> SoC's list of interrupts in the GIC:
> 
> RK3588 has:
> 
> 201  irq_hdmitx0_oavp
> 202  irq_hdmitx0_ocec
> 203  irq_hdmitx0_oearcrx
> 204  irq_hdmitx0_omain
> 392  irq_hdmitx0_hpd
> 
> and another set of all of them for hdmitx1
> 
> and RK3576 using the same hdmi IP has:
> 
> 370  irq_hdmitx_oavp
> 371  irq_hdmitx_ocec
> 372  irq_hdmitx_oearcrx
> 373  irq_hdmitx_omain
> 399  irq_hdmitx_hpd
> 
> so I guess the fifth interrupt is meant to be the hotplug?

Yep, that's for the hotplug detection.

> Though I guess this should be specificed in the name-list too.

My understanding from Andy was that HPD interrupt is Rockchip platform
specific, hence I made it part of rockchip,rk3588-dw-hdmi-qp.yaml.

> From the SoC's manual it looks like the controller is set up from
> different modules.
> Like AVP is the audio-video-packet-module, there is a Main and CEC Module
> as well as a eARC RX controller inside. I'd guess it might be possible
> other SoC vendors could leave out specific modules?
> 
> 
> TL;DR I think those clocks and interrupts are dependent on how the
> IP core was synthesized, so for now I'd think we can only guarantee
> that they are true for rk3588 and rk3576.
> 
> So I guess they should move to the rockchip-specific part of the binding
> until we have more hdmi-qp controllers in the field?

If that's the case, then we should simply drop the common binding
altogether for now.

Thanks,
Cristian
Andy Yan Sept. 1, 2024, 6:40 a.m. UTC | #4
Hi,

在 2024-09-01 05:53:39,"Cristian Ciocaltea" <cristian.ciocaltea@collabora.com> 写道:
>On 8/31/24 4:58 PM, Heiko Stübner wrote:
>> Hi,
>> 
>> Am Samstag, 31. August 2024, 08:16:26 CEST schrieb Krzysztof Kozlowski:
>>> On Sat, Aug 31, 2024 at 12:55:29AM +0300, Cristian Ciocaltea wrote:
>> 
>>>> +  clocks:
>>>> +    minItems: 4
>>>> +    maxItems: 6
>>>> +    items:
>>>> +      - description: Peripheral/APB bus clock
>>>> +      - description: EARC RX biphase clock
>>>> +      - description: Reference clock
>>>> +      - description: Audio interface clock
>>>> +    additionalItems: true
>>>
>>> What is the usefulness of all this? How can you even be sure that each
>>> implementation of this core will have exactly these clocks?
>>>
>>>> +
>>>> +  clock-names:
>>>> +    minItems: 4
>>>> +    maxItems: 6
>>>> +    items:
>>>> +      - const: pclk
>>>> +      - const: earc
>>>> +      - const: ref
>>>> +      - const: aud
>>>> +    additionalItems: true
>>>> +
>>>> +  interrupts:
>>>> +    minItems: 4
>>>> +    maxItems: 5
>>>> +    items:
>>>> +      - description: AVP Unit interrupt
>>>> +      - description: CEC interrupt
>>>> +      - description: eARC RX interrupt
>>>> +      - description: Main Unit interrupt
>>>
>>> If these are real pins, then this seems more possible, but
>>> additionalItems does not make me happy.
>> 
>> So while not "pins", the interrupts are separately specified in the
>> SoC's list of interrupts in the GIC:
>> 
>> RK3588 has:
>> 
>> 201  irq_hdmitx0_oavp
>> 202  irq_hdmitx0_ocec
>> 203  irq_hdmitx0_oearcrx
>> 204  irq_hdmitx0_omain
>> 392  irq_hdmitx0_hpd
>> 
>> and another set of all of them for hdmitx1
>> 
>> and RK3576 using the same hdmi IP has:
>> 
>> 370  irq_hdmitx_oavp
>> 371  irq_hdmitx_ocec
>> 372  irq_hdmitx_oearcrx
>> 373  irq_hdmitx_omain
>> 399  irq_hdmitx_hpd

The first four interrupts are export from the DW-HDMI-QP IP core。
The fifth HPD interrupts is a rockchip design。

>> 
>> so I guess the fifth interrupt is meant to be the hotplug?
>
>Yep, that's for the hotplug detection.
>
>> Though I guess this should be specificed in the name-list too.
>
>My understanding from Andy was that HPD interrupt is Rockchip platform
>specific, hence I made it part of rockchip,rk3588-dw-hdmi-qp.yaml.
>
>> From the SoC's manual it looks like the controller is set up from
>> different modules.
>> Like AVP is the audio-video-packet-module, there is a Main and CEC Module
>> as well as a eARC RX controller inside. I'd guess it might be possible
>> other SoC vendors could leave out specific modules?
>> 
>> 
>> TL;DR I think those clocks and interrupts are dependent on how the
>> IP core was synthesized, so for now I'd think we can only guarantee
>> that they are true for rk3588 and rk3576.
>> 
>> So I guess they should move to the rockchip-specific part of the binding
>> until we have more hdmi-qp controllers in the field?
>
>If that's the case, then we should simply drop the common binding
>altogether for now.
>
>Thanks,
>Cristian
>
>_______________________________________________
>Linux-rockchip mailing list
>Linux-rockchip@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-rockchip
Krzysztof Kozlowski Sept. 1, 2024, 10:23 a.m. UTC | #5
On 31/08/2024 15:58, Heiko Stübner wrote:
> 
> so I guess the fifth interrupt is meant to be the hotplug?
> Though I guess this should be specificed in the name-list too.
> 
> From the SoC's manual it looks like the controller is set up from
> different modules.
> Like AVP is the audio-video-packet-module, there is a Main and CEC Module
> as well as a eARC RX controller inside. I'd guess it might be possible
> other SoC vendors could leave out specific modules?
> 
> 
> TL;DR I think those clocks and interrupts are dependent on how the
> IP core was synthesized, so for now I'd think we can only guarantee
> that they are true for rk3588 and rk3576.
> 
> So I guess they should move to the rockchip-specific part of the binding
> until we have more hdmi-qp controllers in the field?

Which would leave empty "common" binding.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml b/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml
new file mode 100644
index 000000000000..771f7fba6c50
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml
@@ -0,0 +1,88 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/synopsys,dw-hdmi-qp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common Properties for Synopsys DesignWare HDMI QP TX Controller IP
+
+maintainers:
+  - Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
+
+description: |
+  The Synopsys DesignWare HDMI 2.1 Quad-Pixel (QP) TX Controller IP core
+  supports the following features, among others:
+
+  * Fixed Rate Link (FRL)
+  * Display Stream Compression (DSC)
+  * 4K@120Hz and 8K@60Hz video modes
+  * Variable Refresh Rate (VRR) including Quick Media Switching (QMS)
+  * Fast Vactive (FVA)
+  * SCDC I2C DDC access
+  * Multi-stream audio
+  * Enhanced Audio Return Channel (EARC)
+
+  Note this is not a full dt-binding specification, but is meant to be
+  referenced by platform-specific bindings for this IP core.
+
+properties:
+  reg:
+    maxItems: 1
+
+  clocks:
+    minItems: 4
+    maxItems: 6
+    items:
+      - description: Peripheral/APB bus clock
+      - description: EARC RX biphase clock
+      - description: Reference clock
+      - description: Audio interface clock
+    additionalItems: true
+
+  clock-names:
+    minItems: 4
+    maxItems: 6
+    items:
+      - const: pclk
+      - const: earc
+      - const: ref
+      - const: aud
+    additionalItems: true
+
+  interrupts:
+    minItems: 4
+    maxItems: 5
+    items:
+      - description: AVP Unit interrupt
+      - description: CEC interrupt
+      - description: eARC RX interrupt
+      - description: Main Unit interrupt
+    additionalItems: true
+
+  interrupt-names:
+    minItems: 4
+    maxItems: 5
+    items:
+      - const: avp
+      - const: cec
+      - const: earc
+      - const: main
+    additionalItems: true
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Video port for RGB/YUV input.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Video port for HDMI/eDP output.
+
+    required:
+      - port@0
+      - port@1
+
+additionalProperties: true