[Intel-gfx] [PATCH v4 3/5] drm/i915: Make sprite updates atomic

ville.syrjala at linux.intel.com ville.syrjala at linux.intel.com
Thu Feb 13 16:42:52 CET 2014


From: Ville Syrjälä <ville.syrjala at linux.intel.com>

Add a mechanism by which we can evade the leading edge of vblank. This
guarantees that no two sprite register writes will straddle on either
side of the vblank start, and that means all the writes will be latched
together in one atomic operation.

We do the vblank evade by checking the scanline counter, and if it's too
close to the start of vblank (too close has been hardcoded to 100usec
for now), we will wait for the vblank start to pass. In order to
eliminate random delayes from the rest of the system, we operate with
interrupts disabled, except when waiting for the vblank obviously.

Note that we now go digging through pipe_to_crtc_mapping[] in the
vblank interrupt handler, which is a bit dangerous since we set up
interrupts before the crtcs. However in this case since it's the vblank
interrupt, we don't actually unmask it until some piece of code
requests it.

v2: preempt_check_resched() calls after local_irq_enable() (Jesse)
    Hook up the vblank irq stuff on BDW as well
v3: Pass intel_crtc instead of drm_crtc (Daniel)
    Warn if crtc.mutex isn't locked (Daniel)
    Add an explicit compiler barrier and document the barriers (Daniel)
    Note the irq vs. modeset setup madness in the commit message (Daniel)
v4: Use prepare_to_wait() & co. directly and eliminate vbl_received

Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c      | 27 ++++++++--
 drivers/gpu/drm/i915/intel_display.c |  2 +
 drivers/gpu/drm/i915/intel_drv.h     |  2 +
 drivers/gpu/drm/i915/intel_sprite.c  | 95 ++++++++++++++++++++++++++++++++++++
 4 files changed, 122 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ffeda27..578747a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1530,6 +1530,13 @@ static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
 	}
 }
 
+static void intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
+{
+	struct intel_crtc *crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
+
+	wake_up(&crtc->vbl_wait);
+}
+
 static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1580,8 +1587,10 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 	spin_unlock(&dev_priv->irq_lock);
 
 	for_each_pipe(pipe) {
-		if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS)
+		if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS) {
 			drm_handle_vblank(dev, pipe);
+			intel_pipe_handle_vblank(dev, pipe);
+		}
 
 		if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV) {
 			intel_prepare_page_flip(dev, pipe);
@@ -1807,8 +1816,10 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
 		DRM_ERROR("Poison interrupt\n");
 
 	for_each_pipe(pipe) {
-		if (de_iir & DE_PIPE_VBLANK(pipe))
+		if (de_iir & DE_PIPE_VBLANK(pipe)) {
 			drm_handle_vblank(dev, pipe);
+			intel_pipe_handle_vblank(dev, pipe);
+		}
 
 		if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe))
 			if (intel_set_cpu_fifo_underrun_reporting(dev, pipe, false))
@@ -1857,8 +1868,10 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
 		intel_opregion_asle_intr(dev);
 
 	for_each_pipe(i) {
-		if (de_iir & (DE_PIPE_VBLANK_IVB(i)))
+		if (de_iir & (DE_PIPE_VBLANK_IVB(i))) {
 			drm_handle_vblank(dev, i);
+			intel_pipe_handle_vblank(dev, i);
+		}
 
 		/* plane/pipes map 1:1 on ilk+ */
 		if (de_iir & DE_PLANE_FLIP_DONE_IVB(i)) {
@@ -2000,8 +2013,10 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg)
 			continue;
 
 		pipe_iir = I915_READ(GEN8_DE_PIPE_IIR(pipe));
-		if (pipe_iir & GEN8_PIPE_VBLANK)
+		if (pipe_iir & GEN8_PIPE_VBLANK) {
 			drm_handle_vblank(dev, pipe);
+			intel_pipe_handle_vblank(dev, pipe);
+		}
 
 		if (pipe_iir & GEN8_PIPE_FLIP_DONE) {
 			intel_prepare_page_flip(dev, pipe);
@@ -3274,6 +3289,8 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
 	if (!drm_handle_vblank(dev, pipe))
 		return false;
 
+	intel_pipe_handle_vblank(dev, pipe);
+
 	if ((iir & flip_pending) == 0)
 		return false;
 
@@ -3457,6 +3474,8 @@ static bool i915_handle_vblank(struct drm_device *dev,
 	if (!drm_handle_vblank(dev, pipe))
 		return false;
 
+	intel_pipe_handle_vblank(dev, pipe);
+
 	if ((iir & flip_pending) == 0)
 		return false;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ac05da4..d35952b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10298,6 +10298,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 		intel_crtc->plane = !pipe;
 	}
 
+	init_waitqueue_head(&intel_crtc->vbl_wait);
+
 	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
 	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
 	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a471d53..1e6dae4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -376,6 +376,8 @@ struct intel_crtc {
 		/* watermarks currently being used  */
 		struct intel_pipe_wm active;
 	} wm;
+
+	wait_queue_head_t vbl_wait;
 };
 
 struct intel_plane_wm_parameters {
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 336ae6c..f455a2e 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -37,6 +37,71 @@
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
 
+static unsigned int usecs_to_scanlines(const struct drm_display_mode *mode, unsigned int usecs)
+{
+	/* paranoia */
+	if (!mode->crtc_htotal)
+		return 1;
+
+	return DIV_ROUND_UP(usecs * mode->crtc_clock, 1000 * mode->crtc_htotal);
+}
+
+static void intel_pipe_update_start(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	const struct drm_display_mode *mode = &crtc->config.adjusted_mode;
+	enum pipe pipe = crtc->pipe;
+	/* FIXME needs to be calibrated sensibly */
+	unsigned int min = mode->crtc_vblank_start - usecs_to_scanlines(mode, 100);
+	unsigned int max = mode->crtc_vblank_start - 1;
+	long timeout = msecs_to_jiffies_timeout(1);
+	unsigned int scanline;
+	DEFINE_WAIT(wait);
+
+	WARN_ON(!mutex_is_locked(&crtc->base.mutex));
+
+	if (WARN_ON(drm_vblank_get(dev, pipe)))
+		return;
+
+	local_irq_disable();
+
+	for (;;) {
+		/*
+		 * prepare_to_wait() has a memory barrier, which guarantees
+		 * other CPUs can see the task state update by the time we
+		 * read the scanline.
+		 */
+		prepare_to_wait(&crtc->vbl_wait, &wait, TASK_UNINTERRUPTIBLE);
+
+		scanline = intel_get_crtc_scanline(crtc);
+		if (scanline < min || scanline > max)
+			break;
+
+		if (timeout <= 0) {
+			DRM_ERROR("Potential atomic update failure on pipe %c\n",
+				  pipe_name(crtc->pipe));
+			break;
+		}
+
+		local_irq_enable();
+		preempt_check_resched();
+
+		timeout = schedule_timeout(timeout);
+
+		local_irq_disable();
+	}
+
+	finish_wait(&crtc->vbl_wait, &wait);
+
+	drm_vblank_put(dev, pipe);
+}
+
+static void intel_pipe_update_end(struct intel_crtc *crtc)
+{
+	local_irq_enable();
+	preempt_check_resched();
+}
+
 static void
 vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 		 struct drm_framebuffer *fb,
@@ -48,6 +113,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 	struct drm_device *dev = dplane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(dplane);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_plane->pipe;
 	int plane = intel_plane->plane;
 	u32 sprctl;
@@ -131,6 +197,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 							fb->pitches[0]);
 	linear_offset -= sprsurf_offset;
 
+	intel_pipe_update_start(intel_crtc);
+
 	I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
 	I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
 
@@ -144,6 +212,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 	I915_WRITE(SPSURF(pipe, plane), i915_gem_obj_ggtt_offset(obj) +
 		   sprsurf_offset);
 	POSTING_READ(SPSURF(pipe, plane));
+
+	intel_pipe_update_end(intel_crtc);
 }
 
 static void
@@ -152,15 +222,20 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	struct drm_device *dev = dplane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(dplane);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_plane->pipe;
 	int plane = intel_plane->plane;
 
+	intel_pipe_update_start(intel_crtc);
+
 	I915_WRITE(SPCNTR(pipe, plane), I915_READ(SPCNTR(pipe, plane)) &
 		   ~SP_ENABLE);
 	/* Activate double buffered register update */
 	I915_WRITE(SPSURF(pipe, plane), 0);
 	POSTING_READ(SPSURF(pipe, plane));
 
+	intel_pipe_update_end(intel_crtc);
+
 	intel_update_sprite_watermarks(dplane, crtc, 0, 0, false, false);
 }
 
@@ -226,6 +301,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_plane->pipe;
 	u32 sprctl, sprscale = 0;
 	unsigned long sprsurf_offset, linear_offset;
@@ -299,6 +375,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 					       pixel_size, fb->pitches[0]);
 	linear_offset -= sprsurf_offset;
 
+	intel_pipe_update_start(intel_crtc);
+
 	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
 	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
 
@@ -318,6 +396,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	I915_WRITE(SPRSURF(pipe),
 		   i915_gem_obj_ggtt_offset(obj) + sprsurf_offset);
 	POSTING_READ(SPRSURF(pipe));
+
+	intel_pipe_update_end(intel_crtc);
 }
 
 static void
@@ -326,8 +406,11 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_plane->pipe;
 
+	intel_pipe_update_start(intel_crtc);
+
 	I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);
 	/* Can't leave the scaler enabled... */
 	if (intel_plane->can_scale)
@@ -336,6 +419,8 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	I915_WRITE(SPRSURF(pipe), 0);
 	POSTING_READ(SPRSURF(pipe));
 
+	intel_pipe_update_end(intel_crtc);
+
 	/*
 	 * Avoid underruns when disabling the sprite.
 	 * FIXME remove once watermark updates are done properly.
@@ -410,6 +495,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_plane->pipe;
 	unsigned long dvssurf_offset, linear_offset;
 	u32 dvscntr, dvsscale;
@@ -478,6 +564,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 					       pixel_size, fb->pitches[0]);
 	linear_offset -= dvssurf_offset;
 
+	intel_pipe_update_start(intel_crtc);
+
 	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
 	I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
 
@@ -492,6 +580,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	I915_WRITE(DVSSURF(pipe),
 		   i915_gem_obj_ggtt_offset(obj) + dvssurf_offset);
 	POSTING_READ(DVSSURF(pipe));
+
+	intel_pipe_update_end(intel_crtc);
 }
 
 static void
@@ -500,8 +590,11 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_plane->pipe;
 
+	intel_pipe_update_start(intel_crtc);
+
 	I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE);
 	/* Disable the scaler */
 	I915_WRITE(DVSSCALE(pipe), 0);
@@ -509,6 +602,8 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	I915_WRITE(DVSSURF(pipe), 0);
 	POSTING_READ(DVSSURF(pipe));
 
+	intel_pipe_update_end(intel_crtc);
+
 	/*
 	 * Avoid underruns when disabling the sprite.
 	 * FIXME remove once watermark updates are done properly.
-- 
1.8.3.2




More information about the Intel-gfx mailing list