Commit 14667a4b authored by Chris Wilson's avatar Chris Wilson Committed by Daniel Vetter

drm/i915: Finish any pending operations on the framebuffer before disabling

Similar to the case where we are changing from one framebuffer to
another, we need to be sure that there are no pending WAIT_FOR_EVENTs on
the pipe for the current framebuffer before switching. If we disable the
pipe, and then try to execute a WAIT_FOR_EVENT it will block
indefinitely and cause a GPU hang.

We attempted to fix this in commit 85345517
(drm/i915: Retire any pending operations on the old scanout when switching)
for the case of mode switching, but this leaves the condition where we
are switching off the pipe vulnerable.

There still remains the race condition were a display may be unplugged,
switched off by the core, a uevent sent to notify the DDX and the DDX
may issue a WAIT_FOR_EVENT before it processes the uevent. This window
does not exist if the pipe is only switched off in response to the
uevent. Time to make sure that is so...
Reported-by: default avatarFrancis Leblanc <Francis.Leblanc-Lebeau@verint.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36515
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45413Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarEugeni Dodonov <eugeni.dodonov@intel.com>
[danvet: fixup spelling in comment, noticed by Eugeni.]
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent b2a71642
...@@ -2244,6 +2244,33 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb, ...@@ -2244,6 +2244,33 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
return 0; return 0;
} }
static int
intel_finish_fb(struct drm_framebuffer *old_fb)
{
struct drm_i915_gem_object *obj = to_intel_framebuffer(old_fb)->obj;
struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
bool was_interruptible = dev_priv->mm.interruptible;
int ret;
wait_event(dev_priv->pending_flip_queue,
atomic_read(&dev_priv->mm.wedged) ||
atomic_read(&obj->pending_flip) == 0);
/* Big Hammer, we also need to ensure that any pending
* MI_WAIT_FOR_EVENT inside a user batch buffer on the
* current scanout is retired before unpinning the old
* framebuffer.
*
* This should only fail upon a hung GPU, in which case we
* can safely continue.
*/
dev_priv->mm.interruptible = false;
ret = i915_gem_object_finish_gpu(obj);
dev_priv->mm.interruptible = was_interruptible;
return ret;
}
static int static int
intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
struct drm_framebuffer *old_fb) struct drm_framebuffer *old_fb)
...@@ -2282,25 +2309,8 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, ...@@ -2282,25 +2309,8 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
return ret; return ret;
} }
if (old_fb) { if (old_fb)
struct drm_i915_private *dev_priv = dev->dev_private; intel_finish_fb(old_fb);
struct drm_i915_gem_object *obj = to_intel_framebuffer(old_fb)->obj;
wait_event(dev_priv->pending_flip_queue,
atomic_read(&dev_priv->mm.wedged) ||
atomic_read(&obj->pending_flip) == 0);
/* Big Hammer, we also need to ensure that any pending
* MI_WAIT_FOR_EVENT inside a user batch buffer on the
* current scanout is retired before unpinning the old
* framebuffer.
*
* This should only fail upon a hung GPU, in which case we
* can safely continue.
*/
ret = i915_gem_object_finish_gpu(obj);
(void) ret;
}
ret = intel_pipe_set_base_atomic(crtc, crtc->fb, x, y, ret = intel_pipe_set_base_atomic(crtc, crtc->fb, x, y,
LEAVE_ATOMIC_MODE_SET); LEAVE_ATOMIC_MODE_SET);
...@@ -3371,6 +3381,23 @@ static void intel_crtc_disable(struct drm_crtc *crtc) ...@@ -3371,6 +3381,23 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private; struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
struct drm_device *dev = crtc->dev; struct drm_device *dev = crtc->dev;
/* Flush any pending WAITs before we disable the pipe. Note that
* we need to drop the struct_mutex in order to acquire it again
* during the lowlevel dpms routines around a couple of the
* operations. It does not look trivial nor desirable to move
* that locking higher. So instead we leave a window for the
* submission of further commands on the fb before we can actually
* disable it. This race with userspace exists anyway, and we can
* only rely on the pipe being disabled by userspace after it
* receives the hotplug notification and has flushed any pending
* batches.
*/
if (crtc->fb) {
mutex_lock(&dev->struct_mutex);
intel_finish_fb(crtc->fb);
mutex_unlock(&dev->struct_mutex);
}
crtc_funcs->dpms(crtc, DRM_MODE_DPMS_OFF); crtc_funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
assert_plane_disabled(dev->dev_private, to_intel_crtc(crtc)->plane); assert_plane_disabled(dev->dev_private, to_intel_crtc(crtc)->plane);
assert_pipe_disabled(dev->dev_private, to_intel_crtc(crtc)->pipe); assert_pipe_disabled(dev->dev_private, to_intel_crtc(crtc)->pipe);
......
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