Sorry, my previous description may not have been accurate enough.
When|radeon_audio_detect|is triggered, the disconnected state of the
connector has not yet been uploaded to DRM.
This prevents other components (functions not included
in|radeon_dvi_detect|/|radeon_dp_detect|) from retrieving the latest
state of the connector.
|radeon_audio_component_notify|is a function
within|radeon_dvi_detect|/|radeon_dp_detect|,
while|radeon_audio_component_get_eld|is a callback function used by the
audio component (|snd_hda_intel|).
Its triggering works as follows:|radeon_audio_component_notify|notifies
the audio component to retrieve the latest state,
but when|radeon_audio_component_get_eld|actually
executes,|radeon_dvi_detect|/|radeon_dp_detect|has not yet completed,
resulting in incorrect retrieval of the connector's state.
You mentioned this looks like a race condition, and that is indeed the
case, but their starting points are different.
Furthermore,|radeon_audio_component_notify|already
has|mutex_lock(&connector->eld_mutex)|and|mutex_unlock(&connector->eld_mutex)|,
staticvoidradeon_audio_component_notify(structradeon_device*rdev, intport)
{
structdrm_audio_component*acomp;
mutex_lock(&rdev->audio.component_mutex);
acomp=rdev->audio.component;
if(acomp&&acomp->audio_ops&&acomp->audio_ops->pin_eld_notify)
acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
port, -1);
mutex_unlock(&rdev->audio.component_mutex);
}
but this doesn't work
because|radeon_audio_component_notify|and|radeon_audio_component_get_eld|do
not have a hierarchical relationship;
instead, it's an indirect notification-based
relationship.|radeon_audio_component_notify|notifies|snd_hda_intel|, and
then|snd_hda_intel|executes|radeon_audio_component_get_eld|.
Unless we acquire the lock in|radeon_audio_component_notify|and release
it in|radeon_audio_component_get_eld|—but this could lead to a deadlock,
because|radeon_audio_component_get_eld|is triggered through more means
than just|radeon_audio_component_notify|.
在 2025/9/25 01:58, Alex Deucher 写道:
On Wed, Sep 24, 2025 at 7:44 AM<[email protected]> wrote:
From: Wang Jiang<[email protected]>
The audio detection process in the Radeon driver is as follows:
radeon_dvi_detect/radeon_dp_detect -> radeon_audio_detect -> radeon_audio_enable
-> radeon_audio_component_notify -> radeon_audio_component_get_eld
When HDMI is unplugged, radeon_dvi_detect is triggered.
At this point, radeon_audio_detect is triggered before radeon_dvi_detect has
finished (which also means the new state of the connector has not been
reported).
In this scenario, radeon_audio_detect can detect that the connector is
disconnected (because the parameter is passed down),
but it is very likely that the audio callback function
radeon_audio_component_get_eld cannot detect the disconnection of the connector.
As a result, when the audio component (radeon_audio_component_get_eld) performs
detection, the connector's state is not shown as disconnected,
and connector->eld is not zero, causing the audio component to think the audio
driver is still working.
I have added a new member (enable_mask) to the audio structure to record the
audio enable status.
Only when radeon_audio_component_get_eld detects that enable_mask is not zero
will it continue to work.
There might be other solutions, such as placing
radeon_audio_detect/radeon_audio_component_notify after the completion of
radeon_XX_detect.
However, I found that this would require significant changes (or perhaps it's
just my limited coding skills?).
This still looks like a race. I think the get_eld() callback can get
called whenever. The proper fix is probably to hold the
connector->eld_mutex in radeon_audio_detect().
Alex
Signed-off-by: Wang Jiang<[email protected]>
---
drivers/gpu/drm/radeon/radeon.h | 1 +
drivers/gpu/drm/radeon/radeon_audio.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 63c47585afbc..2d0a411e3ed6 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1745,6 +1745,7 @@ struct r600_audio_pin {
u32 offset;
bool connected;
u32 id;
+ u8 enable_mask;
};
struct r600_audio {
diff --git a/drivers/gpu/drm/radeon/radeon_audio.c
b/drivers/gpu/drm/radeon/radeon_audio.c
index 8d64ba18572e..a0717895cc8a 100644
--- a/drivers/gpu/drm/radeon/radeon_audio.c
+++ b/drivers/gpu/drm/radeon/radeon_audio.c
@@ -212,6 +212,7 @@ static void radeon_audio_enable(struct radeon_device *rdev,
if (rdev->audio.funcs->enable)
rdev->audio.funcs->enable(rdev, pin, enable_mask);
+ rdev->audio.pin[pin->id].enable_mask = enable_mask;
radeon_audio_component_notify(rdev, pin->id);
}
@@ -274,6 +275,7 @@ int radeon_audio_init(struct radeon_device *rdev)
rdev->audio.pin[i].connected = false;
rdev->audio.pin[i].offset = pin_offsets[i];
rdev->audio.pin[i].id = i;
+ rdev->audio.pin[i].enable_mask = 0;
}
radeon_audio_interface_init(rdev);
@@ -760,6 +762,9 @@ static int radeon_audio_component_get_eld(struct device
*kdev, int port,
if (!rdev->audio.enabled || !rdev->mode_info.mode_config_initialized)
return 0;
+ if (rdev->audio.pin[port].enable_mask == 0)
+ return 0;
+
list_for_each_entry(connector, &dev->mode_config.connector_list, head)
{
const struct drm_connector_helper_funcs *connector_funcs =
connector->helper_private;
--
2.25.1
