* [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports [not found] <1476208368-5710-1-git-send-email-ville.syrjala@linux.intel.com> @ 2016-10-11 17:52 ` ville.syrjala 2016-10-13 17:59 ` [Intel-gfx] " Jim Bride 2016-10-11 17:52 ` [PATCH 2/4] drm/i915: Respect alternate_ddc_pin " ville.syrjala 2016-10-11 17:52 ` [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation ville.syrjala 2 siblings, 1 reply; 11+ messages in thread From: ville.syrjala @ 2016-10-11 17:52 UTC (permalink / raw) To: intel-gfx; +Cc: stable, Maarten Maathuis From: Ville Syrjälä <ville.syrjala@linux.intel.com> The VBT provides the platform a way to mix and match the DDI ports vs. AUX channels. Currently we only trust the VBT for DDI E, which has no corresponding AUX channel of its own. However it is possible that some board might use some non-standard DDI vs. AUX port routing even for the other ports. Perhaps for signal routing reasons or something, So let's generalize this and trust the VBT for all ports. For now we'll limit this to DDI platforms, as we trust the VBT a bit more there anyway when it comes to the DDI ports. I've structured the code in a way that would allow us to easily expand this to other platforms as well, by simply filling in the ddi_port_info. v2: Drop whitespace changes, keep MISSING_CASE() for unknown aux ch assignment, include a commit message, include debug message during init Cc: stable@vger.kernel.org Cc: Maarten Maathuis <madman2003@gmail.com> Tested-by: Maarten Maathuis <madman2003@gmail.com> References: 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_dp.c | 71 +++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5992093e1814..b0753b272101 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1108,6 +1108,44 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) return ret; } +static enum port intel_aux_port(struct drm_i915_private *dev_priv, + enum port port) +{ + const struct ddi_vbt_port_info *info = + &dev_priv->vbt.ddi_port_info[port]; + enum port aux_port; + + if (!info->alternate_aux_channel) { + DRM_DEBUG_KMS("using AUX %c for port %c (platform default)\n", + port_name(port), port_name(port)); + return port; + } + + switch (info->alternate_aux_channel) { + case DP_AUX_A: + aux_port = PORT_A; + break; + case DP_AUX_B: + aux_port = PORT_B; + break; + case DP_AUX_C: + aux_port = PORT_C; + break; + case DP_AUX_D: + aux_port = PORT_D; + break; + default: + MISSING_CASE(info->alternate_aux_channel); + aux_port = PORT_A; + break; + } + + DRM_DEBUG_KMS("using AUX %c for port %c (VBT)\n", + port_name(aux_port), port_name(port)); + + return aux_port; +} + static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv, enum port port) { @@ -1168,36 +1206,9 @@ static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv, } } -/* - * On SKL we don't have Aux for port E so we rely - * on VBT to set a proper alternate aux channel. - */ -static enum port skl_porte_aux_port(struct drm_i915_private *dev_priv) -{ - const struct ddi_vbt_port_info *info = - &dev_priv->vbt.ddi_port_info[PORT_E]; - - switch (info->alternate_aux_channel) { - case DP_AUX_A: - return PORT_A; - case DP_AUX_B: - return PORT_B; - case DP_AUX_C: - return PORT_C; - case DP_AUX_D: - return PORT_D; - default: - MISSING_CASE(info->alternate_aux_channel); - return PORT_A; - } -} - static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv, enum port port) { - if (port == PORT_E) - port = skl_porte_aux_port(dev_priv); - switch (port) { case PORT_A: case PORT_B: @@ -1213,9 +1224,6 @@ static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv, static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv, enum port port, int index) { - if (port == PORT_E) - port = skl_porte_aux_port(dev_priv); - switch (port) { case PORT_A: case PORT_B: @@ -1253,7 +1261,8 @@ static i915_reg_t intel_aux_data_reg(struct drm_i915_private *dev_priv, static void intel_aux_reg_init(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp)); - enum port port = dp_to_dig_port(intel_dp)->port; + enum port port = intel_aux_port(dev_priv, + dp_to_dig_port(intel_dp)->port); int i; intel_dp->aux_ch_ctl_reg = intel_aux_ctl_reg(dev_priv, port); -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports 2016-10-11 17:52 ` [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports ville.syrjala @ 2016-10-13 17:59 ` Jim Bride 0 siblings, 0 replies; 11+ messages in thread From: Jim Bride @ 2016-10-13 17:59 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx, Maarten Maathuis, stable On Tue, Oct 11, 2016 at 08:52:45PM +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. > AUX channels. Currently we only trust the VBT for DDI E, which has no > corresponding AUX channel of its own. However it is possible that some > board might use some non-standard DDI vs. AUX port routing even for > the other ports. Perhaps for signal routing reasons or something, > So let's generalize this and trust the VBT for all ports. > > For now we'll limit this to DDI platforms, as we trust the VBT a bit > more there anyway when it comes to the DDI ports. I've structured > the code in a way that would allow us to easily expand this to > other platforms as well, by simply filling in the ddi_port_info. > > v2: Drop whitespace changes, keep MISSING_CASE() for unknown > aux ch assignment, include a commit message, include debug > message during init > > Cc: stable@vger.kernel.org > Cc: Maarten Maathuis <madman2003@gmail.com> > Tested-by: Maarten Maathuis <madman2003@gmail.com> > References: https://bugs.freedesktop.org/show_bug.cgi?id=97877 > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> Reviewed-by: Jim Bride <jim.bride@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 71 +++++++++++++++++++++++------------------ > 1 file changed, 40 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 5992093e1814..b0753b272101 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1108,6 +1108,44 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) > return ret; > } > > +static enum port intel_aux_port(struct drm_i915_private *dev_priv, > + enum port port) > +{ > + const struct ddi_vbt_port_info *info = > + &dev_priv->vbt.ddi_port_info[port]; > + enum port aux_port; > + > + if (!info->alternate_aux_channel) { > + DRM_DEBUG_KMS("using AUX %c for port %c (platform default)\n", > + port_name(port), port_name(port)); > + return port; > + } > + > + switch (info->alternate_aux_channel) { > + case DP_AUX_A: > + aux_port = PORT_A; > + break; > + case DP_AUX_B: > + aux_port = PORT_B; > + break; > + case DP_AUX_C: > + aux_port = PORT_C; > + break; > + case DP_AUX_D: > + aux_port = PORT_D; > + break; > + default: > + MISSING_CASE(info->alternate_aux_channel); > + aux_port = PORT_A; > + break; > + } > + > + DRM_DEBUG_KMS("using AUX %c for port %c (VBT)\n", > + port_name(aux_port), port_name(port)); > + > + return aux_port; > +} > + > static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv, > enum port port) > { > @@ -1168,36 +1206,9 @@ static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv, > } > } > > -/* > - * On SKL we don't have Aux for port E so we rely > - * on VBT to set a proper alternate aux channel. > - */ > -static enum port skl_porte_aux_port(struct drm_i915_private *dev_priv) > -{ > - const struct ddi_vbt_port_info *info = > - &dev_priv->vbt.ddi_port_info[PORT_E]; > - > - switch (info->alternate_aux_channel) { > - case DP_AUX_A: > - return PORT_A; > - case DP_AUX_B: > - return PORT_B; > - case DP_AUX_C: > - return PORT_C; > - case DP_AUX_D: > - return PORT_D; > - default: > - MISSING_CASE(info->alternate_aux_channel); > - return PORT_A; > - } > -} > - > static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv, > enum port port) > { > - if (port == PORT_E) > - port = skl_porte_aux_port(dev_priv); > - > switch (port) { > case PORT_A: > case PORT_B: > @@ -1213,9 +1224,6 @@ static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv, > static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv, > enum port port, int index) > { > - if (port == PORT_E) > - port = skl_porte_aux_port(dev_priv); > - > switch (port) { > case PORT_A: > case PORT_B: > @@ -1253,7 +1261,8 @@ static i915_reg_t intel_aux_data_reg(struct drm_i915_private *dev_priv, > static void intel_aux_reg_init(struct intel_dp *intel_dp) > { > struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp)); > - enum port port = dp_to_dig_port(intel_dp)->port; > + enum port port = intel_aux_port(dev_priv, > + dp_to_dig_port(intel_dp)->port); > int i; > > intel_dp->aux_ch_ctl_reg = intel_aux_ctl_reg(dev_priv, port); > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports [not found] <1476208368-5710-1-git-send-email-ville.syrjala@linux.intel.com> 2016-10-11 17:52 ` [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports ville.syrjala @ 2016-10-11 17:52 ` ville.syrjala [not found] ` <CAGZ4FERgWkshoRwyHdZdqTP_ODkfXuBU_+oJ+cmPoAZvUF97pA@mail.gmail.com> 2016-10-13 18:06 ` [Intel-gfx] " Jim Bride 2016-10-11 17:52 ` [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation ville.syrjala 2 siblings, 2 replies; 11+ messages in thread From: ville.syrjala @ 2016-10-11 17:52 UTC (permalink / raw) To: intel-gfx; +Cc: stable, Maarten Maathuis 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
[parent not found: <CAGZ4FERgWkshoRwyHdZdqTP_ODkfXuBU_+oJ+cmPoAZvUF97pA@mail.gmail.com>]
* Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports [not found] ` <CAGZ4FERgWkshoRwyHdZdqTP_ODkfXuBU_+oJ+cmPoAZvUF97pA@mail.gmail.com> @ 2016-10-12 10:57 ` Ville Syrjälä 0 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2016-10-12 10:57 UTC (permalink / raw) To: Maarten Maathuis; +Cc: intel-gfx, stable 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports 2016-10-11 17:52 ` [PATCH 2/4] drm/i915: Respect alternate_ddc_pin " ville.syrjala [not found] ` <CAGZ4FERgWkshoRwyHdZdqTP_ODkfXuBU_+oJ+cmPoAZvUF97pA@mail.gmail.com> @ 2016-10-13 18:06 ` Jim Bride 2016-10-13 18:29 ` Ville Syrjälä 1 sibling, 1 reply; 11+ messages in thread From: Jim Bride @ 2016-10-13 18:06 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx, Maarten Maathuis, stable 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports 2016-10-13 18:06 ` [Intel-gfx] " Jim Bride @ 2016-10-13 18:29 ` Ville Syrjälä 2016-10-17 6:50 ` Daniel Vetter 0 siblings, 1 reply; 11+ messages in thread From: Ville Syrjälä @ 2016-10-13 18:29 UTC (permalink / raw) To: Jim Bride; +Cc: intel-gfx, Maarten Maathuis, stable 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 -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports 2016-10-13 18:29 ` Ville Syrjälä @ 2016-10-17 6:50 ` Daniel Vetter 0 siblings, 0 replies; 11+ messages in thread From: Daniel Vetter @ 2016-10-17 6:50 UTC (permalink / raw) To: Ville Syrjälä; +Cc: Jim Bride, intel-gfx, Maarten Maathuis, stable 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation [not found] <1476208368-5710-1-git-send-email-ville.syrjala@linux.intel.com> 2016-10-11 17:52 ` [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports ville.syrjala 2016-10-11 17:52 ` [PATCH 2/4] drm/i915: Respect alternate_ddc_pin " ville.syrjala @ 2016-10-11 17:52 ` ville.syrjala 2016-10-13 18:17 ` [Intel-gfx] " Jim Bride ` (2 more replies) 2 siblings, 3 replies; 11+ messages in thread From: ville.syrjala @ 2016-10-11 17:52 UTC (permalink / raw) To: intel-gfx; +Cc: stable, Maarten Maathuis From: Ville Syrjälä <ville.syrjala@linux.intel.com> Now that we use the AUX and GMBUS assignment from VBT for all ports, let's clean up the sanitization of the port information a bit. Previosuly we only did this for port E, and only complained about a non-standard assignment for the other ports. But as we know that non-standard assignments are a fact of life, let's expand the sanitization to all the ports. v2: Include a commit message, fix up the comments a bit Cc: stable@vger.kernel.org Cc: Maarten Maathuis <madman2003@gmail.com> Tested-by: Maarten Maathuis <madman2003@gmail.com> References: 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_bios.c | 116 +++++++++++++++++++++++--------------- 1 file changed, 71 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 83667e8cdd6b..6d1ffa815e97 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1035,6 +1035,71 @@ static u8 translate_iboost(u8 val) return mapping[val]; } +static void sanitize_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]; + enum port p; + + for_each_port_masked(p, (1 << port) - 1) { + struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; + + if (info->alternate_ddc_pin != i->alternate_ddc_pin) + continue; + + DRM_DEBUG_KMS("port %c trying to use the same DDC pin (0x%x) as port %c, " + "disabling port %c DVI/HDMI support\n", + port_name(p), i->alternate_ddc_pin, + port_name(port), port_name(p)); + + /* + * If we have multiple ports supposedly sharing the + * pin, then dvi/hdmi couldn't exist on the shared + * port. Otherwise they share the same ddc bin and + * system couldn't communicate with them separately. + * + * Due to parsing the ports in alphabetical order, + * a higher port will always clobber a lower one. + */ + i->supports_dvi = false; + i->supports_hdmi = false; + i->alternate_ddc_pin = 0; + } +} + +static void sanitize_aux_ch(struct drm_i915_private *dev_priv, + enum port port) +{ + const struct ddi_vbt_port_info *info = + &dev_priv->vbt.ddi_port_info[port]; + enum port p; + + for_each_port_masked(p, (1 << port) - 1) { + struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; + + if (info->alternate_aux_channel != i->alternate_aux_channel) + continue; + + DRM_DEBUG_KMS("port %c trying to use the same AUX CH (0x%x) as port %c, " + "disabling port %c DP support\n", + port_name(p), i->alternate_aux_channel, + port_name(port), port_name(p)); + + /* + * If we have multiple ports supposedlt sharing the + * aux channel, then DP couldn't exist on the shared + * port. Otherwise they share the same aux channel + * and system couldn't communicate with them separately. + * + * Due to parsing the ports in alphabetical order, + * a higher port will always clobber a lower one. + */ + i->supports_dp = false; + i->alternate_aux_channel = 0; + } +} + static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, const struct bdb_header *bdb) { @@ -1109,54 +1174,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port)); if (is_dvi) { - if (port == PORT_E) { - info->alternate_ddc_pin = ddc_pin; - /* if DDIE share ddc pin with other port, then - * dvi/hdmi couldn't exist on the shared port. - * Otherwise they share the same ddc bin and system - * couldn't communicate with them seperately. */ - if (ddc_pin == DDC_PIN_B) { - dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0; - dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0; - } else if (ddc_pin == DDC_PIN_C) { - dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0; - dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0; - } else if (ddc_pin == DDC_PIN_D) { - dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0; - dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0; - } - } else if (ddc_pin == DDC_PIN_B && port != PORT_B) - DRM_DEBUG_KMS("Unexpected DDC pin for port B\n"); - else if (ddc_pin == DDC_PIN_C && port != PORT_C) - DRM_DEBUG_KMS("Unexpected DDC pin for port C\n"); - else if (ddc_pin == DDC_PIN_D && port != PORT_D) - DRM_DEBUG_KMS("Unexpected DDC pin for port D\n"); + info->alternate_ddc_pin = ddc_pin; + + sanitize_ddc_pin(dev_priv, port); } if (is_dp) { - if (port == PORT_E) { - info->alternate_aux_channel = aux_channel; - /* if DDIE share aux channel with other port, then - * DP couldn't exist on the shared port. Otherwise - * they share the same aux channel and system - * couldn't communicate with them seperately. */ - if (aux_channel == DP_AUX_A) - dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0; - else if (aux_channel == DP_AUX_B) - dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0; - else if (aux_channel == DP_AUX_C) - dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0; - else if (aux_channel == DP_AUX_D) - dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0; - } - else if (aux_channel == DP_AUX_A && port != PORT_A) - DRM_DEBUG_KMS("Unexpected AUX channel for port A\n"); - else if (aux_channel == DP_AUX_B && port != PORT_B) - DRM_DEBUG_KMS("Unexpected AUX channel for port B\n"); - else if (aux_channel == DP_AUX_C && port != PORT_C) - DRM_DEBUG_KMS("Unexpected AUX channel for port C\n"); - else if (aux_channel == DP_AUX_D && port != PORT_D) - DRM_DEBUG_KMS("Unexpected AUX channel for port D\n"); + info->alternate_aux_channel = aux_channel; + + sanitize_aux_ch(dev_priv, port); } if (bdb->version >= 158) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation 2016-10-11 17:52 ` [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation ville.syrjala @ 2016-10-13 18:17 ` Jim Bride 2016-10-17 18:00 ` Ville Syrjälä 2016-10-17 18:07 ` [PATCH v3 " ville.syrjala 2 siblings, 0 replies; 11+ messages in thread From: Jim Bride @ 2016-10-13 18:17 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx, Maarten Maathuis, stable On Tue, Oct 11, 2016 at 08:52:47PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > Now that we use the AUX and GMBUS assignment from VBT for all ports, > let's clean up the sanitization of the port information a bit. > Previosuly we only did this for port E, and only complained about a > non-standard assignment for the other ports. But as we know that > non-standard assignments are a fact of life, let's expand the > sanitization to all the ports. > > v2: Include a commit message, fix up the comments a bit > > Cc: stable@vger.kernel.org > Cc: Maarten Maathuis <madman2003@gmail.com> > Tested-by: Maarten Maathuis <madman2003@gmail.com> > References: https://bugs.freedesktop.org/show_bug.cgi?id=97877 > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> Reviewed-by: Jim Bride <jim.bride@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_bios.c | 116 +++++++++++++++++++++++--------------- > 1 file changed, 71 insertions(+), 45 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 83667e8cdd6b..6d1ffa815e97 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -1035,6 +1035,71 @@ static u8 translate_iboost(u8 val) > return mapping[val]; > } > > +static void sanitize_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]; > + enum port p; > + > + for_each_port_masked(p, (1 << port) - 1) { > + struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; > + > + if (info->alternate_ddc_pin != i->alternate_ddc_pin) > + continue; > + > + DRM_DEBUG_KMS("port %c trying to use the same DDC pin (0x%x) as port %c, " > + "disabling port %c DVI/HDMI support\n", > + port_name(p), i->alternate_ddc_pin, > + port_name(port), port_name(p)); > + > + /* > + * If we have multiple ports supposedly sharing the > + * pin, then dvi/hdmi couldn't exist on the shared > + * port. Otherwise they share the same ddc bin and > + * system couldn't communicate with them separately. > + * > + * Due to parsing the ports in alphabetical order, > + * a higher port will always clobber a lower one. > + */ > + i->supports_dvi = false; > + i->supports_hdmi = false; > + i->alternate_ddc_pin = 0; > + } > +} > + > +static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > + enum port port) > +{ > + const struct ddi_vbt_port_info *info = > + &dev_priv->vbt.ddi_port_info[port]; > + enum port p; > + > + for_each_port_masked(p, (1 << port) - 1) { > + struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; > + > + if (info->alternate_aux_channel != i->alternate_aux_channel) > + continue; > + > + DRM_DEBUG_KMS("port %c trying to use the same AUX CH (0x%x) as port %c, " > + "disabling port %c DP support\n", > + port_name(p), i->alternate_aux_channel, > + port_name(port), port_name(p)); > + > + /* > + * If we have multiple ports supposedlt sharing the > + * aux channel, then DP couldn't exist on the shared > + * port. Otherwise they share the same aux channel > + * and system couldn't communicate with them separately. > + * > + * Due to parsing the ports in alphabetical order, > + * a higher port will always clobber a lower one. > + */ > + i->supports_dp = false; > + i->alternate_aux_channel = 0; > + } > +} > + > static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > const struct bdb_header *bdb) > { > @@ -1109,54 +1174,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port)); > > if (is_dvi) { > - if (port == PORT_E) { > - info->alternate_ddc_pin = ddc_pin; > - /* if DDIE share ddc pin with other port, then > - * dvi/hdmi couldn't exist on the shared port. > - * Otherwise they share the same ddc bin and system > - * couldn't communicate with them seperately. */ > - if (ddc_pin == DDC_PIN_B) { > - dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0; > - dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0; > - } else if (ddc_pin == DDC_PIN_C) { > - dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0; > - dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0; > - } else if (ddc_pin == DDC_PIN_D) { > - dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0; > - dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0; > - } > - } else if (ddc_pin == DDC_PIN_B && port != PORT_B) > - DRM_DEBUG_KMS("Unexpected DDC pin for port B\n"); > - else if (ddc_pin == DDC_PIN_C && port != PORT_C) > - DRM_DEBUG_KMS("Unexpected DDC pin for port C\n"); > - else if (ddc_pin == DDC_PIN_D && port != PORT_D) > - DRM_DEBUG_KMS("Unexpected DDC pin for port D\n"); > + info->alternate_ddc_pin = ddc_pin; > + > + sanitize_ddc_pin(dev_priv, port); > } > > if (is_dp) { > - if (port == PORT_E) { > - info->alternate_aux_channel = aux_channel; > - /* if DDIE share aux channel with other port, then > - * DP couldn't exist on the shared port. Otherwise > - * they share the same aux channel and system > - * couldn't communicate with them seperately. */ > - if (aux_channel == DP_AUX_A) > - dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0; > - else if (aux_channel == DP_AUX_B) > - dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0; > - else if (aux_channel == DP_AUX_C) > - dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0; > - else if (aux_channel == DP_AUX_D) > - dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0; > - } > - else if (aux_channel == DP_AUX_A && port != PORT_A) > - DRM_DEBUG_KMS("Unexpected AUX channel for port A\n"); > - else if (aux_channel == DP_AUX_B && port != PORT_B) > - DRM_DEBUG_KMS("Unexpected AUX channel for port B\n"); > - else if (aux_channel == DP_AUX_C && port != PORT_C) > - DRM_DEBUG_KMS("Unexpected AUX channel for port C\n"); > - else if (aux_channel == DP_AUX_D && port != PORT_D) > - DRM_DEBUG_KMS("Unexpected AUX channel for port D\n"); > + info->alternate_aux_channel = aux_channel; > + > + sanitize_aux_ch(dev_priv, port); > } > > if (bdb->version >= 158) { > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation 2016-10-11 17:52 ` [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation ville.syrjala 2016-10-13 18:17 ` [Intel-gfx] " Jim Bride @ 2016-10-17 18:00 ` Ville Syrjälä 2016-10-17 18:07 ` [PATCH v3 " ville.syrjala 2 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2016-10-17 18:00 UTC (permalink / raw) To: intel-gfx; +Cc: stable, Maarten Maathuis On Tue, Oct 11, 2016 at 08:52:47PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > Now that we use the AUX and GMBUS assignment from VBT for all ports, > let's clean up the sanitization of the port information a bit. > Previosuly we only did this for port E, and only complained about a > non-standard assignment for the other ports. But as we know that > non-standard assignments are a fact of life, let's expand the > sanitization to all the ports. > > v2: Include a commit message, fix up the comments a bit > > Cc: stable@vger.kernel.org > Cc: Maarten Maathuis <madman2003@gmail.com> > Tested-by: Maarten Maathuis <madman2003@gmail.com> > References: 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_bios.c | 116 +++++++++++++++++++++++--------------- > 1 file changed, 71 insertions(+), 45 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 83667e8cdd6b..6d1ffa815e97 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -1035,6 +1035,71 @@ static u8 translate_iboost(u8 val) > return mapping[val]; > } > > +static void sanitize_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]; > + enum port p; > + Hmm. And I just realized this code is probably slightly garbage. If the current port doesn't have alternate_ddc_pin/alternate_aux_channel, then we'd proceed to clobber any lower port without the thing as well. Now, potentially all DDI platforms have this stuff fully decked out in VBT so it might not matter in practice. But better safe that sorry, so I'll send out a new version of this patch that checks here for the zero case in both of these functions. > + for_each_port_masked(p, (1 << port) - 1) { > + struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; > + > + if (info->alternate_ddc_pin != i->alternate_ddc_pin) > + continue; > + > + DRM_DEBUG_KMS("port %c trying to use the same DDC pin (0x%x) as port %c, " > + "disabling port %c DVI/HDMI support\n", > + port_name(p), i->alternate_ddc_pin, > + port_name(port), port_name(p)); > + > + /* > + * If we have multiple ports supposedly sharing the > + * pin, then dvi/hdmi couldn't exist on the shared > + * port. Otherwise they share the same ddc bin and > + * system couldn't communicate with them separately. > + * > + * Due to parsing the ports in alphabetical order, > + * a higher port will always clobber a lower one. > + */ > + i->supports_dvi = false; > + i->supports_hdmi = false; > + i->alternate_ddc_pin = 0; > + } > +} > + > +static void sanitize_aux_ch(struct drm_i915_private *dev_priv, > + enum port port) > +{ > + const struct ddi_vbt_port_info *info = > + &dev_priv->vbt.ddi_port_info[port]; > + enum port p; > + > + for_each_port_masked(p, (1 << port) - 1) { > + struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; > + > + if (info->alternate_aux_channel != i->alternate_aux_channel) > + continue; > + > + DRM_DEBUG_KMS("port %c trying to use the same AUX CH (0x%x) as port %c, " > + "disabling port %c DP support\n", > + port_name(p), i->alternate_aux_channel, > + port_name(port), port_name(p)); > + > + /* > + * If we have multiple ports supposedlt sharing the > + * aux channel, then DP couldn't exist on the shared > + * port. Otherwise they share the same aux channel > + * and system couldn't communicate with them separately. > + * > + * Due to parsing the ports in alphabetical order, > + * a higher port will always clobber a lower one. > + */ > + i->supports_dp = false; > + i->alternate_aux_channel = 0; > + } > +} > + > static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > const struct bdb_header *bdb) > { > @@ -1109,54 +1174,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, > DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port)); > > if (is_dvi) { > - if (port == PORT_E) { > - info->alternate_ddc_pin = ddc_pin; > - /* if DDIE share ddc pin with other port, then > - * dvi/hdmi couldn't exist on the shared port. > - * Otherwise they share the same ddc bin and system > - * couldn't communicate with them seperately. */ > - if (ddc_pin == DDC_PIN_B) { > - dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0; > - dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0; > - } else if (ddc_pin == DDC_PIN_C) { > - dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0; > - dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0; > - } else if (ddc_pin == DDC_PIN_D) { > - dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0; > - dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0; > - } > - } else if (ddc_pin == DDC_PIN_B && port != PORT_B) > - DRM_DEBUG_KMS("Unexpected DDC pin for port B\n"); > - else if (ddc_pin == DDC_PIN_C && port != PORT_C) > - DRM_DEBUG_KMS("Unexpected DDC pin for port C\n"); > - else if (ddc_pin == DDC_PIN_D && port != PORT_D) > - DRM_DEBUG_KMS("Unexpected DDC pin for port D\n"); > + info->alternate_ddc_pin = ddc_pin; > + > + sanitize_ddc_pin(dev_priv, port); > } > > if (is_dp) { > - if (port == PORT_E) { > - info->alternate_aux_channel = aux_channel; > - /* if DDIE share aux channel with other port, then > - * DP couldn't exist on the shared port. Otherwise > - * they share the same aux channel and system > - * couldn't communicate with them seperately. */ > - if (aux_channel == DP_AUX_A) > - dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0; > - else if (aux_channel == DP_AUX_B) > - dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0; > - else if (aux_channel == DP_AUX_C) > - dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0; > - else if (aux_channel == DP_AUX_D) > - dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0; > - } > - else if (aux_channel == DP_AUX_A && port != PORT_A) > - DRM_DEBUG_KMS("Unexpected AUX channel for port A\n"); > - else if (aux_channel == DP_AUX_B && port != PORT_B) > - DRM_DEBUG_KMS("Unexpected AUX channel for port B\n"); > - else if (aux_channel == DP_AUX_C && port != PORT_C) > - DRM_DEBUG_KMS("Unexpected AUX channel for port C\n"); > - else if (aux_channel == DP_AUX_D && port != PORT_D) > - DRM_DEBUG_KMS("Unexpected AUX channel for port D\n"); > + info->alternate_aux_channel = aux_channel; > + > + sanitize_aux_ch(dev_priv, port); > } > > if (bdb->version >= 158) { > -- > 2.7.4 -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation 2016-10-11 17:52 ` [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation ville.syrjala 2016-10-13 18:17 ` [Intel-gfx] " Jim Bride 2016-10-17 18:00 ` Ville Syrjälä @ 2016-10-17 18:07 ` ville.syrjala 2 siblings, 0 replies; 11+ messages in thread From: ville.syrjala @ 2016-10-17 18:07 UTC (permalink / raw) To: intel-gfx; +Cc: stable, Maarten Maathuis From: Ville Syrjälä <ville.syrjala@linux.intel.com> Now that we use the AUX and GMBUS assignment from VBT for all ports, let's clean up the sanitization of the port information a bit. Previosuly we only did this for port E, and only complained about a non-standard assignment for the other ports. But as we know that non-standard assignments are a fact of life, let's expand the sanitization to all the ports. v2: Include a commit message, fix up the comments a bit v3: Don't clobber other ports if the current port has no alternate aux ch/ddc pin Cc: stable@vger.kernel.org Cc: Maarten Maathuis <madman2003@gmail.com> Tested-by: Maarten Maathuis <madman2003@gmail.com> References: https://bugs.freedesktop.org/show_bug.cgi?id=97877 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Link: http://patchwork.freedesktop.org/patch/msgid/1476208368-5710-4-git-send-email-ville.syrjala@linux.intel.com Reviewed-by: Jim Bride <jim.bride@linux.intel.com> (v2) --- drivers/gpu/drm/i915/intel_bios.c | 122 ++++++++++++++++++++++++-------------- 1 file changed, 77 insertions(+), 45 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 83667e8cdd6b..a8ff8c099685 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1035,6 +1035,77 @@ static u8 translate_iboost(u8 val) return mapping[val]; } +static void sanitize_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]; + enum port p; + + if (!info->alternate_ddc_pin) + return; + + for_each_port_masked(p, (1 << port) - 1) { + struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; + + if (info->alternate_ddc_pin != i->alternate_ddc_pin) + continue; + + DRM_DEBUG_KMS("port %c trying to use the same DDC pin (0x%x) as port %c, " + "disabling port %c DVI/HDMI support\n", + port_name(p), i->alternate_ddc_pin, + port_name(port), port_name(p)); + + /* + * If we have multiple ports supposedly sharing the + * pin, then dvi/hdmi couldn't exist on the shared + * port. Otherwise they share the same ddc bin and + * system couldn't communicate with them separately. + * + * Due to parsing the ports in alphabetical order, + * a higher port will always clobber a lower one. + */ + i->supports_dvi = false; + i->supports_hdmi = false; + i->alternate_ddc_pin = 0; + } +} + +static void sanitize_aux_ch(struct drm_i915_private *dev_priv, + enum port port) +{ + const struct ddi_vbt_port_info *info = + &dev_priv->vbt.ddi_port_info[port]; + enum port p; + + if (!info->alternate_aux_channel) + return; + + for_each_port_masked(p, (1 << port) - 1) { + struct ddi_vbt_port_info *i = &dev_priv->vbt.ddi_port_info[p]; + + if (info->alternate_aux_channel != i->alternate_aux_channel) + continue; + + DRM_DEBUG_KMS("port %c trying to use the same AUX CH (0x%x) as port %c, " + "disabling port %c DP support\n", + port_name(p), i->alternate_aux_channel, + port_name(port), port_name(p)); + + /* + * If we have multiple ports supposedlt sharing the + * aux channel, then DP couldn't exist on the shared + * port. Otherwise they share the same aux channel + * and system couldn't communicate with them separately. + * + * Due to parsing the ports in alphabetical order, + * a higher port will always clobber a lower one. + */ + i->supports_dp = false; + i->alternate_aux_channel = 0; + } +} + static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, const struct bdb_header *bdb) { @@ -1109,54 +1180,15 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port, DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port)); if (is_dvi) { - if (port == PORT_E) { - info->alternate_ddc_pin = ddc_pin; - /* if DDIE share ddc pin with other port, then - * dvi/hdmi couldn't exist on the shared port. - * Otherwise they share the same ddc bin and system - * couldn't communicate with them seperately. */ - if (ddc_pin == DDC_PIN_B) { - dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0; - dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0; - } else if (ddc_pin == DDC_PIN_C) { - dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0; - dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0; - } else if (ddc_pin == DDC_PIN_D) { - dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0; - dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0; - } - } else if (ddc_pin == DDC_PIN_B && port != PORT_B) - DRM_DEBUG_KMS("Unexpected DDC pin for port B\n"); - else if (ddc_pin == DDC_PIN_C && port != PORT_C) - DRM_DEBUG_KMS("Unexpected DDC pin for port C\n"); - else if (ddc_pin == DDC_PIN_D && port != PORT_D) - DRM_DEBUG_KMS("Unexpected DDC pin for port D\n"); + info->alternate_ddc_pin = ddc_pin; + + sanitize_ddc_pin(dev_priv, port); } if (is_dp) { - if (port == PORT_E) { - info->alternate_aux_channel = aux_channel; - /* if DDIE share aux channel with other port, then - * DP couldn't exist on the shared port. Otherwise - * they share the same aux channel and system - * couldn't communicate with them seperately. */ - if (aux_channel == DP_AUX_A) - dev_priv->vbt.ddi_port_info[PORT_A].supports_dp = 0; - else if (aux_channel == DP_AUX_B) - dev_priv->vbt.ddi_port_info[PORT_B].supports_dp = 0; - else if (aux_channel == DP_AUX_C) - dev_priv->vbt.ddi_port_info[PORT_C].supports_dp = 0; - else if (aux_channel == DP_AUX_D) - dev_priv->vbt.ddi_port_info[PORT_D].supports_dp = 0; - } - else if (aux_channel == DP_AUX_A && port != PORT_A) - DRM_DEBUG_KMS("Unexpected AUX channel for port A\n"); - else if (aux_channel == DP_AUX_B && port != PORT_B) - DRM_DEBUG_KMS("Unexpected AUX channel for port B\n"); - else if (aux_channel == DP_AUX_C && port != PORT_C) - DRM_DEBUG_KMS("Unexpected AUX channel for port C\n"); - else if (aux_channel == DP_AUX_D && port != PORT_D) - DRM_DEBUG_KMS("Unexpected AUX channel for port D\n"); + info->alternate_aux_channel = aux_channel; + + sanitize_aux_ch(dev_priv, port); } if (bdb->version >= 158) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-10-17 18:07 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1476208368-5710-1-git-send-email-ville.syrjala@linux.intel.com>
2016-10-11 17:52 ` [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports ville.syrjala
2016-10-13 17:59 ` [Intel-gfx] " Jim Bride
2016-10-11 17:52 ` [PATCH 2/4] drm/i915: Respect alternate_ddc_pin " ville.syrjala
[not found] ` <CAGZ4FERgWkshoRwyHdZdqTP_ODkfXuBU_+oJ+cmPoAZvUF97pA@mail.gmail.com>
2016-10-12 10:57 ` Ville Syrjälä
2016-10-13 18:06 ` [Intel-gfx] " Jim Bride
2016-10-13 18:29 ` Ville Syrjälä
2016-10-17 6:50 ` Daniel Vetter
2016-10-11 17:52 ` [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation ville.syrjala
2016-10-13 18:17 ` [Intel-gfx] " Jim Bride
2016-10-17 18:00 ` Ville Syrjälä
2016-10-17 18:07 ` [PATCH v3 " ville.syrjala
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).