* [PATCH 1/3] drm/i915: check the DDC and AUX bits of the VBT on DDI machines
2013-08-28 15:45 [PATCH 0/3] More Haswell VBT code Paulo Zanoni
@ 2013-08-28 15:45 ` Paulo Zanoni
2013-09-03 15:01 ` Rodrigo Vivi
2013-08-28 15:45 ` [PATCH 2/3] drm/i915: add some assertions about VBT DDI port types Paulo Zanoni
2013-08-28 15:45 ` [PATCH 3/3] drm/i915: don't init DP or HDMI when not supported by DDI port Paulo Zanoni
2 siblings, 1 reply; 8+ messages in thread
From: Paulo Zanoni @ 2013-08-28 15:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Our code currently assumes that port X will use the DP AUX channel X
and the DDC pin X. The VBT should tell us how things are mapped, so
add some WARNs in case we discover our assumptions are wrong (or in
case the VBT is just wrong, which is also perfectly possible).
Why would someone wire port B to AUX C and DDC D?
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_bios.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 79bb651..a6700ab 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -575,6 +575,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
uint8_t hdmi_level_shift;
int i, j;
+ bool is_dvi, is_dp;
+ uint8_t aux_channel;
int dvo_ports[][2] = {
{0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0},
};
@@ -598,6 +600,31 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
if (!child)
return;
+ aux_channel = child->raw[25];
+
+ is_dvi = child->common.device_type & (1 << 4);
+ is_dp = child->common.device_type & (1 << 2);
+
+ if (is_dvi) {
+ WARN(child->common.ddc_pin == 0x05 && port != PORT_B,
+ "Unexpected DDC pin for port B\n");
+ WARN(child->common.ddc_pin == 0x04 && port != PORT_C,
+ "Unexpected DDC pin for port C\n");
+ WARN(child->common.ddc_pin == 0x06 && port != PORT_D,
+ "Unexpected DDC pin for port D\n");
+ }
+
+ if (is_dp) {
+ WARN(aux_channel == 0x40 && port != PORT_A,
+ "Unexpected AUX channel for port A\n");
+ WARN(aux_channel == 0x10 && port != PORT_B,
+ "Unexpected AUX channel for port B\n");
+ WARN(aux_channel == 0x20 && port != PORT_C,
+ "Unexpected AUX channel for port C\n");
+ WARN(aux_channel == 0x30 && port != PORT_D,
+ "Unexpected AUX channel for port D\n");
+ }
+
if (bdb->version >= 158) {
/* The VBT HDMI level shift values match the table we have. */
hdmi_level_shift = child->raw[7] & 0xF;
--
1.8.1.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/3] drm/i915: check the DDC and AUX bits of the VBT on DDI machines
2013-08-28 15:45 ` [PATCH 1/3] drm/i915: check the DDC and AUX bits of the VBT on DDI machines Paulo Zanoni
@ 2013-09-03 15:01 ` Rodrigo Vivi
0 siblings, 0 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2013-09-03 15:01 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
On Wed, Aug 28, 2013 at 12:45 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Our code currently assumes that port X will use the DP AUX channel X
> and the DDC pin X. The VBT should tell us how things are mapped, so
> add some WARNs in case we discover our assumptions are wrong (or in
> case the VBT is just wrong, which is also perfectly possible).
>
> Why would someone wire port B to AUX C and DDC D?
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_bios.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 79bb651..a6700ab 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -575,6 +575,8 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
> uint8_t hdmi_level_shift;
> int i, j;
> + bool is_dvi, is_dp;
> + uint8_t aux_channel;
> int dvo_ports[][2] = {
> {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0},
> };
> @@ -598,6 +600,31 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> if (!child)
> return;
>
> + aux_channel = child->raw[25];
> +
> + is_dvi = child->common.device_type & (1 << 4);
> + is_dp = child->common.device_type & (1 << 2);
> +
> + if (is_dvi) {
> + WARN(child->common.ddc_pin == 0x05 && port != PORT_B,
> + "Unexpected DDC pin for port B\n");
> + WARN(child->common.ddc_pin == 0x04 && port != PORT_C,
> + "Unexpected DDC pin for port C\n");
> + WARN(child->common.ddc_pin == 0x06 && port != PORT_D,
> + "Unexpected DDC pin for port D\n");
> + }
> +
> + if (is_dp) {
> + WARN(aux_channel == 0x40 && port != PORT_A,
> + "Unexpected AUX channel for port A\n");
> + WARN(aux_channel == 0x10 && port != PORT_B,
> + "Unexpected AUX channel for port B\n");
> + WARN(aux_channel == 0x20 && port != PORT_C,
> + "Unexpected AUX channel for port C\n");
> + WARN(aux_channel == 0x30 && port != PORT_D,
> + "Unexpected AUX channel for port D\n");
> + }
> +
> if (bdb->version >= 158) {
> /* The VBT HDMI level shift values match the table we have. */
> hdmi_level_shift = child->raw[7] & 0xF;
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] drm/i915: add some assertions about VBT DDI port types
2013-08-28 15:45 [PATCH 0/3] More Haswell VBT code Paulo Zanoni
2013-08-28 15:45 ` [PATCH 1/3] drm/i915: check the DDC and AUX bits of the VBT on DDI machines Paulo Zanoni
@ 2013-08-28 15:45 ` Paulo Zanoni
2013-09-03 15:03 ` Rodrigo Vivi
2013-08-28 15:45 ` [PATCH 3/3] drm/i915: don't init DP or HDMI when not supported by DDI port Paulo Zanoni
2 siblings, 1 reply; 8+ messages in thread
From: Paulo Zanoni @ 2013-08-28 15:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Our code makes a lot of assumptions regarding what each DDI port
actually supports, and the VBT should tell us what is really happening
in the hardware. So parse the information provided by the VBT and
check if any of our assumptions is wrong.
Our driver also has a history of not really trusting the VBT, so a
WARN here could mean that:
a) our coding assumptions are wrong
b) the VBT is wrong
c) we're incorrectly parsing the VBT
d) the checks are wrong
But I really hope we won't ever trigger any of those WARNs.
v2: Don't check the redundant "Capabilities" field from byte 24 since
it doesn't seem to be used.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_bios.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index a6700ab..4003dbf 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -575,7 +575,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
uint8_t hdmi_level_shift;
int i, j;
- bool is_dvi, is_dp;
+ bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
uint8_t aux_channel;
int dvo_ports[][2] = {
{0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0},
@@ -604,6 +604,24 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
is_dvi = child->common.device_type & (1 << 4);
is_dp = child->common.device_type & (1 << 2);
+ is_crt = child->common.device_type & (1 << 0);
+ is_hdmi = is_dvi && (child->common.device_type & (1 << 11)) == 0;
+ is_edp = is_dp && (child->common.device_type & (1 << 12));
+
+ DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n",
+ port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt);
+
+ WARN(is_edp && is_dvi, "Internal DP port %c is TMDS compatible\n",
+ port_name(port));
+ WARN(is_crt && port != PORT_E, "Port %c is analog\n", port_name(port));
+ WARN(is_crt && (is_dvi || is_dp),
+ "Analog port %c is also DP or TMDS compatible\n", port_name(port));
+ WARN(is_dvi && (port == PORT_A || port == PORT_E),
+ "Port %c is TMDS compabile\n", port_name(port));
+ WARN(!is_dvi && !is_dp && !is_crt,
+ "Port %c is not DP/TMDS/CRT compatible\n", port_name(port));
+ WARN(is_edp && (port == PORT_B || port == PORT_C || port == PORT_E),
+ "Port %c is internal DP\n", port_name(port));
if (is_dvi) {
WARN(child->common.ddc_pin == 0x05 && port != PORT_B,
--
1.8.1.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] drm/i915: add some assertions about VBT DDI port types
2013-08-28 15:45 ` [PATCH 2/3] drm/i915: add some assertions about VBT DDI port types Paulo Zanoni
@ 2013-09-03 15:03 ` Rodrigo Vivi
0 siblings, 0 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2013-09-03 15:03 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
On Wed, Aug 28, 2013 at 12:45 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Our code makes a lot of assumptions regarding what each DDI port
> actually supports, and the VBT should tell us what is really happening
> in the hardware. So parse the information provided by the VBT and
> check if any of our assumptions is wrong.
>
> Our driver also has a history of not really trusting the VBT, so a
> WARN here could mean that:
> a) our coding assumptions are wrong
> b) the VBT is wrong
> c) we're incorrectly parsing the VBT
> d) the checks are wrong
>
> But I really hope we won't ever trigger any of those WARNs.
>
> v2: Don't check the redundant "Capabilities" field from byte 24 since
> it doesn't seem to be used.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_bios.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index a6700ab..4003dbf 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -575,7 +575,7 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> struct ddi_vbt_port_info *info = &dev_priv->vbt.ddi_port_info[port];
> uint8_t hdmi_level_shift;
> int i, j;
> - bool is_dvi, is_dp;
> + bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
> uint8_t aux_channel;
> int dvo_ports[][2] = {
> {0, 10}, {1, 7}, {2, 8}, {3, 9}, {6, /* Unused */ 0},
> @@ -604,6 +604,24 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>
> is_dvi = child->common.device_type & (1 << 4);
> is_dp = child->common.device_type & (1 << 2);
> + is_crt = child->common.device_type & (1 << 0);
> + is_hdmi = is_dvi && (child->common.device_type & (1 << 11)) == 0;
> + is_edp = is_dp && (child->common.device_type & (1 << 12));
> +
> + DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n",
> + port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt);
> +
> + WARN(is_edp && is_dvi, "Internal DP port %c is TMDS compatible\n",
> + port_name(port));
> + WARN(is_crt && port != PORT_E, "Port %c is analog\n", port_name(port));
> + WARN(is_crt && (is_dvi || is_dp),
> + "Analog port %c is also DP or TMDS compatible\n", port_name(port));
> + WARN(is_dvi && (port == PORT_A || port == PORT_E),
> + "Port %c is TMDS compabile\n", port_name(port));
> + WARN(!is_dvi && !is_dp && !is_crt,
> + "Port %c is not DP/TMDS/CRT compatible\n", port_name(port));
> + WARN(is_edp && (port == PORT_B || port == PORT_C || port == PORT_E),
> + "Port %c is internal DP\n", port_name(port));
>
> if (is_dvi) {
> WARN(child->common.ddc_pin == 0x05 && port != PORT_B,
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] drm/i915: don't init DP or HDMI when not supported by DDI port
2013-08-28 15:45 [PATCH 0/3] More Haswell VBT code Paulo Zanoni
2013-08-28 15:45 ` [PATCH 1/3] drm/i915: check the DDC and AUX bits of the VBT on DDI machines Paulo Zanoni
2013-08-28 15:45 ` [PATCH 2/3] drm/i915: add some assertions about VBT DDI port types Paulo Zanoni
@ 2013-08-28 15:45 ` Paulo Zanoni
2013-09-03 15:07 ` Rodrigo Vivi
2 siblings, 1 reply; 8+ messages in thread
From: Paulo Zanoni @ 2013-08-28 15:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
There's no reason to init a DP connector if the encoder just supports
HDMI: we'll just waste hundreds and hundreds of cycles trying to do DP
AUX transactions to detect if there's something there. Same goes for a
DP connector that doesn't support HDMI, but I'm not sure these
actually exist.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +++
drivers/gpu/drm/i915/intel_bios.c | 13 ++++++++++++-
drivers/gpu/drm/i915/intel_ddi.c | 30 ++++++++++++++++++++++--------
3 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 645dd74..8720f31 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1041,6 +1041,9 @@ enum modeset_restore {
struct ddi_vbt_port_info {
uint8_t hdmi_level_shift;
+ bool supports_dvi;
+ bool supports_hdmi;
+ bool supports_dp;
};
struct intel_vbt_data {
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 4003dbf..cd823b9 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -608,6 +608,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
is_hdmi = is_dvi && (child->common.device_type & (1 << 11)) == 0;
is_edp = is_dp && (child->common.device_type & (1 << 12));
+ info->supports_dvi = is_dvi;
+ info->supports_hdmi = is_hdmi;
+ info->supports_dp = is_dp;
+
DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n",
port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt);
@@ -762,8 +766,15 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
enum port port;
for (port = PORT_A; port <= PORT_E; port++) {
+ struct ddi_vbt_port_info *info =
+ &dev_priv->vbt.ddi_port_info[port];
+
/* Recommended BSpec default: 800mV 0dB. */
- dev_priv->vbt.ddi_port_info[port].hdmi_level_shift = 6;
+ info->hdmi_level_shift = 6;
+
+ info->supports_dvi = (port != PORT_A && port != PORT_E);
+ info->supports_hdmi = info->supports_dvi;
+ info->supports_dp = (port != PORT_E);
}
}
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ece226d..d344977 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1325,6 +1325,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
struct drm_encoder *encoder;
struct intel_connector *hdmi_connector = NULL;
struct intel_connector *dp_connector = NULL;
+ bool init_hdmi, init_dp;
+
+ init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
+ dev_priv->vbt.ddi_port_info[port].supports_hdmi);
+ init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
+ if (!init_dp && !init_hdmi) {
+ DRM_ERROR("VBT says port %c is not DVI/HDMI/DP compatible\n",
+ port_name(port));
+ init_hdmi = true;
+ init_dp = true;
+ }
intel_dig_port = kzalloc(sizeof(struct intel_digital_port), GFP_KERNEL);
if (!intel_dig_port)
@@ -1362,19 +1373,22 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
intel_encoder->cloneable = false;
intel_encoder->hot_plug = intel_ddi_hot_plug;
- if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
- drm_encoder_cleanup(encoder);
- kfree(intel_dig_port);
- kfree(dp_connector);
- return;
+ if (init_dp) {
+ if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
+ drm_encoder_cleanup(encoder);
+ kfree(intel_dig_port);
+ kfree(dp_connector);
+ return;
+ }
}
- if (intel_encoder->type != INTEL_OUTPUT_EDP) {
+ /* In theory we don't need the encoder->type check, but leave it just in
+ * case we have some really bad VBTs... */
+ if (intel_encoder->type != INTEL_OUTPUT_EDP && init_hdmi) {
hdmi_connector = kzalloc(sizeof(struct intel_connector),
GFP_KERNEL);
- if (!hdmi_connector) {
+ if (!hdmi_connector)
return;
- }
intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
intel_hdmi_init_connector(intel_dig_port, hdmi_connector);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 3/3] drm/i915: don't init DP or HDMI when not supported by DDI port
2013-08-28 15:45 ` [PATCH 3/3] drm/i915: don't init DP or HDMI when not supported by DDI port Paulo Zanoni
@ 2013-09-03 15:07 ` Rodrigo Vivi
2013-09-03 16:53 ` Paulo Zanoni
0 siblings, 1 reply; 8+ messages in thread
From: Rodrigo Vivi @ 2013-09-03 15:07 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
Do we really trust VBT that much?
Anyways,
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
On Wed, Aug 28, 2013 at 12:45 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> There's no reason to init a DP connector if the encoder just supports
> HDMI: we'll just waste hundreds and hundreds of cycles trying to do DP
> AUX transactions to detect if there's something there. Same goes for a
> DP connector that doesn't support HDMI, but I'm not sure these
> actually exist.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_bios.c | 13 ++++++++++++-
> drivers/gpu/drm/i915/intel_ddi.c | 30 ++++++++++++++++++++++--------
> 3 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 645dd74..8720f31 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1041,6 +1041,9 @@ enum modeset_restore {
>
> struct ddi_vbt_port_info {
> uint8_t hdmi_level_shift;
> + bool supports_dvi;
> + bool supports_hdmi;
> + bool supports_dp;
> };
>
> struct intel_vbt_data {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 4003dbf..cd823b9 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -608,6 +608,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
> is_hdmi = is_dvi && (child->common.device_type & (1 << 11)) == 0;
> is_edp = is_dp && (child->common.device_type & (1 << 12));
>
> + info->supports_dvi = is_dvi;
> + info->supports_hdmi = is_hdmi;
> + info->supports_dp = is_dp;
> +
> DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n",
> port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt);
>
> @@ -762,8 +766,15 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
> enum port port;
>
> for (port = PORT_A; port <= PORT_E; port++) {
> + struct ddi_vbt_port_info *info =
> + &dev_priv->vbt.ddi_port_info[port];
> +
> /* Recommended BSpec default: 800mV 0dB. */
> - dev_priv->vbt.ddi_port_info[port].hdmi_level_shift = 6;
> + info->hdmi_level_shift = 6;
> +
> + info->supports_dvi = (port != PORT_A && port != PORT_E);
> + info->supports_hdmi = info->supports_dvi;
> + info->supports_dp = (port != PORT_E);
> }
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index ece226d..d344977 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1325,6 +1325,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> struct drm_encoder *encoder;
> struct intel_connector *hdmi_connector = NULL;
> struct intel_connector *dp_connector = NULL;
> + bool init_hdmi, init_dp;
> +
> + init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
> + dev_priv->vbt.ddi_port_info[port].supports_hdmi);
> + init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
> + if (!init_dp && !init_hdmi) {
> + DRM_ERROR("VBT says port %c is not DVI/HDMI/DP compatible\n",
> + port_name(port));
> + init_hdmi = true;
> + init_dp = true;
> + }
>
> intel_dig_port = kzalloc(sizeof(struct intel_digital_port), GFP_KERNEL);
> if (!intel_dig_port)
> @@ -1362,19 +1373,22 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> intel_encoder->cloneable = false;
> intel_encoder->hot_plug = intel_ddi_hot_plug;
>
> - if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
> - drm_encoder_cleanup(encoder);
> - kfree(intel_dig_port);
> - kfree(dp_connector);
> - return;
> + if (init_dp) {
> + if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
> + drm_encoder_cleanup(encoder);
> + kfree(intel_dig_port);
> + kfree(dp_connector);
> + return;
> + }
> }
>
> - if (intel_encoder->type != INTEL_OUTPUT_EDP) {
> + /* In theory we don't need the encoder->type check, but leave it just in
> + * case we have some really bad VBTs... */
> + if (intel_encoder->type != INTEL_OUTPUT_EDP && init_hdmi) {
> hdmi_connector = kzalloc(sizeof(struct intel_connector),
> GFP_KERNEL);
> - if (!hdmi_connector) {
> + if (!hdmi_connector)
> return;
> - }
>
> intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
> intel_hdmi_init_connector(intel_dig_port, hdmi_connector);
> --
> 1.8.1.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 3/3] drm/i915: don't init DP or HDMI when not supported by DDI port
2013-09-03 15:07 ` Rodrigo Vivi
@ 2013-09-03 16:53 ` Paulo Zanoni
0 siblings, 0 replies; 8+ messages in thread
From: Paulo Zanoni @ 2013-09-03 16:53 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
2013/9/3 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> Do we really trust VBT that much?
We already trust it for some very important things, like eDP. Also, if
the VBT is wrong we'll start seeing all sorts of WARNs due to the
previous patches. Anyway, we can always revert...
Thanks for the reviews!
>
> Anyways,
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>
> On Wed, Aug 28, 2013 at 12:45 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> There's no reason to init a DP connector if the encoder just supports
>> HDMI: we'll just waste hundreds and hundreds of cycles trying to do DP
>> AUX transactions to detect if there's something there. Same goes for a
>> DP connector that doesn't support HDMI, but I'm not sure these
>> actually exist.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 3 +++
>> drivers/gpu/drm/i915/intel_bios.c | 13 ++++++++++++-
>> drivers/gpu/drm/i915/intel_ddi.c | 30 ++++++++++++++++++++++--------
>> 3 files changed, 37 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 645dd74..8720f31 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1041,6 +1041,9 @@ enum modeset_restore {
>>
>> struct ddi_vbt_port_info {
>> uint8_t hdmi_level_shift;
>> + bool supports_dvi;
>> + bool supports_hdmi;
>> + bool supports_dp;
>> };
>>
>> struct intel_vbt_data {
>> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
>> index 4003dbf..cd823b9 100644
>> --- a/drivers/gpu/drm/i915/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/intel_bios.c
>> @@ -608,6 +608,10 @@ static void parse_ddi_port(struct drm_i915_private *dev_priv, enum port port,
>> is_hdmi = is_dvi && (child->common.device_type & (1 << 11)) == 0;
>> is_edp = is_dp && (child->common.device_type & (1 << 12));
>>
>> + info->supports_dvi = is_dvi;
>> + info->supports_hdmi = is_hdmi;
>> + info->supports_dp = is_dp;
>> +
>> DRM_DEBUG_KMS("Port %c VBT info: DP:%d HDMI:%d DVI:%d EDP:%d CRT:%d\n",
>> port_name(port), is_dp, is_hdmi, is_dvi, is_edp, is_crt);
>>
>> @@ -762,8 +766,15 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
>> enum port port;
>>
>> for (port = PORT_A; port <= PORT_E; port++) {
>> + struct ddi_vbt_port_info *info =
>> + &dev_priv->vbt.ddi_port_info[port];
>> +
>> /* Recommended BSpec default: 800mV 0dB. */
>> - dev_priv->vbt.ddi_port_info[port].hdmi_level_shift = 6;
>> + info->hdmi_level_shift = 6;
>> +
>> + info->supports_dvi = (port != PORT_A && port != PORT_E);
>> + info->supports_hdmi = info->supports_dvi;
>> + info->supports_dp = (port != PORT_E);
>> }
>> }
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index ece226d..d344977 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -1325,6 +1325,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>> struct drm_encoder *encoder;
>> struct intel_connector *hdmi_connector = NULL;
>> struct intel_connector *dp_connector = NULL;
>> + bool init_hdmi, init_dp;
>> +
>> + init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
>> + dev_priv->vbt.ddi_port_info[port].supports_hdmi);
>> + init_dp = dev_priv->vbt.ddi_port_info[port].supports_dp;
>> + if (!init_dp && !init_hdmi) {
>> + DRM_ERROR("VBT says port %c is not DVI/HDMI/DP compatible\n",
>> + port_name(port));
>> + init_hdmi = true;
>> + init_dp = true;
>> + }
>>
>> intel_dig_port = kzalloc(sizeof(struct intel_digital_port), GFP_KERNEL);
>> if (!intel_dig_port)
>> @@ -1362,19 +1373,22 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>> intel_encoder->cloneable = false;
>> intel_encoder->hot_plug = intel_ddi_hot_plug;
>>
>> - if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
>> - drm_encoder_cleanup(encoder);
>> - kfree(intel_dig_port);
>> - kfree(dp_connector);
>> - return;
>> + if (init_dp) {
>> + if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
>> + drm_encoder_cleanup(encoder);
>> + kfree(intel_dig_port);
>> + kfree(dp_connector);
>> + return;
>> + }
>> }
>>
>> - if (intel_encoder->type != INTEL_OUTPUT_EDP) {
>> + /* In theory we don't need the encoder->type check, but leave it just in
>> + * case we have some really bad VBTs... */
>> + if (intel_encoder->type != INTEL_OUTPUT_EDP && init_hdmi) {
>> hdmi_connector = kzalloc(sizeof(struct intel_connector),
>> GFP_KERNEL);
>> - if (!hdmi_connector) {
>> + if (!hdmi_connector)
>> return;
>> - }
>>
>> intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
>> intel_hdmi_init_connector(intel_dig_port, hdmi_connector);
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 8+ messages in thread