ACK/cmnt: [PATCH V2 1/2][SRU][H/OEM-5.10] UBUNTU: SAUCE: drm/i915: Stop force enabling pipe bottom color gammma/csc

Kleber Souza kleber.souza at
Fri Oct 8 07:32:00 UTC 2021

On 04.10.21 07:48, Koba Ko wrote:
> From: Ville Syrjälä <ville.syrjala at>
> BugLink:
> 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:
> Signed-off-by: Ville Syrjälä <ville.syrjala at>
> Ref:
> Signed-off-by: Koba Ko <koba.ko at>

As Tim mentioned, this patch has been applied to linux-next so we
should use the provenance block from it and have something like:

Signed-off-by: Ville Syrjälä <ville.syrjala at>
Reviewed-by: Uma Shankar <uma.shankar at>
(backported from f22f4e5be89c4296d76eaa9ba83dda46bdf11134 linux-next)
Signed-off-by: Koba Ko <koba.ko at>

Note that it should be "backported" instead of "cherry picked" as
the original patch can't be applied with 'git am' due to context

The patch subject should also have "UBUNTU: SAUCE:" removed.

Acked-by: Kleber Sacilotto de Souza <kleber.souza at>


> ---
>   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 c55e3c504c6d0..a35462277637c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -18392,13 +18392,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 (INTEL_GEN(dev_priv) >= 9)
> -			intel_de_write(dev_priv, SKL_BOTTOM_COLOR(crtc->pipe),
> +		/* 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

More information about the kernel-team mailing list