All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org
Subject: [CI-only 2/8] drm/i915: Use vblank worker to unpin old legacy cursor fb safely
Date: Fri, 12 Apr 2024 15:09:38 +0200	[thread overview]
Message-ID: <20240412130946.13148-2-maarten.lankhorst@linux.intel.com> (raw)
In-Reply-To: <20240412130946.13148-1-maarten.lankhorst@linux.intel.com>

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.

This patch is slightly reworked by Maarten

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

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 23a122ee20c9..3cebeaa6e8b4 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -674,6 +674,17 @@ 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);
+}
+
 static int
 intel_legacy_cursor_update(struct drm_plane *_plane,
 			   struct drm_crtc *_crtc,
@@ -817,14 +828,25 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
 
 	intel_psr_unlock(crtc_state);
 
-	intel_plane_unpin_fb(old_plane_state);
+	if (old_plane_state->ggtt_vma != new_plane_state->ggtt_vma) {
+		drm_vblank_work_init(&old_plane_state->unpin_work, &crtc->base,
+				     intel_cursor_unpin_work);
+
+		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_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a92b67adee9c..46982fdc6a30 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"
@@ -6932,6 +6933,8 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state)
 			continue;
 
 		intel_crtc_disable_planes(state, crtc);
+
+		drm_vblank_work_flush_all(&crtc->base);
 	}
 
 	/* 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 0f4bd5710796..563637f20abf 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -730,6 +730,9 @@ struct intel_plane_state {
 	struct intel_fb_view view;
 	u32 phys_dma_addr; /* for cursor_needs_physical */
 
+	/* for legacy cursor fb unpin */
+	struct drm_vblank_work unpin_work;
+
 	/* Plane pxp decryption state */
 	bool decrypt;
 
-- 
2.43.0


  reply	other threads:[~2024-04-12 13:09 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-12 13:09 [CI-only 1/8] drm: Add drm_vblank_work_flush_all() Maarten Lankhorst
2024-04-12 13:09 ` Maarten Lankhorst [this message]
2024-04-12 13:09 ` [CI-only 3/8] drm/i915: Use the same vblank worker for atomic unpin Maarten Lankhorst
2024-04-12 13:09 ` [CI-only 4/8] drm/xe/display: Preparations for preallocating dpt bo Maarten Lankhorst
2024-04-12 13:09 ` [CI-only 5/8] drm/xe: Remove safety check from __xe_ttm_stolen_io_mem_reserve_stolen Maarten Lankhorst
2024-04-12 13:09 ` [CI-only 6/8] drm/xe: Use simple xchg to cache DPT Maarten Lankhorst
2024-04-12 13:09 ` [CI-only 7/8] drm/xe/display: Prevent overwriting original GGTT when taking over initial FB Maarten Lankhorst
2024-04-12 13:09 ` [CI-only 8/8] drm/xe/display: Re-use display vmas when possible Maarten Lankhorst
2024-04-15 11:52 ` ✗ Fi.CI.SPARSE: warning for series starting with [CI-only,1/8] drm: Add drm_vblank_work_flush_all() Patchwork
2024-04-15 11:59 ` ✓ Fi.CI.BAT: success " Patchwork
2024-04-15 12:14 ` ✓ CI.Patch_applied: " Patchwork
2024-04-15 12:15 ` ✗ CI.checkpatch: warning " Patchwork
2024-04-15 12:15 ` ✓ CI.KUnit: success " Patchwork
2024-04-15 12:27 ` ✓ CI.Build: " Patchwork
2024-04-15 12:29 ` ✓ CI.Hooks: " Patchwork
2024-04-15 12:31 ` ✗ CI.checksparse: warning " Patchwork
2024-04-15 12:55 ` ✓ CI.BAT: success " Patchwork
2024-04-15 14:50 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-04-15 17:01 ` ✗ CI.FULL: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240412130946.13148-2-maarten.lankhorst@linux.intel.com \
    --to=maarten.lankhorst@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=intel-xe@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.