Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [High] Audio state is permanently broken upon reconnect if a full modeset does not occur. - [Low] Spurious ELD notifications with an invalid pin (-1) during normal CRTC disable sequence. Pre-existing issues: - [Critical] Data race between ELD notification and audio component unbind leads to Use-After-Free. -- commit 0c399aeba0fb63f7bdb4374bcb0fb67d5cd679bb Author: Nícolas F. R. A. Prado <[email protected]> drm/amd/display: Always notify the HDA driver on connector disconnect This patch modifies the display detect logic to call amdgpu_dm_audio_eld_notify() when a sink is disconnected. This ensures the HDA driver is notified even if no subsequent atomic commit occurs. > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_audio.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_audio.c > index 13c9a9d145baa..c9da19335eacf 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_audio.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_audio.c > @@ -168,7 +168,7 @@ void amdgpu_dm_audio_fini(struct amdgpu_device *adev) > } > EXPORT_IF_KUNIT(amdgpu_dm_audio_fini); > > -STATIC_IFN_KUNIT void amdgpu_dm_audio_eld_notify(struct amdgpu_device *adev, > int pin) > +void amdgpu_dm_audio_eld_notify(struct amdgpu_device *adev, int pin) > { > struct drm_audio_component *acomp = adev->dm.audio_component; > [Severity: Critical] This is a pre-existing issue, but can this lead to a use-after-free if the HDA module is unloaded concurrently? Looking at amdgpu_dm_audio_eld_notify(), the acomp pointer and its audio_ops are checked and dereferenced without holding any locks: if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) { Concurrently, if the HDA module is unloaded, amdgpu_dm_audio_component_unbind() clears these component fields. If the notify function is preempted after its check, could it dereference the freed audio_ops? [ ... ] > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > index 300ee26f26ff0..9286f86a20cd7 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c [ ... ] > @@ -601,6 +603,13 @@ void amdgpu_dm_update_connector_after_detect( > /* Set CP to DESIRED if it was ENABLED, so we can re-enable it > again on hotplug */ > if (connector->state->content_protection == > DRM_MODE_CONTENT_PROTECTION_ENABLED) > connector->state->content_protection = > DRM_MODE_CONTENT_PROTECTION_DESIRED; > + > + mutex_lock(&adev->dm.audio_lock); > + inst = aconnector->audio_inst; > + aconnector->audio_inst = -1; > + mutex_unlock(&adev->dm.audio_lock); > + if (inst != -1) > + amdgpu_dm_audio_eld_notify(adev, inst); [Severity: High] Does wiping aconnector->audio_inst to -1 on disconnect permanently break audio upon reconnect if a full modeset does not occur? If a display is reconnected and userspace triggers an atomic commit that does not require a full modeset (like a fastset), amdgpu_dm_commit_audio() skips the connector because drm_atomic_crtc_needs_modeset() is false. In that scenario, does audio_inst remain -1, preventing the HDA driver from ever being notified of the new ELD? [Severity: Low] Also, does setting audio_inst to -1 here cause spurious ELD notifications with an invalid pin during a normal CRTC disable sequence? If a display is disconnected, audio_inst becomes -1. Later, when a modeset disables the CRTC, amdgpu_dm_commit_audio() reads the -1 and unconditionally calls amdgpu_dm_audio_eld_notify() with the invalid pin: amdgpu_dm_commit_audio() { ... inst = aconnector->audio_inst; aconnector->audio_inst = -1; mutex_unlock(&adev->dm.audio_lock); amdgpu_dm_audio_eld_notify(adev, inst); } Does this violate the cross-subsystem API by logging a bogus debug message and passing an invalid index to the HDA driver? > } > > update_subconnector_property(aconnector); -- Sashiko AI review · https://sashiko.dev/#/patchset/20260626-amdgpu-dm-always-eld-notify-on-disconnect-v1-1-40ce473cd...@collabora.com?part=1
