[PATCH 1/6] drm/atomic: Ensure that drm_connector_index is stable

List overview All Threads
Download

newer

older

[PATCH] drm/atomic: Add missing...

[PATCH 1/1] DRM-UDL: Deletion of...

Daniel Vetter

19 Nov 2014 19 Nov '14

5:38 p.m.

I've totally forgotten that with DP MST connectors can now be hotplugged. And failed to adapt Rob's drm_atomic_state code (which predates connector hotplugging) to the new realities.

The first step is to make sure that the connector indices used to access the arrays of pointers are stable. The connection mutex gives us enough guarantees for that, which means we won't unecessarily block on concurrent modesets or background probing.

So add a locking WARN_ON and shuffle the code slightly to make sure we always hold the right lock.

Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_atomic.c | 8 ++++---- drivers/gpu/drm/drm_crtc.c | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index ed22a719440f..90b2d1644bd7 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -295,15 +295,15 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state, struct drm_mode_config *config = &connector->dev->mode_config; struct drm_connector_state *connector_state;

+ ret = drm_modeset_lock(&config->connection_mutex, state->acquire_ctx); + if (ret) + return ERR_PTR(ret); + index = drm_connector_index(connector);

if (state->connector_states[index]) return state->connector_states[index];

- ret = drm_modeset_lock(&config->connection_mutex, state->acquire_ctx); - if (ret) - return ERR_PTR(ret); - connector_state = connector->funcs->atomic_duplicate_state(connector); if (!connector_state) return ERR_PTR(-ENOMEM); diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 56737e74b59d..5c878f172365 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -867,6 +867,8 @@ int drm_connector_init(struct drm_device *dev,

drm_connector_get_cmdline_mode(connector);

+ /* We should add connectors at the end to avoid upsetting the connector + * index too much. */ list_add_tail(&connector->head, &dev->mode_config.connector_list); dev->mode_config.num_connector++;

@@ -930,6 +932,9 @@ unsigned int drm_connector_index(struct drm_connector *connector) { unsigned int index = 0; struct drm_connector *tmp; + struct drm_mode_config *config = &connector->dev->mode_config; + + WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));

list_for_each_entry(tmp, &connector->dev->mode_config.connector_list, head) { if (tmp == connector)

-- 2.1.1

Show replies by date

Daniel Vetter

19 Nov 19 Nov

5:38 p.m.

New subject: [PATCH 2/6] drm/atomic: Only destroy connector states with connection mutex held

Otherwise the connector might have been unplugged and destroyed while we didn't look. Yet another fallout from DP MST hotplugging that I didn't consider.

To make sure we get this right add an appropriate WARN_ON to drm_atomic_state_clear (obviously only when we actually have a state to clear up). And reorder all the state_clear and backoff calls to make it work out properly.

Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_atomic.c | 9 ++++++--- drivers/gpu/drm/drm_atomic_helper.c | 14 +++++++------- 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 90b2d1644bd7..67c1dc894bd9 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -112,21 +112,24 @@ EXPORT_SYMBOL(drm_atomic_state_alloc); void drm_atomic_state_clear(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; + struct drm_mode_config *config = &dev->mode_config; int i;

DRM_DEBUG_KMS("Clearing atomic state %p\n", state);

- for (i = 0; i < dev->mode_config.num_connector; i++) { + for (i = 0; i < config->num_connector; i++) { struct drm_connector *connector = state->connectors[i];

if (!connector) continue;

+ WARN_ON(!drm_modeset_is_locked(&config->connection_mutex)); + connector->funcs->atomic_destroy_state(connector, state->connector_states[i]); }

- for (i = 0; i < dev->mode_config.num_crtc; i++) { + for (i = 0; i < config->num_crtc; i++) { struct drm_crtc *crtc = state->crtcs[i];

if (!crtc) @@ -136,7 +139,7 @@ void drm_atomic_state_clear(struct drm_atomic_state *state) state->crtc_states[i]); }

- for (i = 0; i < dev->mode_config.num_total_plane; i++) { + for (i = 0; i < config->num_total_plane; i++) { struct drm_plane *plane = state->planes[i];

if (!plane) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index fad2b932cf72..0cd054615920 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1217,8 +1217,8 @@ fail: