Return only active connectors for GET_RESOURCES

Submitted by Stanislav Lisovskiy on Nov. 28, 2018, 1:52 p.m.

Details

Message ID 20181128135210.2939-1-stanislav.lisovskiy@intel.com
State New
Headers show
Series "Return only active connectors for GET_RESOURCES" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Stanislav Lisovskiy Nov. 28, 2018, 1:52 p.m.
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.

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(-)

Patch hide | download patch | download mbox

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++;
 			}
-			count++;
 		}
 	}
 	card_res->count_connectors = count;
+	DRM_DEBUG_KMS("GetResources: writing connectors end - count %d", count);
 	drm_connector_list_iter_end(&conn_iter);
 
 	return ret;

Comments

Quoting Stanislav Lisovskiy (2018-11-28 13:52:10)
> 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.
> 
> 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 (READ_ONCE(connector->registration_state) != DRM_CONNECTOR_REGISTERED)
	continue;
>  
>                 if (drm_lease_held(file_priv, connector->base.id)) {
On Wed, 2018-11-28 at 14:39 +0000, Chris Wilson wrote:
> Quoting Stanislav Lisovskiy (2018-11-28 13:52:10)

> > 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.

> > 

> > 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 (READ_ONCE(connector->registration_state) !=

> DRM_CONNECTOR_REGISTERED)

> 	continue;


Seems to work. This surely looks more compact, however my concern is
that can this cause some regression for other connector types?
That's why I was checking, that this is a DP connector, so that this
doesn't touch other connector types at least.

> >  

> >                 if (drm_lease_held(file_priv, connector->base.id))

> > {





> 

> _______________________________________________

> dri-devel mailing list

> dri-devel@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Best Regards,

Lisovskiy Stanislav