[PATCH 1/9] drm/i915: Allow mmio updates on all platforms, v3.

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

With intel_pipe_update begin/end we ensure that the mmio updates
don't run during vblank interrupt, using the hw counter we can
be sure that when current vblank count != vblank count at the time
of pipe_update_end the mmio update is complete.

This allows us to use mmio updates on all platforms, using the
update_plane call.

With Chris Wilson's patch to skip waiting for vblanks for
legacy_cursor_update this potentially leaves a small race
condition. In case of !legacy_cursor_update we wait for flips to
complete so there's no race for freeing crtc_state. In case of
legacy_cursor_update there's a check for
work->crtc_state == old_crtc_state. In that case the old_crtc_state
is removed from intel_atomic_state and freed by intel_unpin_work_fn.
This ensures that intel_mmio_flip_work_func never uses a freed pointer
to crtc_state.

Changes since v1:
- Split out the flip_work rename.
Changes since v2:
- Do not break bisect by reverting the stall fix for cursor updates,
  instead add crtc_state to intel_flip_work, and make sure it's not
  freed in intel_atomic_commit for legacy cursor updates.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 118 ++++++++---------------------------
 drivers/gpu/drm/i915/intel_drv.h     |   4 +-
 2 files changed, 29 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9ccd76699f48..ae8036b5fe7c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10976,6 +10976,9 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 	BUG_ON(atomic_read(&crtc->unpin_work_count) == 0);
 	atomic_dec(&crtc->unpin_work_count);
 
+	if (work->free_new_crtc_state)
+		intel_crtc_destroy_state(&crtc->base, &work->new_crtc_state->base);
+
 	kfree(work);
 }
 
@@ -11373,9 +11376,6 @@ static bool use_mmio_flip(struct intel_engine_cs *engine,
 	if (engine == NULL)
 		return true;
 
-	if (INTEL_GEN(engine->i915) < 5)
-		return false;
-
 	if (i915.use_mmio_flip < 0)
 		return false;
 	else if (i915.use_mmio_flip > 0)
@@ -11390,92 +11390,15 @@ static bool use_mmio_flip(struct intel_engine_cs *engine,
 		return engine != i915_gem_request_get_engine(obj->last_write_req);
 }
 
-static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
-			     unsigned int rotation,
-			     struct intel_flip_work *work)
-{
-	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
-	const enum pipe pipe = intel_crtc->pipe;
-	u32 ctl, stride, tile_height;
-
-	ctl = I915_READ(PLANE_CTL(pipe, 0));
-	ctl &= ~PLANE_CTL_TILED_MASK;
-	switch (fb->modifier[0]) {
-	case DRM_FORMAT_MOD_NONE:
-		break;
-	case I915_FORMAT_MOD_X_TILED:
-		ctl |= PLANE_CTL_TILED_X;
-		break;
-	case I915_FORMAT_MOD_Y_TILED:
-		ctl |= PLANE_CTL_TILED_Y;
-		break;
-	case I915_FORMAT_MOD_Yf_TILED:
-		ctl |= PLANE_CTL_TILED_YF;
-		break;
-	default:
-		MISSING_CASE(fb->modifier[0]);
-	}
-
-	/*
-	 * The stride is either expressed as a multiple of 64 bytes chunks for
-	 * linear buffers or in number of tiles for tiled buffers.
-	 */
-	if (intel_rotation_90_or_270(rotation)) {
-		/* stride = Surface height in tiles */
-		tile_height = intel_tile_height(dev_priv, fb->modifier[0], 0);
-		stride = DIV_ROUND_UP(fb->height, tile_height);
-	} else {
-		stride = fb->pitches[0] /
-			intel_fb_stride_alignment(dev_priv, fb->modifier[0],
-						  fb->pixel_format);
-	}
-
-	/*
-	 * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
-	 * PLANE_SURF updates, the update is then guaranteed to be atomic.
-	 */
-	I915_WRITE(PLANE_CTL(pipe, 0), ctl);
-	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
-
-	I915_WRITE(PLANE_SURF(pipe, 0), work->gtt_offset);
-	POSTING_READ(PLANE_SURF(pipe, 0));
-}
-
-static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc,
-			     struct intel_flip_work *work)
-{
-	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_framebuffer *intel_fb =
-		to_intel_framebuffer(intel_crtc->base.primary->fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
-	i915_reg_t reg = DSPCNTR(intel_crtc->plane);
-	u32 dspcntr;
-
-	dspcntr = I915_READ(reg);
-
-	if (obj->tiling_mode != I915_TILING_NONE)
-		dspcntr |= DISPPLANE_TILED;
-	else
-		dspcntr &= ~DISPPLANE_TILED;
-
-	I915_WRITE(reg, dspcntr);
-
-	I915_WRITE(DSPSURF(intel_crtc->plane), work->gtt_offset);
-	POSTING_READ(DSPSURF(intel_crtc->plane));
-}
-
 static void intel_mmio_flip_work_func(struct work_struct *w)
 {
 	struct intel_flip_work *work =
 		container_of(w, struct intel_flip_work, mmio_work);
 	struct intel_crtc *crtc = to_intel_crtc(work->crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	struct intel_framebuffer *intel_fb =
-		to_intel_framebuffer(crtc->base.primary->fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
+	struct drm_i915_gem_object *obj = intel_fb_obj(primary->base.state->fb);
 
 	if (work->flip_queued_req)
 		WARN_ON(__i915_wait_request(work->flip_queued_req,
@@ -11489,13 +11412,9 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
 							    MAX_SCHEDULE_TIMEOUT) < 0);
 
 	intel_pipe_update_start(crtc);
-
-	if (INTEL_GEN(dev_priv) >= 9)
-		skl_do_mmio_flip(crtc, work->rotation, work);
-	else
-		/* use_mmio_flip() retricts MMIO flips to ilk+ */
-		ilk_do_mmio_flip(crtc, work);
-
+	primary->update_plane(&primary->base,
+			      work->new_crtc_state,
+			      to_intel_plane_state(primary->base.state));
 	intel_pipe_update_end(crtc, work);
 }
 
@@ -11622,6 +11541,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	if (work == NULL)
 		return -ENOMEM;
 
+	work->new_crtc_state = to_intel_crtc_state(crtc->state);
+
 	work->event = event;
 	work->crtc = crtc;
 	work->old_fb = old_fb;
@@ -11720,7 +11641,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
 						  obj, 0);
 	work->gtt_offset += intel_crtc->dspaddr_offset;
-	work->rotation = crtc->primary->state->rotation;
 
 	if (mmio_flip) {
 		INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
@@ -13779,6 +13699,20 @@ static int intel_atomic_commit(struct drm_device *dev,
 			modeset_put_power_domains(dev_priv, put_domains[i]);
 
 		intel_modeset_verify_crtc(crtc, old_crtc_state, crtc->state);
+
+		if (state->legacy_cursor_update &&
+		    to_intel_crtc(crtc)->flip_work) {
+			struct intel_flip_work *work;
+
+			spin_lock_irq(&dev->event_lock);
+			work = to_intel_crtc(crtc)->flip_work;
+			if (work && &work->new_crtc_state->base == old_crtc_state) {
+				state->crtcs[i] = NULL;
+				state->crtc_states[i] = NULL;
+				work->free_new_crtc_state = true;
+			}
+			spin_unlock_irq(&dev->event_lock);
+		}
 	}
 
 	if (intel_state->modeset)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9b5f6634c558..ab778193bccd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -976,6 +976,9 @@ struct intel_flip_work {
 	struct work_struct unpin_work;
 	struct work_struct mmio_work;
 
+	struct intel_crtc_state *new_crtc_state;
+	bool free_new_crtc_state;
+
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *old_fb;
 	struct drm_i915_gem_object *pending_flip_obj;
@@ -986,7 +989,6 @@ struct intel_flip_work {
 	struct drm_i915_gem_request *flip_queued_req;
 	u32 flip_queued_vblank;
 	u32 flip_ready_vblank;
-	unsigned int rotation;
 };
 
 struct intel_load_detect_pipe {
-- 
2.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx