Commit 4e5b54f1 authored by Daniel Vetter's avatar Daniel Vetter

drm: prevent double-(un)registration for connectors

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!

v3: Review from Chris:
- Improve kerneldoc for late_register/early_unregister callbacks.
- Use mutex_destroy.
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarSean Paul <seanpaul@chromium.org>
Reported-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20161218133545.2106-1-daniel.vetter@ffwll.ch
(cherry picked from commit e73ab00e)
parent 566cf877
...@@ -225,6 +225,7 @@ int drm_connector_init(struct drm_device *dev, ...@@ -225,6 +225,7 @@ int drm_connector_init(struct drm_device *dev,
INIT_LIST_HEAD(&connector->probed_modes); INIT_LIST_HEAD(&connector->probed_modes);
INIT_LIST_HEAD(&connector->modes); INIT_LIST_HEAD(&connector->modes);
mutex_init(&connector->mutex);
connector->edid_blob_ptr = NULL; connector->edid_blob_ptr = NULL;
connector->status = connector_status_unknown; connector->status = connector_status_unknown;
...@@ -359,6 +360,8 @@ void drm_connector_cleanup(struct drm_connector *connector) ...@@ -359,6 +360,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
connector->funcs->atomic_destroy_state(connector, connector->funcs->atomic_destroy_state(connector,
connector->state); connector->state);
mutex_destroy(&connector->mutex);
memset(connector, 0, sizeof(*connector)); memset(connector, 0, sizeof(*connector));
} }
EXPORT_SYMBOL(drm_connector_cleanup); EXPORT_SYMBOL(drm_connector_cleanup);
...@@ -374,14 +377,15 @@ EXPORT_SYMBOL(drm_connector_cleanup); ...@@ -374,14 +377,15 @@ EXPORT_SYMBOL(drm_connector_cleanup);
*/ */
int drm_connector_register(struct drm_connector *connector) int drm_connector_register(struct drm_connector *connector)
{ {
int ret; int ret = 0;
mutex_lock(&connector->mutex);
if (connector->registered) if (connector->registered)
return 0; goto unlock;
ret = drm_sysfs_connector_add(connector); ret = drm_sysfs_connector_add(connector);
if (ret) if (ret)
return ret; goto unlock;
ret = drm_debugfs_connector_add(connector); ret = drm_debugfs_connector_add(connector);
if (ret) { if (ret) {
...@@ -397,12 +401,14 @@ int drm_connector_register(struct drm_connector *connector) ...@@ -397,12 +401,14 @@ int drm_connector_register(struct drm_connector *connector)
drm_mode_object_register(connector->dev, &connector->base); drm_mode_object_register(connector->dev, &connector->base);
connector->registered = true; connector->registered = true;
return 0; goto unlock;
err_debugfs: err_debugfs:
drm_debugfs_connector_remove(connector); drm_debugfs_connector_remove(connector);
err_sysfs: err_sysfs:
drm_sysfs_connector_remove(connector); drm_sysfs_connector_remove(connector);
unlock:
mutex_unlock(&connector->mutex);
return ret; return ret;
} }
EXPORT_SYMBOL(drm_connector_register); EXPORT_SYMBOL(drm_connector_register);
...@@ -415,8 +421,11 @@ EXPORT_SYMBOL(drm_connector_register); ...@@ -415,8 +421,11 @@ EXPORT_SYMBOL(drm_connector_register);
*/ */
void drm_connector_unregister(struct drm_connector *connector) void drm_connector_unregister(struct drm_connector *connector)
{ {
if (!connector->registered) mutex_lock(&connector->mutex);
if (!connector->registered) {
mutex_unlock(&connector->mutex);
return; return;
}
if (connector->funcs->early_unregister) if (connector->funcs->early_unregister)
connector->funcs->early_unregister(connector); connector->funcs->early_unregister(connector);
...@@ -425,6 +434,7 @@ void drm_connector_unregister(struct drm_connector *connector) ...@@ -425,6 +434,7 @@ void drm_connector_unregister(struct drm_connector *connector)
drm_debugfs_connector_remove(connector); drm_debugfs_connector_remove(connector);
connector->registered = false; connector->registered = false;
mutex_unlock(&connector->mutex);
} }
EXPORT_SYMBOL(drm_connector_unregister); EXPORT_SYMBOL(drm_connector_unregister);
......
...@@ -381,6 +381,8 @@ struct drm_connector_funcs { ...@@ -381,6 +381,8 @@ struct drm_connector_funcs {
* core drm connector interfaces. Everything added from this callback * core drm connector interfaces. Everything added from this callback
* should be unregistered in the early_unregister callback. * should be unregistered in the early_unregister callback.
* *
* This is called while holding drm_connector->mutex.
*
* Returns: * Returns:
* *
* 0 on success, or a negative error code on failure. * 0 on success, or a negative error code on failure.
...@@ -395,6 +397,8 @@ struct drm_connector_funcs { ...@@ -395,6 +397,8 @@ struct drm_connector_funcs {
* late_register(). It is called from drm_connector_unregister(), * late_register(). It is called from drm_connector_unregister(),
* early in the driver unload sequence to disable userspace access * early in the driver unload sequence to disable userspace access
* before data structures are torndown. * before data structures are torndown.
*
* This is called while holding drm_connector->mutex.
*/ */
void (*early_unregister)(struct drm_connector *connector); void (*early_unregister)(struct drm_connector *connector);
...@@ -559,7 +563,6 @@ struct drm_cmdline_mode { ...@@ -559,7 +563,6 @@ struct drm_cmdline_mode {
* @interlace_allowed: can this connector handle interlaced modes? * @interlace_allowed: can this connector handle interlaced modes?
* @doublescan_allowed: can this connector handle doublescan? * @doublescan_allowed: can this connector handle doublescan?
* @stereo_allowed: can this connector handle stereo modes? * @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) * @modes: modes available on this connector (from fill_modes() + user)
* @status: one of the drm_connector_status enums (connected, not, or unknown) * @status: one of the drm_connector_status enums (connected, not, or unknown)
* @probed_modes: list of modes derived directly from the display * @probed_modes: list of modes derived directly from the display
...@@ -607,6 +610,13 @@ struct drm_connector { ...@@ -607,6 +610,13 @@ struct drm_connector {
char *name; 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 * @index: Compacted connector index, which matches the position inside
* the mode_config.list for drivers not supporting hot-add/removing. Can * the mode_config.list for drivers not supporting hot-add/removing. Can
...@@ -620,6 +630,10 @@ struct drm_connector { ...@@ -620,6 +630,10 @@ struct drm_connector {
bool interlace_allowed; bool interlace_allowed;
bool doublescan_allowed; bool doublescan_allowed;
bool stereo_allowed; bool stereo_allowed;
/**
* @registered: Is this connector exposed (registered) with userspace?
* Protected by @mutex.
*/
bool registered; bool registered;
struct list_head modes; /* list of modes on this connector */ struct list_head modes; /* list of modes on this connector */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment