* [PATCH 0/4] drm/i915: Trust VBT aux/ddc information
@ 2016-10-11 17:52 ville.syrjala
2016-10-11 17:52 ` ville.syrjala
` (5 more replies)
0 siblings, 6 replies; 33+ messages in thread
From: ville.syrjala @ 2016-10-11 17:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Maarten Maathuis
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
So we have a bug or two ([1] at least) about using an incorrect gmbus pin for
a HDMI connector. So I cooked up some patches to expand our use of VBT for this
information. So far I've kept it limited to DDI platforms, but I think I've seen
another bug about CHV using the incorrect gmbus pin as well, not 100% sure though.
I slapped cc:stable on them since they do fix an actual issue. My track record
with this sort of firmware information hasn't been great as of late, so some
fallout might occur.
Anyways here's the entire series:
git://github.com/vsyrjala/linux.git trust_vbt_ddc_aux_pins_2
[1] https://bugs.freedesktop.org/show_bug.cgi?id=97877
Cc: Maarten Maathuis <madman2003@gmail.com>
Ville Syrjälä (4):
drm/i915: Respect alternate_aux_channel for all DDI ports
drm/i915: Respect alternate_ddc_pin for all DDI ports
drm/i915: Clean up DDI DDC/AUX CH sanitation
drm/i915: Fix whitespace issues
drivers/gpu/drm/i915/intel_bios.c | 116 +++++++++++++++++++++++---------------
drivers/gpu/drm/i915/intel_dp.c | 87 +++++++++++++++-------------
drivers/gpu/drm/i915/intel_hdmi.c | 84 +++++++++++++++------------
3 files changed, 167 insertions(+), 120 deletions(-)
--
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] 33+ messages in thread
* [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports
2016-10-11 17:52 [PATCH 0/4] drm/i915: Trust VBT aux/ddc information ville.syrjala
@ 2016-10-11 17:52 ` ville.syrjala
2016-10-11 17:52 ` ville.syrjala
` (4 subsequent siblings)
5 siblings, 0 replies; 33+ 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] 33+ messages in thread
* [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports
@ 2016-10-11 17:52 ` ville.syrjala
0 siblings, 0 replies; 33+ messages in thread
From: ville.syrjala @ 2016-10-11 17:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Maarten Maathuis, stable
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
2016-10-11 17:52 [PATCH 0/4] drm/i915: Trust VBT aux/ddc information ville.syrjala
@ 2016-10-11 17:52 ` ville.syrjala
2016-10-11 17:52 ` ville.syrjala
` (4 subsequent siblings)
5 siblings, 0 replies; 33+ 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] 33+ messages in thread
* [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
@ 2016-10-11 17:52 ` ville.syrjala
0 siblings, 0 replies; 33+ messages in thread
From: ville.syrjala @ 2016-10-11 17:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Maarten Maathuis, stable
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
2016-10-11 17:52 [PATCH 0/4] drm/i915: Trust VBT aux/ddc information ville.syrjala
@ 2016-10-11 17:52 ` ville.syrjala
2016-10-11 17:52 ` ville.syrjala
` (4 subsequent siblings)
5 siblings, 0 replies; 33+ 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] 33+ messages in thread
* [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
@ 2016-10-11 17:52 ` ville.syrjala
0 siblings, 0 replies; 33+ messages in thread
From: ville.syrjala @ 2016-10-11 17:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Maarten Maathuis, stable
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH 4/4] drm/i915: Fix whitespace issues
2016-10-11 17:52 [PATCH 0/4] drm/i915: Trust VBT aux/ddc information ville.syrjala
` (2 preceding siblings ...)
2016-10-11 17:52 ` ville.syrjala
@ 2016-10-11 17:52 ` ville.syrjala
2016-10-13 18:18 ` Jim Bride
2016-10-11 18:49 ` ✗ Fi.CI.BAT: warning for drm/i915: Trust VBT aux/ddc information Patchwork
2016-10-17 18:54 ` ✗ Fi.CI.BAT: failure for drm/i915: Trust VBT aux/ddc information (rev2) Patchwork
5 siblings, 1 reply; 33+ messages in thread
From: ville.syrjala @ 2016-10-11 17:52 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Fix the poorly indented port parameters to the aux ctl and data
reg functions. This was fallout from the s/i915_mmio_reg_t/i915_reg_t/
that happened during the review of commit f0f59a00a1c9 ("drm/i915:
Type safe register read/write")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b0753b272101..25cde7356b59 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1147,7 +1147,7 @@ static enum port intel_aux_port(struct drm_i915_private *dev_priv,
}
static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
- enum port port)
+ enum port port)
{
switch (port) {
case PORT_B:
@@ -1161,7 +1161,7 @@ static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
}
static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
- enum port port, int index)
+ enum port port, int index)
{
switch (port) {
case PORT_B:
@@ -1175,7 +1175,7 @@ static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
}
static i915_reg_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
- enum port port)
+ enum port port)
{
switch (port) {
case PORT_A:
@@ -1191,7 +1191,7 @@ static i915_reg_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
}
static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv,
- enum port port, int index)
+ enum port port, int index)
{
switch (port) {
case PORT_A:
@@ -1207,7 +1207,7 @@ static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv,
}
static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv,
- enum port port)
+ enum port port)
{
switch (port) {
case PORT_A:
@@ -1222,7 +1222,7 @@ 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)
+ enum port port, int index)
{
switch (port) {
case PORT_A:
@@ -1237,7 +1237,7 @@ static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv,
}
static i915_reg_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv,
- enum port port)
+ enum port port)
{
if (INTEL_INFO(dev_priv)->gen >= 9)
return skl_aux_ctl_reg(dev_priv, port);
@@ -1248,7 +1248,7 @@ static i915_reg_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv,
}
static i915_reg_t intel_aux_data_reg(struct drm_i915_private *dev_priv,
- enum port port, int index)
+ enum port port, int index)
{
if (INTEL_INFO(dev_priv)->gen >= 9)
return skl_aux_data_reg(dev_priv, port, index);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 33+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915: Trust VBT aux/ddc information
2016-10-11 17:52 [PATCH 0/4] drm/i915: Trust VBT aux/ddc information ville.syrjala
` (3 preceding siblings ...)
2016-10-11 17:52 ` [PATCH 4/4] drm/i915: Fix whitespace issues ville.syrjala
@ 2016-10-11 18:49 ` Patchwork
2016-10-12 10:54 ` Ville Syrjälä
2016-10-17 18:54 ` ✗ Fi.CI.BAT: failure for drm/i915: Trust VBT aux/ddc information (rev2) Patchwork
5 siblings, 1 reply; 33+ messages in thread
From: Patchwork @ 2016-10-11 18:49 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Trust VBT aux/ddc information
URL : https://patchwork.freedesktop.org/series/13600/
State : warning
== Summary ==
Series 13600v1 drm/i915: Trust VBT aux/ddc information
https://patchwork.freedesktop.org/api/1.0/series/13600/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (fi-byt-j1900)
Subgroup suspend-read-crc-pipe-c:
pass -> DMESG-WARN (fi-skl-6700k)
Test kms_psr_sink_crc:
Subgroup psr_basic:
pass -> DMESG-WARN (fi-skl-6700hq)
Test pm_backlight:
Subgroup basic-brightness:
skip -> PASS (fi-skl-6700k)
Test vgem_basic:
Subgroup unload:
pass -> SKIP (fi-ivb-3770)
pass -> SKIP (fi-byt-n2820)
skip -> PASS (fi-bsw-n3050)
skip -> PASS (fi-skl-6700k)
skip -> PASS (fi-bdw-5557u)
fi-bdw-5557u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16
fi-bsw-n3050 total:248 pass:205 dwarn:0 dfail:0 fail:0 skip:43
fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31
fi-byt-j1900 total:248 pass:215 dwarn:0 dfail:0 fail:1 skip:32
fi-byt-n2820 total:248 pass:210 dwarn:0 dfail:0 fail:1 skip:37
fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24
fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24
fi-ilk-650 total:248 pass:185 dwarn:0 dfail:0 fail:2 skip:61
fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27
fi-ivb-3770 total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27
fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26
fi-skl-6700hq total:248 pass:223 dwarn:1 dfail:0 fail:0 skip:24
fi-skl-6700k total:248 pass:222 dwarn:2 dfail:0 fail:0 skip:24
fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37
fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39
Results at /archive/results/CI_IGT_test/Patchwork_2676/
41409515b7b3365bcb2c2e9239fdfaa286a51333 drm-intel-nightly: 2016y-10m-11d-17h-55m-34s UTC integration manifest
8e39dff drm/i915: Fix whitespace issues
536a3b5 drm/i915: Clean up DDI DDC/AUX CH sanitation
a01faf4 drm/i915: Respect alternate_ddc_pin for all DDI ports
8e81858 drm/i915: Respect alternate_aux_channel for all DDI ports
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
2016-10-11 17:52 ` ville.syrjala
(?)
@ 2016-10-11 20:04 ` Maarten Maathuis
2016-10-12 10:57 ` Ville Syrjälä
-1 siblings, 1 reply; 33+ messages in thread
From: Maarten Maathuis @ 2016-10-11 20:04 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, stable
[-- Attachment #1.1: Type: text/plain, Size: 6837 bytes --]
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
>
>
--
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.
[-- Attachment #1.2: Type: text/html, Size: 8981 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: ✗ Fi.CI.BAT: warning for drm/i915: Trust VBT aux/ddc information
2016-10-11 18:49 ` ✗ Fi.CI.BAT: warning for drm/i915: Trust VBT aux/ddc information Patchwork
@ 2016-10-12 10:54 ` Ville Syrjälä
0 siblings, 0 replies; 33+ messages in thread
From: Ville Syrjälä @ 2016-10-12 10:54 UTC (permalink / raw)
To: intel-gfx
On Tue, Oct 11, 2016 at 06:49:46PM -0000, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915: Trust VBT aux/ddc information
> URL : https://patchwork.freedesktop.org/series/13600/
> State : warning
>
> == Summary ==
>
> Series 13600v1 drm/i915: Trust VBT aux/ddc information
> https://patchwork.freedesktop.org/api/1.0/series/13600/revisions/1/mbox/
>
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-b:
> dmesg-warn -> PASS (fi-byt-j1900)
> Subgroup suspend-read-crc-pipe-c:
> pass -> DMESG-WARN (fi-skl-6700k)
[ 631.216836] [drm:intel_cpu_fifo_underrun_irq_handler [i915]] *ERROR* CPU pipe C FIFO underrun
moar SKL underruns
> Test kms_psr_sink_crc:
> Subgroup psr_basic:
> pass -> DMESG-WARN (fi-skl-6700hq)
[ 647.478464] [drm:intel_pipe_update_start [i915]] *ERROR* Potential atomic update failure on pipe A
DMC making life difficult for vblank irqs I guess.
> Test pm_backlight:
> Subgroup basic-brightness:
> skip -> PASS (fi-skl-6700k)
> Test vgem_basic:
> Subgroup unload:
> pass -> SKIP (fi-ivb-3770)
> pass -> SKIP (fi-byt-n2820)
> skip -> PASS (fi-bsw-n3050)
> skip -> PASS (fi-skl-6700k)
> skip -> PASS (fi-bdw-5557u)
>
> fi-bdw-5557u total:248 pass:232 dwarn:0 dfail:0 fail:0 skip:16
> fi-bsw-n3050 total:248 pass:205 dwarn:0 dfail:0 fail:0 skip:43
> fi-bxt-t5700 total:248 pass:217 dwarn:0 dfail:0 fail:0 skip:31
> fi-byt-j1900 total:248 pass:215 dwarn:0 dfail:0 fail:1 skip:32
> fi-byt-n2820 total:248 pass:210 dwarn:0 dfail:0 fail:1 skip:37
> fi-hsw-4770 total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24
> fi-hsw-4770r total:248 pass:224 dwarn:0 dfail:0 fail:0 skip:24
> fi-ilk-650 total:248 pass:185 dwarn:0 dfail:0 fail:2 skip:61
> fi-ivb-3520m total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27
> fi-ivb-3770 total:248 pass:221 dwarn:0 dfail:0 fail:0 skip:27
> fi-kbl-7200u total:248 pass:222 dwarn:0 dfail:0 fail:0 skip:26
> fi-skl-6700hq total:248 pass:223 dwarn:1 dfail:0 fail:0 skip:24
> fi-skl-6700k total:248 pass:222 dwarn:2 dfail:0 fail:0 skip:24
> fi-snb-2520m total:248 pass:211 dwarn:0 dfail:0 fail:0 skip:37
> fi-snb-2600 total:248 pass:209 dwarn:0 dfail:0 fail:0 skip:39
>
> Results at /archive/results/CI_IGT_test/Patchwork_2676/
>
> 41409515b7b3365bcb2c2e9239fdfaa286a51333 drm-intel-nightly: 2016y-10m-11d-17h-55m-34s UTC integration manifest
> 8e39dff drm/i915: Fix whitespace issues
> 536a3b5 drm/i915: Clean up DDI DDC/AUX CH sanitation
> a01faf4 drm/i915: Respect alternate_ddc_pin for all DDI ports
> 8e81858 drm/i915: Respect alternate_aux_channel for all DDI ports
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
2016-10-11 20:04 ` Maarten Maathuis
@ 2016-10-12 10:57 ` Ville Syrjälä
0 siblings, 0 replies; 33+ 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] 33+ messages in thread
* Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
@ 2016-10-12 10:57 ` Ville Syrjälä
0 siblings, 0 replies; 33+ 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
2016-10-12 10:57 ` Ville Syrjälä
(?)
@ 2016-10-12 16:56 ` Maarten Maathuis
-1 siblings, 0 replies; 33+ messages in thread
From: Maarten Maathuis @ 2016-10-12 16:56 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx, stable
[-- Attachment #1.1: Type: text/plain, Size: 8208 bytes --]
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
>
--
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.
[-- Attachment #1.2: Type: text/html, Size: 11566 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports
2016-10-11 17:52 ` ville.syrjala
@ 2016-10-13 17:59 ` Jim Bride
-1 siblings, 0 replies; 33+ 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] 33+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915: Respect alternate_aux_channel for all DDI ports
@ 2016-10-13 17:59 ` Jim Bride
0 siblings, 0 replies; 33+ 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] 33+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
2016-10-11 17:52 ` ville.syrjala
@ 2016-10-13 18:06 ` Jim Bride
-1 siblings, 0 replies; 33+ 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] 33+ messages in thread
* Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
@ 2016-10-13 18:06 ` Jim Bride
0 siblings, 0 replies; 33+ 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
2016-10-11 17:52 ` ville.syrjala
@ 2016-10-13 18:17 ` Jim Bride
-1 siblings, 0 replies; 33+ 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] 33+ messages in thread
* Re: [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
@ 2016-10-13 18:17 ` Jim Bride
0 siblings, 0 replies; 33+ 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 4/4] drm/i915: Fix whitespace issues
2016-10-11 17:52 ` [PATCH 4/4] drm/i915: Fix whitespace issues ville.syrjala
@ 2016-10-13 18:18 ` Jim Bride
0 siblings, 0 replies; 33+ messages in thread
From: Jim Bride @ 2016-10-13 18:18 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On Tue, Oct 11, 2016 at 08:52:48PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Fix the poorly indented port parameters to the aux ctl and data
> reg functions. This was fallout from the s/i915_mmio_reg_t/i915_reg_t/
> that happened during the review of commit f0f59a00a1c9 ("drm/i915:
> Type safe register read/write")
>
> 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 | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b0753b272101..25cde7356b59 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1147,7 +1147,7 @@ static enum port intel_aux_port(struct drm_i915_private *dev_priv,
> }
>
> static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
> - enum port port)
> + enum port port)
> {
> switch (port) {
> case PORT_B:
> @@ -1161,7 +1161,7 @@ static i915_reg_t g4x_aux_ctl_reg(struct drm_i915_private *dev_priv,
> }
>
> static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
> - enum port port, int index)
> + enum port port, int index)
> {
> switch (port) {
> case PORT_B:
> @@ -1175,7 +1175,7 @@ static i915_reg_t g4x_aux_data_reg(struct drm_i915_private *dev_priv,
> }
>
> static i915_reg_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
> - enum port port)
> + enum port port)
> {
> switch (port) {
> case PORT_A:
> @@ -1191,7 +1191,7 @@ static i915_reg_t ilk_aux_ctl_reg(struct drm_i915_private *dev_priv,
> }
>
> static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv,
> - enum port port, int index)
> + enum port port, int index)
> {
> switch (port) {
> case PORT_A:
> @@ -1207,7 +1207,7 @@ static i915_reg_t ilk_aux_data_reg(struct drm_i915_private *dev_priv,
> }
>
> static i915_reg_t skl_aux_ctl_reg(struct drm_i915_private *dev_priv,
> - enum port port)
> + enum port port)
> {
> switch (port) {
> case PORT_A:
> @@ -1222,7 +1222,7 @@ 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)
> + enum port port, int index)
> {
> switch (port) {
> case PORT_A:
> @@ -1237,7 +1237,7 @@ static i915_reg_t skl_aux_data_reg(struct drm_i915_private *dev_priv,
> }
>
> static i915_reg_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv,
> - enum port port)
> + enum port port)
> {
> if (INTEL_INFO(dev_priv)->gen >= 9)
> return skl_aux_ctl_reg(dev_priv, port);
> @@ -1248,7 +1248,7 @@ static i915_reg_t intel_aux_ctl_reg(struct drm_i915_private *dev_priv,
> }
>
> static i915_reg_t intel_aux_data_reg(struct drm_i915_private *dev_priv,
> - enum port port, int index)
> + enum port port, int index)
> {
> if (INTEL_INFO(dev_priv)->gen >= 9)
> return skl_aux_data_reg(dev_priv, port, index);
> --
> 2.7.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
2016-10-13 18:06 ` Jim Bride
@ 2016-10-13 18:29 ` Ville Syrjälä
-1 siblings, 0 replies; 33+ 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] 33+ messages in thread
* Re: [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
@ 2016-10-13 18:29 ` Ville Syrjälä
0 siblings, 0 replies; 33+ 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ 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
-1 siblings, 0 replies; 33+ 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] 33+ messages in thread
* Re: [Intel-gfx] [PATCH 2/4] drm/i915: Respect alternate_ddc_pin for all DDI ports
@ 2016-10-17 6:50 ` Daniel Vetter
0 siblings, 0 replies; 33+ 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] 33+ messages in thread
* Re: [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
2016-10-11 17:52 ` ville.syrjala
@ 2016-10-17 18:00 ` Ville Syrjälä
-1 siblings, 0 replies; 33+ 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] 33+ messages in thread
* Re: [PATCH 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
@ 2016-10-17 18:00 ` Ville Syrjälä
0 siblings, 0 replies; 33+ 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] 33+ messages in thread
* [PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
2016-10-11 17:52 ` ville.syrjala
@ 2016-10-17 18:07 ` ville.syrjala
-1 siblings, 0 replies; 33+ 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] 33+ messages in thread
* [PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
@ 2016-10-17 18:07 ` ville.syrjala
0 siblings, 0 replies; 33+ messages in thread
From: ville.syrjala @ 2016-10-17 18:07 UTC (permalink / raw)
To: intel-gfx; +Cc: Maarten Maathuis, stable
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 33+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Trust VBT aux/ddc information (rev2)
2016-10-11 17:52 [PATCH 0/4] drm/i915: Trust VBT aux/ddc information ville.syrjala
` (4 preceding siblings ...)
2016-10-11 18:49 ` ✗ Fi.CI.BAT: warning for drm/i915: Trust VBT aux/ddc information Patchwork
@ 2016-10-17 18:54 ` Patchwork
5 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2016-10-17 18:54 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Trust VBT aux/ddc information (rev2)
URL : https://patchwork.freedesktop.org/series/13600/
State : failure
== Summary ==
Series 13600v2 drm/i915: Trust VBT aux/ddc information
https://patchwork.freedesktop.org/api/1.0/series/13600/revisions/2/mbox/
Test gem_exec_suspend:
Subgroup basic-s3:
incomplete -> DMESG-WARN (fi-byt-n2820)
dmesg-warn -> INCOMPLETE (fi-byt-j1900)
Test pm_backlight:
Subgroup basic-brightness:
skip -> PASS (fi-skl-6700k)
Test vgem_basic:
Subgroup unload:
pass -> SKIP (fi-bdw-5557u)
skip -> PASS (fi-hsw-4770r)
fi-bdw-5557u total:246 pass:230 dwarn:0 dfail:0 fail:0 skip:16
fi-bsw-n3050 total:246 pass:203 dwarn:1 dfail:0 fail:0 skip:42
fi-bxt-t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30
fi-byt-j1900 total:76 pass:62 dwarn:0 dfail:0 fail:1 skip:12
fi-byt-n2820 total:246 pass:209 dwarn:1 dfail:0 fail:1 skip:35
fi-hsw-4770 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
fi-hsw-4770r total:246 pass:223 dwarn:1 dfail:0 fail:0 skip:22
fi-ilk-650 total:246 pass:181 dwarn:0 dfail:0 fail:5 skip:60
fi-ivb-3770 total:246 pass:221 dwarn:0 dfail:0 fail:0 skip:25
fi-skl-6700hq total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23
fi-skl-6700k total:246 pass:222 dwarn:1 dfail:0 fail:0 skip:23
fi-snb-2600 total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37
fi-ivb-3520m failed to connect after reboot
fi-skl-6260u failed to connect after reboot
fi-skl-6770hq failed to connect after reboot
fi-snb-2520m failed to connect after reboot
Results at /archive/results/CI_IGT_test/Patchwork_2739/
7ec75289774dec48c46c37271fb334b7caed3d32 drm-intel-nightly: 2016y-10m-17d-14h-07m-32s UTC integration manifest
13ae602 drm/i915: Fix whitespace issues
ebded98 drm/i915: Clean up DDI DDC/AUX CH sanitation
6ea77f6 drm/i915: Respect alternate_ddc_pin for all DDI ports
49a7f8a drm/i915: Respect alternate_aux_channel for all DDI ports
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
2016-10-17 18:07 ` ville.syrjala
(?)
@ 2016-10-20 21:53 ` Maarten Maathuis
2016-10-20 22:00 ` Maarten Maathuis
-1 siblings, 1 reply; 33+ messages in thread
From: Maarten Maathuis @ 2016-10-20 21:53 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 9076 bytes --]
Also tested v3 on top of 4.8.3 (mainline git is a mess right now for
booting).
I did encounter a seemingly unrelated message during boot (including a
WARN_ON):
[drm:skylake_pfit_enable [i915]] *ERROR* Requesting pfit without getting a
scaler first
I suspect any causal relation with these patches.
On Mon, Oct 17, 2016 at 8:07 PM, <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
> 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
>
>
--
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.
[-- Attachment #1.2: Type: text/html, Size: 12078 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
2016-10-20 21:53 ` Maarten Maathuis
@ 2016-10-20 22:00 ` Maarten Maathuis
2016-10-21 13:17 ` Ville Syrjälä
0 siblings, 1 reply; 33+ messages in thread
From: Maarten Maathuis @ 2016-10-20 22:00 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 9566 bytes --]
I meant DON't suspect
On Thu, Oct 20, 2016 at 11:53 PM, Maarten Maathuis <madman2003@gmail.com>
wrote:
> Also tested v3 on top of 4.8.3 (mainline git is a mess right now for
> booting).
>
> I did encounter a seemingly unrelated message during boot (including a
> WARN_ON):
> [drm:skylake_pfit_enable [i915]] *ERROR* Requesting pfit without getting a
> scaler first
>
> I suspect any causal relation with these patches.
>
> On Mon, Oct 17, 2016 at 8:07 PM, <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
>> 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
>>
>>
>
>
> --
> 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.
>
--
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.
[-- Attachment #1.2: Type: text/html, Size: 12999 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] drm/i915: Clean up DDI DDC/AUX CH sanitation
2016-10-20 22:00 ` Maarten Maathuis
@ 2016-10-21 13:17 ` Ville Syrjälä
0 siblings, 0 replies; 33+ messages in thread
From: Ville Syrjälä @ 2016-10-21 13:17 UTC (permalink / raw)
To: Maarten Maathuis; +Cc: intel-gfx
On Fri, Oct 21, 2016 at 12:00:56AM +0200, Maarten Maathuis wrote:
> I meant DON't suspect
>
> On Thu, Oct 20, 2016 at 11:53 PM, Maarten Maathuis <madman2003@gmail.com>
> wrote:
>
> > Also tested v3 on top of 4.8.3 (mainline git is a mess right now for
> > booting).
> >
> > I did encounter a seemingly unrelated message during boot (including a
> > WARN_ON):
> > [drm:skylake_pfit_enable [i915]] *ERROR* Requesting pfit without getting a
> > scaler first
> >
> > I suspect any causal relation with these patches.
Yeah shouldn't be related.
I've pushed the patches to dinq. Thanks for testing and reviews.
> >
> > On Mon, Oct 17, 2016 at 8:07 PM, <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
> >> 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
> >>
> >>
> >
> >
> > --
> > 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.
> >
>
>
>
> --
> Far away from the primal instinct, the song seems to fade away, the river
> get wider between your thoughts and the things we do and say.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2016-10-21 13:18 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-11 17:52 [PATCH 0/4] drm/i915: Trust VBT aux/ddc information ville.syrjala
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
2016-10-13 17:59 ` [Intel-gfx] " Jim Bride
2016-10-13 17:59 ` Jim Bride
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-11 20:04 ` Maarten Maathuis
2016-10-12 10:57 ` Ville Syrjälä
2016-10-12 10:57 ` Ville Syrjälä
2016-10-12 16:56 ` Maarten Maathuis
2016-10-13 18:06 ` [Intel-gfx] " Jim Bride
2016-10-13 18:06 ` Jim Bride
2016-10-13 18:29 ` [Intel-gfx] " Ville Syrjälä
2016-10-13 18:29 ` Ville Syrjälä
2016-10-17 6:50 ` [Intel-gfx] " Daniel Vetter
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-11 17:52 ` ville.syrjala
2016-10-13 18:17 ` [Intel-gfx] " Jim Bride
2016-10-13 18:17 ` Jim Bride
2016-10-17 18:00 ` Ville Syrjälä
2016-10-17 18:00 ` Ville Syrjälä
2016-10-17 18:07 ` [PATCH v3 " ville.syrjala
2016-10-17 18:07 ` ville.syrjala
2016-10-20 21:53 ` Maarten Maathuis
2016-10-20 22:00 ` Maarten Maathuis
2016-10-21 13:17 ` Ville Syrjälä
2016-10-11 17:52 ` [PATCH 4/4] drm/i915: Fix whitespace issues ville.syrjala
2016-10-13 18:18 ` Jim Bride
2016-10-11 18:49 ` ✗ Fi.CI.BAT: warning for drm/i915: Trust VBT aux/ddc information Patchwork
2016-10-12 10:54 ` Ville Syrjälä
2016-10-17 18:54 ` ✗ Fi.CI.BAT: failure for drm/i915: Trust VBT aux/ddc information (rev2) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.