| Message ID | 20171020172641.16029-1-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 20, 2017 at 10:26:41AM -0700, Rodrigo Vivi wrote: > Starting on CNL we now need to map VBT DDC Pin to > BSPec DDC Pin values. Not a direct translation anymore. > > According to VBT > Block 2 (General Bytes Definition) > DDC Bus > > +----------+-----------+--------------------+ > | DDI Type | VBT Value | Bspec Mapped Value | > +----------+-----------+--------------------+ > | DDI-B | 0x1 | 0x1 | > | DDI-C | 0x2 | 0x2 | > | DDI-D | 0x3 | 0x4 | > | DDI-F | 0x4 | 0x3 | > +----------+-----------+--------------------+ > > v2: Move defines to a better place. > This is actually CNL_PCH not CNL only. > v3: Accepting Ville's suggestions: enums and array to > to make this future proof. > v4: Protect the array access as Ville suggested. > Also accepting all Jani's suggestions: > - use already defined gmbus pin definitions. > - use map_ddc_pin for disambiguation. > - Add /* sic */ comment on inverted values > so people can easily see it it nos a mistake > we have the map 3 -> 4 and 4 -> 3 :/ > > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > Cc: Clinton Taylor <clinton.a.taylor@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/intel_bios.c | 27 +++++++++++++++++---------- > drivers/gpu/drm/i915/intel_vbt_defs.h | 8 ++++++++ > 2 files changed, 25 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index e809a9c347d3..a95cf07bd8e1 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -1071,6 +1071,22 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > } > } > > +static const u8 cnp_ddc_pin_map[] = { > + [DDC_BUS_DDI_B] = GMBUS_PIN_1_BXT, > + [DDC_BUS_DDI_C] = GMBUS_PIN_2_BXT, > + [DDC_BUS_DDI_D] = GMBUS_PIN_4_CNP, /* sic */ > + [DDC_BUS_DDI_F] = GMBUS_PIN_3_BXT, /* sic */ > +}; > + > +static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin) > +{ > + if (HAS_PCH_CNP(dev_priv) && > + vbt_pin > 0 && vbt_pin < ARRAY_SIZE(cnp_ddc_pin_map)) > + return cnp_ddc_pin_map[vbt_pin]; Not quite sure what we should do with invalid pins. But I guess this makes as much sense as anything. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > + > + return vbt_pin; > +} > + > static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > u8 bdb_version) > { > @@ -1163,16 +1179,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port)); > > if (is_dvi) { > - info->alternate_ddc_pin = ddc_pin; > - > - /* > - * All VBTs that we got so far for B Stepping has this > - * information wrong for Port D. So, let's just ignore for now. > - */ > - if (IS_CNL_REVID(dev_priv, CNL_REVID_B0, CNL_REVID_B0) && > - port == PORT_D) { > - info->alternate_ddc_pin = 0; > - } > + info->alternate_ddc_pin = map_ddc_pin(dev_priv, ddc_pin); > > sanitize_ddc_pin(dev_priv, port); > } > diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h > index 404569c9fdfc..f225c288a121 100644 > --- a/drivers/gpu/drm/i915/intel_vbt_defs.h > +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h > @@ -306,6 +306,14 @@ struct bdb_general_features { > > #define LEGACY_CHILD_DEVICE_CONFIG_SIZE 33 > > +/* DDC Bus DDI Type 155+ */ > +enum vbt_gmbus_ddi { > + DDC_BUS_DDI_B = 0x1, > + DDC_BUS_DDI_C, > + DDC_BUS_DDI_D, > + DDC_BUS_DDI_F, > +}; > + > /* > * The child device config, aka the display device data structure, provides a > * description of a port and its configuration on the platform. > -- > 2.13.5
On Fri, Oct 20, 2017 at 05:40:10PM +0000, Ville Syrjälä wrote: > On Fri, Oct 20, 2017 at 10:26:41AM -0700, Rodrigo Vivi wrote: > > Starting on CNL we now need to map VBT DDC Pin to > > BSPec DDC Pin values. Not a direct translation anymore. > > > > According to VBT > > Block 2 (General Bytes Definition) > > DDC Bus > > > > +----------+-----------+--------------------+ > > | DDI Type | VBT Value | Bspec Mapped Value | > > +----------+-----------+--------------------+ > > | DDI-B | 0x1 | 0x1 | > > | DDI-C | 0x2 | 0x2 | > > | DDI-D | 0x3 | 0x4 | > > | DDI-F | 0x4 | 0x3 | > > +----------+-----------+--------------------+ > > > > v2: Move defines to a better place. > > This is actually CNL_PCH not CNL only. > > v3: Accepting Ville's suggestions: enums and array to > > to make this future proof. > > v4: Protect the array access as Ville suggested. > > Also accepting all Jani's suggestions: > > - use already defined gmbus pin definitions. > > - use map_ddc_pin for disambiguation. > > - Add /* sic */ comment on inverted values > > so people can easily see it it nos a mistake > > we have the map 3 -> 4 and 4 -> 3 :/ > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> > > Cc: Clinton Taylor <clinton.a.taylor@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > drivers/gpu/drm/i915/intel_bios.c | 27 +++++++++++++++++---------- > > drivers/gpu/drm/i915/intel_vbt_defs.h | 8 ++++++++ > > 2 files changed, 25 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > > index e809a9c347d3..a95cf07bd8e1 100644 > > --- a/drivers/gpu/drm/i915/intel_bios.c > > +++ b/drivers/gpu/drm/i915/intel_bios.c > > @@ -1071,6 +1071,22 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > > } > > } > > > > +static const u8 cnp_ddc_pin_map[] = { > > + [DDC_BUS_DDI_B] = GMBUS_PIN_1_BXT, > > + [DDC_BUS_DDI_C] = GMBUS_PIN_2_BXT, > > + [DDC_BUS_DDI_D] = GMBUS_PIN_4_CNP, /* sic */ > > + [DDC_BUS_DDI_F] = GMBUS_PIN_3_BXT, /* sic */ > > +}; > > + > > +static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin) > > +{ > > + if (HAS_PCH_CNP(dev_priv) && > > + vbt_pin > 0 && vbt_pin < ARRAY_SIZE(cnp_ddc_pin_map)) > > + return cnp_ddc_pin_map[vbt_pin]; > > Not quite sure what we should do with invalid pins. But I guess this > makes as much sense as anything. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Thanks a lot for all ideas. merged to dinq. > > > + > > + return vbt_pin; > > +} > > + > > static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > > u8 bdb_version) > > { > > @@ -1163,16 +1179,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > > DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port)); > > > > if (is_dvi) { > > - info->alternate_ddc_pin = ddc_pin; > > - > > - /* > > - * All VBTs that we got so far for B Stepping has this > > - * information wrong for Port D. So, let's just ignore for now. > > - */ > > - if (IS_CNL_REVID(dev_priv, CNL_REVID_B0, CNL_REVID_B0) && > > - port == PORT_D) { > > - info->alternate_ddc_pin = 0; > > - } > > + info->alternate_ddc_pin = map_ddc_pin(dev_priv, ddc_pin); > > > > sanitize_ddc_pin(dev_priv, port); > > } > > diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h > > index 404569c9fdfc..f225c288a121 100644 > > --- a/drivers/gpu/drm/i915/intel_vbt_defs.h > > +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h > > @@ -306,6 +306,14 @@ struct bdb_general_features { > > > > #define LEGACY_CHILD_DEVICE_CONFIG_SIZE 33 > > > > +/* DDC Bus DDI Type 155+ */ > > +enum vbt_gmbus_ddi { > > + DDC_BUS_DDI_B = 0x1, > > + DDC_BUS_DDI_C, > > + DDC_BUS_DDI_D, > > + DDC_BUS_DDI_F, > > +}; > > + > > /* > > * The child device config, aka the display device data structure, provides a > > * description of a port and its configuration on the platform. > > -- > > 2.13.5 > > -- > Ville Syrjälä > Intel OTC
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index e809a9c347d3..a95cf07bd8e1 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1071,6 +1071,22 @@ static void sanitize_aux_ch(struct drm_i915_private *dev_priv, } } +static const u8 cnp_ddc_pin_map[] = { + [DDC_BUS_DDI_B] = GMBUS_PIN_1_BXT, + [DDC_BUS_DDI_C] = GMBUS_PIN_2_BXT, + [DDC_BUS_DDI_D] = GMBUS_PIN_4_CNP, /* sic */ + [DDC_BUS_DDI_F] = GMBUS_PIN_3_BXT, /* sic */ +}; + +static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin) +{ + if (HAS_PCH_CNP(dev_priv) && + vbt_pin > 0 && vbt_pin < ARRAY_SIZE(cnp_ddc_pin_map)) + return cnp_ddc_pin_map[vbt_pin]; + + return vbt_pin; +} + static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, u8 bdb_version) { @@ -1163,16 +1179,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port)); if (is_dvi) { - info->alternate_ddc_pin = ddc_pin; - - /* - * All VBTs that we got so far for B Stepping has this - * information wrong for Port D. So, let's just ignore for now. - */ - if (IS_CNL_REVID(dev_priv, CNL_REVID_B0, CNL_REVID_B0) && - port == PORT_D) { - info->alternate_ddc_pin = 0; - } + info->alternate_ddc_pin = map_ddc_pin(dev_priv, ddc_pin); sanitize_ddc_pin(dev_priv, port); } diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h index 404569c9fdfc..f225c288a121 100644 --- a/drivers/gpu/drm/i915/intel_vbt_defs.h +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h @@ -306,6 +306,14 @@ struct bdb_general_features { #define LEGACY_CHILD_DEVICE_CONFIG_SIZE 33 +/* DDC Bus DDI Type 155+ */ +enum vbt_gmbus_ddi { + DDC_BUS_DDI_B = 0x1, + DDC_BUS_DDI_C, + DDC_BUS_DDI_D, + DDC_BUS_DDI_F, +}; + /* * The child device config, aka the display device data structure, provides a * description of a port and its configuration on the platform.
Starting on CNL we now need to map VBT DDC Pin to BSPec DDC Pin values. Not a direct translation anymore. According to VBT Block 2 (General Bytes Definition) DDC Bus +----------+-----------+--------------------+ | DDI Type | VBT Value | Bspec Mapped Value | +----------+-----------+--------------------+ | DDI-B | 0x1 | 0x1 | | DDI-C | 0x2 | 0x2 | | DDI-D | 0x3 | 0x4 | | DDI-F | 0x4 | 0x3 | +----------+-----------+--------------------+ v2: Move defines to a better place. This is actually CNL_PCH not CNL only. v3: Accepting Ville's suggestions: enums and array to to make this future proof. v4: Protect the array access as Ville suggested. Also accepting all Jani's suggestions: - use already defined gmbus pin definitions. - use map_ddc_pin for disambiguation. - Add /* sic */ comment on inverted values so people can easily see it it nos a mistake we have the map 3 -> 4 and 4 -> 3 :/ Cc: Jani Nikula <jani.nikula@intel.com> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com> Cc: Clinton Taylor <clinton.a.taylor@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_bios.c | 27 +++++++++++++++++---------- drivers/gpu/drm/i915/intel_vbt_defs.h | 8 ++++++++ 2 files changed, 25 insertions(+), 10 deletions(-)