All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ville Syrjala <ville.syrjala@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Lucas De Marchi <lucas.demarchi@intel.com>,
	Matt Roper <matthew.d.roper@intel.com>
Subject: [PATCH 9/9] Revert "drm/i915/xe2lpd: Treat cursor plane as regular plane for DDB allocation"
Date: Wed, 13 Dec 2023 12:25:19 +0200	[thread overview]
Message-ID: <20231213102519.13500-10-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <20231213102519.13500-1-ville.syrjala@linux.intel.com>

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

This reverts commit cfeff354f70bb1d0deb0279506e3f7989bc16e28.

A core design consideration with legacy cursor updates is that the
cursor must not touch any other plane, even if we were to force it
to take the slow path. That is the real reason why the cursor uses
a fixed ddb allocation, not because bspec says so.

Treating cursors as any other plane during ddb allocation
violates that, which means we can now pull other planes into
fully unsynced legacy cursor mailbox commits. That is
definitely not something we've ever considered when designing
the rest of the code. The noarm+arm register write split in
particular makes that dangerous as previous updates can get
disarmed pretty much at any random time, and not necessarily
in an order that is actually safe (eg. against ddb overlaps).

So if we were to do this then:
- someone needs to expend the appropriate amount of brain
  cells thinking through all the tricky details
- we should do it for all skl+ platforms since all
  of those have double buffered wm/ddb registers. The current
  arbitrary mtl+ cutoff doesn't really make sense

For the moment just go back to the original behaviour where
the cursor's ddb alloation does not change outside of
modeset/fastset. As of now anything else isn't safe.

Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../gpu/drm/i915/display/intel_atomic_plane.c    |  6 +++---
 drivers/gpu/drm/i915/display/skl_watermark.c     | 16 +++++++---------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 06c2455bdd78..76d77d5a0409 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -217,6 +217,9 @@ intel_plane_relative_data_rate(const struct intel_crtc_state *crtc_state,
 	int width, height;
 	unsigned int rel_data_rate;
 
+	if (plane->id == PLANE_CURSOR)
+		return 0;
+
 	if (!plane_state->uapi.visible)
 		return 0;
 
@@ -244,9 +247,6 @@ intel_plane_relative_data_rate(const struct intel_crtc_state *crtc_state,
 
 	rel_data_rate = width * height * fb->format->cpp[color_plane];
 
-	if (plane->id == PLANE_CURSOR)
-		return rel_data_rate;
-
 	return intel_adjusted_rate(&plane_state->uapi.src,
 				   &plane_state->uapi.dst,
 				   rel_data_rate);
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index 56588d6e24ae..051a02ac01a4 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -1367,7 +1367,7 @@ skl_total_relative_data_rate(const struct intel_crtc_state *crtc_state)
 	u64 data_rate = 0;
 
 	for_each_plane_id_on_crtc(crtc, plane_id) {
-		if (plane_id == PLANE_CURSOR && DISPLAY_VER(i915) < 20)
+		if (plane_id == PLANE_CURSOR)
 			continue;
 
 		data_rate += crtc_state->rel_data_rate[plane_id];
@@ -1514,12 +1514,10 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state *state,
 		return 0;
 
 	/* Allocate fixed number of blocks for cursor. */
-	if (DISPLAY_VER(i915) < 20) {
-		cursor_size = skl_cursor_allocation(crtc_state, num_active);
-		iter.size -= cursor_size;
-		skl_ddb_entry_init(&crtc_state->wm.skl.plane_ddb[PLANE_CURSOR],
-				   alloc->end - cursor_size, alloc->end);
-	}
+	cursor_size = skl_cursor_allocation(crtc_state, num_active);
+	iter.size -= cursor_size;
+	skl_ddb_entry_init(&crtc_state->wm.skl.plane_ddb[PLANE_CURSOR],
+			   alloc->end - cursor_size, alloc->end);
 
 	iter.data_rate = skl_total_relative_data_rate(crtc_state);
 
@@ -1533,7 +1531,7 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state *state,
 			const struct skl_plane_wm *wm =
 				&crtc_state->wm.skl.optimal.planes[plane_id];
 
-			if (plane_id == PLANE_CURSOR && DISPLAY_VER(i915) < 20) {
+			if (plane_id == PLANE_CURSOR) {
 				const struct skl_ddb_entry *ddb =
 					&crtc_state->wm.skl.plane_ddb[plane_id];
 
@@ -1581,7 +1579,7 @@ skl_crtc_allocate_plane_ddb(struct intel_atomic_state *state,
 		const struct skl_plane_wm *wm =
 			&crtc_state->wm.skl.optimal.planes[plane_id];
 
-		if (plane_id == PLANE_CURSOR && DISPLAY_VER(i915) < 20)
+		if (plane_id == PLANE_CURSOR)
 			continue;
 
 		if (DISPLAY_VER(i915) < 11 &&
-- 
2.41.0


  parent reply	other threads:[~2023-12-13 10:26 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-13 10:25 [PATCH 0/9] drm/i915: Cursor vblank evasion Ville Syrjala
2023-12-13 10:25 ` [PATCH 1/9] drm/i915: Decouple intel_crtc_vblank_evade_scanlines() from atomic commits Ville Syrjala
2023-12-20 11:11   ` Shankar, Uma
2023-12-13 10:25 ` [PATCH 2/9] drm/i915: Reorder drm_vblank_put() vs. need_vlv_dsi_wa Ville Syrjala
2023-12-20 11:20   ` Shankar, Uma
2023-12-13 10:25 ` [PATCH 3/9] drm/i915: Introduce struct intel_vblank_evade_ctx Ville Syrjala
2023-12-20 11:22   ` Shankar, Uma
2023-12-13 10:25 ` [PATCH 4/9] drm/i915: Include need_vlv_dsi_wa in intel_vblank_evade_ctx Ville Syrjala
2023-12-20 11:22   ` Shankar, Uma
2023-12-13 10:25 ` [PATCH 5/9] drm/i915: Extract intel_vblank_evade() Ville Syrjala
2023-12-20 11:26   ` Shankar, Uma
2023-12-13 10:25 ` [PATCH 6/9] drm/i915: Move the min/max scanline sanity check into intel_vblank_evade() Ville Syrjala
2023-12-20 11:27   ` Shankar, Uma
2023-12-13 10:25 ` [PATCH 7/9] drm/i915: Move intel_vblank_evade() & co. into intel_vblank.c Ville Syrjala
2023-12-20 11:28   ` Shankar, Uma
2023-12-13 10:25 ` [PATCH 8/9] drm/i915: Perform vblank evasion around legacy cursor updates Ville Syrjala
2023-12-20 11:41   ` Shankar, Uma
2023-12-20 11:45     ` Shankar, Uma
2023-12-20 14:48       ` Ville Syrjälä
2023-12-22  5:07         ` Shankar, Uma
2024-01-16 20:49   ` [PATCH v2 " Ville Syrjala
2024-01-17 13:16     ` Hogander, Jouni
2024-01-18  7:53     ` Hogander, Jouni
2024-01-18  9:03     ` Hogander, Jouni
2023-12-13 10:25 ` Ville Syrjala [this message]
2023-12-13 11:28   ` [PATCH 9/9] Revert "drm/i915/xe2lpd: Treat cursor plane as regular plane for DDB allocation" Lisovskiy, Stanislav
2023-12-13 15:15     ` Ville Syrjälä
2023-12-13 15:29       ` Ville Syrjälä
2023-12-15 11:15         ` Ville Syrjälä
2024-01-09 12:17           ` Lisovskiy, Stanislav
2023-12-13 20:51 ` ✗ Fi.CI.SPARSE: warning for drm/i915: Cursor vblank evasion Patchwork
2023-12-13 21:08 ` ✗ Fi.CI.BAT: failure " Patchwork
2023-12-18 10:35   ` Saarinen, Jani
2024-01-18  8:04     ` Illipilli, TejasreeX
2024-01-19  8:11       ` Saarinen, Jani
2023-12-19 11:54 ` ✓ Fi.CI.BAT: success " Patchwork
2023-12-19 13:51 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-01-09 12:34 ` ✗ Fi.CI.SPARSE: warning for drm/i915: Cursor vblank evasion (rev2) Patchwork
2024-01-09 12:48 ` ✓ Fi.CI.BAT: success " Patchwork
2024-01-09 15:10 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-01-16 21:22 ` ✗ Fi.CI.SPARSE: warning for drm/i915: Cursor vblank evasion (rev3) Patchwork
2024-01-16 21:36 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-01-17  8:02 ` ✓ Fi.CI.IGT: success for drm/i915: Cursor vblank evasion (rev2) Patchwork
2024-01-17  8:07 ` ✓ Fi.CI.BAT: success for drm/i915: Cursor vblank evasion (rev3) Patchwork
2024-01-17 10:31 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-01-17 11:30 ` [PATCH 0/9] drm/i915: Cursor vblank evasion Shankar, Uma
2024-01-17 12:13   ` Ville Syrjälä
2024-01-18  7:57 ` ✓ Fi.CI.IGT: success for drm/i915: Cursor vblank evasion (rev3) Patchwork
2024-01-22 11:43 ` ✗ Fi.CI.SPARSE: warning for drm/i915: Cursor vblank evasion (rev4) Patchwork
2024-01-22 11:56 ` ✓ Fi.CI.BAT: success " Patchwork
2024-01-22 15:03 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-01-22 16:59   ` Ville Syrjälä

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=20231213102519.13500-10-ville.syrjala@linux.intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    --cc=matthew.d.roper@intel.com \
    /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.