Re: [PATCH v1] arm64: dts: qcom: glymur-crd: add DisplayPort audio DAI link

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] arm64: dts: qcom: glymur-crd: dp-dai-link breaks sound probe on mahua-crd
- [Medium] arm64: dts: qcom: glymur-crd: incomplete and unenabled DP audio DAI link
--

commit bb4f3b10ef946c74b24095ec32db334ac56b222c
Author: Chiluka Rohith <rohith.chiluka@oss.qualcomm.com>

arm64: dts: qcom: glymur-crd: add DisplayPort audio DAI link

This commit adds a DisplayPort RX DAI link to the sound node in
glymur-crd.dtsi, wiring the DISPLAY_PORT_RX_1 backend of q6apm to the
mdss_dp1 controller.

> diff --git a/arch/arm64/boot/dts/qcom/glymur-crd.dtsi b/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
> index 58158b51fe295..eee75da201983 100644
> --- a/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
> +++ b/arch/arm64/boot/dts/qcom/glymur-crd.dtsi
> @@ -268,6 +268,22 @@ platform {
>  			};
>  		};
>  
> +		dp-dai-link {
> +			link-name = "DISPLAY_PORT-RX";
> +
> +			codec {
> +				sound-dai = <&mdss_dp1>;
> +			};

[Severity: High]
Does adding this active dp-dai-link node to the shared
arch/arm64/boot/dts/qcom/glymur-crd.dtsi break probing of the entire sound
card on mahua-crd?

arch/arm64/boot/dts/qcom/glymur-crd.dtsi is included by both
arch/arm64/boot/dts/qcom/glymur-crd.dts and
arch/arm64/boot/dts/qcom/mahua-crd.dts:

arch/arm64/boot/dts/qcom/mahua-crd.dts:
    #include "mahua.dtsi"
    #include "glymur-crd.dtsi"

In arch/arm64/boot/dts/qcom/glymur.dtsi, mdss_dp1 defaults to disabled:

arch/arm64/boot/dts/qcom/glymur.dtsi:
    mdss_dp1: displayport-controller@af5c000 {
        ...
        #sound-dai-cells = <0>;

        status = "disabled";
    };

While arch/arm64/boot/dts/qcom/glymur-crd.dts enables mdss_dp1, neither
glymur-crd.dtsi nor mahua-crd.dts enables it.

When booting mahua-crd, x1e80100_platform_probe() -> qcom_snd_parse_of()
iterates over all available child nodes of /sound and calls
snd_soc_of_get_dai_link_codecs() -> snd_soc_of_get_dlc() ->
snd_soc_get_dlc():

sound/soc/qcom/common.c:qcom_snd_parse_of()
    if (codec) {
        ret = snd_soc_of_get_dai_link_codecs(dev, codec, link);
        if (ret < 0) {
            dev_err_probe(dev, ret,
                          "%s: codec dai not found\n", link->name);
            return ret;
        }
    }

Because mdss_dp1 is disabled on mahua-crd, msm_dp_display_probe() never runs
for mdss_dp1 and never registers its ASoC codec component, causing
snd_soc_get_dlc() to return -EPROBE_DEFER permanently and preventing the
qcom,glymur-sndcard sound card (including WSA speaker playback and VA DMIC
capture) from registering on mahua-crd.

> +
> +			cpu {
> +				sound-dai = <&q6apmbedai DISPLAY_PORT_RX_1>;
> +			};
> +
> +			platform {
> +				sound-dai = <&q6apm>;
> +			};
> +		};

[Severity: Medium]
Should this also wire up the first DisplayPort controller (mdss_dp0 with
DISPLAY_PORT_RX_0) and follow the DisplayPort DAI link naming used by other
boards handled by sound/soc/qcom/x1e80100.c?

In arch/arm64/boot/dts/qcom/glymur-crd.dtsi, two DisplayPort-capable USB-C
connectors are defined under pmic-glink (connector@0 routed via usb_0_qmpphy
to mdss_dp0, and connector@1 routed via usb_1_qmpphy to mdss_dp1), and
arch/arm64/boot/dts/qcom/glymur-crd.dts enables both controllers:

arch/arm64/boot/dts/qcom/glymur-crd.dts:
    &mdss_dp0 {
        status = "okay";
    };
    ...
    &mdss_dp1 {
        status = "okay";
    };

However, this patch only adds a link for mdss_dp1 (DISPLAY_PORT_RX_1) and
omits mdss_dp0 (DISPLAY_PORT_RX_0).

In addition, other boards using sound/soc/qcom/x1e80100.c (such as
arch/arm64/boot/dts/qcom/mahua-lenovo-thinkpad-t14s-gen7-lcd.dts, which also
uses compatible = "qcom,glymur-sndcard", as well as x1-crd.dtsi) name these
nodes displayport-0-dai-link and displayport-1-dai-link with link-name set
to "DisplayPort0 Playback" and "DisplayPort1 Playback" instead of
dp-dai-link and "DISPLAY_PORT-RX":

arch/arm64/boot/dts/qcom/mahua-lenovo-thinkpad-t14s-gen7-lcd.dts:
    displayport-0-dai-link {
        link-name = "DisplayPort0 Playback";

        cpu {
            sound-dai = <&q6apmbedai DISPLAY_PORT_RX_0>;
        };

        codec {
            sound-dai = <&mdss_dp0>;
        };

        platform {
            sound-dai = <&q6apm>;
        };
    };

    displayport-1-dai-link {
        link-name = "DisplayPort1 Playback";
        ...
    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105104657.621104-1-rohith.chiluka@oss.qualcomm.com?part=1