On Thu, Aug 24, 2023 at 04:50:20PM -0400, Gil Dekel wrote: > Before sending a uevent to userspace in order to trigger a corrective > modeset, we change the failing connector's link-status to BAD. However, > the downstream MST branch ports are left in their original GOOD state. > > This patch utilizes the drm helper function > drm_dp_set_mst_topology_link_status() to rectify this and set all > downstream MST connectors' link-status to BAD before emitting the uevent > to userspace. > > Signed-off-by: Gil Dekel <gildekel@xxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_dp.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c > index 42353b1ac487..e8b10f59e141 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -5995,16 +5995,20 @@ static void intel_dp_modeset_retry_work_fn(struct work_struct *work) > struct intel_dp *intel_dp = > container_of(work, typeof(*intel_dp), modeset_retry_work); > struct drm_connector *connector = &intel_dp->attached_connector->base; > - drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s]\n", connector->base.id, > - connector->name); > > - /* Grab the locks before changing connector property*/ > - mutex_lock(&connector->dev->mode_config.mutex); > - /* Set connector link status to BAD and send a Uevent to notify > - * userspace to do a modeset. > + /* Set the connector's (and possibly all its downstream MST ports') link > + * status to BAD. > */ > + mutex_lock(&connector->dev->mode_config.mutex); > + drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] link status %d -> %d\n", > + connector->base.id, connector->name, > + connector->state->link_status, DRM_MODE_LINK_STATUS_BAD); > drm_connector_set_link_status_property(connector, > DRM_MODE_LINK_STATUS_BAD); > + if (intel_dp->is_mst) { > + drm_dp_set_mst_topology_link_status(&intel_dp->mst_mgr, > + DRM_MODE_LINK_STATUS_BAD); Something is weird with the locking here. I noticed that on patch 3 this new function also gets the same mutex_lock(&connector->dev->mode_config.mutex); Since you didn't reach the deadlock, I'm clearly missing something on the flow. But regardless of what I could be missing, I believe this is totally not future proof and we will for sure hit dead-lock cases. > + } > mutex_unlock(&connector->dev->mode_config.mutex); > /* Send Hotplug uevent so userspace can reprobe */ > drm_kms_helper_connector_hotplug_event(connector); > -- > Gil Dekel, Software Engineer, Google / ChromeOS Display and Graphics