diff mbox

[16/19] drm/i915: Make unpin async.

Message ID 1461052359-24154-17-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maarten Lankhorst April 19, 2016, 7:52 a.m. UTC
All of intel_post_plane_update is handled there now, so move it over.
This is run after the hw state checker because it can't handle checking
crtc's separately yet.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c  |   7 +
 drivers/gpu/drm/i915/intel_display.c | 288 +++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h     |   4 +-
 3 files changed, 170 insertions(+), 129 deletions(-)

Comments

Lionel Landwerlin April 25, 2016, 4:26 p.m. UTC | #1
On 19/04/16 08:52, Maarten Lankhorst wrote:
> All of intel_post_plane_update is handled there now, so move it over.
> This is run after the hw state checker because it can't handle checking
> crtc's separately yet.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_atomic.c  |   7 +
>   drivers/gpu/drm/i915/intel_display.c | 288 +++++++++++++++++++----------------
>   drivers/gpu/drm/i915/intel_drv.h     |   4 +-
>   3 files changed, 170 insertions(+), 129 deletions(-)

Hey Maarten,

Anyway you could send a rebased version of this patch?
(or point at a branch anywhere)

Thanks!

-
Lionel
Lionel Landwerlin April 25, 2016, 4:26 p.m. UTC | #2
On 19/04/16 08:52, Maarten Lankhorst wrote:
> All of intel_post_plane_update is handled there now, so move it over.
> This is run after the hw state checker because it can't handle checking
> crtc's separately yet.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_atomic.c  |   7 +
>   drivers/gpu/drm/i915/intel_display.c | 288 +++++++++++++++++++----------------
>   drivers/gpu/drm/i915/intel_drv.h     |   4 +-
>   3 files changed, 170 insertions(+), 129 deletions(-)

Hey Maarten,

Anyway you could send a rebased version of this patch?
(or point at a branch anywhere)

Thanks!

-
Lionel
Maarten Lankhorst April 26, 2016, 7:14 a.m. UTC | #3
Op 25-04-16 om 18:26 schreef Lionel Landwerlin:
> On 19/04/16 08:52, Maarten Lankhorst wrote:
>> All of intel_post_plane_update is handled there now, so move it over.
>> This is run after the hw state checker because it can't handle checking
>> crtc's separately yet.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_atomic.c  |   7 +
>>   drivers/gpu/drm/i915/intel_display.c | 288 +++++++++++++++++++----------------
>>   drivers/gpu/drm/i915/intel_drv.h     |   4 +-
>>   3 files changed, 170 insertions(+), 129 deletions(-)
>
> Hey Maarten,
>
> Anyway you could send a rebased version of this patch?
> (or point at a branch anywhere) 
https://cgit.freedesktop.org/~mlankhorst/linux/log/?h=rework-page-flip
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 50ff90aea721..83c927eb8566 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -311,6 +311,13 @@  intel_atomic_state_alloc(struct drm_device *dev)
 void intel_atomic_state_clear(struct drm_atomic_state *s)
 {
 	struct intel_atomic_state *state = to_intel_atomic_state(s);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(state->work); i++) {
+		kfree(state->work[i]);
+		state->work[i] = NULL;
+	}
+
 	drm_atomic_state_default_clear(&state->base);
 	state->dpll_set = state->modeset = false;
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6feb8c6ccb8c..02993aed9780 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4647,39 +4647,6 @@  intel_pre_disable_primary_noatomic(struct drm_crtc *crtc)
 	}
 }
 
-static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
-{
-	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
-	struct drm_atomic_state *old_state = old_crtc_state->base.state;
-	struct intel_crtc_state *pipe_config =
-		to_intel_crtc_state(crtc->base.state);
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_plane *primary = crtc->base.primary;
-	struct drm_plane_state *old_pri_state =
-		drm_atomic_get_existing_plane_state(old_state, primary);
-
-	intel_frontbuffer_flip(dev, pipe_config->fb_bits);
-
-	crtc->wm.cxsr_allowed = true;
-
-	if (pipe_config->update_wm_post && pipe_config->base.active)
-		intel_update_watermarks(&crtc->base);
-
-	if (old_pri_state) {
-		struct intel_plane_state *primary_state =
-			to_intel_plane_state(primary->state);
-		struct intel_plane_state *old_primary_state =
-			to_intel_plane_state(old_pri_state);
-
-		intel_fbc_post_update(crtc);
-
-		if (primary_state->visible &&
-		    (needs_modeset(&pipe_config->base) ||
-		     !old_primary_state->visible))
-			intel_post_enable_primary(&crtc->base);
-	}
-}
-
 static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
@@ -5289,18 +5256,21 @@  modeset_get_crtc_power_domains(struct drm_crtc *crtc,
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum intel_display_power_domain domain;
-	unsigned long domains, new_domains, old_domains;
+	unsigned long domains, new_domains, old_domains, ms_domain = 0;
 
 	old_domains = intel_crtc->enabled_power_domains;
 	intel_crtc->enabled_power_domains = new_domains =
 		get_crtc_power_domains(crtc, crtc_state);
 
-	domains = new_domains & ~old_domains;
+	if (needs_modeset(&crtc_state->base))
+		ms_domain = BIT(POWER_DOMAIN_MODESET);
+
+	domains = (new_domains & ~old_domains) | ms_domain;
 
 	for_each_power_domain(domain, domains)
 		intel_display_power_get(dev_priv, domain);
 
-	return old_domains & ~new_domains;
+	return (old_domains & ~new_domains) | ms_domain;
 }
 
 static void modeset_put_power_domains(struct drm_i915_private *dev_priv,
@@ -6362,6 +6332,12 @@  int intel_display_suspend(struct drm_device *dev)
 		DRM_ERROR("Suspending crtc's failed with %i\n", ret);
 	else
 		dev_priv->modeset_restore_state = state;
+
+	/*
+	 * Make sure all unpin_work completes before returning.
+	 */
+	flush_workqueue(dev_priv->wq);
+
 	return ret;
 }
 
@@ -10965,9 +10941,13 @@  static void intel_unpin_work_fn(struct work_struct *__work)
 	 * that work->new_crtc_state contains valid memory during unpin
 	 * because intel_atomic_commit may free it before this runs.
 	 */
-	if (!work->can_async_unpin)
+	if (!work->can_async_unpin) {
 		intel_crtc_post_flip_update(work, crtc);
 
+		if (dev_priv->display.optimize_watermarks)
+			dev_priv->display.optimize_watermarks(work->new_crtc_state);
+	}
+
 	if (work->fb_bits & to_intel_plane(crtc->primary)->frontbuffer_bit)
 		intel_fbc_post_update(intel_crtc);
 
@@ -10997,6 +10977,10 @@  static void intel_unpin_work_fn(struct work_struct *__work)
 		spin_unlock_irq(&dev->event_lock);
 	}
 
+	/* New crtc_state freed? */
+	if (work->free_new_crtc_state)
+		intel_crtc_destroy_state(crtc, &work->new_crtc_state->base);
+
 	intel_crtc_destroy_state(crtc, &work->old_crtc_state->base);
 
 	for (i = 0; i < work->num_planes; i++) {
@@ -11086,6 +11070,11 @@  static void intel_mmio_flip_work_func(struct work_struct *w)
 	struct drm_i915_gem_request *req;
 	int i;
 
+	if (!needs_modeset(&crtc_state->base) && crtc_state->update_pipe) {
+		work->put_power_domains =
+			modeset_get_crtc_power_domains(crtc, crtc_state);
+	}
+
 	for (i = 0; i < work->num_planes; i++) {
 		struct intel_plane_state *old_plane_state = work->old_plane_state[i];
 
@@ -13038,11 +13027,33 @@  static int intel_atomic_check(struct drm_device *dev,
 	return 0;
 }
 
+static bool needs_work(struct drm_crtc_state *crtc_state)
+{
+	/* hw state checker needs to run */
+	if (needs_modeset(crtc_state))
+		return true;
+
+	/* unpin old fb's, possibly vblank update */
+	if (crtc_state->planes_changed)
+		return true;
+
+	/* pipe parameters need to be updated, and hw state checker */
+	if (to_intel_crtc_state(crtc_state)->update_pipe)
+		return true;
+
+	/* vblank event requested? */
+	if (crtc_state->event)
+		return true;
+
+	return false;
+}
+
 static int intel_atomic_prepare_commit(struct drm_device *dev,
 				       struct drm_atomic_state *state,
 				       bool async)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_plane_state *plane_state;
 	struct drm_crtc_state *crtc_state;
 	struct drm_plane *plane;
@@ -13055,12 +13066,24 @@  static int intel_atomic_prepare_commit(struct drm_device *dev,
 	}
 
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
 		ret = intel_crtc_wait_for_pending_flips(crtc);
 		if (ret)
 			return ret;
 
-		if (atomic_read(&to_intel_crtc(crtc)->unpin_work_count) >= 2)
+		if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
 			flush_workqueue(dev_priv->wq);
+
+		/* test if we need to update something */
+		if (!needs_work(crtc_state))
+			continue;
+
+		intel_state->work[i] =
+			kzalloc(sizeof(**intel_state->work), GFP_KERNEL);
+
+		if (!intel_state->work[i])
+			return -ENOMEM;
 	}
 
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
@@ -13113,69 +13136,96 @@  u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc)
 	return dev->driver->get_vblank_counter(dev, crtc->pipe);
 }
 
-static void intel_atomic_wait_for_vblanks(struct drm_device *dev,
-					  struct drm_i915_private *dev_priv,
-					  unsigned crtc_mask)
+static void intel_prepare_work(struct drm_crtc *crtc,
+			       struct intel_flip_work *work,
+			       struct drm_atomic_state *state,
+			       struct drm_crtc_state *old_crtc_state)
 {
-	unsigned last_vblank_count[I915_MAX_PIPES];
-	enum pipe pipe;
-	int ret;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_plane_state *old_plane_state;
+	struct drm_plane *plane;
+	int i, j = 0;
 
-	if (!crtc_mask)
-		return;
+	INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
+	INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
+	atomic_inc(&intel_crtc->unpin_work_count);
 
-	for_each_pipe(dev_priv, pipe) {
-		struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+	for_each_plane_in_state(state, plane, old_plane_state, i) {
+		struct intel_plane_state *old_state = to_intel_plane_state(old_plane_state);
+		struct intel_plane_state *new_state = to_intel_plane_state(plane->state);
 
-		if (!((1 << pipe) & crtc_mask))
+		if (old_state->base.crtc != crtc &&
+		    new_state->base.crtc != crtc)
 			continue;
 
-		ret = drm_crtc_vblank_get(crtc);
-		if (WARN_ON(ret != 0)) {
-			crtc_mask &= ~(1 << pipe);
-			continue;
+		if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
+			plane->fb = new_state->base.fb;
+			crtc->x = new_state->base.src_x >> 16;
+			crtc->y = new_state->base.src_y >> 16;
 		}
 
-		last_vblank_count[pipe] = drm_crtc_vblank_count(crtc);
+		old_state->wait_req = new_state->wait_req;
+		new_state->wait_req = NULL;
+
+		old_state->base.fence = new_state->base.fence;
+		new_state->base.fence = NULL;
+
+		/* remove plane state from the atomic state and move it to work */
+		old_plane_state->state = NULL;
+		state->planes[i] = NULL;
+		state->plane_states[i] = NULL;
+
+		work->old_plane_state[j] = old_state;
+		work->new_plane_state[j++] = new_state;
 	}
 
-	for_each_pipe(dev_priv, pipe) {
-		struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
-		long lret;
+	old_crtc_state->state = NULL;
+	state->crtcs[drm_crtc_index(crtc)] = NULL;
+	state->crtc_states[drm_crtc_index(crtc)] = NULL;
 
-		if (!((1 << pipe) & crtc_mask))
-			continue;
+	work->old_crtc_state = to_intel_crtc_state(old_crtc_state);
+	work->new_crtc_state = to_intel_crtc_state(crtc->state);
+	work->num_planes = j;
 
-		lret = wait_event_timeout(dev->vblank[pipe].queue,
-				last_vblank_count[pipe] !=
-					drm_crtc_vblank_count(crtc),
-				msecs_to_jiffies(50));
+	work->event = crtc->state->event;
+	crtc->state->event = NULL;
 
-		WARN_ON(!lret);
+	if (!work->new_crtc_state->update_wm_post &&
+	    !needs_modeset(crtc->state) && !work->new_crtc_state->update_pipe)
+		work->can_async_unpin = true;
 
-		drm_crtc_vblank_put(crtc);
-	}
+	work->fb_bits = work->new_crtc_state->fb_bits;
 }
 
-static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
+static void intel_schedule_unpin(struct drm_crtc *crtc,
+				 struct intel_atomic_state *state,
+				 struct intel_flip_work *work)
 {
-	/* fb updated, need to unpin old fb */
-	if (crtc_state->fb_changed)
-		return true;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	/* wm changes, need vblank before final wm's */
-	if (crtc_state->update_wm_post)
-		return true;
+	to_intel_crtc(crtc)->config = work->new_crtc_state;
 
-	/*
-	 * cxsr is re-enabled after vblank.
-	 * This is already handled by crtc_state->update_wm_post,
-	 * but added for clarity.
-	 */
-	if (crtc_state->disable_cxsr)
-		return true;
+	queue_work(dev_priv->wq, &work->unpin_work);
+}
 
-	return false;
+static void intel_schedule_update(struct drm_crtc *crtc,
+				  struct intel_atomic_state *state,
+				  struct intel_flip_work *work)
+{
+	struct drm_device *dev = crtc->dev;
+
+	if (work->can_async_unpin) {
+		INIT_LIST_HEAD(&work->head);
+		intel_schedule_unpin(crtc, state, work);
+		return;
+	}
+
+	spin_lock_irq(&dev->event_lock);
+	list_add_tail(&work->head, &to_intel_crtc(crtc)->flip_work);
+	spin_unlock_irq(&dev->event_lock);
+
+	intel_schedule_unpin(crtc, state, work);
 }
 
 /**
@@ -13202,11 +13252,7 @@  static int intel_atomic_commit(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc_state *old_crtc_state;
 	struct drm_crtc *crtc;
-	struct intel_crtc_state *intel_cstate;
 	int ret = 0, i;
-	bool hw_check = intel_state->modeset;
-	unsigned long put_domains[I915_MAX_PIPES] = {};
-	unsigned crtc_vblank_mask = 0;
 
 	ret = intel_atomic_prepare_commit(dev, state, async);
 	if (ret) {
@@ -13223,27 +13269,20 @@  static int intel_atomic_commit(struct drm_device *dev,
 		       sizeof(intel_state->min_pixclk));
 		dev_priv->active_crtcs = intel_state->active_crtcs;
 		dev_priv->atomic_cdclk_freq = intel_state->cdclk;
-
-		intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
 	}
 
 	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-		if (needs_modeset(crtc->state) ||
-		    to_intel_crtc_state(crtc->state)->update_pipe) {
-			hw_check = true;
-
-			put_domains[to_intel_crtc(crtc)->pipe] =
-				modeset_get_crtc_power_domains(crtc,
-					to_intel_crtc_state(crtc->state));
-		}
-
 		if (!needs_modeset(crtc->state))
 			continue;
 
 		intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
 
+		intel_state->work[i]->put_power_domains =
+			modeset_get_crtc_power_domains(crtc,
+				to_intel_crtc_state(crtc->state));
+
 		if (old_crtc_state->active) {
 			intel_crtc_disable_planes(crtc, old_crtc_state->plane_mask);
 			dev_priv->display.crtc_disable(crtc);
@@ -13300,46 +13339,37 @@  static int intel_atomic_commit(struct drm_device *dev,
 		if (crtc->state->active &&
 		    (crtc->state->planes_changed || update_pipe))
 			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
-
-		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
-			crtc_vblank_mask |= 1 << i;
 	}
 
 	/* FIXME: add subpixel order */
 
-	if (!state->legacy_cursor_update)
-		intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
-
-	/*
-	 * Now that the vblank has passed, we can go ahead and program the
-	 * optimal watermarks on platforms that need two-step watermark
-	 * programming.
-	 *
-	 * TODO: Move this (and other cleanup) to an async worker eventually.
-	 */
-	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
-		intel_cstate = to_intel_crtc_state(crtc->state);
-
-		if (dev_priv->display.optimize_watermarks)
-			dev_priv->display.optimize_watermarks(intel_cstate);
-	}
-
 	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
-		intel_post_plane_update(to_intel_crtc_state(old_crtc_state));
-
-		if (put_domains[i])
-			modeset_put_power_domains(dev_priv, put_domains[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);
+				if (!list_empty(&intel_crtc->flip_work))
+					work = list_last_entry(&intel_crtc->flip_work,
+							       struct intel_flip_work, head);
+
+				if (work && work->new_crtc_state == to_intel_crtc_state(old_crtc_state)) {
+					work->free_new_crtc_state = true;
+					state->crtc_states[i] = NULL;
+					state->crtcs[i] = NULL;
+				}
+				spin_unlock_irq(&dev->event_lock);
+			}
+			continue;
+		}
 
-		intel_modeset_verify_crtc(crtc, old_crtc_state, crtc->state);
+		intel_state->work[i] = NULL;
+		intel_prepare_work(crtc, work, state, old_crtc_state);
+		intel_schedule_update(crtc, intel_state, work);
 	}
 
-	if (intel_state->modeset)
-		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
-
-	mutex_lock(&dev->struct_mutex);
-	drm_atomic_helper_cleanup_planes(dev, state);
-	mutex_unlock(&dev->struct_mutex);
-
 	drm_atomic_state_free(state);
 
 	/* As one of the primary mmio accessors, KMS has a high likelihood
@@ -13594,6 +13624,8 @@  static void intel_begin_crtc_commit(struct drm_crtc *crtc,
 		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);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d2cfb50332be..9d8c6c165f8d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -293,6 +293,8 @@  struct intel_atomic_state {
 	unsigned int active_crtcs;
 	unsigned int min_pixclk[I915_MAX_PIPES];
 
+	struct intel_flip_work *work[I915_MAX_PIPES];
+
 	struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
 	struct intel_wm_config wm_config;
 
@@ -949,7 +951,7 @@  struct intel_flip_work {
 	unsigned put_power_domains;
 	unsigned num_planes;
 
-	bool can_async_unpin;
+	bool can_async_unpin, free_new_crtc_state;
 	unsigned fb_bits;
 
 	struct intel_crtc_state *old_crtc_state, *new_crtc_state;