Re: [PATCH v2 05/28] drm/i915/dp_link_caps: Move forced link param helpers to link caps

[Imre Deak <imre.deak@intel.com>]

On Tue, Jun 23, 2026 at 01:22:41PM +0300, Luca Coelho wrote:
> On Tue, 2026-06-16 at 23:08 +0300, Imre Deak wrote:
> > Move the helpers handling forced link parameters to intel_dp_link_caps.c.
> > Their functionality is part of the link capability logic and will be
> > updated to use the link capability state in follow-up changes.
> > 
> > Return the forced link rate and lane count through a
> > struct intel_dp_link_config, which is the canonical way the rest of the
> > link capability API will also accept and return link configurations.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c       | 39 ++++++++++---------
> >  .../gpu/drm/i915/display/intel_dp_link_caps.c | 22 +++++++++++
> >  .../gpu/drm/i915/display/intel_dp_link_caps.h |  5 +++
> >  3 files changed, 47 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index a34d3704a5667..7643fe079e15b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -364,17 +364,16 @@ int intel_dp_max_common_lane_count(struct intel_dp *intel_dp)
> >  	return intel_dp->max_common_lane_count;
> >  }
> >  
> > -static int forced_lane_count(struct intel_dp *intel_dp)
> > -{
> > -	return clamp(intel_dp->link.force_lane_count, 1, intel_dp_max_common_lane_count(intel_dp));
> > -}
> > -
> >  int intel_dp_max_lane_count(struct intel_dp *intel_dp)
> >  {
> > +	struct intel_dp_link_caps *link_caps = intel_dp->link.caps;
> > +	struct intel_dp_link_config forced_params;
> >  	int lane_count;
> >  
> > +	intel_dp_link_caps_get_forced_params(link_caps, &forced_params);
> > +
> >  	if (intel_dp->link.force_lane_count)
> > -		lane_count = forced_lane_count(intel_dp);
> > +		lane_count = forced_params.lane_count;
> >  	else
> >  		lane_count = intel_dp->link.max_lane_count;
> >  
> > @@ -391,8 +390,12 @@ int intel_dp_max_lane_count(struct intel_dp *intel_dp)
> >  
> >  static int intel_dp_min_lane_count(struct intel_dp *intel_dp)
> >  {
> > +	struct intel_dp_link_config forced_params;
> > +
> > +	intel_dp_link_caps_get_forced_params(intel_dp->link.caps, &forced_params);
> > +
> >  	if (intel_dp->link.force_lane_count)
> > -		return forced_lane_count(intel_dp);
> > +		return forced_params.lane_count;
> >  
> >  	return 1;
> >  }
> > @@ -1655,23 +1658,17 @@ static void intel_dp_print_rates(struct intel_dp *intel_dp)
> >  	drm_dbg_kms(display->drm, "common rates: %s\n", seq_buf_str(&s));
> >  }
> >  
> > -static int forced_link_rate(struct intel_dp *intel_dp)
> > -{
> > -	int len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->link.force_rate);
> > -
> > -	if (len == 0)
> > -		return intel_dp_common_rate(intel_dp, 0);
> > -
> > -	return intel_dp_common_rate(intel_dp, len - 1);
> > -}
> > -
> >  int
> >  intel_dp_max_link_rate(struct intel_dp *intel_dp)
> >  {
> > +	struct intel_dp_link_caps *link_caps = intel_dp->link.caps;
> > +	struct intel_dp_link_config forced_params;
> >  	int len;
> >  
> > +	intel_dp_link_caps_get_forced_params(link_caps, &forced_params);
> > +
> >  	if (intel_dp->link.force_rate)
> > -		return forced_link_rate(intel_dp);
> > +		return forced_params.rate;
> >  
> >  	len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->link.max_rate);
> >  
> > @@ -1681,8 +1678,12 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
> >  static int
> >  intel_dp_min_link_rate(struct intel_dp *intel_dp)
> >  {
> > +	struct intel_dp_link_config forced_params;
> > +
> > +	intel_dp_link_caps_get_forced_params(intel_dp->link.caps, &forced_params);
> > +
> >  	if (intel_dp->link.force_rate)
> > -		return forced_link_rate(intel_dp);
> > +		return forced_params.rate;
> >  
> >  	return intel_dp_common_rate(intel_dp, 0);
> >  }
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_caps.c b/drivers/gpu/drm/i915/display/intel_dp_link_caps.c
> > index 37ffd714c6a42..1d3a3ff007a03 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_caps.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_caps.c
> > @@ -41,6 +41,28 @@ int intel_dp_max_common_rate(struct intel_dp *intel_dp)
> >  	return intel_dp_common_rate(intel_dp, intel_dp->num_common_rates - 1);
> >  }
> >  
> > +static int forced_lane_count(struct intel_dp *intel_dp)
> > +{
> > +	return clamp(intel_dp->link.force_lane_count, 1, intel_dp_max_common_lane_count(intel_dp));
> > +}
> > +
> > +static int forced_link_rate(struct intel_dp *intel_dp)
> > +{
> > +	int len = intel_dp_common_len_rate_limit(intel_dp, intel_dp->link.force_rate);
> > +
> > +	if (len == 0)
> > +		return intel_dp_common_rate(intel_dp, 0);
> > +
> > +	return intel_dp_common_rate(intel_dp, len - 1);
> > +}
> > +
> > +void intel_dp_link_caps_get_forced_params(struct intel_dp_link_caps *link_caps,
> > +					  struct intel_dp_link_config *forced_params)
> 
> Is this really _get_? Usually "get" in the function name in my head
> returns something (which in this case would be the forced_params), but
> here we're actually setting them.

The function really just returns the forced params tracked in link_caps
and the returning happens via the forced_params struct pointer passed to
the function; so get is the proper name for the function. "set" would
set the forced params in link_caps.

> 
> Should it be _set_, or maybe "parse"?
> 
> 
> > +{
> > +	forced_params->rate = forced_link_rate(link_caps->dp);
> > +	forced_params->lane_count = forced_lane_count(link_caps->dp);
> > +}
> > +
> >  struct intel_dp_link_caps *intel_dp_link_caps_init(struct intel_dp *intel_dp)
> >  {
> >  	struct intel_dp_link_caps *link_caps;
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_caps.h b/drivers/gpu/drm/i915/display/intel_dp_link_caps.h
> > index 3248777d1287f..61dbce86ee3d0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_link_caps.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_link_caps.h
> > @@ -5,12 +5,17 @@
> >  #define __INTEL_DP_LINK_CAPS_H__
> >  
> >  struct intel_dp;
> > +struct intel_dp_link_caps;
> > +struct intel_dp_link_config;
> >  
> >  int intel_dp_common_len_rate_limit(const struct intel_dp *intel_dp,
> >  				   int max_rate);
> >  int intel_dp_common_rate(struct intel_dp *intel_dp, int index);
> >  int intel_dp_max_common_rate(struct intel_dp *intel_dp);
> >  
> > +void intel_dp_link_caps_get_forced_params(struct intel_dp_link_caps *link_caps,
> > +					  struct intel_dp_link_config *forced_params);
> > +
> >  struct intel_dp_link_caps *intel_dp_link_caps_init(struct intel_dp *intel_dp);
> >  void intel_dp_link_caps_cleanup(struct intel_dp_link_caps *link_caps);
> >  
> 
> --
> Cheers,
> Luca.