Re: [PATCH 2/7] drm/i915: Extract intel_dp_has_dsc()

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

On Mon, May 20, 2024 at 01:47:34PM +0300, Jani Nikula wrote:
> On Fri, 17 May 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Extract a helper to check whether the source+sink combo
> > supports DSC. That basic check is needed both during mode
> > validation and compute config. We'll also need to add extra
> > checks to both places, so having a single place for it is nicer.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 1e88449fe5f2..7bf283b4df7f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1220,6 +1220,19 @@ bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp,
> >  	       connector->force_bigjoiner_enable;
> >  }
> >  
> > +static bool intel_dp_has_dsc(struct intel_connector *connector)
> 
> Why not const?

We've generally not consted these things. And then whenver add
one const somewhere it usually ends up getting in the way later,
not because we need mutability but simply because we want to
call something that doesn't have the const.

I suppose if we do want to start consting things more we should
just do some kind of bigger pass over the whole codebase so that
that there's less chance of pain later.

We're also not using container_of_const() for these right now,
so the const can vanish semi-accidentally when casting things.

I suppose this thing might be low level enough that the const
could be kept. I'll have another think about it.

> 
> > +{
> > +	struct drm_i915_private *i915 = to_i915(connector->base.dev);
> > +
> > +	if (!HAS_DSC(i915))
> > +		return false;
> > +
> > +	if (!drm_dp_sink_supports_dsc(connector->dp.dsc_dpcd))
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> >  static enum drm_mode_status
> >  intel_dp_mode_valid(struct drm_connector *_connector,
> >  		    struct drm_display_mode *mode)
> > @@ -1274,8 +1287,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
> >  	mode_rate = intel_dp_link_required(target_clock,
> >  					   intel_dp_mode_min_output_bpp(connector, mode));
> >  
> > -	if (HAS_DSC(dev_priv) &&
> > -	    drm_dp_sink_supports_dsc(connector->dp.dsc_dpcd)) {
> > +	if (intel_dp_has_dsc(connector)) {
> >  		enum intel_output_format sink_format, output_format;
> >  		int pipe_bpp;
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel