Commit ce87ea15 authored by Daniel Vetter's avatar Daniel Vetter

drm/i915: Unbreak gpu reset vs. modeset locking

Taking the modeset locks unconditionally isn't the greatest idea,
because atm that part is still broken and times out (and then atomic
keels over). And there's really no reason to do so, the old code
didn't do that either.

To make the patch a bit simpler let's also nuke 2 cases that are only
around for the old mmioflip paths. Atomic nonblocking workers will not
die (minus bugs) when a gpu reset happens.

And of course this doesn't fix any of the gpu reset vs. modeset
deadlock fun, but it at least stop modern CI machines from keeling
over all over the place for no reason at all.

And we still have the explicit testcases to run the fake gpu reset, so
coverage isn't that much worse.

v2: Split out additional changes on top, restrict this to purely reducing
the critical section of modeset locks.

v2: Review from Maarten
- update comments
- don't oops when state is NULL in intel_finish_reset, but try to at
  least still drop locks properly. The hw is going to be toast anyway.

Fixes: 73974893 ("drm/i915: Fix modeset handling during gpu reset, v5.")
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170719125502.25696-3-daniel.vetter@ffwll.ch
parent afa8ce5b
...@@ -3413,26 +3413,6 @@ static void intel_complete_page_flips(struct drm_i915_private *dev_priv) ...@@ -3413,26 +3413,6 @@ static void intel_complete_page_flips(struct drm_i915_private *dev_priv)
intel_finish_page_flip_cs(dev_priv, crtc->pipe); intel_finish_page_flip_cs(dev_priv, crtc->pipe);
} }
static void intel_update_primary_planes(struct drm_device *dev)
{
struct drm_crtc *crtc;
for_each_crtc(dev, crtc) {
struct intel_plane *plane = to_intel_plane(crtc->primary);
struct intel_plane_state *plane_state =
to_intel_plane_state(plane->base.state);
if (plane_state->base.visible) {
trace_intel_update_plane(&plane->base,
to_intel_crtc(crtc));
plane->update_plane(plane,
to_intel_crtc_state(crtc->state),
plane_state);
}
}
}
static int static int
__intel_display_resume(struct drm_device *dev, __intel_display_resume(struct drm_device *dev,
struct drm_atomic_state *state, struct drm_atomic_state *state,
...@@ -3485,6 +3465,12 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv) ...@@ -3485,6 +3465,12 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv)
struct drm_atomic_state *state; struct drm_atomic_state *state;
int ret; int ret;
/* reset doesn't touch the display */
if (!i915.force_reset_modeset_test &&
!gpu_reset_clobbers_display(dev_priv))
return;
/* /*
* Need mode_config.mutex so that we don't * Need mode_config.mutex so that we don't
* trample ongoing ->detect() and whatnot. * trample ongoing ->detect() and whatnot.
...@@ -3498,12 +3484,6 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv) ...@@ -3498,12 +3484,6 @@ void intel_prepare_reset(struct drm_i915_private *dev_priv)
drm_modeset_backoff(ctx); drm_modeset_backoff(ctx);
} }
/* reset doesn't touch the display, but flips might get nuked anyway, */
if (!i915.force_reset_modeset_test &&
!gpu_reset_clobbers_display(dev_priv))
return;
/* /*
* Disabling the crtcs gracefully seems nicer. Also the * Disabling the crtcs gracefully seems nicer. Also the
* g33 docs say we should at least disable all the planes. * g33 docs say we should at least disable all the planes.
...@@ -3533,6 +3513,14 @@ void intel_finish_reset(struct drm_i915_private *dev_priv) ...@@ -3533,6 +3513,14 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
struct drm_atomic_state *state = dev_priv->modeset_restore_state; struct drm_atomic_state *state = dev_priv->modeset_restore_state;
int ret; int ret;
/* reset doesn't touch the display */
if (!i915.force_reset_modeset_test &&
!gpu_reset_clobbers_display(dev_priv))
return;
if (!state)
goto unlock;
/* /*
* Flips in the rings will be nuked by the reset, * Flips in the rings will be nuked by the reset,
* so complete all pending flips so that user space * so complete all pending flips so that user space
...@@ -3544,22 +3532,10 @@ void intel_finish_reset(struct drm_i915_private *dev_priv) ...@@ -3544,22 +3532,10 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
/* reset doesn't touch the display */ /* reset doesn't touch the display */
if (!gpu_reset_clobbers_display(dev_priv)) { if (!gpu_reset_clobbers_display(dev_priv)) {
if (!state) { /* for testing only restore the display */
/*
* Flips in the rings have been nuked by the reset,
* so update the base address of all primary
* planes to the the last fb to make sure we're
* showing the correct fb after a reset.
*
* FIXME: Atomic will make this obsolete since we won't schedule
* CS-based flips (which might get lost in gpu resets) any more.
*/
intel_update_primary_planes(dev);
} else {
ret = __intel_display_resume(dev, state, ctx); ret = __intel_display_resume(dev, state, ctx);
if (ret) if (ret)
DRM_ERROR("Restoring old state failed with %i\n", ret); DRM_ERROR("Restoring old state failed with %i\n", ret);
}
} else { } else {
/* /*
* The display has been reset as well, * The display has been reset as well,
...@@ -3583,8 +3559,8 @@ void intel_finish_reset(struct drm_i915_private *dev_priv) ...@@ -3583,8 +3559,8 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
intel_hpd_init(dev_priv); intel_hpd_init(dev_priv);
} }
if (state)
drm_atomic_state_put(state); drm_atomic_state_put(state);
unlock:
drm_modeset_drop_locks(ctx); drm_modeset_drop_locks(ctx);
drm_modeset_acquire_fini(ctx); drm_modeset_acquire_fini(ctx);
mutex_unlock(&dev->mode_config.mutex); mutex_unlock(&dev->mode_config.mutex);
......
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