On Wed, 6 Mar 2024 at 21:50, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > There are cases where the userspace might still send another > frame after the HPD disconnect causing a modeset cycle after > a disconnect. This messes the internal state machine of MSM DP driver > and can lead to a crash as there can be an imbalance between > bridge_disable() and bridge_enable(). > > This was also previously reported on [1] for which [2] was posted > and helped resolve the issue by rejecting commits if the DP is not > in connected state. > > The change resolved the bug but there can also be another race condition. > If hpd_event_thread does not pick up the EV_USER_NOTIFICATION and process it > link_ready will also not be set to false allowing the frame to sneak in. > > Lets move setting link_ready outside of hpd_event_thread() processing to > eliminate a window of race condition. > > [1] : https://gitlab.freedesktop.org/drm/msm/-/issues/17 > [2] : https://lore.kernel.org/all/1664408211-25314-1-git-send-email-quic_khsieh@xxxxxxxxxxx/ > > Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> > --- > drivers/gpu/drm/msm/dp/dp_display.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c > index 068d44eeaa07..e00092904ccc 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -345,8 +345,6 @@ static int dp_display_send_hpd_notification(struct dp_display_private *dp, > dp->panel->downstream_ports); > } > > - dp->dp_display.link_ready = hpd; > - > drm_dbg_dp(dp->drm_dev, "type=%d hpd=%d\n", > dp->dp_display.connector_type, hpd); > drm_bridge_hpd_notify(bridge, dp->dp_display.link_ready); > @@ -399,6 +397,8 @@ static int dp_display_process_hpd_high(struct dp_display_private *dp) > goto end; > } > > + dp->dp_display.link_ready = true; Do we need any kind of locking now? > + > dp_add_event(dp, EV_USER_NOTIFICATION, true, 0); > > end: > @@ -466,6 +466,8 @@ static int dp_display_notify_disconnect(struct device *dev) > { > struct dp_display_private *dp = dev_get_dp_display_private(dev); > > + dp->dp_display.link_ready = false; > + > dp_add_event(dp, EV_USER_NOTIFICATION, false, 0); > > return 0; > @@ -487,6 +489,7 @@ static int dp_display_handle_port_status_changed(struct dp_display_private *dp) > drm_dbg_dp(dp->drm_dev, "sink count is zero, nothing to do\n"); > if (dp->hpd_state != ST_DISCONNECTED) { > dp->hpd_state = ST_DISCONNECT_PENDING; > + dp->dp_display.link_ready = false; > dp_add_event(dp, EV_USER_NOTIFICATION, false, 0); > } > } else { > -- > 2.34.1 > -- With best wishes Dmitry
