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

* [PATCH v5 1/2] drm/i915: Add support for retrying hotplug
@ 2019-07-12  0:53 José Roberto de Souza
  2019-07-12  0:53 ` [PATCH v5 2/2] drm/i915: Enable hotplug retry José Roberto de Souza
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: José Roberto de Souza @ 2019-07-12  0:53 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

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)
v5: Making the working queue used explicit through all the callers to
hotplug_work (Ville)

Tested-by: Timo Aaltonen <tjaalton@ubuntu.com>
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 8445244aa593..c89d0c7543dc 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4070,14 +4070,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);
 
@@ -4099,7 +4101,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 24592d985bcb..a870ecab78a7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4856,14 +4856,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);
 
@@ -4882,7 +4884,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..342587d91d57 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);
+		queue_delayed_work(system_wq, &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);
+		queue_delayed_work(system_wq, &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 213843a93c4e..c5e2dfd7ef80 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -1921,12 +1921,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 41245acb0a0f..9fd2af2e21d0 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4085,7 +4085,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 75f7751dd0ee..2afd4fd05f3e 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 770f9f6aad84..e20c0d7cf849 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

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