Re: [PATCH 12/14] dt-bindings: display: rockchip,dw-hdmi: Add compatible for RK3588

Cristian Ciocaltea
2024-06-06T19:28:43Z

On 6/6/24 5:58 PM, Rob Herring wrote:
> On Thu, Jun 6, 2024 at 5:51 AM Cristian Ciocaltea
> <cristian.ciocaltea@collabora.com> wrote:
>>
>> On 6/6/24 2:22 AM, Rob Herring wrote:
>>> On Sat, Jun 01, 2024 at 04:12:34PM +0300, Cristian Ciocaltea wrote:
>>>> Document the Synopsys DesignWare HDMI 2.1 Quad-Pixel (QP) TX controller
>>>> found on Rockchip RK3588 SoC family.
>>>>
>>>> Since RK3588 uses different clocks than previous Rockchip SoCs and also
>>>> requires a couple of reset lines and some additional properties, provide
>>>> the required changes in the binding to accommodate all variants.
>>>>
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>>>> ---
>>>> .../display/rockchip/rockchip,dw-hdmi.yaml | 127 +++++++++++++++------
>>>> 1 file changed, 90 insertions(+), 37 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
>>>> index 2aac62219ff6..60d6b815227f 100644
>>>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
>>>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-hdmi.yaml
>>>> @@ -10,12 +10,10 @@ maintainers:
>>>> - Mark Yao <markyao0591@gmail.com>
>>>>
>>>> description: |
>>>> - The HDMI transmitter is a Synopsys DesignWare HDMI 1.4 TX controller IP
>>>> - with a companion PHY IP.
>>>> -
>>>> -allOf:
>>>> - - $ref: ../bridge/synopsys,dw-hdmi.yaml#
>>>> - - $ref: /schemas/sound/dai-common.yaml#
>>>> + For SoCs up to RK3568, the HDMI transmitter is a Synopsys DesignWare
>>>> + HDMI 1.4 TX controller IP with a companion PHY IP.
>>>> + The RK3588 SoC integrates the Synopsys DesignWare HDMI 2.1 Quad-Pixel (QP)
>>>> + TX controller IP and a HDMI/eDP TX Combo PHY based on a Samsung IP block.
>>>>
>>>> properties:
>>>> compatible:
>>>> @@ -25,6 +23,7 @@ properties:
>>>> - rockchip,rk3328-dw-hdmi
>>>> - rockchip,rk3399-dw-hdmi
>>>> - rockchip,rk3568-dw-hdmi
>>>> + - rockchip,rk3588-dw-hdmi
>>>>
>>>> reg-io-width:
>>>> const: 4
>>>> @@ -40,36 +39,6 @@ properties:
>>>> A 1.8V supply that powers up the SoC internal circuitry. The pin name on the
>>>> SoC usually is HDMI_TX_AVDD_1V8.
>>>>
>>>> - clocks:
>>>> - minItems: 2
>>>> - items:
>>>> - - {}
>>>> - - {}
>>>> - # The next three clocks are all optional, but shall be specified in this
>>>> - # order when present.
>>>> - - description: The HDMI CEC controller main clock
>>>> - - description: Power for GRF IO
>>>> - - description: External clock for some HDMI PHY (old clock name, deprecated)
>>>> - - description: External clock for some HDMI PHY (new name)
>>>> -
>>>> - clock-names:
>>>> - minItems: 2
>>>> - items:
>>>> - - {}
>>>> - - {}
>>>> - - enum:
>>>> - - cec
>>>> - - grf
>>>> - - vpll
>>>> - - ref
>>>> - - enum:
>>>> - - grf
>>>> - - vpll
>>>> - - ref
>>>> - - enum:
>>>> - - vpll
>>>> - - ref
>>>> -
>>>> ddc-i2c-bus:
>>>> $ref: /schemas/types.yaml#/definitions/phandle
>>>> description:
>>>> @@ -131,13 +100,97 @@ properties:
>>>> required:
>>>> - compatible
>>>> - reg
>>>> - - reg-io-width
>>>> - clocks
>>>> - clock-names
>>>> - interrupts
>>>> - ports
>>>> - rockchip,grf
>>>>
>>>> +allOf:
>>>> + - $ref: /schemas/sound/dai-common.yaml#
>>>> + - if:
>>>> + properties:
>>>> + compatible:
>>>> + contains:
>>>> + enum:
>>>> + - rockchip,rk3588-dw-hdmi
>>>> + then:
>>>> + properties:
>>>> + reg:
>>>> + maxItems: 1
>>>> +
>>>> + clocks:
>>>> + minItems: 1
>>>> + items:
>>>> + - description: APB system interface clock
>>>> + # The next clocks are optional, but shall be specified in this
>>>> + # order when present.
>>>> + - description: TMDS/FRL link clock
>>>> + - description: EARC RX biphase clock
>>>> + - description: Reference clock
>>>> + - description: Audio interface clock
>>>> + - description: Video datapath clock
>>>> +
>>>> + clock-names:
>>>> + minItems: 1
>>>> + items:
>>>> + - const: pclk
>>>> + - enum: [hdp, earc, ref, aud, hclk_vo1]
>>>> + - enum: [earc, ref, aud, hclk_vo1]
>>>> + - enum: [ref, aud, hclk_vo1]
>>>> + - enum: [aud, hclk_vo1]
>>>> + - const: hclk_vo1
>>>> +
>>>> + resets:
>>>> + minItems: 2
>>>> + maxItems: 2
>>>> +
>>>> + reset-names:
>>>> + items:
>>>> + - const: ref
>>>> + - const: hdp
>>>> +
>>>> + interrupts:
>>>> + minItems: 1
>>>> + maxItems: 5
>>>> +
>>>> + rockchip,vo1_grf:
>>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>>> + description: Some QP related data is accessed through VO1 GRF regs
>>>> +
>>>> + required:
>>>> + - resets
>>>> + - reset-names
>>>> + - rockchip,vo1_grf
>>>> +
>>>> + else:
>>>> + $ref: ../bridge/synopsys,dw-hdmi.yaml#
>>>
>>> This is odd... With this plus the amount of conditional schema, I think
>>> this should be a new schema doc. Doesn't have to have a common
>>> schema. You can let the 2nd user of this IP block do that.
>>
>> Yes, v2 is going to be a completely separated driver implementation.
>
> That actually has nothing to do with the decision here. Bindings are
> separate from drivers.

Right, I should have properly explained that initially this QP
controller has been handled more like another variant of those found in
the older SoCs, rather than a totally new one. It doesn't really have
anything in common, except that the IP comes from Synopsys, hence it
makes sense to also have a dedicated schema.

Thanks,
Cristian