All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915: Stop force enabling pipe bottom color gammma/csc
@ 2021-09-28 18:51 Ville Syrjala
  2021-09-28 20:09 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ville Syrjala @ 2021-09-28 18:51 UTC (permalink / raw)
  To: intel-gfx

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

While sanitizing the hardware state we're currently forcing
the pipe bottom color legacy csc/gamma bits on. That is not a
good idea as BIOSen are likely to leave gabage in the LUTs and
so doing this causes ugly visual glitches if and when the
planes covering the background get disabled. This was exactly
the case on this Dell Precision 5560 tgl laptop.

On icl+ we don't normally even use these legacy bits
anymore and instead use their GAMMA_MODE counterparts.
On earlier platforms the bits are used, but we still
shouldn't force them on without knowing what's in the LUT.

So two options, get rid of the whole thing, or do what
intel_color_commit() does to make sure the bottom color state
matches whatever out hardware readout produced. I chose the
latter since it'll match what happens on older platforms when
the primary plane gets turned off. In fact let's just call
intel_color_commit(). It'll also do some CSC programming but
since we don't have readout for that it'll actually just set
to all zeros. So in the unlikely case of CSC actually being
enabld by the BIOS we'll end up with all black until the first
atomic commit happens.

Still not totally sure what we should do about color management
features here in general. Probably the safest  thing would be to
force everything off exactly at the same time when we disable
the primary plane as there is no guarantees that whatever the
LUTs/CSCs contain make any sense whatsoever without the
specific pixel data in the BIOS fb. And if we preserve the
primary plane then we should disable the color management
features exactly when the primary plane fb contents first
changes since the new content assumes more or less no
transformations. But of course synchronizing front buffer
rendering with anything else is a bit hard...

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3534
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f27c294beb92..17ce51576ed2 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -11991,13 +11991,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 				intel_plane_disable_noatomic(crtc, plane);
 		}
 
-		/*
-		 * Disable any background color set by the BIOS, but enable the
-		 * gamma and CSC to match how we program our planes.
-		 */
-		if (DISPLAY_VER(dev_priv) >= 9)
-			intel_de_write(dev_priv, SKL_BOTTOM_COLOR(crtc->pipe),
-				       SKL_BOTTOM_COLOR_GAMMA_ENABLE | SKL_BOTTOM_COLOR_CSC_ENABLE);
+		/* Disable any background color/etc. set by the BIOS */
+		intel_color_commit(crtc_state);
 	}
 
 	/* Adjust the state of the output pipe according to whether we
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-01 18:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 18:51 [Intel-gfx] [PATCH] drm/i915: Stop force enabling pipe bottom color gammma/csc Ville Syrjala
2021-09-28 20:09 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2021-09-28 22:36 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2021-10-01 17:12 ` [Intel-gfx] [PATCH] " Shankar, Uma
2021-10-01 17:45   ` Ville Syrjälä
2021-10-01 17:59     ` Ville Syrjälä

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.