Re: [Intel-gfx] [PATCH v2 12/12] drm/i915/dp: Disable DFP RGB->YCbCr conversion for now

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

On Fri, Apr 01, 2022 at 07:00:02AM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Sent: Tuesday, March 22, 2022 5:30 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma
> > <uma.shankar@intel.com>
> > Subject: [PATCH v2 12/12] drm/i915/dp: Disable DFP RGB->YCbCr conversion for
> > now
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We lack sufficient state tracking to figure out whether we want the DFP to perform
> > the RGB->YCbCr conversion for us or not. So currently we are blindly just enabling
> > that all the time when supported by the DFP. That is nonsense. So until we imporve
> > our state tracking for this just disable the feature.
> > 
> > Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > Cc: Uma Shankar <uma.shankar@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index d3d458b3267d..a57581198a47 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1180,6 +1180,7 @@ static bool intel_dp_supports_dsc(struct intel_dp
> > *intel_dp,  static bool intel_dp_is_ycbcr420(struct intel_dp *intel_dp,
> >  				 const struct intel_crtc_state *crtc_state)  {
> > +	/* FIXME see intel_dp_update_420() regarding rgb_to_ycbcr */
> >  	return crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420
> > ||
> >  		(crtc_state->output_format ==
> > INTEL_OUTPUT_FORMAT_YCBCR444 &&
> >  		 intel_dp->dfp.ycbcr_444_to_420);
> > @@ -2574,6 +2575,7 @@ void intel_dp_configure_protocol_converter(struct
> > intel_dp *intel_dp,
> >  			    "Failed to %s protocol converter YCbCr 4:2:0 conversion
> > mode\n",
> >  			    str_enable_disable(intel_dp->dfp.ycbcr_444_to_420));
> > 
> > +	/* FIXME see intel_dp_update_420() regarding rgb_to_ycbcr */
> >  	tmp = intel_dp->dfp.rgb_to_ycbcr ?
> >  		DP_CONVERSION_BT709_RGB_YCBCR_ENABLE : 0;
> > 
> > @@ -4430,6 +4432,14 @@ intel_dp_update_420(struct intel_dp *intel_dp)
> >  	rgb_to_ycbcr = drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp-
> > >dpcd,
> >  								 intel_dp-
> > >downstream_ports,
> > 
> > DP_DS_HDMI_BT709_RGB_YCBCR_CONV);
> > +	/*
> > +	 * FIXME need to actually track whether we're really
> > +	 * going to be doing the RGB->YCbCr connversion or not.
> > +	 * We can't tell by simply looking at intel_dp->dfp.rgb_to_ycbcr.
> > +	 * Readout is going to annoying due to having to read that
> > +	 * state from external hardware that may vanish at any time :(
> > +	 */
> > +	rgb_to_ycbcr = false;
> 
> I think this will break 8k yuv pcon case as of now.
> @Nautiyal, Ankit K Can we get that fixed. We will need 8k YUV from source due to
> bandwidth constraints.
> 
> Rest of the series all looks nice and good.

Left this one alone for now. The FRL bandwidth stuff needs more work as
well anyway, so someone needs to take a good look at all of this at some
point.

Pushed the rest. Thanks for the review.

-- 
Ville Syrjälä
Intel