[PATCH] drm: Make the bw/link rate calculations more forgiving

[Sean Paul]

On Wed, Jul 17, 2019 at 07:20:29PM +0300, Ville Syrjälä wrote:
> On Wed, Jul 17, 2019 at 12:01:48PM -0400, Sean Paul wrote:
> > From: Sean Paul <seanpaul at chromium.org>
> > 
> > Although the DisplayPort spec explicitly calls out the 1.62/2.7/5.4/8.1
> > link rates, the value of LINK_BW_SET is calculated.  The DisplayPort
> > spec says "Main-Link Bandwidth Setting = Value x 0.27Gbps/lane".
> > 
> > A bridge that we're looking to upstream uses 6.75Gbps rate (value 0x19)
> > [1], and that precludes it from using these functions.
> 
> The DP spec has this note:
> "A MyDP Source device, upon reading the MAX_LINK_RATE register of the
>  downstream DPRX programmed to 19h (which can be the case only for a
>  MyDP-to-Legacy or MyDP-to-DP lane count converter) can program the
>  LINK_BW_SET register (DPCD Address 00100h) to 19h to enable 6.75Gbps/lane."
> 
> Which I guess is the thing you're seeing.

Thanks for digging this up. The spec I reviewed didn't have this, but it did
mention some more esoteric exceptions to the Big 4 (it might not even have HBR3
either, I'm not sure. /me really needs to get up-to-date specs) defined rates. I'll
add this to my commit message when I apply it.

> 
> But I think the patch makes sense in any case. Otherwise anyone who
> plugs in some new display to a machine with an old kernel will likely
> get that WARN. Assuming the driver correctly limits the max link rate
> based on the source device capabilities there's no harm in letting
> the sink declare higher limits.

Yep, and not only the WARN, but they'll also get 1.62Gbps rate/code. So
calculating the value seems like the safer route to take.

> 
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Thanks! I'll apply it to drm-misc-next.

Sean

> 
> > 
> > This patch calculates the values according to spec instead of
> > restricting these values to one of the DP_LINK_BW_* #defines.
> > 
> > No functional change for the well-defined values, but we lose the
> > warning for ill-defined bw values.
> > 
> > Signed-off-by: Sean Paul <seanpaul at chromium.org>
> > 
> > [1] https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1689251/2/drivers/gpu/drm/bridge/analogix/anx7625.c#636
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 31 ++++---------------------------
> >  1 file changed, 4 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > index 0b994d083a89..ffc68d305afe 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -152,38 +152,15 @@ EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
> >  
> >  u8 drm_dp_link_rate_to_bw_code(int link_rate)
> >  {
> > -	switch (link_rate) {
> > -	default:
> > -		WARN(1, "unknown DP link rate %d, using %x\n", link_rate,
> > -		     DP_LINK_BW_1_62);
> > -		/* fall through */
> > -	case 162000:
> > -		return DP_LINK_BW_1_62;
> > -	case 270000:
> > -		return DP_LINK_BW_2_7;
> > -	case 540000:
> > -		return DP_LINK_BW_5_4;
> > -	case 810000:
> > -		return DP_LINK_BW_8_1;
> > -	}
> > +	/* Spec says link_bw = link_rate / 0.27Gbps */
> > +	return link_rate / 27000;
> >  }
> >  EXPORT_SYMBOL(drm_dp_link_rate_to_bw_code);
> >  
> >  int drm_dp_bw_code_to_link_rate(u8 link_bw)
> >  {
> > -	switch (link_bw) {
> > -	default:
> > -		WARN(1, "unknown DP link BW code %x, using 162000\n", link_bw);
> > -		/* fall through */
> > -	case DP_LINK_BW_1_62:
> > -		return 162000;
> > -	case DP_LINK_BW_2_7:
> > -		return 270000;
> > -	case DP_LINK_BW_5_4:
> > -		return 540000;
> > -	case DP_LINK_BW_8_1:
> > -		return 810000;
> > -	}
> > +	/* Spec says link_rate = link_bw * 0.27Gbps */
> > +	return link_bw * 27000;
> >  }
> >  EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
> >  
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Sean Paul, Software Engineer, Google / Chromium OS