On Fri, 20 Oct 2017, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Fri, Oct 20, 2017 at 12:11:29PM +0300, Jani Nikula wrote:
>> On Thu, 19 Oct 2017, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> 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.nikula@xxxxxxxxx>
>> > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>> > Cc: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
>> > Cc: Clinton Taylor <clinton.a.taylor@xxxxxxxxx>
>> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
>> > ---
>> > 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];
>
> Do we need an ARRAY_SIZE() check? I didn't actually check how many bits
> we have in the VBT for this.
Oh yes, it's u8.
BR,
Jani.
>
>> > +
>> > + 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.
>
> +1
>
>>
>> > +};
>> > +
>> > /*
>> > * 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
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
