public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* RFC asynchronous vblank tasks
@ 2013-07-04 22:07 Chris Wilson
  2013-07-04 22:07 ` [PATCH 1/5] drm/i915: Asynchronously unpin the old framebuffer after the next vblank Chris Wilson
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Chris Wilson @ 2013-07-04 22:07 UTC (permalink / raw)
  To: intel-gfx

This is an old series to make sprite and primary plane flip that little
bit more asynchronous. Shaving 16-20ms off X start up time is no small
feat when we can start X in around 100-200ms.
-Chris

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/5] drm/i915: Asynchronously unpin the old framebuffer after the next vblank
  2013-07-04 22:07 RFC asynchronous vblank tasks Chris Wilson
@ 2013-07-04 22:07 ` Chris Wilson
  2013-07-04 22:07 ` [PATCH 2/5] drm/i915/sprite: Make plane switching asynchronous Chris Wilson
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2013-07-04 22:07 UTC (permalink / raw)
  To: intel-gfx

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.) As the unpinning is
then deferred, we need to make sure we hold a reference to the object
until we execute the callback.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dff3b93..4b50d5f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1921,6 +1921,7 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
 		goto err_unpin;
 
 	i915_gem_object_pin_fence(obj);
+	drm_gem_object_reference(&obj->base);
 
 	dev_priv->mm.interruptible = true;
 	return 0;
@@ -1936,6 +1937,7 @@ void intel_unpin_fb_obj(struct drm_i915_gem_object *obj)
 {
 	i915_gem_object_unpin_fence(obj);
 	i915_gem_object_unpin(obj);
+	drm_gem_object_unreference(&obj->base);
 }
 
 /* Computes the linear offset to the base tile and adjusts x, y. bytes per pixel
@@ -2259,6 +2261,26 @@ static void intel_crtc_update_sarea_pos(struct drm_crtc *crtc, int x, int y)
 	}
 }
 
+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 *fb)
@@ -2321,9 +2343,12 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 	crtc->y = y;
 
 	if (old_fb) {
+		struct drm_i915_gem_object *old_obj =
+			to_intel_framebuffer(old_fb)->obj;
 		if (intel_crtc->active && old_fb != fb)
-			intel_wait_for_vblank(dev, intel_crtc->pipe);
-		intel_unpin_fb_obj(to_intel_framebuffer(old_fb)->obj);
+			intel_crtc_queue_unpin(intel_crtc, old_obj);
+		else
+			intel_unpin_fb_obj(old_obj);
 	}
 
 	intel_update_fbc(dev);
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/5] drm/i915/sprite: Make plane switching asynchronous
  2013-07-04 22:07 RFC asynchronous vblank tasks Chris Wilson
  2013-07-04 22:07 ` [PATCH 1/5] drm/i915: Asynchronously unpin the old framebuffer after the next vblank Chris Wilson
@ 2013-07-04 22:07 ` Chris Wilson
  2013-07-04 22:07 ` [PATCH 3/5] drm/i915: Synchronize userspace palette LUT (i.e. gamma) changes to vblank Chris Wilson
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2013-07-04 22:07 UTC (permalink / raw)
  To: intel-gfx

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      |  2 ++
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++
 drivers/gpu/drm/i915/intel_sprite.c  | 69 ++++++++++++++++++++++++------------
 4 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 72e5693..74a3f01 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3411,6 +3411,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)
@@ -3487,6 +3488,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 SPROFFSET(pipe) _PIPE(pipe, _SPRA_OFFSET, _SPRB_OFFSET)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4b50d5f..dc4bf49 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2270,7 +2270,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 19df4ec..e6dda01 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -383,6 +383,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 {
@@ -648,6 +649,8 @@ extern int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
 			  int x, int y, struct drm_framebuffer *old_fb);
 extern void intel_modeset_disable(struct drm_device *dev);
 extern void intel_crtc_restore_mode(struct drm_crtc *crtc);
+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_crtc_update_dpms(struct drm_crtc *crtc);
 extern void intel_encoder_destroy(struct drm_encoder *encoder);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 9d95589..d129282 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -397,6 +397,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,
@@ -602,6 +612,35 @@ format_is_yuv(uint32_t format)
 	}
 }
 
+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) != i915_gem_obj_ggtt_offset(obj)) {
+		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,
@@ -830,20 +869,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		intel_disable_primary(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);
@@ -861,16 +888,12 @@ intel_disable_plane(struct drm_plane *plane)
 		intel_enable_primary(plane->crtc);
 	intel_plane->disable_plane(plane);
 
-	if (!intel_plane->obj)
-		goto out;
-
-	intel_wait_for_vblank(dev, intel_plane->pipe);
-
 	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;
 }
@@ -1029,6 +1052,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 		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;
@@ -1061,6 +1085,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane)
 			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.8.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/5] drm/i915: Synchronize userspace palette LUT (i.e. gamma) changes to vblank
  2013-07-04 22:07 RFC asynchronous vblank tasks Chris Wilson
  2013-07-04 22:07 ` [PATCH 1/5] drm/i915: Asynchronously unpin the old framebuffer after the next vblank Chris Wilson
  2013-07-04 22:07 ` [PATCH 2/5] drm/i915/sprite: Make plane switching asynchronous Chris Wilson
@ 2013-07-04 22:07 ` Chris Wilson
  2013-07-04 22:07 ` [PATCH 4/5] drm/i915: Up/downclock LVDS on vblanks Chris Wilson
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2013-07-04 22:07 UTC (permalink / raw)
  To: intel-gfx

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 | 61 ++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dc4bf49..279474a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -44,6 +44,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);
 
 static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 				struct intel_crtc_config *pipe_config);
@@ -3457,7 +3458,7 @@ static void haswell_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_ddi_set_pipe_settings(crtc);
 	intel_ddi_enable_transcoder_func(crtc);
@@ -6411,47 +6412,58 @@ 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_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	enum pipe pipe = intel_crtc->pipe;
-	int palreg = PALETTE(pipe);
-	int i;
+	enum pipe pipe = crtc->pipe;
 	bool reenable_ips = false;
+	int reg, i;
 
-	/* The clocks have to be on to load the palette. */
-	if (!crtc->enabled || !intel_crtc->active)
+	if (!crtc->base.enabled || !crtc->active)
 		return;
 
-	if (!HAS_PCH_SPLIT(dev_priv->dev))
+	if (!HAS_PCH_SPLIT(dev))
 		assert_pll_enabled(dev_priv, pipe);
 
-	/* use legacy palette for Ironlake */
-	if (HAS_PCH_SPLIT(dev))
-		palreg = LGC_PALETTE(pipe);
-
 	/* Workaround : Do not read or write the pipe palette/gamma data while
 	 * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
 	 */
-	if (intel_crtc->config.ips_enabled &&
+	if (crtc->config.ips_enabled &&
 	    ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) ==
 	     GAMMA_MODE_MODE_SPLIT)) {
-		hsw_disable_ips(intel_crtc);
+		hsw_disable_ips(crtc);
 		reenable_ips = true;
 	}
 
-	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]);
-	}
+	/* use legacy palette for Ironlake */
+	reg = PALETTE(pipe);
+	if (HAS_PCH_SPLIT(crtc->base.dev))
+		reg = LGC_PALETTE(pipe);
+
+	for (i = 0; i < 256; i++)
+		I915_WRITE(reg + 4 * i,
+			   crtc->lut_r[i] << 16 |
+			   crtc->lut_g[i] << 8  |
+			   crtc->lut_b[i]);
 
 	if (reenable_ips)
-		hsw_enable_ips(intel_crtc);
+		hsw_enable_ips(crtc);
+}
+
+/** 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)
@@ -6749,6 +6761,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.8.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/5] drm/i915: Up/downclock LVDS on vblanks
  2013-07-04 22:07 RFC asynchronous vblank tasks Chris Wilson
                   ` (2 preceding siblings ...)
  2013-07-04 22:07 ` [PATCH 3/5] drm/i915: Synchronize userspace palette LUT (i.e. gamma) changes to vblank Chris Wilson
@ 2013-07-04 22:07 ` Chris Wilson
  2013-07-04 22:07 ` [PATCH 5/5] drm/i915: Boost DMA qos whilst performing uninterruptible waits for the GPU Chris Wilson
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2013-07-04 22:07 UTC (permalink / raw)
  To: intel-gfx

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 | 96 +++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 279474a..68e8b62 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -43,6 +43,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);
 
@@ -2169,7 +2170,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);
 }
@@ -7184,71 +7186,85 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev,
 	return mode;
 }
 
-static void intel_increase_pllclock(struct drm_crtc *crtc)
+static void __gmch_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);
+		assert_panel_unlocked(dev_priv, crtc->pipe);
+		I915_WRITE(dpll_reg, dpll & ~DISPLAY_RATE_SELECT_FPA1);
+	}
+}
 
-		dpll &= ~DISPLAY_RATE_SELECT_FPA1;
-		I915_WRITE(dpll_reg, dpll);
-		intel_wait_for_vblank(dev, pipe);
+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;
 
-		dpll = I915_READ(dpll_reg);
-		if (dpll & DISPLAY_RATE_SELECT_FPA1)
-			DRM_DEBUG_DRIVER("failed to upclock LVDS!\n");
-	}
+	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);
 }
 
-static void intel_decrease_pllclock(struct drm_crtc *crtc)
+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);
 
-	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,
+				       __gmch_crtc_increase_pllclock,
+				       NULL))
+		__gmch_crtc_increase_pllclock(intel_crtc, NULL);
+}
+
+static void __gmch_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) {
-		int pipe = intel_crtc->pipe;
-		int dpll_reg = DPLL(pipe);
-		int dpll;
+	DRM_DEBUG_DRIVER("downclocking LVDS\n");
 
-		DRM_DEBUG_DRIVER("downclocking LVDS\n");
+	assert_panel_unlocked(dev_priv, crtc->pipe);
+	I915_WRITE(dpll_reg, I915_READ(dpll_reg) | DISPLAY_RATE_SELECT_FPA1);
+}
 
-		assert_panel_unlocked(dev_priv, pipe);
+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);
 
-		dpll = I915_READ(dpll_reg);
-		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");
-	}
+	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,
+				       __gmch_crtc_decrease_pllclock,
+				       NULL))
+		__gmch_crtc_decrease_pllclock(intel_crtc, NULL);
 }
 
 void intel_mark_busy(struct drm_device *dev)
@@ -10393,7 +10409,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;
 
 	/*
 	 * Interrupts and polling as the first thing to avoid creating havoc.
@@ -10422,8 +10437,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);
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 5/5] drm/i915: Boost DMA qos whilst performing uninterruptible waits for the GPU
  2013-07-04 22:07 RFC asynchronous vblank tasks Chris Wilson
                   ` (3 preceding siblings ...)
  2013-07-04 22:07 ` [PATCH 4/5] drm/i915: Up/downclock LVDS on vblanks Chris Wilson
@ 2013-07-04 22:07 ` Chris Wilson
  2013-07-05  8:42 ` RFC asynchronous vblank tasks Daniel Vetter
  2013-07-05  8:48 ` [PATCH] drm/i915: Introduce vblank work function Chris Wilson
  6 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2013-07-04 22:07 UTC (permalink / raw)
  To: intel-gfx

If we are forced to do an uninterruptible wait for a result from the
GPU, boost the qos of DMA operations to minimise the latency and
minimise the amount of time spent waiting for the operation whilst
uninterruptible.

This shaves a modicum of time off waiting for GPU operations to
complete before performing a MMIO flip, for example.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f532a01..f5da4a0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1013,14 +1013,22 @@ static int __wait_seqno(struct intel_ring_buffer *ring, u32 seqno,
 	 i915_reset_in_progress(&dev_priv->gpu_error) || \
 	 reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
 	do {
-		if (interruptible)
+		if (interruptible) {
 			end = wait_event_interruptible_timeout(ring->irq_queue,
 							       EXIT_COND,
 							       timeout_jiffies);
-		else
+		} else {
+			struct pm_qos_request qos;
+
+			memset(&qos, 0, sizeof(qos));
+			pm_qos_add_request(&qos, PM_QOS_CPU_DMA_LATENCY, 0);
+
 			end = wait_event_timeout(ring->irq_queue, EXIT_COND,
 						 timeout_jiffies);
 
+			pm_qos_remove_request(&qos);
+		}
+
 		/* We need to check whether any gpu reset happened in between
 		 * the caller grabbing the seqno and now ... */
 		if (reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: RFC asynchronous vblank tasks
  2013-07-04 22:07 RFC asynchronous vblank tasks Chris Wilson
                   ` (4 preceding siblings ...)
  2013-07-04 22:07 ` [PATCH 5/5] drm/i915: Boost DMA qos whilst performing uninterruptible waits for the GPU Chris Wilson
@ 2013-07-05  8:42 ` Daniel Vetter
  2013-07-05  8:48 ` [PATCH] drm/i915: Introduce vblank work function Chris Wilson
  6 siblings, 0 replies; 13+ messages in thread
From: Daniel Vetter @ 2013-07-05  8:42 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Thu, Jul 04, 2013 at 11:07:06PM +0100, Chris Wilson wrote:
> This is an old series to make sprite and primary plane flip that little
> bit more asynchronous. Shaving 16-20ms off X start up time is no small
> feat when we can start X in around 100-200ms.

Summarizing our discussion on irc from yesterday:
- The first patch seems to be missing ;-)

- The names should imo get more bikeshedding to be more consistent with
  the core delayed_work interfaces, e.g. intel_crtc_schedule_vblank_work.
  The important part is to be able to easily distinguish work items fired
  off from the vblank handler which run in process context (in a workqueue
  somewhere) and vblank callbacks that run in interrupt context since
  they're more timing critical and don't need sleeping looks. Ville wants
  that for his watermark stuff. And I think we could use it to avoid a
  bunch of vblank waits in e.g. the fbc or DRRS code that we need to
  implement workarounds.

- Imo if we have more delayed unpins we really need to integrate this into
  the eviction code. Atm we get mostly away with it since most users don't
  have per-crtc framebuffers and we have hence at most 2 buffers in-flight
  at any time due to pageflipping. The real fun of making that happen is
  to avoid deadlocks.

Especially the last point imo is a blocker for the delayed unpin patches
(the other stuff could go in), and I think it should also convert the
delayed unpin work we use in the pageflip handler while at it.

I think we need a few pieces to make this work:
- global delayed_unpin list_head of objects which can be unpinned after
  something has happened (vblank, pageflip, ...)
- second list where objects are stored which can be unpinned
- irqsafe spinlock to protect it
- waitqueue that gets woken up every time someone pushes a new obj from
  the delayed_unpin list to the unpin list

Since we're dealing with framebuffer objects everywhere I'd say we could
use those (and hence also shift around the refcounting to there). Plain
gem objects get their delayed unpin through the normal active list, so I
don't think this reduces the usefulness.

We could also embed the vblank callback into the framebuffer object.

Now the actual unpinning happens in two places:

1) From a work item which gets scheduled every time the interrupt handler
pushes something onto the unpin list. In pseudo-code:

mutex_lock(dev->struct_mutex);
while (1) {
	spin_lock_irqsave(delayed_unpin_lock);
	fb = first_list_item(unpin_list);
	if (fb)
		list_del(fb->unpin_list);
	spin_unlock_irqrestore(delayed_unpin_lock);

	if (!fb)
		break;
	/* the unpin list holds a reference which we inherit */
	unpin_fb(fb);
	unref_fb(fb);
}
mutex_unlock(dev->struct_mutex);

The same function could also be used to trim pin references in the
pageflip code instead of the currently-used flush_work.

2) From the evict code, at the very end shortly before we give up on
everything. We check whether the delayed_unpin list is empty, and if it's
not we wait on the waitqueue for new stuff to get moved from the delayed
unpin list to the unpin list. Every time we get woken up we grab the
spinlock and unpin the framebuffer. We have to do this ourselves since we
already hold the dev->struct_mutex lock. We can probably reuse the inner
loop from the work item above for that.

Note that if we do that only after we've search the entire active list we
could idle the gpu completely to make sure that the pageflips can indeed
happen. Still for paranoia the wait should also check for gpu hangs, so we
need to wake up the unpin_queue too when a gpu hang happens like we
already do with all the rinq waitqueues.

I don't think that we need anything special before we disable the crtc
since I expect the generic vblank callback infrastructure to already take
care of this by waiting for any pending vblank callbacks to complete
before it kills the crtc.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] drm/i915: Introduce vblank work function
  2013-07-04 22:07 RFC asynchronous vblank tasks Chris Wilson
                   ` (5 preceding siblings ...)
  2013-07-05  8:42 ` RFC asynchronous vblank tasks Daniel Vetter
@ 2013-07-05  8:48 ` Chris Wilson
  2013-12-06 10:44   ` Bloomfield, Jon
  6 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2013-07-05  8:48 UTC (permalink / raw)
  To: intel-gfx

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 | 79 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  8 +++-
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b70ce4e..dff3b93 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -795,6 +795,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
@@ -2835,6 +2903,8 @@ static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 	if (crtc->fb == NULL)
 		return;
 
+	flush_work(&to_intel_crtc(crtc)->vblank_work.work);
+
 	WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
 
 	wait_event(dev_priv->pending_flip_queue,
@@ -9097,6 +9167,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);
 
 	if (drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256))
@@ -10296,11 +10370,16 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	 */
 	drm_kms_helper_poll_fini(dev);
 
+	/* Clear the vblank worker prior to taking any locks */
+	flush_scheduled_work();
+
 	mutex_lock(&dev->struct_mutex);
 
 	intel_unregister_dsm_handler();
 
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		flush_work(&to_intel_crtc(crtc)->vblank_work.work);
+
 		/* Skip inactive CRTCs */
 		if (!crtc->fb)
 			continue;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 619caf9..19df4ec 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -314,10 +314,16 @@ struct intel_crtc {
 	bool primary_disabled; /* is the crtc obscured by a plane? */
 	bool lowfreq_avail;
 	struct intel_overlay *overlay;
-	struct intel_unpin_work *unpin_work;
 
+	struct intel_unpin_work *unpin_work;
 	atomic_t unpin_work_count;
 
+	struct intel_crtc_vblank_work {
+		struct work_struct work;
+		struct mutex mutex;
+		struct list_head tasks;
+	} vblank_work;
+
 	/* Display surface base address adjustement for pageflips. Note that on
 	 * gen4+ this only adjusts up to a tile, offsets within a tile are
 	 * handled in the hw itself (with the TILEOFF register). */
-- 
1.8.3.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915: Introduce vblank work function
  2013-07-05  8:48 ` [PATCH] drm/i915: Introduce vblank work function Chris Wilson
@ 2013-12-06 10:44   ` Bloomfield, Jon
  2013-12-06 12:06     ` Daniel Vetter
  0 siblings, 1 reply; 13+ messages in thread
From: Bloomfield, Jon @ 2013-12-06 10:44 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx@lists.freedesktop.org

What's the status of this patch ? I can't find any subsequent mention of it, but we currently use it in one of our Android development trees. I'm trying to work out whether to retain it or replace it.

Was it killed off, or is it still in the pipeline ?

Thanks.

> -----Original Message-----
> From: intel-gfx-bounces+jon.bloomfield=intel.com@lists.freedesktop.org
> [mailto:intel-gfx-bounces+jon.bloomfield=intel.com@lists.freedesktop.org]
> On Behalf Of Chris Wilson
> Sent: Friday, July 05, 2013 9:49 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function
> 
> 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 | 79
> ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  8 +++-
>  2 files changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b70ce4e..dff3b93 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -795,6 +795,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
> @@ -2835,6 +2903,8 @@ static void intel_crtc_wait_for_pending_flips(struct
> drm_crtc *crtc)
>  	if (crtc->fb == NULL)
>  		return;
> 
> +	flush_work(&to_intel_crtc(crtc)->vblank_work.work);
> +
>  	WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
> 
>  	wait_event(dev_priv->pending_flip_queue,
> @@ -9097,6 +9167,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);
> 
>  	if (drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256)) @@ -
> 10296,11 +10370,16 @@ void intel_modeset_cleanup(struct drm_device
> *dev)
>  	 */
>  	drm_kms_helper_poll_fini(dev);
> 
> +	/* Clear the vblank worker prior to taking any locks */
> +	flush_scheduled_work();
> +
>  	mutex_lock(&dev->struct_mutex);
> 
>  	intel_unregister_dsm_handler();
> 
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		flush_work(&to_intel_crtc(crtc)->vblank_work.work);
> +
>  		/* Skip inactive CRTCs */
>  		if (!crtc->fb)
>  			continue;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 619caf9..19df4ec 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -314,10 +314,16 @@ struct intel_crtc {
>  	bool primary_disabled; /* is the crtc obscured by a plane? */
>  	bool lowfreq_avail;
>  	struct intel_overlay *overlay;
> -	struct intel_unpin_work *unpin_work;
> 
> +	struct intel_unpin_work *unpin_work;
>  	atomic_t unpin_work_count;
> 
> +	struct intel_crtc_vblank_work {
> +		struct work_struct work;
> +		struct mutex mutex;
> +		struct list_head tasks;
> +	} vblank_work;
> +
>  	/* Display surface base address adjustement for pageflips. Note that
> on
>  	 * gen4+ this only adjusts up to a tile, offsets within a tile are
>  	 * handled in the hw itself (with the TILEOFF register). */
> --
> 1.8.3.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915: Introduce vblank work function
  2013-12-06 10:44   ` Bloomfield, Jon
@ 2013-12-06 12:06     ` Daniel Vetter
  2013-12-06 12:12       ` Bloomfield, Jon
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2013-12-06 12:06 UTC (permalink / raw)
  To: Bloomfield, Jon; +Cc: intel-gfx@lists.freedesktop.org

On Fri, Dec 06, 2013 at 10:44:15AM +0000, Bloomfield, Jon wrote:
> What's the status of this patch ? I can't find any subsequent mention of
> it, but we currently use it in one of our Android development trees. I'm
> trying to work out whether to retain it or replace it.
> 
> Was it killed off, or is it still in the pipeline ?

Stalled atm I think. The overall concept of a vblank worker/timer support
code is still useful imo. I think I've written up all the bikesheds
Chris&I discussed on irc in my other reply.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915: Introduce vblank work function
  2013-12-06 12:06     ` Daniel Vetter
@ 2013-12-06 12:12       ` Bloomfield, Jon
  2013-12-06 13:42         ` Daniel Vetter
  0 siblings, 1 reply; 13+ messages in thread
From: Bloomfield, Jon @ 2013-12-06 12:12 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx@lists.freedesktop.org

Ok thanks. 

To add weight to it becoming official in some form, we're using it for various deferred operations:
	drm/i915: Make plane switching asynchronous
	drm/i915: Asynchronously unpin the old framebuffer after the next vblank	

They aren't my patches but I believe they should be upstreamed in the near future. The claim is that these give a noticeable performance boost.

I'll leave it in and hope it becomes official.

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Friday, December 06, 2013 12:07 PM
> To: Bloomfield, Jon
> Cc: Chris Wilson; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function
> 
> On Fri, Dec 06, 2013 at 10:44:15AM +0000, Bloomfield, Jon wrote:
> > What's the status of this patch ? I can't find any subsequent mention
> > of it, but we currently use it in one of our Android development
> > trees. I'm trying to work out whether to retain it or replace it.
> >
> > Was it killed off, or is it still in the pipeline ?
> 
> Stalled atm I think. The overall concept of a vblank worker/timer support
> code is still useful imo. I think I've written up all the bikesheds Chris&I
> discussed on irc in my other reply.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915: Introduce vblank work function
  2013-12-06 12:12       ` Bloomfield, Jon
@ 2013-12-06 13:42         ` Daniel Vetter
  2013-12-06 16:49           ` Jesse Barnes
  0 siblings, 1 reply; 13+ messages in thread
From: Daniel Vetter @ 2013-12-06 13:42 UTC (permalink / raw)
  To: Bloomfield, Jon; +Cc: intel-gfx@lists.freedesktop.org

On Fri, Dec 06, 2013 at 12:12:21PM +0000, Bloomfield, Jon wrote:
> Ok thanks. 
> 
> To add weight to it becoming official in some form, we're using it for various deferred operations:
> 	drm/i915: Make plane switching asynchronous
> 	drm/i915: Asynchronously unpin the old framebuffer after the next vblank	
> 
> They aren't my patches but I believe they should be upstreamed in the near future. The claim is that these give a noticeable performance boost.
> 
> I'll leave it in and hope it becomes official.

For this stuff the upstream plane is to merge Ville's nuclear pageflip
code, which is the full deal solution for all these issues. I haven't read
his latest wip code to see what exactly he's using for all the vblank
work.
-Daniel

> 
> > -----Original Message-----
> > From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Friday, December 06, 2013 12:07 PM
> > To: Bloomfield, Jon
> > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function
> > 
> > On Fri, Dec 06, 2013 at 10:44:15AM +0000, Bloomfield, Jon wrote:
> > > What's the status of this patch ? I can't find any subsequent mention
> > > of it, but we currently use it in one of our Android development
> > > trees. I'm trying to work out whether to retain it or replace it.
> > >
> > > Was it killed off, or is it still in the pipeline ?
> > 
> > Stalled atm I think. The overall concept of a vblank worker/timer support
> > code is still useful imo. I think I've written up all the bikesheds Chris&I
> > discussed on irc in my other reply.
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] drm/i915: Introduce vblank work function
  2013-12-06 13:42         ` Daniel Vetter
@ 2013-12-06 16:49           ` Jesse Barnes
  0 siblings, 0 replies; 13+ messages in thread
From: Jesse Barnes @ 2013-12-06 16:49 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx@lists.freedesktop.org

On Fri, 6 Dec 2013 14:42:29 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Dec 06, 2013 at 12:12:21PM +0000, Bloomfield, Jon wrote:
> > Ok thanks. 
> > 
> > To add weight to it becoming official in some form, we're using it for various deferred operations:
> > 	drm/i915: Make plane switching asynchronous
> > 	drm/i915: Asynchronously unpin the old framebuffer after the next vblank	
> > 
> > They aren't my patches but I believe they should be upstreamed in the near future. The claim is that these give a noticeable performance boost.
> > 
> > I'll leave it in and hope it becomes official.
> 
> For this stuff the upstream plane is to merge Ville's nuclear pageflip
> code, which is the full deal solution for all these issues. I haven't read
> his latest wip code to see what exactly he's using for all the vblank
> work.

I don't think that should block getting the vblank worker code in along
with the trivial change to the sprite code to avoid a wait_vblank in
the buffer switching case.  We've needed that since this code went in,
but for some reason Chris's code has been stalled the whole time (and
didn't we have a wq patch for the sprite unpin even before that?).

-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-12-06 16:47 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-04 22:07 RFC asynchronous vblank tasks Chris Wilson
2013-07-04 22:07 ` [PATCH 1/5] drm/i915: Asynchronously unpin the old framebuffer after the next vblank Chris Wilson
2013-07-04 22:07 ` [PATCH 2/5] drm/i915/sprite: Make plane switching asynchronous Chris Wilson
2013-07-04 22:07 ` [PATCH 3/5] drm/i915: Synchronize userspace palette LUT (i.e. gamma) changes to vblank Chris Wilson
2013-07-04 22:07 ` [PATCH 4/5] drm/i915: Up/downclock LVDS on vblanks Chris Wilson
2013-07-04 22:07 ` [PATCH 5/5] drm/i915: Boost DMA qos whilst performing uninterruptible waits for the GPU Chris Wilson
2013-07-05  8:42 ` RFC asynchronous vblank tasks Daniel Vetter
2013-07-05  8:48 ` [PATCH] drm/i915: Introduce vblank work function Chris Wilson
2013-12-06 10:44   ` Bloomfield, Jon
2013-12-06 12:06     ` Daniel Vetter
2013-12-06 12:12       ` Bloomfield, Jon
2013-12-06 13:42         ` Daniel Vetter
2013-12-06 16:49           ` Jesse Barnes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox