[PATCH] RFC drm: Do not leak connector_status_unknown to userspace

[Daniel Vetter]

On Sat, Jun 09, 2012 at 12:24:49AM +0100, Chris Wilson wrote:
> A detect() probe may return 'unknown' to indicate that it was not able
> to successfully determine the connector status. However, exposing this
> to userspace just results in lots of confusion as it is uncertain
> whether or not that is a valid connection. The usual result is for X to
> create a mode that encompasses the unknown connector causing an abnormal
> setup on the known good outputs.
> 
> This patch initialises the connector status to disconnected and
> thereafter ignores indeterminate results from detect(), leaving the
> status as the old value. This prevents the status from being set to
> unknown, and so leaked to userspace, and so leaves the connector as
> disconnected until we have a valid connection and also prevents that
> valid connection being dropped due to a failed probe.
> 
> The alternative to this is to never return unknown from the drivers, but
> return connector->status instead so that the value never changes on a
> failed probe (and also to initialise the connector to disconnected in
> the driver).

Hm, this is a bit at odds with how I've re-defined unknown in my hpd
rework. There it essentially means that that the kernel truely has no idea
what the status is because the world changed (i.e. resume or fresh boot)
and detect hasn't been run yet. So imo we should initialize the connector
state to unknown (and reset it to unknown after resume).

I agree with the 'not leaking unknown' to userspace, although I'm unsure
about the implications. I guess we just have to try and see what happens.

Yours, Daniel
> ---
>  drivers/gpu/drm/drm_crtc.c        |    1 +
>  drivers/gpu/drm/drm_crtc_helper.c |   20 +++++++++++++-------
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 08a7aa7..22f58b9 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -495,6 +495,7 @@ int drm_connector_init(struct drm_device *dev,
>  	INIT_LIST_HEAD(&connector->probed_modes);
>  	INIT_LIST_HEAD(&connector->modes);
>  	connector->edid_blob_ptr = NULL;
> +	connector->status = connector_status_disconnected;
>  
>  	list_add_tail(&connector->head, &dev->mode_config.connector_list);
>  	dev->mode_config.num_connector++;
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 3252e70..143d6d5 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -108,7 +108,11 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>  		if (connector->funcs->force)
>  			connector->funcs->force(connector);
>  	} else {
> -		connector->status = connector->funcs->detect(connector, true);
> +		enum drm_connector_status status;
> +
> +		status = connector->funcs->detect(connector, true);
> +		if (status)
> +			connector->status = status;
>  		drm_kms_helper_poll_enable(dev);
>  	}
>  
> @@ -949,13 +953,15 @@ static void output_poll_execute(struct work_struct *work)
>  		    !(connector->polled & DRM_CONNECTOR_POLL_HPD))
>  			continue;
>  
> -		connector->status = connector->funcs->detect(connector, false);
> -		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
> -			      connector->base.id,
> -			      drm_get_connector_name(connector),
> -			      old_status, connector->status);
> -		if (old_status != connector->status)
> +		new_status = connector->funcs->detect(connector, false);
> +		if (new_status && old_status != new_status) {
> +			DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
> +				      connector->base.id,
> +				      drm_get_connector_name(connector),
> +				      old_status, new_status);
> +			connector->status = new_status;
>  			changed = true;
> +		}
>  	}
>  
>  	mutex_unlock(&dev->mode_config.mutex);
> -- 
> 1.7.10
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48