| Message ID | 20210201150228.10001-1-shawn.c.lee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/vbt: update DP max link rate table | expand |
On Mon, Feb 01, 2021 at 11:02:28PM +0800, Lee Shawn C wrote: > According to Bspec #20124, max link rate table for DP was updated > at BDB version 230. Max link rate can support upto UHBR. > > After migrate to BDB v230, the definition for LBR, HBR2 and HBR3 > were changed. For backward compatibility. If BDB version was > from 216 to 229. Driver have to follow original rule to configure > DP max link rate value from VBT. > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Cooper Chiou <cooper.chiou@intel.com> > Cc: William Tseng <william.tseng@intel.com> > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > --- > drivers/gpu/drm/i915/display/intel_bios.c | 24 ++++++++++++++++++- > drivers/gpu/drm/i915/display/intel_vbt_defs.h | 14 +++++++---- > 2 files changed, 32 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c > index 04337ac6f8c4..be1f732e6550 100644 > --- a/drivers/gpu/drm/i915/display/intel_bios.c > +++ b/drivers/gpu/drm/i915/display/intel_bios.c > @@ -1876,7 +1876,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, > /* DP max link rate for CNL+ */ > if (bdb_version >= 216) { > switch (child->dp_max_link_rate) { > - default: > + case VBT_DP_MAX_LINK_RATE_UHBR20: > + info->dp_max_link_rate = 2000000; > + break; > + case VBT_DP_MAX_LINK_RATE_UHBR13P5: > + info->dp_max_link_rate = 1350000; > + break; > + case VBT_DP_MAX_LINK_RATE_UHBR10: > + info->dp_max_link_rate = 1000000; > + break; > case VBT_DP_MAX_LINK_RATE_HBR3: > info->dp_max_link_rate = 810000; > break; > @@ -1889,7 +1897,21 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, > case VBT_DP_MAX_LINK_RATE_LBR: > info->dp_max_link_rate = 162000; > break; > + case VBT_DP_MAX_LINK_RATE_DEFAULT: > + default: > + info->dp_max_link_rate = 0; > + break; > + } > + > + if (bdb_version < 230) { > + if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_DEFAULT) > + info->dp_max_link_rate = 810000; > + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_LBR) > + info->dp_max_link_rate = 540000; > + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_HBR2) > + info->dp_max_link_rate = 162000; > } I would split this into two separate functions, one does the new mapping, the other the old mapping. > + > drm_dbg_kms(&dev_priv->drm, > "VBT DP max link rate for port %c: %d\n", > port_name(port), info->dp_max_link_rate); > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h > index 6d10fa037751..578a54b33f32 100644 > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h > @@ -343,10 +343,14 @@ enum vbt_gmbus_ddi { > #define DP_AUX_H 0x80 > #define DP_AUX_I 0x90 > > -#define VBT_DP_MAX_LINK_RATE_HBR3 0 > -#define VBT_DP_MAX_LINK_RATE_HBR2 1 > +#define VBT_DP_MAX_LINK_RATE_DEFAULT 0 > +#define VBT_DP_MAX_LINK_RATE_LBR 1 > #define VBT_DP_MAX_LINK_RATE_HBR 2 > -#define VBT_DP_MAX_LINK_RATE_LBR 3 > +#define VBT_DP_MAX_LINK_RATE_HBR2 3 > +#define VBT_DP_MAX_LINK_RATE_HBR3 4 > +#define VBT_DP_MAX_LINK_RATE_UHBR10 5 > +#define VBT_DP_MAX_LINK_RATE_UHBR13P5 6 > +#define VBT_DP_MAX_LINK_RATE_UHBR20 7 And we should keep both old and new names for these. Sadly I can't right now check the spec since it no longer has the old stuff apparently, and the VBT section got moved around so the history no longer shows anything either :( I'll have to pull the whole thing down I guess so I can double check against the old version. > > /* > * The child device config, aka the display device data structure, provides a > @@ -445,8 +449,8 @@ struct child_device_config { > u16 dp_gpio_pin_num; /* 195 */ > u8 dp_iboost_level:4; /* 196 */ > u8 hdmi_iboost_level:4; /* 196 */ > - u8 dp_max_link_rate:2; /* 216 CNL+ */ > - u8 dp_max_link_rate_reserved:6; /* 216 */ > + u8 dp_max_link_rate:3; /* 230 */ > + u8 dp_max_link_rate_reserved:5; /* 230 */ > } __packed; > > struct bdb_general_definitions { > -- > 2.17.1
On Fri, Feb 05, 2021, at 8:26 p.m, Ville Syrjälä wrote: >On Mon, Feb 01, 2021 at 11:02:28PM +0800, Lee Shawn C wrote: >> According to Bspec #20124, max link rate table for DP was updated at >> BDB version 230. Max link rate can support upto UHBR. >> >> After migrate to BDB v230, the definition for LBR, HBR2 and HBR3 were >> changed. For backward compatibility. If BDB version was from 216 to >> 229. Driver have to follow original rule to configure DP max link rate >> value from VBT. >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> Cc: Imre Deak <imre.deak@intel.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Cc: Cooper Chiou <cooper.chiou@intel.com> >> Cc: William Tseng <william.tseng@intel.com> >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_bios.c | 24 ++++++++++++++++++- >> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 14 +++++++---- >> 2 files changed, 32 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c >> b/drivers/gpu/drm/i915/display/intel_bios.c >> index 04337ac6f8c4..be1f732e6550 100644 >> --- a/drivers/gpu/drm/i915/display/intel_bios.c >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c >> @@ -1876,7 +1876,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, >> /* DP max link rate for CNL+ */ >> if (bdb_version >= 216) { >> switch (child->dp_max_link_rate) { >> - default: >> + case VBT_DP_MAX_LINK_RATE_UHBR20: >> + info->dp_max_link_rate = 2000000; >> + break; >> + case VBT_DP_MAX_LINK_RATE_UHBR13P5: >> + info->dp_max_link_rate = 1350000; >> + break; >> + case VBT_DP_MAX_LINK_RATE_UHBR10: >> + info->dp_max_link_rate = 1000000; >> + break; >> case VBT_DP_MAX_LINK_RATE_HBR3: >> info->dp_max_link_rate = 810000; >> break; >> @@ -1889,7 +1897,21 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, >> case VBT_DP_MAX_LINK_RATE_LBR: >> info->dp_max_link_rate = 162000; >> break; >> + case VBT_DP_MAX_LINK_RATE_DEFAULT: >> + default: >> + info->dp_max_link_rate = 0; >> + break; >> + } >> + >> + if (bdb_version < 230) { >> + if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_DEFAULT) >> + info->dp_max_link_rate = 810000; >> + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_LBR) >> + info->dp_max_link_rate = 540000; >> + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_HBR2) >> + info->dp_max_link_rate = 162000; >> } > >I would split this into two separate functions, one does the new mapping, the other the old mapping. > I will split this into two separate functions in patch v2. >> + >> drm_dbg_kms(&dev_priv->drm, >> "VBT DP max link rate for port %c: %d\n", >> port_name(port), info->dp_max_link_rate); diff --git >> a/drivers/gpu/drm/i915/display/intel_vbt_defs.h >> b/drivers/gpu/drm/i915/display/intel_vbt_defs.h >> index 6d10fa037751..578a54b33f32 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h >> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h >> @@ -343,10 +343,14 @@ enum vbt_gmbus_ddi { #define DP_AUX_H 0x80 >> #define DP_AUX_I 0x90 >> >> -#define VBT_DP_MAX_LINK_RATE_HBR3 0 >> -#define VBT_DP_MAX_LINK_RATE_HBR2 1 >> +#define VBT_DP_MAX_LINK_RATE_DEFAULT 0 >> +#define VBT_DP_MAX_LINK_RATE_LBR 1 >> #define VBT_DP_MAX_LINK_RATE_HBR 2 >> -#define VBT_DP_MAX_LINK_RATE_LBR 3 >> +#define VBT_DP_MAX_LINK_RATE_HBR2 3 >> +#define VBT_DP_MAX_LINK_RATE_HBR3 4 >> +#define VBT_DP_MAX_LINK_RATE_UHBR10 5 >> +#define VBT_DP_MAX_LINK_RATE_UHBR13P5 6 >> +#define VBT_DP_MAX_LINK_RATE_UHBR20 7 > >And we should keep both old and new names for these. > >Sadly I can't right now check the spec since it no longer has the >old stuff apparently, and the VBT section got moved around so the >history no longer shows anything either :( I'll have to pull the whole >thing down I guess so I can double check against the old version. > Do you know any kernel doc or suggestion about the naming rule (for new and old BDB version) to solve the problem like this? Just want to know how i915 dirver manage the definition issue before. Best regards, Shawn >> >> /* >> * The child device config, aka the display device data structure, >> provides a @@ -445,8 +449,8 @@ struct child_device_config { >> u16 dp_gpio_pin_num; /* 195 */ >> u8 dp_iboost_level:4; /* 196 */ >> u8 hdmi_iboost_level:4; /* 196 */ >> - u8 dp_max_link_rate:2; /* 216 CNL+ */ >> - u8 dp_max_link_rate_reserved:6; /* 216 */ >> + u8 dp_max_link_rate:3; /* 230 */ >> + u8 dp_max_link_rate_reserved:5; /* 230 */ >> } __packed; >> >> struct bdb_general_definitions { >> -- >> 2.17.1 > >-- >Ville Syrjälä >Intel >
On Mon, Feb 08, 2021 at 01:31:57PM +0000, Lee, Shawn C wrote: > On Fri, Feb 05, 2021, at 8:26 p.m, Ville Syrjälä wrote: > >On Mon, Feb 01, 2021 at 11:02:28PM +0800, Lee Shawn C wrote: > >> According to Bspec #20124, max link rate table for DP was updated at > >> BDB version 230. Max link rate can support upto UHBR. > >> > >> After migrate to BDB v230, the definition for LBR, HBR2 and HBR3 were > >> changed. For backward compatibility. If BDB version was from 216 to > >> 229. Driver have to follow original rule to configure DP max link rate > >> value from VBT. > >> > >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > >> Cc: Imre Deak <imre.deak@intel.com> > >> Cc: Jani Nikula <jani.nikula@linux.intel.com> > >> Cc: Cooper Chiou <cooper.chiou@intel.com> > >> Cc: William Tseng <william.tseng@intel.com> > >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_bios.c | 24 ++++++++++++++++++- > >> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 14 +++++++---- > >> 2 files changed, 32 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > >> b/drivers/gpu/drm/i915/display/intel_bios.c > >> index 04337ac6f8c4..be1f732e6550 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_bios.c > >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c > >> @@ -1876,7 +1876,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, > >> /* DP max link rate for CNL+ */ > >> if (bdb_version >= 216) { > >> switch (child->dp_max_link_rate) { > >> - default: > >> + case VBT_DP_MAX_LINK_RATE_UHBR20: > >> + info->dp_max_link_rate = 2000000; > >> + break; > >> + case VBT_DP_MAX_LINK_RATE_UHBR13P5: > >> + info->dp_max_link_rate = 1350000; > >> + break; > >> + case VBT_DP_MAX_LINK_RATE_UHBR10: > >> + info->dp_max_link_rate = 1000000; > >> + break; > >> case VBT_DP_MAX_LINK_RATE_HBR3: > >> info->dp_max_link_rate = 810000; > >> break; > >> @@ -1889,7 +1897,21 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, > >> case VBT_DP_MAX_LINK_RATE_LBR: > >> info->dp_max_link_rate = 162000; > >> break; > >> + case VBT_DP_MAX_LINK_RATE_DEFAULT: > >> + default: > >> + info->dp_max_link_rate = 0; > >> + break; > >> + } > >> + > >> + if (bdb_version < 230) { > >> + if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_DEFAULT) > >> + info->dp_max_link_rate = 810000; > >> + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_LBR) > >> + info->dp_max_link_rate = 540000; > >> + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_HBR2) > >> + info->dp_max_link_rate = 162000; > >> } > > > >I would split this into two separate functions, one does the new mapping, the other the old mapping. > > > > I will split this into two separate functions in patch v2. Actually looking through the VBT history this seems to have been retroactively changed for already rev 216+ to follow the new definitions. And naturally no actual explanation given. So it's the same old VBT==snafu as always. I guess the real question is whether any machines migth have shipped that depened on the old defitions? Unless someone manages to find that out I think we might just have to change this to follow only the new style and hope we don't regress a lot of machines. I was a bit hopeful that this might have fixed [1], but looks like that just has this set to 0 which doesn't give us the desired 2.7Gbps with either the old or new definition :( [1] https://gitlab.freedesktop.org/drm/intel/-/issues/3034
On Wed, Feb 10, 2021 at 04:51 p.m, Ville Syrjälä wrote: >On Mon, Feb 08, 2021 at 01:31:57PM +0000, Lee, Shawn C wrote: >> On Fri, Feb 05, 2021, at 8:26 p.m, Ville Syrjälä wrote: >> >On Mon, Feb 01, 2021 at 11:02:28PM +0800, Lee Shawn C wrote: >> >> According to Bspec #20124, max link rate table for DP was updated >> >> at BDB version 230. Max link rate can support upto UHBR. >> >> >> >> After migrate to BDB v230, the definition for LBR, HBR2 and HBR3 >> >> were changed. For backward compatibility. If BDB version was from >> >> 216 to 229. Driver have to follow original rule to configure DP max >> >> link rate value from VBT. >> >> >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> >> >> Cc: Imre Deak <imre.deak@intel.com> >> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> >> Cc: Cooper Chiou <cooper.chiou@intel.com> >> >> Cc: William Tseng <william.tseng@intel.com> >> >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/display/intel_bios.c | 24 ++++++++++++++++++- >> >> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 14 +++++++---- >> >> 2 files changed, 32 insertions(+), 6 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c >> >> b/drivers/gpu/drm/i915/display/intel_bios.c >> >> index 04337ac6f8c4..be1f732e6550 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c >> >> @@ -1876,7 +1876,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, >> >> /* DP max link rate for CNL+ */ >> >> if (bdb_version >= 216) { >> >> switch (child->dp_max_link_rate) { >> >> - default: >> >> + case VBT_DP_MAX_LINK_RATE_UHBR20: >> >> + info->dp_max_link_rate = 2000000; >> >> + break; >> >> + case VBT_DP_MAX_LINK_RATE_UHBR13P5: >> >> + info->dp_max_link_rate = 1350000; >> >> + break; >> >> + case VBT_DP_MAX_LINK_RATE_UHBR10: >> >> + info->dp_max_link_rate = 1000000; >> >> + break; >> >> case VBT_DP_MAX_LINK_RATE_HBR3: >> >> info->dp_max_link_rate = 810000; >> >> break; >> >> @@ -1889,7 +1897,21 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, >> >> case VBT_DP_MAX_LINK_RATE_LBR: >> >> info->dp_max_link_rate = 162000; >> >> break; >> >> + case VBT_DP_MAX_LINK_RATE_DEFAULT: >> >> + default: >> >> + info->dp_max_link_rate = 0; >> >> + break; >> >> + } >> >> + >> >> + if (bdb_version < 230) { >> >> + if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_DEFAULT) >> >> + info->dp_max_link_rate = 810000; >> >> + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_LBR) >> >> + info->dp_max_link_rate = 540000; >> >> + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_HBR2) >> >> + info->dp_max_link_rate = 162000; >> >> } >> > >> >I would split this into two separate functions, one does the new mapping, the other the old mapping. >> > >> >> I will split this into two separate functions in patch v2. > >Actually looking through the VBT history this seems to have been >retroactively changed for already rev 216+ to follow the new >definitions. And naturally no actual explanation given. So it's >the same old VBT==snafu as always. > >I guess the real question is whether any machines migth have shipped >that depened on the old defitions? Unless someone manages to >find that out I think we might just have to change this to follow >only the new style and hope we don't regress a lot of machines. > Agree that we should just have the change follow new definition. But as you mentioned, we are not sure any machines have shipped with the old definition. :( In my opinion, we should follow the new style. If we got bug report, then we can consider to add some codes for backward compatible. >I was a bit hopeful that this might have fixed [1], but looks >like that just has this set to 0 which doesn't give us the desired >2.7Gbps with either the old or new definition :( > >[1] https://gitlab.freedesktop.org/drm/intel/-/issues/3034 > Just like what you said. This change should not be able to give 2.7GHz on eDP port to help on this issue. This might not be a good idea. But how about to add this OUI into quirk list? Then driver can give bandwidth limitation for particular panel. [ 2.422930] [drm:drm_dp_read_desc [drm_kms_helper]] AUX A/DDI A/PHY A: DP sink: OUI 38-ec-11 dev-ID HW-rev 0.0 SW-rev 0.0 quirks 0x0000 Best regards, Shawn >-- >Ville Syrjälä >Intel >_______________________________________________ >Intel-gfx mailing list >Intel-gfx@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Thu, Feb 11, 2021 at 05:22:05AM +0000, Lee, Shawn C wrote: > > On Wed, Feb 10, 2021 at 04:51 p.m, Ville Syrjälä wrote: > >On Mon, Feb 08, 2021 at 01:31:57PM +0000, Lee, Shawn C wrote: > >> On Fri, Feb 05, 2021, at 8:26 p.m, Ville Syrjälä wrote: > >> >On Mon, Feb 01, 2021 at 11:02:28PM +0800, Lee Shawn C wrote: > >> >> According to Bspec #20124, max link rate table for DP was updated > >> >> at BDB version 230. Max link rate can support upto UHBR. > >> >> > >> >> After migrate to BDB v230, the definition for LBR, HBR2 and HBR3 > >> >> were changed. For backward compatibility. If BDB version was from > >> >> 216 to 229. Driver have to follow original rule to configure DP max > >> >> link rate value from VBT. > >> >> > >> >> Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > >> >> Cc: Imre Deak <imre.deak@intel.com> > >> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> > >> >> Cc: Cooper Chiou <cooper.chiou@intel.com> > >> >> Cc: William Tseng <william.tseng@intel.com> > >> >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > >> >> --- > >> >> drivers/gpu/drm/i915/display/intel_bios.c | 24 ++++++++++++++++++- > >> >> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 14 +++++++---- > >> >> 2 files changed, 32 insertions(+), 6 deletions(-) > >> >> > >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c > >> >> b/drivers/gpu/drm/i915/display/intel_bios.c > >> >> index 04337ac6f8c4..be1f732e6550 100644 > >> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c > >> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c > >> >> @@ -1876,7 +1876,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, > >> >> /* DP max link rate for CNL+ */ > >> >> if (bdb_version >= 216) { > >> >> switch (child->dp_max_link_rate) { > >> >> - default: > >> >> + case VBT_DP_MAX_LINK_RATE_UHBR20: > >> >> + info->dp_max_link_rate = 2000000; > >> >> + break; > >> >> + case VBT_DP_MAX_LINK_RATE_UHBR13P5: > >> >> + info->dp_max_link_rate = 1350000; > >> >> + break; > >> >> + case VBT_DP_MAX_LINK_RATE_UHBR10: > >> >> + info->dp_max_link_rate = 1000000; > >> >> + break; > >> >> case VBT_DP_MAX_LINK_RATE_HBR3: > >> >> info->dp_max_link_rate = 810000; > >> >> break; > >> >> @@ -1889,7 +1897,21 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, > >> >> case VBT_DP_MAX_LINK_RATE_LBR: > >> >> info->dp_max_link_rate = 162000; > >> >> break; > >> >> + case VBT_DP_MAX_LINK_RATE_DEFAULT: > >> >> + default: > >> >> + info->dp_max_link_rate = 0; > >> >> + break; > >> >> + } > >> >> + > >> >> + if (bdb_version < 230) { > >> >> + if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_DEFAULT) > >> >> + info->dp_max_link_rate = 810000; > >> >> + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_LBR) > >> >> + info->dp_max_link_rate = 540000; > >> >> + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_HBR2) > >> >> + info->dp_max_link_rate = 162000; > >> >> } > >> > > >> >I would split this into two separate functions, one does the new mapping, the other the old mapping. > >> > > >> > >> I will split this into two separate functions in patch v2. > > > >Actually looking through the VBT history this seems to have been > >retroactively changed for already rev 216+ to follow the new > >definitions. And naturally no actual explanation given. So it's > >the same old VBT==snafu as always. > > > >I guess the real question is whether any machines migth have shipped > >that depened on the old defitions? Unless someone manages to > >find that out I think we might just have to change this to follow > >only the new style and hope we don't regress a lot of machines. > > > > Agree that we should just have the change follow new definition. > But as you mentioned, we are not sure any machines have shipped > with the old definition. :( > > In my opinion, we should follow the new style. If we got bug report, > then we can consider to add some codes for backward compatible. I went trawling in some really dark waters and found out that Windows seems to do what you did originally, ie. use the old definition for 216+, and the new definition for 230+. So we should just do the same.
diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c index 04337ac6f8c4..be1f732e6550 100644 --- a/drivers/gpu/drm/i915/display/intel_bios.c +++ b/drivers/gpu/drm/i915/display/intel_bios.c @@ -1876,7 +1876,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, /* DP max link rate for CNL+ */ if (bdb_version >= 216) { switch (child->dp_max_link_rate) { - default: + case VBT_DP_MAX_LINK_RATE_UHBR20: + info->dp_max_link_rate = 2000000; + break; + case VBT_DP_MAX_LINK_RATE_UHBR13P5: + info->dp_max_link_rate = 1350000; + break; + case VBT_DP_MAX_LINK_RATE_UHBR10: + info->dp_max_link_rate = 1000000; + break; case VBT_DP_MAX_LINK_RATE_HBR3: info->dp_max_link_rate = 810000; break; @@ -1889,7 +1897,21 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, case VBT_DP_MAX_LINK_RATE_LBR: info->dp_max_link_rate = 162000; break; + case VBT_DP_MAX_LINK_RATE_DEFAULT: + default: + info->dp_max_link_rate = 0; + break; + } + + if (bdb_version < 230) { + if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_DEFAULT) + info->dp_max_link_rate = 810000; + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_LBR) + info->dp_max_link_rate = 540000; + else if (child->dp_max_link_rate == VBT_DP_MAX_LINK_RATE_HBR2) + info->dp_max_link_rate = 162000; } + drm_dbg_kms(&dev_priv->drm, "VBT DP max link rate for port %c: %d\n", port_name(port), info->dp_max_link_rate); diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h index 6d10fa037751..578a54b33f32 100644 --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h @@ -343,10 +343,14 @@ enum vbt_gmbus_ddi { #define DP_AUX_H 0x80 #define DP_AUX_I 0x90 -#define VBT_DP_MAX_LINK_RATE_HBR3 0 -#define VBT_DP_MAX_LINK_RATE_HBR2 1 +#define VBT_DP_MAX_LINK_RATE_DEFAULT 0 +#define VBT_DP_MAX_LINK_RATE_LBR 1 #define VBT_DP_MAX_LINK_RATE_HBR 2 -#define VBT_DP_MAX_LINK_RATE_LBR 3 +#define VBT_DP_MAX_LINK_RATE_HBR2 3 +#define VBT_DP_MAX_LINK_RATE_HBR3 4 +#define VBT_DP_MAX_LINK_RATE_UHBR10 5 +#define VBT_DP_MAX_LINK_RATE_UHBR13P5 6 +#define VBT_DP_MAX_LINK_RATE_UHBR20 7 /* * The child device config, aka the display device data structure, provides a @@ -445,8 +449,8 @@ struct child_device_config { u16 dp_gpio_pin_num; /* 195 */ u8 dp_iboost_level:4; /* 196 */ u8 hdmi_iboost_level:4; /* 196 */ - u8 dp_max_link_rate:2; /* 216 CNL+ */ - u8 dp_max_link_rate_reserved:6; /* 216 */ + u8 dp_max_link_rate:3; /* 230 */ + u8 dp_max_link_rate_reserved:5; /* 230 */ } __packed; struct bdb_general_definitions {
According to Bspec #20124, max link rate table for DP was updated at BDB version 230. Max link rate can support upto UHBR. After migrate to BDB v230, the definition for LBR, HBR2 and HBR3 were changed. For backward compatibility. If BDB version was from 216 to 229. Driver have to follow original rule to configure DP max link rate value from VBT. Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Imre Deak <imre.deak@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Cooper Chiou <cooper.chiou@intel.com> Cc: William Tseng <william.tseng@intel.com> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> --- drivers/gpu/drm/i915/display/intel_bios.c | 24 ++++++++++++++++++- drivers/gpu/drm/i915/display/intel_vbt_defs.h | 14 +++++++---- 2 files changed, 32 insertions(+), 6 deletions(-)