[PATCH v2] drm/amdgpu: Only enable HDMI audio if display supports it

[PATCH v2] drm/amdgpu: Only enable HDMI audio if display supports it

[Panpan Ren]

From: renpanpan <renpanpan@kylinos.cn>

The amdgpu legacy DCE audio path (dce_v6/v8/v10) unconditionally calls
audio_enable(pin, true) at the end of afmt_setmode(), regardless of
whether the connected display advertises audio support in its EDID.

This causes a phantom HDMI audio output to appear for displays that
have no audio capability (no basic audio bit, no Audio Data Block in
EDID), because the GPU sets the AUDIO_ENABLED register bit, which
drives the HDA codec's ELDV pin sense, making the codec report the
jack as audio-available.

The radeon, nouveau, and dw-hdmi drivers all gate audio enablement on
the display's audio capability. Apply the same check to amdgpu's DCE
afmt_setmode functions, while still respecting the AMDGPU_AUDIO_ENABLE
override for users who explicitly force audio on.

Fixes: e2cdf640cbb5 ("drm/amdgpu: add display controller implementation for si v10")
Cc: stable@vger.kernel.org
Signed-off-by: renpanpan <renpanpan@kylinos.cn>
---
Changes in v2:
- Respect AMDGPU_AUDIO_ENABLE override for users who force audio on
- Add Fixes tag and Cc: stable@vger.kernel.org for stable backport

 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 6 ++++--
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c  | 6 ++++--
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c  | 6 ++++--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index f2977fe6d824..69d0c6041d75 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1705,8 +1705,10 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
 	WREG32(mmAFMT_RAMP_CONTROL2 + dig->afmt->offset, 0x00000001);
 	WREG32(mmAFMT_RAMP_CONTROL3 + dig->afmt->offset, 0x00000001);
 
-	/* enable audio after to setting up hw */
-	dce_v10_0_audio_enable(adev, dig->afmt->pin, true);
+	/* Only enable audio if the display supports it or is forced on */
+	if (connector->display_info.has_audio ||
+	    to_amdgpu_connector(connector)->audio == AMDGPU_AUDIO_ENABLE)
+		dce_v10_0_audio_enable(adev, dig->afmt->pin, true);
 }
 
 static void dce_v10_0_afmt_enable(struct drm_encoder *encoder, bool enable)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index c68de0fe1d7d..226cfce321ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1748,8 +1748,10 @@ static void dce_v6_0_afmt_setmode(struct drm_encoder *encoder,
 		dce_v6_0_audio_dp_enable(encoder, 1);
 	}
 
-	/* enable audio after setting up hw */
-	dce_v6_0_audio_enable(adev, dig->afmt->pin, true);
+	/* Only enable audio if the display supports it or is forced on */
+	if (connector->display_info.has_audio ||
+	    amdgpu_connector->audio == AMDGPU_AUDIO_ENABLE)
+		dce_v6_0_audio_enable(adev, dig->afmt->pin, true);
 }
 
 static void dce_v6_0_afmt_enable(struct drm_encoder *encoder, bool enable)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index c3906270f25e..99f0c5759d75 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1653,8 +1653,10 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
 	WREG32(mmAFMT_RAMP_CONTROL2 + offset, 0x00000001);
 	WREG32(mmAFMT_RAMP_CONTROL3 + offset, 0x00000001);
 
-	/* enable audio after setting up hw */
-	dce_v8_0_audio_enable(adev, dig->afmt->pin, true);
+	/* Only enable audio if the display supports it or is forced on */
+	if (connector->display_info.has_audio ||
+	    to_amdgpu_connector(connector)->audio == AMDGPU_AUDIO_ENABLE)
+		dce_v8_0_audio_enable(adev, dig->afmt->pin, true);
 }
 
 static void dce_v8_0_afmt_enable(struct drm_encoder *encoder, bool enable)
-- 
2.43.0

Re: [PATCH v2] drm/amdgpu: Only enable HDMI audio if display supports it

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/amdgpu: dce_v*_0_afmt_setmode() disables shared audio pin when !has_audio
--

commit 82b015798aa29dad02e55c15ad68c627d16b7df9
Author: renpanpan <renpanpan@kylinos.cn>

drm/amdgpu: Only enable HDMI audio if display supports it

This commit gates the final audio_enable(adev, dig->afmt->pin, true) call at
the end of dce_v6_0_afmt_setmode(), dce_v8_0_afmt_setmode(), and
dce_v10_0_afmt_setmode() on connector->display_info.has_audio or the
AMDGPU_AUDIO_ENABLE override.

> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 7de7adc8f225c..a692227ddfefa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -1705,8 +1705,10 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
>  	WREG32(mmAFMT_RAMP_CONTROL2 + dig->afmt->offset, 0x00000001);
>  	WREG32(mmAFMT_RAMP_CONTROL3 + dig->afmt->offset, 0x00000001);
>  
> -	/* enable audio after to setting up hw */
> -	dce_v10_0_audio_enable(adev, dig->afmt->pin, true);
> +	/* Only enable audio if the display supports it or is forced on */
> +	if (connector->display_info.has_audio ||
> +	    to_amdgpu_connector(connector)->audio == AMDGPU_AUDIO_ENABLE)
> +		dce_v10_0_audio_enable(adev, dig->afmt->pin, true);
>  }
>  
>  static void dce_v10_0_afmt_enable(struct drm_encoder *encoder, bool enable)
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 3c1d6901127d1..d21efce004f56 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -1748,8 +1748,10 @@ static void dce_v6_0_afmt_setmode(struct drm_encoder *encoder,
>  		dce_v6_0_audio_dp_enable(encoder, 1);
>  	}
>  
> -	/* enable audio after setting up hw */
> -	dce_v6_0_audio_enable(adev, dig->afmt->pin, true);
> +	/* Only enable audio if the display supports it or is forced on */
> +	if (connector->display_info.has_audio ||
> +	    amdgpu_connector->audio == AMDGPU_AUDIO_ENABLE)
> +		dce_v6_0_audio_enable(adev, dig->afmt->pin, true);
>  }
>  
>  static void dce_v6_0_afmt_enable(struct drm_encoder *encoder, bool enable)
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index a17944d5e8434..22c0d91c3041c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1653,8 +1653,10 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
>  	WREG32(mmAFMT_RAMP_CONTROL2 + offset, 0x00000001);
>  	WREG32(mmAFMT_RAMP_CONTROL3 + offset, 0x00000001);
>  
> -	/* enable audio after setting up hw */
> -	dce_v8_0_audio_enable(adev, dig->afmt->pin, true);
> +	/* Only enable audio if the display supports it or is forced on */
> +	if (connector->display_info.has_audio ||
> +	    to_amdgpu_connector(connector)->audio == AMDGPU_AUDIO_ENABLE)
> +		dce_v8_0_audio_enable(adev, dig->afmt->pin, true);
>  }

[Severity: Medium]
Can gating only the final audio_enable(..., true) call in
dce_v6_0_afmt_setmode(), dce_v8_0_afmt_setmode(), and
dce_v10_0_afmt_setmode() disable active audio on another connected display?

In dce_v6_0.c, dce_v8_0.c, and dce_v10_0.c, the audio_get_pin() helpers do
not track per-encoder pin ownership and simply return the first connected pin
in adev->mode_info.audio.pin[].

At the start of dce_v10_0_afmt_setmode() (and similarly in
dce_v6_0_afmt_setmode() and dce_v8_0_afmt_setmode()), the function
unconditionally grabs and disables that pin before configuring the hardware:

	/* disable audio prior to setting up hw */
	dig->afmt->pin = dce_v10_0_audio_get_pin(adev);
	dce_v10_0_audio_enable(adev, dig->afmt->pin, false);

It then programs the DTO, pin endpoint registers (speaker allocation, SAD,
lipsync), and enables HDMI_AUDIO_INFO_SEND and AFMT_AUDIO_SAMPLE_SEND.

When a non-audio HDMI monitor (or a non-audio DP monitor on DCE6) sets mode
while another encoder is driving an audio-capable display, afmt_setmode() on
the non-audio encoder will grab the same audio pin, disable it via
audio_enable(adev, dig->afmt->pin, false), overwrite its endpoint and DTO
state, and then skip re-enabling it at the end of the function.

Furthermore, because dig->afmt->pin remains assigned on the non-audio
encoder, a subsequent call to dce_v6_0_afmt_enable(), dce_v8_0_afmt_enable(),
or dce_v10_0_afmt_enable() with enable == false on the non-audio encoder will
also call audio_enable(adev, dig->afmt->pin, false) and shut off the shared
pin:

	if (!enable && dig->afmt->pin) {
		dce_v10_0_audio_enable(adev, dig->afmt->pin, false);
		dig->afmt->pin = NULL;
	}

Should the audio capability check happen before acquiring and disabling
dig->afmt->pin (or in the caller before enabling AFMT audio), similar to how
radeon checks drm_detect_monitor_audio() before setting up audio?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103033914.21660-1-renpanpan@kylinos.cn?part=1