Quoting Kuogee Hsieh (2022-04-25 15:29:30) > > On 4/20/2022 3:38 PM, Stephen Boyd wrote: > > Quoting Kuogee Hsieh (2022-04-14 14:03:43) > > > >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > >> index 01453db..f5bd8f5 100644 > >> --- a/drivers/gpu/drm/msm/dp/dp_display.c > >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c > >> @@ -615,24 +598,21 @@ static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) > >> if (dp->link->sink_count == 0) { > >> dp_display_host_phy_exit(dp); > >> } > >> + dp_display_notify_disconnect(&dp->pdev->dev); > >> mutex_unlock(&dp->event_mutex); > >> return 0; > >> - } > >> - > >> - if (state == ST_DISCONNECT_PENDING) { > >> + } else if (state == ST_DISCONNECT_PENDING) { > >> mutex_unlock(&dp->event_mutex); > >> return 0; > >> - } > >> - > >> - if (state == ST_CONNECT_PENDING) { > >> - /* wait until CONNECTED */ > >> - dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 1); /* delay = 1 */ > >> + } else if (state == ST_CONNECT_PENDING) { > > I take it that ST_CONNECT_PENDING is sort of like "userspace hasn't > > handled the uevent yet and modeset hasn't been called but the link is > > setup and now we want to tear it down". The state name may want to be > > changed to something else. > yes, how about change to ST_MAINLINK_READY? Sure. > >> @@ -1529,8 +1480,11 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder) > >> > >> mutex_lock(&dp_display->event_mutex); > >> > >> - /* stop sentinel checking */ > >> - dp_del_event(dp_display, EV_CONNECT_PENDING_TIMEOUT); > >> + state = dp_display->hpd_state; > >> + if (state != ST_DISPLAY_OFF && state != ST_CONNECT_PENDING) { > > Is this to guard against userspace doing an atomic commit when the > > display isn't connected? Is that even possible? > > No, it used to guard follow scenario in timing order, > > 1) plugin had been handled and mainlink is ready, > > 2) userspace hasn't handled the uevent yet and modeset hasn't been called > > 3) unplugged happen, mainlink be teared down > > 4) user space start to response to uevent and try to enable display. > (it too late since mainlink had been teared down) > Ok. Thanks for clarifying.