Re: [PATCH 2/2] drm/i915: use the HDMI DDI buffer translations from VBT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2013/9/3 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>:
> On Wed, Aug 28, 2013 at 12:39 PM, Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>>
>> We currently use the recommended values from BSpec, but the VBT
>> specifies the correct value to use for the hardware we have, so use
>> it. We also fall back to the recommended value in case we can't find
>> the VBT.
>>
>> In addition, this code also provides some infrastructure to parse more
>> information about the DDI ports. There's a lot more information we
>> could extract and use in the future.
>>
>> v2: - Move some code to init_vbt_defaults.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h   |  6 ++++
>>  drivers/gpu/drm/i915/intel_bios.c | 69 +++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_ddi.c  | 24 ++++++++++++--
>>  3 files changed, 97 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 08fe1b6..645dd74 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1039,6 +1039,10 @@ enum modeset_restore {
>>         MODESET_SUSPENDED,
>>  };
>>
>> +struct ddi_vbt_port_info {
>> +       uint8_t hdmi_level_shift;
>> +};
>> +
>>  struct intel_vbt_data {
>>         struct drm_display_mode *lfp_lvds_vbt_mode; /* if any */
>>         struct drm_display_mode *sdvo_lvds_vbt_mode; /* if any */
>> @@ -1068,6 +1072,8 @@ struct intel_vbt_data {
>>
>>         int child_dev_num;
>>         union child_device_config *child_dev;
>> +
>> +       struct ddi_vbt_port_info ddi_port_info[5];
>>  };
>>
>>  enum intel_ddb_partitioning {
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 8a27925..79bb651 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -568,6 +568,63 @@ parse_edp(struct drm_i915_private *dev_priv, struct bdb_header *bdb)
>>         }
>>  }
>>
>> +static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>> +                          struct bdb_header *bdb)
>> +{
>> +       union child_device_config *it, *child = NULL;
>> +       struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
>> +       uint8_t hdmi_level_shift;
>> +       int i, j;
>> +       int dvo_ports[][2] = {
>> +               {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0},
>> +       };
>> +       int n_dvo_ports[] = {2, 2, 2, 2, 1};
>
> I hadn't get this until you explain on irc, but I have no better idea
> how to make it more clear, so just ignore me ;)

I'll follow Daniel's suggestion and also add a comment. I agree this
is not beautiful code :(

>
>> +
>> +       /* Find the child device to use, abort if more than one found. */
>> +       for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
>> +               it = dev_priv->vbt.child_dev + i;
>> +
>> +               for (j = 0; j < n_dvo_ports[port]; j++) {
>> +                       if (it->common.dvo_port == dvo_ports[port][j]) {
>> +                               if (child) {
>> +                                       DRM_ERROR("More than one child device for port %c in VBT.\n",
>> +                                                  port_name(port));
>> +                                       return;
>> +                               }
>> +                               child = it;
>> +                       }
>> +               }
>> +       }
>> +       if (!child)
>> +               return;
>> +
>> +       if (bdb->version >= 158) {
>
> 0x8 is available since 157... is it useful to include it?

If the BDB version is 157 we'll use the BSpec-recommended value, which
is exactly 0x8.


>
>> +               /* The VBT HDMI level shift values match the table we have. */
>> +               hdmi_level_shift = child->raw[7] & 0xF;
>> +               if (hdmi_level_shift < 0xC) {
>> +                       DRM_DEBUG_KMS("VBT HDMI level shift for port %c: %d\n",
>> +                                     port_name(port),
>> +                                     hdmi_level_shift);
>> +                       info->hdmi_level_shift = hdmi_level_shift;
>> +               }
>> +       }
>> +}
>> +
>> +static void parse_ddi_ports(struct drm_i915_private *dev_priv,
>> +                           struct bdb_header *bdb)
>> +{
>> +       enum port port;
>> +
>> +       if (!dev_priv->vbt.child_dev_num)
>> +               return;
>> +
>> +       if (bdb->version < 155)
>> +               return;
>
> Some time in the past I had thought about a similar check, but the
> version itself was added after 155...
> So not sure how effective is this...

IMHO if we ever get a list of which fields existed before version 155
we may want to revisit this function. For now I prefer to not parse
the older versions since we don't really know what to expect from
them.

Thanks for the reviews!

>
>> +
>> +       for (port = PORT_A; port <= PORT_E; port++)
>> +               parse_ddi_port(dev_priv, port, bdb);
>> +}
>> +
>>  static void
>>  parse_device_mapping(struct drm_i915_private *dev_priv,
>>                        struct bdb_header *bdb)
>> @@ -655,6 +712,16 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
>>         dev_priv->vbt.lvds_use_ssc = 1;
>>         dev_priv->vbt.lvds_ssc_freq = intel_bios_ssc_frequency(dev, 1);
>>         DRM_DEBUG_KMS("Set default to SSC at %dMHz\n", dev_priv->vbt.lvds_ssc_freq);
>> +
>> +       if (HAS_DDI(dev)) {
>> +               enum port port;
>> +
>> +               for (port = PORT_A; port <= PORT_E; port++) {
>> +                       /* Recommended BSpec default: 800mV 0dB. */
>> +                       dev_priv->vbt.ddi_port_info[port].hdmi_level_shift = 6;
>> +               }
>> +       }
>> +
>>  }
>>
>>  static int __init intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
>> @@ -745,6 +812,8 @@ intel_parse_bios(struct drm_device *dev)
>>         parse_device_mapping(dev_priv, bdb);
>>         parse_driver_features(dev_priv, bdb);
>>         parse_edp(dev_priv, bdb);
>> +       if (HAS_DDI(dev))
>> +               parse_ddi_ports(dev_priv, bdb);
>>
>>         if (bios)
>>                 pci_unmap_rom(pdev, bios);
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index 63aca49..ece226d 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -42,7 +42,6 @@ static const u32 hsw_ddi_translations_dp[] = {
>>         0x80C30FFF, 0x000B0000,
>>         0x00FFFFFF, 0x00040006,
>>         0x80D75FFF, 0x000B0000,
>> -       0x00FFFFFF, 0x00040006          /* HDMI parameters */
>>  };
>>
>>  static const u32 hsw_ddi_translations_fdi[] = {
>> @@ -55,7 +54,22 @@ static const u32 hsw_ddi_translations_fdi[] = {
>>         0x00C30FFF, 0x001E0000,
>>         0x00FFFFFF, 0x00060006,
>>         0x00D75FFF, 0x001E0000,
>> -       0x00FFFFFF, 0x00040006          /* HDMI parameters */
>> +};
>> +
>> +static const u32 hsw_ddi_translations_hdmi[] = {
>> +                               /* Idx  NT mV diff      T mV diff       db  */
>> +       0x00FFFFFF, 0x0006000E, /* 0:   400             400             0   */
>> +       0x00E79FFF, 0x000E000C, /* 1:   400             500             2   */
>> +       0x00D75FFF, 0x0005000A, /* 2:   400             600             3.5 */
>> +       0x00FFFFFF, 0x0005000A, /* 3:   600             600             0   */
>> +       0x00E79FFF, 0x001D0007, /* 4:   600             750             2   */
>> +       0x00D75FFF, 0x000C0004, /* 5:   600             900             3.5 */
>> +       0x00FFFFFF, 0x00040006, /* 6:   800             800             0   */
>> +       0x80E79FFF, 0x00030002, /* 7:   800             1000            2   */
>> +       0x00FFFFFF, 0x00140005, /* 8:   850             850             0   */
>> +       0x00FFFFFF, 0x000C0004, /* 9:   900             900             0   */
>> +       0x00FFFFFF, 0x001C0003, /* 10:  950             950             0   */
>> +       0x80FFFFFF, 0x00030002, /* 11:  1000            1000            0   */
>>  };
>>
>>  static enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
>> @@ -92,12 +106,18 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port)
>>         const u32 *ddi_translations = (port == PORT_E) ?
>>                 hsw_ddi_translations_fdi :
>>                 hsw_ddi_translations_dp;
>> +       int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
>>
>>         for (i = 0, reg = DDI_BUF_TRANS(port);
>>              i < ARRAY_SIZE(hsw_ddi_translations_fdi); i++) {
>>                 I915_WRITE(reg, ddi_translations[i]);
>>                 reg += 4;
>>         }
>> +       /* Entry 9 is for HDMI: */
>> +       for (i = 0; i < 2; i++) {
>> +               I915_WRITE(reg, hsw_ddi_translations_hdmi[hdmi_level * 2 + i]);
>> +               reg += 4;
>> +       }
>>  }
>>
>>  /* Program DDI buffers translations for DP. By default, program ports A-D in DP
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br



-- 
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux