Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Oct 11, 2016 at 10:04:00PM +0200, Maarten Maathuis wrote:
> My name does not include the word "show" (Tested-by tag).

Sorry about that. Some copy-paste fail I suspect. I'll fix it up.

And you actually tested the v1 patches, so I totally forgot to note that
in the tested-by tags :( Care to re-test these v2 versions, just to make
sure I didn't seriously fumble anything?

> 
> On Tue, Oct 11, 2016 at 7:52 PM, <ville.syrjala@linux.intel.com> wrote:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The VBT provides the platform a way to mix and match the DDI ports vs.
> > GMBUS pins. Currently we only trust the VBT for DDI E, which I suppose
> > has no standard GMBUS pin assignment. However, there are machines out
> > there that use a non-standard mapping for the other ports as well.
> > Let's start trusting the VBT on this one for all ports on DDI platforms.
> >
> > I've structured the code such that other platforms could easily start
> > using this as well, by simply filling in the ddi_port_info. IIRC there
> > may be CHV system that might actually need this.
> >
> > v2: Include a commit message, include a debug message during init
> >
> > Cc: stable@vger.kernel.org
> > Cc: Maarten Maathuis <madman2003@gmail.com>
> > Tested-by: Maarten Maatt show huis <madman2003@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 84 ++++++++++++++++++++++--------
> > ---------
> >  1 file changed, 48 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 8d46f5836746..9ca86e901fc8 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1799,6 +1799,50 @@ intel_hdmi_add_properties(struct intel_hdmi
> > *intel_hdmi, struct drm_connector *c
> >         intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> >  }
> >
> > +static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> > +                            enum port port)
> > +{
> > +       const struct ddi_vbt_port_info *info =
> > +               &dev_priv->vbt.ddi_port_info[port];
> > +       u8 ddc_pin;
> > +
> > +       if (info->alternate_ddc_pin) {
> > +               DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (VBT)\n",
> > +                             info->alternate_ddc_pin, port_name(port));
> > +               return info->alternate_ddc_pin;
> > +       }
> > +
> > +       switch (port) {
> > +       case PORT_B:
> > +               if (IS_BROXTON(dev_priv))
> > +                       ddc_pin = GMBUS_PIN_1_BXT;
> > +               else
> > +                       ddc_pin = GMBUS_PIN_DPB;
> > +               break;
> > +       case PORT_C:
> > +               if (IS_BROXTON(dev_priv))
> > +                       ddc_pin = GMBUS_PIN_2_BXT;
> > +               else
> > +                       ddc_pin = GMBUS_PIN_DPC;
> > +               break;
> > +       case PORT_D:
> > +               if (IS_CHERRYVIEW(dev_priv))
> > +                       ddc_pin = GMBUS_PIN_DPD_CHV;
> > +               else
> > +                       ddc_pin = GMBUS_PIN_DPD;
> > +               break;
> > +       default:
> > +               MISSING_CASE(port);
> > +               ddc_pin = GMBUS_PIN_DPB;
> > +               break;
> > +       }
> > +
> > +       DRM_DEBUG_KMS("Using DDC pin 0x%x for port %c (platform
> > default)\n",
> > +                     ddc_pin, port_name(port));
> > +
> > +       return ddc_pin;
> > +}
> > +
> >  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >                                struct intel_connector *intel_connector)
> >  {
> > @@ -1808,7 +1852,6 @@ void intel_hdmi_init_connector(struct
> > intel_digital_port *intel_dig_port,
> >         struct drm_device *dev = intel_encoder->base.dev;
> >         struct drm_i915_private *dev_priv = to_i915(dev);
> >         enum port port = intel_dig_port->port;
> > -       uint8_t alternate_ddc_pin;
> >
> >         DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
> >                       port_name(port));
> > @@ -1826,12 +1869,10 @@ void intel_hdmi_init_connector(struct
> > intel_digital_port *intel_dig_port,
> >         connector->doublescan_allowed = 0;
> >         connector->stereo_allowed = 1;
> >
> > +       intel_hdmi->ddc_bus = intel_hdmi_ddc_pin(dev_priv, port);
> > +
> >         switch (port) {
> >         case PORT_B:
> > -               if (IS_BROXTON(dev_priv))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> > -               else
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> >                 /*
> >                  * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> >                  * interrupts to check the external panel connection.
> > @@ -1842,46 +1883,17 @@ void intel_hdmi_init_connector(struct
> > intel_digital_port *intel_dig_port,
> >                         intel_encoder->hpd_pin = HPD_PORT_B;
> >                 break;
> >         case PORT_C:
> > -               if (IS_BROXTON(dev_priv))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> > -               else
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> >                 intel_encoder->hpd_pin = HPD_PORT_C;
> >                 break;
> >         case PORT_D:
> > -               if (WARN_ON(IS_BROXTON(dev_priv)))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> > -               else if (IS_CHERRYVIEW(dev_priv))
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> > -               else
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> >                 intel_encoder->hpd_pin = HPD_PORT_D;
> >                 break;
> >         case PORT_E:
> > -               /* On SKL PORT E doesn't have seperate GMBUS pin
> > -                *  We rely on VBT to set a proper alternate GMBUS pin. */
> > -               alternate_ddc_pin =
> > -                       dev_priv->vbt.ddi_port_info[
> > PORT_E].alternate_ddc_pin;
> > -               switch (alternate_ddc_pin) {
> > -               case DDC_PIN_B:
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> > -                       break;
> > -               case DDC_PIN_C:
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> > -                       break;
> > -               case DDC_PIN_D:
> > -                       intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> > -                       break;
> > -               default:
> > -                       MISSING_CASE(alternate_ddc_pin);
> > -               }
> >                 intel_encoder->hpd_pin = HPD_PORT_E;
> >                 break;
> > -       case PORT_A:
> > -               intel_encoder->hpd_pin = HPD_PORT_A;
> > -               /* Internal port only for eDP. */
> >         default:
> > -               BUG();
> > +               MISSING_CASE(port);
> > +               return;
> >         }
> >
> >         if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
> > --
> > 2.7.4
> >
> >
> 
> 
> -- 
> Far away from the primal instinct, the song seems to fade away, the river
> get wider between your thoughts and the things we do and say.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx