| Message ID | 1416418691-24747-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Nov 19, 2014 at 12:38 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > 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> Reviewed-by: Rob Clark <robdclark@gmail.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: > > return ret; > backoff: > - drm_atomic_legacy_backoff(state); > drm_atomic_state_clear(state); > + drm_atomic_legacy_backoff(state); > > /* > * Someone might have exchanged the framebuffer while we dropped locks > @@ -1285,8 +1285,8 @@ fail: > > return ret; > backoff: > - drm_atomic_legacy_backoff(state); > drm_atomic_state_clear(state); > + drm_atomic_legacy_backoff(state); > > /* > * Someone might have exchanged the framebuffer while we dropped locks > @@ -1462,8 +1462,8 @@ fail: > > return ret; > backoff: > - drm_atomic_legacy_backoff(state); > drm_atomic_state_clear(state); > + drm_atomic_legacy_backoff(state); > > /* > * Someone might have exchanged the framebuffer while we dropped locks > @@ -1528,8 +1528,8 @@ fail: > > return ret; > backoff: > - drm_atomic_legacy_backoff(state); > drm_atomic_state_clear(state); > + drm_atomic_legacy_backoff(state); > > goto retry; > } > @@ -1587,8 +1587,8 @@ fail: > > return ret; > backoff: > - drm_atomic_legacy_backoff(state); > drm_atomic_state_clear(state); > + drm_atomic_legacy_backoff(state); > > goto retry; > } > @@ -1646,8 +1646,8 @@ fail: > > return ret; > backoff: > - drm_atomic_legacy_backoff(state); > drm_atomic_state_clear(state); > + drm_atomic_legacy_backoff(state); > > goto retry; > } > @@ -1725,8 +1725,8 @@ fail: > > return ret; > backoff: > - drm_atomic_legacy_backoff(state); > drm_atomic_state_clear(state); > + drm_atomic_legacy_backoff(state); > > /* > * Someone might have exchanged the framebuffer while we dropped locks > -- > 2.1.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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: return ret; backoff: - drm_atomic_legacy_backoff(state); drm_atomic_state_clear(state); + drm_atomic_legacy_backoff(state); /* * Someone might have exchanged the framebuffer while we dropped locks @@ -1285,8 +1285,8 @@ fail: return ret; backoff: - drm_atomic_legacy_backoff(state); drm_atomic_state_clear(state); + drm_atomic_legacy_backoff(state); /* * Someone might have exchanged the framebuffer while we dropped locks @@ -1462,8 +1462,8 @@ fail: return ret; backoff: - drm_atomic_legacy_backoff(state); drm_atomic_state_clear(state); + drm_atomic_legacy_backoff(state); /* * Someone might have exchanged the framebuffer while we dropped locks @@ -1528,8 +1528,8 @@ fail: return ret; backoff: - drm_atomic_legacy_backoff(state); drm_atomic_state_clear(state); + drm_atomic_legacy_backoff(state); goto retry; } @@ -1587,8 +1587,8 @@ fail: return ret; backoff: - drm_atomic_legacy_backoff(state); drm_atomic_state_clear(state); + drm_atomic_legacy_backoff(state); goto retry; } @@ -1646,8 +1646,8 @@ fail: return ret; backoff: - drm_atomic_legacy_backoff(state); drm_atomic_state_clear(state); + drm_atomic_legacy_backoff(state); goto retry; } @@ -1725,8 +1725,8 @@ fail: return ret; backoff: - drm_atomic_legacy_backoff(state); drm_atomic_state_clear(state); + drm_atomic_legacy_backoff(state); /* * Someone might have exchanged the framebuffer while we dropped locks
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(-)