Re: [PATCH RESEND] drm/bridge-connector: Fix bridge in drm_connector_hdmi_audio_init()

[Stephan Gerhold <stephan.gerhold@linaro.org>]

On Fri, Jun 20, 2025 at 09:16:16AM +0800, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> The bridge used in drm_connector_hdmi_audio_init() does not correctly
> point to the required audio bridge, which lead to incorrect audio
> configuration input.
> 
> Fixes: 231adeda9f67 ("drm/bridge-connector: hook DisplayPort audio support")
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>

Thanks, this patch fixes the following errors for me on a Qualcomm X1
Elite laptop with v6.16-rc3:

[   10.900256] hdmi-audio-codec hdmi-audio-codec.5.auto: hdmi_codec_probe: Invalid parameters
[   10.901376] hdmi-audio-codec hdmi-audio-codec.4.auto: hdmi_codec_probe: Invalid parameters
[   10.909627] hdmi-audio-codec hdmi-audio-codec.5.auto: probe with driver hdmi-audio-codec failed with error -22
[   10.910813] hdmi-audio-codec hdmi-audio-codec.6.auto: hdmi_codec_probe: Invalid parameters
[   10.918145] hdmi-audio-codec hdmi-audio-codec.4.auto: probe with driver hdmi-audio-codec failed with error -22
[   10.919296] hdmi-audio-codec hdmi-audio-codec.5.auto: hdmi_codec_probe: Invalid parameters
[   10.919298] hdmi-audio-codec hdmi-audio-codec.5.auto: probe with driver hdmi-audio-codec failed with error -22
[   10.928482] hdmi-audio-codec hdmi-audio-codec.6.auto: probe with driver hdmi-audio-codec failed with error -22
[   10.930742] hdmi-audio-codec hdmi-audio-codec.6.auto: hdmi_codec_probe: Invalid parameters
[   10.930744] hdmi-audio-codec hdmi-audio-codec.6.auto: probe with driver hdmi-audio-codec failed with error -22

By de-referencing the "bridge" from the outer scope I think the current
broken code basically de-references some random out-of-bounds memory.
This should be fixed ASAP.

Tested-by: Stephan Gerhold <stephan.gerhold@linaro.org>

> ---
>  drivers/gpu/drm/display/drm_bridge_connector.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 7d2e499ea5de..262e93e07a28 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -708,11 +708,14 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>  	if (bridge_connector->bridge_hdmi_audio ||
>  	    bridge_connector->bridge_dp_audio) {
>  		struct device *dev;
> +		struct drm_bridge *bridge;

This is technically redundant since "bridge" is already defined in the
top-level of this function and shadowing it (declaring it again) doesn't
really make it less confusing.

I would suggest either dropping this line and re-assigning the existing
bridge variable (as it was before commit 231adeda9f67
("drm/bridge-connector: hook DisplayPort audio support")), or perhaps
renaming this to

		struct drm_bridge *audio_bridge;

for less confusion.

>  
>  		if (bridge_connector->bridge_hdmi_audio)
> -			dev = bridge_connector->bridge_hdmi_audio->hdmi_audio_dev;
> +			bridge = bridge_connector->bridge_hdmi_audio;
>  		else
> -			dev = bridge_connector->bridge_dp_audio->hdmi_audio_dev;
> +			bridge = bridge_connector->bridge_dp_audio;
> +
> +		dev = bridge->hdmi_audio_dev;

The "dev" variable seems pointless now, just inline
bridge->hdmi_audio_dev in the call below?

>  
>  		ret = drm_connector_hdmi_audio_init(connector, dev,
>  						    &drm_bridge_connector_hdmi_audio_funcs,

Thanks,
Stephan