| Message ID | 20250529-hpd_display_off-v1-2-ce33bac2987c@oss.qualcomm.com |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm/dp: ST_DISPLAY_OFF hpd cleanup | expand |
On 2 June 2025 18:54:12 BST, Jessica Zhang <jessica.zhang@oss.qualcomm.com> wrote: > > >On 5/30/2025 9:04 AM, Dmitry Baryshkov wrote: >> On Fri, 30 May 2025 at 02:15, Jessica Zhang >> <jessica.zhang@oss.qualcomm.com> wrote: >>> >>> From: Abhinav Kumar <quic_abhinavk@quicinc.com> >>> >>> The checks in msm_dp_bridge_atomic_enable() for making sure that we are in >>> ST_DISPLAY_OFF OR ST_MAINLINK_READY seem redundant. >>> >>> DRM fwk shall not issue any commits if state is not ST_MAINLINK_READY as >>> msm_dp's atomic_check callback returns a failure if state is not >>> ST_MAINLINK_READY. >> >> What if the state changes between atomic_check() and atomic_enable()? >> There are no locks, cable unplugging is async, so it's perfectly >> possible. >> >>> >>> For the ST_DISPLAY_OFF check, its mainly to guard against a scenario that >>> there is an atomic_enable() without a prior atomic_disable() which once >>> again should not really happen. >>> >>> Since it's still possible for the state machine to transition to >>> ST_DISCONNECT_PENDING between atomic_check() and atomic_commit(), change >>> this check to return early if hpd_state is ST_DISCONNECT_PENDING. >> >> Can we really, please, drop the state machine? I had other plans for >> the next week, but maybe I should just do it, so that by the end of >> 6.17 cycle we can have a merged, stable and working solution? This >> topic has been lingering for months without any actual change. > >FWIW, I'm currently working on the state machine rework by the middle of next week. > >I'm anticipating that the rework itself will take some time to get merged, so didn't want MST to get delayed more by this series. Yes, that's fine. I really want HPD to be merged before MST. And if I wasn't explicit, the state machine must be gone. Link training should happen from atomic_enable, detect should be reporting whether there is an actual display plugged, etc. Current code must be dropped. > >Thanks, > >Jessica Zhang > >> >>> >>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com> >>> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com> >>> --- >>> drivers/gpu/drm/msm/dp/dp_display.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c >>> index 1d7cda62d5fb..f2820f06f5dc 100644 >>> --- a/drivers/gpu/drm/msm/dp/dp_display.c >>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c >>> @@ -1512,7 +1512,7 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge, >>> } >>> >>> hpd_state = msm_dp_display->hpd_state; >>> - if (hpd_state != ST_DISPLAY_OFF && hpd_state != ST_MAINLINK_READY) { >>> + if (hpd_state == ST_DISCONNECT_PENDING) { >> >> >> >>> mutex_unlock(&msm_dp_display->event_mutex); >>> return; >>> } >>> >>> -- >>> 2.49.0 >>> >> >> > With best wishes, Dmitry
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 1d7cda62d5fb..f2820f06f5dc 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1512,7 +1512,7 @@ void msm_dp_bridge_atomic_enable(struct drm_bridge *drm_bridge, } hpd_state = msm_dp_display->hpd_state; - if (hpd_state != ST_DISPLAY_OFF && hpd_state != ST_MAINLINK_READY) { + if (hpd_state == ST_DISCONNECT_PENDING) { mutex_unlock(&msm_dp_display->event_mutex); return; }