* [PATCH 0/8] Don't let the ghost eDP haunt us
@ 2013-06-12 20:27 Paulo Zanoni
2013-06-12 20:27 ` [PATCH 1/8] drm/i915: don't check encoder at DP connector destroy() Paulo Zanoni
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-06-12 20:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Hi
This week I sent a patch called "drm/i915: propagate errors from
intel_dp_init_connector" to fix a Haswell bug that was preventing my machine
from booting. Chris provided a nice suggestion for it, so this series should
implement his suggestion, but split in a few patches. While writing this code I
also spotted a few other things, so there are some patches for them here. Patch
3 is the real bug fix and patches 4 & 5 implement Chris's suggestions. Patch 1
is another bug fix and patches 2, 6, 7 & 8 are my bikesheds for the current
code.
Thanks,
Paulo
Paulo Zanoni (8):
drm/i915: don't check encoder at DP connector destroy()
drm/i915: extract intel_edp_init_connector
drm/i915: propagate errors from intel_dp_init_connector
drm/i915: fix the "ghost eDP" connector unwind path
drm/i915: fix the "ghost eDP" encoder unwind path
drm/i915: check the return value of intel_dp_i2c_init
drm/i915: invert the verbosity of intel_enable_fbc
drm/i915: rename intel_dp_destroy to intel_dp_connector_destroy
drivers/gpu/drm/i915/intel_ddi.c | 7 +-
drivers/gpu/drm/i915/intel_dp.c | 180 +++++++++++++++++++++++----------------
drivers/gpu/drm/i915/intel_drv.h | 2 +-
drivers/gpu/drm/i915/intel_pm.c | 3 +-
4 files changed, 113 insertions(+), 79 deletions(-)
--
1.8.1.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/8] drm/i915: don't check encoder at DP connector destroy()
2013-06-12 20:27 [PATCH 0/8] Don't let the ghost eDP haunt us Paulo Zanoni
@ 2013-06-12 20:27 ` Paulo Zanoni
2013-06-12 20:27 ` [PATCH 2/8] drm/i915: extract intel_edp_init_connector Paulo Zanoni
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-06-12 20:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
By the time we call intel_dp_destroy (which destroys the connector)
the encoder may have been destroyed already, so if we use it we may be
reading some free memory. That happens in drm_mode_config_cleanup()
and also inside intel_dp_init_connector() when we detect a ghost eDP.
I also hope this may solve some random memory bugs.
Reported-by kmemcheck.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6d56075..dbfb32d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2683,13 +2683,14 @@ done:
static void
intel_dp_destroy(struct drm_connector *connector)
{
- struct intel_dp *intel_dp = intel_attached_dp(connector);
struct intel_connector *intel_connector = to_intel_connector(connector);
if (!IS_ERR_OR_NULL(intel_connector->edid))
kfree(intel_connector->edid);
- if (is_edp(intel_dp))
+ /* Can't call is_edp() since the encoder may have been destroyed
+ * already. */
+ if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
intel_panel_fini(&intel_connector->panel);
drm_sysfs_connector_remove(connector);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/8] drm/i915: extract intel_edp_init_connector
2013-06-12 20:27 [PATCH 0/8] Don't let the ghost eDP haunt us Paulo Zanoni
2013-06-12 20:27 ` [PATCH 1/8] drm/i915: don't check encoder at DP connector destroy() Paulo Zanoni
@ 2013-06-12 20:27 ` Paulo Zanoni
2013-06-12 20:27 ` [PATCH 3/8] drm/i915: propagate errors from intel_dp_init_connector Paulo Zanoni
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-06-12 20:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Because intel_dp_init_connector is too big for my poor little brain.
No functional changes.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 151 ++++++++++++++++++++++------------------
1 file changed, 82 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dbfb32d..90b01f5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2955,6 +2955,86 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
I915_READ(pp_div_reg));
}
+static bool intel_edp_init_connector(struct intel_dp *intel_dp,
+ struct intel_connector *intel_connector)
+{
+ struct drm_connector *connector = &intel_connector->base;
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_display_mode *fixed_mode = NULL;
+ struct edp_power_seq power_seq = { 0 };
+ bool has_dpcd;
+ struct drm_display_mode *scan;
+ struct edid *edid;
+
+ if (!is_edp(intel_dp))
+ return true;
+
+ intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
+
+ /* Cache DPCD and EDID for edp. */
+ ironlake_edp_panel_vdd_on(intel_dp);
+ has_dpcd = intel_dp_get_dpcd(intel_dp);
+ ironlake_edp_panel_vdd_off(intel_dp, false);
+
+ if (has_dpcd) {
+ if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11)
+ dev_priv->no_aux_handshake =
+ intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
+ DP_NO_AUX_HANDSHAKE_LINK_TRAINING;
+ } else {
+ /* if this fails, presume the device is a ghost */
+ DRM_INFO("failed to retrieve link info, disabling eDP\n");
+ intel_dp_encoder_destroy(&intel_dig_port->base.base);
+ intel_dp_destroy(connector);
+ return false;
+ }
+
+ /* We now know it's not a ghost, init power sequence regs. */
+ intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
+ &power_seq);
+
+ ironlake_edp_panel_vdd_on(intel_dp);
+ edid = drm_get_edid(connector, &intel_dp->adapter);
+ if (edid) {
+ if (drm_add_edid_modes(connector, edid)) {
+ drm_mode_connector_update_edid_property(connector,
+ edid);
+ drm_edid_to_eld(connector, edid);
+ } else {
+ kfree(edid);
+ edid = ERR_PTR(-EINVAL);
+ }
+ } else {
+ edid = ERR_PTR(-ENOENT);
+ }
+ intel_connector->edid = edid;
+
+ /* prefer fixed mode from EDID if available */
+ list_for_each_entry(scan, &connector->probed_modes, head) {
+ if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
+ fixed_mode = drm_mode_duplicate(dev, scan);
+ break;
+ }
+ }
+
+ /* fallback to VBT if available for eDP */
+ if (!fixed_mode && dev_priv->vbt.lfp_lvds_vbt_mode) {
+ fixed_mode = drm_mode_duplicate(dev,
+ dev_priv->vbt.lfp_lvds_vbt_mode);
+ if (fixed_mode)
+ fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
+ }
+
+ ironlake_edp_panel_vdd_off(intel_dp, false);
+
+ intel_panel_init(&intel_connector->panel, fixed_mode);
+ intel_panel_setup_backlight(connector);
+
+ return true;
+}
+
void
intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
struct intel_connector *intel_connector)
@@ -2964,8 +3044,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct drm_device *dev = intel_encoder->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_display_mode *fixed_mode = NULL;
- struct edp_power_seq power_seq = { 0 };
enum port port = intel_dig_port->port;
const char *name = NULL;
int type;
@@ -3066,75 +3144,10 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
BUG();
}
- if (is_edp(intel_dp))
- intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
-
intel_dp_i2c_init(intel_dp, intel_connector, name);
- /* Cache DPCD and EDID for edp. */
- if (is_edp(intel_dp)) {
- bool ret;
- struct drm_display_mode *scan;
- struct edid *edid;
-
- ironlake_edp_panel_vdd_on(intel_dp);
- ret = intel_dp_get_dpcd(intel_dp);
- ironlake_edp_panel_vdd_off(intel_dp, false);
-
- if (ret) {
- if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11)
- dev_priv->no_aux_handshake =
- intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
- DP_NO_AUX_HANDSHAKE_LINK_TRAINING;
- } else {
- /* if this fails, presume the device is a ghost */
- DRM_INFO("failed to retrieve link info, disabling eDP\n");
- intel_dp_encoder_destroy(&intel_encoder->base);
- intel_dp_destroy(connector);
- return;
- }
-
- /* We now know it's not a ghost, init power sequence regs. */
- intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
- &power_seq);
-
- ironlake_edp_panel_vdd_on(intel_dp);
- edid = drm_get_edid(connector, &intel_dp->adapter);
- if (edid) {
- if (drm_add_edid_modes(connector, edid)) {
- drm_mode_connector_update_edid_property(connector, edid);
- drm_edid_to_eld(connector, edid);
- } else {
- kfree(edid);
- edid = ERR_PTR(-EINVAL);
- }
- } else {
- edid = ERR_PTR(-ENOENT);
- }
- intel_connector->edid = edid;
-
- /* prefer fixed mode from EDID if available */
- list_for_each_entry(scan, &connector->probed_modes, head) {
- if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
- fixed_mode = drm_mode_duplicate(dev, scan);
- break;
- }
- }
-
- /* fallback to VBT if available for eDP */
- if (!fixed_mode && dev_priv->vbt.lfp_lvds_vbt_mode) {
- fixed_mode = drm_mode_duplicate(dev, dev_priv->vbt.lfp_lvds_vbt_mode);
- if (fixed_mode)
- fixed_mode->type |= DRM_MODE_TYPE_PREFERRED;
- }
-
- ironlake_edp_panel_vdd_off(intel_dp, false);
- }
-
- if (is_edp(intel_dp)) {
- intel_panel_init(&intel_connector->panel, fixed_mode);
- intel_panel_setup_backlight(connector);
- }
+ if (!intel_edp_init_connector(intel_dp, intel_connector))
+ return;
intel_dp_add_properties(intel_dp, connector);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/8] drm/i915: propagate errors from intel_dp_init_connector
2013-06-12 20:27 [PATCH 0/8] Don't let the ghost eDP haunt us Paulo Zanoni
2013-06-12 20:27 ` [PATCH 1/8] drm/i915: don't check encoder at DP connector destroy() Paulo Zanoni
2013-06-12 20:27 ` [PATCH 2/8] drm/i915: extract intel_edp_init_connector Paulo Zanoni
@ 2013-06-12 20:27 ` Paulo Zanoni
2013-06-12 20:27 ` [PATCH 4/8] drm/i915: fix the "ghost eDP" connector unwind path Paulo Zanoni
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-06-12 20:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
In case we detect a "ghost eDP", intel_edp_init_connector frees both
the connector and encoder and then returns. On Haswell, intel_ddi_init
then tries to use the freed encoder on the HDMI initialization path
since the following commit:
commit 21a8e6a4853b2ed39fa4c5188a710f2cf1b92026
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date: Wed Apr 10 23:28:35 2013 +0200
drm/i915: don't setup hdmi for port D edp in ddi_init
So now on intel_ddi_init we check for the "ghost eDP" case and return
without trying to initialize HDMI. This way we won't try to read the
freed "intel_encoder" struct in the next "if" statement.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_ddi.c | 3 ++-
drivers/gpu/drm/i915/intel_dp.c | 6 ++++--
drivers/gpu/drm/i915/intel_drv.h | 2 +-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 224ce25..0f835d1 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1356,7 +1356,8 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
intel_encoder->cloneable = false;
intel_encoder->hot_plug = intel_ddi_hot_plug;
- intel_dp_init_connector(intel_dig_port, dp_connector);
+ if (!intel_dp_init_connector(intel_dig_port, dp_connector))
+ return;
if (intel_encoder->type != INTEL_OUTPUT_EDP) {
hdmi_connector = kzalloc(sizeof(struct intel_connector),
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 90b01f5..46b3e3b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3035,7 +3035,7 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
return true;
}
-void
+bool
intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
struct intel_connector *intel_connector)
{
@@ -3147,7 +3147,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
intel_dp_i2c_init(intel_dp, intel_connector, name);
if (!intel_edp_init_connector(intel_dp, intel_connector))
- return;
+ return false;
intel_dp_add_properties(intel_dp, connector);
@@ -3159,6 +3159,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
u32 temp = I915_READ(PEG_BAND_GAP_DATA);
I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
}
+
+ return true;
}
void
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 02e5d65..d100bee 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -586,7 +586,7 @@ extern bool intel_lvds_init(struct drm_device *dev);
extern bool intel_is_dual_link_lvds(struct drm_device *dev);
extern void intel_dp_init(struct drm_device *dev, int output_reg,
enum port port);
-extern void intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
+extern bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
struct intel_connector *intel_connector);
extern void intel_dp_init_link_config(struct intel_dp *intel_dp);
extern void intel_dp_start_link_train(struct intel_dp *intel_dp);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/8] drm/i915: fix the "ghost eDP" connector unwind path
2013-06-12 20:27 [PATCH 0/8] Don't let the ghost eDP haunt us Paulo Zanoni
` (2 preceding siblings ...)
2013-06-12 20:27 ` [PATCH 3/8] drm/i915: propagate errors from intel_dp_init_connector Paulo Zanoni
@ 2013-06-12 20:27 ` Paulo Zanoni
2013-06-12 20:27 ` [PATCH 5/8] drm/i915: fix the "ghost eDP" encoder " Paulo Zanoni
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-06-12 20:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Because calling intel_dp_destroy inside intel_edp_init_connector is
just wrong. This is the initialization path, so we should properly
unwind all the initialization through the whole caller stack.
On the intel_dp_destroy function we do the following:
1 - Free edid if it exists
2 - Call intel_panel_fini in case it's eDP
3 - Call drm_sysfs_connector_remove
4 - Call drm_connector_cleanup
5 - Free the connector
And here is how we unwind each specific step:
1 - No need as we still didn't assign anything
2 - No need as we still didn't call intel_panel_init
3 - Call it in the same function that called drm_sysfs_connector_add
4 - Call it in the same function that called drm_connector_init
5 - Free it in the same function that allocated it
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_ddi.c | 4 +++-
drivers/gpu/drm/i915/intel_dp.c | 9 ++++++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0f835d1..aed363c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1356,8 +1356,10 @@ 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))
+ if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
+ kfree(dp_connector);
return;
+ }
if (intel_encoder->type != INTEL_OUTPUT_EDP) {
hdmi_connector = kzalloc(sizeof(struct intel_connector),
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 46b3e3b..da7460c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2987,7 +2987,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
/* if this fails, presume the device is a ghost */
DRM_INFO("failed to retrieve link info, disabling eDP\n");
intel_dp_encoder_destroy(&intel_dig_port->base.base);
- intel_dp_destroy(connector);
return false;
}
@@ -3146,8 +3145,11 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
intel_dp_i2c_init(intel_dp, intel_connector, name);
- if (!intel_edp_init_connector(intel_dp, intel_connector))
+ if (!intel_edp_init_connector(intel_dp, intel_connector)) {
+ drm_sysfs_connector_remove(connector);
+ drm_connector_cleanup(connector);
return false;
+ }
intel_dp_add_properties(intel_dp, connector);
@@ -3206,5 +3208,6 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
intel_encoder->cloneable = false;
intel_encoder->hot_plug = intel_dp_hot_plug;
- intel_dp_init_connector(intel_dig_port, intel_connector);
+ if (!intel_dp_init_connector(intel_dig_port, intel_connector))
+ kfree(intel_connector);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/8] drm/i915: fix the "ghost eDP" encoder unwind path
2013-06-12 20:27 [PATCH 0/8] Don't let the ghost eDP haunt us Paulo Zanoni
` (3 preceding siblings ...)
2013-06-12 20:27 ` [PATCH 4/8] drm/i915: fix the "ghost eDP" connector unwind path Paulo Zanoni
@ 2013-06-12 20:27 ` Paulo Zanoni
2013-06-12 20:27 ` [PATCH 6/8] drm/i915: check the return value of intel_dp_i2c_init Paulo Zanoni
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-06-12 20:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Because calling intel_dp_encoder_destroy inside
intel_edp_init_connector is just wrong. This is the initialization
path, so we should properly unwind all the initialization through the
whole caller stack.
On the intel_dp_encoder_destroy function we do the following:
1 - Call i2c_del_adapter
2 - Call drm_encoder_cleanup
3 - If edp:
3.1 - Cancel panel_vdd_work
3.2 - Call ironlake_panel_vdd_of_sync
4 - Free the encoder
And here is how we unwind each specific step:
1 - We have intel_dp_init_connector -> intel_dp_i2c_init ->
i2c_dp_aux_add_bus -> i2c_add_adapter, so we call
i2c_del_dapter at intel_dp_init_connector
2 - Call it in the same function that called drm_encoder_init
3 - Call it in the same function that called INIT_DELAYED_WORK
4 - Free it in the same function that allocated it
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_ddi.c | 2 ++
drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index aed363c..324211a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1357,6 +1357,8 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
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;
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index da7460c..b311eaf 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2986,7 +2986,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
} else {
/* if this fails, presume the device is a ghost */
DRM_INFO("failed to retrieve link info, disabling eDP\n");
- intel_dp_encoder_destroy(&intel_dig_port->base.base);
return false;
}
@@ -3146,6 +3145,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
intel_dp_i2c_init(intel_dp, intel_connector, name);
if (!intel_edp_init_connector(intel_dp, intel_connector)) {
+ i2c_del_adapter(&intel_dp->adapter);
+ if (is_edp(intel_dp)) {
+ cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
+ mutex_lock(&dev->mode_config.mutex);
+ ironlake_panel_vdd_off_sync(intel_dp);
+ mutex_unlock(&dev->mode_config.mutex);
+ }
drm_sysfs_connector_remove(connector);
drm_connector_cleanup(connector);
return false;
@@ -3208,6 +3214,9 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
intel_encoder->cloneable = false;
intel_encoder->hot_plug = intel_dp_hot_plug;
- if (!intel_dp_init_connector(intel_dig_port, intel_connector))
+ if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
+ drm_encoder_cleanup(encoder);
+ kfree(intel_dig_port);
kfree(intel_connector);
+ }
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 6/8] drm/i915: check the return value of intel_dp_i2c_init
2013-06-12 20:27 [PATCH 0/8] Don't let the ghost eDP haunt us Paulo Zanoni
` (4 preceding siblings ...)
2013-06-12 20:27 ` [PATCH 5/8] drm/i915: fix the "ghost eDP" encoder " Paulo Zanoni
@ 2013-06-12 20:27 ` Paulo Zanoni
2013-06-12 20:27 ` [PATCH 7/8] drm/i915: invert the verbosity of intel_enable_fbc Paulo Zanoni
2013-06-12 20:27 ` [PATCH 8/8] drm/i915: rename intel_dp_destroy to intel_dp_connector_destroy Paulo Zanoni
7 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-06-12 20:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
We've been ignoring this return value, so print a nice backtrace in
case it's not what we expected.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b311eaf..89e30ec 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3044,7 +3044,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
struct drm_i915_private *dev_priv = dev->dev_private;
enum port port = intel_dig_port->port;
const char *name = NULL;
- int type;
+ int type, error;
/* Preserve the current hw state. */
intel_dp->DP = I915_READ(intel_dp->output_reg);
@@ -3142,7 +3142,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
BUG();
}
- intel_dp_i2c_init(intel_dp, intel_connector, name);
+ error = intel_dp_i2c_init(intel_dp, intel_connector, name);
+ WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
+ error, port_name(port));
if (!intel_edp_init_connector(intel_dp, intel_connector)) {
i2c_del_adapter(&intel_dp->adapter);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 7/8] drm/i915: invert the verbosity of intel_enable_fbc
2013-06-12 20:27 [PATCH 0/8] Don't let the ghost eDP haunt us Paulo Zanoni
` (5 preceding siblings ...)
2013-06-12 20:27 ` [PATCH 6/8] drm/i915: check the return value of intel_dp_i2c_init Paulo Zanoni
@ 2013-06-12 20:27 ` Paulo Zanoni
2013-06-12 20:27 ` [PATCH 8/8] drm/i915: rename intel_dp_destroy to intel_dp_connector_destroy Paulo Zanoni
7 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-06-12 20:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
We currently print a DRM_DEBUG_KMS message on the happy path and don't
print anything on the "failed to allocate" path. On some desktop
environments (e.g., Unity) I see the "scheduling delayed FBC enable"
thousands and thousands of times on my dmesg.
So kill the useless message for the happy case, saving a lot of dmesg
space, and properly signal the "kzalloc fail" case.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_pm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index adc44e4..7093402 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -381,6 +381,7 @@ void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
work = kzalloc(sizeof *work, GFP_KERNEL);
if (work == NULL) {
+ DRM_ERROR("Failed to allocate FBC work structure\n");
dev_priv->display.enable_fbc(crtc, interval);
return;
}
@@ -392,8 +393,6 @@ void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
dev_priv->fbc_work = work;
- DRM_DEBUG_KMS("scheduling delayed FBC enable\n");
-
/* Delay the actual enabling to let pageflipping cease and the
* display to settle before starting the compression. Note that
* this delay also serves a second purpose: it allows for a
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 8/8] drm/i915: rename intel_dp_destroy to intel_dp_connector_destroy
2013-06-12 20:27 [PATCH 0/8] Don't let the ghost eDP haunt us Paulo Zanoni
` (6 preceding siblings ...)
2013-06-12 20:27 ` [PATCH 7/8] drm/i915: invert the verbosity of intel_enable_fbc Paulo Zanoni
@ 2013-06-12 20:27 ` Paulo Zanoni
7 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2013-06-12 20:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Because it's the function that destroys the connector, not the
encoder. And we already have intel_dp_encoder_destroy.
This has annoyed me for a long time.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 89e30ec..8708a0c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2681,7 +2681,7 @@ done:
}
static void
-intel_dp_destroy(struct drm_connector *connector)
+intel_dp_connector_destroy(struct drm_connector *connector)
{
struct intel_connector *intel_connector = to_intel_connector(connector);
@@ -2724,7 +2724,7 @@ static const struct drm_connector_funcs intel_dp_connector_funcs = {
.detect = intel_dp_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.set_property = intel_dp_set_property,
- .destroy = intel_dp_destroy,
+ .destroy = intel_dp_connector_destroy,
};
static const struct drm_connector_helper_funcs intel_dp_connector_helper_funcs = {
--
1.8.1.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-06-12 20:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-12 20:27 [PATCH 0/8] Don't let the ghost eDP haunt us Paulo Zanoni
2013-06-12 20:27 ` [PATCH 1/8] drm/i915: don't check encoder at DP connector destroy() Paulo Zanoni
2013-06-12 20:27 ` [PATCH 2/8] drm/i915: extract intel_edp_init_connector Paulo Zanoni
2013-06-12 20:27 ` [PATCH 3/8] drm/i915: propagate errors from intel_dp_init_connector Paulo Zanoni
2013-06-12 20:27 ` [PATCH 4/8] drm/i915: fix the "ghost eDP" connector unwind path Paulo Zanoni
2013-06-12 20:27 ` [PATCH 5/8] drm/i915: fix the "ghost eDP" encoder " Paulo Zanoni
2013-06-12 20:27 ` [PATCH 6/8] drm/i915: check the return value of intel_dp_i2c_init Paulo Zanoni
2013-06-12 20:27 ` [PATCH 7/8] drm/i915: invert the verbosity of intel_enable_fbc Paulo Zanoni
2013-06-12 20:27 ` [PATCH 8/8] drm/i915: rename intel_dp_destroy to intel_dp_connector_destroy Paulo Zanoni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).