[Intel-gfx] FBC patchset

[Chris Wilson]

Oh dear, it was all looking too good. Works fine with just one output.
Runs into a nasty race if you add a second and start unplugging things...

The issue is that when unplugging an output, userspace sees this and
appropriately reconfigures from an extended desktop to just using, for the
sake of argument, the LVDS. As the desktop is changing size this requires
a new framebuffer and so we begin to teardown the two crtcs and replace
with just the one with the new fb. The first thing we do is disable the
crtc connected to the unplugged display. This triggers an
intel_update_fbc() which spots that it can now enable FBC on the current
single crtc + old fb and promptly dispatches a delayed task to do so.

The mode reconfiguration continues apace and we disable the other crtc and
start to replace the plane's FB. This involves a couple of
wait-for-vblanks, and so is quite slow. During this time we avoid taking
the struct_mutex. Meanwhile, the delayed task kicks off on a second CPU
grabs the struct_mutex and reconfigures the FBC registers. Apparently
enabling FBC on a dead plane is an undefined operation. Hilarity ensues,
followed by a swift reboot.

Bumping to 250ms sufficiently delays the task to miss the race, but we
can not foretell just how long any given crtc modeset will take. So what
we need is to take the mode_config.lock in order to prevent concurrent
access to the FBC registers and also to prevent the fb from disappearing
beneath us:

---
 drivers/gpu/drm/i915/intel_display.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e375500..bec9e1d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1603,6 +1603,11 @@ static void intel_fbc_work_fn(struct work_struct *__work)
 	struct drm_device *dev = work->crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	if (!mutex_trylock(&dev->mode_config.mutex)) {
+		schedule_delayed_work(&work->work, msecs_to_jiffies(100));
+		return;
+	}
+
 	mutex_lock(&dev->struct_mutex);
 	if (work == dev_priv->fbc_work) {
 		/* Double check that we haven't switched fb without cancelling
@@ -1620,6 +1625,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
 		dev_priv->fbc_work = NULL;
 	}
 	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&dev->mode_config.mutex);
 
 	kfree(work);
 }
@@ -1684,7 +1690,7 @@ static void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
 	 * and indeed performing the enable as a co-routine and not
 	 * waiting synchronously upon the vblank.
 	 */
-	schedule_delayed_work(&work->work, msecs_to_jiffies(50));
+	schedule_delayed_work(&work->work, msecs_to_jiffies(250));
 }
 
 void intel_disable_fbc(struct drm_device *dev)
-- 
1.7.6

-- 
Chris Wilson, Intel Open Source Technology Centre