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@intel.com
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); }