[Intel-gfx] [PATCH 3/2] Revert "drm/i915: Allow nonblocking update of pageflips."

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Mon May 23 14:08:58 UTC 2016


This reverts commit d55dbd06bb5e1399aba9ab5227465339d1bbefff.

It lacks a description, removes the flip trace point,
doesn't handle -EBUSY if a flip is already queued
and needs to be redone.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
Patch 3/2 which CI probably doesn't handle correctly,
so trybot results should be on https://patchwork.freedesktop.org/series/7570/

 drivers/gpu/drm/i915/intel_display.c | 350 ++++++++++++++++++++++++++---------
 1 file changed, 266 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 21c0a2f3102b..f49ef629c4a8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -108,6 +108,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
+static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
+static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
 	struct intel_crtc_state *crtc_state);
 static void skylake_pfit_enable(struct intel_crtc *crtc);
@@ -10984,7 +10986,7 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_request *req;
-	int i, ret;
+	int i;
 
 	if (!needs_modeset(&crtc_state->base) && crtc_state->update_pipe) {
 		work->put_power_domains =
@@ -11006,14 +11008,7 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
 					    &dev_priv->rps.mmioflips));
 	}
 
-	ret = drm_crtc_vblank_get(crtc);
-	I915_STATE_WARN(ret < 0, "enabling vblank failed with %i\n", ret);
-
-	if (work->num_planes &&
-	    work->old_plane_state[0]->base.plane == crtc->primary)
-		intel_fbc_enable(intel_crtc, work->new_crtc_state, work->new_plane_state[0]);
-
-	intel_frontbuffer_flip_prepare(dev, work->fb_bits);
+	intel_frontbuffer_flip_prepare(dev, crtc_state->fb_bits);
 
 	intel_pipe_update_start(intel_crtc);
 	if (!needs_modeset(&crtc_state->base)) {
@@ -11032,15 +11027,206 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
 		struct intel_plane_state *new_plane_state = work->new_plane_state[i];
 		struct intel_plane *plane = to_intel_plane(new_plane_state->base.plane);
 
-		if (new_plane_state->visible)
-			plane->update_plane(&plane->base, crtc_state, new_plane_state);
-		else
-			plane->disable_plane(&plane->base, crtc);
+		plane->update_plane(&plane->base, crtc_state, new_plane_state);
 	}
 
 	intel_pipe_update_end(intel_crtc, work);
 }
 
+static struct fence *intel_get_excl_fence(struct drm_i915_gem_object *obj)
+{
+	struct reservation_object *resv;
+
+
+	if (!obj->base.dma_buf)
+		return NULL;
+
+	resv = obj->base.dma_buf->resv;
+
+	/* For framebuffer backed by dmabuf, wait for fence */
+	while (1) {
+		struct fence *fence_excl, *ret = NULL;
+
+		rcu_read_lock();
+
+		fence_excl = rcu_dereference(resv->fence_excl);
+		if (fence_excl)
+			ret = fence_get_rcu(fence_excl);
+
+		rcu_read_unlock();
+
+		if (ret == fence_excl)
+			return ret;
+	}
+}
+
+static int intel_crtc_page_flip(struct drm_crtc *crtc,
+				struct drm_framebuffer *fb,
+				struct drm_pending_vblank_event *event,
+				uint32_t page_flip_flags)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_plane_state *old_state, *new_state = NULL;
+	struct drm_crtc_state *new_crtc_state = NULL;
+	struct drm_framebuffer *old_fb = crtc->primary->state->fb;
+	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_plane *primary = crtc->primary;
+	struct intel_flip_work *work;
+	int ret;
+
+	old_state = crtc->primary->state;
+
+	if (!crtc->state->active)
+		return -EINVAL;
+
+	/*
+	 * drm_mode_page_flip_ioctl() should already catch this, but double
+	 * check to be safe.  In the future we may enable pageflipping from
+	 * a disabled primary plane.
+	 */
+	if (WARN_ON(intel_fb_obj(old_fb) == NULL))
+		return -EBUSY;
+
+	/* Can't change pixel format via MI display flips. */
+	if (fb->pixel_format != old_fb->pixel_format)
+		return -EINVAL;
+
+	/*
+	 * TILEOFF/LINOFF registers can't be changed via MI display flips.
+	 * Note that pitch changes could also affect these register.
+	 */
+	if (INTEL_INFO(dev)->gen > 3 &&
+	    (fb->offsets[0] != old_fb->offsets[0] ||
+	     fb->pitches[0] != old_fb->pitches[0]))
+		return -EINVAL;
+
+	work = kzalloc(sizeof(*work), GFP_KERNEL);
+	new_crtc_state = intel_crtc_duplicate_state(crtc);
+	new_state = intel_plane_duplicate_state(primary);
+
+	if (!work || !new_crtc_state || !new_state) {
+		ret = -ENOMEM;
+		goto cleanup;
+	}
+
+	drm_framebuffer_unreference(new_state->fb);
+	drm_framebuffer_reference(fb);
+	new_state->fb = fb;
+
+	work->event = event;
+	INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
+	INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
+
+	work->new_crtc_state = to_intel_crtc_state(new_crtc_state);
+	work->old_crtc_state = intel_crtc->config;
+
+	work->fb_bits = to_intel_plane(primary)->frontbuffer_bit;
+	work->new_crtc_state->fb_bits = work->fb_bits;
+
+	work->can_async_unpin = true;
+	work->num_planes = 1;
+	work->old_plane_state[0] = to_intel_plane_state(old_state);
+	work->new_plane_state[0] = to_intel_plane_state(new_state);
+
+	/* Step 1: vblank waiting and workqueue throttling,
+	 * similar to intel_atomic_prepare_commit
+	 */
+	ret = drm_crtc_vblank_get(crtc);
+	if (ret)
+		goto cleanup;
+
+	/* We borrow the event spin lock for protecting flip_work */
+	spin_lock_irq(&dev->event_lock);
+	if (!list_empty(&intel_crtc->flip_work)) {
+		struct intel_flip_work *old_work;
+
+		old_work = list_last_entry(&intel_crtc->flip_work,
+					   struct intel_flip_work, head);
+
+		/* Before declaring the flip queue wedged, check if
+		 * the hardware completed the operation behind our backs.
+		 */
+		if (pageflip_finished(intel_crtc, old_work)) {
+			DRM_DEBUG_DRIVER("flip queue: previous flip completed, continuing\n");
+			page_flip_completed(intel_crtc, old_work);
+		} else {
+			DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
+			spin_unlock_irq(&dev->event_lock);
+
+			ret = -EBUSY;
+			goto cleanup_vblank;
+		}
+	}
+	list_add_tail(&work->head, &intel_crtc->flip_work);
+	spin_unlock_irq(&dev->event_lock);
+
+	if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
+		flush_workqueue(dev_priv->wq);
+
+	/* step 2, similar to intel_prepare_plane_fb */
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		goto cleanup_work;
+
+	ret = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
+	if (ret)
+		goto cleanup_unlock;
+
+	i915_gem_track_fb(intel_fb_obj(old_fb), obj,
+			  to_intel_plane(primary)->frontbuffer_bit);
+
+	/* point of no return, swap state */
+	primary->state = new_state;
+	crtc->state = new_crtc_state;
+	intel_crtc->config = to_intel_crtc_state(new_crtc_state);
+	primary->fb = fb;
+
+	/* scheduling flip work */
+	atomic_inc(&intel_crtc->unpin_work_count);
+
+	if (obj->last_write_req &&
+	    !i915_gem_request_completed(obj->last_write_req, true))
+		i915_gem_request_assign(&work->old_plane_state[0]->wait_req,
+					obj->last_write_req);
+
+	if (obj->base.dma_buf)
+		work->old_plane_state[0]->base.fence = intel_get_excl_fence(obj);
+
+	intel_fbc_pre_update(intel_crtc,
+			     to_intel_crtc_state(new_crtc_state),
+			     to_intel_plane_state(new_state));
+
+	schedule_work(&work->mmio_work);
+
+	mutex_unlock(&dev->struct_mutex);
+
+	trace_i915_flip_request(intel_crtc->plane, obj);
+
+	return 0;
+
+cleanup_unlock:
+	mutex_unlock(&dev->struct_mutex);
+cleanup_work:
+	spin_lock_irq(&dev->event_lock);
+	list_del(&work->head);
+	spin_unlock_irq(&dev->event_lock);
+
+cleanup_vblank:
+	drm_crtc_vblank_put(crtc);
+cleanup:
+	if (new_state)
+		intel_plane_destroy_state(primary, new_state);
+
+	if (new_crtc_state)
+		intel_crtc_destroy_state(crtc, new_crtc_state);
+
+	intel_free_flip_work(work);
+	return ret;
+}
+
+
 /**
  * intel_wm_need_update - Check whether watermarks need updating
  * @plane: drm plane
@@ -11317,6 +11503,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 
 static const struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.mode_set_base_atomic = intel_pipe_set_base_atomic,
+	.atomic_begin = intel_begin_crtc_commit,
+	.atomic_flush = intel_finish_crtc_commit,
 	.atomic_check = intel_crtc_atomic_check,
 };
 
@@ -12736,6 +12924,11 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
 	struct drm_crtc *crtc;
 	int i, ret;
 
+	if (nonblock) {
+		DRM_DEBUG_KMS("i915 does not yet support nonblocking commit\n");
+		return -EINVAL;
+	}
+
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		struct intel_flip_work *work;
@@ -12777,11 +12970,6 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
 		}
 	}
 
-	if (intel_state->modeset && nonblock) {
-		DRM_DEBUG_ATOMIC("Nonblock modesets are not yet supported!\n");
-		return -EINVAL;
-	}
-
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
 	if (ret)
 		return ret;
@@ -12935,41 +13123,13 @@ static void intel_schedule_unpin(struct drm_crtc *crtc,
 	queue_work(dev_priv->wq, &work->unpin_work);
 }
 
-static void intel_schedule_flip(struct drm_crtc *crtc,
-				struct intel_atomic_state *state,
-				struct intel_flip_work *work,
-				bool nonblock)
-{
-	struct intel_crtc_state *crtc_state = work->new_crtc_state;
-
-	if (crtc_state->base.planes_changed ||
-	    needs_modeset(&crtc_state->base) ||
-	    crtc_state->update_pipe) {
-		if (nonblock)
-			schedule_work(&work->mmio_work);
-		else
-			intel_mmio_flip_work_func(&work->mmio_work);
-	} else {
-		int ret;
-
-		ret = drm_crtc_vblank_get(crtc);
-		I915_STATE_WARN(ret < 0, "enabling vblank failed with %i\n", ret);
-
-		work->flip_queued_vblank = intel_crtc_get_vblank_counter(to_intel_crtc(crtc));
-		smp_mb__before_atomic();
-		atomic_set(&work->pending, 1);
-	}
-}
-
 static void intel_schedule_update(struct drm_crtc *crtc,
 				  struct intel_atomic_state *state,
-				  struct intel_flip_work *work,
-				  bool nonblock)
+				  struct intel_flip_work *work)
 {
 	struct drm_device *dev = crtc->dev;
-	struct intel_crtc_state *pipe_config = work->new_crtc_state;
 
-	if (!pipe_config->base.active && work->can_async_unpin) {
+	if (work->can_async_unpin) {
 		INIT_LIST_HEAD(&work->head);
 		intel_schedule_unpin(crtc, state, work);
 		return;
@@ -12979,10 +13139,7 @@ static void intel_schedule_update(struct drm_crtc *crtc,
 	list_add_tail(&work->head, &to_intel_crtc(crtc)->flip_work);
 	spin_unlock_irq(&dev->event_lock);
 
-	if (!pipe_config->base.active)
-		intel_schedule_unpin(crtc, state, work);
-	else
-		intel_schedule_flip(crtc, state, work, nonblock);
+	intel_schedule_unpin(crtc, state, work);
 }
 
 /**
@@ -13077,9 +13234,11 @@ static int intel_atomic_commit(struct drm_device *dev,
 
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
-		struct intel_flip_work *work = intel_state->work[i];
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		bool modeset = needs_modeset(crtc->state);
+		struct intel_crtc_state *pipe_config =
+			to_intel_crtc_state(crtc->state);
+		bool update_pipe = !modeset && pipe_config->update_pipe;
 
 		crtc_mask |= drm_crtc_mask(crtc);
 
@@ -13091,6 +13250,22 @@ static int intel_atomic_commit(struct drm_device *dev,
 		if (!modeset)
 			intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
 
+		if (crtc->state->active &&
+		    drm_atomic_get_existing_plane_state(state, crtc->primary))
+			intel_fbc_enable(intel_crtc, pipe_config, to_intel_plane_state(crtc->primary->state));
+
+		if (crtc->state->active &&
+		    (crtc->state->planes_changed || update_pipe))
+			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
+	}
+
+	/* FIXME: add subpixel order */
+
+	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+		struct intel_flip_work *work =
+			intel_state->work[i];
+
 		if (!work) {
 			if (!list_empty_careful(&intel_crtc->flip_work)) {
 				spin_lock_irq(&dev->event_lock);
@@ -13110,7 +13285,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 
 		intel_state->work[i] = NULL;
 		intel_prepare_work(crtc, work, state, old_crtc_state);
-		intel_schedule_update(crtc, intel_state, work, nonblock);
+		intel_schedule_update(crtc, intel_state, work);
 	}
 
 	if (!nonblock && !state->legacy_cursor_update) {
@@ -13119,8 +13294,6 @@ static int intel_atomic_commit(struct drm_device *dev,
 				intel_crtc_wait_for_pending_flips(crtc, false);
 	}
 
-	/* FIXME: add subpixel order */
-
 	drm_atomic_state_free(state);
 
 	/* As one of the primary mmio accessors, KMS has a high likelihood
@@ -13184,38 +13357,11 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.set_property = drm_atomic_helper_crtc_set_property,
 	.destroy = intel_crtc_destroy,
-	.page_flip = drm_atomic_helper_page_flip,
+	.page_flip = intel_crtc_page_flip,
 	.atomic_duplicate_state = intel_crtc_duplicate_state,
 	.atomic_destroy_state = intel_crtc_destroy_state,
 };
 
-static struct fence *intel_get_excl_fence(struct drm_i915_gem_object *obj)
-{
-	struct reservation_object *resv;
-
-
-	if (!obj->base.dma_buf)
-		return NULL;
-
-	resv = obj->base.dma_buf->resv;
-
-	/* For framebuffer backed by dmabuf, wait for fence */
-	while (1) {
-		struct fence *fence_excl, *ret = NULL;
-
-		rcu_read_lock();
-
-		fence_excl = rcu_dereference(resv->fence_excl);
-		if (fence_excl)
-			ret = fence_get_rcu(fence_excl);
-
-		rcu_read_unlock();
-
-		if (ret == fence_excl)
-			return ret;
-	}
-}
-
 /**
  * intel_prepare_plane_fb - Prepare fb for usage on plane
  * @plane: drm plane to prepare for
@@ -13402,6 +13548,42 @@ intel_check_primary_plane(struct drm_plane *plane,
 					     &state->visible);
 }
 
+static void intel_begin_crtc_commit(struct drm_crtc *crtc,
+				    struct drm_crtc_state *old_crtc_state)
+{
+	struct drm_device *dev = crtc->dev;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *old_intel_state =
+		to_intel_crtc_state(old_crtc_state);
+	bool modeset = needs_modeset(crtc->state);
+
+	intel_frontbuffer_flip_prepare(dev, to_intel_crtc_state(crtc->state)->fb_bits);
+
+	/* Perform vblank evasion around commit operation */
+	intel_pipe_update_start(intel_crtc);
+
+	if (modeset)
+		return;
+
+	if (crtc->state->color_mgmt_changed || to_intel_crtc_state(crtc->state)->update_pipe) {
+		intel_color_set_csc(crtc->state);
+		intel_color_load_luts(crtc->state);
+	}
+
+	if (to_intel_crtc_state(crtc->state)->update_pipe)
+		intel_update_pipe_config(intel_crtc, old_intel_state);
+	else if (INTEL_INFO(dev)->gen >= 9)
+		skl_detach_scalers(intel_crtc);
+}
+
+static void intel_finish_crtc_commit(struct drm_crtc *crtc,
+				     struct drm_crtc_state *old_crtc_state)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	intel_pipe_update_end(intel_crtc, NULL);
+}
+
 /**
  * intel_plane_destroy - destroy a plane
  * @plane: plane to destroy
-- 
2.5.5




More information about the Intel-gfx mailing list