[PATCH v4 1/2] drm/i915: Add support for retrying hotplug

[PATCH v4 1/2] drm/i915: Add support for retrying hotplug

[José Roberto de Souza]

From: Imre Deak <imre.deak@intel.com>

There is some scenarios that we are aware that sink probe can fail,
so lets add the infrastructure to let hotplug() hook to request
another probe after some time.

v2: Handle shared HPD pins (Imre)
v3: Rebased
v4: Renamed INTEL_HOTPLUG_NOCHANGE to INTEL_HOTPLUG_UNCHANGED to keep
it consistent(Rodrigo)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c     | 12 ++--
 drivers/gpu/drm/i915/display/intel_dp.c      | 12 ++--
 drivers/gpu/drm/i915/display/intel_hotplug.c | 59 +++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_hotplug.h |  5 +-
 drivers/gpu/drm/i915/display/intel_sdvo.c    |  8 ++-
 drivers/gpu/drm/i915/i915_debugfs.c          |  2 +-
 drivers/gpu/drm/i915/i915_drv.h              |  3 +-
 drivers/gpu/drm/i915/intel_drv.h             | 11 +++-
 8 files changed, 80 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index ad638e7f27bb..734c004800f8 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4047,14 +4047,16 @@ static int intel_hdmi_reset_link(struct intel_encoder *encoder,
 	return modeset_pipe(&crtc->base, ctx);
 }
 
-static bool intel_ddi_hotplug(struct intel_encoder *encoder,
-			      struct intel_connector *connector)
+static enum intel_hotplug_state
+intel_ddi_hotplug(struct intel_encoder *encoder,
+		  struct intel_connector *connector,
+		  bool irq_received)
 {
 	struct drm_modeset_acquire_ctx ctx;
-	bool changed;
+	enum intel_hotplug_state state;
 	int ret;
 
-	changed = intel_encoder_hotplug(encoder, connector);
+	state = intel_encoder_hotplug(encoder, connector, irq_received);
 
 	drm_modeset_acquire_init(&ctx, 0);
 
@@ -4076,7 +4078,7 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder,
 	drm_modeset_acquire_fini(&ctx);
 	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
 
-	return changed;
+	return state;
 }
 
 static struct intel_connector *
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0bdb7ecc5a81..4423abbc7907 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4853,14 +4853,16 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
  * retrain the link to get a picture. That's in case no
  * userspace component reacted to intermittent HPD dip.
  */
-static bool intel_dp_hotplug(struct intel_encoder *encoder,
-			     struct intel_connector *connector)
+static enum intel_hotplug_state
+intel_dp_hotplug(struct intel_encoder *encoder,
+		 struct intel_connector *connector,
+		 bool irq_received)
 {
 	struct drm_modeset_acquire_ctx ctx;
-	bool changed;
+	enum intel_hotplug_state state;
 	int ret;
 
-	changed = intel_encoder_hotplug(encoder, connector);
+	state = intel_encoder_hotplug(encoder, connector, irq_received);
 
 	drm_modeset_acquire_init(&ctx, 0);
 
@@ -4879,7 +4881,7 @@ static bool intel_dp_hotplug(struct intel_encoder *encoder,
 	drm_modeset_acquire_fini(&ctx);
 	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
 
-	return changed;
+	return state;
 }
 
 static void intel_dp_check_service_irq(struct intel_dp *intel_dp)
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index ea3de4acc850..2ca92780c659 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -112,6 +112,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
 
 #define HPD_STORM_DETECT_PERIOD		1000
 #define HPD_STORM_REENABLE_DELAY	(2 * 60 * 1000)
+#define HPD_RETRY_DELAY			1000
 
 /**
  * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ storm on a pin
@@ -266,8 +267,10 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
 	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
 }
 
-bool intel_encoder_hotplug(struct intel_encoder *encoder,
-			   struct intel_connector *connector)
+enum intel_hotplug_state
+intel_encoder_hotplug(struct intel_encoder *encoder,
+		      struct intel_connector *connector,
+		      bool irq_received)
 {
 	struct drm_device *dev = connector->base.dev;
 	enum drm_connector_status old_status;
@@ -279,7 +282,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
 		drm_helper_probe_detect(&connector->base, NULL, false);
 
 	if (old_status == connector->base.status)
-		return false;
+		return INTEL_HOTPLUG_UNCHANGED;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
 		      connector->base.base.id,
@@ -287,7 +290,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
 		      drm_get_connector_status_name(old_status),
 		      drm_get_connector_status_name(connector->base.status));
 
-	return true;
+	return INTEL_HOTPLUG_CHANGED;
 }
 
 static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder)
@@ -339,7 +342,7 @@ static void i915_digport_work_func(struct work_struct *work)
 		spin_lock_irq(&dev_priv->irq_lock);
 		dev_priv->hotplug.event_bits |= old_bits;
 		spin_unlock_irq(&dev_priv->irq_lock);
-		schedule_work(&dev_priv->hotplug.hotplug_work);
+		schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0);
 	}
 }
 
@@ -349,14 +352,16 @@ static void i915_digport_work_func(struct work_struct *work)
 static void i915_hotplug_work_func(struct work_struct *work)
 {
 	struct drm_i915_private *dev_priv =
-		container_of(work, struct drm_i915_private, hotplug.hotplug_work);
+		container_of(work, struct drm_i915_private,
+			     hotplug.hotplug_work.work);
 	struct drm_device *dev = &dev_priv->drm;
 	struct intel_connector *intel_connector;
 	struct intel_encoder *intel_encoder;
 	struct drm_connector *connector;
 	struct drm_connector_list_iter conn_iter;
-	bool changed = false;
+	u32 changed = 0, retry = 0;
 	u32 hpd_event_bits;
+	u32 hpd_retry_bits;
 
 	mutex_lock(&dev->mode_config.mutex);
 	DRM_DEBUG_KMS("running encoder hotplug functions\n");
@@ -365,6 +370,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
 
 	hpd_event_bits = dev_priv->hotplug.event_bits;
 	dev_priv->hotplug.event_bits = 0;
+	hpd_retry_bits = dev_priv->hotplug.retry_bits;
+	dev_priv->hotplug.retry_bits = 0;
 
 	/* Enable polling for connectors which had HPD IRQ storms */
 	intel_hpd_irq_storm_switch_to_polling(dev_priv);
@@ -373,16 +380,29 @@ static void i915_hotplug_work_func(struct work_struct *work)
 
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter) {
+		u32 hpd_bit;
+
 		intel_connector = to_intel_connector(connector);
 		if (!intel_connector->encoder)
 			continue;
 		intel_encoder = intel_connector->encoder;
-		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
+		hpd_bit = BIT(intel_encoder->hpd_pin);
+		if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) {
 			DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
 				      connector->name, intel_encoder->hpd_pin);
 
-			changed |= intel_encoder->hotplug(intel_encoder,
-							  intel_connector);
+			switch (intel_encoder->hotplug(intel_encoder,
+						       intel_connector,
+						       hpd_event_bits & hpd_bit)) {
+			case INTEL_HOTPLUG_UNCHANGED:
+				break;
+			case INTEL_HOTPLUG_CHANGED:
+				changed |= hpd_bit;
+				break;
+			case INTEL_HOTPLUG_RETRY:
+				retry |= hpd_bit;
+				break;
+			}
 		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
@@ -390,6 +410,17 @@ static void i915_hotplug_work_func(struct work_struct *work)
 
 	if (changed)
 		drm_kms_helper_hotplug_event(dev);
+
+	/* Remove shared HPD pins that have changed */
+	retry &= ~changed;
+	if (retry) {
+		spin_lock_irq(&dev_priv->irq_lock);
+		dev_priv->hotplug.retry_bits |= retry;
+		spin_unlock_irq(&dev_priv->irq_lock);
+
+		mod_delayed_work(system_wq, &dev_priv->hotplug.hotplug_work,
+				 msecs_to_jiffies(HPD_RETRY_DELAY));
+	}
 }
 
 
@@ -516,7 +547,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 	if (queue_dig)
 		queue_work(dev_priv->hotplug.dp_wq, &dev_priv->hotplug.dig_port_work);
 	if (queue_hp)
-		schedule_work(&dev_priv->hotplug.hotplug_work);
+		schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0);
 }
 
 /**
@@ -636,7 +667,8 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
 
 void intel_hpd_init_work(struct drm_i915_private *dev_priv)
 {
-	INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func);
+	INIT_DELAYED_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.poll_init_work, i915_hpd_poll_init_work);
 	INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
@@ -650,11 +682,12 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
 	dev_priv->hotplug.long_port_mask = 0;
 	dev_priv->hotplug.short_port_mask = 0;
 	dev_priv->hotplug.event_bits = 0;
+	dev_priv->hotplug.retry_bits = 0;
 
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	cancel_work_sync(&dev_priv->hotplug.dig_port_work);
-	cancel_work_sync(&dev_priv->hotplug.hotplug_work);
+	cancel_delayed_work_sync(&dev_priv->hotplug.hotplug_work);
 	cancel_work_sync(&dev_priv->hotplug.poll_init_work);
 	cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
index 805f897dbb7a..b0cd447b7fbc 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.h
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
@@ -15,8 +15,9 @@ struct intel_connector;
 struct intel_encoder;
 
 void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
-bool intel_encoder_hotplug(struct intel_encoder *encoder,
-			   struct intel_connector *connector);
+enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
+					       struct intel_connector *connector,
+					       bool irq_received);
 void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 			   u32 pin_mask, u32 long_mask);
 void intel_hpd_init(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index 3fe8eaef6bd8..639317b84e51 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -1915,12 +1915,14 @@ static void intel_sdvo_enable_hotplug(struct intel_encoder *encoder)
 			     &intel_sdvo->hotplug_active, 2);
 }
 
-static bool intel_sdvo_hotplug(struct intel_encoder *encoder,
-			       struct intel_connector *connector)
+static enum intel_hotplug_state
+intel_sdvo_hotplug(struct intel_encoder *encoder,
+		   struct intel_connector *connector,
+		   bool irq_received)
 {
 	intel_sdvo_enable_hotplug(encoder);
 
-	return intel_encoder_hotplug(encoder, connector);
+	return intel_encoder_hotplug(encoder, connector, irq_received);
 }
 
 static bool
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3e4f58f19362..6d4bc983915f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4090,7 +4090,7 @@ static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data)
 	 */
 	intel_synchronize_irq(dev_priv);
 	flush_work(&dev_priv->hotplug.dig_port_work);
-	flush_work(&dev_priv->hotplug.hotplug_work);
+	flush_delayed_work(&dev_priv->hotplug.hotplug_work);
 
 	seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold);
 	seq_printf(m, "Detected: %s\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f9878cbef4d9..f471307aa704 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -163,7 +163,7 @@ enum hpd_pin {
 #define HPD_STORM_DEFAULT_THRESHOLD 50
 
 struct i915_hotplug {
-	struct work_struct hotplug_work;
+	struct delayed_work hotplug_work;
 
 	struct {
 		unsigned long last_jiffies;
@@ -175,6 +175,7 @@ struct i915_hotplug {
 		} state;
 	} stats[HPD_NUM_PINS];
 	u32 event_bits;
+	u32 retry_bits;
 	struct delayed_work reenable_work;
 
 	u32 long_port_mask;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 24c63ed45c6f..e58d3751ed43 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -101,14 +101,21 @@ struct intel_fbdev {
 	struct mutex hpd_lock;
 };
 
+enum intel_hotplug_state {
+	INTEL_HOTPLUG_UNCHANGED,
+	INTEL_HOTPLUG_CHANGED,
+	INTEL_HOTPLUG_RETRY,
+};
+
 struct intel_encoder {
 	struct drm_encoder base;
 
 	enum intel_output_type type;
 	enum port port;
 	unsigned int cloneable;
-	bool (*hotplug)(struct intel_encoder *encoder,
-			struct intel_connector *connector);
+	enum intel_hotplug_state (*hotplug)(struct intel_encoder *encoder,
+					    struct intel_connector *connector,
+					    bool irq_received);
 	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
 						      struct intel_crtc_state *,
 						      struct drm_connector_state *);
-- 
2.22.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 2/2] drm/i915: Enable hotplug retry

[José Roberto de Souza]

Right now we are aware of two cases that needs another hotplug retry:
- Unpowered type-c dongles
- HDMI slow unplug

Both have a complete explanation in the code to schedule another run
of the hotplug handler.

It could have more checks to just trigger the retry in those two
specific cases but why would sink signal a long pulse if there is
no change? Also the drawback of running the hotplug handler again
is really low and that could fix another cases that we are not
aware.

Also retrying for old DP ports(non-DDI) to make it consistent and not
cause CI failures if those systems are connected to chamelium boards
that will be used to simulate the issues reported in here.

v2: Also retrying for old DP ports(non-DDI)(Imre)

v4: Renamed INTEL_HOTPLUG_NOCHANGE to INTEL_HOTPLUG_UNCHANGED to keep
it consistent(Rodrigo)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c  | 21 +++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.c   |  7 ++++++
 drivers/gpu/drm/i915/display/intel_hdmi.c | 28 ++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 734c004800f8..ea6d1873f6cb 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4052,6 +4052,7 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
 		  struct intel_connector *connector,
 		  bool irq_received)
 {
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	struct drm_modeset_acquire_ctx ctx;
 	enum intel_hotplug_state state;
 	int ret;
@@ -4078,6 +4079,26 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
 	drm_modeset_acquire_fini(&ctx);
 	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
 
+	/*
+	 * Unpowered type-c dongles can take some time to boot and be
+	 * responsible, so here giving some time to those dongles to power up
+	 * and then retrying the probe.
+	 *
+	 * On many platforms the HDMI live state signal is known to be
+	 * unreliable, so we can't use it to detect if a sink is connected or
+	 * not. Instead we detect if it's connected based on whether we can
+	 * read the EDID or not. That in turn has a problem during disconnect,
+	 * since the HPD interrupt may be raised before the DDC lines get
+	 * disconnected (due to how the required length of DDC vs. HPD
+	 * connector pins are specified) and so we'll still be able to get a
+	 * valid EDID. To solve this schedule another detection cycle if this
+	 * time around we didn't detect any change in the sink's connection
+	 * status.
+	 */
+	if (state == INTEL_HOTPLUG_UNCHANGED && irq_received &&
+	    !dig_port->dp.is_mst)
+		state = INTEL_HOTPLUG_RETRY;
+
 	return state;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 4423abbc7907..7106a2d80f79 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4881,6 +4881,13 @@ intel_dp_hotplug(struct intel_encoder *encoder,
 	drm_modeset_acquire_fini(&ctx);
 	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
 
+	/*
+	 * Keeping it consistent with intel_ddi_hotplug() and
+	 * intel_hdmi_hotplug().
+	 */
+	if (state == INTEL_HOTPLUG_UNCHANGED && irq_received)
+		state = INTEL_HOTPLUG_RETRY;
+
 	return state;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 0ebec69bbbfc..26c8556f6980 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -3143,6 +3143,32 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 		DRM_DEBUG_KMS("CEC notifier get failed\n");
 }
 
+static enum intel_hotplug_state
+intel_hdmi_hotplug(struct intel_encoder *encoder,
+		   struct intel_connector *connector, bool irq_received)
+{
+	enum intel_hotplug_state state;
+
+	state = intel_encoder_hotplug(encoder, connector, irq_received);
+
+	/*
+	 * On many platforms the HDMI live state signal is known to be
+	 * unreliable, so we can't use it to detect if a sink is connected or
+	 * not. Instead we detect if it's connected based on whether we can
+	 * read the EDID or not. That in turn has a problem during disconnect,
+	 * since the HPD interrupt may be raised before the DDC lines get
+	 * disconnected (due to how the required length of DDC vs. HPD
+	 * connector pins are specified) and so we'll still be able to get a
+	 * valid EDID. To solve this schedule another detection cycle if this
+	 * time around we didn't detect any change in the sink's connection
+	 * status.
+	 */
+	if (state == INTEL_HOTPLUG_UNCHANGED && irq_received)
+		state = INTEL_HOTPLUG_RETRY;
+
+	return state;
+}
+
 void intel_hdmi_init(struct drm_i915_private *dev_priv,
 		     i915_reg_t hdmi_reg, enum port port)
 {
@@ -3166,7 +3192,7 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
 			 &intel_hdmi_enc_funcs, DRM_MODE_ENCODER_TMDS,
 			 "HDMI %c", port_name(port));
 
-	intel_encoder->hotplug = intel_encoder_hotplug;
+	intel_encoder->hotplug = intel_hdmi_hotplug;
 	intel_encoder->compute_config = intel_hdmi_compute_config;
 	if (HAS_PCH_SPLIT(dev_priv)) {
 		intel_encoder->disable = pch_disable_hdmi;
-- 
2.22.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [v4,1/2] drm/i915: Add support for retrying hotplug

[Patchwork]

== Series Details ==

Series: series starting with [v4,1/2] drm/i915: Add support for retrying hotplug
URL   : https://patchwork.freedesktop.org/series/63523/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6453 -> Patchwork_13613
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/

Known issues
------------

  Here are the changes found in Patchwork_13613 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_addfb_basic@unused-pitches:
    - fi-icl-dsi:         [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/fi-icl-dsi/igt@kms_addfb_basic@unused-pitches.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/fi-icl-dsi/igt@kms_addfb_basic@unused-pitches.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [PASS][3] -> [FAIL][4] ([fdo#110387])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

  
#### Possible fixes ####

  * igt@gem_basic@create-fd-close:
    - fi-icl-u3:          [DMESG-WARN][5] ([fdo#107724]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/fi-icl-u3/igt@gem_basic@create-fd-close.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/fi-icl-u3/igt@gem_basic@create-fd-close.html

  * igt@kms_chamelium@hdmi-edid-read:
    - {fi-icl-u4}:        [FAIL][7] ([fdo#111045] / [fdo#111046 ]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/fi-icl-u4/igt@kms_chamelium@hdmi-edid-read.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][9] ([fdo#109485]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
    - {fi-icl-u4}:        [FAIL][11] ([fdo#111045]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/fi-icl-u4/igt@kms_chamelium@hdmi-hpd-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/fi-icl-u4/igt@kms_chamelium@hdmi-hpd-fast.html
    - fi-kbl-7567u:       [FAIL][13] ([fdo#109485]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/fi-kbl-7567u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-blb-e6850:       [INCOMPLETE][15] ([fdo#107718]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         [DMESG-WARN][17] ([fdo#106387]) -> [PASS][18] +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html

  
#### Warnings ####

  * igt@i915_selftest@live_sanitycheck:
    - fi-icl-u3:          [DMESG-WARN][19] ([fdo#110801]) -> [DMESG-WARN][20] ([fdo#107724] / [fdo#110801])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/fi-icl-u3/igt@i915_selftest@live_sanitycheck.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/fi-icl-u3/igt@i915_selftest@live_sanitycheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
  [fdo#110387]: https://bugs.freedesktop.org/show_bug.cgi?id=110387
  [fdo#110801]: https://bugs.freedesktop.org/show_bug.cgi?id=110801
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111046 ]: https://bugs.freedesktop.org/show_bug.cgi?id=111046 
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096


Participating hosts (53 -> 47)
------------------------------

  Missing    (6): fi-kbl-soraka fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6453 -> Patchwork_13613

  CI_DRM_6453: ee7ea857b5d32c7b58b5994201e0e1e194266206 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5092: 2a66ae6626d5583240509f84117d1345a799b75a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13613: 69a68876da19321a157a831edc8a1ee629d9683a @ git://anongit.freedesktop.org/gfx-ci/linux


== Kernel 32bit build ==

Warning: Kernel 32bit buildtest failed:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/build_32bit.log

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
Kernel: arch/x86/boot/bzImage is ready  (#1)
  Building modules, stage 2.
  MODPOST 112 modules
ERROR: "__udivdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
scripts/Makefile.modpost:91: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
Makefile:1287: recipe for target 'modules' failed
make: *** [modules] Error 2


== Linux commits ==

69a68876da19 drm/i915: Enable hotplug retry
0f7522b11496 drm/i915: Add support for retrying hotplug

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 1/2] drm/i915: Add support for retrying hotplug

[Nathan Ciobanu]

On Wed, Jul 10, 2019 at 03:14:59PM -0700, José Roberto de Souza wrote:
> From: Imre Deak <imre.deak@intel.com>
> 
> There is some scenarios that we are aware that sink probe can fail,
> so lets add the infrastructure to let hotplug() hook to request
> another probe after some time.
> 
> v2: Handle shared HPD pins (Imre)
> v3: Rebased
> v4: Renamed INTEL_HOTPLUG_NOCHANGE to INTEL_HOTPLUG_UNCHANGED to keep
> it consistent(Rodrigo)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 12 ++--
>  drivers/gpu/drm/i915/display/intel_dp.c      | 12 ++--
>  drivers/gpu/drm/i915/display/intel_hotplug.c | 59 +++++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_hotplug.h |  5 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c    |  8 ++-
>  drivers/gpu/drm/i915/i915_debugfs.c          |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h              |  3 +-
>  drivers/gpu/drm/i915/intel_drv.h             | 11 +++-
>  8 files changed, 80 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index ad638e7f27bb..734c004800f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4047,14 +4047,16 @@ static int intel_hdmi_reset_link(struct intel_encoder *encoder,
>  	return modeset_pipe(&crtc->base, ctx);
>  }
>  
> -static bool intel_ddi_hotplug(struct intel_encoder *encoder,
> -			      struct intel_connector *connector)
> +static enum intel_hotplug_state
> +intel_ddi_hotplug(struct intel_encoder *encoder,
> +		  struct intel_connector *connector,
> +		  bool irq_received)
>  {
>  	struct drm_modeset_acquire_ctx ctx;
> -	bool changed;
> +	enum intel_hotplug_state state;
>  	int ret;
>  
> -	changed = intel_encoder_hotplug(encoder, connector);
> +	state = intel_encoder_hotplug(encoder, connector, irq_received);
>  
>  	drm_modeset_acquire_init(&ctx, 0);
>  
> @@ -4076,7 +4078,7 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder,
>  	drm_modeset_acquire_fini(&ctx);
>  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
>  
> -	return changed;
> +	return state;
>  }
>  
>  static struct intel_connector *
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0bdb7ecc5a81..4423abbc7907 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4853,14 +4853,16 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
>   * retrain the link to get a picture. That's in case no
>   * userspace component reacted to intermittent HPD dip.
>   */
> -static bool intel_dp_hotplug(struct intel_encoder *encoder,
> -			     struct intel_connector *connector)
> +static enum intel_hotplug_state
> +intel_dp_hotplug(struct intel_encoder *encoder,
> +		 struct intel_connector *connector,
> +		 bool irq_received)
>  {
>  	struct drm_modeset_acquire_ctx ctx;
> -	bool changed;
> +	enum intel_hotplug_state state;
>  	int ret;
>  
> -	changed = intel_encoder_hotplug(encoder, connector);
> +	state = intel_encoder_hotplug(encoder, connector, irq_received);
>  
>  	drm_modeset_acquire_init(&ctx, 0);
>  
> @@ -4879,7 +4881,7 @@ static bool intel_dp_hotplug(struct intel_encoder *encoder,
>  	drm_modeset_acquire_fini(&ctx);
>  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
>  
> -	return changed;
> +	return state;
>  }
>  
>  static void intel_dp_check_service_irq(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index ea3de4acc850..2ca92780c659 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -112,6 +112,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
>  
>  #define HPD_STORM_DETECT_PERIOD		1000
>  #define HPD_STORM_REENABLE_DELAY	(2 * 60 * 1000)
> +#define HPD_RETRY_DELAY			1000
>  
>  /**
>   * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ storm on a pin
> @@ -266,8 +267,10 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
>  }
>  
> -bool intel_encoder_hotplug(struct intel_encoder *encoder,
> -			   struct intel_connector *connector)
> +enum intel_hotplug_state
> +intel_encoder_hotplug(struct intel_encoder *encoder,
> +		      struct intel_connector *connector,
> +		      bool irq_received)
>  {
>  	struct drm_device *dev = connector->base.dev;
>  	enum drm_connector_status old_status;
> @@ -279,7 +282,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
>  		drm_helper_probe_detect(&connector->base, NULL, false);
>  
>  	if (old_status == connector->base.status)
> -		return false;
> +		return INTEL_HOTPLUG_UNCHANGED;
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
>  		      connector->base.base.id,
> @@ -287,7 +290,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
>  		      drm_get_connector_status_name(old_status),
>  		      drm_get_connector_status_name(connector->base.status));
>  
> -	return true;
> +	return INTEL_HOTPLUG_CHANGED;
>  }
>  
>  static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder)
> @@ -339,7 +342,7 @@ static void i915_digport_work_func(struct work_struct *work)
>  		spin_lock_irq(&dev_priv->irq_lock);
>  		dev_priv->hotplug.event_bits |= old_bits;
>  		spin_unlock_irq(&dev_priv->irq_lock);
> -		schedule_work(&dev_priv->hotplug.hotplug_work);
> +		schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0);
>  	}
>  }
>  
> @@ -349,14 +352,16 @@ static void i915_digport_work_func(struct work_struct *work)
>  static void i915_hotplug_work_func(struct work_struct *work)
>  {
>  	struct drm_i915_private *dev_priv =
> -		container_of(work, struct drm_i915_private, hotplug.hotplug_work);
> +		container_of(work, struct drm_i915_private,
> +			     hotplug.hotplug_work.work);
>  	struct drm_device *dev = &dev_priv->drm;
>  	struct intel_connector *intel_connector;
>  	struct intel_encoder *intel_encoder;
>  	struct drm_connector *connector;
>  	struct drm_connector_list_iter conn_iter;
> -	bool changed = false;
> +	u32 changed = 0, retry = 0;
>  	u32 hpd_event_bits;
> +	u32 hpd_retry_bits;
>  
>  	mutex_lock(&dev->mode_config.mutex);
>  	DRM_DEBUG_KMS("running encoder hotplug functions\n");
> @@ -365,6 +370,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  
>  	hpd_event_bits = dev_priv->hotplug.event_bits;
>  	dev_priv->hotplug.event_bits = 0;
> +	hpd_retry_bits = dev_priv->hotplug.retry_bits;
> +	dev_priv->hotplug.retry_bits = 0;
>  
>  	/* Enable polling for connectors which had HPD IRQ storms */
>  	intel_hpd_irq_storm_switch_to_polling(dev_priv);
> @@ -373,16 +380,29 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  
>  	drm_connector_list_iter_begin(dev, &conn_iter);
>  	drm_for_each_connector_iter(connector, &conn_iter) {
> +		u32 hpd_bit;
> +
>  		intel_connector = to_intel_connector(connector);
>  		if (!intel_connector->encoder)
>  			continue;
>  		intel_encoder = intel_connector->encoder;
> -		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
> +		hpd_bit = BIT(intel_encoder->hpd_pin);
> +		if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) {
>  			DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
>  				      connector->name, intel_encoder->hpd_pin);
>  
> -			changed |= intel_encoder->hotplug(intel_encoder,
> -							  intel_connector);
> +			switch (intel_encoder->hotplug(intel_encoder,
> +						       intel_connector,
> +						       hpd_event_bits & hpd_bit)) {
> +			case INTEL_HOTPLUG_UNCHANGED:
> +				break;
> +			case INTEL_HOTPLUG_CHANGED:
> +				changed |= hpd_bit;
> +				break;
> +			case INTEL_HOTPLUG_RETRY:
> +				retry |= hpd_bit;
> +				break;
> +			}
>  		}
>  	}
>  	drm_connector_list_iter_end(&conn_iter);
> @@ -390,6 +410,17 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  
>  	if (changed)
>  		drm_kms_helper_hotplug_event(dev);
> +
> +	/* Remove shared HPD pins that have changed */
> +	retry &= ~changed;
> +	if (retry) {
> +		spin_lock_irq(&dev_priv->irq_lock);
> +		dev_priv->hotplug.retry_bits |= retry;
> +		spin_unlock_irq(&dev_priv->irq_lock);
> +
> +		mod_delayed_work(system_wq, &dev_priv->hotplug.hotplug_work,
> +				 msecs_to_jiffies(HPD_RETRY_DELAY));
> +	}
>  }
>  
>  
> @@ -516,7 +547,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  	if (queue_dig)
>  		queue_work(dev_priv->hotplug.dp_wq, &dev_priv->hotplug.dig_port_work);
>  	if (queue_hp)
> -		schedule_work(&dev_priv->hotplug.hotplug_work);
> +		schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0);
>  }
>  
>  /**
> @@ -636,7 +667,8 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
>  
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv)
>  {
> -	INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func);
> +	INIT_DELAYED_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.poll_init_work, i915_hpd_poll_init_work);
>  	INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
> @@ -650,11 +682,12 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
>  	dev_priv->hotplug.long_port_mask = 0;
>  	dev_priv->hotplug.short_port_mask = 0;
>  	dev_priv->hotplug.event_bits = 0;
> +	dev_priv->hotplug.retry_bits = 0;
>  
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
>  	cancel_work_sync(&dev_priv->hotplug.dig_port_work);
> -	cancel_work_sync(&dev_priv->hotplug.hotplug_work);
> +	cancel_delayed_work_sync(&dev_priv->hotplug.hotplug_work);
>  	cancel_work_sync(&dev_priv->hotplug.poll_init_work);
>  	cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
> index 805f897dbb7a..b0cd447b7fbc 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> @@ -15,8 +15,9 @@ struct intel_connector;
>  struct intel_encoder;
>  
>  void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
> -bool intel_encoder_hotplug(struct intel_encoder *encoder,
> -			   struct intel_connector *connector);
> +enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
> +					       struct intel_connector *connector,
> +					       bool irq_received);
>  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  			   u32 pin_mask, u32 long_mask);
>  void intel_hpd_init(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 3fe8eaef6bd8..639317b84e51 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -1915,12 +1915,14 @@ static void intel_sdvo_enable_hotplug(struct intel_encoder *encoder)
>  			     &intel_sdvo->hotplug_active, 2);
>  }
>  
> -static bool intel_sdvo_hotplug(struct intel_encoder *encoder,
> -			       struct intel_connector *connector)
> +static enum intel_hotplug_state
> +intel_sdvo_hotplug(struct intel_encoder *encoder,
> +		   struct intel_connector *connector,
> +		   bool irq_received)
>  {
>  	intel_sdvo_enable_hotplug(encoder);
>  
> -	return intel_encoder_hotplug(encoder, connector);
> +	return intel_encoder_hotplug(encoder, connector, irq_received);
>  }
>  
>  static bool
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3e4f58f19362..6d4bc983915f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4090,7 +4090,7 @@ static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data)
>  	 */
>  	intel_synchronize_irq(dev_priv);
>  	flush_work(&dev_priv->hotplug.dig_port_work);
> -	flush_work(&dev_priv->hotplug.hotplug_work);
> +	flush_delayed_work(&dev_priv->hotplug.hotplug_work);
>  
>  	seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold);
>  	seq_printf(m, "Detected: %s\n",
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f9878cbef4d9..f471307aa704 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -163,7 +163,7 @@ enum hpd_pin {
>  #define HPD_STORM_DEFAULT_THRESHOLD 50
>  
>  struct i915_hotplug {
> -	struct work_struct hotplug_work;
> +	struct delayed_work hotplug_work;
>  
>  	struct {
>  		unsigned long last_jiffies;
> @@ -175,6 +175,7 @@ struct i915_hotplug {
>  		} state;
>  	} stats[HPD_NUM_PINS];
>  	u32 event_bits;
> +	u32 retry_bits;
>  	struct delayed_work reenable_work;
>  
>  	u32 long_port_mask;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 24c63ed45c6f..e58d3751ed43 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -101,14 +101,21 @@ struct intel_fbdev {
>  	struct mutex hpd_lock;
>  };
>  
> +enum intel_hotplug_state {
> +	INTEL_HOTPLUG_UNCHANGED,
> +	INTEL_HOTPLUG_CHANGED,
> +	INTEL_HOTPLUG_RETRY,
> +};
> +
>  struct intel_encoder {
>  	struct drm_encoder base;
>  
>  	enum intel_output_type type;
>  	enum port port;
>  	unsigned int cloneable;
> -	bool (*hotplug)(struct intel_encoder *encoder,
> -			struct intel_connector *connector);
> +	enum intel_hotplug_state (*hotplug)(struct intel_encoder *encoder,
> +					    struct intel_connector *connector,
> +					    bool irq_received);
>  	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
>  						      struct intel_crtc_state *,
>  						      struct drm_connector_state *);
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 2/2] drm/i915: Enable hotplug retry

[Nathan Ciobanu]

On Wed, Jul 10, 2019 at 03:15:00PM -0700, José Roberto de Souza wrote:
> Right now we are aware of two cases that needs another hotplug retry:
> - Unpowered type-c dongles
> - HDMI slow unplug
> 
> Both have a complete explanation in the code to schedule another run
> of the hotplug handler.
> 
> It could have more checks to just trigger the retry in those two
> specific cases but why would sink signal a long pulse if there is
> no change? Also the drawback of running the hotplug handler again
> is really low and that could fix another cases that we are not
> aware.
> 
> Also retrying for old DP ports(non-DDI) to make it consistent and not
> cause CI failures if those systems are connected to chamelium boards
> that will be used to simulate the issues reported in here.
> 
> v2: Also retrying for old DP ports(non-DDI)(Imre)
> 
> v4: Renamed INTEL_HOTPLUG_NOCHANGE to INTEL_HOTPLUG_UNCHANGED to keep
> it consistent(Rodrigo)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c  | 21 +++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dp.c   |  7 ++++++
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 28 ++++++++++++++++++++++-
>  3 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 734c004800f8..ea6d1873f6cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4052,6 +4052,7 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
>  		  struct intel_connector *connector,
>  		  bool irq_received)
>  {
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>  	struct drm_modeset_acquire_ctx ctx;
>  	enum intel_hotplug_state state;
>  	int ret;
> @@ -4078,6 +4079,26 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
>  	drm_modeset_acquire_fini(&ctx);
>  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
>  
> +	/*
> +	 * Unpowered type-c dongles can take some time to boot and be
> +	 * responsible, so here giving some time to those dongles to power up
> +	 * and then retrying the probe.
> +	 *
> +	 * On many platforms the HDMI live state signal is known to be
> +	 * unreliable, so we can't use it to detect if a sink is connected or
> +	 * not. Instead we detect if it's connected based on whether we can
> +	 * read the EDID or not. That in turn has a problem during disconnect,
> +	 * since the HPD interrupt may be raised before the DDC lines get
> +	 * disconnected (due to how the required length of DDC vs. HPD
> +	 * connector pins are specified) and so we'll still be able to get a
> +	 * valid EDID. To solve this schedule another detection cycle if this
> +	 * time around we didn't detect any change in the sink's connection
> +	 * status.
> +	 */
> +	if (state == INTEL_HOTPLUG_UNCHANGED && irq_received &&
> +	    !dig_port->dp.is_mst)
> +		state = INTEL_HOTPLUG_RETRY;
> +
>  	return state;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4423abbc7907..7106a2d80f79 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4881,6 +4881,13 @@ intel_dp_hotplug(struct intel_encoder *encoder,
>  	drm_modeset_acquire_fini(&ctx);
>  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
>  
> +	/*
> +	 * Keeping it consistent with intel_ddi_hotplug() and
> +	 * intel_hdmi_hotplug().
> +	 */
> +	if (state == INTEL_HOTPLUG_UNCHANGED && irq_received)
> +		state = INTEL_HOTPLUG_RETRY;
> +
>  	return state;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 0ebec69bbbfc..26c8556f6980 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3143,6 +3143,32 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>  		DRM_DEBUG_KMS("CEC notifier get failed\n");
>  }
>  
> +static enum intel_hotplug_state
> +intel_hdmi_hotplug(struct intel_encoder *encoder,
> +		   struct intel_connector *connector, bool irq_received)
> +{
> +	enum intel_hotplug_state state;
> +
> +	state = intel_encoder_hotplug(encoder, connector, irq_received);
> +
> +	/*
> +	 * On many platforms the HDMI live state signal is known to be
> +	 * unreliable, so we can't use it to detect if a sink is connected or
> +	 * not. Instead we detect if it's connected based on whether we can
> +	 * read the EDID or not. That in turn has a problem during disconnect,
> +	 * since the HPD interrupt may be raised before the DDC lines get
> +	 * disconnected (due to how the required length of DDC vs. HPD
> +	 * connector pins are specified) and so we'll still be able to get a
> +	 * valid EDID. To solve this schedule another detection cycle if this
> +	 * time around we didn't detect any change in the sink's connection
> +	 * status.
> +	 */
> +	if (state == INTEL_HOTPLUG_UNCHANGED && irq_received)
> +		state = INTEL_HOTPLUG_RETRY;
> +
> +	return state;
> +}
> +
>  void intel_hdmi_init(struct drm_i915_private *dev_priv,
>  		     i915_reg_t hdmi_reg, enum port port)
>  {
> @@ -3166,7 +3192,7 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
>  			 &intel_hdmi_enc_funcs, DRM_MODE_ENCODER_TMDS,
>  			 "HDMI %c", port_name(port));
>  
> -	intel_encoder->hotplug = intel_encoder_hotplug;
> +	intel_encoder->hotplug = intel_hdmi_hotplug;
>  	intel_encoder->compute_config = intel_hdmi_compute_config;
>  	if (HAS_PCH_SPLIT(dev_priv)) {
>  		intel_encoder->disable = pch_disable_hdmi;
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 1/2] drm/i915: Add support for retrying hotplug

[Ville Syrjälä]

On Wed, Jul 10, 2019 at 03:14:59PM -0700, José Roberto de Souza wrote:
> From: Imre Deak <imre.deak@intel.com>
> 
> There is some scenarios that we are aware that sink probe can fail,
> so lets add the infrastructure to let hotplug() hook to request
> another probe after some time.
> 
> v2: Handle shared HPD pins (Imre)
> v3: Rebased
> v4: Renamed INTEL_HOTPLUG_NOCHANGE to INTEL_HOTPLUG_UNCHANGED to keep
> it consistent(Rodrigo)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 12 ++--
>  drivers/gpu/drm/i915/display/intel_dp.c      | 12 ++--
>  drivers/gpu/drm/i915/display/intel_hotplug.c | 59 +++++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_hotplug.h |  5 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c    |  8 ++-
>  drivers/gpu/drm/i915/i915_debugfs.c          |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h              |  3 +-
>  drivers/gpu/drm/i915/intel_drv.h             | 11 +++-
>  8 files changed, 80 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index ad638e7f27bb..734c004800f8 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4047,14 +4047,16 @@ static int intel_hdmi_reset_link(struct intel_encoder *encoder,
>  	return modeset_pipe(&crtc->base, ctx);
>  }
>  
> -static bool intel_ddi_hotplug(struct intel_encoder *encoder,
> -			      struct intel_connector *connector)
> +static enum intel_hotplug_state
> +intel_ddi_hotplug(struct intel_encoder *encoder,
> +		  struct intel_connector *connector,
> +		  bool irq_received)
>  {
>  	struct drm_modeset_acquire_ctx ctx;
> -	bool changed;
> +	enum intel_hotplug_state state;
>  	int ret;
>  
> -	changed = intel_encoder_hotplug(encoder, connector);
> +	state = intel_encoder_hotplug(encoder, connector, irq_received);
>  
>  	drm_modeset_acquire_init(&ctx, 0);
>  
> @@ -4076,7 +4078,7 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder,
>  	drm_modeset_acquire_fini(&ctx);
>  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
>  
> -	return changed;
> +	return state;
>  }
>  
>  static struct intel_connector *
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0bdb7ecc5a81..4423abbc7907 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4853,14 +4853,16 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
>   * retrain the link to get a picture. That's in case no
>   * userspace component reacted to intermittent HPD dip.
>   */
> -static bool intel_dp_hotplug(struct intel_encoder *encoder,
> -			     struct intel_connector *connector)
> +static enum intel_hotplug_state
> +intel_dp_hotplug(struct intel_encoder *encoder,
> +		 struct intel_connector *connector,
> +		 bool irq_received)
>  {
>  	struct drm_modeset_acquire_ctx ctx;
> -	bool changed;
> +	enum intel_hotplug_state state;
>  	int ret;
>  
> -	changed = intel_encoder_hotplug(encoder, connector);
> +	state = intel_encoder_hotplug(encoder, connector, irq_received);
>  
>  	drm_modeset_acquire_init(&ctx, 0);
>  
> @@ -4879,7 +4881,7 @@ static bool intel_dp_hotplug(struct intel_encoder *encoder,
>  	drm_modeset_acquire_fini(&ctx);
>  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
>  
> -	return changed;
> +	return state;
>  }
>  
>  static void intel_dp_check_service_irq(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index ea3de4acc850..2ca92780c659 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -112,6 +112,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
>  
>  #define HPD_STORM_DETECT_PERIOD		1000
>  #define HPD_STORM_REENABLE_DELAY	(2 * 60 * 1000)
> +#define HPD_RETRY_DELAY			1000
>  
>  /**
>   * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ storm on a pin
> @@ -266,8 +267,10 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
>  }
>  
> -bool intel_encoder_hotplug(struct intel_encoder *encoder,
> -			   struct intel_connector *connector)
> +enum intel_hotplug_state
> +intel_encoder_hotplug(struct intel_encoder *encoder,
> +		      struct intel_connector *connector,
> +		      bool irq_received)
>  {
>  	struct drm_device *dev = connector->base.dev;
>  	enum drm_connector_status old_status;
> @@ -279,7 +282,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
>  		drm_helper_probe_detect(&connector->base, NULL, false);
>  
>  	if (old_status == connector->base.status)
> -		return false;
> +		return INTEL_HOTPLUG_UNCHANGED;
>  
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
>  		      connector->base.base.id,
> @@ -287,7 +290,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
>  		      drm_get_connector_status_name(old_status),
>  		      drm_get_connector_status_name(connector->base.status));
>  
> -	return true;
> +	return INTEL_HOTPLUG_CHANGED;
>  }
>  
>  static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder)
> @@ -339,7 +342,7 @@ static void i915_digport_work_func(struct work_struct *work)
>  		spin_lock_irq(&dev_priv->irq_lock);
>  		dev_priv->hotplug.event_bits |= old_bits;
>  		spin_unlock_irq(&dev_priv->irq_lock);
> -		schedule_work(&dev_priv->hotplug.hotplug_work);
> +		schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0);

Random thought: Should open code this with queue_delayed_work() instead
so that we can be sure the wq really is the same between this call
and the mod_delayed_work()?

>  	}
>  }
>  
> @@ -349,14 +352,16 @@ static void i915_digport_work_func(struct work_struct *work)
>  static void i915_hotplug_work_func(struct work_struct *work)
>  {
>  	struct drm_i915_private *dev_priv =
> -		container_of(work, struct drm_i915_private, hotplug.hotplug_work);
> +		container_of(work, struct drm_i915_private,
> +			     hotplug.hotplug_work.work);
>  	struct drm_device *dev = &dev_priv->drm;
>  	struct intel_connector *intel_connector;
>  	struct intel_encoder *intel_encoder;
>  	struct drm_connector *connector;
>  	struct drm_connector_list_iter conn_iter;
> -	bool changed = false;
> +	u32 changed = 0, retry = 0;
>  	u32 hpd_event_bits;
> +	u32 hpd_retry_bits;
>  
>  	mutex_lock(&dev->mode_config.mutex);
>  	DRM_DEBUG_KMS("running encoder hotplug functions\n");
> @@ -365,6 +370,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  
>  	hpd_event_bits = dev_priv->hotplug.event_bits;
>  	dev_priv->hotplug.event_bits = 0;
> +	hpd_retry_bits = dev_priv->hotplug.retry_bits;
> +	dev_priv->hotplug.retry_bits = 0;
>  
>  	/* Enable polling for connectors which had HPD IRQ storms */
>  	intel_hpd_irq_storm_switch_to_polling(dev_priv);
> @@ -373,16 +380,29 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  
>  	drm_connector_list_iter_begin(dev, &conn_iter);
>  	drm_for_each_connector_iter(connector, &conn_iter) {
> +		u32 hpd_bit;
> +
>  		intel_connector = to_intel_connector(connector);
>  		if (!intel_connector->encoder)
>  			continue;
>  		intel_encoder = intel_connector->encoder;
> -		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
> +		hpd_bit = BIT(intel_encoder->hpd_pin);
> +		if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) {
>  			DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
>  				      connector->name, intel_encoder->hpd_pin);
>  
> -			changed |= intel_encoder->hotplug(intel_encoder,
> -							  intel_connector);
> +			switch (intel_encoder->hotplug(intel_encoder,
> +						       intel_connector,
> +						       hpd_event_bits & hpd_bit)) {
> +			case INTEL_HOTPLUG_UNCHANGED:
> +				break;
> +			case INTEL_HOTPLUG_CHANGED:
> +				changed |= hpd_bit;
> +				break;
> +			case INTEL_HOTPLUG_RETRY:
> +				retry |= hpd_bit;
> +				break;
> +			}
>  		}
>  	}
>  	drm_connector_list_iter_end(&conn_iter);
> @@ -390,6 +410,17 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  
>  	if (changed)
>  		drm_kms_helper_hotplug_event(dev);
> +
> +	/* Remove shared HPD pins that have changed */
> +	retry &= ~changed;
> +	if (retry) {
> +		spin_lock_irq(&dev_priv->irq_lock);
> +		dev_priv->hotplug.retry_bits |= retry;
> +		spin_unlock_irq(&dev_priv->irq_lock);
> +
> +		mod_delayed_work(system_wq, &dev_priv->hotplug.hotplug_work,
> +				 msecs_to_jiffies(HPD_RETRY_DELAY));
> +	}
>  }
>  
>  
> @@ -516,7 +547,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  	if (queue_dig)
>  		queue_work(dev_priv->hotplug.dp_wq, &dev_priv->hotplug.dig_port_work);
>  	if (queue_hp)
> -		schedule_work(&dev_priv->hotplug.hotplug_work);
> +		schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0);
>  }
>  
>  /**
> @@ -636,7 +667,8 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
>  
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv)
>  {
> -	INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func);
> +	INIT_DELAYED_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.poll_init_work, i915_hpd_poll_init_work);
>  	INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
> @@ -650,11 +682,12 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
>  	dev_priv->hotplug.long_port_mask = 0;
>  	dev_priv->hotplug.short_port_mask = 0;
>  	dev_priv->hotplug.event_bits = 0;
> +	dev_priv->hotplug.retry_bits = 0;
>  
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
>  	cancel_work_sync(&dev_priv->hotplug.dig_port_work);
> -	cancel_work_sync(&dev_priv->hotplug.hotplug_work);
> +	cancel_delayed_work_sync(&dev_priv->hotplug.hotplug_work);
>  	cancel_work_sync(&dev_priv->hotplug.poll_init_work);
>  	cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
> index 805f897dbb7a..b0cd447b7fbc 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> @@ -15,8 +15,9 @@ struct intel_connector;
>  struct intel_encoder;
>  
>  void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
> -bool intel_encoder_hotplug(struct intel_encoder *encoder,
> -			   struct intel_connector *connector);
> +enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
> +					       struct intel_connector *connector,
> +					       bool irq_received);
>  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  			   u32 pin_mask, u32 long_mask);
>  void intel_hpd_init(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 3fe8eaef6bd8..639317b84e51 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -1915,12 +1915,14 @@ static void intel_sdvo_enable_hotplug(struct intel_encoder *encoder)
>  			     &intel_sdvo->hotplug_active, 2);
>  }
>  
> -static bool intel_sdvo_hotplug(struct intel_encoder *encoder,
> -			       struct intel_connector *connector)
> +static enum intel_hotplug_state
> +intel_sdvo_hotplug(struct intel_encoder *encoder,
> +		   struct intel_connector *connector,
> +		   bool irq_received)
>  {
>  	intel_sdvo_enable_hotplug(encoder);
>  
> -	return intel_encoder_hotplug(encoder, connector);
> +	return intel_encoder_hotplug(encoder, connector, irq_received);
>  }
>  
>  static bool
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3e4f58f19362..6d4bc983915f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4090,7 +4090,7 @@ static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data)
>  	 */
>  	intel_synchronize_irq(dev_priv);
>  	flush_work(&dev_priv->hotplug.dig_port_work);
> -	flush_work(&dev_priv->hotplug.hotplug_work);
> +	flush_delayed_work(&dev_priv->hotplug.hotplug_work);
>  
>  	seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold);
>  	seq_printf(m, "Detected: %s\n",
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f9878cbef4d9..f471307aa704 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -163,7 +163,7 @@ enum hpd_pin {
>  #define HPD_STORM_DEFAULT_THRESHOLD 50
>  
>  struct i915_hotplug {
> -	struct work_struct hotplug_work;
> +	struct delayed_work hotplug_work;
>  
>  	struct {
>  		unsigned long last_jiffies;
> @@ -175,6 +175,7 @@ struct i915_hotplug {
>  		} state;
>  	} stats[HPD_NUM_PINS];
>  	u32 event_bits;
> +	u32 retry_bits;
>  	struct delayed_work reenable_work;
>  
>  	u32 long_port_mask;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 24c63ed45c6f..e58d3751ed43 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -101,14 +101,21 @@ struct intel_fbdev {
>  	struct mutex hpd_lock;
>  };
>  
> +enum intel_hotplug_state {
> +	INTEL_HOTPLUG_UNCHANGED,
> +	INTEL_HOTPLUG_CHANGED,
> +	INTEL_HOTPLUG_RETRY,
> +};
> +
>  struct intel_encoder {
>  	struct drm_encoder base;
>  
>  	enum intel_output_type type;
>  	enum port port;
>  	unsigned int cloneable;
> -	bool (*hotplug)(struct intel_encoder *encoder,
> -			struct intel_connector *connector);
> +	enum intel_hotplug_state (*hotplug)(struct intel_encoder *encoder,
> +					    struct intel_connector *connector,
> +					    bool irq_received);
>  	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
>  						      struct intel_crtc_state *,
>  						      struct drm_connector_state *);
> -- 
> 2.22.0

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 2/2] drm/i915: Enable hotplug retry

[Shane McKee]

On Thu, Jul 11, 2019 at 12:49:35PM -0700, Nathan Ciobanu wrote:
> On Wed, Jul 10, 2019 at 03:15:00PM -0700, José Roberto de Souza wrote:
> > Right now we are aware of two cases that needs another hotplug retry:
> > - Unpowered type-c dongles
> > - HDMI slow unplug
> > 
> > Both have a complete explanation in the code to schedule another run
> > of the hotplug handler.
> > 
> > It could have more checks to just trigger the retry in those two
> > specific cases but why would sink signal a long pulse if there is
> > no change? Also the drawback of running the hotplug handler again
> > is really low and that could fix another cases that we are not
> > aware.
> > 
> > Also retrying for old DP ports(non-DDI) to make it consistent and not
> > cause CI failures if those systems are connected to chamelium boards
> > that will be used to simulate the issues reported in here.
> > 
> > v2: Also retrying for old DP ports(non-DDI)(Imre)
> > 
> > v4: Renamed INTEL_HOTPLUG_NOCHANGE to INTEL_HOTPLUG_UNCHANGED to keep
> > it consistent(Rodrigo)
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
Tested-by: Shane McKee <shane.mckee@intel.com>
> > Reviewed-by: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c  | 21 +++++++++++++++++
> >  drivers/gpu/drm/i915/display/intel_dp.c   |  7 ++++++
> >  drivers/gpu/drm/i915/display/intel_hdmi.c | 28 ++++++++++++++++++++++-
> >  3 files changed, 55 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 734c004800f8..ea6d1873f6cb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4052,6 +4052,7 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
> >  		  struct intel_connector *connector,
> >  		  bool irq_received)
> >  {
> > +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >  	struct drm_modeset_acquire_ctx ctx;
> >  	enum intel_hotplug_state state;
> >  	int ret;
> > @@ -4078,6 +4079,26 @@ intel_ddi_hotplug(struct intel_encoder *encoder,
> >  	drm_modeset_acquire_fini(&ctx);
> >  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
> >  
> > +	/*
> > +	 * Unpowered type-c dongles can take some time to boot and be
> > +	 * responsible, so here giving some time to those dongles to power up
> > +	 * and then retrying the probe.
> > +	 *
> > +	 * On many platforms the HDMI live state signal is known to be
> > +	 * unreliable, so we can't use it to detect if a sink is connected or
> > +	 * not. Instead we detect if it's connected based on whether we can
> > +	 * read the EDID or not. That in turn has a problem during disconnect,
> > +	 * since the HPD interrupt may be raised before the DDC lines get
> > +	 * disconnected (due to how the required length of DDC vs. HPD
> > +	 * connector pins are specified) and so we'll still be able to get a
> > +	 * valid EDID. To solve this schedule another detection cycle if this
> > +	 * time around we didn't detect any change in the sink's connection
> > +	 * status.
> > +	 */
> > +	if (state == INTEL_HOTPLUG_UNCHANGED && irq_received &&
> > +	    !dig_port->dp.is_mst)
> > +		state = INTEL_HOTPLUG_RETRY;
> > +
> >  	return state;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 4423abbc7907..7106a2d80f79 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4881,6 +4881,13 @@ intel_dp_hotplug(struct intel_encoder *encoder,
> >  	drm_modeset_acquire_fini(&ctx);
> >  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
> >  
> > +	/*
> > +	 * Keeping it consistent with intel_ddi_hotplug() and
> > +	 * intel_hdmi_hotplug().
> > +	 */
> > +	if (state == INTEL_HOTPLUG_UNCHANGED && irq_received)
> > +		state = INTEL_HOTPLUG_RETRY;
> > +
> >  	return state;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index 0ebec69bbbfc..26c8556f6980 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -3143,6 +3143,32 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> >  		DRM_DEBUG_KMS("CEC notifier get failed\n");
> >  }
> >  
> > +static enum intel_hotplug_state
> > +intel_hdmi_hotplug(struct intel_encoder *encoder,
> > +		   struct intel_connector *connector, bool irq_received)
> > +{
> > +	enum intel_hotplug_state state;
> > +
> > +	state = intel_encoder_hotplug(encoder, connector, irq_received);
> > +
> > +	/*
> > +	 * On many platforms the HDMI live state signal is known to be
> > +	 * unreliable, so we can't use it to detect if a sink is connected or
> > +	 * not. Instead we detect if it's connected based on whether we can
> > +	 * read the EDID or not. That in turn has a problem during disconnect,
> > +	 * since the HPD interrupt may be raised before the DDC lines get
> > +	 * disconnected (due to how the required length of DDC vs. HPD
> > +	 * connector pins are specified) and so we'll still be able to get a
> > +	 * valid EDID. To solve this schedule another detection cycle if this
> > +	 * time around we didn't detect any change in the sink's connection
> > +	 * status.
> > +	 */
> > +	if (state == INTEL_HOTPLUG_UNCHANGED && irq_received)
> > +		state = INTEL_HOTPLUG_RETRY;
> > +
> > +	return state;
> > +}
> > +
> >  void intel_hdmi_init(struct drm_i915_private *dev_priv,
> >  		     i915_reg_t hdmi_reg, enum port port)
> >  {
> > @@ -3166,7 +3192,7 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
> >  			 &intel_hdmi_enc_funcs, DRM_MODE_ENCODER_TMDS,
> >  			 "HDMI %c", port_name(port));
> >  
> > -	intel_encoder->hotplug = intel_encoder_hotplug;
> > +	intel_encoder->hotplug = intel_hdmi_hotplug;
> >  	intel_encoder->compute_config = intel_hdmi_compute_config;
> >  	if (HAS_PCH_SPLIT(dev_priv)) {
> >  		intel_encoder->disable = pch_disable_hdmi;
> > -- 
> > 2.22.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 1/2] drm/i915: Add support for retrying hotplug

[Shane McKee]

On Thu, Jul 11, 2019 at 11:05:15PM +0300, Ville Syrjälä wrote:
> On Wed, Jul 10, 2019 at 03:14:59PM -0700, José Roberto de Souza wrote:
> > From: Imre Deak <imre.deak@intel.com>
> > 
> > There is some scenarios that we are aware that sink probe can fail,
> > so lets add the infrastructure to let hotplug() hook to request
> > another probe after some time.
> > 
> > v2: Handle shared HPD pins (Imre)
> > v3: Rebased
> > v4: Renamed INTEL_HOTPLUG_NOCHANGE to INTEL_HOTPLUG_UNCHANGED to keep
> > it consistent(Rodrigo)
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Tested-by: Shane McKee <shane.mckee@intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c     | 12 ++--
> >  drivers/gpu/drm/i915/display/intel_dp.c      | 12 ++--
> >  drivers/gpu/drm/i915/display/intel_hotplug.c | 59 +++++++++++++++-----
> >  drivers/gpu/drm/i915/display/intel_hotplug.h |  5 +-
> >  drivers/gpu/drm/i915/display/intel_sdvo.c    |  8 ++-
> >  drivers/gpu/drm/i915/i915_debugfs.c          |  2 +-
> >  drivers/gpu/drm/i915/i915_drv.h              |  3 +-
> >  drivers/gpu/drm/i915/intel_drv.h             | 11 +++-
> >  8 files changed, 80 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index ad638e7f27bb..734c004800f8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4047,14 +4047,16 @@ static int intel_hdmi_reset_link(struct intel_encoder *encoder,
> >  	return modeset_pipe(&crtc->base, ctx);
> >  }
> >  
> > -static bool intel_ddi_hotplug(struct intel_encoder *encoder,
> > -			      struct intel_connector *connector)
> > +static enum intel_hotplug_state
> > +intel_ddi_hotplug(struct intel_encoder *encoder,
> > +		  struct intel_connector *connector,
> > +		  bool irq_received)
> >  {
> >  	struct drm_modeset_acquire_ctx ctx;
> > -	bool changed;
> > +	enum intel_hotplug_state state;
> >  	int ret;
> >  
> > -	changed = intel_encoder_hotplug(encoder, connector);
> > +	state = intel_encoder_hotplug(encoder, connector, irq_received);
> >  
> >  	drm_modeset_acquire_init(&ctx, 0);
> >  
> > @@ -4076,7 +4078,7 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder,
> >  	drm_modeset_acquire_fini(&ctx);
> >  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
> >  
> > -	return changed;
> > +	return state;
> >  }
> >  
> >  static struct intel_connector *
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 0bdb7ecc5a81..4423abbc7907 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4853,14 +4853,16 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
> >   * retrain the link to get a picture. That's in case no
> >   * userspace component reacted to intermittent HPD dip.
> >   */
> > -static bool intel_dp_hotplug(struct intel_encoder *encoder,
> > -			     struct intel_connector *connector)
> > +static enum intel_hotplug_state
> > +intel_dp_hotplug(struct intel_encoder *encoder,
> > +		 struct intel_connector *connector,
> > +		 bool irq_received)
> >  {
> >  	struct drm_modeset_acquire_ctx ctx;
> > -	bool changed;
> > +	enum intel_hotplug_state state;
> >  	int ret;
> >  
> > -	changed = intel_encoder_hotplug(encoder, connector);
> > +	state = intel_encoder_hotplug(encoder, connector, irq_received);
> >  
> >  	drm_modeset_acquire_init(&ctx, 0);
> >  
> > @@ -4879,7 +4881,7 @@ static bool intel_dp_hotplug(struct intel_encoder *encoder,
> >  	drm_modeset_acquire_fini(&ctx);
> >  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
> >  
> > -	return changed;
> > +	return state;
> >  }
> >  
> >  static void intel_dp_check_service_irq(struct intel_dp *intel_dp)
> > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > index ea3de4acc850..2ca92780c659 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > @@ -112,6 +112,7 @@ enum hpd_pin intel_hpd_pin_default(struct drm_i915_private *dev_priv,
> >  
> >  #define HPD_STORM_DETECT_PERIOD		1000
> >  #define HPD_STORM_REENABLE_DELAY	(2 * 60 * 1000)
> > +#define HPD_RETRY_DELAY			1000
> >  
> >  /**
> >   * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ storm on a pin
> > @@ -266,8 +267,10 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> >  	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
> >  }
> >  
> > -bool intel_encoder_hotplug(struct intel_encoder *encoder,
> > -			   struct intel_connector *connector)
> > +enum intel_hotplug_state
> > +intel_encoder_hotplug(struct intel_encoder *encoder,
> > +		      struct intel_connector *connector,
> > +		      bool irq_received)
> >  {
> >  	struct drm_device *dev = connector->base.dev;
> >  	enum drm_connector_status old_status;
> > @@ -279,7 +282,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
> >  		drm_helper_probe_detect(&connector->base, NULL, false);
> >  
> >  	if (old_status == connector->base.status)
> > -		return false;
> > +		return INTEL_HOTPLUG_UNCHANGED;
> >  
> >  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
> >  		      connector->base.base.id,
> > @@ -287,7 +290,7 @@ bool intel_encoder_hotplug(struct intel_encoder *encoder,
> >  		      drm_get_connector_status_name(old_status),
> >  		      drm_get_connector_status_name(connector->base.status));
> >  
> > -	return true;
> > +	return INTEL_HOTPLUG_CHANGED;
> >  }
> >  
> >  static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder)
> > @@ -339,7 +342,7 @@ static void i915_digport_work_func(struct work_struct *work)
> >  		spin_lock_irq(&dev_priv->irq_lock);
> >  		dev_priv->hotplug.event_bits |= old_bits;
> >  		spin_unlock_irq(&dev_priv->irq_lock);
> > -		schedule_work(&dev_priv->hotplug.hotplug_work);
> > +		schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0);
> 
> Random thought: Should open code this with queue_delayed_work() instead
> so that we can be sure the wq really is the same between this call
> and the mod_delayed_work()?
> 
> >  	}
> >  }
> >  
> > @@ -349,14 +352,16 @@ static void i915_digport_work_func(struct work_struct *work)
> >  static void i915_hotplug_work_func(struct work_struct *work)
> >  {
> >  	struct drm_i915_private *dev_priv =
> > -		container_of(work, struct drm_i915_private, hotplug.hotplug_work);
> > +		container_of(work, struct drm_i915_private,
> > +			     hotplug.hotplug_work.work);
> >  	struct drm_device *dev = &dev_priv->drm;
> >  	struct intel_connector *intel_connector;
> >  	struct intel_encoder *intel_encoder;
> >  	struct drm_connector *connector;
> >  	struct drm_connector_list_iter conn_iter;
> > -	bool changed = false;
> > +	u32 changed = 0, retry = 0;
> >  	u32 hpd_event_bits;
> > +	u32 hpd_retry_bits;
> >  
> >  	mutex_lock(&dev->mode_config.mutex);
> >  	DRM_DEBUG_KMS("running encoder hotplug functions\n");
> > @@ -365,6 +370,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
> >  
> >  	hpd_event_bits = dev_priv->hotplug.event_bits;
> >  	dev_priv->hotplug.event_bits = 0;
> > +	hpd_retry_bits = dev_priv->hotplug.retry_bits;
> > +	dev_priv->hotplug.retry_bits = 0;
> >  
> >  	/* Enable polling for connectors which had HPD IRQ storms */
> >  	intel_hpd_irq_storm_switch_to_polling(dev_priv);
> > @@ -373,16 +380,29 @@ static void i915_hotplug_work_func(struct work_struct *work)
> >  
> >  	drm_connector_list_iter_begin(dev, &conn_iter);
> >  	drm_for_each_connector_iter(connector, &conn_iter) {
> > +		u32 hpd_bit;
> > +
> >  		intel_connector = to_intel_connector(connector);
> >  		if (!intel_connector->encoder)
> >  			continue;
> >  		intel_encoder = intel_connector->encoder;
> > -		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
> > +		hpd_bit = BIT(intel_encoder->hpd_pin);
> > +		if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) {
> >  			DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
> >  				      connector->name, intel_encoder->hpd_pin);
> >  
> > -			changed |= intel_encoder->hotplug(intel_encoder,
> > -							  intel_connector);
> > +			switch (intel_encoder->hotplug(intel_encoder,
> > +						       intel_connector,
> > +						       hpd_event_bits & hpd_bit)) {
> > +			case INTEL_HOTPLUG_UNCHANGED:
> > +				break;
> > +			case INTEL_HOTPLUG_CHANGED:
> > +				changed |= hpd_bit;
> > +				break;
> > +			case INTEL_HOTPLUG_RETRY:
> > +				retry |= hpd_bit;
> > +				break;
> > +			}
> >  		}
> >  	}
> >  	drm_connector_list_iter_end(&conn_iter);
> > @@ -390,6 +410,17 @@ static void i915_hotplug_work_func(struct work_struct *work)
> >  
> >  	if (changed)
> >  		drm_kms_helper_hotplug_event(dev);
> > +
> > +	/* Remove shared HPD pins that have changed */
> > +	retry &= ~changed;
> > +	if (retry) {
> > +		spin_lock_irq(&dev_priv->irq_lock);
> > +		dev_priv->hotplug.retry_bits |= retry;
> > +		spin_unlock_irq(&dev_priv->irq_lock);
> > +
> > +		mod_delayed_work(system_wq, &dev_priv->hotplug.hotplug_work,
> > +				 msecs_to_jiffies(HPD_RETRY_DELAY));
> > +	}
> >  }
> >  
> >  
> > @@ -516,7 +547,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> >  	if (queue_dig)
> >  		queue_work(dev_priv->hotplug.dp_wq, &dev_priv->hotplug.dig_port_work);
> >  	if (queue_hp)
> > -		schedule_work(&dev_priv->hotplug.hotplug_work);
> > +		schedule_delayed_work(&dev_priv->hotplug.hotplug_work, 0);
> >  }
> >  
> >  /**
> > @@ -636,7 +667,8 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
> >  
> >  void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> >  {
> > -	INIT_WORK(&dev_priv->hotplug.hotplug_work, i915_hotplug_work_func);
> > +	INIT_DELAYED_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.poll_init_work, i915_hpd_poll_init_work);
> >  	INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
> > @@ -650,11 +682,12 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
> >  	dev_priv->hotplug.long_port_mask = 0;
> >  	dev_priv->hotplug.short_port_mask = 0;
> >  	dev_priv->hotplug.event_bits = 0;
> > +	dev_priv->hotplug.retry_bits = 0;
> >  
> >  	spin_unlock_irq(&dev_priv->irq_lock);
> >  
> >  	cancel_work_sync(&dev_priv->hotplug.dig_port_work);
> > -	cancel_work_sync(&dev_priv->hotplug.hotplug_work);
> > +	cancel_delayed_work_sync(&dev_priv->hotplug.hotplug_work);
> >  	cancel_work_sync(&dev_priv->hotplug.poll_init_work);
> >  	cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
> >  }
> > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > index 805f897dbb7a..b0cd447b7fbc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > @@ -15,8 +15,9 @@ struct intel_connector;
> >  struct intel_encoder;
> >  
> >  void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
> > -bool intel_encoder_hotplug(struct intel_encoder *encoder,
> > -			   struct intel_connector *connector);
> > +enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
> > +					       struct intel_connector *connector,
> > +					       bool irq_received);
> >  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> >  			   u32 pin_mask, u32 long_mask);
> >  void intel_hpd_init(struct drm_i915_private *dev_priv);
> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > index 3fe8eaef6bd8..639317b84e51 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > @@ -1915,12 +1915,14 @@ static void intel_sdvo_enable_hotplug(struct intel_encoder *encoder)
> >  			     &intel_sdvo->hotplug_active, 2);
> >  }
> >  
> > -static bool intel_sdvo_hotplug(struct intel_encoder *encoder,
> > -			       struct intel_connector *connector)
> > +static enum intel_hotplug_state
> > +intel_sdvo_hotplug(struct intel_encoder *encoder,
> > +		   struct intel_connector *connector,
> > +		   bool irq_received)
> >  {
> >  	intel_sdvo_enable_hotplug(encoder);
> >  
> > -	return intel_encoder_hotplug(encoder, connector);
> > +	return intel_encoder_hotplug(encoder, connector, irq_received);
> >  }
> >  
> >  static bool
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 3e4f58f19362..6d4bc983915f 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -4090,7 +4090,7 @@ static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data)
> >  	 */
> >  	intel_synchronize_irq(dev_priv);
> >  	flush_work(&dev_priv->hotplug.dig_port_work);
> > -	flush_work(&dev_priv->hotplug.hotplug_work);
> > +	flush_delayed_work(&dev_priv->hotplug.hotplug_work);
> >  
> >  	seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold);
> >  	seq_printf(m, "Detected: %s\n",
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index f9878cbef4d9..f471307aa704 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -163,7 +163,7 @@ enum hpd_pin {
> >  #define HPD_STORM_DEFAULT_THRESHOLD 50
> >  
> >  struct i915_hotplug {
> > -	struct work_struct hotplug_work;
> > +	struct delayed_work hotplug_work;
> >  
> >  	struct {
> >  		unsigned long last_jiffies;
> > @@ -175,6 +175,7 @@ struct i915_hotplug {
> >  		} state;
> >  	} stats[HPD_NUM_PINS];
> >  	u32 event_bits;
> > +	u32 retry_bits;
> >  	struct delayed_work reenable_work;
> >  
> >  	u32 long_port_mask;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 24c63ed45c6f..e58d3751ed43 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -101,14 +101,21 @@ struct intel_fbdev {
> >  	struct mutex hpd_lock;
> >  };
> >  
> > +enum intel_hotplug_state {
> > +	INTEL_HOTPLUG_UNCHANGED,
> > +	INTEL_HOTPLUG_CHANGED,
> > +	INTEL_HOTPLUG_RETRY,
> > +};
> > +
> >  struct intel_encoder {
> >  	struct drm_encoder base;
> >  
> >  	enum intel_output_type type;
> >  	enum port port;
> >  	unsigned int cloneable;
> > -	bool (*hotplug)(struct intel_encoder *encoder,
> > -			struct intel_connector *connector);
> > +	enum intel_hotplug_state (*hotplug)(struct intel_encoder *encoder,
> > +					    struct intel_connector *connector,
> > +					    bool irq_received);
> >  	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
> >  						      struct intel_crtc_state *,
> >  						      struct drm_connector_state *);
> > -- 
> > 2.22.0
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [v4,1/2] drm/i915: Add support for retrying hotplug

[Patchwork]

== Series Details ==

Series: series starting with [v4,1/2] drm/i915: Add support for retrying hotplug
URL   : https://patchwork.freedesktop.org/series/63523/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6453_full -> Patchwork_13613_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_13613_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#110854])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-iclb4/igt@gem_exec_balancer@smoke.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-iclb8/igt@gem_exec_balancer@smoke.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([fdo#108686])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-apl6/igt@gem_tiled_swapping@non-threaded.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-apl7/igt@gem_tiled_swapping@non-threaded.html

  * igt@kms_color@pipe-c-ctm-green-to-red:
    - shard-skl:          [PASS][5] -> [FAIL][6] ([fdo#107201])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-skl2/igt@kms_color@pipe-c-ctm-green-to-red.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-skl8/igt@kms_color@pipe-c-ctm-green-to-red.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +5 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-apl6/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-apl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [PASS][9] -> [INCOMPLETE][10] ([fdo#104108])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-skl3/igt@kms_fbcon_fbt@psr-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-skl8/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          [PASS][11] -> [INCOMPLETE][12] ([fdo#109507])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-skl5/igt@kms_flip@flip-vs-suspend-interruptible.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-skl10/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_flip_tiling@flip-changes-tiling-yf:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#108228] / [fdo#108303])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-skl10/igt@kms_flip_tiling@flip-changes-tiling-yf.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-skl4/igt@kms_flip_tiling@flip-changes-tiling-yf.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
    - shard-iclb:         [PASS][15] -> [FAIL][16] ([fdo#103167]) +5 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#108145])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([fdo#108145] / [fdo#110403]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([fdo#103166])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_sprite_render:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109441])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-iclb2/igt@kms_psr@psr2_sprite_render.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-iclb7/igt@kms_psr@psr2_sprite_render.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][25] -> [FAIL][26] ([fdo#99912])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-apl5/igt@kms_setmode@basic.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-apl3/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][27] ([fdo#104108] / [fdo#107773]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-skl9/igt@gem_softpin@noreloc-s3.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-skl10/igt@gem_softpin@noreloc-s3.html

  * igt@i915_pm_rpm@debugfs-read:
    - shard-iclb:         [INCOMPLETE][29] ([fdo#107713] / [fdo#108840]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-iclb7/igt@i915_pm_rpm@debugfs-read.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-iclb6/igt@i915_pm_rpm@debugfs-read.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          [DMESG-WARN][31] ([fdo#108566]) -> [PASS][32] +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-kbl3/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][33] ([fdo#104873]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-glk8/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-glk6/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-apl:          [FAIL][35] ([fdo#103060]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-apl6/igt@kms_flip@modeset-vs-vblank-race.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-apl7/igt@kms_flip@modeset-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - shard-iclb:         [FAIL][37] ([fdo#103167]) -> [PASS][38] +2 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][39] ([fdo#108566]) -> [PASS][40] +3 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-apl5/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-apl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-iclb7/igt@kms_psr@psr2_no_drrs.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-iclb2/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-skl:          [INCOMPLETE][43] ([fdo#104108]) -> [PASS][44] +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-skl5/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-skl10/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  * igt@perf@polling:
    - shard-skl:          [FAIL][45] ([fdo#110728]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-skl8/igt@perf@polling.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-skl3/igt@perf@polling.html

  * igt@tools_test@tools_test:
    - shard-snb:          [SKIP][47] ([fdo#109271]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-snb2/igt@tools_test@tools_test.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-snb5/igt@tools_test@tools_test.html

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbc-1p-indfb-fliptrack:
    - shard-skl:          [FAIL][49] ([fdo#103167]) -> [FAIL][50] ([fdo#108040])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6453/shard-skl7/igt@kms_frontbuffer_tracking@fbc-1p-indfb-fliptrack.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/shard-skl7/igt@kms_frontbuffer_tracking@fbc-1p-indfb-fliptrack.html

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#107201]: https://bugs.freedesktop.org/show_bug.cgi?id=107201
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108228]: https://bugs.freedesktop.org/show_bug.cgi?id=108228
  [fdo#108303]: https://bugs.freedesktop.org/show_bug.cgi?id=108303
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_6453 -> Patchwork_13613

  CI_DRM_6453: ee7ea857b5d32c7b58b5994201e0e1e194266206 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5092: 2a66ae6626d5583240509f84117d1345a799b75a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13613: 69a68876da19321a157a831edc8a1ee629d9683a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13613/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 1/2] drm/i915: Add support for retrying hotplug

[Souza, Jose]

On Thu, 2019-07-11 at 23:05 +0300, Ville Syrjälä wrote:
> On Wed, Jul 10, 2019 at 03:14:59PM -0700, José Roberto de Souza
> wrote:
> > From: Imre Deak <imre.deak@intel.com>
> > 
> > There is some scenarios that we are aware that sink probe can fail,
> > so lets add the infrastructure to let hotplug() hook to request
> > another probe after some time.
> > 
> > v2: Handle shared HPD pins (Imre)
> > v3: Rebased
> > v4: Renamed INTEL_HOTPLUG_NOCHANGE to INTEL_HOTPLUG_UNCHANGED to
> > keep
> > it consistent(Rodrigo)
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_ddi.c     | 12 ++--
> >  drivers/gpu/drm/i915/display/intel_dp.c      | 12 ++--
> >  drivers/gpu/drm/i915/display/intel_hotplug.c | 59 +++++++++++++++-
> > ----
> >  drivers/gpu/drm/i915/display/intel_hotplug.h |  5 +-
> >  drivers/gpu/drm/i915/display/intel_sdvo.c    |  8 ++-
> >  drivers/gpu/drm/i915/i915_debugfs.c          |  2 +-
> >  drivers/gpu/drm/i915/i915_drv.h              |  3 +-
> >  drivers/gpu/drm/i915/intel_drv.h             | 11 +++-
> >  8 files changed, 80 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index ad638e7f27bb..734c004800f8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -4047,14 +4047,16 @@ static int intel_hdmi_reset_link(struct
> > intel_encoder *encoder,
> >  	return modeset_pipe(&crtc->base, ctx);
> >  }
> >  
> > -static bool intel_ddi_hotplug(struct intel_encoder *encoder,
> > -			      struct intel_connector *connector)
> > +static enum intel_hotplug_state
> > +intel_ddi_hotplug(struct intel_encoder *encoder,
> > +		  struct intel_connector *connector,
> > +		  bool irq_received)
> >  {
> >  	struct drm_modeset_acquire_ctx ctx;
> > -	bool changed;
> > +	enum intel_hotplug_state state;
> >  	int ret;
> >  
> > -	changed = intel_encoder_hotplug(encoder, connector);
> > +	state = intel_encoder_hotplug(encoder, connector,
> > irq_received);
> >  
> >  	drm_modeset_acquire_init(&ctx, 0);
> >  
> > @@ -4076,7 +4078,7 @@ static bool intel_ddi_hotplug(struct
> > intel_encoder *encoder,
> >  	drm_modeset_acquire_fini(&ctx);
> >  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
> >  
> > -	return changed;
> > +	return state;
> >  }
> >  
> >  static struct intel_connector *
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 0bdb7ecc5a81..4423abbc7907 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4853,14 +4853,16 @@ int intel_dp_retrain_link(struct
> > intel_encoder *encoder,
> >   * retrain the link to get a picture. That's in case no
> >   * userspace component reacted to intermittent HPD dip.
> >   */
> > -static bool intel_dp_hotplug(struct intel_encoder *encoder,
> > -			     struct intel_connector *connector)
> > +static enum intel_hotplug_state
> > +intel_dp_hotplug(struct intel_encoder *encoder,
> > +		 struct intel_connector *connector,
> > +		 bool irq_received)
> >  {
> >  	struct drm_modeset_acquire_ctx ctx;
> > -	bool changed;
> > +	enum intel_hotplug_state state;
> >  	int ret;
> >  
> > -	changed = intel_encoder_hotplug(encoder, connector);
> > +	state = intel_encoder_hotplug(encoder, connector,
> > irq_received);
> >  
> >  	drm_modeset_acquire_init(&ctx, 0);
> >  
> > @@ -4879,7 +4881,7 @@ static bool intel_dp_hotplug(struct
> > intel_encoder *encoder,
> >  	drm_modeset_acquire_fini(&ctx);
> >  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
> >  
> > -	return changed;
> > +	return state;
> >  }
> >  
> >  static void intel_dp_check_service_irq(struct intel_dp *intel_dp)
> > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > index ea3de4acc850..2ca92780c659 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > @@ -112,6 +112,7 @@ enum hpd_pin intel_hpd_pin_default(struct
> > drm_i915_private *dev_priv,
> >  
> >  #define HPD_STORM_DETECT_PERIOD		1000
> >  #define HPD_STORM_REENABLE_DELAY	(2 * 60 * 1000)
> > +#define HPD_RETRY_DELAY			1000
> >  
> >  /**
> >   * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ
> > storm on a pin
> > @@ -266,8 +267,10 @@ static void
> > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> >  	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
> >  }
> >  
> > -bool intel_encoder_hotplug(struct intel_encoder *encoder,
> > -			   struct intel_connector *connector)
> > +enum intel_hotplug_state
> > +intel_encoder_hotplug(struct intel_encoder *encoder,
> > +		      struct intel_connector *connector,
> > +		      bool irq_received)
> >  {
> >  	struct drm_device *dev = connector->base.dev;
> >  	enum drm_connector_status old_status;
> > @@ -279,7 +282,7 @@ bool intel_encoder_hotplug(struct intel_encoder
> > *encoder,
> >  		drm_helper_probe_detect(&connector->base, NULL, false);
> >  
> >  	if (old_status == connector->base.status)
> > -		return false;
> > +		return INTEL_HOTPLUG_UNCHANGED;
> >  
> >  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
> > %s\n",
> >  		      connector->base.base.id,
> > @@ -287,7 +290,7 @@ bool intel_encoder_hotplug(struct intel_encoder
> > *encoder,
> >  		      drm_get_connector_status_name(old_status),
> >  		      drm_get_connector_status_name(connector-
> > >base.status));
> >  
> > -	return true;
> > +	return INTEL_HOTPLUG_CHANGED;
> >  }
> >  
> >  static bool intel_encoder_has_hpd_pulse(struct intel_encoder
> > *encoder)
> > @@ -339,7 +342,7 @@ static void i915_digport_work_func(struct
> > work_struct *work)
> >  		spin_lock_irq(&dev_priv->irq_lock);
> >  		dev_priv->hotplug.event_bits |= old_bits;
> >  		spin_unlock_irq(&dev_priv->irq_lock);
> > -		schedule_work(&dev_priv->hotplug.hotplug_work);
> > +		schedule_delayed_work(&dev_priv->hotplug.hotplug_work,
> > 0);
> 
> Random thought: Should open code this with queue_delayed_work()
> instead
> so that we can be sure the wq really is the same between this call
> and the mod_delayed_work()?

Just send other version making it explicit but it was already using the
same work queue.

> 
> >  	}
> >  }
> >  
> > @@ -349,14 +352,16 @@ static void i915_digport_work_func(struct
> > work_struct *work)
> >  static void i915_hotplug_work_func(struct work_struct *work)
> >  {
> >  	struct drm_i915_private *dev_priv =
> > -		container_of(work, struct drm_i915_private,
> > hotplug.hotplug_work);
> > +		container_of(work, struct drm_i915_private,
> > +			     hotplug.hotplug_work.work);
> >  	struct drm_device *dev = &dev_priv->drm;
> >  	struct intel_connector *intel_connector;
> >  	struct intel_encoder *intel_encoder;
> >  	struct drm_connector *connector;
> >  	struct drm_connector_list_iter conn_iter;
> > -	bool changed = false;
> > +	u32 changed = 0, retry = 0;
> >  	u32 hpd_event_bits;
> > +	u32 hpd_retry_bits;
> >  
> >  	mutex_lock(&dev->mode_config.mutex);
> >  	DRM_DEBUG_KMS("running encoder hotplug functions\n");
> > @@ -365,6 +370,8 @@ static void i915_hotplug_work_func(struct
> > work_struct *work)
> >  
> >  	hpd_event_bits = dev_priv->hotplug.event_bits;
> >  	dev_priv->hotplug.event_bits = 0;
> > +	hpd_retry_bits = dev_priv->hotplug.retry_bits;
> > +	dev_priv->hotplug.retry_bits = 0;
> >  
> >  	/* Enable polling for connectors which had HPD IRQ storms */
> >  	intel_hpd_irq_storm_switch_to_polling(dev_priv);
> > @@ -373,16 +380,29 @@ static void i915_hotplug_work_func(struct
> > work_struct *work)
> >  
> >  	drm_connector_list_iter_begin(dev, &conn_iter);
> >  	drm_for_each_connector_iter(connector, &conn_iter) {
> > +		u32 hpd_bit;
> > +
> >  		intel_connector = to_intel_connector(connector);
> >  		if (!intel_connector->encoder)
> >  			continue;
> >  		intel_encoder = intel_connector->encoder;
> > -		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
> > +		hpd_bit = BIT(intel_encoder->hpd_pin);
> > +		if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) {
> >  			DRM_DEBUG_KMS("Connector %s (pin %i) received
> > hotplug event.\n",
> >  				      connector->name, intel_encoder-
> > >hpd_pin);
> >  
> > -			changed |= intel_encoder-
> > >hotplug(intel_encoder,
> > -							  intel_connect
> > or);
> > +			switch (intel_encoder->hotplug(intel_encoder,
> > +						       intel_connector,
> > +						       hpd_event_bits &
> > hpd_bit)) {
> > +			case INTEL_HOTPLUG_UNCHANGED:
> > +				break;
> > +			case INTEL_HOTPLUG_CHANGED:
> > +				changed |= hpd_bit;
> > +				break;
> > +			case INTEL_HOTPLUG_RETRY:
> > +				retry |= hpd_bit;
> > +				break;
> > +			}
> >  		}
> >  	}
> >  	drm_connector_list_iter_end(&conn_iter);
> > @@ -390,6 +410,17 @@ static void i915_hotplug_work_func(struct
> > work_struct *work)
> >  
> >  	if (changed)
> >  		drm_kms_helper_hotplug_event(dev);
> > +
> > +	/* Remove shared HPD pins that have changed */
> > +	retry &= ~changed;
> > +	if (retry) {
> > +		spin_lock_irq(&dev_priv->irq_lock);
> > +		dev_priv->hotplug.retry_bits |= retry;
> > +		spin_unlock_irq(&dev_priv->irq_lock);
> > +
> > +		mod_delayed_work(system_wq, &dev_priv-
> > >hotplug.hotplug_work,
> > +				 msecs_to_jiffies(HPD_RETRY_DELAY));
> > +	}
> >  }
> >  
> >  
> > @@ -516,7 +547,7 @@ void intel_hpd_irq_handler(struct
> > drm_i915_private *dev_priv,
> >  	if (queue_dig)
> >  		queue_work(dev_priv->hotplug.dp_wq, &dev_priv-
> > >hotplug.dig_port_work);
> >  	if (queue_hp)
> > -		schedule_work(&dev_priv->hotplug.hotplug_work);
> > +		schedule_delayed_work(&dev_priv->hotplug.hotplug_work,
> > 0);
> >  }
> >  
> >  /**
> > @@ -636,7 +667,8 @@ void intel_hpd_poll_init(struct
> > drm_i915_private *dev_priv)
> >  
> >  void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> >  {
> > -	INIT_WORK(&dev_priv->hotplug.hotplug_work,
> > i915_hotplug_work_func);
> > +	INIT_DELAYED_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.poll_init_work,
> > i915_hpd_poll_init_work);
> >  	INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
> > @@ -650,11 +682,12 @@ void intel_hpd_cancel_work(struct
> > drm_i915_private *dev_priv)
> >  	dev_priv->hotplug.long_port_mask = 0;
> >  	dev_priv->hotplug.short_port_mask = 0;
> >  	dev_priv->hotplug.event_bits = 0;
> > +	dev_priv->hotplug.retry_bits = 0;
> >  
> >  	spin_unlock_irq(&dev_priv->irq_lock);
> >  
> >  	cancel_work_sync(&dev_priv->hotplug.dig_port_work);
> > -	cancel_work_sync(&dev_priv->hotplug.hotplug_work);
> > +	cancel_delayed_work_sync(&dev_priv->hotplug.hotplug_work);
> >  	cancel_work_sync(&dev_priv->hotplug.poll_init_work);
> >  	cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
> >  }
> > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h
> > b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > index 805f897dbb7a..b0cd447b7fbc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > @@ -15,8 +15,9 @@ struct intel_connector;
> >  struct intel_encoder;
> >  
> >  void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
> > -bool intel_encoder_hotplug(struct intel_encoder *encoder,
> > -			   struct intel_connector *connector);
> > +enum intel_hotplug_state intel_encoder_hotplug(struct
> > intel_encoder *encoder,
> > +					       struct intel_connector
> > *connector,
> > +					       bool irq_received);
> >  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> >  			   u32 pin_mask, u32 long_mask);
> >  void intel_hpd_init(struct drm_i915_private *dev_priv);
> > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > index 3fe8eaef6bd8..639317b84e51 100644
> > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > @@ -1915,12 +1915,14 @@ static void
> > intel_sdvo_enable_hotplug(struct intel_encoder *encoder)
> >  			     &intel_sdvo->hotplug_active, 2);
> >  }
> >  
> > -static bool intel_sdvo_hotplug(struct intel_encoder *encoder,
> > -			       struct intel_connector *connector)
> > +static enum intel_hotplug_state
> > +intel_sdvo_hotplug(struct intel_encoder *encoder,
> > +		   struct intel_connector *connector,
> > +		   bool irq_received)
> >  {
> >  	intel_sdvo_enable_hotplug(encoder);
> >  
> > -	return intel_encoder_hotplug(encoder, connector);
> > +	return intel_encoder_hotplug(encoder, connector, irq_received);
> >  }
> >  
> >  static bool
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 3e4f58f19362..6d4bc983915f 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -4090,7 +4090,7 @@ static int i915_hpd_storm_ctl_show(struct
> > seq_file *m, void *data)
> >  	 */
> >  	intel_synchronize_irq(dev_priv);
> >  	flush_work(&dev_priv->hotplug.dig_port_work);
> > -	flush_work(&dev_priv->hotplug.hotplug_work);
> > +	flush_delayed_work(&dev_priv->hotplug.hotplug_work);
> >  
> >  	seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold);
> >  	seq_printf(m, "Detected: %s\n",
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index f9878cbef4d9..f471307aa704 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -163,7 +163,7 @@ enum hpd_pin {
> >  #define HPD_STORM_DEFAULT_THRESHOLD 50
> >  
> >  struct i915_hotplug {
> > -	struct work_struct hotplug_work;
> > +	struct delayed_work hotplug_work;
> >  
> >  	struct {
> >  		unsigned long last_jiffies;
> > @@ -175,6 +175,7 @@ struct i915_hotplug {
> >  		} state;
> >  	} stats[HPD_NUM_PINS];
> >  	u32 event_bits;
> > +	u32 retry_bits;
> >  	struct delayed_work reenable_work;
> >  
> >  	u32 long_port_mask;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 24c63ed45c6f..e58d3751ed43 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -101,14 +101,21 @@ struct intel_fbdev {
> >  	struct mutex hpd_lock;
> >  };
> >  
> > +enum intel_hotplug_state {
> > +	INTEL_HOTPLUG_UNCHANGED,
> > +	INTEL_HOTPLUG_CHANGED,
> > +	INTEL_HOTPLUG_RETRY,
> > +};
> > +
> >  struct intel_encoder {
> >  	struct drm_encoder base;
> >  
> >  	enum intel_output_type type;
> >  	enum port port;
> >  	unsigned int cloneable;
> > -	bool (*hotplug)(struct intel_encoder *encoder,
> > -			struct intel_connector *connector);
> > +	enum intel_hotplug_state (*hotplug)(struct intel_encoder
> > *encoder,
> > +					    struct intel_connector
> > *connector,
> > +					    bool irq_received);
> >  	enum intel_output_type (*compute_output_type)(struct
> > intel_encoder *,
> >  						      struct
> > intel_crtc_state *,
> >  						      struct
> > drm_connector_state *);
> > -- 
> > 2.22.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v4 1/2] drm/i915: Add support for retrying hotplug

[Souza, Jose]

On Thu, 2019-07-11 at 13:55 -0700, Shane McKee wrote:
> On Thu, Jul 11, 2019 at 11:05:15PM +0300, Ville Syrjälä wrote:
> > On Wed, Jul 10, 2019 at 03:14:59PM -0700, José Roberto de Souza
> > wrote:
> > > From: Imre Deak <imre.deak@intel.com>
> > > 
> > > There is some scenarios that we are aware that sink probe can
> > > fail,
> > > so lets add the infrastructure to let hotplug() hook to request
> > > another probe after some time.
> > > 
> > > v2: Handle shared HPD pins (Imre)
> > > v3: Rebased
> > > v4: Renamed INTEL_HOTPLUG_NOCHANGE to INTEL_HOTPLUG_UNCHANGED to
> > > keep
> > > it consistent(Rodrigo)
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Tested-by: Shane McKee <shane.mckee@intel.com>

I missed this =/

I will add it while merging, thanks for testing.


> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_ddi.c     | 12 ++--
> > >  drivers/gpu/drm/i915/display/intel_dp.c      | 12 ++--
> > >  drivers/gpu/drm/i915/display/intel_hotplug.c | 59
> > > +++++++++++++++-----
> > >  drivers/gpu/drm/i915/display/intel_hotplug.h |  5 +-
> > >  drivers/gpu/drm/i915/display/intel_sdvo.c    |  8 ++-
> > >  drivers/gpu/drm/i915/i915_debugfs.c          |  2 +-
> > >  drivers/gpu/drm/i915/i915_drv.h              |  3 +-
> > >  drivers/gpu/drm/i915/intel_drv.h             | 11 +++-
> > >  8 files changed, 80 insertions(+), 32 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index ad638e7f27bb..734c004800f8 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -4047,14 +4047,16 @@ static int intel_hdmi_reset_link(struct
> > > intel_encoder *encoder,
> > >  	return modeset_pipe(&crtc->base, ctx);
> > >  }
> > >  
> > > -static bool intel_ddi_hotplug(struct intel_encoder *encoder,
> > > -			      struct intel_connector *connector)
> > > +static enum intel_hotplug_state
> > > +intel_ddi_hotplug(struct intel_encoder *encoder,
> > > +		  struct intel_connector *connector,
> > > +		  bool irq_received)
> > >  {
> > >  	struct drm_modeset_acquire_ctx ctx;
> > > -	bool changed;
> > > +	enum intel_hotplug_state state;
> > >  	int ret;
> > >  
> > > -	changed = intel_encoder_hotplug(encoder, connector);
> > > +	state = intel_encoder_hotplug(encoder, connector,
> > > irq_received);
> > >  
> > >  	drm_modeset_acquire_init(&ctx, 0);
> > >  
> > > @@ -4076,7 +4078,7 @@ static bool intel_ddi_hotplug(struct
> > > intel_encoder *encoder,
> > >  	drm_modeset_acquire_fini(&ctx);
> > >  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
> > >  
> > > -	return changed;
> > > +	return state;
> > >  }
> > >  
> > >  static struct intel_connector *
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > > b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 0bdb7ecc5a81..4423abbc7907 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -4853,14 +4853,16 @@ int intel_dp_retrain_link(struct
> > > intel_encoder *encoder,
> > >   * retrain the link to get a picture. That's in case no
> > >   * userspace component reacted to intermittent HPD dip.
> > >   */
> > > -static bool intel_dp_hotplug(struct intel_encoder *encoder,
> > > -			     struct intel_connector *connector)
> > > +static enum intel_hotplug_state
> > > +intel_dp_hotplug(struct intel_encoder *encoder,
> > > +		 struct intel_connector *connector,
> > > +		 bool irq_received)
> > >  {
> > >  	struct drm_modeset_acquire_ctx ctx;
> > > -	bool changed;
> > > +	enum intel_hotplug_state state;
> > >  	int ret;
> > >  
> > > -	changed = intel_encoder_hotplug(encoder, connector);
> > > +	state = intel_encoder_hotplug(encoder, connector,
> > > irq_received);
> > >  
> > >  	drm_modeset_acquire_init(&ctx, 0);
> > >  
> > > @@ -4879,7 +4881,7 @@ static bool intel_dp_hotplug(struct
> > > intel_encoder *encoder,
> > >  	drm_modeset_acquire_fini(&ctx);
> > >  	WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
> > >  
> > > -	return changed;
> > > +	return state;
> > >  }
> > >  
> > >  static void intel_dp_check_service_irq(struct intel_dp
> > > *intel_dp)
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > index ea3de4acc850..2ca92780c659 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > > @@ -112,6 +112,7 @@ enum hpd_pin intel_hpd_pin_default(struct
> > > drm_i915_private *dev_priv,
> > >  
> > >  #define HPD_STORM_DETECT_PERIOD		1000
> > >  #define HPD_STORM_REENABLE_DELAY	(2 * 60 * 1000)
> > > +#define HPD_RETRY_DELAY			1000
> > >  
> > >  /**
> > >   * intel_hpd_irq_storm_detect - gather stats and detect HPD IRQ
> > > storm on a pin
> > > @@ -266,8 +267,10 @@ static void
> > > intel_hpd_irq_storm_reenable_work(struct work_struct *work)
> > >  	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
> > >  }
> > >  
> > > -bool intel_encoder_hotplug(struct intel_encoder *encoder,
> > > -			   struct intel_connector *connector)
> > > +enum intel_hotplug_state
> > > +intel_encoder_hotplug(struct intel_encoder *encoder,
> > > +		      struct intel_connector *connector,
> > > +		      bool irq_received)
> > >  {
> > >  	struct drm_device *dev = connector->base.dev;
> > >  	enum drm_connector_status old_status;
> > > @@ -279,7 +282,7 @@ bool intel_encoder_hotplug(struct
> > > intel_encoder *encoder,
> > >  		drm_helper_probe_detect(&connector->base, NULL, false);
> > >  
> > >  	if (old_status == connector->base.status)
> > > -		return false;
> > > +		return INTEL_HOTPLUG_UNCHANGED;
> > >  
> > >  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to
> > > %s\n",
> > >  		      connector->base.base.id,
> > > @@ -287,7 +290,7 @@ bool intel_encoder_hotplug(struct
> > > intel_encoder *encoder,
> > >  		      drm_get_connector_status_name(old_status),
> > >  		      drm_get_connector_status_name(connector-
> > > >base.status));
> > >  
> > > -	return true;
> > > +	return INTEL_HOTPLUG_CHANGED;
> > >  }
> > >  
> > >  static bool intel_encoder_has_hpd_pulse(struct intel_encoder
> > > *encoder)
> > > @@ -339,7 +342,7 @@ static void i915_digport_work_func(struct
> > > work_struct *work)
> > >  		spin_lock_irq(&dev_priv->irq_lock);
> > >  		dev_priv->hotplug.event_bits |= old_bits;
> > >  		spin_unlock_irq(&dev_priv->irq_lock);
> > > -		schedule_work(&dev_priv->hotplug.hotplug_work);
> > > +		schedule_delayed_work(&dev_priv->hotplug.hotplug_work,
> > > 0);
> > 
> > Random thought: Should open code this with queue_delayed_work()
> > instead
> > so that we can be sure the wq really is the same between this call
> > and the mod_delayed_work()?
> > 
> > >  	}
> > >  }
> > >  
> > > @@ -349,14 +352,16 @@ static void i915_digport_work_func(struct
> > > work_struct *work)
> > >  static void i915_hotplug_work_func(struct work_struct *work)
> > >  {
> > >  	struct drm_i915_private *dev_priv =
> > > -		container_of(work, struct drm_i915_private,
> > > hotplug.hotplug_work);
> > > +		container_of(work, struct drm_i915_private,
> > > +			     hotplug.hotplug_work.work);
> > >  	struct drm_device *dev = &dev_priv->drm;
> > >  	struct intel_connector *intel_connector;
> > >  	struct intel_encoder *intel_encoder;
> > >  	struct drm_connector *connector;
> > >  	struct drm_connector_list_iter conn_iter;
> > > -	bool changed = false;
> > > +	u32 changed = 0, retry = 0;
> > >  	u32 hpd_event_bits;
> > > +	u32 hpd_retry_bits;
> > >  
> > >  	mutex_lock(&dev->mode_config.mutex);
> > >  	DRM_DEBUG_KMS("running encoder hotplug functions\n");
> > > @@ -365,6 +370,8 @@ static void i915_hotplug_work_func(struct
> > > work_struct *work)
> > >  
> > >  	hpd_event_bits = dev_priv->hotplug.event_bits;
> > >  	dev_priv->hotplug.event_bits = 0;
> > > +	hpd_retry_bits = dev_priv->hotplug.retry_bits;
> > > +	dev_priv->hotplug.retry_bits = 0;
> > >  
> > >  	/* Enable polling for connectors which had HPD IRQ storms */
> > >  	intel_hpd_irq_storm_switch_to_polling(dev_priv);
> > > @@ -373,16 +380,29 @@ static void i915_hotplug_work_func(struct
> > > work_struct *work)
> > >  
> > >  	drm_connector_list_iter_begin(dev, &conn_iter);
> > >  	drm_for_each_connector_iter(connector, &conn_iter) {
> > > +		u32 hpd_bit;
> > > +
> > >  		intel_connector = to_intel_connector(connector);
> > >  		if (!intel_connector->encoder)
> > >  			continue;
> > >  		intel_encoder = intel_connector->encoder;
> > > -		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
> > > +		hpd_bit = BIT(intel_encoder->hpd_pin);
> > > +		if ((hpd_event_bits | hpd_retry_bits) & hpd_bit) {
> > >  			DRM_DEBUG_KMS("Connector %s (pin %i) received
> > > hotplug event.\n",
> > >  				      connector->name, intel_encoder-
> > > >hpd_pin);
> > >  
> > > -			changed |= intel_encoder-
> > > >hotplug(intel_encoder,
> > > -							  intel_connect
> > > or);
> > > +			switch (intel_encoder->hotplug(intel_encoder,
> > > +						       intel_connector,
> > > +						       hpd_event_bits &
> > > hpd_bit)) {
> > > +			case INTEL_HOTPLUG_UNCHANGED:
> > > +				break;
> > > +			case INTEL_HOTPLUG_CHANGED:
> > > +				changed |= hpd_bit;
> > > +				break;
> > > +			case INTEL_HOTPLUG_RETRY:
> > > +				retry |= hpd_bit;
> > > +				break;
> > > +			}
> > >  		}
> > >  	}
> > >  	drm_connector_list_iter_end(&conn_iter);
> > > @@ -390,6 +410,17 @@ static void i915_hotplug_work_func(struct
> > > work_struct *work)
> > >  
> > >  	if (changed)
> > >  		drm_kms_helper_hotplug_event(dev);
> > > +
> > > +	/* Remove shared HPD pins that have changed */
> > > +	retry &= ~changed;
> > > +	if (retry) {
> > > +		spin_lock_irq(&dev_priv->irq_lock);
> > > +		dev_priv->hotplug.retry_bits |= retry;
> > > +		spin_unlock_irq(&dev_priv->irq_lock);
> > > +
> > > +		mod_delayed_work(system_wq, &dev_priv-
> > > >hotplug.hotplug_work,
> > > +				 msecs_to_jiffies(HPD_RETRY_DELAY));
> > > +	}
> > >  }
> > >  
> > >  
> > > @@ -516,7 +547,7 @@ void intel_hpd_irq_handler(struct
> > > drm_i915_private *dev_priv,
> > >  	if (queue_dig)
> > >  		queue_work(dev_priv->hotplug.dp_wq, &dev_priv-
> > > >hotplug.dig_port_work);
> > >  	if (queue_hp)
> > > -		schedule_work(&dev_priv->hotplug.hotplug_work);
> > > +		schedule_delayed_work(&dev_priv->hotplug.hotplug_work,
> > > 0);
> > >  }
> > >  
> > >  /**
> > > @@ -636,7 +667,8 @@ void intel_hpd_poll_init(struct
> > > drm_i915_private *dev_priv)
> > >  
> > >  void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> > >  {
> > > -	INIT_WORK(&dev_priv->hotplug.hotplug_work,
> > > i915_hotplug_work_func);
> > > +	INIT_DELAYED_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.poll_init_work,
> > > i915_hpd_poll_init_work);
> > >  	INIT_DELAYED_WORK(&dev_priv->hotplug.reenable_work,
> > > @@ -650,11 +682,12 @@ void intel_hpd_cancel_work(struct
> > > drm_i915_private *dev_priv)
> > >  	dev_priv->hotplug.long_port_mask = 0;
> > >  	dev_priv->hotplug.short_port_mask = 0;
> > >  	dev_priv->hotplug.event_bits = 0;
> > > +	dev_priv->hotplug.retry_bits = 0;
> > >  
> > >  	spin_unlock_irq(&dev_priv->irq_lock);
> > >  
> > >  	cancel_work_sync(&dev_priv->hotplug.dig_port_work);
> > > -	cancel_work_sync(&dev_priv->hotplug.hotplug_work);
> > > +	cancel_delayed_work_sync(&dev_priv->hotplug.hotplug_work);
> > >  	cancel_work_sync(&dev_priv->hotplug.poll_init_work);
> > >  	cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h
> > > b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > > index 805f897dbb7a..b0cd447b7fbc 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > > @@ -15,8 +15,9 @@ struct intel_connector;
> > >  struct intel_encoder;
> > >  
> > >  void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
> > > -bool intel_encoder_hotplug(struct intel_encoder *encoder,
> > > -			   struct intel_connector *connector);
> > > +enum intel_hotplug_state intel_encoder_hotplug(struct
> > > intel_encoder *encoder,
> > > +					       struct intel_connector
> > > *connector,
> > > +					       bool irq_received);
> > >  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > >  			   u32 pin_mask, u32 long_mask);
> > >  void intel_hpd_init(struct drm_i915_private *dev_priv);
> > > diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > > b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > > index 3fe8eaef6bd8..639317b84e51 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> > > @@ -1915,12 +1915,14 @@ static void
> > > intel_sdvo_enable_hotplug(struct intel_encoder *encoder)
> > >  			     &intel_sdvo->hotplug_active, 2);
> > >  }
> > >  
> > > -static bool intel_sdvo_hotplug(struct intel_encoder *encoder,
> > > -			       struct intel_connector *connector)
> > > +static enum intel_hotplug_state
> > > +intel_sdvo_hotplug(struct intel_encoder *encoder,
> > > +		   struct intel_connector *connector,
> > > +		   bool irq_received)
> > >  {
> > >  	intel_sdvo_enable_hotplug(encoder);
> > >  
> > > -	return intel_encoder_hotplug(encoder, connector);
> > > +	return intel_encoder_hotplug(encoder, connector, irq_received);
> > >  }
> > >  
> > >  static bool
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 3e4f58f19362..6d4bc983915f 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -4090,7 +4090,7 @@ static int i915_hpd_storm_ctl_show(struct
> > > seq_file *m, void *data)
> > >  	 */
> > >  	intel_synchronize_irq(dev_priv);
> > >  	flush_work(&dev_priv->hotplug.dig_port_work);
> > > -	flush_work(&dev_priv->hotplug.hotplug_work);
> > > +	flush_delayed_work(&dev_priv->hotplug.hotplug_work);
> > >  
> > >  	seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold);
> > >  	seq_printf(m, "Detected: %s\n",
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index f9878cbef4d9..f471307aa704 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -163,7 +163,7 @@ enum hpd_pin {
> > >  #define HPD_STORM_DEFAULT_THRESHOLD 50
> > >  
> > >  struct i915_hotplug {
> > > -	struct work_struct hotplug_work;
> > > +	struct delayed_work hotplug_work;
> > >  
> > >  	struct {
> > >  		unsigned long last_jiffies;
> > > @@ -175,6 +175,7 @@ struct i915_hotplug {
> > >  		} state;
> > >  	} stats[HPD_NUM_PINS];
> > >  	u32 event_bits;
> > > +	u32 retry_bits;
> > >  	struct delayed_work reenable_work;
> > >  
> > >  	u32 long_port_mask;
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 24c63ed45c6f..e58d3751ed43 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -101,14 +101,21 @@ struct intel_fbdev {
> > >  	struct mutex hpd_lock;
> > >  };
> > >  
> > > +enum intel_hotplug_state {
> > > +	INTEL_HOTPLUG_UNCHANGED,
> > > +	INTEL_HOTPLUG_CHANGED,
> > > +	INTEL_HOTPLUG_RETRY,
> > > +};
> > > +
> > >  struct intel_encoder {
> > >  	struct drm_encoder base;
> > >  
> > >  	enum intel_output_type type;
> > >  	enum port port;
> > >  	unsigned int cloneable;
> > > -	bool (*hotplug)(struct intel_encoder *encoder,
> > > -			struct intel_connector *connector);
> > > +	enum intel_hotplug_state (*hotplug)(struct intel_encoder
> > > *encoder,
> > > +					    struct intel_connector
> > > *connector,
> > > +					    bool irq_received);
> > >  	enum intel_output_type (*compute_output_type)(struct
> > > intel_encoder *,
> > >  						      struct
> > > intel_crtc_state *,
> > >  						      struct
> > > drm_connector_state *);
> > > -- 
> > > 2.22.0
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx