| Message ID | 20190717160148.256826-1-sean@poorly.run (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Make the bw/link rate calculations more forgiving | expand |
On Wed, Jul 17, 2019 at 12:01:48PM -0400, Sean Paul wrote: > From: Sean Paul <seanpaul@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. 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. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@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@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@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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Ville Syrjälä > Intel
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);