Commit e73ab00e authored by Daniel Vetter's avatar Daniel Vetter
Browse files

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
parent 2ab8c5f8

drivers/gpu/drm/drm_connector.c

+15 −5
Original line number Original line Diff line number Diff line
@@ -224,6 +224,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) 
		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); 
 */

 */

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) 
	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); 
 */

 */

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) 
	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);

include/drm/drm_connector.h

+15 −1
Original line number Original line Diff line number Diff line
@@ -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 { 
	 * 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 { 
 * @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 { 




	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 { 
	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 */

CRA Git | Maintained and supported by SUSTech CRA and CCSE