Re: [Intel-gfx] [PATCH 2/3] drm/i915: Update eDP fast link training link rate parsing

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

On Thu, Jun 02, 2022 at 12:38:48PM +0300, Jani Nikula wrote:
> On Wed, 01 Jun 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We're not parsing the 5.4 Gbps value for the old eDP fast link
> > training link rate, nor are we parsing the new fast link training
> > link rate field. Remedy both.
> >
> > Note that we're not even using this information for anything
> > currently, so should perhaps just nuke it all unless someone
> > is planning on implementing fast link training finally...
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_bios.c     | 32 ++++++++++++-------
> >  drivers/gpu/drm/i915/display/intel_vbt_defs.h |  1 +
> >  2 files changed, 21 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> > index c42b9e7d0dce..d701854dc429 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> > @@ -1367,18 +1367,26 @@ parse_edp(struct drm_i915_private *i915,
> >  
> >  	panel->vbt.edp.pps = *edp_pps;
> >  
> > -	switch (edp_link_params->rate) {
> > -	case EDP_RATE_1_62:
> > -		panel->vbt.edp.rate = DP_LINK_BW_1_62;
> > -		break;
> > -	case EDP_RATE_2_7:
> > -		panel->vbt.edp.rate = DP_LINK_BW_2_7;
> > -		break;
> > -	default:
> > -		drm_dbg_kms(&i915->drm,
> > -			    "VBT has unknown eDP link rate value %u\n",
> > -			     edp_link_params->rate);
> > -		break;
> > +	if (i915->vbt.version >= 224) {
> > +		panel->vbt.edp.rate =
> > +			edp->edp_fast_link_training_rate[panel_type] * 2;
> 
> So this is in units of 200 kHz.

Right. Or even more specifically it's 200 kbps units (based on the example
values in the spec). So we'll want *20 here to get the 10 kbps units we
use.

> 
> > +	} else {
> > +		switch (edp_link_params->rate) {
> > +		case EDP_RATE_1_62:
> > +			panel->vbt.edp.rate = DP_LINK_BW_1_62;
> 
> However the DP_LINK_BW_* stuff are the values to be written to DPCD. So
> different things end up in panel->vbt.edp.rate depending on VBT
> version. It's a mess. Should probably have the rate there and throw out
> the DP_LINK_BW_* macros.

Oh right, I forgot those are the DPCD values. I'll switch to real
numbers here.

> 
> Otherwise, the logic seems fine.
> 
> BR,
> Jani.
> 
> > +			break;
> > +		case EDP_RATE_2_7:
> > +			panel->vbt.edp.rate = DP_LINK_BW_2_7;
> > +			break;
> > +		case EDP_RATE_5_4:
> > +			panel->vbt.edp.rate = DP_LINK_BW_5_4;
> > +			break;
> > +		default:
> > +			drm_dbg_kms(&i915->drm,
> > +				    "VBT has unknown eDP link rate value %u\n",
> > +				    edp_link_params->rate);
> > +			break;
> > +		}
> >  	}
> >  
> >  	switch (edp_link_params->lanes) {
> > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > index 14f1e1cc92c5..58aee0a040cf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > @@ -638,6 +638,7 @@ struct bdb_sdvo_panel_dtds {
> >  #define EDP_30BPP	2
> >  #define EDP_RATE_1_62	0
> >  #define EDP_RATE_2_7	1
> > +#define EDP_RATE_5_4	2
> >  #define EDP_LANE_1	0
> >  #define EDP_LANE_2	1
> >  #define EDP_LANE_4	3
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel