[RFC] vblank worker for asynchronous unpinning and other tasks

[RFC] vblank worker for asynchronous unpinning and other tasks

[Chris Wilson]

Just a small series of patches that I like to get some feedback on.

The central goal of the series is to enable asynchronous sprite updates.
However, being able to asynchronously queue work to take place after the
next vblank is useful elsewhere so I have tried to generalise and find
some other users.

Please review,
-Chris

[PATCH 1/5] drm/i915: Introduce vblank work function

[Chris Wilson]

Throughout the driver, we have a number of pieces of code that must wait
for a vblank before we can update some state. Often these could be run
asynchronously since they are merely freeing a resource no long
referenced by a double-buffered registered. So we introduce a vblank
worker upon which we queue various tasks to be run after the next
vvblank.

This will be used in the next patches to avoid unnecessary stalls when
updating registers and for freeing resources.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   77 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |    8 ++++
 2 files changed, 85 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e5cf1eb..f879633 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -843,6 +843,74 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe)
 		DRM_DEBUG_KMS("vblank wait timed out\n");
 }
 
+struct intel_crtc_vblank_task {
+	struct list_head list;
+	void (*func)(struct intel_crtc *, void *data);
+	void *data;
+};
+
+static void intel_crtc_vblank_work_fn(struct work_struct *_work)
+{
+	struct intel_crtc_vblank_work *work =
+		container_of(_work, struct intel_crtc_vblank_work, work);
+	struct intel_crtc *crtc =
+		container_of(work, struct intel_crtc, vblank_work);
+	struct list_head tasks;
+
+	intel_wait_for_vblank(crtc->base.dev, crtc->pipe);
+
+	mutex_lock(&crtc->vblank_work.mutex);
+	list_replace_init(&work->tasks, &tasks);
+	mutex_unlock(&crtc->vblank_work.mutex);
+
+	while (!list_empty(&tasks)) {
+		struct intel_crtc_vblank_task *task
+			= list_first_entry(&tasks,
+					   struct intel_crtc_vblank_task,
+					   list);
+
+		task->func(crtc, task->data);
+		list_del(&task->list);
+		kfree(task);
+	}
+}
+
+static int intel_crtc_add_vblank_task(struct intel_crtc *crtc,
+				      bool single,
+				      void (*func)(struct intel_crtc *,
+						   void *data),
+				      void *data)
+{
+	struct intel_crtc_vblank_task *task;
+	struct intel_crtc_vblank_work *work = &crtc->vblank_work;
+
+	task = kzalloc(sizeof *task, GFP_KERNEL);
+	if (task == NULL)
+		return -ENOMEM;
+	task->func = func;
+	task->data = data;
+
+	mutex_lock(&work->mutex);
+	if (list_empty(&work->tasks)) {
+		schedule_work(&work->work);
+	} else if (single) {
+		struct intel_crtc_vblank_task *old;
+		list_for_each_entry(old, &work->tasks, list) {
+			if (task->func == func && task->data == data) {
+				func = NULL;
+				break;
+			}
+		}
+	}
+	if (func)
+		list_add(&task->list, &work->tasks);
+	else
+		kfree(task);
+	mutex_unlock(&work->mutex);
+
+	return 0;
+}
+
 /*
  * intel_wait_for_pipe_off - wait for pipe to turn off
  * @dev: drm device
@@ -2532,6 +2600,8 @@ static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 
+	cancel_work_sync(&to_intel_crtc(crtc)->vblank_work.work);
+
 	if (crtc->fb == NULL)
 		return;
 
@@ -6056,6 +6126,10 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	if (intel_crtc == NULL)
 		return;
 
+	mutex_init(&intel_crtc->vblank_work.mutex);
+	INIT_LIST_HEAD(&intel_crtc->vblank_work.tasks);
+	INIT_WORK(&intel_crtc->vblank_work.work, intel_crtc_vblank_work_fn);
+
 	drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs);
 
 	drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256);
@@ -7730,6 +7804,9 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	struct drm_crtc *crtc;
 	struct intel_crtc *intel_crtc;
 
+	/* Clear the vblank worker prior to taking any locks */
+	flush_scheduled_work();
+
 	drm_kms_helper_poll_fini(dev);
 	mutex_lock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f1e27ce..5ca07e0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -158,6 +158,12 @@ struct intel_connector {
 	struct intel_encoder *encoder;
 };
 
+struct intel_crtc_vblank_work {
+	struct work_struct work;
+	struct mutex mutex;
+	struct list_head tasks;
+};
+
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
@@ -172,6 +178,8 @@ struct intel_crtc {
 	struct intel_unpin_work *unpin_work;
 	int fdi_lanes;
 
+	struct intel_crtc_vblank_work vblank_work;
+
 	struct drm_i915_gem_object *cursor_bo;
 	uint32_t cursor_addr;
 	int16_t cursor_x, cursor_y;
-- 
1.7.10

[PATCH 2/5] drm/i915: Asynchronously unpin the old framebuffer after the next vblank

[Chris Wilson]

Upon completion of a modeset, we must wait for the next vblank before
releasing the old framebufer. (The scanout registers are double-buffered
and update on the next vblank. If we unpin the old scanout too soon we
run the risk of accessing invalid memory, so we first need to wait until
the scanout is reading from the new framebuffer.)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f879633..03e5d86 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1950,6 +1950,26 @@ intel_finish_fb(struct drm_framebuffer *old_fb)
 	return ret;
 }
 
+static void intel_crtc_unpin_work_fn(struct intel_crtc *crtc, void *obj)
+{
+	struct drm_device *dev = crtc->base.dev;
+
+	mutex_lock(&dev->struct_mutex);
+	intel_unpin_fb_obj(obj);
+	mutex_unlock(&dev->struct_mutex);
+}
+
+static void
+intel_crtc_queue_unpin(struct intel_crtc *crtc,
+		       struct drm_i915_gem_object *obj)
+{
+	if (intel_crtc_add_vblank_task(crtc, false,
+				       intel_crtc_unpin_work_fn, obj)) {
+		intel_wait_for_vblank(crtc->base.dev, crtc->pipe);
+		intel_unpin_fb_obj(obj);
+	}
+}
+
 static int
 intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		    struct drm_framebuffer *old_fb)
@@ -2000,10 +2020,9 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		return ret;
 	}
 
-	if (old_fb) {
-		intel_wait_for_vblank(dev, intel_crtc->pipe);
-		intel_unpin_fb_obj(to_intel_framebuffer(old_fb)->obj);
-	}
+	if (old_fb)
+		intel_crtc_queue_unpin(intel_crtc,
+				       to_intel_framebuffer(old_fb)->obj);
 
 	intel_update_fbc(dev);
 	mutex_unlock(&dev->struct_mutex);
-- 
1.7.10

[PATCH 3/5] drm/i915/sprite: Make plane switching asynchronous

[Chris Wilson]

Queue the unpinning of the current plane object to after the next
vblank. For special case benchmarks and others apps that may call
set_plane at a high frequency, we can unpin their objects directly
unless they are "live".

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_reg.h      |    4 ++
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 drivers/gpu/drm/i915/intel_drv.h     |    3 ++
 drivers/gpu/drm/i915/intel_sprite.c  |   67 ++++++++++++++++++++++++----------
 4 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5ac9837..a4c801e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2988,6 +2988,7 @@
 #define DVSSTRIDE(pipe) _PIPE(pipe, _DVSASTRIDE, _DVSBSTRIDE)
 #define DVSPOS(pipe) _PIPE(pipe, _DVSAPOS, _DVSBPOS)
 #define DVSSURF(pipe) _PIPE(pipe, _DVSASURF, _DVSBSURF)
+#define DVSSURFLIVE(pipe) _PIPE(pipe, _DVSASURFLIVE, _DVSBSURFLIVE)
 #define DVSKEYMAX(pipe) _PIPE(pipe, _DVSAKEYMAXVAL, _DVSBKEYMAXVAL)
 #define DVSSIZE(pipe) _PIPE(pipe, _DVSASIZE, _DVSBSIZE)
 #define DVSSCALE(pipe) _PIPE(pipe, _DVSASCALE, _DVSBSCALE)
@@ -3028,6 +3029,7 @@
 #define _SPRA_SURF		0x7029c
 #define _SPRA_KEYMAX		0x702a0
 #define _SPRA_TILEOFF		0x702a4
+#define _SPRA_SURFLIVE		0x702ac
 #define _SPRA_SCALE		0x70304
 #define   SPRITE_SCALE_ENABLE	(1<<31)
 #define   SPRITE_FILTER_MASK	(3<<29)
@@ -3048,6 +3050,7 @@
 #define _SPRB_SURF		0x7129c
 #define _SPRB_KEYMAX		0x712a0
 #define _SPRB_TILEOFF		0x712a4
+#define _SPRB_SURFLIVE		0x712ac
 #define _SPRB_SCALE		0x71304
 #define _SPRB_GAMC		0x71400
 
@@ -3059,6 +3062,7 @@
 #define SPRKEYVAL(pipe) _PIPE(pipe, _SPRA_KEYVAL, _SPRB_KEYVAL)
 #define SPRKEYMSK(pipe) _PIPE(pipe, _SPRA_KEYMSK, _SPRB_KEYMSK)
 #define SPRSURF(pipe) _PIPE(pipe, _SPRA_SURF, _SPRB_SURF)
+#define SPRSURFLIVE(pipe) _PIPE(pipe, _SPRA_SURFLIVE, _SPRB_SURFLIVE)
 #define SPRKEYMAX(pipe) _PIPE(pipe, _SPRA_KEYMAX, _SPRB_KEYMAX)
 #define SPRTILEOFF(pipe) _PIPE(pipe, _SPRA_TILEOFF, _SPRB_TILEOFF)
 #define SPRSCALE(pipe) _PIPE(pipe, _SPRA_SCALE, _SPRB_SCALE)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 03e5d86..edc5c9b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1959,7 +1959,7 @@ static void intel_crtc_unpin_work_fn(struct intel_crtc *crtc, void *obj)
 	mutex_unlock(&dev->struct_mutex);
 }
 
-static void
+void
 intel_crtc_queue_unpin(struct intel_crtc *crtc,
 		       struct drm_i915_gem_object *obj)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5ca07e0..e839a67 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -210,6 +210,7 @@ struct intel_plane {
 			       struct drm_intel_sprite_colorkey *key);
 	void (*get_colorkey)(struct drm_plane *plane,
 			     struct drm_intel_sprite_colorkey *key);
+	u32 (*current_surface)(struct drm_plane *plane);
 };
 
 struct intel_watermark_params {
@@ -364,6 +365,8 @@ extern void intel_panel_disable_backlight(struct drm_device *dev);
 extern void intel_panel_destroy_backlight(struct drm_device *dev);
 extern enum drm_connector_status intel_panel_detect(struct drm_device *dev);
 
+extern void intel_crtc_queue_unpin(struct intel_crtc *crtc,
+				   struct drm_i915_gem_object *obj);
 extern void intel_crtc_load_lut(struct drm_crtc *crtc);
 extern void intel_encoder_prepare(struct drm_encoder *encoder);
 extern void intel_encoder_commit(struct drm_encoder *encoder);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index fbf03b9..11545ca 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -207,6 +207,16 @@ ivb_get_colorkey(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key)
 		key->flags = I915_SET_COLORKEY_NONE;
 }
 
+static u32
+ivb_current_surface(struct drm_plane *plane)
+{
+	struct intel_plane *intel_plane;
+
+	intel_plane = to_intel_plane(plane);
+
+	return SPRSURFLIVE(intel_plane->pipe);
+}
+
 static void
 ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 		 struct drm_i915_gem_object *obj, int crtc_x, int crtc_y,
@@ -387,6 +397,35 @@ ilk_get_colorkey(struct drm_plane *plane, struct drm_intel_sprite_colorkey *key)
 		key->flags = I915_SET_COLORKEY_NONE;
 }
 
+static u32
+ilk_current_surface(struct drm_plane *plane)
+{
+	struct intel_plane *intel_plane;
+
+	intel_plane = to_intel_plane(plane);
+
+	return DVSSURFLIVE(intel_plane->pipe);
+}
+
+static void
+intel_plane_queue_unpin(struct intel_plane *plane,
+			struct drm_i915_gem_object *obj)
+{
+	/*
+	 * If the surface is currently being scanned out, we need to
+	 * wait until the next vblank event latches in the new base address
+	 * before we unpin it, or we may end up displaying the wrong data.
+	 * However, if the old object isn't currently 'live', we can just
+	 * unpin right away.
+	 */
+	if (plane->current_surface(&plane->base) != obj->gtt_offset) {
+		intel_unpin_fb_obj(obj);
+		return;
+	}
+
+	intel_crtc_queue_unpin(to_intel_crtc(plane->base.crtc), obj);
+}
+
 static int
 intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		   struct drm_framebuffer *fb, int crtc_x, int crtc_y,
@@ -492,20 +531,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	}
 
 	/* Unpin old obj after new one is active to avoid ugliness */
-	if (old_obj) {
-		/*
-		 * It's fairly common to simply update the position of
-		 * an existing object.  In that case, we don't need to
-		 * wait for vblank to avoid ugliness, we only need to
-		 * do the pin & ref bookkeeping.
-		 */
-		if (old_obj != obj) {
-			mutex_unlock(&dev->struct_mutex);
-			intel_wait_for_vblank(dev, to_intel_crtc(crtc)->pipe);
-			mutex_lock(&dev->struct_mutex);
-		}
-		intel_unpin_fb_obj(old_obj);
-	}
+	if (old_obj)
+		intel_plane_queue_unpin(intel_plane, old_obj);
 
 out_unlock:
 	mutex_unlock(&dev->struct_mutex);
@@ -527,14 +554,12 @@ intel_disable_plane(struct drm_plane *plane)
 
 	intel_plane->disable_plane(plane);
 
-	if (!intel_plane->obj)
-		goto out;
-
 	mutex_lock(&dev->struct_mutex);
-	intel_unpin_fb_obj(intel_plane->obj);
-	intel_plane->obj = NULL;
+	if (intel_plane->obj) {
+		intel_plane_queue_unpin(intel_plane, intel_plane->obj);
+		intel_plane->obj = NULL;
+	}
 	mutex_unlock(&dev->struct_mutex);
-out:
 
 	return ret;
 }
@@ -658,6 +683,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe)
 		intel_plane->disable_plane = ilk_disable_plane;
 		intel_plane->update_colorkey = ilk_update_colorkey;
 		intel_plane->get_colorkey = ilk_get_colorkey;
+		intel_plane->current_surface = ilk_current_surface;
 
 		if (IS_GEN6(dev)) {
 			plane_formats = snb_plane_formats;
@@ -674,6 +700,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe)
 		intel_plane->disable_plane = ivb_disable_plane;
 		intel_plane->update_colorkey = ivb_update_colorkey;
 		intel_plane->get_colorkey = ivb_get_colorkey;
+		intel_plane->current_surface = ivb_current_surface;
 
 		plane_formats = snb_plane_formats;
 		num_plane_formats = ARRAY_SIZE(snb_plane_formats);
-- 
1.7.10

[PATCH 4/5] drm/i915: Synchronize userspace palette LUT (i.e. gamma) changes to vblank

[Chris Wilson]

Adam Jackson was watching the screensaver fade out and expressed a
desire for the gamma updates to be synchronized to vblank to avoid the
unsightly tears.

Reported-by: Adam Jackson <ajax@redhat.com>
Cc: Adam Jackson <ajax@redhat.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |   44 ++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index edc5c9b..35901eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -47,6 +47,7 @@
 bool intel_pipe_has_type(struct drm_crtc *crtc, int type);
 static void intel_increase_pllclock(struct drm_crtc *crtc);
 static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
+static void __intel_crtc_load_lut(struct intel_crtc *crtc, void *data);
 
 typedef struct {
 	/* given values */
@@ -2811,7 +2812,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	 * On ILK+ LUT must be loaded before the pipe is running but with
 	 * clocks enabled
 	 */
-	intel_crtc_load_lut(crtc);
+	__intel_crtc_load_lut(to_intel_crtc(crtc), NULL);
 
 	intel_enable_pipe(dev_priv, pipe, is_pch_port);
 	intel_enable_plane(dev_priv, plane, pipe);
@@ -4724,31 +4725,43 @@ void intel_write_eld(struct drm_encoder *encoder,
 		dev_priv->display.write_eld(connector, crtc);
 }
 
-/** Loads the palette/gamma unit for the CRTC with the prepared values */
-void intel_crtc_load_lut(struct drm_crtc *crtc)
+static void __intel_crtc_load_lut(struct intel_crtc *crtc,
+				  void *data)
 {
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int palreg = PALETTE(intel_crtc->pipe);
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	int reg;
 	int i;
 
-	/* The clocks have to be on to load the palette. */
-	if (!crtc->enabled || !intel_crtc->active)
+	if (!crtc->base.enabled || !crtc->active)
 		return;
 
 	/* use legacy palette for Ironlake */
-	if (HAS_PCH_SPLIT(dev))
-		palreg = LGC_PALETTE(intel_crtc->pipe);
+	reg = PALETTE(crtc->pipe);
+	if (HAS_PCH_SPLIT(crtc->base.dev))
+		reg = LGC_PALETTE(crtc->pipe);
 
 	for (i = 0; i < 256; i++) {
-		I915_WRITE(palreg + 4 * i,
-			   (intel_crtc->lut_r[i] << 16) |
-			   (intel_crtc->lut_g[i] << 8) |
-			   intel_crtc->lut_b[i]);
+		I915_WRITE(reg + 4 * i,
+			   crtc->lut_r[i] << 16 |
+			   crtc->lut_g[i] << 8  |
+			   crtc->lut_b[i]);
 	}
 }
 
+/** Loads the palette/gamma unit for the CRTC with the prepared values */
+void intel_crtc_load_lut(struct drm_crtc *crtc)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	/* The clocks have to be on to load the palette. */
+	if (!crtc->enabled || !intel_crtc->active)
+		return;
+
+	if (intel_crtc_add_vblank_task(intel_crtc, true,
+				       __intel_crtc_load_lut, NULL))
+		__intel_crtc_load_lut(intel_crtc, NULL);
+}
+
 static void i845_update_cursor(struct drm_crtc *crtc, u32 base)
 {
 	struct drm_device *dev = crtc->dev;
@@ -5036,6 +5049,7 @@ static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 	int end = (start + size > 256) ? 256 : start + size, i;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
+	/* We race here with setting the lut and reading it during vblank. */
 	for (i = start; i < end; i++) {
 		intel_crtc->lut_r[i] = red[i] >> 8;
 		intel_crtc->lut_g[i] = green[i] >> 8;
-- 
1.7.10

[PATCH 5/5] drm/i915: Up/downclock LVDS on vblanks

[Chris Wilson]

Jesse mentioned that we had reports of flickering due to switching
clocks for powersaving and that would be a useful task to be run at
vblank.

<Find some testers>

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |  111 ++++++++++++++++++++--------------
 1 file changed, 64 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 35901eb..6af8d50 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -46,6 +46,7 @@
 
 bool intel_pipe_has_type(struct drm_crtc *crtc, int type);
 static void intel_increase_pllclock(struct drm_crtc *crtc);
+static void intel_crtc_restore_pllclock(struct drm_crtc *crtc);
 static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
 static void __intel_crtc_load_lut(struct intel_crtc *crtc, void *data);
 
@@ -1919,7 +1920,8 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 
 	if (dev_priv->display.disable_fbc)
 		dev_priv->display.disable_fbc(dev);
-	intel_increase_pllclock(crtc);
+
+	intel_crtc_restore_pllclock(crtc);
 
 	return dev_priv->display.update_plane(crtc, fb, x, y);
 }
@@ -2028,6 +2030,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 	intel_update_fbc(dev);
 	mutex_unlock(&dev->struct_mutex);
 
+	intel_increase_pllclock(crtc);
 	if (!dev->primary->master)
 		return 0;
 
@@ -5472,73 +5475,91 @@ static void intel_crtc_idle_timer(unsigned long arg)
 	queue_work(dev_priv->wq, &dev_priv->idle_work);
 }
 
-static void intel_increase_pllclock(struct drm_crtc *crtc)
+static void __intel_crtc_increase_pllclock(struct intel_crtc *crtc,
+					   void *data)
 {
-	struct drm_device *dev = crtc->dev;
-	drm_i915_private_t *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
-	int dpll_reg = DPLL(pipe);
+	drm_i915_private_t *dev_priv = crtc->base.dev->dev_private;
+	int dpll_reg = DPLL(crtc->pipe);
 	int dpll;
 
-	if (HAS_PCH_SPLIT(dev))
-		return;
-
-	if (!dev_priv->lvds_downclock_avail)
-		return;
-
 	dpll = I915_READ(dpll_reg);
-	if (!HAS_PIPE_CXSR(dev) && (dpll & DISPLAY_RATE_SELECT_FPA1)) {
+	if (dpll & DISPLAY_RATE_SELECT_FPA1) {
 		DRM_DEBUG_DRIVER("upclocking LVDS\n");
 
-		assert_panel_unlocked(dev_priv, pipe);
-
-		dpll &= ~DISPLAY_RATE_SELECT_FPA1;
-		I915_WRITE(dpll_reg, dpll);
-		intel_wait_for_vblank(dev, pipe);
-
-		dpll = I915_READ(dpll_reg);
-		if (dpll & DISPLAY_RATE_SELECT_FPA1)
-			DRM_DEBUG_DRIVER("failed to upclock LVDS!\n");
+		assert_panel_unlocked(dev_priv, crtc->pipe);
+		I915_WRITE(dpll_reg, dpll & ~DISPLAY_RATE_SELECT_FPA1);
 	}
 
 	/* Schedule downclock */
-	mod_timer(&intel_crtc->idle_timer, jiffies +
-		  msecs_to_jiffies(CRTC_IDLE_TIMEOUT));
+	mod_timer(&crtc->idle_timer,
+		  jiffies + msecs_to_jiffies(CRTC_IDLE_TIMEOUT));
 }
 
-static void intel_decrease_pllclock(struct drm_crtc *crtc)
+static void intel_crtc_restore_pllclock(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	int reg;
+
+	if (HAS_PCH_SPLIT(dev) || HAS_PIPE_CXSR(dev))
+		return;
+
+	reg = DPLL(to_intel_crtc(crtc)->pipe);
+	I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_RATE_SELECT_FPA1);
+
+	del_timer(&to_intel_crtc(crtc)->idle_timer);
+}
+
+static void intel_increase_pllclock(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
-	int dpll_reg = DPLL(pipe);
-	int dpll = I915_READ(dpll_reg);
 
-	if (HAS_PCH_SPLIT(dev))
+	if (HAS_PCH_SPLIT(dev) || HAS_PIPE_CXSR(dev))
 		return;
 
 	if (!dev_priv->lvds_downclock_avail)
 		return;
 
+	if (intel_crtc_add_vblank_task(intel_crtc, true,
+				       __intel_crtc_increase_pllclock,
+				       NULL))
+		__intel_crtc_increase_pllclock(intel_crtc, NULL);
+}
+
+static void __intel_crtc_decrease_pllclock(struct intel_crtc *crtc,
+					   void *data)
+{
+	drm_i915_private_t *dev_priv = crtc->base.dev->dev_private;
+	int dpll_reg = DPLL(crtc->pipe);
+
 	/*
 	 * Since this is called by a timer, we should never get here in
 	 * the manual case.
 	 */
-	if (!HAS_PIPE_CXSR(dev) && intel_crtc->lowfreq_avail) {
-		DRM_DEBUG_DRIVER("downclocking LVDS\n");
+	DRM_DEBUG_DRIVER("downclocking LVDS\n");
 
-		assert_panel_unlocked(dev_priv, pipe);
+	assert_panel_unlocked(dev_priv, crtc->pipe);
+	I915_WRITE(dpll_reg, I915_READ(dpll_reg) | DISPLAY_RATE_SELECT_FPA1);
+}
 
-		dpll |= DISPLAY_RATE_SELECT_FPA1;
-		I915_WRITE(dpll_reg, dpll);
-		intel_wait_for_vblank(dev, pipe);
-		dpll = I915_READ(dpll_reg);
-		if (!(dpll & DISPLAY_RATE_SELECT_FPA1))
-			DRM_DEBUG_DRIVER("failed to downclock LVDS!\n");
-	}
+static void intel_decrease_pllclock(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	if (HAS_PCH_SPLIT(dev) || HAS_PIPE_CXSR(dev))
+		return;
+
+	if (!dev_priv->lvds_downclock_avail || !intel_crtc->lowfreq_avail)
+		return;
 
+	if (intel_crtc_add_vblank_task(intel_crtc, true,
+				       __intel_crtc_decrease_pllclock,
+				       NULL))
+		__intel_crtc_decrease_pllclock(intel_crtc, NULL);
 }
 
 /**
@@ -7835,7 +7856,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
-	struct intel_crtc *intel_crtc;
 
 	/* Clear the vblank worker prior to taking any locks */
 	flush_scheduled_work();
@@ -7851,8 +7871,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
 		if (!crtc->fb)
 			continue;
 
-		intel_crtc = to_intel_crtc(crtc);
-		intel_increase_pllclock(crtc);
+		intel_crtc_restore_pllclock(crtc);
 	}
 
 	intel_disable_fbc(dev);
@@ -7880,10 +7899,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	flush_scheduled_work();
 
 	/* Shut off idle work before the crtcs get freed. */
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-		intel_crtc = to_intel_crtc(crtc);
-		del_timer_sync(&intel_crtc->idle_timer);
-	}
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
+		del_timer_sync(&to_intel_crtc(crtc)->idle_timer);
 	del_timer_sync(&dev_priv->idle_timer);
 	cancel_work_sync(&dev_priv->idle_work);
 
-- 
1.7.10