[Intel-gfx] [PATCH 1/3] drm/i915: Extract intel_dp_output_format()

[Ville Syrjälä]

On Thu, Sep 17, 2020 at 04:57:09PM -0700, Navare, Manasi wrote:
> On Fri, Sep 18, 2020 at 12:43:33AM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Refactor the output_format calculation into a helper so that
> > we can reuse it for mode validation as well.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 32 +++++++++++++++----------
> >  1 file changed, 20 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index bf1e9cf1c0f3..ad9b8b16fadb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -592,6 +592,22 @@ static u8 intel_dp_dsc_get_slice_count(struct intel_dp *intel_dp,
> >  	return 0;
> >  }
> >  
> > +static enum intel_output_format
> > +intel_dp_output_format(struct drm_connector *connector,
> > +		       const struct drm_display_mode *mode)
> > +{
> > +	struct intel_dp *intel_dp = intel_attached_dp(to_intel_connector(connector));
> > +	const struct drm_display_info *info = &connector->display_info;
> > +
> > +	if (!drm_mode_is_420_only(info, mode))
> > +		return INTEL_OUTPUT_FORMAT_RGB;
> > +
> > +	if (intel_dp->dfp.ycbcr_444_to_420)
> > +		return INTEL_OUTPUT_FORMAT_YCBCR444;
> > +	else
> > +		return INTEL_OUTPUT_FORMAT_YCBCR420;
> > +}
> > +
> >  static bool intel_dp_hdisplay_bad(struct drm_i915_private *dev_priv,
> >  				  int hdisplay)
> >  {
> > @@ -2430,27 +2446,20 @@ intel_dp_compute_link_config(struct intel_encoder *encoder,
> >  }
> >  
> >  static int
> > -intel_dp_ycbcr420_config(struct intel_dp *intel_dp,
> > -			 struct intel_crtc_state *crtc_state,
> > +intel_dp_ycbcr420_config(struct intel_crtc_state *crtc_state,
> >  			 const struct drm_connector_state *conn_state)
> >  {
> >  	struct drm_connector *connector = conn_state->connector;
> > -	const struct drm_display_info *info = &connector->display_info;
> >  	const struct drm_display_mode *adjusted_mode =
> >  		&crtc_state->hw.adjusted_mode;
> >  
> >  	if (!connector->ycbcr_420_allowed)
> >  		return 0;
> >  
> > -	if (!drm_mode_is_420_only(info, adjusted_mode))
> > -		return 0;
> > +	crtc_state->output_format = intel_dp_output_format(connector, adjusted_mode);
> 
> So by default if its not 420_only then we set it to RGB?

Yes. The code is still a bit messy because we have three places where
we set this. Probably will try to unify it a bit more, and try to get
the lspcon stuff looking more like any other protocol converter.

Actually IIRC I noticed that some lspcon chips seem to have some
of the 1.3 protocol converter registers even though they only
advertise DPCD 1.2. Not yet sure how to handle that in the
cleanest way possible...

-- 
Ville Syrjälä
Intel