[PATCH] drm/i915/display: Fix RGB limited range handling for DP

[Almahallawy, Khaled]

Thank you for your patch. This fix enables us to pass the DP1.4 Link
Layer Test "4.2.2.8 EDID Read on IRQ HPD Event after Branch Device
Detection," which was failing due to the following error:

"0003.610.625: Main Stream Attributes do not match expected values:
0003.610.669:   Received 28h color format differs from expected 20h."

Tested-by: Khaled Almahallawy <khaled.almahallawy at intel.com>

On Tue, 2025-07-01 at 14:15 +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Sent: Tuesday, July 1, 2025 2:02 PM
> > To: Shankar, Uma <uma.shankar at intel.com>
> > Cc: intel-gfx at lists.freedesktop.org;
> > intel-xe at lists.freedesktop.org; Borah,
> > Chaitanya Kumar <chaitanya.kumar.borah at intel.com>; Almahallawy,
> > Khaled
> > <khaled.almahallawy at intel.com>
> > Subject: Re: [PATCH] drm/i915/display: Fix RGB limited range
> > handling for DP
> > 
> > On Tue, Jul 01, 2025 at 01:47:56PM +0530, Uma Shankar wrote:
> > > RGB limited range should be selected only if explicitly asked by
> > > userspace by the broadcast RGB property with LIMITED_RANGE. This
> > > is
> > > mostly enabled in case of CEA modes.
> > > 
> > > Display port by default uses Full Range, fixed the same. This
> > > will
> > > help set correct MSA information for colorimetry. Fixes a CTS
> > > issue
> > > wrt colorimetry.
> > 
> > If the CTS is expecting that then it's not following the actual DP
> > spec.
> > Originally the spec required limited range output for cea modes,
> > but later version
> > relaxed it to "should" and also notes that full range may also be
> > used. Presumably
> > it was realized that existing implementations had no consitency
> > here and so both
> > behaviours must be allowed.
> 
> Hi Ville,
> Yes, there has been a flip flop. Ideally both should be allowed as
> spec is not enforcing.
> 
> However, we can default to FULL RANGE and do Limited range if
> userspace explicitly asks for it
> using the broadcast RGB property. DP mostly have the FULL RANGE while
> limited range is more
> in the HDMI TVs. This will make CTS also happy. What do you suggest ?
> 
> Regards,
> Uma Shankar
> 
> > > 
> > > Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_dp.c | 21 +++++++------------
> > > --
> > >  1 file changed, 7 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index f48912f308df..8758b9d60d5e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -2708,8 +2708,6 @@ bool intel_dp_limited_color_range(const
> > > struct
> > > intel_crtc_state *crtc_state,  {
> > >  	const struct intel_digital_connector_state
> > > *intel_conn_state =
> > >  		to_intel_digital_connector_state(conn_state);
> > > -	const struct drm_display_mode *adjusted_mode =
> > > -		&crtc_state->hw.adjusted_mode;
> > > 
> > >  	/*
> > >  	 * Our YCbCr output is always limited range.
> > > @@ -2721,18 +2719,13 @@ bool intel_dp_limited_color_range(const
> > > struct
> > intel_crtc_state *crtc_state,
> > >  	if (crtc_state->output_format !=
> > > INTEL_OUTPUT_FORMAT_RGB)
> > >  		return false;
> > > 
> > > -	if (intel_conn_state->broadcast_rgb ==
> > INTEL_BROADCAST_RGB_AUTO) {
> > > -		/*
> > > -		 * See:
> > > -		 * CEA-861-E - 5.1 Default Encoding Parameters
> > > -		 * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video
> > > Colorimetry
> > > -		 */
> > > -		return crtc_state->pipe_bpp != 18 &&
> > > -
> > > 			drm_default_rgb_quant_range(adjusted_mode) ==
> > > -			HDMI_QUANTIZATION_RANGE_LIMITED;
> > > -	} else {
> > > -		return intel_conn_state->broadcast_rgb ==
> > > -			INTEL_BROADCAST_RGB_LIMITED;
> > > +	switch (intel_conn_state->broadcast_rgb) {
> > > +	case INTEL_BROADCAST_RGB_LIMITED:
> > > +		return true;
> > > +	case INTEL_BROADCAST_RGB_FULL:
> > > +	case INTEL_BROADCAST_RGB_AUTO:
> > > +	default:
> > > +		return false;
> > >  	}
> > >  }
> > > 
> > > --
> > > 2.42.0
> > 
> > --
> > Ville Syrjälä
> > Intel