All of lore.kernel.org
 help / color / mirror / Atom feed
* [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
* 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

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.