Message ID | 1476208368-5710-3-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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 > >
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.
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 >
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
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
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 --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)) {