Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [Medium] The `EDID_NO_RESPONSE` error path for HDMI/DVI connectors returns 
`false` without calling `link_disconnect_sink()`, leaving `link->local_sink` 
populated and leading to a false-positive connection state on the next poll.
--

commit c7392818ef1ec0f54b3aa58060b607f2f261bed1
Author: WenTao Liang <[email protected]>

fix: drm/amd/display: detect_link_and_local_sink: DP alt mode timeout path 
leaks prev_sink reference

prev_sink is unconditionally retained via dc_sink_retain at function
entry, but the DP alt mode timeout path inside SIGNAL_TYPE_DISPLAY_PORT
returns false without releasing prev_sink. All other return paths in the
function correctly call dc_sink_release(prev_sink), making this the only
missing cleanup.

> diff --git a/drivers/gpu/drm/amd/display/dc/link/link_detection.c 
> b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
> index a3212fd151d16..7d8951fecd570 100644
> --- a/drivers/gpu/drm/amd/display/dc/link/link_detection.c
> +++ b/drivers/gpu/drm/amd/display/dc/link/link_detection.c
> @@ -1164,8 +1164,11 @@ static bool detect_link_and_local_sink(struct dc_link 
> *link,
>                           link->link_enc->features.flags.bits.DP_IS_USB_C == 
> 1) {
>  
>                               /* if alt mode times out, return false */
> -                             if (!wait_for_entering_dp_alt_mode(link))
> +                             if (!wait_for_entering_dp_alt_mode(link)) {
> +                                     if (prev_sink)
> +                                             dc_sink_release(prev_sink);
>                                       return false;
> +                             }
>                       }

[Severity: Medium]
This isn't a bug introduced by this patch, but there is a pre-existing
issue further down in detect_link_and_local_sink() related to sink cleanup
when EDID reads fail.

When an HDMI or DVI device asserts HPD but fails DDC/EDID reads, the
code handles the EDID_NO_RESPONSE case like this:

drivers/gpu/drm/amd/display/dc/link/link_detection.c:detect_link_and_local_sink()
 {
    ...
                        if (dc_is_hdmi_signal(link->connector_signal) ||
                            dc_is_dvi_signal(link->connector_signal)) {
                                if (prev_sink)
                                        dc_sink_release(prev_sink);

                                return false;
                        }
    ...
}

Does this error path need to call link_disconnect_sink(link) before
returning false?

It appears that link->local_sink is populated earlier in this function
when dc_sink_create() returns a new reference. By returning false without
disconnecting, link->local_sink remains populated.

Could this lead to a false-positive connection state on the next poll,
where amdgpu_dm_connector_detect() sees link->local_sink != NULL and
short-circuits to report the connector as connected, potentially causing
the compositor to attempt modesetting on an invalid, partially-initialized
sink with no EDID?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to