Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-xe] [PATCH 1/3] drm/i915: Use vblank worker to unpin old legacy cursor fb safely
@ 2023-09-20 14:03 Ville Syrjala
  2023-09-20 14:03 ` [Intel-xe] [PATCH 2/3] drm/vblank: Warn when silently cancelling vblank works Ville Syrjala
                   ` (14 more replies)
  0 siblings, 15 replies; 19+ messages in thread
From: Ville Syrjala @ 2023-09-20 14:03 UTC (permalink / raw)
  To: intel-xe

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The cursor hardware only does sync updates, and thus the hardware
will be scanning out from the old fb until the next start of vblank.
So in order to make the legacy cursor fastpath actually safe we
should not unpin the old fb until we're sure the hardware has
ceased accessing it. The simplest approach is to just use a vblank
work here to do the delayed unpin.

Not 100% sure it's a good idea to put this onto the same high
priority vblank worker as eg. our timing critical gamma updates.
But let's keep it simple for now, and it we later discover that
this is causing problems we can think about adding a lower
priority worker for such things.

v2: wait for cursor unpins before turning off the vblank irq

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_cursor.c   | 36 +++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_cursor.h   |  2 ++
 drivers/gpu/drm/i915/display/intel_display.c  |  3 ++
 .../drm/i915/display/intel_display_types.h    |  4 +++
 4 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 8ed8a623fa98..afe1d0fa0537 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -605,6 +605,26 @@ static bool intel_cursor_format_mod_supported(struct drm_plane *_plane,
 	return format == DRM_FORMAT_ARGB8888;
 }
 
+static void intel_cursor_unpin_work(struct kthread_work *base)
+{
+	struct drm_vblank_work *work = to_drm_vblank_work(base);
+	struct intel_plane_state *plane_state =
+		container_of(work, typeof(*plane_state), unpin_work);
+	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
+
+	intel_plane_unpin_fb(plane_state);
+	intel_plane_destroy_state(&plane->base, &plane_state->uapi);
+
+	if (atomic_dec_and_test(&plane->cursor.pending_unpins))
+		wake_up_var(&plane->cursor.pending_unpins);
+}
+
+void intel_cursor_wait_unpin_works(struct intel_plane *plane)
+{
+	wait_var_event(&plane->cursor.pending_unpins,
+		       !atomic_read(&plane->cursor.pending_unpins));
+}
+
 static int
 intel_legacy_cursor_update(struct drm_plane *_plane,
 			   struct drm_crtc *_crtc,
@@ -732,14 +752,26 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
 
 	local_irq_enable();
 
-	intel_plane_unpin_fb(old_plane_state);
+	if (old_plane_state->hw.fb != new_plane_state->hw.fb) {
+		drm_vblank_work_init(&old_plane_state->unpin_work, &crtc->base,
+				     intel_cursor_unpin_work);
+
+		atomic_inc(&plane->cursor.pending_unpins);
+		drm_vblank_work_schedule(&old_plane_state->unpin_work,
+					 drm_crtc_accurate_vblank_count(&crtc->base) + 1,
+					 false);
+
+		old_plane_state = NULL;
+	} else {
+		intel_plane_unpin_fb(old_plane_state);
+	}
 
 out_free:
 	if (new_crtc_state)
 		intel_crtc_destroy_state(&crtc->base, &new_crtc_state->uapi);
 	if (ret)
 		intel_plane_destroy_state(&plane->base, &new_plane_state->uapi);
-	else
+	else if (old_plane_state)
 		intel_plane_destroy_state(&plane->base, &old_plane_state->uapi);
 	return ret;
 
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.h b/drivers/gpu/drm/i915/display/intel_cursor.h
index ce333bf4c2d5..e778aff77129 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.h
+++ b/drivers/gpu/drm/i915/display/intel_cursor.h
@@ -14,4 +14,6 @@ struct intel_plane *
 intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 			  enum pipe pipe);
 
+void intel_cursor_wait_unpin_works(struct intel_plane *plane);
+
 #endif
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b2e16cb8d026..f5a45722636b 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -65,6 +65,7 @@
 #include "intel_crt.h"
 #include "intel_crtc.h"
 #include "intel_crtc_state_dump.h"
+#include "intel_cursor.h"
 #include "intel_ddi.h"
 #include "intel_de.h"
 #include "intel_display_driver.h"
@@ -6669,6 +6670,8 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 
 		intel_pre_plane_update(state, crtc);
 		intel_crtc_disable_planes(state, crtc);
+
+		intel_cursor_wait_unpin_works(to_intel_plane(crtc->base.cursor));
 	}
 
 	/* Only disable port sync and MST slaves */
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index c0931d89d0dd..bf17ae833ff1 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -710,6 +710,9 @@ struct intel_plane_state {
 
 	struct intel_fb_view view;
 
+	/* for legacy cursor fb unpin */
+	struct drm_vblank_work unpin_work;
+
 	/* Plane pxp decryption state */
 	bool decrypt;
 
@@ -1523,6 +1526,7 @@ struct intel_plane {
 
 	struct {
 		u32 base, cntl, size;
+		atomic_t pending_unpins;
 	} cursor;
 
 	struct intel_fbc *fbc;
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2023-10-03 13:34 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-20 14:03 [Intel-xe] [PATCH 1/3] drm/i915: Use vblank worker to unpin old legacy cursor fb safely Ville Syrjala
2023-09-20 14:03 ` [Intel-xe] [PATCH 2/3] drm/vblank: Warn when silently cancelling vblank works Ville Syrjala
2023-09-20 19:55   ` Lyude Paul
2023-10-03 13:34     ` Ville Syrjälä
2023-09-20 14:03 ` [Intel-xe] [PATCH 3/3] maybe xe is broken enough that it gives you a new vma every time? Ville Syrjala
2023-09-20 17:27   ` [Intel-xe] [PATCH v2 " Ville Syrjala
2023-09-20 14:06 ` [Intel-xe] ✓ CI.Patch_applied: success for series starting with [1/3] drm/i915: Use vblank worker to unpin old legacy cursor fb safely Patchwork
2023-09-20 14:06 ` [Intel-xe] ✗ CI.checkpatch: warning " Patchwork
2023-09-20 14:07 ` [Intel-xe] ✓ CI.KUnit: success " Patchwork
2023-09-20 14:14 ` [Intel-xe] ✓ CI.Build: " Patchwork
2023-09-20 14:15 ` [Intel-xe] ✓ CI.Hooks: " Patchwork
2023-09-20 14:16 ` [Intel-xe] ✗ CI.checksparse: warning " Patchwork
2023-09-20 14:48 ` [Intel-xe] ✗ CI.BAT: failure " Patchwork
2023-09-20 18:45 ` [Intel-xe] ✓ CI.Patch_applied: success for series starting with [1/3] drm/i915: Use vblank worker to unpin old legacy cursor fb safely (rev2) Patchwork
2023-09-20 18:45 ` [Intel-xe] ✗ CI.checkpatch: warning " Patchwork
2023-09-20 18:46 ` [Intel-xe] ✓ CI.KUnit: success " Patchwork
2023-09-20 18:53 ` [Intel-xe] ✓ CI.Build: " Patchwork
2023-09-20 18:54 ` [Intel-xe] ✓ CI.Hooks: " Patchwork
2023-09-20 18:55 ` [Intel-xe] ✗ CI.checksparse: warning " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox