Re: [PATCH RFC v3 09/10] dt-bindings: display: rockchip: dw-dp: fix sound DAI cells

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 3922 bytes --]

Hi DT binding maintainers (& Dmitry for his DP background),

Can you have a look at this please? It has been a month, since I
send this and multiple months since I send RFCv1. Would be good
to have your preference here, so that I can implement it accordingly
in the next version.

On Fri, Jun 12, 2026 at 08:00:55PM +0200, Sebastian Reichel wrote:
> The RK3588 and RK3576 DesignWare DisplayPort controllers both have two
> possible DAI interfaces: I2S and S/PDIF. Thus it is needed to have an
> argument to select the right interface.
> 
> In case of RK3576 this is not enough though. The RK3576 has the same IP
> as RK3588, but configured with Multi Stream Transport (MST) enabled for
> up to 3 displays and thus has a total of 6 DAI interfaces (I2S and
> S/PDIF for each possible stream. Meanwhile the RK3588 does not support
> MST and thus has only 2 DAI interfaces.
> 
> The binding update right now only supports the simple single stream
> transport (SST) setup. To avoid further DT ABI breakage (or complicated
> bindings supporting different number of arguments), it's probably a good
> idea to take MST into account now even though the upstream Linux driver
> does not yet support it.
> 
> I see two options:
> 
> 1. Adding yet another cell, so that we have the following:
>    <&dp_ctrl [display_stream] [i2s_or_spdif]>; potentially append
>    extra input ports for MST video data to existing ports node
>    (e.g. port@2). I would only handle the sound DAI part in my
>    patch and basically use '0' for the display stream and just
>    leave the option of using '1' or '2' once MST support is added.

My guess is, that this is preferred.

> 2. The vendor kernel creates a sub-node for each supported display
>    stream and puts the ports mapping as well as the DAI reference
>    into that. This bundles all information for one display stream
>    together, which creates a clean look but the subnode does not
>    really describe any real thing in the hardware.

FWIW that looks like this:

dp@<address> {
    props-from-this-binding;

    /* SST or MST stream 0 */
    dp0 {
        #sound-dai-cells = <1>;
        ports { ... };
    };

    /* MST stream 1 */
    dp1 {
        #sound-dai-cells = <1>;
        ports { ... };
    };

    /* MST stream 2 */
    dp2 {
        #sound-dai-cells = <1>;
        ports { ... };
    };
};

While the grouping looks nice to me from the audio/video controller
to DP controller point of view, it's less sensible for the
connection from the DP controller to the DP/USB-C connector as all
streams are bundled at that point.

Thanks and greetings,

-- Sebastian

> As upstream MST support seems to be quite limited, I wish for some
> feedback about the preferred way to handle this.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  .../devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml         | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml
> index 2b0d9e23e943..1303d0e2145a 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml
> @@ -83,7 +83,8 @@ properties:
>      maxItems: 1
>  
>    "#sound-dai-cells":
> -    const: 0
> +    const: 1
> +    description: 0 for I2S, 1 for SPDIF
>  
>  required:
>    - compatible
> @@ -144,7 +145,7 @@ examples:
>          resets = <&cru SRST_DP0>;
>          phys = <&usbdp_phy0 PHY_TYPE_DP>;
>          power-domains = <&power RK3588_PD_VO0>;
> -        #sound-dai-cells = <0>;
> +        #sound-dai-cells = <1>;
>  
>          ports {
>            #address-cells = <1>;
> 
> -- 
> 2.53.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]