diff mbox

[1/2] drm/i915: Call encoder hotplug for init and resume cases

Message ID 1450262886-10156-1-git-send-email-sonika.jindal@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sonika.jindal@intel.com Dec. 16, 2015, 10:48 a.m. UTC
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(-)

Comments

Daniel Vetter Dec. 16, 2015, 1:46 p.m. UTC | #1
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
sonika.jindal@intel.com Dec. 17, 2015, 4:23 a.m. UTC | #2
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
diff mbox

Patch

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;
 	}