[Intel-gfx] [PATCH 08/13] drm: prevent double-(un)registration for connectors

[Chris Wilson]

On Wed, Dec 14, 2016 at 12:08:09AM +0100, Daniel Vetter wrote:
> If we're unlucky then the registration from a hotplugged connector
> might race with the final registration step on driver load. And since
> MST topology discover is asynchronous that's even somewhat likely.
> 
> v2: Also update the kerneldoc for @registered!
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Reported-by: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 18 +++++++++++++-----
>  include/drm/drm_connector.h     | 12 +++++++++++-
>  2 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b33334e09b00..0d4728704099 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -223,6 +223,7 @@ int drm_connector_init(struct drm_device *dev,
>  
>  	INIT_LIST_HEAD(&connector->probed_modes);
>  	INIT_LIST_HEAD(&connector->modes);
> +	mutex_init(&connector->mutex);

There's a mutex_destroy() for drm_connector_free() for that extra bit of
debug icing.

> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 0e41a2e184a9..a24559ef8bb7 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -559,7 +559,6 @@ struct drm_cmdline_mode {
>   * @interlace_allowed: can this connector handle interlaced modes?
>   * @doublescan_allowed: can this connector handle doublescan?
>   * @stereo_allowed: can this connector handle stereo modes?
> - * @registered: is this connector exposed (registered) with userspace?
>   * @modes: modes available on this connector (from fill_modes() + user)
>   * @status: one of the drm_connector_status enums (connected, not, or unknown)
>   * @probed_modes: list of modes derived directly from the display
> @@ -608,6 +607,13 @@ struct drm_connector {
>  	char *name;
>  
>  	/**
> +	 * @mutex: Lock for general connector state, but currently only protects
> +	 * @registered. Most of the connector state is still protected by the
> +	 * mutex in &drm_mode_config.
> +	 */
> +	struct mutex mutex;
> +
> +	/**
>  	 * @index: Compacted connector index, which matches the position inside
>  	 * the mode_config.list for drivers not supporting hot-add/removing. Can
>  	 * be used as an array index. It is invariant over the lifetime of the
> @@ -620,6 +626,10 @@ struct drm_connector {
>  	bool interlace_allowed;
>  	bool doublescan_allowed;
>  	bool stereo_allowed;
> +	/**
> +	 * @registered: Is this connector exposed (registered) with userspace?
> +	 * Protected by @mutex.

early_unregister, late_register should also be documented as being
called under connector->mutex.

With those minor improvements,
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre