[Intel-gfx] [PATCH v2] drm/i915: Per-DDI I_boost override

Ville Syrjälä ville.syrjala at linux.intel.com
Fri Jul 3 08:21:58 PDT 2015


On Fri, Jul 03, 2015 at 12:09:43PM -0300, Paulo Zanoni wrote:
> 2015-07-03 8:28 GMT-03:00 Antti Koskipaa <antti.koskipaa at linux.intel.com>:
> > An OEM may request increased I_boost beyond the recommended values
> > by specifying an I_boost value to be applied to all swing entries for
> > a port. These override values are specified in VBT.
> >
> > v2: rebase and remove unused iboost_bit variable
> >
> > Issue: VIZ-5676
> > Signed-off-by: Antti Koskipaa <antti.koskipaa at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h   |  3 +++
> >  drivers/gpu/drm/i915/intel_bios.c | 21 +++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_bios.h |  9 +++++++++
> >  drivers/gpu/drm/i915/intel_ddi.c  | 38 ++++++++++++++++++++++++++++++--------
> >  4 files changed, 63 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 1dbd957..6aa8083 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1410,6 +1410,9 @@ struct ddi_vbt_port_info {
> >         uint8_t supports_dvi:1;
> >         uint8_t supports_hdmi:1;
> >         uint8_t supports_dp:1;
> > +
> > +       uint8_t dp_boost_level;
> > +       uint8_t hdmi_boost_level;
> >  };
> >
> >  enum psr_lines_to_wait {
> > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> > index 2ff9eb0..76e12f5 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/intel_bios.c
> > @@ -886,6 +886,17 @@ err:
> >         memset(dev_priv->vbt.dsi.sequence, 0, sizeof(dev_priv->vbt.dsi.sequence));
> >  }
> >
> > +static u8 translate_iboost(u8 val)
> > +{
> > +       static const u8 mapping[] = { 1, 3, 7 }; /* See VBT spec */
> > +
> > +       if (val >= ARRAY_SIZE(mapping)) {
> > +               DRM_DEBUG_KMS("Unsupported I_boost value found in VBT (%d), display may not work properly\n", val);
> > +               return 0;
> > +       }
> > +       return mapping[val];
> > +}
> > +
> >  static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> >                            const struct bdb_header *bdb)
> >  {
> > @@ -986,6 +997,16 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> >                               hdmi_level_shift);
> >                 info->hdmi_level_shift = hdmi_level_shift;
> >         }
> > +
> > +       /* Parse the I_boost config for SKL and above */
> > +       if (bdb->version >= 196 && (child->common.flags_1 & IBOOST_ENABLE)) {
> > +               info->dp_boost_level = translate_iboost(child->common.iboost_level & 0xF);
> > +               DRM_DEBUG_KMS("VBT (e)DP boost level for port %c: %d\n",
> > +                             port_name(port), info->dp_boost_level);
> > +               info->hdmi_boost_level = translate_iboost(child->common.iboost_level >> 4);
> > +               DRM_DEBUG_KMS("VBT HDMI boost level for port %c: %d\n",
> > +                             port_name(port), info->hdmi_boost_level);
> > +       }
> >  }
> >
> >  static void parse_ddi_ports(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> > index af0b476..8edd75c 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.h
> > +++ b/drivers/gpu/drm/i915/intel_bios.h
> > @@ -231,6 +231,10 @@ struct old_child_dev_config {
> >  /* This one contains field offsets that are known to be common for all BDB
> >   * versions. Notice that the meaning of the contents contents may still change,
> >   * but at least the offsets are consistent. */
> > +
> > +/* Definitions for flags_1 */
> > +#define IBOOST_ENABLE (1<<3)
> > +
> >  struct common_child_dev_config {
> >         u16 handle;
> >         u16 device_type;
> > @@ -239,8 +243,13 @@ struct common_child_dev_config {
> >         u8 not_common2[2];
> >         u8 ddc_pin;
> >         u16 edid_ptr;
> > +       u8 obsolete;
> > +       u8 flags_1;
> > +       u8 not_common3[13];
> > +       u8 iboost_level;
> 
> In the old VBT spec I have, each child_dev_config is supposed to have
> only 33 bytes. But in this patch you're increasing it to 38. I believe
> this is what's causing the errors I see when I boot my BDW.
> 
> Are you sure they increased the VBT's ChildDevInfo to more than 33
> bytes? I don't have access to your VBT spec right now, so I can't do a
> proper review or a suggestion on how to fix the problem.

The size depends on the version. On BSW last I looked it was maybe 36 or
37 bytes.

I'm not sure we want to change common_child_dev_config since it's meant
to be some kind of common stuff. So maybe add another member into the
union for this stuff. Of course that still incrases the size of the union
so other changes are required to make it work.

Fortunately we copy these things out from the VBT, so I think just
making sure the memcpy() doesn't try to copy too much and fixing the size
check should be all that's needed. That would leave any extra we didn't
fill with the copy zeroed, and if having it zeroed isn't good enough
we can of course sprinkle more version checks around.

And I guess we should still check that the size is at least 33 bytes
since that was the official size for the longest time.

> 
> >  } __packed;
> >
> > +
> >  /* This field changes depending on the BDB version, so the most reliable way to
> >   * read it is by checking the BDB version and reading the raw pointer. */
> >  union child_device_config {
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 15fc66a..c83f15f 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -440,6 +440,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port,
> >  {
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         u32 reg;
> > +       u32 iboost_bit = 0;
> >         int i, n_hdmi_entries, n_dp_entries, n_edp_entries, hdmi_default_entry,
> >             size;
> >         int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
> > @@ -465,6 +466,10 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port,
> >                 ddi_translations_hdmi =
> >                                 skl_get_buf_trans_hdmi(dev, &n_hdmi_entries);
> >                 hdmi_default_entry = 8;
> > +               /* If we're boosting the current, set bit 31 of trans1 */
> > +               if (dev_priv->vbt.ddi_port_info[port].hdmi_boost_level ||
> > +                   dev_priv->vbt.ddi_port_info[port].dp_boost_level)
> > +                       iboost_bit = 1<<31;
> >         } else if (IS_BROADWELL(dev)) {
> >                 ddi_translations_fdi = bdw_ddi_translations_fdi;
> >                 ddi_translations_dp = bdw_ddi_translations_dp;
> > @@ -525,7 +530,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port,
> >         }
> >
> >         for (i = 0, reg = DDI_BUF_TRANS(port); i < size; i++) {
> > -               I915_WRITE(reg, ddi_translations[i].trans1);
> > +               I915_WRITE(reg, ddi_translations[i].trans1 | iboost_bit);
> >                 reg += 4;
> >                 I915_WRITE(reg, ddi_translations[i].trans2);
> >                 reg += 4;
> > @@ -540,7 +545,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port,
> >                 hdmi_level = hdmi_default_entry;
> >
> >         /* Entry 9 is for HDMI: */
> > -       I915_WRITE(reg, ddi_translations_hdmi[hdmi_level].trans1);
> > +       I915_WRITE(reg, ddi_translations_hdmi[hdmi_level].trans1 | iboost_bit);
> >         reg += 4;
> >         I915_WRITE(reg, ddi_translations_hdmi[hdmi_level].trans2);
> >         reg += 4;
> > @@ -2083,18 +2088,35 @@ static void skl_ddi_set_iboost(struct drm_device *dev, u32 level,
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         const struct ddi_buf_trans *ddi_translations;
> >         uint8_t iboost;
> > +       uint8_t dp_iboost, hdmi_iboost;
> >         int n_entries;
> >         u32 reg;
> >
> > +       /* VBT may override standard boost values */
> > +       dp_iboost = dev_priv->vbt.ddi_port_info[port].dp_boost_level;
> > +       hdmi_iboost = dev_priv->vbt.ddi_port_info[port].hdmi_boost_level;
> > +
> >         if (type == INTEL_OUTPUT_DISPLAYPORT) {
> > -               ddi_translations = skl_get_buf_trans_dp(dev, &n_entries);
> > -               iboost = ddi_translations[port].i_boost;
> > +               if (dp_iboost) {
> > +                       iboost = dp_iboost;
> > +               } else {
> > +                       ddi_translations = skl_get_buf_trans_dp(dev, &n_entries);
> > +                       iboost = ddi_translations[port].i_boost;
> > +               }
> >         } else if (type == INTEL_OUTPUT_EDP) {
> > -               ddi_translations = skl_get_buf_trans_edp(dev, &n_entries);
> > -               iboost = ddi_translations[port].i_boost;
> > +               if (dp_iboost) {
> > +                       iboost = dp_iboost;
> > +               } else {
> > +                       ddi_translations = skl_get_buf_trans_edp(dev, &n_entries);
> > +                       iboost = ddi_translations[port].i_boost;
> > +               }
> >         } else if (type == INTEL_OUTPUT_HDMI) {
> > -               ddi_translations = skl_get_buf_trans_hdmi(dev, &n_entries);
> > -               iboost = ddi_translations[port].i_boost;
> > +               if (hdmi_iboost) {
> > +                       iboost = hdmi_iboost;
> > +               } else {
> > +                       ddi_translations = skl_get_buf_trans_hdmi(dev, &n_entries);
> > +                       iboost = ddi_translations[port].i_boost;
> > +               }
> >         } else {
> >                 return;
> >         }
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC


More information about the Intel-gfx mailing list