* [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
@ 2015-12-16 10:48 Sonika Jindal
2015-12-16 10:48 ` [PATCH 2/2] drm/i915: Add hot_plug hook for hdmi encoder Sonika Jindal
2015-12-16 13:46 ` [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases Daniel Vetter
0 siblings, 2 replies; 12+ messages in thread
From: Sonika Jindal @ 2015-12-16 10:48 UTC (permalink / raw)
To: intel-gfx
Call the encoders, call the hot_plug if it is registered.
This is required for connected boot and resume cases to generate
fake hpd resulting in reading of edid.
Removing the initial sdvo hot_plug call too so that it will be called
just once from this loop.
v2: Schedule a work function to call hot_plug. On CHT, it runs into a
deadlock if we call ->hot_plug inside hpd_init. This is because, hot_plug
calls set_edid which tries to acquire the power domain and if power
well is disabled, we enable power well and call hpd_init again.
So, schedule a work function from here to call hot_plug and run a
detect cycle.
Cc: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_hotplug.c | 26 ++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_sdvo.c | 1 -
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bc865e23..4f037b9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -228,6 +228,7 @@ enum hpd_pin {
struct i915_hotplug {
struct work_struct hotplug_work;
+ struct work_struct edid_work;
struct {
unsigned long last_jiffies;
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index b177857..72d8fe8 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -442,6 +442,24 @@ void intel_hpd_irq_handler(struct drm_device *dev,
schedule_work(&dev_priv->hotplug.hotplug_work);
}
+static void i915_edid_work_func(struct work_struct *work)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(work, struct drm_i915_private, hotplug.edid_work);
+ struct drm_device *dev = dev_priv->dev;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ struct intel_encoder *encoder;
+
+ mutex_lock(&mode_config->mutex);
+ list_for_each_entry(encoder, &mode_config->encoder_list,
+ base.head) {
+ if (encoder->hot_plug)
+ encoder->hot_plug(encoder);
+ }
+ mutex_unlock(&mode_config->mutex);
+ drm_helper_hpd_irq_event(dev);
+}
+
/**
* intel_hpd_init - initializes and enables hpd support
* @dev_priv: i915 device instance
@@ -482,12 +500,19 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
if (dev_priv->display.hpd_irq_setup)
dev_priv->display.hpd_irq_setup(dev);
spin_unlock_irq(&dev_priv->irq_lock);
+
+ /*
+ * Connected boot / resume scenarios can't generate new hot plug.
+ * So, probe it manually.
+ */
+ schedule_work(&dev_priv->hotplug.edid_work);
}
void intel_hpd_init_work(struct drm_i915_private *dev_priv)
{
INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func);
INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func);
+ INIT_WORK(&dev_priv->hotplug.edid_work, i915_edid_work_func);
INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
intel_hpd_irq_storm_reenable_work);
}
@@ -504,5 +529,6 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
cancel_work_sync(&dev_priv->hotplug.dig_port_work);
cancel_work_sync(&dev_priv->hotplug.hotplug_work);
+ cancel_work_sync(&dev_priv->hotplug.edid_work);
cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
}
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 06679f1..4238a02 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2466,7 +2466,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
* Ensure that they get re-enabled when an interrupt happens.
*/
intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
- intel_sdvo_enable_hotplug(intel_encoder);
} else {
intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] drm/i915: Add hot_plug hook for hdmi encoder
2015-12-16 10:48 [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases Sonika Jindal
@ 2015-12-16 10:48 ` Sonika Jindal
2015-12-16 13:46 ` [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases Daniel Vetter
1 sibling, 0 replies; 12+ messages in thread
From: Sonika Jindal @ 2015-12-16 10:48 UTC (permalink / raw)
To: intel-gfx
From: Shashank Sharma <shashank.sharma@intel.com>
This patch adds a separate probe function for HDMI
EDID read over DDC channel. This function has been
registered as a .hot_plug handler for HDMI encoder.
The current implementation of hdmi_detect()
function re-sets the cached HDMI edid (in connector->detect_edid) in
every detect call.This function gets called many times, sometimes
directly from userspace probes, forcing drivers to read EDID every
detect function call.This causes several problems like:
1. Race conditions in multiple hot_plug / unplug cases, between
interrupts bottom halves and userspace detections.
2. Many Un-necessary EDID reads for single hotplug/unplug
3. HDMI complaince failures which expects only one EDID read per hotplug
This function will be serving the purpose of really reading the EDID
by really probing the DDC channel, and updating the cached EDID.
The plan is to:
1. i915 IRQ handler bottom half function already calls
intel_encoder->hotplug() function. Adding This probe function which
will read the EDID only in case of a hotplug / unplug.
2. During init_connector this probe will be called to read the edid
3. Reuse the cached EDID in hdmi_detect() function.
The "< gen7" check is there because this was tested only for >=gen7
platforms. For older platforms the hotplug/reading edid path remains same.
v2: Calling set_edid instead of hdmi_probe during init.
Also, for platforms having DDI, intel_encoder for DP and HDMI is same
(taken from intel_dig_port), so for DP also, hot_plug function gets called
which is not intended here. So, check for HDMI in intel_hdmi_probe
Rely on HPD for updating edid only for platforms gen > 8 and also for VLV.
v3: Dropping the gen < 8 || !VLV check. Now all platforms should rely on
hotplug or init for updating the edid.(Daniel)
Also, calling hdmi_probe in init instead of set_edid
v4: Renaming intel_hdmi_probe to intel_hdmi_hot_plug.
Also calling this hotplug handler from intel_hpd_init to take care of init
resume scenarios.
v5: Moved the call to encoder hotplug during init to separate patch(Daniel)
v6: Rebased and maintaining authorship.
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
drivers/gpu/drm/i915/intel_hdmi.c | 58 ++++++++++++++++++++++++++++++---------
1 file changed, 45 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index bdd462e..16dd2a7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1381,18 +1381,16 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force)
return connected;
}
-static enum drm_connector_status
-intel_hdmi_detect(struct drm_connector *connector, bool force)
+void intel_hdmi_hot_plug(struct intel_encoder *intel_encoder)
{
- enum drm_connector_status status;
- struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
- struct drm_i915_private *dev_priv = to_i915(connector->dev);
+ struct intel_hdmi *intel_hdmi =
+ enc_to_intel_hdmi(&intel_encoder->base);
+ struct intel_connector *intel_connector =
+ intel_hdmi->attached_connector;
+ struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
bool live_status = false;
unsigned int retry = 3;
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
- connector->base.id, connector->name);
-
intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
while (!live_status && --retry) {
@@ -1400,21 +1398,53 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
hdmi_to_dig_port(intel_hdmi));
mdelay(10);
}
+ intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
if (!live_status)
DRM_DEBUG_KMS("Live status not up!");
- intel_hdmi_unset_edid(connector);
+ /*
+ * We are here, means there is a hotplug or a force
+ * detection. Clear the cached EDID and probe the
+ * DDC bus to check the current status of HDMI.
+ */
+ intel_hdmi_unset_edid(&intel_connector->base);
+ if (intel_hdmi_set_edid(&intel_connector->base, live_status))
+ DRM_DEBUG_DRIVER("DDC probe: got EDID\n");
+ else
+ DRM_DEBUG_DRIVER("DDC probe: no EDID\n");
+}
+
+static enum drm_connector_status
+intel_hdmi_detect(struct drm_connector *connector, bool force)
+{
+ enum drm_connector_status status;
+ struct intel_connector *intel_connector =
+ to_intel_connector(connector);
- if (intel_hdmi_set_edid(connector, live_status)) {
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
+ connector->base.id, connector->name);
+
+ /*
+ * There are many userspace calls which probe EDID from
+ * detect path. In case of multiple hotplug/unplug, these
+ * can cause race conditions while probing EDID. Also its
+ * waste of CPU cycles to read the EDID again and again
+ * unless there is a real hotplug.
+ * So, rely on hotplugs and init to read edid.
+ * Check connector status based on availability of cached EDID.
+ */
+
+ if (intel_connector->detect_edid) {
struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
status = connector_status_connected;
- } else
+ DRM_DEBUG_DRIVER("hdmi status = connected\n");
+ } else {
status = connector_status_disconnected;
-
- intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
+ DRM_DEBUG_DRIVER("hdmi status = disconnected\n");
+ }
return status;
}
@@ -2131,6 +2161,8 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
intel_hdmi_add_properties(intel_hdmi, connector);
+ intel_encoder->hot_plug = intel_hdmi_hot_plug;
+
intel_connector_attach_encoder(intel_connector, intel_encoder);
drm_connector_register(connector);
intel_hdmi->attached_connector = intel_connector;
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
2015-12-16 10:48 [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases Sonika Jindal
2015-12-16 10:48 ` [PATCH 2/2] drm/i915: Add hot_plug hook for hdmi encoder Sonika Jindal
@ 2015-12-16 13:46 ` Daniel Vetter
2015-12-17 4:23 ` Jindal, Sonika
1 sibling, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2015-12-16 13:46 UTC (permalink / raw)
To: Sonika Jindal; +Cc: intel-gfx
On Wed, Dec 16, 2015 at 04:18:05PM +0530, Sonika Jindal wrote:
> Call the encoders, call the hot_plug if it is registered.
> This is required for connected boot and resume cases to generate
> fake hpd resulting in reading of edid.
> Removing the initial sdvo hot_plug call too so that it will be called
> just once from this loop.
>
> v2: Schedule a work function to call hot_plug. On CHT, it runs into a
> deadlock if we call ->hot_plug inside hpd_init. This is because, hot_plug
> calls set_edid which tries to acquire the power domain and if power
> well is disabled, we enable power well and call hpd_init again.
> So, schedule a work function from here to call hot_plug and run a
> detect cycle.
>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
>
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_hotplug.c | 26 ++++++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_sdvo.c | 1 -
> 3 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index bc865e23..4f037b9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -228,6 +228,7 @@ enum hpd_pin {
>
> struct i915_hotplug {
> struct work_struct hotplug_work;
> + struct work_struct edid_work;
>
> struct {
> unsigned long last_jiffies;
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index b177857..72d8fe8 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -442,6 +442,24 @@ void intel_hpd_irq_handler(struct drm_device *dev,
> schedule_work(&dev_priv->hotplug.hotplug_work);
> }
>
> +static void i915_edid_work_func(struct work_struct *work)
> +{
> + struct drm_i915_private *dev_priv =
> + container_of(work, struct drm_i915_private, hotplug.edid_work);
> + struct drm_device *dev = dev_priv->dev;
> + struct drm_mode_config *mode_config = &dev->mode_config;
> + struct intel_encoder *encoder;
> +
> + mutex_lock(&mode_config->mutex);
> + list_for_each_entry(encoder, &mode_config->encoder_list,
> + base.head) {
> + if (encoder->hot_plug)
> + encoder->hot_plug(encoder);
> + }
> + mutex_unlock(&mode_config->mutex);
> + drm_helper_hpd_irq_event(dev);
> +}
Why do we need a completely new hand-rolled work? My idea was to reuse the
existing hpd irq handler, but instead of just scheduling that (which won't
do anything) fake-inject a full set of hpd interrupts into it. Well, we
need to inject short-pulse ones to avoid upsetting dp mst.
-Daniel
> +
> /**
> * intel_hpd_init - initializes and enables hpd support
> * @dev_priv: i915 device instance
> @@ -482,12 +500,19 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> if (dev_priv->display.hpd_irq_setup)
> dev_priv->display.hpd_irq_setup(dev);
> spin_unlock_irq(&dev_priv->irq_lock);
> +
> + /*
> + * Connected boot / resume scenarios can't generate new hot plug.
> + * So, probe it manually.
> + */
> + schedule_work(&dev_priv->hotplug.edid_work);
> }
>
> void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> {
> INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func);
> INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func);
> + INIT_WORK(&dev_priv->hotplug.edid_work, i915_edid_work_func);
> INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
> intel_hpd_irq_storm_reenable_work);
> }
> @@ -504,5 +529,6 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
>
> cancel_work_sync(&dev_priv->hotplug.dig_port_work);
> cancel_work_sync(&dev_priv->hotplug.hotplug_work);
> + cancel_work_sync(&dev_priv->hotplug.edid_work);
> cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
> }
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 06679f1..4238a02 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2466,7 +2466,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> * Ensure that they get re-enabled when an interrupt happens.
> */
> intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
> - intel_sdvo_enable_hotplug(intel_encoder);
> } else {
> intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
> }
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
2015-12-16 13:46 ` [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases Daniel Vetter
@ 2015-12-17 4:23 ` Jindal, Sonika
0 siblings, 0 replies; 12+ messages in thread
From: Jindal, Sonika @ 2015-12-17 4:23 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On 12/16/2015 7:16 PM, Daniel Vetter wrote:
> On Wed, Dec 16, 2015 at 04:18:05PM +0530, Sonika Jindal wrote:
>> Call the encoders, call the hot_plug if it is registered.
>> This is required for connected boot and resume cases to generate
>> fake hpd resulting in reading of edid.
>> Removing the initial sdvo hot_plug call too so that it will be called
>> just once from this loop.
>>
>> v2: Schedule a work function to call hot_plug. On CHT, it runs into a
>> deadlock if we call ->hot_plug inside hpd_init. This is because, hot_plug
>> calls set_edid which tries to acquire the power domain and if power
>> well is disabled, we enable power well and call hpd_init again.
>> So, schedule a work function from here to call hot_plug and run a
>> detect cycle.
>>
>> Cc: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>>
>> drivers/gpu/drm/i915/i915_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_hotplug.c | 26 ++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_sdvo.c | 1 -
>> 3 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index bc865e23..4f037b9 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -228,6 +228,7 @@ enum hpd_pin {
>>
>> struct i915_hotplug {
>> struct work_struct hotplug_work;
>> + struct work_struct edid_work;
>>
>> struct {
>> unsigned long last_jiffies;
>> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
>> index b177857..72d8fe8 100644
>> --- a/drivers/gpu/drm/i915/intel_hotplug.c
>> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
>> @@ -442,6 +442,24 @@ void intel_hpd_irq_handler(struct drm_device *dev,
>> schedule_work(&dev_priv->hotplug.hotplug_work);
>> }
>>
>> +static void i915_edid_work_func(struct work_struct *work)
>> +{
>> + struct drm_i915_private *dev_priv =
>> + container_of(work, struct drm_i915_private, hotplug.edid_work);
>> + struct drm_device *dev = dev_priv->dev;
>> + struct drm_mode_config *mode_config = &dev->mode_config;
>> + struct intel_encoder *encoder;
>> +
>> + mutex_lock(&mode_config->mutex);
>> + list_for_each_entry(encoder, &mode_config->encoder_list,
>> + base.head) {
>> + if (encoder->hot_plug)
>> + encoder->hot_plug(encoder);
>> + }
>> + mutex_unlock(&mode_config->mutex);
>> + drm_helper_hpd_irq_event(dev);
>> +}
> Why do we need a completely new hand-rolled work? My idea was to reuse the
> existing hpd irq handler, but instead of just scheduling that (which won't
> do anything) fake-inject a full set of hpd interrupts into it. Well, we
> need to inject short-pulse ones to avoid upsetting dp mst.
> -Daniel
Hmm, this suggestion came from Siva as well to just set the event_bits.
Let me try that.
Thanks,
Sonika
>> +
>> /**
>> * intel_hpd_init - initializes and enables hpd support
>> * @dev_priv: i915 device instance
>> @@ -482,12 +500,19 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>> if (dev_priv->display.hpd_irq_setup)
>> dev_priv->display.hpd_irq_setup(dev);
>> spin_unlock_irq(&dev_priv->irq_lock);
>> +
>> + /*
>> + * Connected boot / resume scenarios can't generate new hot plug.
>> + * So, probe it manually.
>> + */
>> + schedule_work(&dev_priv->hotplug.edid_work);
>> }
>>
>> void intel_hpd_init_work(struct drm_i915_private *dev_priv)
>> {
>> INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func);
>> INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func);
>> + INIT_WORK(&dev_priv->hotplug.edid_work, i915_edid_work_func);
>> INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
>> intel_hpd_irq_storm_reenable_work);
>> }
>> @@ -504,5 +529,6 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
>>
>> cancel_work_sync(&dev_priv->hotplug.dig_port_work);
>> cancel_work_sync(&dev_priv->hotplug.hotplug_work);
>> + cancel_work_sync(&dev_priv->hotplug.edid_work);
>> cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>> index 06679f1..4238a02 100644
>> --- a/drivers/gpu/drm/i915/intel_sdvo.c
>> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
>> @@ -2466,7 +2466,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>> * Ensure that they get re-enabled when an interrupt happens.
>> */
>> intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
>> - intel_sdvo_enable_hotplug(intel_encoder);
>> } else {
>> intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
>> }
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915: Add hot_plug hook for hdmi encoder
@ 2015-09-28 13:34 Daniel Vetter
2015-10-05 11:13 ` [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases Sonika Jindal
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2015-09-28 13:34 UTC (permalink / raw)
To: Sonika Jindal; +Cc: intel-gfx
On Mon, Sep 28, 2015 at 02:26:04PM +0530, Sonika Jindal wrote:
> This patch adds a separate probe function for HDMI
> EDID read over DDC channel. This function has been
> registered as a .hot_plug handler for HDMI encoder.
>
> The current implementation of hdmi_detect()
> function re-sets the cached HDMI edid (in connector->detect_edid) in
> every detect call.This function gets called many times, sometimes
> directly from userspace probes, forcing drivers to read EDID every
> detect function call.This causes several problems like:
>
> 1. Race conditions in multiple hot_plug / unplug cases, between
> interrupts bottom halves and userspace detections.
> 2. Many Un-necessary EDID reads for single hotplug/unplug
> 3. HDMI complaince failures which expects only one EDID read per hotplug
>
> This function will be serving the purpose of really reading the EDID
> by really probing the DDC channel, and updating the cached EDID.
>
> The plan is to:
> 1. i915 IRQ handler bottom half function already calls
> intel_encoder->hotplug() function. Adding This probe function which
> will read the EDID only in case of a hotplug / unplug.
> 2. During init_connector this probe will be called to read the edid
> 3. Reuse the cached EDID in hdmi_detect() function.
>
> The "< gen7" check is there because this was tested only for >=gen7
> platforms. For older platforms the hotplug/reading edid path remains same.
>
> v2: Calling set_edid instead of hdmi_probe during init.
> Also, for platforms having DDI, intel_encoder for DP and HDMI is same
> (taken from intel_dig_port), so for DP also, hot_plug function gets called
> which is not intended here. So, check for HDMI in intel_hdmi_probe
> Rely on HPD for updating edid only for platforms gen > 8 and also for VLV.
>
> v3: Dropping the gen < 8 || !VLV check. Now all platforms should rely on
> hotplug or init for updating the edid.(Daniel)
> Also, calling hdmi_probe in init instead of set_edid
>
> v4: Renaming intel_hdmi_probe to intel_hdmi_hot_plug and changing the patch
> subject. Also calling this hotplug handler from intel_hpd_init to take care
> of init resume scenarios.
>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
> drivers/gpu/drm/i915/intel_hdmi.c | 54 +++++++++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++
> 2 files changed, 54 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index bb33c66..9c1a308 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1369,18 +1369,16 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force)
> return connected;
> }
>
> -static enum drm_connector_status
> -intel_hdmi_detect(struct drm_connector *connector, bool force)
> +void intel_hdmi_hot_plug(struct intel_encoder *intel_encoder)
> {
> - enum drm_connector_status status;
> - struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> - struct drm_i915_private *dev_priv = to_i915(connector->dev);
> + struct intel_hdmi *intel_hdmi =
> + enc_to_intel_hdmi(&intel_encoder->base);
> + struct intel_connector *intel_connector =
> + intel_hdmi->attached_connector;
> + struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
> bool live_status = false;
> unsigned int retry = 3;
>
> - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> - connector->base.id, connector->name);
> -
> while (!live_status && --retry) {
> live_status = intel_digital_port_connected(dev_priv,
> hdmi_to_dig_port(intel_hdmi));
> @@ -1390,15 +1388,48 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> if (!live_status)
> DRM_DEBUG_KMS("Live status not up!");
>
> - intel_hdmi_unset_edid(connector);
> + /*
> + * We are here, means there is a hotplug or a force
> + * detection. Clear the cached EDID and probe the
> + * DDC bus to check the current status of HDMI.
> + */
> + intel_hdmi_unset_edid(&intel_connector->base);
> + if (intel_hdmi_set_edid(&intel_connector->base, live_status))
> + DRM_DEBUG_DRIVER("DDC probe: got EDID\n");
> + else
> + DRM_DEBUG_DRIVER("DDC probe: no EDID\n");
> +}
>
> - if (intel_hdmi_set_edid(connector, live_status)) {
> +static enum drm_connector_status
> +intel_hdmi_detect(struct drm_connector *connector, bool force)
> +{
> + enum drm_connector_status status;
> + struct intel_connector *intel_connector =
> + to_intel_connector(connector);
> +
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> + connector->base.id, connector->name);
> +
> + /*
> + * There are many userspace calls which probe EDID from
> + * detect path. In case of multiple hotplug/unplug, these
> + * can cause race conditions while probing EDID. Also its
> + * waste of CPU cycles to read the EDID again and again
> + * unless there is a real hotplug.
> + * So, rely on hotplugs and init to read edid.
> + * Check connector status based on availability of cached EDID.
> + */
> +
> + if (intel_connector->detect_edid) {
> struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>
> hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> status = connector_status_connected;
> - } else
> + DRM_DEBUG_DRIVER("hdmi status = connected\n");
> + } else {
> status = connector_status_disconnected;
> + DRM_DEBUG_DRIVER("hdmi status = disconnected\n");
> + }
>
> return status;
> }
> @@ -2131,6 +2162,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> intel_connector->unregister = intel_connector_unregister;
>
> intel_hdmi_add_properties(intel_hdmi, connector);
> + intel_encoder->hot_plug = intel_hdmi_hot_plug;
>
> intel_connector_attach_encoder(intel_connector, intel_encoder);
> drm_connector_register(connector);
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 53c0173..36e16f6 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -459,6 +459,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> struct drm_device *dev = dev_priv->dev;
> struct drm_mode_config *mode_config = &dev->mode_config;
> struct drm_connector *connector;
> + struct intel_encoder *encoder;
> int i;
>
> for_each_hpd_pin(i) {
> @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> if (dev_priv->display.hpd_irq_setup)
> dev_priv->display.hpd_irq_setup(dev);
> spin_unlock_irq(&dev_priv->irq_lock);
> +
> + /*
> + * Connected boot / resume scenarios can't generate new hot plug.
> + * So, probe it manually.
> + */
> + list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> + base.head) {
> + if (encoder->hot_plug)
> + encoder->hot_plug(encoder);
> + }
Since this doesn't just change hdmi but every encoder I think we should
pull it out into a separate prep patch. Especially interactions with dp
mst handling look tricky, have you tested that nothing blows up with that?
The split-out patch should definitely explain in detail how this interacts
with dp hpd handling.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
2015-09-28 13:34 [PATCH] drm/i915: Add hot_plug hook for hdmi encoder Daniel Vetter
@ 2015-10-05 11:13 ` Sonika Jindal
2015-10-08 13:35 ` Ville Syrjälä
0 siblings, 1 reply; 12+ messages in thread
From: Sonika Jindal @ 2015-10-05 11:13 UTC (permalink / raw)
To: intel-gfx
For all the encoders, call the hot_plug if it is registered.
This is required for connected boot and resume cases to generate
fake hpd resulting in reading of edid.
Removing the initial sdvo hot_plug call too so that it will be called
just once from this loop.
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++++++
drivers/gpu/drm/i915/intel_sdvo.c | 1 -
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 53c0173..eac4757 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -458,6 +458,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv->dev;
struct drm_mode_config *mode_config = &dev->mode_config;
+ struct intel_encoder *encoder;
struct drm_connector *connector;
int i;
@@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
if (dev_priv->display.hpd_irq_setup)
dev_priv->display.hpd_irq_setup(dev);
spin_unlock_irq(&dev_priv->irq_lock);
+
+ /*
+ * Connected boot / resume scenarios can't generate new hot plug.
+ * So, probe it manually.
+ */
+ list_for_each_entry(encoder, &dev->mode_config.encoder_list,
+ base.head) {
+ if (encoder->hot_plug)
+ encoder->hot_plug(encoder);
+ }
}
void intel_hpd_init_work(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 05521b5..55859e9 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2433,7 +2433,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
* Ensure that they get re-enabled when an interrupt happens.
*/
intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
- intel_sdvo_enable_hotplug(intel_encoder);
} else {
intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
}
--
1.7.10.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
2015-10-05 11:13 ` [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases Sonika Jindal
@ 2015-10-08 13:35 ` Ville Syrjälä
2015-10-08 14:38 ` Jani Nikula
2015-10-12 12:24 ` Sharma, Shashank
0 siblings, 2 replies; 12+ messages in thread
From: Ville Syrjälä @ 2015-10-08 13:35 UTC (permalink / raw)
To: Sonika Jindal; +Cc: intel-gfx
On Mon, Oct 05, 2015 at 04:43:14PM +0530, Sonika Jindal wrote:
> For all the encoders, call the hot_plug if it is registered.
> This is required for connected boot and resume cases to generate
> fake hpd resulting in reading of edid.
> Removing the initial sdvo hot_plug call too so that it will be called
> just once from this loop.
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
> drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++++++
> drivers/gpu/drm/i915/intel_sdvo.c | 1 -
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 53c0173..eac4757 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -458,6 +458,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> {
> struct drm_device *dev = dev_priv->dev;
> struct drm_mode_config *mode_config = &dev->mode_config;
> + struct intel_encoder *encoder;
> struct drm_connector *connector;
> int i;
>
> @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> if (dev_priv->display.hpd_irq_setup)
> dev_priv->display.hpd_irq_setup(dev);
> spin_unlock_irq(&dev_priv->irq_lock);
> +
> + /*
> + * Connected boot / resume scenarios can't generate new hot plug.
> + * So, probe it manually.
> + */
> + list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> + base.head) {
> + if (encoder->hot_plug)
> + encoder->hot_plug(encoder);
> + }
This breaks the world on CHV
[ 3187.575198] [drm:intel_hdmi_hot_plug] Live status not up!
[ 3187.585154] =============================================
[ 3187.595010] [ INFO: possible recursive locking detected ]
[ 3187.604685] 4.3.0-rc4-bsw+ #2488 Tainted: G U W
[ 3187.614366] ---------------------------------------------
[ 3187.623892] Xorg/32212 is trying to acquire lock:
[ 3187.632635] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
[ 3187.647492]
[ 3187.647492] but task is already holding lock:
[ 3187.661054] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
[ 3187.675960]
[ 3187.675960] other info that might help us debug this:
[ 3187.690459] Possible unsafe locking scenario:
[ 3187.690459]
[ 3187.704224] CPU0
[ 3187.710485] ----
[ 3187.716711] lock(&power_domains->lock);
[ 3187.724718] lock(&power_domains->lock);
[ 3187.732663]
[ 3187.732663] *** DEADLOCK ***
[ 3187.732663]
[ 3187.749460] May be due to missing lock nesting notation
[ 3187.749460]
[ 3187.763833] 5 locks held by Xorg/32212:
[ 3187.771523] #0: (drm_global_mutex){+.+.+.}, at: [<ffffffffa02ad5c8>] drm_release+0x3b/0x49b [drm]
[ 3187.785216] #1: (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffffa02c4d85>] drm_modeset_lock_all+0x54/0xcd [drm]
[ 3187.800437] #2: (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffffa02c4d8f>] drm_modeset_lock_all+0x5e/0xcd [drm]
[ 3187.815488] #3: (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffffa02c46b0>] drm_modeset_lock+0x75/0xfc [drm]
[ 3187.830094] #4: (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
[ 3187.845534]
[ 3187.845534] stack backtrace:
[ 3187.857685] CPU: 2 PID: 32212 Comm: Xorg Tainted: G U W 4.3.0-rc4-bsw+ #2488
[ 3187.870331] Hardware name: Intel Corporation CHERRYVIEW C0 PLATFORM/Braswell CRB, BIOS BRAS.X64.B085.R00.1509110553 09/11/2015
[ 3187.886827] 0000000000000000 ffff880175eff8e0 ffffffff8128d59e ffffffff823f5ee0
[ 3187.898904] ffff880175eff958 ffffffff810a7a08 0000000000000000 ffff880179d1c5d0
[ 3187.910954] 0000000000000004 0000000000000006 45422a91588a4c3e 0000000000000005
[ 3187.923011] Call Trace:
[ 3187.929451] [<ffffffff8128d59e>] dump_stack+0x4e/0x79
[ 3187.938912] [<ffffffff810a7a08>] __lock_acquire+0x7ab/0x12af
[ 3187.949027] [<ffffffff810a8d04>] lock_acquire+0x10e/0x1a9
[ 3187.958859] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
[ 3187.970476] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
[ 3187.982011] [<ffffffff814df4a3>] mutex_lock_nested+0x71/0x346
[ 3187.992167] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
[ 3188.003655] [<ffffffff814e20ee>] ? _raw_spin_unlock_irqrestore+0x4b/0x60
[ 3188.014829] [<ffffffff81364565>] ? __pm_runtime_resume+0x71/0x7e
[ 3188.025269] [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
[ 3188.036544] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
[ 3188.047968] [<ffffffffa03e8cc4>] intel_hdmi_set_edid+0x3f/0xd6 [i915]
[ 3188.058766] [<ffffffffa03e8e1a>] intel_hdmi_hot_plug+0xbf/0xfb [i915]
[ 3188.069484] [<ffffffffa03c9e78>] intel_hpd_init+0xfa/0x10b [i915]
[ 3188.079753] [<ffffffffa036968d>] vlv_display_power_well_init+0xdb/0xe8 [i915]
[ 3188.091224] [<ffffffffa0369763>] chv_pipe_power_well_enable+0x62/0x67 [i915]
[ 3188.102594] [<ffffffffa036ad55>] intel_display_power_get+0xa0/0xcb [i915]
[ 3188.113657] [<ffffffffa03bdd7a>] modeset_get_crtc_power_domains+0x11d/0x13c [i915]
[ 3188.125589] [<ffffffffa03bdfc1>] intel_atomic_commit+0x228/0xf1b [i915]
[ 3188.136522] [<ffffffffa02c586c>] ? drm_atomic_check_only+0x37b/0x4da [drm]
[ 3188.147676] [<ffffffffa02c5a18>] drm_atomic_commit+0x4d/0x52 [drm]
[ 3188.158023] [<ffffffffa032fa25>] restore_fbdev_mode+0x11e/0x286 [drm_kms_helper]
[ 3188.169734] [<ffffffffa0331443>] drm_fb_helper_restore_fbdev_mode_unlocked+0x36/0x6d [drm_kms_helper]
[ 3188.183557] [<ffffffffa03d270f>] intel_fbdev_restore_mode+0x22/0x7a [i915]
[ 3188.194701] [<ffffffffa03f70b8>] i915_driver_lastclose+0xe/0x15 [i915]
[ 3188.205361] [<ffffffffa02ad4c2>] drm_lastclose+0x3f/0x10a [drm]
[ 3188.215323] [<ffffffffa02ad9ee>] drm_release+0x461/0x49b [drm]
[ 3188.225195] [<ffffffff8118577c>] __fput+0x100/0x1b3
[ 3188.233956] [<ffffffff81185865>] ____fput+0xe/0x10
[ 3188.242594] [<ffffffff81084540>] task_work_run+0x6a/0x93
[ 3188.251816] [<ffffffff810015d6>] prepare_exit_to_usermode+0x9e/0xaf
[ 3188.262111] [<ffffffff810017d6>] syscall_return_slowpath+0x1ef/0x264
[ 3188.272486] [<ffffffff81084462>] ? task_work_add+0x44/0x53
[ 3188.281885] [<ffffffff811858e3>] ? fput+0x7c/0x83
[ 3188.290389] [<ffffffff810a6e52>] ? trace_hardirqs_on_caller+0x16/0x196
[ 3188.300941] [<ffffffff81000c87>] ? trace_hardirqs_on_thunk+0x17/0x19
[ 3188.311245] [<ffffffff814e2ab1>] int_ret_from_sys_call+0x25/0x9f
> }
>
> void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 05521b5..55859e9 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2433,7 +2433,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> * Ensure that they get re-enabled when an interrupt happens.
> */
> intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
> - intel_sdvo_enable_hotplug(intel_encoder);
> } else {
> intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
> }
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
2015-10-08 13:35 ` Ville Syrjälä
@ 2015-10-08 14:38 ` Jani Nikula
2015-10-08 19:54 ` Daniel Vetter
2015-10-12 12:24 ` Sharma, Shashank
1 sibling, 1 reply; 12+ messages in thread
From: Jani Nikula @ 2015-10-08 14:38 UTC (permalink / raw)
To: Ville Syrjälä, Sonika Jindal; +Cc: intel-gfx
On Thu, 08 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, Oct 05, 2015 at 04:43:14PM +0530, Sonika Jindal wrote:
>> For all the encoders, call the hot_plug if it is registered.
>> This is required for connected boot and resume cases to generate
>> fake hpd resulting in reading of edid.
>> Removing the initial sdvo hot_plug call too so that it will be called
>> just once from this loop.
>>
>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++++++
>> drivers/gpu/drm/i915/intel_sdvo.c | 1 -
>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
>> index 53c0173..eac4757 100644
>> --- a/drivers/gpu/drm/i915/intel_hotplug.c
>> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
>> @@ -458,6 +458,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>> {
>> struct drm_device *dev = dev_priv->dev;
>> struct drm_mode_config *mode_config = &dev->mode_config;
>> + struct intel_encoder *encoder;
>> struct drm_connector *connector;
>> int i;
>>
>> @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>> if (dev_priv->display.hpd_irq_setup)
>> dev_priv->display.hpd_irq_setup(dev);
>> spin_unlock_irq(&dev_priv->irq_lock);
>> +
>> + /*
>> + * Connected boot / resume scenarios can't generate new hot plug.
>> + * So, probe it manually.
>> + */
>> + list_for_each_entry(encoder, &dev->mode_config.encoder_list,
>> + base.head) {
>> + if (encoder->hot_plug)
>> + encoder->hot_plug(encoder);
>> + }
>
>
> This breaks the world on CHV
Also patch 2/2 breaks: https://bugs.freedesktop.org/show_bug.cgi?id=88081
BR,
Jani.
>
> [ 3187.575198] [drm:intel_hdmi_hot_plug] Live status not up!
> [ 3187.585154] =============================================
> [ 3187.595010] [ INFO: possible recursive locking detected ]
> [ 3187.604685] 4.3.0-rc4-bsw+ #2488 Tainted: G U W
> [ 3187.614366] ---------------------------------------------
> [ 3187.623892] Xorg/32212 is trying to acquire lock:
> [ 3187.632635] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
> [ 3187.647492]
> [ 3187.647492] but task is already holding lock:
> [ 3187.661054] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
> [ 3187.675960]
> [ 3187.675960] other info that might help us debug this:
> [ 3187.690459] Possible unsafe locking scenario:
> [ 3187.690459]
> [ 3187.704224] CPU0
> [ 3187.710485] ----
> [ 3187.716711] lock(&power_domains->lock);
> [ 3187.724718] lock(&power_domains->lock);
> [ 3187.732663]
> [ 3187.732663] *** DEADLOCK ***
> [ 3187.732663]
> [ 3187.749460] May be due to missing lock nesting notation
> [ 3187.749460]
> [ 3187.763833] 5 locks held by Xorg/32212:
> [ 3187.771523] #0: (drm_global_mutex){+.+.+.}, at: [<ffffffffa02ad5c8>] drm_release+0x3b/0x49b [drm]
> [ 3187.785216] #1: (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffffa02c4d85>] drm_modeset_lock_all+0x54/0xcd [drm]
> [ 3187.800437] #2: (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffffa02c4d8f>] drm_modeset_lock_all+0x5e/0xcd [drm]
> [ 3187.815488] #3: (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffffa02c46b0>] drm_modeset_lock+0x75/0xfc [drm]
> [ 3187.830094] #4: (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
> [ 3187.845534]
> [ 3187.845534] stack backtrace:
> [ 3187.857685] CPU: 2 PID: 32212 Comm: Xorg Tainted: G U W 4.3.0-rc4-bsw+ #2488
> [ 3187.870331] Hardware name: Intel Corporation CHERRYVIEW C0 PLATFORM/Braswell CRB, BIOS BRAS.X64.B085.R00.1509110553 09/11/2015
> [ 3187.886827] 0000000000000000 ffff880175eff8e0 ffffffff8128d59e ffffffff823f5ee0
> [ 3187.898904] ffff880175eff958 ffffffff810a7a08 0000000000000000 ffff880179d1c5d0
> [ 3187.910954] 0000000000000004 0000000000000006 45422a91588a4c3e 0000000000000005
> [ 3187.923011] Call Trace:
> [ 3187.929451] [<ffffffff8128d59e>] dump_stack+0x4e/0x79
> [ 3187.938912] [<ffffffff810a7a08>] __lock_acquire+0x7ab/0x12af
> [ 3187.949027] [<ffffffff810a8d04>] lock_acquire+0x10e/0x1a9
> [ 3187.958859] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
> [ 3187.970476] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
> [ 3187.982011] [<ffffffff814df4a3>] mutex_lock_nested+0x71/0x346
> [ 3187.992167] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
> [ 3188.003655] [<ffffffff814e20ee>] ? _raw_spin_unlock_irqrestore+0x4b/0x60
> [ 3188.014829] [<ffffffff81364565>] ? __pm_runtime_resume+0x71/0x7e
> [ 3188.025269] [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
> [ 3188.036544] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
> [ 3188.047968] [<ffffffffa03e8cc4>] intel_hdmi_set_edid+0x3f/0xd6 [i915]
> [ 3188.058766] [<ffffffffa03e8e1a>] intel_hdmi_hot_plug+0xbf/0xfb [i915]
> [ 3188.069484] [<ffffffffa03c9e78>] intel_hpd_init+0xfa/0x10b [i915]
> [ 3188.079753] [<ffffffffa036968d>] vlv_display_power_well_init+0xdb/0xe8 [i915]
> [ 3188.091224] [<ffffffffa0369763>] chv_pipe_power_well_enable+0x62/0x67 [i915]
> [ 3188.102594] [<ffffffffa036ad55>] intel_display_power_get+0xa0/0xcb [i915]
> [ 3188.113657] [<ffffffffa03bdd7a>] modeset_get_crtc_power_domains+0x11d/0x13c [i915]
> [ 3188.125589] [<ffffffffa03bdfc1>] intel_atomic_commit+0x228/0xf1b [i915]
> [ 3188.136522] [<ffffffffa02c586c>] ? drm_atomic_check_only+0x37b/0x4da [drm]
> [ 3188.147676] [<ffffffffa02c5a18>] drm_atomic_commit+0x4d/0x52 [drm]
> [ 3188.158023] [<ffffffffa032fa25>] restore_fbdev_mode+0x11e/0x286 [drm_kms_helper]
> [ 3188.169734] [<ffffffffa0331443>] drm_fb_helper_restore_fbdev_mode_unlocked+0x36/0x6d [drm_kms_helper]
> [ 3188.183557] [<ffffffffa03d270f>] intel_fbdev_restore_mode+0x22/0x7a [i915]
> [ 3188.194701] [<ffffffffa03f70b8>] i915_driver_lastclose+0xe/0x15 [i915]
> [ 3188.205361] [<ffffffffa02ad4c2>] drm_lastclose+0x3f/0x10a [drm]
> [ 3188.215323] [<ffffffffa02ad9ee>] drm_release+0x461/0x49b [drm]
> [ 3188.225195] [<ffffffff8118577c>] __fput+0x100/0x1b3
> [ 3188.233956] [<ffffffff81185865>] ____fput+0xe/0x10
> [ 3188.242594] [<ffffffff81084540>] task_work_run+0x6a/0x93
> [ 3188.251816] [<ffffffff810015d6>] prepare_exit_to_usermode+0x9e/0xaf
> [ 3188.262111] [<ffffffff810017d6>] syscall_return_slowpath+0x1ef/0x264
> [ 3188.272486] [<ffffffff81084462>] ? task_work_add+0x44/0x53
> [ 3188.281885] [<ffffffff811858e3>] ? fput+0x7c/0x83
> [ 3188.290389] [<ffffffff810a6e52>] ? trace_hardirqs_on_caller+0x16/0x196
> [ 3188.300941] [<ffffffff81000c87>] ? trace_hardirqs_on_thunk+0x17/0x19
> [ 3188.311245] [<ffffffff814e2ab1>] int_ret_from_sys_call+0x25/0x9f
>
>
>> }
>>
>> void intel_hpd_init_work(struct drm_i915_private *dev_priv)
>> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>> index 05521b5..55859e9 100644
>> --- a/drivers/gpu/drm/i915/intel_sdvo.c
>> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
>> @@ -2433,7 +2433,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>> * Ensure that they get re-enabled when an interrupt happens.
>> */
>> intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
>> - intel_sdvo_enable_hotplug(intel_encoder);
>> } else {
>> intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
>> }
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
2015-10-08 14:38 ` Jani Nikula
@ 2015-10-08 19:54 ` Daniel Vetter
2015-10-09 4:31 ` Jindal, Sonika
2015-12-10 4:27 ` Sonika Jindal
0 siblings, 2 replies; 12+ messages in thread
From: Daniel Vetter @ 2015-10-08 19:54 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
On Thu, Oct 08, 2015 at 05:38:58PM +0300, Jani Nikula wrote:
> On Thu, 08 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Mon, Oct 05, 2015 at 04:43:14PM +0530, Sonika Jindal wrote:
> >> For all the encoders, call the hot_plug if it is registered.
> >> This is required for connected boot and resume cases to generate
> >> fake hpd resulting in reading of edid.
> >> Removing the initial sdvo hot_plug call too so that it will be called
> >> just once from this loop.
> >>
> >> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++++++
> >> drivers/gpu/drm/i915/intel_sdvo.c | 1 -
> >> 2 files changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> >> index 53c0173..eac4757 100644
> >> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> >> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> >> @@ -458,6 +458,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> >> {
> >> struct drm_device *dev = dev_priv->dev;
> >> struct drm_mode_config *mode_config = &dev->mode_config;
> >> + struct intel_encoder *encoder;
> >> struct drm_connector *connector;
> >> int i;
> >>
> >> @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> >> if (dev_priv->display.hpd_irq_setup)
> >> dev_priv->display.hpd_irq_setup(dev);
> >> spin_unlock_irq(&dev_priv->irq_lock);
> >> +
> >> + /*
> >> + * Connected boot / resume scenarios can't generate new hot plug.
> >> + * So, probe it manually.
> >> + */
> >> + list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> >> + base.head) {
> >> + if (encoder->hot_plug)
> >> + encoder->hot_plug(encoder);
> >> + }
> >
> >
> > This breaks the world on CHV
>
> Also patch 2/2 breaks: https://bugs.freedesktop.org/show_bug.cgi?id=88081
Both reverted, thanks for the reveport.
-Daniel
>
> BR,
> Jani.
>
>
> >
> > [ 3187.575198] [drm:intel_hdmi_hot_plug] Live status not up!
> > [ 3187.585154] =============================================
> > [ 3187.595010] [ INFO: possible recursive locking detected ]
> > [ 3187.604685] 4.3.0-rc4-bsw+ #2488 Tainted: G U W
> > [ 3187.614366] ---------------------------------------------
> > [ 3187.623892] Xorg/32212 is trying to acquire lock:
> > [ 3187.632635] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
> > [ 3187.647492]
> > [ 3187.647492] but task is already holding lock:
> > [ 3187.661054] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
> > [ 3187.675960]
> > [ 3187.675960] other info that might help us debug this:
> > [ 3187.690459] Possible unsafe locking scenario:
> > [ 3187.690459]
> > [ 3187.704224] CPU0
> > [ 3187.710485] ----
> > [ 3187.716711] lock(&power_domains->lock);
> > [ 3187.724718] lock(&power_domains->lock);
> > [ 3187.732663]
> > [ 3187.732663] *** DEADLOCK ***
> > [ 3187.732663]
> > [ 3187.749460] May be due to missing lock nesting notation
> > [ 3187.749460]
> > [ 3187.763833] 5 locks held by Xorg/32212:
> > [ 3187.771523] #0: (drm_global_mutex){+.+.+.}, at: [<ffffffffa02ad5c8>] drm_release+0x3b/0x49b [drm]
> > [ 3187.785216] #1: (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffffa02c4d85>] drm_modeset_lock_all+0x54/0xcd [drm]
> > [ 3187.800437] #2: (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffffa02c4d8f>] drm_modeset_lock_all+0x5e/0xcd [drm]
> > [ 3187.815488] #3: (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffffa02c46b0>] drm_modeset_lock+0x75/0xfc [drm]
> > [ 3187.830094] #4: (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
> > [ 3187.845534]
> > [ 3187.845534] stack backtrace:
> > [ 3187.857685] CPU: 2 PID: 32212 Comm: Xorg Tainted: G U W 4.3.0-rc4-bsw+ #2488
> > [ 3187.870331] Hardware name: Intel Corporation CHERRYVIEW C0 PLATFORM/Braswell CRB, BIOS BRAS.X64.B085.R00.1509110553 09/11/2015
> > [ 3187.886827] 0000000000000000 ffff880175eff8e0 ffffffff8128d59e ffffffff823f5ee0
> > [ 3187.898904] ffff880175eff958 ffffffff810a7a08 0000000000000000 ffff880179d1c5d0
> > [ 3187.910954] 0000000000000004 0000000000000006 45422a91588a4c3e 0000000000000005
> > [ 3187.923011] Call Trace:
> > [ 3187.929451] [<ffffffff8128d59e>] dump_stack+0x4e/0x79
> > [ 3187.938912] [<ffffffff810a7a08>] __lock_acquire+0x7ab/0x12af
> > [ 3187.949027] [<ffffffff810a8d04>] lock_acquire+0x10e/0x1a9
> > [ 3187.958859] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
> > [ 3187.970476] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
> > [ 3187.982011] [<ffffffff814df4a3>] mutex_lock_nested+0x71/0x346
> > [ 3187.992167] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
> > [ 3188.003655] [<ffffffff814e20ee>] ? _raw_spin_unlock_irqrestore+0x4b/0x60
> > [ 3188.014829] [<ffffffff81364565>] ? __pm_runtime_resume+0x71/0x7e
> > [ 3188.025269] [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
> > [ 3188.036544] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
> > [ 3188.047968] [<ffffffffa03e8cc4>] intel_hdmi_set_edid+0x3f/0xd6 [i915]
> > [ 3188.058766] [<ffffffffa03e8e1a>] intel_hdmi_hot_plug+0xbf/0xfb [i915]
> > [ 3188.069484] [<ffffffffa03c9e78>] intel_hpd_init+0xfa/0x10b [i915]
> > [ 3188.079753] [<ffffffffa036968d>] vlv_display_power_well_init+0xdb/0xe8 [i915]
> > [ 3188.091224] [<ffffffffa0369763>] chv_pipe_power_well_enable+0x62/0x67 [i915]
> > [ 3188.102594] [<ffffffffa036ad55>] intel_display_power_get+0xa0/0xcb [i915]
> > [ 3188.113657] [<ffffffffa03bdd7a>] modeset_get_crtc_power_domains+0x11d/0x13c [i915]
> > [ 3188.125589] [<ffffffffa03bdfc1>] intel_atomic_commit+0x228/0xf1b [i915]
> > [ 3188.136522] [<ffffffffa02c586c>] ? drm_atomic_check_only+0x37b/0x4da [drm]
> > [ 3188.147676] [<ffffffffa02c5a18>] drm_atomic_commit+0x4d/0x52 [drm]
> > [ 3188.158023] [<ffffffffa032fa25>] restore_fbdev_mode+0x11e/0x286 [drm_kms_helper]
> > [ 3188.169734] [<ffffffffa0331443>] drm_fb_helper_restore_fbdev_mode_unlocked+0x36/0x6d [drm_kms_helper]
> > [ 3188.183557] [<ffffffffa03d270f>] intel_fbdev_restore_mode+0x22/0x7a [i915]
> > [ 3188.194701] [<ffffffffa03f70b8>] i915_driver_lastclose+0xe/0x15 [i915]
> > [ 3188.205361] [<ffffffffa02ad4c2>] drm_lastclose+0x3f/0x10a [drm]
> > [ 3188.215323] [<ffffffffa02ad9ee>] drm_release+0x461/0x49b [drm]
> > [ 3188.225195] [<ffffffff8118577c>] __fput+0x100/0x1b3
> > [ 3188.233956] [<ffffffff81185865>] ____fput+0xe/0x10
> > [ 3188.242594] [<ffffffff81084540>] task_work_run+0x6a/0x93
> > [ 3188.251816] [<ffffffff810015d6>] prepare_exit_to_usermode+0x9e/0xaf
> > [ 3188.262111] [<ffffffff810017d6>] syscall_return_slowpath+0x1ef/0x264
> > [ 3188.272486] [<ffffffff81084462>] ? task_work_add+0x44/0x53
> > [ 3188.281885] [<ffffffff811858e3>] ? fput+0x7c/0x83
> > [ 3188.290389] [<ffffffff810a6e52>] ? trace_hardirqs_on_caller+0x16/0x196
> > [ 3188.300941] [<ffffffff81000c87>] ? trace_hardirqs_on_thunk+0x17/0x19
> > [ 3188.311245] [<ffffffff814e2ab1>] int_ret_from_sys_call+0x25/0x9f
> >
> >
> >> }
> >>
> >> void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> >> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> >> index 05521b5..55859e9 100644
> >> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> >> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> >> @@ -2433,7 +2433,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> >> * Ensure that they get re-enabled when an interrupt happens.
> >> */
> >> intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
> >> - intel_sdvo_enable_hotplug(intel_encoder);
> >> } else {
> >> intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
> >> }
> >> --
> >> 1.7.10.4
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
2015-10-08 19:54 ` Daniel Vetter
@ 2015-10-09 4:31 ` Jindal, Sonika
2015-12-10 4:27 ` Sonika Jindal
1 sibling, 0 replies; 12+ messages in thread
From: Jindal, Sonika @ 2015-10-09 4:31 UTC (permalink / raw)
To: Daniel Vetter, Jani Nikula; +Cc: intel-gfx
On 10/9/2015 1:24 AM, Daniel Vetter wrote:
> On Thu, Oct 08, 2015 at 05:38:58PM +0300, Jani Nikula wrote:
>> On Thu, 08 Oct 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>> On Mon, Oct 05, 2015 at 04:43:14PM +0530, Sonika Jindal wrote:
>>>> For all the encoders, call the hot_plug if it is registered.
>>>> This is required for connected boot and resume cases to generate
>>>> fake hpd resulting in reading of edid.
>>>> Removing the initial sdvo hot_plug call too so that it will be called
>>>> just once from this loop.
>>>>
>>>> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++++++
>>>> drivers/gpu/drm/i915/intel_sdvo.c | 1 -
>>>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
>>>> index 53c0173..eac4757 100644
>>>> --- a/drivers/gpu/drm/i915/intel_hotplug.c
>>>> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
>>>> @@ -458,6 +458,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>>>> {
>>>> struct drm_device *dev = dev_priv->dev;
>>>> struct drm_mode_config *mode_config = &dev->mode_config;
>>>> + struct intel_encoder *encoder;
>>>> struct drm_connector *connector;
>>>> int i;
>>>>
>>>> @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>>>> if (dev_priv->display.hpd_irq_setup)
>>>> dev_priv->display.hpd_irq_setup(dev);
>>>> spin_unlock_irq(&dev_priv->irq_lock);
>>>> +
>>>> + /*
>>>> + * Connected boot / resume scenarios can't generate new hot plug.
>>>> + * So, probe it manually.
>>>> + */
>>>> + list_for_each_entry(encoder, &dev->mode_config.encoder_list,
>>>> + base.head) {
>>>> + if (encoder->hot_plug)
>>>> + encoder->hot_plug(encoder);
>>>> + }
>>>
>>>
>>> This breaks the world on CHV
>>
>> Also patch 2/2 breaks: https://bugs.freedesktop.org/show_bug.cgi?id=88081
>
Hmm, will check why live status is not up for chv..
Regards,
Sonika
> Both reverted, thanks for the reveport.
> -Daniel
>
>>
>> BR,
>> Jani.
>>
>>
>>>
>>> [ 3187.575198] [drm:intel_hdmi_hot_plug] Live status not up!
>>> [ 3187.585154] =============================================
>>> [ 3187.595010] [ INFO: possible recursive locking detected ]
>>> [ 3187.604685] 4.3.0-rc4-bsw+ #2488 Tainted: G U W
>>> [ 3187.614366] ---------------------------------------------
>>> [ 3187.623892] Xorg/32212 is trying to acquire lock:
>>> [ 3187.632635] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
>>> [ 3187.647492]
>>> [ 3187.647492] but task is already holding lock:
>>> [ 3187.661054] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
>>> [ 3187.675960]
>>> [ 3187.675960] other info that might help us debug this:
>>> [ 3187.690459] Possible unsafe locking scenario:
>>> [ 3187.690459]
>>> [ 3187.704224] CPU0
>>> [ 3187.710485] ----
>>> [ 3187.716711] lock(&power_domains->lock);
>>> [ 3187.724718] lock(&power_domains->lock);
>>> [ 3187.732663]
>>> [ 3187.732663] *** DEADLOCK ***
>>> [ 3187.732663]
>>> [ 3187.749460] May be due to missing lock nesting notation
>>> [ 3187.749460]
>>> [ 3187.763833] 5 locks held by Xorg/32212:
>>> [ 3187.771523] #0: (drm_global_mutex){+.+.+.}, at: [<ffffffffa02ad5c8>] drm_release+0x3b/0x49b [drm]
>>> [ 3187.785216] #1: (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffffa02c4d85>] drm_modeset_lock_all+0x54/0xcd [drm]
>>> [ 3187.800437] #2: (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffffa02c4d8f>] drm_modeset_lock_all+0x5e/0xcd [drm]
>>> [ 3187.815488] #3: (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffffa02c46b0>] drm_modeset_lock+0x75/0xfc [drm]
>>> [ 3187.830094] #4: (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
>>> [ 3187.845534]
>>> [ 3187.845534] stack backtrace:
>>> [ 3187.857685] CPU: 2 PID: 32212 Comm: Xorg Tainted: G U W 4.3.0-rc4-bsw+ #2488
>>> [ 3187.870331] Hardware name: Intel Corporation CHERRYVIEW C0 PLATFORM/Braswell CRB, BIOS BRAS.X64.B085.R00.1509110553 09/11/2015
>>> [ 3187.886827] 0000000000000000 ffff880175eff8e0 ffffffff8128d59e ffffffff823f5ee0
>>> [ 3187.898904] ffff880175eff958 ffffffff810a7a08 0000000000000000 ffff880179d1c5d0
>>> [ 3187.910954] 0000000000000004 0000000000000006 45422a91588a4c3e 0000000000000005
>>> [ 3187.923011] Call Trace:
>>> [ 3187.929451] [<ffffffff8128d59e>] dump_stack+0x4e/0x79
>>> [ 3187.938912] [<ffffffff810a7a08>] __lock_acquire+0x7ab/0x12af
>>> [ 3187.949027] [<ffffffff810a8d04>] lock_acquire+0x10e/0x1a9
>>> [ 3187.958859] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
>>> [ 3187.970476] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
>>> [ 3187.982011] [<ffffffff814df4a3>] mutex_lock_nested+0x71/0x346
>>> [ 3187.992167] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
>>> [ 3188.003655] [<ffffffff814e20ee>] ? _raw_spin_unlock_irqrestore+0x4b/0x60
>>> [ 3188.014829] [<ffffffff81364565>] ? __pm_runtime_resume+0x71/0x7e
>>> [ 3188.025269] [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915]
>>> [ 3188.036544] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915]
>>> [ 3188.047968] [<ffffffffa03e8cc4>] intel_hdmi_set_edid+0x3f/0xd6 [i915]
>>> [ 3188.058766] [<ffffffffa03e8e1a>] intel_hdmi_hot_plug+0xbf/0xfb [i915]
>>> [ 3188.069484] [<ffffffffa03c9e78>] intel_hpd_init+0xfa/0x10b [i915]
>>> [ 3188.079753] [<ffffffffa036968d>] vlv_display_power_well_init+0xdb/0xe8 [i915]
>>> [ 3188.091224] [<ffffffffa0369763>] chv_pipe_power_well_enable+0x62/0x67 [i915]
>>> [ 3188.102594] [<ffffffffa036ad55>] intel_display_power_get+0xa0/0xcb [i915]
>>> [ 3188.113657] [<ffffffffa03bdd7a>] modeset_get_crtc_power_domains+0x11d/0x13c [i915]
>>> [ 3188.125589] [<ffffffffa03bdfc1>] intel_atomic_commit+0x228/0xf1b [i915]
>>> [ 3188.136522] [<ffffffffa02c586c>] ? drm_atomic_check_only+0x37b/0x4da [drm]
>>> [ 3188.147676] [<ffffffffa02c5a18>] drm_atomic_commit+0x4d/0x52 [drm]
>>> [ 3188.158023] [<ffffffffa032fa25>] restore_fbdev_mode+0x11e/0x286 [drm_kms_helper]
>>> [ 3188.169734] [<ffffffffa0331443>] drm_fb_helper_restore_fbdev_mode_unlocked+0x36/0x6d [drm_kms_helper]
>>> [ 3188.183557] [<ffffffffa03d270f>] intel_fbdev_restore_mode+0x22/0x7a [i915]
>>> [ 3188.194701] [<ffffffffa03f70b8>] i915_driver_lastclose+0xe/0x15 [i915]
>>> [ 3188.205361] [<ffffffffa02ad4c2>] drm_lastclose+0x3f/0x10a [drm]
>>> [ 3188.215323] [<ffffffffa02ad9ee>] drm_release+0x461/0x49b [drm]
>>> [ 3188.225195] [<ffffffff8118577c>] __fput+0x100/0x1b3
>>> [ 3188.233956] [<ffffffff81185865>] ____fput+0xe/0x10
>>> [ 3188.242594] [<ffffffff81084540>] task_work_run+0x6a/0x93
>>> [ 3188.251816] [<ffffffff810015d6>] prepare_exit_to_usermode+0x9e/0xaf
>>> [ 3188.262111] [<ffffffff810017d6>] syscall_return_slowpath+0x1ef/0x264
>>> [ 3188.272486] [<ffffffff81084462>] ? task_work_add+0x44/0x53
>>> [ 3188.281885] [<ffffffff811858e3>] ? fput+0x7c/0x83
>>> [ 3188.290389] [<ffffffff810a6e52>] ? trace_hardirqs_on_caller+0x16/0x196
>>> [ 3188.300941] [<ffffffff81000c87>] ? trace_hardirqs_on_thunk+0x17/0x19
>>> [ 3188.311245] [<ffffffff814e2ab1>] int_ret_from_sys_call+0x25/0x9f
>>>
>>>
>>>> }
>>>>
>>>> void intel_hpd_init_work(struct drm_i915_private *dev_priv)
>>>> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>>>> index 05521b5..55859e9 100644
>>>> --- a/drivers/gpu/drm/i915/intel_sdvo.c
>>>> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
>>>> @@ -2433,7 +2433,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>>>> * Ensure that they get re-enabled when an interrupt happens.
>>>> */
>>>> intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
>>>> - intel_sdvo_enable_hotplug(intel_encoder);
>>>> } else {
>>>> intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
>>>> }
>>>> --
>>>> 1.7.10.4
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>> --
>>> Ville Syrjälä
>>> Intel OTC
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
2015-10-08 19:54 ` Daniel Vetter
2015-10-09 4:31 ` Jindal, Sonika
@ 2015-12-10 4:27 ` Sonika Jindal
1 sibling, 0 replies; 12+ messages in thread
From: Sonika Jindal @ 2015-12-10 4:27 UTC (permalink / raw)
To: intel-gfx
Call the encoders, call the hot_plug if it is registered.
This is required for connected boot and resume cases to generate
fake hpd resulting in reading of edid.
Removing the initial sdvo hot_plug call too so that it will be called
just once from this loop.
v2: Schedule a work function to call hot_plug
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_hotplug.c | 26 ++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_sdvo.c | 1 -
3 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bc865e23..4f037b9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -228,6 +228,7 @@ enum hpd_pin {
struct i915_hotplug {
struct work_struct hotplug_work;
+ struct work_struct edid_work;
struct {
unsigned long last_jiffies;
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index b177857..72d8fe8 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -442,6 +442,24 @@ void intel_hpd_irq_handler(struct drm_device *dev,
schedule_work(&dev_priv->hotplug.hotplug_work);
}
+static void i915_edid_work_func(struct work_struct *work)
+{
+ struct drm_i915_private *dev_priv =
+ container_of(work, struct drm_i915_private, hotplug.edid_work);
+ struct drm_device *dev = dev_priv->dev;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ struct intel_encoder *encoder;
+
+ mutex_lock(&mode_config->mutex);
+ list_for_each_entry(encoder, &mode_config->encoder_list,
+ base.head) {
+ if (encoder->hot_plug)
+ encoder->hot_plug(encoder);
+ }
+ mutex_unlock(&mode_config->mutex);
+ drm_helper_hpd_irq_event(dev);
+}
+
/**
* intel_hpd_init - initializes and enables hpd support
* @dev_priv: i915 device instance
@@ -482,12 +500,19 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
if (dev_priv->display.hpd_irq_setup)
dev_priv->display.hpd_irq_setup(dev);
spin_unlock_irq(&dev_priv->irq_lock);
+
+ /*
+ * Connected boot / resume scenarios can't generate new hot plug.
+ * So, probe it manually.
+ */
+ schedule_work(&dev_priv->hotplug.edid_work);
}
void intel_hpd_init_work(struct drm_i915_private *dev_priv)
{
INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func);
INIT_WORK(&dev_priv->hotplug.dig_port_work, i915_digport_work_func);
+ INIT_WORK(&dev_priv->hotplug.edid_work, i915_edid_work_func);
INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
intel_hpd_irq_storm_reenable_work);
}
@@ -504,5 +529,6 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
cancel_work_sync(&dev_priv->hotplug.dig_port_work);
cancel_work_sync(&dev_priv->hotplug.hotplug_work);
+ cancel_work_sync(&dev_priv->hotplug.edid_work);
cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
}
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 06679f1..4238a02 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2466,7 +2466,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
* Ensure that they get re-enabled when an interrupt happens.
*/
intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
- intel_sdvo_enable_hotplug(intel_encoder);
} else {
intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
2015-10-08 13:35 ` Ville Syrjälä
2015-10-08 14:38 ` Jani Nikula
@ 2015-10-12 12:24 ` Sharma, Shashank
2015-10-15 1:32 ` Jindal, Sonika
1 sibling, 1 reply; 12+ messages in thread
From: Sharma, Shashank @ 2015-10-12 12:24 UTC (permalink / raw)
To: Ville Syrjälä, Vetter, Daniel; +Cc: intel-gfx
We were debugging this issue, and we could find the root cause:
In function:
Intel_hpd_init()
|
encoder->hotplug()
|
display_power_get()
|
Intel_powe_well_enable()
|
power_well->ops->enable()
|
chv_pipe_power_well_enable()
|
vlv_display_power_well_init()
|
intel_hdp_init()
This function ends up calling intel_hpd_init, Which is causing the mutex deadlock due to recursion, as we are calling encoder->hotplug() from hpd_init().
Intel_hpd_init()
|
encoder->hotplug()
|
display_power_get()
There are two solutions here:
- remove hpd_init() from get_calls
- remove encoder->hotplug() function call from hpd_init() and put it in some other place. We have added this function from encoder_init() for android trees, and it works well there.
Regards
Shashank
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Ville Syrjälä
Sent: Thursday, October 08, 2015 7:06 PM
To: Jindal, Sonika
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
On Mon, Oct 05, 2015 at 04:43:14PM +0530, Sonika Jindal wrote:
> For all the encoders, call the hot_plug if it is registered.
> This is required for connected boot and resume cases to generate fake
> hpd resulting in reading of edid.
> Removing the initial sdvo hot_plug call too so that it will be called
> just once from this loop.
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
> drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++++++
> drivers/gpu/drm/i915/intel_sdvo.c | 1 -
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 53c0173..eac4757 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -458,6 +458,7 @@ void intel_hpd_init(struct drm_i915_private
> *dev_priv) {
> struct drm_device *dev = dev_priv->dev;
> struct drm_mode_config *mode_config = &dev->mode_config;
> + struct intel_encoder *encoder;
> struct drm_connector *connector;
> int i;
>
> @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> if (dev_priv->display.hpd_irq_setup)
> dev_priv->display.hpd_irq_setup(dev);
> spin_unlock_irq(&dev_priv->irq_lock);
> +
> + /*
> + * Connected boot / resume scenarios can't generate new hot plug.
> + * So, probe it manually.
> + */
> + list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> + base.head) {
> + if (encoder->hot_plug)
> + encoder->hot_plug(encoder);
> + }
This breaks the world on CHV
[ 3187.575198] [drm:intel_hdmi_hot_plug] Live status not up!
[ 3187.585154] =============================================
[ 3187.595010] [ INFO: possible recursive locking detected ]
[ 3187.604685] 4.3.0-rc4-bsw+ #2488 Tainted: G U W
[ 3187.614366] ---------------------------------------------
[ 3187.623892] Xorg/32212 is trying to acquire lock:
[ 3187.632635] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3187.647492] [ 3187.647492] but task is already holding lock:
[ 3187.661054] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3187.675960] [ 3187.675960] other info that might help us debug this:
[ 3187.690459] Possible unsafe locking scenario:
[ 3187.690459]
[ 3187.704224] CPU0
[ 3187.710485] ----
[ 3187.716711] lock(&power_domains->lock);
[ 3187.724718] lock(&power_domains->lock);
[ 3187.732663]
[ 3187.732663] *** DEADLOCK ***
[ 3187.732663]
[ 3187.749460] May be due to missing lock nesting notation [ 3187.749460] [ 3187.763833] 5 locks held by Xorg/32212:
[ 3187.771523] #0: (drm_global_mutex){+.+.+.}, at: [<ffffffffa02ad5c8>] drm_release+0x3b/0x49b [drm] [ 3187.785216] #1: (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffffa02c4d85>] drm_modeset_lock_all+0x54/0xcd [drm] [ 3187.800437] #2: (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffffa02c4d8f>] drm_modeset_lock_all+0x5e/0xcd [drm] [ 3187.815488] #3: (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffffa02c46b0>] drm_modeset_lock+0x75/0xfc [drm] [ 3187.830094] #4: (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3187.845534] [ 3187.845534] stack backtrace:
[ 3187.857685] CPU: 2 PID: 32212 Comm: Xorg Tainted: G U W 4.3.0-rc4-bsw+ #2488
[ 3187.870331] Hardware name: Intel Corporation CHERRYVIEW C0 PLATFORM/Braswell CRB, BIOS BRAS.X64.B085.R00.1509110553 09/11/2015 [ 3187.886827] 0000000000000000 ffff880175eff8e0 ffffffff8128d59e ffffffff823f5ee0 [ 3187.898904] ffff880175eff958 ffffffff810a7a08 0000000000000000 ffff880179d1c5d0 [ 3187.910954] 0000000000000004 0000000000000006 45422a91588a4c3e 0000000000000005 [ 3187.923011] Call Trace:
[ 3187.929451] [<ffffffff8128d59e>] dump_stack+0x4e/0x79 [ 3187.938912] [<ffffffff810a7a08>] __lock_acquire+0x7ab/0x12af [ 3187.949027] [<ffffffff810a8d04>] lock_acquire+0x10e/0x1a9 [ 3187.958859] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3187.970476] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3187.982011] [<ffffffff814df4a3>] mutex_lock_nested+0x71/0x346 [ 3187.992167] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3188.003655] [<ffffffff814e20ee>] ? _raw_spin_unlock_irqrestore+0x4b/0x60
[ 3188.014829] [<ffffffff81364565>] ? __pm_runtime_resume+0x71/0x7e [ 3188.025269] [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3188.036544] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3188.047968] [<ffffffffa03e8cc4>] intel_hdmi_set_edid+0x3f/0xd6 [i915] [ 3188.058766] [<ffffffffa03e8e1a>] intel_hdmi_hot_plug+0xbf/0xfb [i915] [ 3188.069484] [<ffffffffa03c9e78>] intel_hpd_init+0xfa/0x10b [i915] [ 3188.079753] [<ffffffffa036968d>] vlv_display_power_well_init+0xdb/0xe8 [i915] [ 3188.091224] [<ffffffffa0369763>] chv_pipe_power_well_enable+0x62/0x67 [i915] [ 3188.102594] [<ffffffffa036ad55>] intel_display_power_get+0xa0/0xcb [i915] [ 3188.113657] [<ffffffffa03bdd7a>] modeset_get_crtc_power_domains+0x11d/0x13c [i915] [ 3188.125589] [<ffffffffa03bdfc1>] intel_atomic_commit+0x228/0xf1b [i915] [ 3188.136522] [<ffffffffa02c586c>] ? drm_atomic_check_only+0x37b/0x4da [drm] [ 3188.147676] [<ffffffffa02c5a18>] drm_atomic_commit+0x4d/0x52 [drm] [ 3188.158023] [<ffffffffa032fa25>] restore_fbdev_mode+0x11e/0x286 [drm_kms_helper] [ 3188.169734] [<ffffffffa0331443>] drm_fb_helper_restore_fbdev_mode_unlocked+0x36/0x6d [drm_kms_helper] [ 3188.183557] [<ffffffffa03d270f>] intel_fbdev_restore_mode+0x22/0x7a [i915] [ 3188.194701] [<ffffffffa03f70b8>] i915_driver_lastclose+0xe/0x15 [i915] [ 3188.205361] [<ffffffffa02ad4c2>] drm_lastclose+0x3f/0x10a [drm] [ 3188.215323] [<ffffffffa02ad9ee>] drm_release+0x461/0x49b [drm] [ 3188.225195] [<ffffffff8118577c>] __fput+0x100/0x1b3 [ 3188.233956] [<ffffffff81185865>] ____fput+0xe/0x10 [ 3188.242594] [<ffffffff81084540>] task_work_run+0x6a/0x93 [ 3188.251816] [<ffffffff810015d6>] prepare_exit_to_usermode+0x9e/0xaf
[ 3188.262111] [<ffffffff810017d6>] syscall_return_slowpath+0x1ef/0x264
[ 3188.272486] [<ffffffff81084462>] ? task_work_add+0x44/0x53 [ 3188.281885] [<ffffffff811858e3>] ? fput+0x7c/0x83 [ 3188.290389] [<ffffffff810a6e52>] ? trace_hardirqs_on_caller+0x16/0x196
[ 3188.300941] [<ffffffff81000c87>] ? trace_hardirqs_on_thunk+0x17/0x19 [ 3188.311245] [<ffffffff814e2ab1>] int_ret_from_sys_call+0x25/0x9f
> }
>
> void intel_hpd_init_work(struct drm_i915_private *dev_priv) diff
> --git a/drivers/gpu/drm/i915/intel_sdvo.c
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index 05521b5..55859e9 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2433,7 +2433,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> * Ensure that they get re-enabled when an interrupt happens.
> */
> intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
> - intel_sdvo_enable_hotplug(intel_encoder);
> } else {
> intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
> }
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
2015-10-12 12:24 ` Sharma, Shashank
@ 2015-10-15 1:32 ` Jindal, Sonika
0 siblings, 0 replies; 12+ messages in thread
From: Jindal, Sonika @ 2015-10-15 1:32 UTC (permalink / raw)
To: Sharma, Shashank, Ville Syrjälä, Vetter, Daniel; +Cc: intel-gfx
Had some discussion with Daniel on IRC about how we can fix the deadlock.
We couldn't decide upon any solution but I can give it a try.
He suggested to have a work function for ->hot_plug which will get the power domain lock and not allow ->hot_plug if the power_domain is shut.
Then when it calls hpt_init again from power_domain get, ->hot_plug gets called.
But this looks racy, we can try this out.
If there are any other suggestions, please let me know.
I will work on this after 26th Oct.
Regards,
Sonika
-----Original Message-----
From: Sharma, Shashank
Sent: Monday, October 12, 2015 5:54 PM
To: Ville Syrjälä; Vetter, Daniel
Cc: intel-gfx@lists.freedesktop.org; Mukherjee, Indranil; Jindal, Sonika
Subject: RE: [Intel-gfx] [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
We were debugging this issue, and we could find the root cause:
In function:
Intel_hpd_init()
|
encoder->hotplug()
|
display_power_get()
|
Intel_powe_well_enable()
|
power_well->ops->enable()
|
chv_pipe_power_well_enable()
|
vlv_display_power_well_init()
|
intel_hdp_init()
This function ends up calling intel_hpd_init, Which is causing the mutex deadlock due to recursion, as we are calling encoder->hotplug() from hpd_init().
Intel_hpd_init()
|
encoder->hotplug()
|
display_power_get()
There are two solutions here:
- remove hpd_init() from get_calls
- remove encoder->hotplug() function call from hpd_init() and put it in some other place. We have added this function from encoder_init() for android trees, and it works well there.
Regards
Shashank
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Ville Syrjälä
Sent: Thursday, October 08, 2015 7:06 PM
To: Jindal, Sonika
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases
On Mon, Oct 05, 2015 at 04:43:14PM +0530, Sonika Jindal wrote:
> For all the encoders, call the hot_plug if it is registered.
> This is required for connected boot and resume cases to generate fake
> hpd resulting in reading of edid.
> Removing the initial sdvo hot_plug call too so that it will be called
> just once from this loop.
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> ---
> drivers/gpu/drm/i915/intel_hotplug.c | 11 +++++++++++
> drivers/gpu/drm/i915/intel_sdvo.c | 1 -
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 53c0173..eac4757 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -458,6 +458,7 @@ void intel_hpd_init(struct drm_i915_private
> *dev_priv) {
> struct drm_device *dev = dev_priv->dev;
> struct drm_mode_config *mode_config = &dev->mode_config;
> + struct intel_encoder *encoder;
> struct drm_connector *connector;
> int i;
>
> @@ -482,6 +483,16 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> if (dev_priv->display.hpd_irq_setup)
> dev_priv->display.hpd_irq_setup(dev);
> spin_unlock_irq(&dev_priv->irq_lock);
> +
> + /*
> + * Connected boot / resume scenarios can't generate new hot plug.
> + * So, probe it manually.
> + */
> + list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> + base.head) {
> + if (encoder->hot_plug)
> + encoder->hot_plug(encoder);
> + }
This breaks the world on CHV
[ 3187.575198] [drm:intel_hdmi_hot_plug] Live status not up!
[ 3187.585154] =============================================
[ 3187.595010] [ INFO: possible recursive locking detected ]
[ 3187.604685] 4.3.0-rc4-bsw+ #2488 Tainted: G U W
[ 3187.614366] ---------------------------------------------
[ 3187.623892] Xorg/32212 is trying to acquire lock:
[ 3187.632635] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3187.647492] [ 3187.647492] but task is already holding lock:
[ 3187.661054] (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3187.675960] [ 3187.675960] other info that might help us debug this:
[ 3187.690459] Possible unsafe locking scenario:
[ 3187.690459]
[ 3187.704224] CPU0
[ 3187.710485] ----
[ 3187.716711] lock(&power_domains->lock);
[ 3187.724718] lock(&power_domains->lock);
[ 3187.732663]
[ 3187.732663] *** DEADLOCK ***
[ 3187.732663]
[ 3187.749460] May be due to missing lock nesting notation [ 3187.749460] [ 3187.763833] 5 locks held by Xorg/32212:
[ 3187.771523] #0: (drm_global_mutex){+.+.+.}, at: [<ffffffffa02ad5c8>] drm_release+0x3b/0x49b [drm] [ 3187.785216] #1: (&dev->mode_config.mutex){+.+.+.}, at: [<ffffffffa02c4d85>] drm_modeset_lock_all+0x54/0xcd [drm] [ 3187.800437] #2: (crtc_ww_class_acquire){+.+.+.}, at: [<ffffffffa02c4d8f>] drm_modeset_lock_all+0x5e/0xcd [drm] [ 3187.815488] #3: (crtc_ww_class_mutex){+.+.+.}, at: [<ffffffffa02c46b0>] drm_modeset_lock+0x75/0xfc [drm] [ 3187.830094] #4: (&power_domains->lock){+.+...}, at: [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3187.845534] [ 3187.845534] stack backtrace:
[ 3187.857685] CPU: 2 PID: 32212 Comm: Xorg Tainted: G U W 4.3.0-rc4-bsw+ #2488
[ 3187.870331] Hardware name: Intel Corporation CHERRYVIEW C0 PLATFORM/Braswell CRB, BIOS BRAS.X64.B085.R00.1509110553 09/11/2015 [ 3187.886827] 0000000000000000 ffff880175eff8e0 ffffffff8128d59e ffffffff823f5ee0 [ 3187.898904] ffff880175eff958 ffffffff810a7a08 0000000000000000 ffff880179d1c5d0 [ 3187.910954] 0000000000000004 0000000000000006 45422a91588a4c3e 0000000000000005 [ 3187.923011] Call Trace:
[ 3187.929451] [<ffffffff8128d59e>] dump_stack+0x4e/0x79 [ 3187.938912] [<ffffffff810a7a08>] __lock_acquire+0x7ab/0x12af [ 3187.949027] [<ffffffff810a8d04>] lock_acquire+0x10e/0x1a9 [ 3187.958859] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3187.970476] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3187.982011] [<ffffffff814df4a3>] mutex_lock_nested+0x71/0x346 [ 3187.992167] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3188.003655] [<ffffffff814e20ee>] ? _raw_spin_unlock_irqrestore+0x4b/0x60
[ 3188.014829] [<ffffffff81364565>] ? __pm_runtime_resume+0x71/0x7e [ 3188.025269] [<ffffffffa036aced>] intel_display_power_get+0x38/0xcb [i915] [ 3188.036544] [<ffffffffa036aced>] ? intel_display_power_get+0x38/0xcb [i915] [ 3188.047968] [<ffffffffa03e8cc4>] intel_hdmi_set_edid+0x3f/0xd6 [i915] [ 3188.058766] [<ffffffffa03e8e1a>] intel_hdmi_hot_plug+0xbf/0xfb [i915] [ 3188.069484] [<ffffffffa03c9e78>] intel_hpd_init+0xfa/0x10b [i915] [ 3188.079753] [<ffffffffa036968d>] vlv_display_power_well_init+0xdb/0xe8 [i915] [ 3188.091224] [<ffffffffa0369763>] chv_pipe_power_well_enable+0x62/0x67 [i915] [ 3188.102594] [<ffffffffa036ad55>] intel_display_power_get+0xa0/0xcb [i915] [ 3188.113657] [<ffffffffa03bdd7a>] modeset_get_crtc_power_domains+0x11d/0x13c [i915] [ 3188.125589] [<ffffffffa03bdfc1>] intel_atomic_commit+0x228/0xf1b [i915] [ 3188.136522] [<ffffffffa02c586c>] ? drm_atomic_check_only+0x37b/0x4da [drm] [ 3188.147676] [<ffffffffa02c5a18>] drm_atomic_commit+0x4d/0x52 [drm] [ 3188.158023] [<ffffffffa032fa25>] restore_fbdev_mode+0x11e/0x286 [drm_kms_helper] [ 3188.169734] [<ffffffffa0331443>] drm_fb_helper_restore_fbdev_mode_unlocked+0x36/0x6d [drm_kms_helper] [ 3188.183557] [<ffffffffa03d270f>] intel_fbdev_restore_mode+0x22/0x7a [i915] [ 3188.194701] [<ffffffffa03f70b8>] i915_driver_lastclose+0xe/0x15 [i915] [ 3188.205361] [<ffffffffa02ad4c2>] drm_lastclose+0x3f/0x10a [drm] [ 3188.215323] [<ffffffffa02ad9ee>] drm_release+0x461/0x49b [drm] [ 3188.225195] [<ffffffff8118577c>] __fput+0x100/0x1b3 [ 3188.233956] [<ffffffff81185865>] ____fput+0xe/0x10 [ 3188.242594] [<ffffffff81084540>] task_work_run+0x6a/0x93 [ 3188.251816] [<ffffffff810015d6>] prepare_exit_to_usermode+0x9e/0xaf
[ 3188.262111] [<ffffffff810017d6>] syscall_return_slowpath+0x1ef/0x264
[ 3188.272486] [<ffffffff81084462>] ? task_work_add+0x44/0x53 [ 3188.281885] [<ffffffff811858e3>] ? fput+0x7c/0x83 [ 3188.290389] [<ffffffff810a6e52>] ? trace_hardirqs_on_caller+0x16/0x196
[ 3188.300941] [<ffffffff81000c87>] ? trace_hardirqs_on_thunk+0x17/0x19 [ 3188.311245] [<ffffffff814e2ab1>] int_ret_from_sys_call+0x25/0x9f
> }
>
> void intel_hpd_init_work(struct drm_i915_private *dev_priv) diff
> --git a/drivers/gpu/drm/i915/intel_sdvo.c
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index 05521b5..55859e9 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2433,7 +2433,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> * Ensure that they get re-enabled when an interrupt happens.
> */
> intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
> - intel_sdvo_enable_hotplug(intel_encoder);
> } else {
> intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
> }
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-12-17 4:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-16 10:48 [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases Sonika Jindal
2015-12-16 10:48 ` [PATCH 2/2] drm/i915: Add hot_plug hook for hdmi encoder Sonika Jindal
2015-12-16 13:46 ` [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases Daniel Vetter
2015-12-17 4:23 ` Jindal, Sonika
-- strict thread matches above, loose matches on Subject: below --
2015-09-28 13:34 [PATCH] drm/i915: Add hot_plug hook for hdmi encoder Daniel Vetter
2015-10-05 11:13 ` [PATCH 1/2] drm/i915: Call encoder hotplug for init and resume cases Sonika Jindal
2015-10-08 13:35 ` Ville Syrjälä
2015-10-08 14:38 ` Jani Nikula
2015-10-08 19:54 ` Daniel Vetter
2015-10-09 4:31 ` Jindal, Sonika
2015-12-10 4:27 ` Sonika Jindal
2015-10-12 12:24 ` Sharma, Shashank
2015-10-15 1:32 ` Jindal, Sonika
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.