Re: [PATCH v3] Return only active connectors for get_resources ioctl

[Lyude Paul <lyude@redhat.com>]

On Thu, 2018-11-29 at 13:09 +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
> 
> v3: Refactored initial condition to look more compact.
>     Thanks to Chris Wilson for giving a hint.
> 
> 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 | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c
> b/drivers/gpu/drm/drm_mode_config.c
> index ee80788f2c40..3e2cd959a66a 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -143,22 +143,28 @@ 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 &&
>  		    (connector->connector_type ==
> DRM_MODE_CONNECTOR_WRITEBACK))
>  			continue;
>  
> +		if (READ_ONCE(connector->registration_state) !=
> DRM_CONNECTOR_REGISTERED)
> +			continue;
> +
This conditional should just use drm_connector_is_unregistered() instead I'm
fairly sure.

>  		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;
>  			}
> +			DRM_DEBUG_KMS("GetResources: connector %s", connector-
> >name);
>  			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;
-- 
Cheers,
	Lyude Paul