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

New issues:
- [High] Incomplete fix leaves eDP branch devices gated on sink_count and skips 
critical initialization.

Pre-existing issues:
- [High] PM runtime usage counter underflow on pm_runtime_resume_and_get 
failure.
--

commit 2e22ab220d8aa377241f1c1461ea7061493df07f
Author: Vivek Kumar <[email protected]>

    drm/msm/dp: don't gate eDP on DP_SINK_COUNT
    
    msm_dp_display_prepare_link(), msm_dp_display_disable(), and
    msm_dp_hpd_unplug_handle() all treat dp->link->sink_count == 0 as
    "nothing is connected", and in msm_dp_display_prepare_link() this
    causes probe/enable to fail outright with -ENOTCONN.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index bc646d172abe0..9254f080f0ea6 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
[ ... ]
> @@ -634,7 +640,7 @@ static int msm_dp_display_prepare_link(struct 
> msm_dp_display_private *dp)
>               return rc;
>       }

[Severity: High]
This is a pre-existing issue, but can this early return cause a PM runtime
usage counter underflow?

If pm_runtime_resume_and_get() fails here in msm_dp_display_prepare_link(), it
returns a negative error and drops the reference. This failure propagates back,
and eventually the DRM core calls msm_dp_display_atomic_post_disable(), which
unconditionally executes msm_dp_display_unprepare() and calls
pm_runtime_put_sync(), decrementing a counter that was never incremented.

> -     if (dp->link->sink_count == 0)
> +     if (msm_dp_link_no_sink(dp))

[Severity: High]
Does this fix miss the eDP sink count exemption in 
msm_dp_panel_read_link_caps(),
leaving eDP branch devices gated and causing critical initialization to be
skipped?

When an eDP branch device with a 0 sink_count is connected,
msm_dp_panel_read_link_caps() in dp_panel.c still aborts:

                count = drm_dp_read_sink_count(panel->aux);
                if (!count) {
                        panel->link->sink_count = 0;
                        return -ENOTCONN;
                }

Because this failure propagates back to msm_dp_display_process_hpd_high(), it
skips EDID reading and PSM link configuration:

        rc = msm_dp_panel_read_link_caps(dp->panel, connector);
        if (rc)
                goto end;

Then, earlier in msm_dp_display_prepare_link(), the return value of
msm_dp_hpd_plug_handle() (which calls msm_dp_display_process_hpd_high) is
ignored for eDP:

        if (msm_dp_display->is_edp)
                msm_dp_hpd_plug_handle(dp);

As a result, the driver proceeds to use the display in an uninitialized and
degraded state.

>               return -ENOTCONN;
>  
>       if (!msm_dp_display->power_on) {

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

Reply via email to