On Wed, Nov 28, 2018 at 10:21:33PM +0100, Daniel Vetter wrote: > On Wed, Nov 28, 2018 at 09:51:13PM +0100, Daniel Vetter wrote: > > On Wed, Nov 28, 2018 at 03:55:58PM +0200, Stanislav Lisovskiy wrote: > > > Currently kernel might allocate different connector ids > > > for the same outputs in case of DP MST, which seems to > > > confuse userspace. There are can be different connector > > > ids in the list, which could be assigned to the same > > > output, while being in different states. > > > This results in issues, like external displays staying > > > blank after quick unplugging and plugging back(bug #106250). > > > Returning only active DP connectors fixes the issue. > > > > > > v2: Removed caps from the title > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106250 > > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx> > > > --- > > > drivers/gpu/drm/drm_mode_config.c | 16 +++++++++++----- > > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c > > > index ee80788f2c40..ec5b2b08a45e 100644 > > > --- a/drivers/gpu/drm/drm_mode_config.c > > > +++ b/drivers/gpu/drm/drm_mode_config.c > > > @@ -143,6 +143,7 @@ int drm_mode_getresources(struct drm_device *dev, void *data, > > > drm_connector_list_iter_begin(dev, &conn_iter); > > > count = 0; > > > connector_id = u64_to_user_ptr(card_res->connector_id_ptr); > > > + DRM_DEBUG_KMS("GetResources: writing connectors start"); > > > drm_for_each_connector_iter(connector, &conn_iter) { > > > /* only expose writeback connectors if userspace understands them */ > > > if (!file_priv->writeback_connectors && > > > @@ -150,15 +151,20 @@ int drm_mode_getresources(struct drm_device *dev, void *data, > > > continue; > > > > > > if (drm_lease_held(file_priv, connector->base.id)) { > > > - if (count < card_res->count_connectors && > > > - put_user(connector->base.id, connector_id + count)) { > > > - drm_connector_list_iter_end(&conn_iter); > > > - return -EFAULT; > > > + if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort || > > > + connector->status != connector_status_disconnected) { > > > + if (count < card_res->count_connectors && > > > + put_user(connector->base.id, connector_id + count)) { > > > + drm_connector_list_iter_end(&conn_iter); > > > + return -EFAULT; > > > + } > > > + DRM_DEBUG_KMS("GetResources: connector %s", connector->name); > > > + count++; > > > > I tried to read the bug and I have no idea what's going on here. Userspace > > is supposed to shut off outputs that are disconnected, whether that's DP, > > DP MST or something else shouldn't matter. New connectors can come&go as > > they see fit. Also not really something special. > > > > Why do we need to dynamically hide an output here? Note that this also > > affects normal DP ports, which I have no idea is actually what you want to > > do or not. > > For entertainment and other reasons, testing the below diff would be > interesting. Now also with real locking: diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index 4de247ddf05f..1b83567aa922 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -494,11 +494,16 @@ static struct drm_connector *intel_dp_add_mst_connector(struct drm_dp_mst_topolo static void intel_dp_register_mst_connector(struct drm_connector *connector) { struct drm_i915_private *dev_priv = to_i915(connector->dev); + struct drm_mode_config *config = &connector->dev->mode_config; if (dev_priv->fbdev) drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper, connector); + spin_lock_irq(&config->connector_list_lock); + list_move(&connector->head, &config->connector_list); + spin_unlock_irq(&config->connector_list_lock); + drm_connector_register(connector); } -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
