On Thu, 19 Oct 2017, Rodrigo Vivi <rodrigo.v...@intel.com> 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.
>
> Cc: Jani Nikula <jani.nik...@intel.com>
> Cc: Paulo Zanoni <paulo.r.zan...@intel.com>
> Cc: Anusha Srivatsa <anusha.sriva...@intel.com>
> Cc: Clinton Taylor <clinton.a.tay...@intel.com>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.v...@intel.com>
> ---
> drivers/gpu/drm/i915/intel_bios.c | 26 ++++++++++++++++----------
> drivers/gpu/drm/i915/intel_vbt_defs.h | 15 +++++++++++++++
> 2 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index dc4fad30bf4f..79638308664c 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1071,6 +1071,21 @@ static void sanitize_aux_ch(struct drm_i915_private
> *dev_priv,
> }
> }
>
> +static const enum bspec_gmbus_pin cnp_ddc_pin_map[] = {
> + [DDC_BUS_DDI_B] = GMBUS_PIN_1,
> + [DDC_BUS_DDI_C] = GMBUS_PIN_2,
> + [DDC_BUS_DDI_D] = GMBUS_PIN_4,
> + [DDC_BUS_DDI_F] = GMBUS_PIN_3,
I think the two last lines deserve comments like /* sic */ or something.
> +};
> +
> +static u8 get_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin)
Nitpick, You're passing in a pin and you get a pin? How about
"map_ddc_pin"? Like your array name suggests?
> +{
> + if (HAS_PCH_CNP(dev_priv))
> + 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)
> {
> @@ -1164,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 = get_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 7e2a96712d07..dfc9f7a43668 100644
> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> @@ -308,6 +308,21 @@ 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,
> +};
> +
> +enum bspec_gmbus_pin {
> + GMBUS_PIN_1 = 0x1,
> + GMBUS_PIN_2,
> + GMBUS_PIN_3,
> + GMBUS_PIN_4,
You already have these defined in i915_reg.h. The naming isn't pretty,
but the above doesn't make much sense either.
> +};
> +
> /*
> * The child device config, aka the display device data structure, provides a
> * description of a port and its configuration on the platform.
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
