drm/i915/color: Stop using non-posted DSB writes for legacy LUT

Commit Message

Ville Syrjälä Oct. 30, 2024, 9:07 p.m. UTC

From: Ville Syrjälä <[e-mail]>

Apparently using non-posted DSB writes to update the legacy
LUT can cause CPU MMIO accesses to fail on TGL. Stop using
them for the legacy LUT updates, and instead switch to using
the double write approach (which is the other empirically
found workaround for the issue of DSB failing to correctly
update the legacy LUT).

Cc: [e-mail]
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12494
Fixes: 25ea3411bd23 ("drm/i915/dsb: Use non-posted register writes for legacy LUT")
Signed-off-by: Ville Syrjälä <[e-mail]>
---
 drivers/gpu/drm/i915/display/intel_color.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Ville Syrjälä Oct. 31, 2024, 12:16 p.m. UTC | #1

On Thu, Oct 31, 2024 at 09:47:39AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915/color: Stop using non-posted DSB writes for legacy LUT
> URL   : https://patchwork.freedesktop.org/series/140739/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_15615_full -> Patchwork_140739v1_full
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_140739v1_full absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_140739v1_full, please notify your bug team ([e-mail]) to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   
> 
> Participating hosts (10 -> 10)
> ------------------------------
> 
>   No changes in participating hosts
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_140739v1_full:
> 
> ### CI changes ###
> 
> #### Possible regressions ####
> 
<snip> 
> #### Warnings ####
> 
>   * igt@kms_big_fb@y-tiled-8bpp-rotate-90:
>     - shard-tglu:         [SKIP][47] -> [ABORT][48]
>    [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15615/shard-tglu-6/igt@[e-mail]
>    [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_140739v1/shard-tglu-5/igt@[e-mail]

Looks like an actual mismatch in the LUT. Troubling...

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 174753625bca..aa50ecaf368d 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1357,19 +1357,19 @@  static void ilk_load_lut_8(const struct intel_crtc_state *crtc_state,
 	lut = blob->data;
 
 	/*
-	 * DSB fails to correctly load the legacy LUT
-	 * unless we either write each entry twice,
-	 * or use non-posted writes
+	 * DSB fails to correctly load the legacy LUT unless
+	 * we either write each entry twice, or use non-posted
+	 * writes. However using non-posted writes can cause
+	 * CPU MMIO accesses to fail on TGL, so we choose to
+	 * use the double write approach.
 	 */
-	if (crtc_state->dsb_color_vblank)
-		intel_dsb_nonpost_start(crtc_state->dsb_color_vblank);
-
-	for (i = 0; i < 256; i++)
+	for (i = 0; i < 256; i++) {
 		ilk_lut_write(crtc_state, LGC_PALETTE(pipe, i),
 			      i9xx_lut_8(&lut[i]));
-
-	if (crtc_state->dsb_color_vblank)
-		intel_dsb_nonpost_end(crtc_state->dsb_color_vblank);
+		if (crtc_state->dsb_color_vblank)
+			ilk_lut_write(crtc_state, LGC_PALETTE(pipe, i),
+				      i9xx_lut_8(&lut[i]));
+	}
 }
 
 static void ilk_load_lut_10(const struct intel_crtc_state *crtc_state,