[PATCH 0/2][SRU][D] drm/i915: Fix hotplug issues

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2][SRU][D] drm/i915: Fix hotplug issues

You-Sheng Yang
BugLink: http://bugs.launchpad.net/bugs/1835001

[Impact]
System does not always auto detect disconnection of external monitor on
Intel VGA.

[Fix]
Two changes from drm-intel tree are necessary to introduce hotplug
retrying to fix this issue. They are:

  * drm/i915: Add support for retrying hotplug
  * drm/i915: Enable hotplug retry

[Test Case]
1. open "system settings > display settings"
2. connect laptop to an external HDMI monitor via HDMI port & cable
3. unplug the HDMI cable
4. check if "display settings" automatically detects the removal of the
external monitor

[Regression potential]
Medium. For all the prerequisites they have been landed since 4.18, and
these two patches will only be merged some time later. Basically it
turns a currently existing kernel work to a delayed work so that it may
retry probing HDMI hotplug status.

Imre Deak (1):
  drm/i915: Add support for retrying hotplug

José Roberto de Souza (1):
  drm/i915: Enable hotplug retry

 drivers/gpu/drm/i915/i915_debugfs.c  |  2 +-
 drivers/gpu/drm/i915/i915_drv.h      |  3 +-
 drivers/gpu/drm/i915/intel_ddi.c     | 33 +++++++++++++---
 drivers/gpu/drm/i915/intel_dp.c      | 19 ++++++---
 drivers/gpu/drm/i915/intel_drv.h     | 16 ++++++--
 drivers/gpu/drm/i915/intel_hdmi.c    | 28 ++++++++++++-
 drivers/gpu/drm/i915/intel_hotplug.c | 59 ++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++--
 8 files changed, 135 insertions(+), 33 deletions(-)

--
2.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

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

You-Sheng Yang
From: Imre Deak <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1835001

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 <[hidden email]>
Cc: Ville Syrjälä <[hidden email]>
Reviewed-by: Rodrigo Vivi <[hidden email]>
Signed-off-by: José Roberto de Souza <[hidden email]>
Signed-off-by: Jani Nikula <[hidden email]>
Signed-off-by: Imre Deak <[hidden email]>
Link: https://patchwork.freedesktop.org/patch/msgid/20190712005343.24571-1-jose.souza@...
(backported from commit 3944709df8e9298225fc2b29e53ee8e6f4b26618
git://anongit.freedesktop.org/drm-intel)
Signed-off-by: You-Sheng Yang <[hidden email]>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  2 +-
 drivers/gpu/drm/i915/i915_drv.h      |  3 +-
 drivers/gpu/drm/i915/intel_ddi.c     | 12 +++---
 drivers/gpu/drm/i915/intel_dp.c      | 12 +++---
 drivers/gpu/drm/i915/intel_drv.h     | 16 ++++++--
 drivers/gpu/drm/i915/intel_hotplug.c | 59 ++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++--
 7 files changed, 80 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 40a61ef9aac1..8677473f5011 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4606,7 +4606,7 @@ static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data)
  */
  synchronize_irq(dev_priv->drm.irq);
  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 d84893191009..a3eab0942c70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -152,7 +152,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;
@@ -164,6 +164,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_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7edce1b7b348..6841ede41702 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3971,14 +3971,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);
 
@@ -4000,7 +4002,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/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dcd1df5322e8..89cc30df54d6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4627,14 +4627,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);
 
@@ -4653,7 +4655,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/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e9ddeaf05a14..51569073e4c4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -221,14 +221,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 *);
@@ -1904,8 +1911,9 @@ int intel_dsi_dcs_init_backlight_funcs(struct intel_connector *intel_connector);
 void intel_dvo_init(struct drm_i915_private *dev_priv);
 /* intel_hotplug.c */
 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);
 
 /* legacy fbdev emulation in intel_fbdev.c */
 #ifdef CONFIG_DRM_FBDEV_EMULATION
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index e24174d08fed..cc44d57b91d1 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/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
@@ -265,8 +266,10 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
  intel_runtime_pm_put(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)
 {
  struct drm_device *dev = connector->base.dev;
  enum drm_connector_status old_status;
@@ -278,7 +281,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,
@@ -286,7 +289,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)
@@ -338,7 +341,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);
  }
 }
 
@@ -348,14 +351,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");
@@ -364,6 +369,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);
@@ -372,16 +379,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);
@@ -389,6 +409,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));
+ }
 }
 
 
@@ -515,7 +546,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);
 }
 
 /**
@@ -635,7 +666,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,
@@ -649,11 +681,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/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 5805ec1aba12..f814f5c6bd23 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1722,12 +1722,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
--
2.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2][SRU][D] drm/i915: Enable hotplug retry

You-Sheng Yang
In reply to this post by You-Sheng Yang
From: José Roberto de Souza <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1835001

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)

Tested-by: Timo Aaltonen <[hidden email]>
Cc: Ville Syrjälä <[hidden email]>
Cc: Imre Deak <[hidden email]>
Cc: Jani Nikula <[hidden email]>
Reviewed-by: Imre Deak <[hidden email]>
Signed-off-by: José Roberto de Souza <[hidden email]>
Link: https://patchwork.freedesktop.org/patch/msgid/20190712005343.24571-2-jose.souza@...
(backported from commit bb80c9255770fa1ed54e889a6bee628bdd0f6762
git://anongit.freedesktop.org/drm-intel)
Signed-off-by: You-Sheng Yang <[hidden email]>
---
 drivers/gpu/drm/i915/intel_ddi.c  | 21 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c   |  7 +++++++
 drivers/gpu/drm/i915/intel_hdmi.c | 28 +++++++++++++++++++++++++++-
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 6841ede41702..090a183d68ce 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3976,6 +3976,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;
@@ -4002,6 +4003,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/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 89cc30df54d6..746a788006d7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4655,6 +4655,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/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 24aa2eb1eacd..ef80692f9175 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2462,6 +2462,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)
 {
@@ -2485,7 +2511,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.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [PATCH 0/2][SRU][D] drm/i915: Fix hotplug issues

Connor Kuehl
In reply to this post by You-Sheng Yang
On 7/23/19 12:39 AM, You-Sheng Yang wrote:

> BugLink: http://bugs.launchpad.net/bugs/1835001
>
> [Impact]
> System does not always auto detect disconnection of external monitor on
> Intel VGA.
>
> [Fix]
> Two changes from drm-intel tree are necessary to introduce hotplug
> retrying to fix this issue. They are:
>
>   * drm/i915: Add support for retrying hotplug
>   * drm/i915: Enable hotplug retry
>
> [Test Case]
> 1. open "system settings > display settings"
> 2. connect laptop to an external HDMI monitor via HDMI port & cable
> 3. unplug the HDMI cable
> 4. check if "display settings" automatically detects the removal of the
> external monitor
>
> [Regression potential]
> Medium. For all the prerequisites they have been landed since 4.18, and
> these two patches will only be merged some time later. Basically it
> turns a currently existing kernel work to a delayed work so that it may
> retry probing HDMI hotplug status.
>
> Imre Deak (1):
>   drm/i915: Add support for retrying hotplug
>
> José Roberto de Souza (1):
>   drm/i915: Enable hotplug retry
>
>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h      |  3 +-
>  drivers/gpu/drm/i915/intel_ddi.c     | 33 +++++++++++++---
>  drivers/gpu/drm/i915/intel_dp.c      | 19 ++++++---
>  drivers/gpu/drm/i915/intel_drv.h     | 16 ++++++--
>  drivers/gpu/drm/i915/intel_hdmi.c    | 28 ++++++++++++-
>  drivers/gpu/drm/i915/intel_hotplug.c | 59 ++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++--
>  8 files changed, 135 insertions(+), 33 deletions(-)
>

I think since these patches come from an upstream maintainer's tree,
their commit subjects should have "(upstream)" in them [1].

Aside from that, with positive test results:

Acked-by: Connor Kuehl <[hidden email]>

[1] https://wiki.ubuntu.com/Kernel/Dev/StablePatchFormat

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [PATCH 0/2][SRU][D] drm/i915: Fix hotplug issues

Stefan Bader-2
In reply to this post by You-Sheng Yang
On 23.07.19 09:39, You-Sheng Yang wrote:

> BugLink: http://bugs.launchpad.net/bugs/1835001
>
> [Impact]
> System does not always auto detect disconnection of external monitor on
> Intel VGA.
>
> [Fix]
> Two changes from drm-intel tree are necessary to introduce hotplug
> retrying to fix this issue. They are:
>
>   * drm/i915: Add support for retrying hotplug
>   * drm/i915: Enable hotplug retry
>
> [Test Case]
> 1. open "system settings > display settings"
> 2. connect laptop to an external HDMI monitor via HDMI port & cable
> 3. unplug the HDMI cable
> 4. check if "display settings" automatically detects the removal of the
> external monitor
>
> [Regression potential]
> Medium. For all the prerequisites they have been landed since 4.18, and
> these two patches will only be merged some time later. Basically it
> turns a currently existing kernel work to a delayed work so that it may
> retry probing HDMI hotplug status.
>
> Imre Deak (1):
>   drm/i915: Add support for retrying hotplug
>
> José Roberto de Souza (1):
>   drm/i915: Enable hotplug retry
>
>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h      |  3 +-
>  drivers/gpu/drm/i915/intel_ddi.c     | 33 +++++++++++++---
>  drivers/gpu/drm/i915/intel_dp.c      | 19 ++++++---
>  drivers/gpu/drm/i915/intel_drv.h     | 16 ++++++--
>  drivers/gpu/drm/i915/intel_hdmi.c    | 28 ++++++++++++-
>  drivers/gpu/drm/i915/intel_hotplug.c | 59 ++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++--
>  8 files changed, 135 insertions(+), 33 deletions(-)
>

Those are not upstream yet (though now in linux-next). The tag for neither
upstream linux nor linux-next normally would be "UBUNTU: SAUCE: " but since they
are already queued for -next we just need to ensure the SHA1 references are
updated (if needed).
Since this is a bigger change and by not being upstream likely did not receive
as much testing as it could have, I would be ok with adding this to Disco but
ignore the submission for Bionic for now.

Acked-by: Stefan Bader <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [PATCH 0/2][SRU][D] drm/i915: Fix hotplug issues

Khalid Elmously
In reply to this post by You-Sheng Yang
On 2019-07-23 15:39:26 , You-Sheng Yang wrote:

> BugLink: http://bugs.launchpad.net/bugs/1835001
>
> [Impact]
> System does not always auto detect disconnection of external monitor on
> Intel VGA.
>
> [Fix]
> Two changes from drm-intel tree are necessary to introduce hotplug
> retrying to fix this issue. They are:
>
>   * drm/i915: Add support for retrying hotplug
>   * drm/i915: Enable hotplug retry
>
> [Test Case]
> 1. open "system settings > display settings"
> 2. connect laptop to an external HDMI monitor via HDMI port & cable
> 3. unplug the HDMI cable
> 4. check if "display settings" automatically detects the removal of the
> external monitor
>
> [Regression potential]
> Medium. For all the prerequisites they have been landed since 4.18, and
> these two patches will only be merged some time later. Basically it
> turns a currently existing kernel work to a delayed work so that it may
> retry probing HDMI hotplug status.
>
> Imre Deak (1):
>   drm/i915: Add support for retrying hotplug
>
> José Roberto de Souza (1):
>   drm/i915: Enable hotplug retry
>
>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h      |  3 +-
>  drivers/gpu/drm/i915/intel_ddi.c     | 33 +++++++++++++---
>  drivers/gpu/drm/i915/intel_dp.c      | 19 ++++++---
>  drivers/gpu/drm/i915/intel_drv.h     | 16 ++++++--
>  drivers/gpu/drm/i915/intel_hdmi.c    | 28 ++++++++++++-
>  drivers/gpu/drm/i915/intel_hotplug.c | 59 ++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_sdvo.c    |  8 ++--
>  8 files changed, 135 insertions(+), 33 deletions(-)
>
> --
> 2.20.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team