[PATCH v8 1/2] drm/i915: Introduce async plane update to i915

Helen Koike helen.koike at collabora.com
Thu Mar 7 16:49:47 UTC 2019


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

Add implementation for async plane update callbacks

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



More information about the dri-devel mailing list