diff mbox

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

Message ID 1476208368-5710-3-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjala Oct. 11, 2016, 5:52 p.m. UTC
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(-)

Comments

Maarten Maathuis Oct. 11, 2016, 8:04 p.m. UTC | #1
My name does not include the word "show" (Tested-by tag).

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
>
>
Ville Syrjala Oct. 12, 2016, 10:57 a.m. UTC | #2
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.
Maarten Maathuis Oct. 12, 2016, 4:56 p.m. UTC | #3
Retested the _2 branch, works fine as well.

On Wed, Oct 12, 2016 at 12:57 PM, Ville Syrjälä <
ville.syrjala@linux.intel.com> wrote:

> 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
>
jim.bride@linux.intel.com Oct. 13, 2016, 6:06 p.m. UTC | #4
On Tue, Oct 11, 2016 at 08:52:46PM +0300, 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;

In the code removed below there's a specific case covering Broxton that
isn't accounted for here.  Are we sure that we don't need that logic here?

Jim

> +		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
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjala Oct. 13, 2016, 6:29 p.m. UTC | #5
On Thu, Oct 13, 2016 at 11:06:55AM -0700, Jim Bride wrote:
> On Tue, Oct 11, 2016 at 08:52:46PM +0300, 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;
> 
> In the code removed below there's a specific case covering Broxton that
> isn't accounted for here.  Are we sure that we don't need that logic here?

Yeah, BXT doesn't have port D.

I guess we could split this up into a few platform specific functions that
could then have the MISSING_CASE() for the impossible ports. A bit like
the AUX ctl/data register setup functions we have. Do pople want that
level of paranoia here?

> 
> Jim
> 
> > +		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
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter Oct. 17, 2016, 6:50 a.m. UTC | #6
On Thu, Oct 13, 2016 at 09:29:30PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 13, 2016 at 11:06:55AM -0700, Jim Bride wrote:
> > On Tue, Oct 11, 2016 at 08:52:46PM +0300, 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;
> > 
> > In the code removed below there's a specific case covering Broxton that
> > isn't accounted for here.  Are we sure that we don't need that logic here?
> 
> Yeah, BXT doesn't have port D.
> 
> I guess we could split this up into a few platform specific functions that
> could then have the MISSING_CASE() for the impossible ports. A bit like
> the AUX ctl/data register setup functions we have. Do pople want that
> level of paranoia here?

tbh seems overkill, we should go boom in the platform gmbus code if you
end up with an impossible port.
-Daniel

> 
> > 
> > Jim
> > 
> > > +		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
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

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)) {