Re: [Intel-gfx] [PATCH] drm/i915/display: Prevent double YUV range correction on HDR planes

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Dec 14, 2020 at 10:57:03PM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: andrescj via sendgmr <andrescj@andrescj-cros-specialist.c.googlers.com>
> > On Behalf Of Andres Calderon Jaramillo
> > Sent: Tuesday, December 15, 2020 3:50 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Shankar, Uma <uma.shankar@intel.com>; Venkatesh Reddy, Sushma
> > <sushma.venkatesh.reddy@intel.com>; seanpaul@chromium.org; Andres
> > Calderon Jaramillo <andrescj@chromium.org>
> > Subject: [PATCH] drm/i915/display: Prevent double YUV range correction on HDR
> > planes
> > 
> > From: Andres Calderon Jaramillo <andrescj@chromium.org>
> > 
> > Prevent the ICL HDR plane pipeline from performing YUV color range correction
> > twice when the input is in limited range.
> > 
> > Before this patch the following could happen: user space gives us a YUV buffer in
> > limited range; per the pipeline in [1], the plane would first go through a "YUV
> > Range correct" stage that expands the range; the plane would then go through
> > the "Input CSC" stage which would also expand the range because
> > icl_program_input_csc() would use a matrix and an offset that assume limited-
> > range input; this would ultimately cause dark and light colors to appear darker
> > and lighter than they should respectively.
> > 
> > This is an issue because if a buffer switches between being scanned out and
> > being composited with the GPU, the user will see a color difference.
> > If this switching happens quickly and frequently, the user will perceive this as a
> > flickering.
> > 
> > [1] https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-icllp-
> > vol12-displayengine_0.pdf#page=281
> 
> Change looks good to me, double conversion should not be done.
> Plane input csc coefficients take care of the limited range.

Might make sense to actually use the hw range correction instead.
Would avoid having to keep around two sets of coefficients.

Also the question now becomes: How can our tests be passing if
we're doing the range correction twice?

> 
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> 
> > Signed-off-by: Andres Calderon Jaramillo <andrescj@chromium.org>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 761be8deaa9b..aeea344b06ad 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -4811,6 +4811,13 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state
> > *crtc_state,
> >  			plane_color_ctl |=
> > PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> >  	} else if (fb->format->is_yuv) {
> >  		plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> > +
> > +		/*
> > +		 * Disable the YUV range correction stage because the input CSC
> > +		 * stage already takes care of range conversion by using separate
> > +		 * matrices and offsets depending on the color range.
> > +		 */
> > +		plane_color_ctl |=
> > PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> >  	}
> > 
> >  	return plane_color_ctl;
> > --
> > 2.29.2.684.gfbc64c5ab5-goog
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx