Re: [Intel-gfx] [PATCH v2] Return only active connectors for get_resources ioctl

[Daniel Vetter <daniel@ffwll.ch>]

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

>  			}
> -			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;
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch