[PATCH 1/5] drm/irq: Add drm_crtc_vblank_reset

* [PATCH 1/5] drm/irq: Add drm_crtc_vblank_reset
@ 2015-02-13 20:03 Daniel Vetter
  2015-02-13 20:03 ` [PATCH 2/5] drm/i915: Drop pipe_enable checks in vblank funcs Daniel Vetter
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Daniel Vetter @ 2015-02-13 20:03 UTC (permalink / raw)
  To: Intel Graphics Development, DRI Development
  Cc: Daniel Vetter, Laurent Pinchart, Daniel Vetter

At driver load we need to tell the vblank code about the state of the
pipes, so that the logic around reject vblank_get when the pipe is off
works correctly.

Thus far i915 used drm_vblank_off, but one of the side-effects of it
is that it also saves the vblank counter. And for that it calls down
into the ->get_vblank_counter hook. Which isn't really a good idea
when the pipe is off for a few reasons:
- With runtime pm the register might not respond.
- If the pipe is off some datastructures might not be around or
  unitialized.

The later is what blew up on gen3: We look at intel_crtc->config to
compute the vblank counter, and for a disabled pipe at boot-up that's
just not there. Thus far this was papered over by a check for
intel_crtc->active, but I want to get rid of that (since it's fairly
race, vblank hooks are called from all kinds of places).

So prep for that by adding a _reset functions which only does what we
really need to be done at driver load: Mark the vblank pipe as off,
but don't do any vblank counter saving or event flushing - neither of
that is required.

v2: Clarify the code flow slightly as suggested by Ville.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_irq.c            | 32 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  6 +++---
 include/drm/drmP.h                   |  1 +
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 75647e7f012b..1e5fb1b994d7 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1226,6 +1226,38 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
 EXPORT_SYMBOL(drm_crtc_vblank_off);
 
 /**
+ * drm_crtc_vblank_reset - reset vblank state to off on a CRTC
+ * @crtc: CRTC in question
+ *
+ * Drivers can use this function to reset the vblank state to off at load time.
+ * Drivers should use this together with the drm_crtc_vblank_off() and
+ * drm_crtc_vblank_on() functions. The diffrence comparet to
+ * drm_crtc_vblank_off() is that this function doesn't save the vblank counter
+ * and hence doesn't need to call any driver hooks.
+ */
+void drm_crtc_vblank_reset(struct drm_crtc *drm_crtc)
+{
+	struct drm_device *dev = drm_crtc->dev;
+	unsigned long irqflags;
+	int crtc = drm_crtc_index(drm_crtc);
+	struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
+
+	spin_lock_irqsave(&dev->vbl_lock, irqflags);
+	/*
+	 * Prevent subsequent drm_vblank_get() from enabling the vblank
+	 * interrupt by bumping the refcount.
+	 */
+	if (!vblank->inmodeset) {
+		atomic_inc(&vblank->refcount);
+		vblank->inmodeset = 1;
+	}
+	spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
+
+	WARN_ON(!list_empty(&dev->vblank_event_list));
+}
+EXPORT_SYMBOL(drm_crtc_vblank_reset);
+
+/**
  * drm_vblank_on - enable vblank events on a CRTC
  * @dev: DRM device
  * @crtc: CRTC in question
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 423ef959264d..8dcf6b512236 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13294,11 +13294,11 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 	I915_WRITE(reg, I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
 
 	/* restore vblank interrupts to correct state */
+	drm_crtc_vblank_reset(&crtc->base);
 	if (crtc->active) {
 		update_scanline_offset(crtc);
-		drm_vblank_on(dev, crtc->pipe);
-	} else
-		drm_vblank_off(dev, crtc->pipe);
+		drm_crtc_vblank_on(&crtc->base);
+	}
 
 	/* We need to sanitize the plane -> pipe mapping first because this will
 	 * disable the crtc (and hence change the state) if it is wrong. Note
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index e928625a9da0..54c6ea1e5866 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -922,6 +922,7 @@ extern void drm_crtc_wait_one_vblank(struct drm_crtc *crtc);
 extern void drm_vblank_off(struct drm_device *dev, int crtc);
 extern void drm_vblank_on(struct drm_device *dev, int crtc);
 extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
+extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
 extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
 extern void drm_vblank_cleanup(struct drm_device *dev);
 
-- 
1.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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