All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 1/2] drm/i915: Introduce async plane update to i915
@ 2019-03-07 16:49 Helen Koike
  2019-03-07 16:49 ` [PATCH v8 2/2] drm/i915: update cursors asynchronously through atomic Helen Koike
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Helen Koike @ 2019-03-07 16:49 UTC (permalink / raw)
  To: dri-devel; +Cc: tfiga, mcasas, zhenyu.z.wang, daniel.vetter, tina.zhang, kernel

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Add implementation for async plane update callbacks

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Tina Zhang <tina.zhang@intel.com>
Signed-off-by: Helen Koike <helen.koike@collabora.com>

---
Hi,

This patch just adds the callbacks for atomic async update, the next in
the series will replace the legacy cursor implementation with these
callbacks.

I tested with igt (ksm_cursor_legacy and plane_cursor_legacy).

This patch depends on "[PATCH] drm: don't block fb changes for async plane updates",
otherwise there will be a regression on igt tests:

        cursor-vs-flip-atomic-transitions-varying-size
        cursor-vs-flip-toggle
        cursor-vs-flip-varying-size

with errors of type:

"CRITICAL: completed 97 cursor updated in a period of 30 flips, we
expect to complete approximately 15360 updates, with the threshold set
at 7680"

This happens because what should be async updates are being turned into
syncronous updates.

Thanks
Helen

Changes in v8:
 - v7: https://lkml.org/lkml/2018/6/8/168
 - v7 was splited in two, one that adds the async callbacks and another
 that updates the cursor.
 - rebase with drm-intel
 - allow async update in all types of planes, not only cursor
 - add watermark checks in async update
 - remove bypass of intel_prepare_plane_fb() in case of async update
 - add missing drm_atomic_helper_cleanup_planes(dev, state) call in
 intel_atomic_commit().
 - use swap() function in async update to set the old_fb in the
 new_state object.
 - use helpers intel_update_plane()/intel_disable_plane()

Changes in v7:
- Rebase on top of drm-intel repository. Hopefully now will play
  nicely with autobuilders.

Changes in v6:
- Rework the intel_plane_atomic_async_update due driver changed from
  last time.
- Removed the mutex_lock/unlock as causes a deadlock.

Changes in v5:
- Call drm_atomic_helper_async_check() from the check hook

Changes in v4:
- Set correct vma to new state for cleanup
- Move size checks back to drivers (Ville Syrjälä)

Changes in v3:
- Move fb setting to core and use new state (Eric Anholt)

 drivers/gpu/drm/i915/intel_atomic_plane.c | 71 +++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c      |  9 +++
 2 files changed, 80 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 7961cf0e6951..f516b227dba9 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -318,10 +318,81 @@ void i9xx_update_planes_on_crtc(struct intel_atomic_state *state,
 	}
 }
 
+static int intel_plane_atomic_async_check(struct drm_plane *plane,
+					  struct drm_plane_state *state)
+{
+	struct drm_crtc_state *crtc_state;
+
+	crtc_state = drm_atomic_get_existing_crtc_state(state->state,
+							state->crtc);
+	if (WARN_ON(!crtc_state))
+		return -EINVAL;
+
+	/*
+	 * When crtc is inactive or there is a modeset pending,
+	 * wait for it to complete in the slowpath
+	 */
+	if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
+		return -EINVAL;
+
+	/*
+	 * If any parameters change that may affect watermarks,
+	 * take the slowpath. Only changing fb or position should be
+	 * in the fastpath.
+	 */
+	if (plane->state->crtc != state->crtc ||
+	    plane->state->src_w != state->src_w ||
+	    plane->state->src_h != state->src_h ||
+	    plane->state->crtc_w != state->crtc_w ||
+	    plane->state->crtc_h != state->crtc_h ||
+	    !plane->state->fb != !state->fb)
+		return -EINVAL;
+
+	return 0;
+}
+
+static void intel_plane_atomic_async_update(struct drm_plane *plane,
+					    struct drm_plane_state *new_state)
+{
+	struct intel_atomic_state *intel_new_state =
+		to_intel_atomic_state(new_state->state);
+	struct intel_plane *intel_plane = to_intel_plane(plane);
+	struct drm_crtc *crtc = plane->state->crtc;
+	struct intel_crtc_state *new_crtc_state;
+	struct intel_crtc *intel_crtc;
+	int i;
+
+	for_each_new_intel_crtc_in_state(intel_new_state, intel_crtc,
+					 new_crtc_state, i)
+		WARN_ON(new_crtc_state->wm.need_postvbl_update ||
+			new_crtc_state->update_wm_post);
+
+	i915_gem_track_fb(intel_fb_obj(plane->state->fb),
+			  intel_fb_obj(new_state->fb),
+			  intel_plane->frontbuffer_bit);
+
+	plane->state->src_x = new_state->src_x;
+	plane->state->src_y = new_state->src_y;
+	plane->state->crtc_x = new_state->crtc_x;
+	plane->state->crtc_y = new_state->crtc_y;
+
+	swap(plane->state->fb, new_state->fb);
+
+	if (plane->state->visible)
+		intel_update_plane(intel_plane,
+				   to_intel_crtc_state(crtc->state),
+				   to_intel_plane_state(plane->state));
+	else
+		intel_disable_plane(intel_plane,
+				    to_intel_crtc_state(crtc->state));
+}
+
 const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
 	.prepare_fb = intel_prepare_plane_fb,
 	.cleanup_fb = intel_cleanup_plane_fb,
 	.atomic_check = intel_plane_atomic_check,
+	.atomic_async_check = intel_plane_atomic_async_check,
+	.atomic_async_update = intel_plane_atomic_async_update,
 };
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 415d8968f2c5..244e1c94277d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13464,6 +13464,15 @@ static int intel_atomic_commit(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	int ret = 0;
 
+	if (state->async_update) {
+		ret = drm_atomic_helper_prepare_planes(dev, state);
+		if (ret)
+			return ret;
+		drm_atomic_helper_async_commit(dev, state);
+		drm_atomic_helper_cleanup_planes(dev, state);
+		return 0;
+	}
+
 	drm_atomic_state_get(state);
 	i915_sw_fence_init(&intel_state->commit_ready,
 			   intel_atomic_commit_ready);
-- 
2.20.1

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

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

* [PATCH v8 2/2] drm/i915: update cursors asynchronously through atomic
  2019-03-07 16:49 [PATCH v8 1/2] drm/i915: Introduce async plane update to i915 Helen Koike
@ 2019-03-07 16:49 ` Helen Koike
  2019-03-08  8:08 ` [PATCH v8 1/2] drm/i915: Introduce async plane update to i915 Zhang, Tina
  2019-03-08  9:55 ` Jani Nikula
  2 siblings, 0 replies; 6+ messages in thread
From: Helen Koike @ 2019-03-07 16:49 UTC (permalink / raw)
  To: dri-devel; +Cc: tfiga, mcasas, zhenyu.z.wang, daniel.vetter, tina.zhang, kernel

From: Gustavo Padovan <gustavo.padovan@collabora.com>

Replace the legacy cursor implementation by the async callbacks

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Signed-off-by: Helen Koike <helen.koike@collabora.com>

---

Changes in v8:
 - v7: https://lkml.org/lkml/2018/6/8/168
 - v7 was splited in two, one that adds the async callbacks and another
 that updates the cursor.
 - Update comment in intel_pm.c that was referencing
 intel_plane_atomic_async_update()

 drivers/gpu/drm/i915/intel_display.c | 165 +--------------------------
 drivers/gpu/drm/i915/intel_pm.c      |   2 +-
 2 files changed, 6 insertions(+), 161 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 244e1c94277d..bd84d5b9c441 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13005,6 +13005,10 @@ static int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		return ret;
 
+	if (state->legacy_cursor_update)
+		state->async_update = !drm_atomic_helper_async_check(dev,
+								     state);
+
 	intel_fbc_choose_crtc(dev_priv, intel_state);
 	return calc_watermark_data(intel_state);
 }
@@ -13477,34 +13481,6 @@ static int intel_atomic_commit(struct drm_device *dev,
 	i915_sw_fence_init(&intel_state->commit_ready,
 			   intel_atomic_commit_ready);
 
-	/*
-	 * The intel_legacy_cursor_update() fast path takes care
-	 * of avoiding the vblank waits for simple cursor
-	 * movement and flips. For cursor on/off and size changes,
-	 * we want to perform the vblank waits so that watermark
-	 * updates happen during the correct frames. Gen9+ have
-	 * double buffered watermarks and so shouldn't need this.
-	 *
-	 * Unset state->legacy_cursor_update before the call to
-	 * drm_atomic_helper_setup_commit() because otherwise
-	 * drm_atomic_helper_wait_for_flip_done() is a noop and
-	 * we get FIFO underruns because we didn't wait
-	 * for vblank.
-	 *
-	 * FIXME doing watermarks and fb cleanup from a vblank worker
-	 * (assuming we had any) would solve these problems.
-	 */
-	if (INTEL_GEN(dev_priv) < 9 && state->legacy_cursor_update) {
-		struct intel_crtc_state *new_crtc_state;
-		struct intel_crtc *crtc;
-		int i;
-
-		for_each_new_intel_crtc_in_state(intel_state, crtc, new_crtc_state, i)
-			if (new_crtc_state->wm.need_postvbl_update ||
-			    new_crtc_state->update_wm_post)
-				state->legacy_cursor_update = false;
-	}
-
 	ret = intel_atomic_prepare_commit(dev, state);
 	if (ret) {
 		DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
@@ -14012,139 +13988,8 @@ static const struct drm_plane_funcs i8xx_plane_funcs = {
 	.format_mod_supported = i8xx_plane_format_mod_supported,
 };
 
-static int
-intel_legacy_cursor_update(struct drm_plane *plane,
-			   struct drm_crtc *crtc,
-			   struct drm_framebuffer *fb,
-			   int crtc_x, int crtc_y,
-			   unsigned int crtc_w, unsigned int crtc_h,
-			   u32 src_x, u32 src_y,
-			   u32 src_w, u32 src_h,
-			   struct drm_modeset_acquire_ctx *ctx)
-{
-	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
-	int ret;
-	struct drm_plane_state *old_plane_state, *new_plane_state;
-	struct intel_plane *intel_plane = to_intel_plane(plane);
-	struct drm_framebuffer *old_fb;
-	struct intel_crtc_state *crtc_state =
-		to_intel_crtc_state(crtc->state);
-	struct intel_crtc_state *new_crtc_state;
-
-	/*
-	 * When crtc is inactive or there is a modeset pending,
-	 * wait for it to complete in the slowpath
-	 */
-	if (!crtc_state->base.active || needs_modeset(&crtc_state->base) ||
-	    crtc_state->update_pipe)
-		goto slow;
-
-	old_plane_state = plane->state;
-	/*
-	 * Don't do an async update if there is an outstanding commit modifying
-	 * the plane.  This prevents our async update's changes from getting
-	 * overridden by a previous synchronous update's state.
-	 */
-	if (old_plane_state->commit &&
-	    !try_wait_for_completion(&old_plane_state->commit->hw_done))
-		goto slow;
-
-	/*
-	 * If any parameters change that may affect watermarks,
-	 * take the slowpath. Only changing fb or position should be
-	 * in the fastpath.
-	 */
-	if (old_plane_state->crtc != crtc ||
-	    old_plane_state->src_w != src_w ||
-	    old_plane_state->src_h != src_h ||
-	    old_plane_state->crtc_w != crtc_w ||
-	    old_plane_state->crtc_h != crtc_h ||
-	    !old_plane_state->fb != !fb)
-		goto slow;
-
-	new_plane_state = intel_plane_duplicate_state(plane);
-	if (!new_plane_state)
-		return -ENOMEM;
-
-	new_crtc_state = to_intel_crtc_state(intel_crtc_duplicate_state(crtc));
-	if (!new_crtc_state) {
-		ret = -ENOMEM;
-		goto out_free;
-	}
-
-	drm_atomic_set_fb_for_plane(new_plane_state, fb);
-
-	new_plane_state->src_x = src_x;
-	new_plane_state->src_y = src_y;
-	new_plane_state->src_w = src_w;
-	new_plane_state->src_h = src_h;
-	new_plane_state->crtc_x = crtc_x;
-	new_plane_state->crtc_y = crtc_y;
-	new_plane_state->crtc_w = crtc_w;
-	new_plane_state->crtc_h = crtc_h;
-
-	ret = intel_plane_atomic_check_with_state(crtc_state, new_crtc_state,
-						  to_intel_plane_state(old_plane_state),
-						  to_intel_plane_state(new_plane_state));
-	if (ret)
-		goto out_free;
-
-	ret = mutex_lock_interruptible(&dev_priv->drm.struct_mutex);
-	if (ret)
-		goto out_free;
-
-	ret = intel_plane_pin_fb(to_intel_plane_state(new_plane_state));
-	if (ret)
-		goto out_unlock;
-
-	intel_fb_obj_flush(intel_fb_obj(fb), ORIGIN_FLIP);
-
-	old_fb = old_plane_state->fb;
-	i915_gem_track_fb(intel_fb_obj(old_fb), intel_fb_obj(fb),
-			  intel_plane->frontbuffer_bit);
-
-	/* Swap plane state */
-	plane->state = new_plane_state;
-
-	/*
-	 * We cannot swap crtc_state as it may be in use by an atomic commit or
-	 * page flip that's running simultaneously. If we swap crtc_state and
-	 * destroy the old state, we will cause a use-after-free there.
-	 *
-	 * Only update active_planes, which is needed for our internal
-	 * bookkeeping. Either value will do the right thing when updating
-	 * planes atomically. If the cursor was part of the atomic update then
-	 * we would have taken the slowpath.
-	 */
-	crtc_state->active_planes = new_crtc_state->active_planes;
-
-	if (plane->state->visible)
-		intel_update_plane(intel_plane, crtc_state,
-				   to_intel_plane_state(plane->state));
-	else
-		intel_disable_plane(intel_plane, crtc_state);
-
-	intel_plane_unpin_fb(to_intel_plane_state(old_plane_state));
-
-out_unlock:
-	mutex_unlock(&dev_priv->drm.struct_mutex);
-out_free:
-	if (new_crtc_state)
-		intel_crtc_destroy_state(crtc, &new_crtc_state->base);
-	if (ret)
-		intel_plane_destroy_state(plane, new_plane_state);
-	else
-		intel_plane_destroy_state(plane, old_plane_state);
-	return ret;
-
-slow:
-	return drm_atomic_helper_update_plane(plane, crtc, fb,
-					      crtc_x, crtc_y, crtc_w, crtc_h,
-					      src_x, src_y, src_w, src_h, ctx);
-}
-
 static const struct drm_plane_funcs intel_cursor_plane_funcs = {
-	.update_plane = intel_legacy_cursor_update,
+	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
 	.destroy = intel_plane_destroy,
 	.atomic_get_property = intel_plane_atomic_get_property,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4c0e43caa5cd..1bfbb3ec39d4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -818,7 +818,7 @@ static bool intel_wm_plane_visible(const struct intel_crtc_state *crtc_state,
 	 * can happen faster than the vrefresh rate, and the current
 	 * watermark code doesn't handle that correctly. Cursor updates
 	 * which set/clear the fb or change the cursor size are going
-	 * to get throttled by intel_legacy_cursor_update() to work
+	 * to get throttled by intel_plane_atomic_async_update() to work
 	 * around this problem with the watermark code.
 	 */
 	if (plane->id == PLANE_CURSOR)
-- 
2.20.1

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

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

* RE: [PATCH v8 1/2] drm/i915: Introduce async plane update to i915
  2019-03-07 16:49 [PATCH v8 1/2] drm/i915: Introduce async plane update to i915 Helen Koike
  2019-03-07 16:49 ` [PATCH v8 2/2] drm/i915: update cursors asynchronously through atomic Helen Koike
@ 2019-03-08  8:08 ` Zhang, Tina
  2019-03-08  9:55 ` Jani Nikula
  2 siblings, 0 replies; 6+ messages in thread
From: Zhang, Tina @ 2019-03-08  8:08 UTC (permalink / raw)
  To: Helen Koike, dri-devel
  Cc: mcasas, Wang, Zhenyu Z, daniel.vetter, tfiga, kernel

Tested-by: Tina Zhang <tina.zhang@intel.com>

> -----Original Message-----
> From: Helen Koike [mailto:helen.koike@collabora.com]
> Sent: Friday, March 8, 2019 12:50 AM
> To: dri-devel@lists.freedesktop.org
> Cc: tfiga@chromium.org; mcasas@google.com; Wang, Zhenyu Z
> <zhenyu.z.wang@intel.com>; daniel.vetter@ffwll.ch; Zhang, Tina
> <tina.zhang@intel.com>; kernel@collabora.com; ville.syrjala@linux.intel.com
> Subject: [PATCH v8 1/2] drm/i915: Introduce async plane update to i915
> 
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
> 
> Add implementation for async plane update callbacks
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
> 
> ---
> Hi,
> 
> This patch just adds the callbacks for atomic async update, the next in the series
> will replace the legacy cursor implementation with these callbacks.
> 
> I tested with igt (ksm_cursor_legacy and plane_cursor_legacy).
> 
> This patch depends on "[PATCH] drm: don't block fb changes for async plane
> updates", otherwise there will be a regression on igt tests:
> 
>         cursor-vs-flip-atomic-transitions-varying-size
>         cursor-vs-flip-toggle
>         cursor-vs-flip-varying-size
> 
> with errors of type:
> 
> "CRITICAL: completed 97 cursor updated in a period of 30 flips, we expect to
> complete approximately 15360 updates, with the threshold set at 7680"
> 
> This happens because what should be async updates are being turned into
> syncronous updates.
> 
> Thanks
> Helen
> 
> Changes in v8:
>  - v7: https://lkml.org/lkml/2018/6/8/168
>  - v7 was splited in two, one that adds the async callbacks and another  that
> updates the cursor.
>  - rebase with drm-intel
>  - allow async update in all types of planes, not only cursor
>  - add watermark checks in async update
>  - remove bypass of intel_prepare_plane_fb() in case of async update
>  - add missing drm_atomic_helper_cleanup_planes(dev, state) call in
> intel_atomic_commit().
>  - use swap() function in async update to set the old_fb in the  new_state object.
>  - use helpers intel_update_plane()/intel_disable_plane()
> 
> Changes in v7:
> - Rebase on top of drm-intel repository. Hopefully now will play
>   nicely with autobuilders.
> 
> Changes in v6:
> - Rework the intel_plane_atomic_async_update due driver changed from
>   last time.
> - Removed the mutex_lock/unlock as causes a deadlock.
> 
> Changes in v5:
> - Call drm_atomic_helper_async_check() from the check hook
> 
> Changes in v4:
> - Set correct vma to new state for cleanup
> - Move size checks back to drivers (Ville Syrjälä)
> 
> Changes in v3:
> - Move fb setting to core and use new state (Eric Anholt)
> 
>  drivers/gpu/drm/i915/intel_atomic_plane.c | 71 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c      |  9 +++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 7961cf0e6951..f516b227dba9 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -318,10 +318,81 @@ void i9xx_update_planes_on_crtc(struct
> intel_atomic_state *state,
>  	}
>  }
> 
> +static int intel_plane_atomic_async_check(struct drm_plane *plane,
> +					  struct drm_plane_state *state)
> +{
> +	struct drm_crtc_state *crtc_state;
> +
> +	crtc_state = drm_atomic_get_existing_crtc_state(state->state,
> +							state->crtc);
> +	if (WARN_ON(!crtc_state))
> +		return -EINVAL;
> +
> +	/*
> +	 * When crtc is inactive or there is a modeset pending,
> +	 * wait for it to complete in the slowpath
> +	 */
> +	if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
> +		return -EINVAL;
> +
> +	/*
> +	 * If any parameters change that may affect watermarks,
> +	 * take the slowpath. Only changing fb or position should be
> +	 * in the fastpath.
> +	 */
> +	if (plane->state->crtc != state->crtc ||
> +	    plane->state->src_w != state->src_w ||
> +	    plane->state->src_h != state->src_h ||
> +	    plane->state->crtc_w != state->crtc_w ||
> +	    plane->state->crtc_h != state->crtc_h ||
> +	    !plane->state->fb != !state->fb)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static void intel_plane_atomic_async_update(struct drm_plane *plane,
> +					    struct drm_plane_state *new_state)
> {
> +	struct intel_atomic_state *intel_new_state =
> +		to_intel_atomic_state(new_state->state);
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct drm_crtc *crtc = plane->state->crtc;
> +	struct intel_crtc_state *new_crtc_state;
> +	struct intel_crtc *intel_crtc;
> +	int i;
> +
> +	for_each_new_intel_crtc_in_state(intel_new_state, intel_crtc,
> +					 new_crtc_state, i)
> +		WARN_ON(new_crtc_state->wm.need_postvbl_update ||
> +			new_crtc_state->update_wm_post);
> +
> +	i915_gem_track_fb(intel_fb_obj(plane->state->fb),
> +			  intel_fb_obj(new_state->fb),
> +			  intel_plane->frontbuffer_bit);
> +
> +	plane->state->src_x = new_state->src_x;
> +	plane->state->src_y = new_state->src_y;
> +	plane->state->crtc_x = new_state->crtc_x;
> +	plane->state->crtc_y = new_state->crtc_y;
> +
> +	swap(plane->state->fb, new_state->fb);
> +
> +	if (plane->state->visible)
> +		intel_update_plane(intel_plane,
> +				   to_intel_crtc_state(crtc->state),
> +				   to_intel_plane_state(plane->state));
> +	else
> +		intel_disable_plane(intel_plane,
> +				    to_intel_crtc_state(crtc->state));
> +}
> +
>  const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
>  	.prepare_fb = intel_prepare_plane_fb,
>  	.cleanup_fb = intel_cleanup_plane_fb,
>  	.atomic_check = intel_plane_atomic_check,
> +	.atomic_async_check = intel_plane_atomic_async_check,
> +	.atomic_async_update = intel_plane_atomic_async_update,
>  };
> 
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 415d8968f2c5..244e1c94277d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13464,6 +13464,15 @@ static int intel_atomic_commit(struct
> drm_device *dev,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	int ret = 0;
> 
> +	if (state->async_update) {
> +		ret = drm_atomic_helper_prepare_planes(dev, state);
> +		if (ret)
> +			return ret;
> +		drm_atomic_helper_async_commit(dev, state);
> +		drm_atomic_helper_cleanup_planes(dev, state);
> +		return 0;
> +	}
> +
>  	drm_atomic_state_get(state);
>  	i915_sw_fence_init(&intel_state->commit_ready,
>  			   intel_atomic_commit_ready);
> --
> 2.20.1

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

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

* Re: [PATCH v8 1/2] drm/i915: Introduce async plane update to i915
  2019-03-07 16:49 [PATCH v8 1/2] drm/i915: Introduce async plane update to i915 Helen Koike
  2019-03-07 16:49 ` [PATCH v8 2/2] drm/i915: update cursors asynchronously through atomic Helen Koike
  2019-03-08  8:08 ` [PATCH v8 1/2] drm/i915: Introduce async plane update to i915 Zhang, Tina
@ 2019-03-08  9:55 ` Jani Nikula
  2019-03-08 14:51   ` Helen Koike
  2 siblings, 1 reply; 6+ messages in thread
From: Jani Nikula @ 2019-03-08  9:55 UTC (permalink / raw)
  To: Helen Koike, dri-devel
  Cc: tfiga, mcasas, zhenyu.z.wang, daniel.vetter, tina.zhang, kernel


Please send drm/i915 patches also to intel-gfx@lists.freedesktop.org to
trigger CI on the patches among other things.

BR,
Jani.


On Thu, 07 Mar 2019, Helen Koike <helen.koike@collabora.com> wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.com>
>
> Add implementation for async plane update callbacks
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>
> ---
> Hi,
>
> This patch just adds the callbacks for atomic async update, the next in
> the series will replace the legacy cursor implementation with these
> callbacks.
>
> I tested with igt (ksm_cursor_legacy and plane_cursor_legacy).
>
> This patch depends on "[PATCH] drm: don't block fb changes for async plane updates",
> otherwise there will be a regression on igt tests:
>
>         cursor-vs-flip-atomic-transitions-varying-size
>         cursor-vs-flip-toggle
>         cursor-vs-flip-varying-size
>
> with errors of type:
>
> "CRITICAL: completed 97 cursor updated in a period of 30 flips, we
> expect to complete approximately 15360 updates, with the threshold set
> at 7680"
>
> This happens because what should be async updates are being turned into
> syncronous updates.
>
> Thanks
> Helen
>
> Changes in v8:
>  - v7: https://lkml.org/lkml/2018/6/8/168
>  - v7 was splited in two, one that adds the async callbacks and another
>  that updates the cursor.
>  - rebase with drm-intel
>  - allow async update in all types of planes, not only cursor
>  - add watermark checks in async update
>  - remove bypass of intel_prepare_plane_fb() in case of async update
>  - add missing drm_atomic_helper_cleanup_planes(dev, state) call in
>  intel_atomic_commit().
>  - use swap() function in async update to set the old_fb in the
>  new_state object.
>  - use helpers intel_update_plane()/intel_disable_plane()
>
> Changes in v7:
> - Rebase on top of drm-intel repository. Hopefully now will play
>   nicely with autobuilders.
>
> Changes in v6:
> - Rework the intel_plane_atomic_async_update due driver changed from
>   last time.
> - Removed the mutex_lock/unlock as causes a deadlock.
>
> Changes in v5:
> - Call drm_atomic_helper_async_check() from the check hook
>
> Changes in v4:
> - Set correct vma to new state for cleanup
> - Move size checks back to drivers (Ville Syrjälä)
>
> Changes in v3:
> - Move fb setting to core and use new state (Eric Anholt)
>
>  drivers/gpu/drm/i915/intel_atomic_plane.c | 71 +++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c      |  9 +++
>  2 files changed, 80 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 7961cf0e6951..f516b227dba9 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -318,10 +318,81 @@ void i9xx_update_planes_on_crtc(struct intel_atomic_state *state,
>  	}
>  }
>  
> +static int intel_plane_atomic_async_check(struct drm_plane *plane,
> +					  struct drm_plane_state *state)
> +{
> +	struct drm_crtc_state *crtc_state;
> +
> +	crtc_state = drm_atomic_get_existing_crtc_state(state->state,
> +							state->crtc);
> +	if (WARN_ON(!crtc_state))
> +		return -EINVAL;
> +
> +	/*
> +	 * When crtc is inactive or there is a modeset pending,
> +	 * wait for it to complete in the slowpath
> +	 */
> +	if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
> +		return -EINVAL;
> +
> +	/*
> +	 * If any parameters change that may affect watermarks,
> +	 * take the slowpath. Only changing fb or position should be
> +	 * in the fastpath.
> +	 */
> +	if (plane->state->crtc != state->crtc ||
> +	    plane->state->src_w != state->src_w ||
> +	    plane->state->src_h != state->src_h ||
> +	    plane->state->crtc_w != state->crtc_w ||
> +	    plane->state->crtc_h != state->crtc_h ||
> +	    !plane->state->fb != !state->fb)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static void intel_plane_atomic_async_update(struct drm_plane *plane,
> +					    struct drm_plane_state *new_state)
> +{
> +	struct intel_atomic_state *intel_new_state =
> +		to_intel_atomic_state(new_state->state);
> +	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct drm_crtc *crtc = plane->state->crtc;
> +	struct intel_crtc_state *new_crtc_state;
> +	struct intel_crtc *intel_crtc;
> +	int i;
> +
> +	for_each_new_intel_crtc_in_state(intel_new_state, intel_crtc,
> +					 new_crtc_state, i)
> +		WARN_ON(new_crtc_state->wm.need_postvbl_update ||
> +			new_crtc_state->update_wm_post);
> +
> +	i915_gem_track_fb(intel_fb_obj(plane->state->fb),
> +			  intel_fb_obj(new_state->fb),
> +			  intel_plane->frontbuffer_bit);
> +
> +	plane->state->src_x = new_state->src_x;
> +	plane->state->src_y = new_state->src_y;
> +	plane->state->crtc_x = new_state->crtc_x;
> +	plane->state->crtc_y = new_state->crtc_y;
> +
> +	swap(plane->state->fb, new_state->fb);
> +
> +	if (plane->state->visible)
> +		intel_update_plane(intel_plane,
> +				   to_intel_crtc_state(crtc->state),
> +				   to_intel_plane_state(plane->state));
> +	else
> +		intel_disable_plane(intel_plane,
> +				    to_intel_crtc_state(crtc->state));
> +}
> +
>  const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
>  	.prepare_fb = intel_prepare_plane_fb,
>  	.cleanup_fb = intel_cleanup_plane_fb,
>  	.atomic_check = intel_plane_atomic_check,
> +	.atomic_async_check = intel_plane_atomic_async_check,
> +	.atomic_async_update = intel_plane_atomic_async_update,
>  };
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 415d8968f2c5..244e1c94277d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13464,6 +13464,15 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	int ret = 0;
>  
> +	if (state->async_update) {
> +		ret = drm_atomic_helper_prepare_planes(dev, state);
> +		if (ret)
> +			return ret;
> +		drm_atomic_helper_async_commit(dev, state);
> +		drm_atomic_helper_cleanup_planes(dev, state);
> +		return 0;
> +	}
> +
>  	drm_atomic_state_get(state);
>  	i915_sw_fence_init(&intel_state->commit_ready,
>  			   intel_atomic_commit_ready);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v8 1/2] drm/i915: Introduce async plane update to i915
  2019-03-08  9:55 ` Jani Nikula
@ 2019-03-08 14:51   ` Helen Koike
  2019-03-08 15:34     ` Jani Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Helen Koike @ 2019-03-08 14:51 UTC (permalink / raw)
  To: Jani Nikula, dri-devel
  Cc: tfiga, mcasas, zhenyu.z.wang, daniel.vetter, tina.zhang, kernel

Hi Jani,

On 3/8/19 6:55 AM, Jani Nikula wrote:
> 
> Please send drm/i915 patches also to intel-gfx@lists.freedesktop.org to
> trigger CI on the patches among other things.

Sure! Is there a way to tell the CI to pick up the dependency [1] as well?

[1] "[PATCH] drm: don't block fb changes for async plane updates"

Regards,
Helen

> 
> BR,
> Jani.
> 
> 
> On Thu, 07 Mar 2019, Helen Koike <helen.koike@collabora.com> wrote:
>> From: Gustavo Padovan <gustavo.padovan@collabora.com>
>>
>> Add implementation for async plane update callbacks
>>
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
>> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>>
>> ---
>> Hi,
>>
>> This patch just adds the callbacks for atomic async update, the next in
>> the series will replace the legacy cursor implementation with these
>> callbacks.
>>
>> I tested with igt (ksm_cursor_legacy and plane_cursor_legacy).
>>
>> This patch depends on "[PATCH] drm: don't block fb changes for async plane updates",
>> otherwise there will be a regression on igt tests:
>>
>>         cursor-vs-flip-atomic-transitions-varying-size
>>         cursor-vs-flip-toggle
>>         cursor-vs-flip-varying-size
>>
>> with errors of type:
>>
>> "CRITICAL: completed 97 cursor updated in a period of 30 flips, we
>> expect to complete approximately 15360 updates, with the threshold set
>> at 7680"
>>
>> This happens because what should be async updates are being turned into
>> syncronous updates.
>>
>> Thanks
>> Helen
>>
>> Changes in v8:
>>  - v7: https://lkml.org/lkml/2018/6/8/168
>>  - v7 was splited in two, one that adds the async callbacks and another
>>  that updates the cursor.
>>  - rebase with drm-intel
>>  - allow async update in all types of planes, not only cursor
>>  - add watermark checks in async update
>>  - remove bypass of intel_prepare_plane_fb() in case of async update
>>  - add missing drm_atomic_helper_cleanup_planes(dev, state) call in
>>  intel_atomic_commit().
>>  - use swap() function in async update to set the old_fb in the
>>  new_state object.
>>  - use helpers intel_update_plane()/intel_disable_plane()
>>
>> Changes in v7:
>> - Rebase on top of drm-intel repository. Hopefully now will play
>>   nicely with autobuilders.
>>
>> Changes in v6:
>> - Rework the intel_plane_atomic_async_update due driver changed from
>>   last time.
>> - Removed the mutex_lock/unlock as causes a deadlock.
>>
>> Changes in v5:
>> - Call drm_atomic_helper_async_check() from the check hook
>>
>> Changes in v4:
>> - Set correct vma to new state for cleanup
>> - Move size checks back to drivers (Ville Syrjälä)
>>
>> Changes in v3:
>> - Move fb setting to core and use new state (Eric Anholt)
>>
>>  drivers/gpu/drm/i915/intel_atomic_plane.c | 71 +++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_display.c      |  9 +++
>>  2 files changed, 80 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
>> index 7961cf0e6951..f516b227dba9 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
>> @@ -318,10 +318,81 @@ void i9xx_update_planes_on_crtc(struct intel_atomic_state *state,
>>  	}
>>  }
>>  
>> +static int intel_plane_atomic_async_check(struct drm_plane *plane,
>> +					  struct drm_plane_state *state)
>> +{
>> +	struct drm_crtc_state *crtc_state;
>> +
>> +	crtc_state = drm_atomic_get_existing_crtc_state(state->state,
>> +							state->crtc);
>> +	if (WARN_ON(!crtc_state))
>> +		return -EINVAL;
>> +
>> +	/*
>> +	 * When crtc is inactive or there is a modeset pending,
>> +	 * wait for it to complete in the slowpath
>> +	 */
>> +	if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
>> +		return -EINVAL;
>> +
>> +	/*
>> +	 * If any parameters change that may affect watermarks,
>> +	 * take the slowpath. Only changing fb or position should be
>> +	 * in the fastpath.
>> +	 */
>> +	if (plane->state->crtc != state->crtc ||
>> +	    plane->state->src_w != state->src_w ||
>> +	    plane->state->src_h != state->src_h ||
>> +	    plane->state->crtc_w != state->crtc_w ||
>> +	    plane->state->crtc_h != state->crtc_h ||
>> +	    !plane->state->fb != !state->fb)
>> +		return -EINVAL;
>> +
>> +	return 0;
>> +}
>> +
>> +static void intel_plane_atomic_async_update(struct drm_plane *plane,
>> +					    struct drm_plane_state *new_state)
>> +{
>> +	struct intel_atomic_state *intel_new_state =
>> +		to_intel_atomic_state(new_state->state);
>> +	struct intel_plane *intel_plane = to_intel_plane(plane);
>> +	struct drm_crtc *crtc = plane->state->crtc;
>> +	struct intel_crtc_state *new_crtc_state;
>> +	struct intel_crtc *intel_crtc;
>> +	int i;
>> +
>> +	for_each_new_intel_crtc_in_state(intel_new_state, intel_crtc,
>> +					 new_crtc_state, i)
>> +		WARN_ON(new_crtc_state->wm.need_postvbl_update ||
>> +			new_crtc_state->update_wm_post);
>> +
>> +	i915_gem_track_fb(intel_fb_obj(plane->state->fb),
>> +			  intel_fb_obj(new_state->fb),
>> +			  intel_plane->frontbuffer_bit);
>> +
>> +	plane->state->src_x = new_state->src_x;
>> +	plane->state->src_y = new_state->src_y;
>> +	plane->state->crtc_x = new_state->crtc_x;
>> +	plane->state->crtc_y = new_state->crtc_y;
>> +
>> +	swap(plane->state->fb, new_state->fb);
>> +
>> +	if (plane->state->visible)
>> +		intel_update_plane(intel_plane,
>> +				   to_intel_crtc_state(crtc->state),
>> +				   to_intel_plane_state(plane->state));
>> +	else
>> +		intel_disable_plane(intel_plane,
>> +				    to_intel_crtc_state(crtc->state));
>> +}
>> +
>>  const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
>>  	.prepare_fb = intel_prepare_plane_fb,
>>  	.cleanup_fb = intel_cleanup_plane_fb,
>>  	.atomic_check = intel_plane_atomic_check,
>> +	.atomic_async_check = intel_plane_atomic_async_check,
>> +	.atomic_async_update = intel_plane_atomic_async_update,
>>  };
>>  
>>  /**
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 415d8968f2c5..244e1c94277d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13464,6 +13464,15 @@ static int intel_atomic_commit(struct drm_device *dev,
>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>>  	int ret = 0;
>>  
>> +	if (state->async_update) {
>> +		ret = drm_atomic_helper_prepare_planes(dev, state);
>> +		if (ret)
>> +			return ret;
>> +		drm_atomic_helper_async_commit(dev, state);
>> +		drm_atomic_helper_cleanup_planes(dev, state);
>> +		return 0;
>> +	}
>> +
>>  	drm_atomic_state_get(state);
>>  	i915_sw_fence_init(&intel_state->commit_ready,
>>  			   intel_atomic_commit_ready);
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v8 1/2] drm/i915: Introduce async plane update to i915
  2019-03-08 14:51   ` Helen Koike
@ 2019-03-08 15:34     ` Jani Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2019-03-08 15:34 UTC (permalink / raw)
  To: Helen Koike, dri-devel
  Cc: tfiga, mcasas, zhenyu.z.wang, daniel.vetter, tina.zhang, kernel

On Fri, 08 Mar 2019, Helen Koike <helen.koike@collabora.com> wrote:
> Hi Jani,
>
> On 3/8/19 6:55 AM, Jani Nikula wrote:
>> 
>> Please send drm/i915 patches also to intel-gfx@lists.freedesktop.org to
>> trigger CI on the patches among other things.
>
> Sure! Is there a way to tell the CI to pick up the dependency [1] as well?

No. You should post these as part of a self-contained series. Looks like
you already have other driver changes in the series.

BR,
Jani.


>
> [1] "[PATCH] drm: don't block fb changes for async plane updates"
>
> Regards,
> Helen
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> On Thu, 07 Mar 2019, Helen Koike <helen.koike@collabora.com> wrote:
>>> From: Gustavo Padovan <gustavo.padovan@collabora.com>
>>>
>>> Add implementation for async plane update callbacks
>>>
>>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
>>> Signed-off-by: Helen Koike <helen.koike@collabora.com>
>>>
>>> ---
>>> Hi,
>>>
>>> This patch just adds the callbacks for atomic async update, the next in
>>> the series will replace the legacy cursor implementation with these
>>> callbacks.
>>>
>>> I tested with igt (ksm_cursor_legacy and plane_cursor_legacy).
>>>
>>> This patch depends on "[PATCH] drm: don't block fb changes for async plane updates",
>>> otherwise there will be a regression on igt tests:
>>>
>>>         cursor-vs-flip-atomic-transitions-varying-size
>>>         cursor-vs-flip-toggle
>>>         cursor-vs-flip-varying-size
>>>
>>> with errors of type:
>>>
>>> "CRITICAL: completed 97 cursor updated in a period of 30 flips, we
>>> expect to complete approximately 15360 updates, with the threshold set
>>> at 7680"
>>>
>>> This happens because what should be async updates are being turned into
>>> syncronous updates.
>>>
>>> Thanks
>>> Helen
>>>
>>> Changes in v8:
>>>  - v7: https://lkml.org/lkml/2018/6/8/168
>>>  - v7 was splited in two, one that adds the async callbacks and another
>>>  that updates the cursor.
>>>  - rebase with drm-intel
>>>  - allow async update in all types of planes, not only cursor
>>>  - add watermark checks in async update
>>>  - remove bypass of intel_prepare_plane_fb() in case of async update
>>>  - add missing drm_atomic_helper_cleanup_planes(dev, state) call in
>>>  intel_atomic_commit().
>>>  - use swap() function in async update to set the old_fb in the
>>>  new_state object.
>>>  - use helpers intel_update_plane()/intel_disable_plane()
>>>
>>> Changes in v7:
>>> - Rebase on top of drm-intel repository. Hopefully now will play
>>>   nicely with autobuilders.
>>>
>>> Changes in v6:
>>> - Rework the intel_plane_atomic_async_update due driver changed from
>>>   last time.
>>> - Removed the mutex_lock/unlock as causes a deadlock.
>>>
>>> Changes in v5:
>>> - Call drm_atomic_helper_async_check() from the check hook
>>>
>>> Changes in v4:
>>> - Set correct vma to new state for cleanup
>>> - Move size checks back to drivers (Ville Syrjälä)
>>>
>>> Changes in v3:
>>> - Move fb setting to core and use new state (Eric Anholt)
>>>
>>>  drivers/gpu/drm/i915/intel_atomic_plane.c | 71 +++++++++++++++++++++++
>>>  drivers/gpu/drm/i915/intel_display.c      |  9 +++
>>>  2 files changed, 80 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
>>> index 7961cf0e6951..f516b227dba9 100644
>>> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
>>> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
>>> @@ -318,10 +318,81 @@ void i9xx_update_planes_on_crtc(struct intel_atomic_state *state,
>>>  	}
>>>  }
>>>  
>>> +static int intel_plane_atomic_async_check(struct drm_plane *plane,
>>> +					  struct drm_plane_state *state)
>>> +{
>>> +	struct drm_crtc_state *crtc_state;
>>> +
>>> +	crtc_state = drm_atomic_get_existing_crtc_state(state->state,
>>> +							state->crtc);
>>> +	if (WARN_ON(!crtc_state))
>>> +		return -EINVAL;
>>> +
>>> +	/*
>>> +	 * When crtc is inactive or there is a modeset pending,
>>> +	 * wait for it to complete in the slowpath
>>> +	 */
>>> +	if (!crtc_state->active || to_intel_crtc_state(crtc_state)->update_pipe)
>>> +		return -EINVAL;
>>> +
>>> +	/*
>>> +	 * If any parameters change that may affect watermarks,
>>> +	 * take the slowpath. Only changing fb or position should be
>>> +	 * in the fastpath.
>>> +	 */
>>> +	if (plane->state->crtc != state->crtc ||
>>> +	    plane->state->src_w != state->src_w ||
>>> +	    plane->state->src_h != state->src_h ||
>>> +	    plane->state->crtc_w != state->crtc_w ||
>>> +	    plane->state->crtc_h != state->crtc_h ||
>>> +	    !plane->state->fb != !state->fb)
>>> +		return -EINVAL;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void intel_plane_atomic_async_update(struct drm_plane *plane,
>>> +					    struct drm_plane_state *new_state)
>>> +{
>>> +	struct intel_atomic_state *intel_new_state =
>>> +		to_intel_atomic_state(new_state->state);
>>> +	struct intel_plane *intel_plane = to_intel_plane(plane);
>>> +	struct drm_crtc *crtc = plane->state->crtc;
>>> +	struct intel_crtc_state *new_crtc_state;
>>> +	struct intel_crtc *intel_crtc;
>>> +	int i;
>>> +
>>> +	for_each_new_intel_crtc_in_state(intel_new_state, intel_crtc,
>>> +					 new_crtc_state, i)
>>> +		WARN_ON(new_crtc_state->wm.need_postvbl_update ||
>>> +			new_crtc_state->update_wm_post);
>>> +
>>> +	i915_gem_track_fb(intel_fb_obj(plane->state->fb),
>>> +			  intel_fb_obj(new_state->fb),
>>> +			  intel_plane->frontbuffer_bit);
>>> +
>>> +	plane->state->src_x = new_state->src_x;
>>> +	plane->state->src_y = new_state->src_y;
>>> +	plane->state->crtc_x = new_state->crtc_x;
>>> +	plane->state->crtc_y = new_state->crtc_y;
>>> +
>>> +	swap(plane->state->fb, new_state->fb);
>>> +
>>> +	if (plane->state->visible)
>>> +		intel_update_plane(intel_plane,
>>> +				   to_intel_crtc_state(crtc->state),
>>> +				   to_intel_plane_state(plane->state));
>>> +	else
>>> +		intel_disable_plane(intel_plane,
>>> +				    to_intel_crtc_state(crtc->state));
>>> +}
>>> +
>>>  const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
>>>  	.prepare_fb = intel_prepare_plane_fb,
>>>  	.cleanup_fb = intel_cleanup_plane_fb,
>>>  	.atomic_check = intel_plane_atomic_check,
>>> +	.atomic_async_check = intel_plane_atomic_async_check,
>>> +	.atomic_async_update = intel_plane_atomic_async_update,
>>>  };
>>>  
>>>  /**
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 415d8968f2c5..244e1c94277d 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -13464,6 +13464,15 @@ static int intel_atomic_commit(struct drm_device *dev,
>>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>>>  	int ret = 0;
>>>  
>>> +	if (state->async_update) {
>>> +		ret = drm_atomic_helper_prepare_planes(dev, state);
>>> +		if (ret)
>>> +			return ret;
>>> +		drm_atomic_helper_async_commit(dev, state);
>>> +		drm_atomic_helper_cleanup_planes(dev, state);
>>> +		return 0;
>>> +	}
>>> +
>>>  	drm_atomic_state_get(state);
>>>  	i915_sw_fence_init(&intel_state->commit_ready,
>>>  			   intel_atomic_commit_ready);
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2019-03-08 15:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-07 16:49 [PATCH v8 1/2] drm/i915: Introduce async plane update to i915 Helen Koike
2019-03-07 16:49 ` [PATCH v8 2/2] drm/i915: update cursors asynchronously through atomic Helen Koike
2019-03-08  8:08 ` [PATCH v8 1/2] drm/i915: Introduce async plane update to i915 Zhang, Tina
2019-03-08  9:55 ` Jani Nikula
2019-03-08 14:51   ` Helen Koike
2019-03-08 15:34     ` Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.