[PATCH 0/7][SRU][OEM-B][B] drm/i915: Fix hotplug issues

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

[PATCH 0/7][SRU][OEM-B][B] 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 along with their prerequisites are
necessary to introduce hotplug retrying to fix this issue. They are:

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

All the prerequisites have been landed to kernel mainline, but the
target two changes have not.

[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
the last 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

Ville Syrjälä (5):
  drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
  drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
  drm/i915: Move SST DP link retraining into the ->post_hotplug() hook
  drm/i915: Nuke intel_dp->channel_eq_status
  drm/i915: Track whether the DP link is trained or not

 drivers/gpu/drm/i915/i915_debugfs.c           |   2 +
 drivers/gpu/drm/i915/i915_drv.h               |   3 +-
 drivers/gpu/drm/i915/intel_crt.c              |   4 +-
 drivers/gpu/drm/i915/intel_ddi.c              | 171 +++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c               | 179 +++++++++++++-----
 drivers/gpu/drm/i915/intel_dp_link_training.c |   8 +-
 drivers/gpu/drm/i915/intel_drv.h              |  18 +-
 drivers/gpu/drm/i915/intel_hdmi.c             |  27 +++
 drivers/gpu/drm/i915/intel_hotplug.c          |  76 ++++++--
 drivers/gpu/drm/i915/intel_sdvo.c             |  14 +-
 10 files changed, 423 insertions(+), 79 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/7][SRU][OEM-B][B] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook

You-Sheng Yang
From: Ville Syrjälä <[hidden email]>

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

Allow encoders to customize their hotplug processing by moving the
intel_hpd_irq_event() code into an encoder hotplug vfunc. Currently
only SDVO needs this to re-enable hotplug signalling in the SDVO
chip. We'll use this same hook for DP/HDMI link management later.

Reviewed-by: Jani Nikula <[hidden email]>
Signed-off-by: Ville Syrjälä <[hidden email]>
Reviewed-by: Lyude Paul <[hidden email]>
Signed-off-by: Lyude Paul <[hidden email]>
Link: https://patchwork.freedesktop.org/patch/msgid/20180117192149.17760-1-ville.syrjala@...
(cherry picked from commit 1b2cb026dc8b6f5cc4043031896a27745ad6f898)
Signed-off-by: You-Sheng Yang <[hidden email]>
---
 drivers/gpu/drm/i915/intel_sdvo.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 7437944b388f..1b9e9ac2fac9 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1690,7 +1690,15 @@ static void intel_sdvo_enable_hotplug(struct intel_encoder *encoder)
  struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
 
  intel_sdvo_write_cmd(intel_sdvo, SDVO_CMD_SET_ACTIVE_HOT_PLUG,
- &intel_sdvo->hotplug_active, 2);
+     &intel_sdvo->hotplug_active, 2);
+}
+
+static bool intel_sdvo_hotplug(struct intel_encoder *encoder,
+       struct intel_connector *connector)
+{
+ intel_sdvo_enable_hotplug(encoder);
+
+ return intel_encoder_hotplug(encoder, connector);
 }
 
 static bool
@@ -2494,7 +2502,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
  /* Some SDVO devices have one-shot hotplug interrupts.
  * Ensure that they get re-enabled when an interrupt happens.
  */
- intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
+ intel_encoder->hotplug = intel_sdvo_hotplug;
  intel_sdvo_enable_hotplug(intel_encoder);
  } else {
  intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
--
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/7][SRU][OEM-B][B] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD

You-Sheng Yang
In reply to this post by You-Sheng Yang
From: Ville Syrjälä <[hidden email]>

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

The LG 4k TV I have doesn't deassert HPD when I turn the TV off, but
when I turn it back on it will pulse the HPD line. By that time it has
forgotten everything we told it about scrambling and the clock ratio.
Hence if we want to get a picture out if it again we have to tell it
whether we're currently sending scrambled data or not. Implement
that via the encoder->hotplug() hook.

v2: Force a full modeset to not follow the HDMI 2.0 spec more
    closely (Shashank)

[pushed with whitespace fixes to make sparse happy]
Cc: Shashank Sharma <[hidden email]>
Cc: Maarten Lankhorst <[hidden email]>
Signed-off-by: Ville Syrjälä <[hidden email]>
Signed-off-by: Lyude Paul <[hidden email]>
Link: https://patchwork.freedesktop.org/patch/msgid/20180117192149.17760-1-ville.syrjala@...
(backported from commit dba14b27dd3ca5ce5b3a1d538862e7dce556dba7)
Signed-off-by: You-Sheng Yang <[hidden email]>
---
 drivers/gpu/drm/i915/intel_crt.c     |   4 +-
 drivers/gpu/drm/i915/intel_ddi.c     | 146 +++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c      |   1 +
 drivers/gpu/drm/i915/intel_drv.h     |   6 +-
 drivers/gpu/drm/i915/intel_hdmi.c    |   1 +
 drivers/gpu/drm/i915/intel_hotplug.c |  25 ++---
 6 files changed, 168 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 437339f5d098..8bc8bdceceb7 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -970,8 +970,10 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
  crt->base.power_domain = POWER_DOMAIN_PORT_CRT;
 
  if (I915_HAS_HOTPLUG(dev_priv) &&
-    !dmi_check_system(intel_spurious_crt_detect))
+    !dmi_check_system(intel_spurious_crt_detect)) {
  crt->base.hpd_pin = HPD_CRT;
+ crt->base.hotplug = intel_encoder_hotplug;
+ }
 
  if (HAS_DDI(dev_priv)) {
  crt->base.port = PORT_E;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index bf55d9055b6c..e941111d563a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -25,6 +25,7 @@
  *
  */
 
+#include <drm/drm_scdc_helper.h>
 #include "i915_drv.h"
 #include "intel_drv.h"
 
@@ -2705,6 +2706,147 @@ intel_ddi_init_dp_connector(struct intel_digital_port *intel_dig_port)
  return connector;
 }
 
+static int modeset_pipe(struct drm_crtc *crtc,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_atomic_state *state;
+ struct drm_crtc_state *crtc_state;
+ int ret;
+
+ state = drm_atomic_state_alloc(crtc->dev);
+ if (!state)
+ return -ENOMEM;
+
+ state->acquire_ctx = ctx;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state)) {
+ ret = PTR_ERR(crtc_state);
+ goto out;
+ }
+
+ crtc_state->mode_changed = true;
+
+ ret = drm_atomic_add_affected_connectors(state, crtc);
+ if (ret)
+ goto out;
+
+ ret = drm_atomic_add_affected_planes(state, crtc);
+ if (ret)
+ goto out;
+
+ ret = drm_atomic_commit(state);
+ if (ret)
+ goto out;
+
+ return 0;
+
+ out:
+ drm_atomic_state_put(state);
+
+ return ret;
+}
+
+static int intel_hdmi_reset_link(struct intel_encoder *encoder,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_hdmi *hdmi = enc_to_intel_hdmi(&encoder->base);
+ struct intel_connector *connector = hdmi->attached_connector;
+ struct i2c_adapter *adapter =
+ intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus);
+ struct drm_connector_state *conn_state;
+ struct intel_crtc_state *crtc_state;
+ struct intel_crtc *crtc;
+ u8 config;
+ int ret;
+
+ if (!connector || connector->base.status != connector_status_connected)
+ return 0;
+
+ ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
+       ctx);
+ if (ret)
+ return ret;
+
+ conn_state = connector->base.state;
+
+ crtc = to_intel_crtc(conn_state->crtc);
+ if (!crtc)
+ return 0;
+
+ ret = drm_modeset_lock(&crtc->base.mutex, ctx);
+ if (ret)
+ return ret;
+
+ crtc_state = to_intel_crtc_state(crtc->base.state);
+
+ WARN_ON(!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI));
+
+ if (!crtc_state->base.active)
+ return 0;
+
+ if (!crtc_state->hdmi_high_tmds_clock_ratio &&
+    !crtc_state->hdmi_scrambling)
+ return 0;
+
+ if (conn_state->commit &&
+    !try_wait_for_completion(&conn_state->commit->hw_done))
+ return 0;
+
+ ret = drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, &config);
+ if (ret < 0) {
+ DRM_ERROR("Failed to read TMDS config: %d\n", ret);
+ return 0;
+ }
+
+ if (!!(config & SCDC_TMDS_BIT_CLOCK_RATIO_BY_40) ==
+    crtc_state->hdmi_high_tmds_clock_ratio &&
+    !!(config & SCDC_SCRAMBLING_ENABLE) ==
+    crtc_state->hdmi_scrambling)
+ return 0;
+
+ /*
+ * HDMI 2.0 says that one should not send scrambled data
+ * prior to configuring the sink scrambling, and that
+ * TMDS clock/data transmission should be suspended when
+ * changing the TMDS clock rate in the sink. So let's
+ * just do a full modeset here, even though some sinks
+ * would be perfectly happy if were to just reconfigure
+ * the SCDC settings on the fly.
+ */
+ return modeset_pipe(&crtc->base, ctx);
+}
+
+static bool intel_ddi_hotplug(struct intel_encoder *encoder,
+      struct intel_connector *connector)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ bool changed;
+ int ret;
+
+ changed = intel_encoder_hotplug(encoder, connector);
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ for (;;) {
+ ret = intel_hdmi_reset_link(encoder, &ctx);
+
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ continue;
+ }
+
+ break;
+ }
+
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+ WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
+
+ return changed;
+}
+
 static struct intel_connector *
 intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port)
 {
@@ -2787,6 +2929,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
  drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
  DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
 
+ if (init_hdmi)
+ intel_encoder->hotplug = intel_ddi_hotplug;
+ else
+ intel_encoder->hotplug = intel_encoder_hotplug;
  intel_encoder->compute_config = intel_ddi_compute_config;
  intel_encoder->enable = intel_enable_ddi;
  if (IS_GEN9_LP(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2ddc401727ba..ffe0034e0937 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6135,6 +6135,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
      "DP %c", port_name(port)))
  goto err_encoder_init;
 
+ intel_encoder->hotplug = intel_encoder_hotplug;
  intel_encoder->compute_config = intel_dp_compute_config;
  intel_encoder->get_hw_state = intel_dp_get_hw_state;
  intel_encoder->get_config = intel_dp_get_config;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a7459a17d2d4..57a0da7bd62c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -215,7 +215,8 @@ struct intel_encoder {
  enum intel_output_type type;
  enum port port;
  unsigned int cloneable;
- void (*hot_plug)(struct intel_encoder *);
+ bool (*hotplug)(struct intel_encoder *encoder,
+ struct intel_connector *connector);
  bool (*compute_config)(struct intel_encoder *,
        struct intel_crtc_state *,
        struct drm_connector_state *);
@@ -1611,7 +1612,8 @@ 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);
 
 /* legacy fbdev emulation in intel_fbdev.c */
 #ifdef CONFIG_DRM_FBDEV_EMULATION
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index d05b3cb9e150..e4659cf6d420 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2087,6 +2087,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->compute_config = intel_hdmi_compute_config;
  if (HAS_PCH_SPLIT(dev_priv)) {
  intel_encoder->disable = pch_disable_hdmi;
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 875d5d218d5c..279b44f5faf1 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -263,24 +263,26 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
  intel_runtime_pm_put(dev_priv);
 }
 
-static bool intel_hpd_irq_event(struct drm_device *dev,
- struct drm_connector *connector)
+bool intel_encoder_hotplug(struct intel_encoder *encoder,
+   struct intel_connector *connector)
 {
+ struct drm_device *dev = connector->base.dev;
  enum drm_connector_status old_status;
 
  WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
- old_status = connector->status;
+ old_status = connector->base.status;
 
- connector->status = drm_helper_probe_detect(connector, NULL, false);
+ connector->base.status =
+ drm_helper_probe_detect(&connector->base, NULL, false);
 
- if (old_status == connector->status)
+ if (old_status == connector->base.status)
  return false;
 
  DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
-      connector->base.id,
-      connector->name,
+      connector->base.base.id,
+      connector->base.name,
       drm_get_connector_status_name(old_status),
-      drm_get_connector_status_name(connector->status));
+      drm_get_connector_status_name(connector->base.status));
 
  return true;
 }
@@ -370,10 +372,9 @@ static void i915_hotplug_work_func(struct work_struct *work)
  if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
  DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
       connector->name, intel_encoder->hpd_pin);
- if (intel_encoder->hot_plug)
- intel_encoder->hot_plug(intel_encoder);
- if (intel_hpd_irq_event(dev, connector))
- changed = true;
+
+ changed |= intel_encoder->hotplug(intel_encoder,
+  intel_connector);
  }
  }
  drm_connector_list_iter_end(&conn_iter);
--
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 3/7][SRU][OEM-B][B] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook

You-Sheng Yang
In reply to this post by You-Sheng Yang
From: Ville Syrjälä <[hidden email]>

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

Doing link retraining from the short pulse handler is problematic since
that might introduce deadlocks with MST sideband processing. Currently
we don't retrain MST links from this code, but we want to change that.
So better to move the entire thing to the hotplug work. We can utilize
the new encoder->hotplug() hook for this.

The only thing we leave in the short pulse handler is the link status
check. That one still depends on the link parameters stored under
intel_dp, so no locking around that but races should be mostly harmless
as the actual retraining code will recheck the link state if we
end up there by mistake.

v2: Rebase due to ->post_hotplug() now being just ->hotplug()
    Check the connector type to figure out if we should do
    the HDMI thing or the DP think for DDI

[pushed with whitespace changes for sparse]
Cc: Manasi Navare <[hidden email]>
Cc: Maarten Lankhorst <[hidden email]>
Signed-off-by: Ville Syrjälä <[hidden email]>
Acked-by: Manasi Navare <[hidden email]>
Signed-off-by: Lyude Paul <[hidden email]>
Link: https://patchwork.freedesktop.org/patch/msgid/20180117192149.17760-3-ville.syrjala@...
(backported from commit c85d200e832197e23ceeadfda9745646a242fd46)
Signed-off-by: You-Sheng Yang <[hidden email]>
---
 drivers/gpu/drm/i915/intel_ddi.c |  10 +-
 drivers/gpu/drm/i915/intel_dp.c  | 169 ++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_drv.h |   2 +
 3 files changed, 126 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e941111d563a..04b2ed81d805 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2830,7 +2830,10 @@ static bool intel_ddi_hotplug(struct intel_encoder *encoder,
  drm_modeset_acquire_init(&ctx, 0);
 
  for (;;) {
- ret = intel_hdmi_reset_link(encoder, &ctx);
+ if (connector->base.connector_type == DRM_MODE_CONNECTOR_HDMIA)
+ ret = intel_hdmi_reset_link(encoder, &ctx);
+ else
+ ret = intel_dp_retrain_link(encoder, &ctx);
 
  if (ret == -EDEADLK) {
  drm_modeset_backoff(&ctx);
@@ -2929,10 +2932,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
  drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
  DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
 
- if (init_hdmi)
- intel_encoder->hotplug = intel_ddi_hotplug;
- else
- intel_encoder->hotplug = intel_encoder_hotplug;
+ intel_encoder->hotplug = intel_ddi_hotplug;
  intel_encoder->compute_config = intel_ddi_compute_config;
  intel_encoder->enable = intel_enable_ddi;
  if (IS_GEN9_LP(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ffe0034e0937..8fb9a1cbfb96 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4242,12 +4242,84 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
  return -EINVAL;
 }
 
-static void
-intel_dp_retrain_link(struct intel_dp *intel_dp)
+static bool
+intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
+{
+ u8 link_status[DP_LINK_STATUS_SIZE];
+
+ if (!intel_dp_get_link_status(intel_dp, link_status)) {
+ DRM_ERROR("Failed to get link status\n");
+ return false;
+ }
+
+ /*
+ * Validate the cached values of intel_dp->link_rate and
+ * intel_dp->lane_count before attempting to retrain.
+ */
+ if (!intel_dp_link_params_valid(intel_dp, intel_dp->link_rate,
+ intel_dp->lane_count))
+ return false;
+
+ /* Retrain if Channel EQ or CR not ok */
+ return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
+}
+
+/*
+ * If display is now connected check links status,
+ * there has been known issues of link loss triggering
+ * long pulse.
+ *
+ * Some sinks (eg. ASUS PB287Q) seem to perform some
+ * weird HPD ping pong during modesets. So we can apparently
+ * end up with HPD going low during a modeset, and then
+ * going back up soon after. And once that happens we must
+ * retrain the link to get a picture. That's in case no
+ * userspace component reacted to intermittent HPD dip.
+ */
+int intel_dp_retrain_link(struct intel_encoder *encoder,
+  struct drm_modeset_acquire_ctx *ctx)
 {
- struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
  struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
- struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
+ struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
+ struct intel_connector *connector = intel_dp->attached_connector;
+ struct drm_connector_state *conn_state;
+ struct intel_crtc_state *crtc_state;
+ struct intel_crtc *crtc;
+ int ret;
+
+ /* FIXME handle the MST connectors as well */
+
+ if (!connector || connector->base.status != connector_status_connected)
+ return 0;
+
+ ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
+       ctx);
+ if (ret)
+ return ret;
+
+ conn_state = connector->base.state;
+
+ crtc = to_intel_crtc(conn_state->crtc);
+ if (!crtc)
+ return 0;
+
+ ret = drm_modeset_lock(&crtc->base.mutex, ctx);
+ if (ret)
+ return ret;
+
+ crtc_state = to_intel_crtc_state(crtc->base.state);
+
+ WARN_ON(!intel_crtc_has_dp_encoder(crtc_state));
+
+ if (!crtc_state->base.active)
+ return 0;
+
+ if (conn_state->commit &&
+    !try_wait_for_completion(&conn_state->commit->hw_done))
+ return 0;
+
+ if (!intel_dp_needs_link_retrain(intel_dp))
+ return 0;
 
  /* Suppress underruns caused by re-training */
  intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
@@ -4265,43 +4337,49 @@ intel_dp_retrain_link(struct intel_dp *intel_dp)
  if (crtc->config->has_pch_encoder)
  intel_set_pch_fifo_underrun_reporting(dev_priv,
       intel_crtc_pch_transcoder(crtc), true);
+
+ return 0;
 }
 
-static void
-intel_dp_check_link_status(struct intel_dp *intel_dp)
+/*
+ * If display is now connected check links status,
+ * there has been known issues of link loss triggering
+ * long pulse.
+ *
+ * Some sinks (eg. ASUS PB287Q) seem to perform some
+ * weird HPD ping pong during modesets. So we can apparently
+ * end up with HPD going low during a modeset, and then
+ * going back up soon after. And once that happens we must
+ * 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)
 {
- struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
- struct drm_device *dev = intel_dp_to_dev(intel_dp);
- u8 link_status[DP_LINK_STATUS_SIZE];
+ struct drm_modeset_acquire_ctx ctx;
+ bool changed;
+ int ret;
 
- WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
+ changed = intel_encoder_hotplug(encoder, connector);
 
- if (!intel_dp_get_link_status(intel_dp, link_status)) {
- DRM_ERROR("Failed to get link status\n");
- return;
- }
+ drm_modeset_acquire_init(&ctx, 0);
 
- if (!intel_encoder->base.crtc)
- return;
+ for (;;) {
+ ret = intel_dp_retrain_link(encoder, &ctx);
 
- if (!to_intel_crtc(intel_encoder->base.crtc)->active)
- return;
+ if (ret == -EDEADLK) {
+ drm_modeset_backoff(&ctx);
+ continue;
+ }
 
- /*
- * Validate the cached values of intel_dp->link_rate and
- * intel_dp->lane_count before attempting to retrain.
- */
- if (!intel_dp_link_params_valid(intel_dp, intel_dp->link_rate,
- intel_dp->lane_count))
- return;
+ break;
+ }
 
- /* Retrain if Channel EQ or CR not ok */
- if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
- DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
-      intel_encoder->base.name);
+ drm_modeset_drop_locks(&ctx);
+ drm_modeset_acquire_fini(&ctx);
+ WARN(ret, "Acquiring modeset locks failed with %i\n", ret);
 
- intel_dp_retrain_link(intel_dp);
- }
+ return changed;
 }
 
 /*
@@ -4360,9 +4438,10 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
  DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
  }
 
- drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
- intel_dp_check_link_status(intel_dp);
- drm_modeset_unlock(&dev->mode_config.connection_mutex);
+ /* defer to the hotplug work for link retraining if needed */
+ if (intel_dp_needs_link_retrain(intel_dp))
+ return false;
+
  if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
  DRM_DEBUG_KMS("Link Training Compliance Test requested\n");
  /* Send a Hotplug Uevent to userspace to start modeset */
@@ -4753,20 +4832,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
  */
  status = connector_status_disconnected;
  goto out;
- } else {
- /*
- * If display is now connected check links status,
- * there has been known issues of link loss triggerring
- * long pulse.
- *
- * Some sinks (eg. ASUS PB287Q) seem to perform some
- * weird HPD ping pong during modesets. So we can apparently
- * end up with HPD going low during a modeset, and then
- * going back up soon after. And once that happens we must
- * retrain the link to get a picture. That's in case no
- * userspace component reacted to intermittent HPD dip.
- */
- intel_dp_check_link_status(intel_dp);
  }
 
  /*
@@ -5110,7 +5175,11 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
  }
 
  if (!intel_dp->is_mst) {
- if (!intel_dp_short_pulse(intel_dp)) {
+ bool handled;
+
+ handled = intel_dp_short_pulse(intel_dp);
+
+ if (!handled) {
  intel_dp->detect_done = false;
  goto put_power;
  }
@@ -6135,7 +6204,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
      "DP %c", port_name(port)))
  goto err_encoder_init;
 
- intel_encoder->hotplug = intel_encoder_hotplug;
+ intel_encoder->hotplug = intel_dp_hotplug;
  intel_encoder->compute_config = intel_dp_compute_config;
  intel_encoder->get_hw_state = intel_dp_get_hw_state;
  intel_encoder->get_config = intel_dp_get_config;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 57a0da7bd62c..5ad62aa51f08 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1533,6 +1533,8 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp,
     int link_rate, uint8_t lane_count);
 void intel_dp_start_link_train(struct intel_dp *intel_dp);
 void intel_dp_stop_link_train(struct intel_dp *intel_dp);
+int intel_dp_retrain_link(struct intel_encoder *encoder,
+  struct drm_modeset_acquire_ctx *ctx);
 void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
--
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 4/7][SRU][OEM-B][B] drm/i915: Nuke intel_dp->channel_eq_status

You-Sheng Yang
In reply to this post by You-Sheng Yang
From: Ville Syrjälä <[hidden email]>

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

intel_dp->channel_eq_status is used in exactly one function, and we
don't need it to persist between calls. So just go back to using a
local variable instead.

Signed-off-by: Ville Syrjälä <[hidden email]>
Reviewed-by: Rodrigo Vivi <[hidden email]>
Reviewed-by: Lyude Paul <[hidden email]>
Reviewed-by: Manasi Navare <[hidden email]>
Signed-off-by: Lyude Paul <[hidden email]>
Link: https://patchwork.freedesktop.org/patch/msgid/20180117192149.17760-4-ville.syrjala@...
(cherry picked from commit 2fed7955bf4c2e87e8b3759939fd0ad961da776e)
Signed-off-by: You-Sheng Yang <[hidden email]>
---
 drivers/gpu/drm/i915/intel_dp_link_training.c | 6 +++---
 drivers/gpu/drm/i915/intel_drv.h              | 1 -
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 05907fa8a553..1314f5d87d7d 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -248,6 +248,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
  int tries;
  u32 training_pattern;
  uint8_t link_status[DP_LINK_STATUS_SIZE];
+ bool channel_eq = false;
 
  training_pattern = intel_dp_training_pattern(intel_dp);
 
@@ -259,7 +260,6 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
  return false;
  }
 
- intel_dp->channel_eq_status = false;
  for (tries = 0; tries < 5; tries++) {
 
  drm_dp_link_train_channel_eq_delay(intel_dp->dpcd);
@@ -279,7 +279,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
 
  if (drm_dp_channel_eq_ok(link_status,
  intel_dp->lane_count)) {
- intel_dp->channel_eq_status = true;
+ channel_eq = true;
  DRM_DEBUG_KMS("Channel EQ done. DP Training "
       "successful\n");
  break;
@@ -301,7 +301,7 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
 
  intel_dp_set_idle_link_train(intel_dp);
 
- return intel_dp->channel_eq_status;
+ return channel_eq;
 
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5ad62aa51f08..cede0f93de39 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -964,7 +964,6 @@ struct intel_dp {
  bool link_mst;
  bool has_audio;
  bool detect_done;
- bool channel_eq_status;
  bool reset_link_params;
  uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
  uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
--
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 5/7][SRU][OEM-B][B] drm/i915: Track whether the DP link is trained or not

You-Sheng Yang
In reply to this post by You-Sheng Yang
From: Ville Syrjälä <[hidden email]>

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

LSPCON likes to throw short HPDs during the enable seqeunce prior to the
link being trained. These obviously result in the channel CR/EQ check
failing and thus we schedule a pointless hotplug work to retrain the
link. Avoid that by ignoring the bad CR/EQ status until we've actually
initially trained the link.

I've not actually investigated to see what LSPCON is trying to signal
with the short pulse. But as long as it signals anything I think we're
supposed to check the link status anyway, so I don't really see other
good ways to solve this. I've not seen these short pulses being
generated by normal DP sinks.

Signed-off-by: Ville Syrjälä <[hidden email]>
Reviewed-by: Lyude Paul <[hidden email]>
Signed-off-by: Lyude Paul <[hidden email]>
Link: https://patchwork.freedesktop.org/patch/msgid/20180117192149.17760-5-ville.syrjala@...
(cherry picked from commit edb2e5301c4489d8c99b0f3d86a074df27f6f8ff)
Signed-off-by: You-Sheng Yang <[hidden email]>
---
 drivers/gpu/drm/i915/intel_ddi.c              |  2 ++
 drivers/gpu/drm/i915/intel_dp.c               | 10 +++++++---
 drivers/gpu/drm/i915/intel_dp_link_training.c |  2 ++
 drivers/gpu/drm/i915/intel_drv.h              |  1 +
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 04b2ed81d805..123e400abf9b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2455,6 +2455,8 @@ static void intel_disable_ddi_dp(struct intel_encoder *encoder,
 {
  struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
+ intel_dp->link_trained = false;
+
  if (old_crtc_state->has_audio)
  intel_audio_codec_disable(encoder);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8fb9a1cbfb96..b9813e1d9229 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1826,6 +1826,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp,
       int link_rate, uint8_t lane_count,
       bool link_mst)
 {
+ intel_dp->link_trained = false;
  intel_dp->link_rate = link_rate;
  intel_dp->lane_count = lane_count;
  intel_dp->link_mst = link_mst;
@@ -2664,6 +2665,8 @@ static void intel_disable_dp(struct intel_encoder *encoder,
 {
  struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 
+ intel_dp->link_trained = false;
+
  if (old_crtc_state->has_audio)
  intel_audio_codec_disable(encoder);
 
@@ -4247,10 +4250,11 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
 {
  u8 link_status[DP_LINK_STATUS_SIZE];
 
- if (!intel_dp_get_link_status(intel_dp, link_status)) {
- DRM_ERROR("Failed to get link status\n");
+ if (!intel_dp->link_trained)
+ return false;
+
+ if (!intel_dp_get_link_status(intel_dp, link_status))
  return false;
- }
 
  /*
  * Validate the cached values of intel_dp->link_rate and
diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
index 1314f5d87d7d..78f1fe934da3 100644
--- a/drivers/gpu/drm/i915/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
@@ -307,6 +307,8 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
 
 void intel_dp_stop_link_train(struct intel_dp *intel_dp)
 {
+ intel_dp->link_trained = true;
+
  intel_dp_set_link_train(intel_dp,
  DP_TRAINING_PATTERN_DISABLE);
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cede0f93de39..4e05a94ddf24 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -962,6 +962,7 @@ struct intel_dp {
  uint8_t lane_count;
  uint8_t sink_count;
  bool link_mst;
+ bool link_trained;
  bool has_audio;
  bool detect_done;
  bool reset_link_params;
--
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 6/7][SRU][OEM-B][B] drm/i915: Add support for retrying hotplug

You-Sheng Yang
In reply to this post by 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, 81 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c65e381b85f3..8ca15f7f37e1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4637,6 +4637,8 @@ static int i915_hpd_storm_ctl_show(struct seq_file *m, void *data)
  struct drm_i915_private *dev_priv = m->private;
  struct i915_hotplug *hotplug = &dev_priv->hotplug;
 
+ flush_delayed_work(&dev_priv->hotplug.hotplug_work);
+
  seq_printf(m, "Threshold: %d\n", hotplug->hpd_storm_threshold);
  seq_printf(m, "Detected: %s\n",
    yesno(delayed_work_pending(&hotplug->reenable_work)));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 96b8ac6df733..2ffe6dc6cad0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -430,7 +430,7 @@ enum hpd_pin {
 #define HPD_STORM_DEFAULT_THRESHOLD 5
 
 struct i915_hotplug {
- struct work_struct hotplug_work;
+ struct delayed_work hotplug_work;
 
  struct {
  unsigned long last_jiffies;
@@ -442,6 +442,7 @@ struct i915_hotplug {
  } state;
  } stats[HPD_NUM_PINS];
  u32 event_bits;
+ u32 retry_bits;
  struct delayed_work reenable_work;
 
  struct intel_digital_port *irq_port[I915_MAX_PORTS];
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 123e400abf9b..5aac71f23b2f 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2820,14 +2820,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);
 
@@ -2849,7 +2851,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 b9813e1d9229..27076de4e4ae 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4357,14 +4357,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);
 
@@ -4383,7 +4385,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;
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4e05a94ddf24..aed109b97946 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -209,14 +209,21 @@ struct intel_fbdev {
  int preferred_bpp;
 };
 
+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);
  bool (*compute_config)(struct intel_encoder *,
        struct intel_crtc_state *,
        struct drm_connector_state *);
@@ -1614,8 +1621,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 279b44f5faf1..08a09c7d3627 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -129,6 +129,7 @@ enum hpd_pin intel_hpd_pin(enum port port)
 
 #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
@@ -263,8 +264,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;
@@ -276,7 +279,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,
@@ -284,7 +287,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 void i915_digport_work_func(struct work_struct *work)
@@ -331,7 +334,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);
  }
 }
 
@@ -341,14 +344,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");
@@ -357,6 +362,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;
 
  /* Disable hotplug on connectors that hit an irq storm. */
  intel_hpd_irq_storm_disable(dev_priv);
@@ -365,16 +372,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);
@@ -382,6 +402,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));
+ }
 }
 
 
@@ -480,7 +511,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);
 }
 
 /**
@@ -600,7 +631,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,
@@ -614,11 +646,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 1b9e9ac2fac9..a49b2f107919 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1693,12 +1693,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 7/7][SRU][OEM-B][B] 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 5aac71f23b2f..af60d03a2085 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2825,6 +2825,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;
@@ -2851,6 +2852,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 27076de4e4ae..0b0e104af88e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4385,6 +4385,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 e4659cf6d420..e6f652005df6 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2064,6 +2064,32 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
  }
 }
 
+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)
 {
@@ -2087,7 +2113,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/APPLIED[OEM-B]: [PATCH 0/7][SRU][OEM-B][B] drm/i915: Fix hotplug issues

AceLan Kao
In reply to this post by You-Sheng Yang
Applied on Ubuntu-oem-4.15.0-1048.53
Acked-By: AceLan Kao <[hidden email]>

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