linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Add detection of changing of edid on between suspend and resume
@ 2018-08-03 16:34 Gwan-gyeong Mun
  2018-08-06 12:09 ` [v2] " Gwan-gyeong Mun
  2018-08-09 11:13 ` [v3] " Gwan-gyeong Mun
  0 siblings, 2 replies; 5+ messages in thread
From: Gwan-gyeong Mun @ 2018-08-03 16:34 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
	intel-gfx, dri-devel

The hotplug detection routine of i915 uses drm_helper_hpd_irq_event(). This helper
can detect changing of status of connector, but it can not detect changing of
edid. Following scenario requires detection of changing of edid.

 1) plug display device to a connector
 2) system suspend
 3) unplug 1)'s display device and plug the other display device to a connector
 4) system resume

It adds edid check routine when a connector status still remains as "connector_status_connected".

Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
Testcase: igt/kms_chamelium/dp-edid-change-during-suspend

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/intel_hotplug.c | 80 +++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 648a13c6043c..b92b83d46ccc 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -507,6 +507,84 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
 	}
 }
 
+/**
+ * intel_hpd_irq_event - hotplug processing
+ * @dev: drm_device
+ *
+ * Drivers can use this function to run a detect cycle on all connectors which
+ * have the DRM_CONNECTOR_POLL_HPD flag set in their &polled member. All other
+ * connectors are ignored, which is useful to avoid reprobing fixed panels.
+ *
+ * This function is useful for drivers which can't or don't track hotplug interrupts
+ * for each connector. This function is based on drm_helper_hpd_irq_event() helper
+ * function and besides it adds edid check routine when a connector status still
+ * remains as "connector_status_connected".
+ *
+ * Following scenario requires detection of changing of edid.
+ *  1) plug display device to a connector
+ *  2) system suspend
+ *  3) unplug 1)'s display device and plug the other display device to a connector
+ *  4) system resume
+
+ * This function must be called from process context with no mode
+ * setting locks held.
+ *
+ * Note that a connector can be both polled and probed from the hotplug handler,
+ * in case the hotplug interrupt is known to be unreliable.
+ */
+static bool intel_hpd_irq_event(struct drm_device *dev)
+{
+	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+	enum drm_connector_status old_status, cur_status;
+	struct edid *old_edid;
+	bool changed = false;
+
+	if (!dev->mode_config.poll_enabled)
+		return false;
+
+	mutex_lock(&dev->mode_config.mutex);
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		/* Only handle HPD capable connectors. */
+		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
+			continue;
+
+		old_status = connector->status;
+		old_edid = to_intel_connector(connector)->detect_edid;
+
+		cur_status = drm_helper_probe_detect(connector, NULL, false);
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
+			      connector->base.id, connector->name,
+			      drm_get_connector_status_name(old_status),
+			      drm_get_connector_status_name(cur_status));
+
+		if (old_status != cur_status)
+			changed = true;
+
+		/* Check changing of edid when a connector status still remains
+		 * as "connector_status_connected". */
+		if (old_status == cur_status &&
+		    cur_status == connector_status_connected) {
+			struct edid *cur_edid = to_intel_connector(connector)->detect_edid;
+
+			if (memcmp(old_edid, cur_edid, sizeof(*cur_edid)) != 0) {
+				changed = true;
+				DRM_DEBUG_KMS("[CONNECTOR:%d:%s] edid updated\n",
+					      connector->base.id,
+					      connector->name);
+			}
+		}
+	}
+	drm_connector_list_iter_end(&conn_iter);
+	mutex_unlock(&dev->mode_config.mutex);
+
+	if (changed)
+		drm_kms_helper_hotplug_event(dev);
+
+	return changed;
+}
+
 static void i915_hpd_poll_init_work(struct work_struct *work)
 {
 	struct drm_i915_private *dev_priv =
@@ -552,7 +630,7 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
 	 * in the middle of disabling polling
 	 */
 	if (!enabled)
-		drm_helper_hpd_irq_event(dev);
+		intel_hpd_irq_event(dev);
 }
 
 /**
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [v2] drm/i915: Add detection of changing of edid on between suspend and resume
  2018-08-03 16:34 [PATCH] drm/i915: Add detection of changing of edid on between suspend and resume Gwan-gyeong Mun
@ 2018-08-06 12:09 ` Gwan-gyeong Mun
  2018-08-09 11:13 ` [v3] " Gwan-gyeong Mun
  1 sibling, 0 replies; 5+ messages in thread
From: Gwan-gyeong Mun @ 2018-08-06 12:09 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Joonas Lahtinen, Rodrigo Vivi, David Airlie, intel-gfx,
	dri-devel, linux-kernel

The hotplug detection routine of i915 uses drm_helper_hpd_irq_event(). This
helper can detect changing of status of connector, but it can not detect changing
of edid. Following scenario requires detection of changing of edid.

 1) plug display device to a connector
 2) system suspend
 3) unplug 1)'s display device and plug the other display device to a connector
 4) system resume

It adds edid check routine when a connector status still remains as "connector_status_connected".

v2: Add NULL check before comparing of EDIDs.

Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
Testcase: igt/kms_chamelium/dp-edid-change-during-suspend

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/intel_hotplug.c | 83 +++++++++++++++++++++++++++-
 1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 648a13c6043c..0bd3e1c38167 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -507,6 +507,87 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
 	}
 }
 
+/**
+ * intel_hpd_irq_event - hotplug processing
+ * @dev: drm_device
+ *
+ * Drivers can use this function to run a detect cycle on all connectors which
+ * have the DRM_CONNECTOR_POLL_HPD flag set in their &polled member. All other
+ * connectors are ignored, which is useful to avoid reprobing fixed panels.
+ *
+ * This function is useful for drivers which can't or don't track hotplug interrupts
+ * for each connector. This function is based on drm_helper_hpd_irq_event() helper
+ * function and besides it adds edid check routine when a connector status still
+ * remains as "connector_status_connected".
+ *
+ * Following scenario requires detection of changing of edid.
+ *  1) plug display device to a connector
+ *  2) system suspend
+ *  3) unplug 1)'s display device and plug the other display device to a connector
+ *  4) system resume
+
+ * This function must be called from process context with no mode
+ * setting locks held.
+ *
+ * Note that a connector can be both polled and probed from the hotplug handler,
+ * in case the hotplug interrupt is known to be unreliable.
+ */
+static bool intel_hpd_irq_event(struct drm_device *dev)
+{
+	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+	enum drm_connector_status old_status, cur_status;
+	struct edid *old_edid;
+	bool changed = false;
+
+	if (!dev->mode_config.poll_enabled)
+		return false;
+
+	mutex_lock(&dev->mode_config.mutex);
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		/* Only handle HPD capable connectors. */
+		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
+			continue;
+
+		old_status = connector->status;
+		old_edid = to_intel_connector(connector)->detect_edid;
+
+		cur_status = drm_helper_probe_detect(connector, NULL, false);
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
+			      connector->base.id, connector->name,
+			      drm_get_connector_status_name(old_status),
+			      drm_get_connector_status_name(cur_status));
+
+		if (old_status != cur_status)
+			changed = true;
+
+		/* Check changing of edid when a connector status still remains
+		 * as "connector_status_connected".
+		 */
+		if (old_status == cur_status &&
+		    cur_status == connector_status_connected) {
+			struct edid *cur_edid = to_intel_connector(connector)->detect_edid;
+			if (!old_edid || !cur_edid)
+				continue;
+
+			if (memcmp(old_edid, cur_edid, sizeof(*cur_edid))) {
+				changed = true;
+				DRM_DEBUG_KMS("[CONNECTOR:%d:%s] edid updated\n",
+					      connector->base.id,
+					      connector->name);
+			}
+		}
+	}
+	drm_connector_list_iter_end(&conn_iter);
+	mutex_unlock(&dev->mode_config.mutex);
+
+	if (changed)
+		drm_kms_helper_hotplug_event(dev);
+
+	return changed;
+}
+
 static void i915_hpd_poll_init_work(struct work_struct *work)
 {
 	struct drm_i915_private *dev_priv =
@@ -552,7 +633,7 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
 	 * in the middle of disabling polling
 	 */
 	if (!enabled)
-		drm_helper_hpd_irq_event(dev);
+		intel_hpd_irq_event(dev);
 }
 
 /**
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [v3] drm/i915: Add detection of changing of edid on between suspend and resume
  2018-08-03 16:34 [PATCH] drm/i915: Add detection of changing of edid on between suspend and resume Gwan-gyeong Mun
  2018-08-06 12:09 ` [v2] " Gwan-gyeong Mun
@ 2018-08-09 11:13 ` Gwan-gyeong Mun
  2018-08-10 13:02   ` [Intel-gfx] " Lisovskiy, Stanislav
  1 sibling, 1 reply; 5+ messages in thread
From: Gwan-gyeong Mun @ 2018-08-09 11:13 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Joonas Lahtinen, Rodrigo Vivi, David Airlie, intel-gfx,
	dri-devel, linux-kernel

The hotplug detection routine of i915 uses drm_helper_hpd_irq_event(). This
helper can detect changing of status of connector, but it can not detect
changing of edid.

Following scenario requires detection of changing of edid.

 1) plug display device to a connector
 2) system suspend
 3) unplug 1)'s display device and plug the other display device to a connector
 4) system resume

It adds edid check routine when a connector status still remains as "connector_status_connected".

v2: Add NULL check before comparing of EDIDs.

Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
Testcase: igt/kms_chamelium/dp-edid-change-during-suspend

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/intel_hotplug.c | 84 +++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 648a13c6043c..965f2d771fc0 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -507,6 +507,88 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
 	}
 }
 
+/**
+ * intel_hpd_irq_event - hotplug processing
+ * @dev: drm_device
+ *
+ * Drivers can use this function to run a detect cycle on all connectors which
+ * have the DRM_CONNECTOR_POLL_HPD flag set in their &polled member. All other
+ * connectors are ignored, which is useful to avoid reprobing fixed panels.
+ *
+ * This function is useful for drivers which can't or don't track hotplug interrupts
+ * for each connector. This function is based on drm_helper_hpd_irq_event() helper
+ * function and besides it adds edid check routine when a connector status still
+ * remains as "connector_status_connected".
+ *
+ * Following scenario requires detection of changing of edid.
+ *  1) plug display device to a connector
+ *  2) system suspend
+ *  3) unplug 1)'s display device and plug the other display device to a connector
+ *  4) system resume
+
+ * This function must be called from process context with no mode
+ * setting locks held.
+ *
+ * Note that a connector can be both polled and probed from the hotplug handler,
+ * in case the hotplug interrupt is known to be unreliable.
+ */
+static bool intel_hpd_irq_event(struct drm_device *dev)
+{
+	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+	enum drm_connector_status old_status, cur_status;
+	struct edid *old_edid;
+	bool changed = false;
+
+	if (!dev->mode_config.poll_enabled)
+		return false;
+
+	mutex_lock(&dev->mode_config.mutex);
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		/* Only handle HPD capable connectors. */
+		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
+			continue;
+
+		old_status = connector->status;
+		old_edid = to_intel_connector(connector)->detect_edid;
+
+		cur_status = drm_helper_probe_detect(connector, NULL, false);
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
+			      connector->base.id, connector->name,
+			      drm_get_connector_status_name(old_status),
+			      drm_get_connector_status_name(cur_status));
+
+		if (old_status != cur_status)
+			changed = true;
+
+		/* Check changing of edid when a connector status still remains
+		 * as "connector_status_connected".
+		 */
+		if (old_status == cur_status &&
+		    cur_status == connector_status_connected) {
+			struct edid *cur_edid = to_intel_connector(connector)->detect_edid;
+
+			if (!old_edid || !cur_edid)
+				continue;
+
+			if (memcmp(old_edid, cur_edid, sizeof(*cur_edid))) {
+				changed = true;
+				DRM_DEBUG_KMS("[CONNECTOR:%d:%s] edid updated\n",
+					      connector->base.id,
+					      connector->name);
+			}
+		}
+	}
+	drm_connector_list_iter_end(&conn_iter);
+	mutex_unlock(&dev->mode_config.mutex);
+
+	if (changed)
+		drm_kms_helper_hotplug_event(dev);
+
+	return changed;
+}
+
 static void i915_hpd_poll_init_work(struct work_struct *work)
 {
 	struct drm_i915_private *dev_priv =
@@ -552,7 +634,7 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
 	 * in the middle of disabling polling
 	 */
 	if (!enabled)
-		drm_helper_hpd_irq_event(dev);
+		intel_hpd_irq_event(dev);
 }
 
 /**
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [v3] drm/i915: Add detection of changing of edid on between suspend and resume
  2018-08-09 11:13 ` [v3] " Gwan-gyeong Mun
@ 2018-08-10 13:02   ` Lisovskiy, Stanislav
  2018-08-13 10:55     ` Mika Kahola
  0 siblings, 1 reply; 5+ messages in thread
From: Lisovskiy, Stanislav @ 2018-08-10 13:02 UTC (permalink / raw)
  To: Mun, Gwan-gyeong, jani.nikula
  Cc: dri-devel, intel-gfx, linux-kernel, Vivi, Rodrigo, airlied

On Thu, 2018-08-09 at 14:13 +0300, Gwan-gyeong Mun wrote:
> The hotplug detection routine of i915 uses
> drm_helper_hpd_irq_event(). This
> helper can detect changing of status of connector, but it can not
> detect
> changing of edid.
> 
> Following scenario requires detection of changing of edid.
> 
>  1) plug display device to a connector
>  2) system suspend
>  3) unplug 1)'s display device and plug the other display device to a
> connector
>  4) system resume
> 
> It adds edid check routine when a connector status still remains as
> "connector_status_connected".
> 
> v2: Add NULL check before comparing of EDIDs.
> 
> Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
> Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
> Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
> Testcase: igt/kms_chamelium/dp-edid-change-during-suspend
> 
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_hotplug.c | 84
> +++++++++++++++++++++++++++-
>  1 file changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> b/drivers/gpu/drm/i915/intel_hotplug.c
> index 648a13c6043c..965f2d771fc0 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -507,6 +507,88 @@ void intel_hpd_init(struct drm_i915_private
> *dev_priv)
>  	}
>  }
>  
> +/**
> + * intel_hpd_irq_event - hotplug processing
> + * @dev: drm_device
> + *
> + * Drivers can use this function to run a detect cycle on all
> connectors which
> + * have the DRM_CONNECTOR_POLL_HPD flag set in their &polled member.
> All other
> + * connectors are ignored, which is useful to avoid reprobing fixed
> panels.
> + *
> + * This function is useful for drivers which can't or don't track
> hotplug interrupts
> + * for each connector. This function is based on
> drm_helper_hpd_irq_event() helper
> + * function and besides it adds edid check routine when a connector
> status still
> + * remains as "connector_status_connected".
> + *
> + * Following scenario requires detection of changing of edid.
> + *  1) plug display device to a connector
> + *  2) system suspend
> + *  3) unplug 1)'s display device and plug the other display device
> to a connector
> + *  4) system resume
> +
> + * This function must be called from process context with no mode
> + * setting locks held.
> + *
> + * Note that a connector can be both polled and probed from the
> hotplug handler,
> + * in case the hotplug interrupt is known to be unreliable.
> + */
> +static bool intel_hpd_irq_event(struct drm_device *dev)
> +{
> +	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
> +	enum drm_connector_status old_status, cur_status;
> +	struct edid *old_edid;
> +	bool changed = false;
> +
> +	if (!dev->mode_config.poll_enabled)
> +		return false;
> +
> +	mutex_lock(&dev->mode_config.mutex);
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
> +		/* Only handle HPD capable connectors. */
> +		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
> +			continue;
> +
> +		old_status = connector->status;
> +		old_edid = to_intel_connector(connector)-
> >detect_edid;
> +
> +		cur_status = drm_helper_probe_detect(connector,
> NULL, false);
> +		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from
> %s to %s\n",
> +			      connector->base.id, connector->name,
> +			      drm_get_connector_status_name(old_stat
> us),
> +			      drm_get_connector_status_name(cur_stat
> us));
> +
> +		if (old_status != cur_status)
> +			changed = true;
> +
> +		/* Check changing of edid when a connector status
> still remains
> +		 * as "connector_status_connected".
> +		 */
> +		if (old_status == cur_status &&
> +		    cur_status == connector_status_connected) {
> +			struct edid *cur_edid =
> to_intel_connector(connector)->detect_edid;
> +
> +			if (!old_edid || !cur_edid)
> +				continue;
> +
> +			if (memcmp(old_edid, cur_edid,
> sizeof(*cur_edid))) {
> +				changed = true;
> +				DRM_DEBUG_KMS("[CONNECTOR:%d:%s]
> edid updated\n",
> +					      connector->base.id,
> +					      connector->name);
> +			}
> +		}
> +	}
> +	drm_connector_list_iter_end(&conn_iter);
> +	mutex_unlock(&dev->mode_config.mutex);
> +
> +	if (changed)
> +		drm_kms_helper_hotplug_event(dev);
> +
> +	return changed;
> +}
> +
>  static void i915_hpd_poll_init_work(struct work_struct *work)
>  {
>  	struct drm_i915_private *dev_priv =
> @@ -552,7 +634,7 @@ static void i915_hpd_poll_init_work(struct
> work_struct *work)
>  	 * in the middle of disabling polling
>  	 */
>  	if (!enabled)
> -		drm_helper_hpd_irq_event(dev);
> +		intel_hpd_irq_event(dev);
>  }

Just wondering, as I saw previously drm_helper_hpd_irq_event function
was used, which basically does the same thing, except doing memcmp
for detecting the edid change. Is it only Intel specific change? As we
could just add this modification to existing helper function or add
another helper function to the drm (drm_probe_helper.c), so that it is
also usable elsewhere..

>  
>  /**
-- 
Best Regards,

Lisovskiy Stanislav

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [v3] drm/i915: Add detection of changing of edid on between suspend and resume
  2018-08-10 13:02   ` [Intel-gfx] " Lisovskiy, Stanislav
@ 2018-08-13 10:55     ` Mika Kahola
  0 siblings, 0 replies; 5+ messages in thread
From: Mika Kahola @ 2018-08-13 10:55 UTC (permalink / raw)
  To: Lisovskiy, Stanislav, Mun, Gwan-gyeong, jani.nikula
  Cc: airlied, intel-gfx, linux-kernel, dri-devel, Vivi, Rodrigo

On Fri, 2018-08-10 at 13:02 +0000, Lisovskiy, Stanislav wrote:
> On Thu, 2018-08-09 at 14:13 +0300, Gwan-gyeong Mun wrote:
> > 
> > The hotplug detection routine of i915 uses
> > drm_helper_hpd_irq_event(). This
> > helper can detect changing of status of connector, but it can not
> > detect
> > changing of edid.
> > 
> > Following scenario requires detection of changing of edid.
> > 
> >  1) plug display device to a connector
> >  2) system suspend
> >  3) unplug 1)'s display device and plug the other display device to
> > a
> > connector
> >  4) system resume
> > 
> > It adds edid check routine when a connector status still remains as
> > "connector_status_connected".
> > 
> > v2: Add NULL check before comparing of EDIDs.
> > 
> > Testcase: igt/kms_chamelium/hdmi-edid-change-during-hibernate
> > Testcase: igt/kms_chamelium/hdmi-edid-change-during-suspend
> > Testcase: igt/kms_chamelium/dp-edid-change-during-hibernate
> > Testcase: igt/kms_chamelium/dp-edid-change-during-suspend
> > 
> > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_hotplug.c | 84
> > +++++++++++++++++++++++++++-
> >  1 file changed, 83 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c
> > b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 648a13c6043c..965f2d771fc0 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -507,6 +507,88 @@ void intel_hpd_init(struct drm_i915_private
> > *dev_priv)
> >  	}
> >  }
> >  
> > +/**
> > + * intel_hpd_irq_event - hotplug processing
> > + * @dev: drm_device
> > + *
> > + * Drivers can use this function to run a detect cycle on all
> > connectors which
> > + * have the DRM_CONNECTOR_POLL_HPD flag set in their &polled
> > member.
> > All other
> > + * connectors are ignored, which is useful to avoid reprobing
> > fixed
> > panels.
> > + *
> > + * This function is useful for drivers which can't or don't track
> > hotplug interrupts
> > + * for each connector. This function is based on
> > drm_helper_hpd_irq_event() helper
> > + * function and besides it adds edid check routine when a
> > connector
> > status still
> > + * remains as "connector_status_connected".
> > + *
> > + * Following scenario requires detection of changing of edid.
> > + *  1) plug display device to a connector
> > + *  2) system suspend
> > + *  3) unplug 1)'s display device and plug the other display
> > device
> > to a connector
> > + *  4) system resume
> > +
> > + * This function must be called from process context with no mode
> > + * setting locks held.
> > + *
> > + * Note that a connector can be both polled and probed from the
> > hotplug handler,
> > + * in case the hotplug interrupt is known to be unreliable.
> > + */
> > +static bool intel_hpd_irq_event(struct drm_device *dev)
> > +{
> > +	struct drm_connector *connector;
> > +	struct drm_connector_list_iter conn_iter;
> > +	enum drm_connector_status old_status, cur_status;
> > +	struct edid *old_edid;
> > +	bool changed = false;
> > +
> > +	if (!dev->mode_config.poll_enabled)
> > +		return false;
> > +
> > +	mutex_lock(&dev->mode_config.mutex);
> > +	drm_connector_list_iter_begin(dev, &conn_iter);
> > +	drm_for_each_connector_iter(connector, &conn_iter) {
> > +		/* Only handle HPD capable connectors. */
> > +		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
> > +			continue;
> > +
> > +		old_status = connector->status;
> > +		old_edid = to_intel_connector(connector)-
> > > 
> > > detect_edid;
> > +
> > +		cur_status = drm_helper_probe_detect(connector,
> > NULL, false);
> > +		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated
> > from
> > %s to %s\n",
> > +			      connector->base.id, connector->name,
> > +			      drm_get_connector_status_name(old_st
> > at
> > us),
> > +			      drm_get_connector_status_name(cur_st
> > at
> > us));
> > +
> > +		if (old_status != cur_status)
> > +			changed = true;
> > +
> > +		/* Check changing of edid when a connector status
> > still remains
> > +		 * as "connector_status_connected".
> > +		 */
> > +		if (old_status == cur_status &&
> > +		    cur_status == connector_status_connected) {
> > +			struct edid *cur_edid =
> > to_intel_connector(connector)->detect_edid;
> > +
> > +			if (!old_edid || !cur_edid)
> > +				continue;
> > +
> > +			if (memcmp(old_edid, cur_edid,
> > sizeof(*cur_edid))) {
> > +				changed = true;
> > +				DRM_DEBUG_KMS("[CONNECTOR:%d:%s]
> > edid updated\n",
> > +					      connector->base.id,
> > +					      connector->name);
> > +			}
> > +		}
> > +	}
> > +	drm_connector_list_iter_end(&conn_iter);
> > +	mutex_unlock(&dev->mode_config.mutex);
> > +
> > +	if (changed)
> > +		drm_kms_helper_hotplug_event(dev);
> > +
> > +	return changed;
> > +}
> > +
> >  static void i915_hpd_poll_init_work(struct work_struct *work)
> >  {
> >  	struct drm_i915_private *dev_priv =
> > @@ -552,7 +634,7 @@ static void i915_hpd_poll_init_work(struct
> > work_struct *work)
> >  	 * in the middle of disabling polling
> >  	 */
> >  	if (!enabled)
> > -		drm_helper_hpd_irq_event(dev);
> > +		intel_hpd_irq_event(dev);
> >  }
> Just wondering, as I saw previously drm_helper_hpd_irq_event function
> was used, which basically does the same thing, except doing memcmp
> for detecting the edid change. Is it only Intel specific change? As
> we
> could just add this modification to existing helper function or add
> another helper function to the drm (drm_probe_helper.c), so that it
> is
> also usable elsewhere..
I second Stanislav's proposal to add this feature as part of drm. This
edid check could be part of existing drm_helper_hpd_irq_event()
function. Other drivers might find this change useful.

> 
> > 
> >  
> >  /**
> -- 
> Best Regards,
> 
> Lisovskiy Stanislav
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Mika Kahola - Intel OTC


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-08-13 10:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-03 16:34 [PATCH] drm/i915: Add detection of changing of edid on between suspend and resume Gwan-gyeong Mun
2018-08-06 12:09 ` [v2] " Gwan-gyeong Mun
2018-08-09 11:13 ` [v3] " Gwan-gyeong Mun
2018-08-10 13:02   ` [Intel-gfx] " Lisovskiy, Stanislav
2018-08-13 10:55     ` Mika Kahola

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).