On Tue, Oct 11, 2016 at 08:52:46PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrj...@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: sta...@vger.kernel.org
> Cc: Maarten Maathuis <madman2...@gmail.com>
> Tested-by: Maarten Maatt show huis <madman2...@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97877
> Signed-off-by: Ville Syrjälä <ville.syrj...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to