[PATCH 1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook

[PATCH 1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     |  4 +++-
 drivers/gpu/drm/i915/intel_ddi.c     |  1 +
 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 | 24 ++++++++++++------------
 drivers/gpu/drm/i915/intel_sdvo.c    | 12 ++++++++++--
 7 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 9f31aea51dff..9bc47cff5409 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -966,8 +966,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 6260a882fbe4..1aeae3e97013 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2866,6 +2866,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));
 
+	intel_encoder->hotplug = intel_encoder_hotplug;
 	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
 	intel_encoder->compute_config = intel_ddi_compute_config;
 	intel_encoder->enable = intel_enable_ddi;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 68229f53d5b8..6bbf14410c2a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6400,6 +6400,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 731dc36d7129..7537b2d542fd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -214,7 +214,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);
 	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
 						      struct intel_crtc_state *,
 						      struct drm_connector_state *);
@@ -1690,7 +1691,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 691f15b59124..4a93cfd7a28e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2348,6 +2348,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..0191c7831a06 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -263,24 +263,25 @@ 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 +371,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);
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 2b8764897d68..5b1ad42ec446 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1692,7 +1692,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
@@ -2496,7 +2504,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.13.6

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

[PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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)

Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 146 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 145 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1aeae3e97013..25793bdc692f 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"
 
@@ -2756,6 +2757,146 @@ 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)
 {
@@ -2866,7 +3007,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));
 
-	intel_encoder->hotplug = intel_encoder_hotplug;
+	if (init_hdmi)
+		intel_encoder->hotplug = intel_ddi_hotplug;
+	else
+		intel_encoder->hotplug = intel_encoder_hotplug;
 	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
 	intel_encoder->compute_config = intel_ddi_compute_config;
 	intel_encoder->enable = intel_enable_ddi;
-- 
2.13.6

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

[PATCH 3/3] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  10 +-
 drivers/gpu/drm/i915/intel_dp.c  | 196 ++++++++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h |   2 +
 3 files changed, 120 insertions(+), 88 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 25793bdc692f..5f3d58f1ae6e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2880,7 +2880,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);
@@ -3007,10 +3010,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_output_type = intel_ddi_compute_output_type;
 	intel_encoder->compute_config = intel_ddi_compute_config;
 	intel_encoder->enable = intel_enable_ddi;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6bbf14410c2a..152016e09a11 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4275,12 +4275,83 @@ 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);
@@ -4298,51 +4369,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 drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
-	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
-	struct drm_connector_state *conn_state =
-		intel_dp->attached_connector->base.state;
-	u8 link_status[DP_LINK_STATUS_SIZE];
-
-	WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));
-
-	if (!intel_dp_get_link_status(intel_dp, link_status)) {
-		DRM_ERROR("Failed to get link status\n");
-		return;
-	}
+	struct drm_modeset_acquire_ctx ctx;
+	bool changed;
+	int ret;
 
-	if (!conn_state->crtc)
-		return;
+	changed = intel_encoder_hotplug(encoder, connector);
 
-	WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
+	drm_modeset_acquire_init(&ctx, 0);
 
-	if (!conn_state->crtc->state->active)
-		return;
+	for (;;) {
+		ret = intel_dp_retrain_link(encoder, &ctx);
 
-	if (conn_state->commit &&
-	    !try_wait_for_completion(&conn_state->commit->hw_done))
-		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;
 }
 
 /*
@@ -4400,7 +4469,9 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
 			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
 	}
 
-	intel_dp_check_link_status(intel_dp);
+	/* 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");
@@ -4785,20 +4856,6 @@ intel_dp_long_pulse(struct intel_connector *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);
 	}
 
 	/*
@@ -5340,37 +5397,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	}
 
 	if (!intel_dp->is_mst) {
-		struct drm_modeset_acquire_ctx ctx;
-		struct drm_connector *connector = &intel_dp->attached_connector->base;
-		struct drm_crtc *crtc;
-		int iret;
-		bool handled = false;
-
-		drm_modeset_acquire_init(&ctx, 0);
-retry:
-		iret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex, &ctx);
-		if (iret)
-			goto err;
-
-		crtc = connector->state->crtc;
-		if (crtc) {
-			iret = drm_modeset_lock(&crtc->mutex, &ctx);
-			if (iret)
-				goto err;
-		}
+		bool handled;
 
 		handled = intel_dp_short_pulse(intel_dp);
 
-err:
-		if (iret == -EDEADLK) {
-			drm_modeset_backoff(&ctx);
-			goto retry;
-		}
-
-		drm_modeset_drop_locks(&ctx);
-		drm_modeset_acquire_fini(&ctx);
-		WARN(iret, "Acquiring modeset locks failed with %i\n", iret);
-
 		/* Short pulse can signify loss of hdcp authentication */
 		intel_hdcp_check_link(intel_dp->attached_connector);
 
@@ -6400,7 +6430,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 7537b2d542fd..1e657efeb6ea 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1611,6 +1611,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.13.6

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

✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
URL   : https://patchwork.freedesktop.org/series/36431/
State : failure

== Summary ==

Series 36431v1 series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
https://patchwork.freedesktop.org/api/1.0/series/36431/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575
Test kms_chamelium:
        Subgroup hdmi-hpd-fast:
                skip       -> FAIL       (fi-kbl-7500u) fdo#102672
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-after-cursor-atomic:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-after-cursor-legacy:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-after-cursor-varying-size:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-before-cursor-varying-size:
                pass       -> FAIL       (fi-skl-6770hq)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> INCOMPLETE (fi-hsw-4770) fdo#103375

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:418s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:434s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:488s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:489s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:485s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:468s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:455s
fi-elk-e7500     total:224  pass:168  dwarn:9   dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:108 time:276s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:515s
fi-hsw-4770      total:244  pass:220  dwarn:0   dfail:0   fail:0   skip:23 
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:400s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:412s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:468s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:413s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:1   skip:23  time:463s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:501s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:506s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:578s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:425s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:515s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:527s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:494s
fi-skl-6770hq    total:288  pass:260  dwarn:0   dfail:0   fail:8   skip:20  time:483s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:431s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:534s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:395s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:577s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:469s

fcf7fdfd2db6cfe8e529c79c5822f555a8b38fd4 drm-tip: 2018y-01m-12d-17h-40m-49s UTC integration manifest
7d0c0ca1a0de drm/i915: Move SST DP link retraining into the ->post_hotplug() hook
f092f454ab8a drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
d6641c359da5 drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook

== Logs ==

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

✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
URL   : https://patchwork.freedesktop.org/series/36431/
State : failure

== Summary ==

Series 36431v1 series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
https://patchwork.freedesktop.org/api/1.0/series/36431/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713
                pass       -> DMESG-WARN (fi-bdw-gvtdvm) fdo#103938 +1
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-after-cursor-atomic:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-after-cursor-legacy:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-after-cursor-varying-size:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-before-cursor-legacy:
                pass       -> FAIL       (fi-skl-6770hq)
        Subgroup basic-flip-before-cursor-varying-size:
                pass       -> FAIL       (fi-skl-6770hq)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-kbl-r) fdo#104172

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103938 https://bugs.freedesktop.org/show_bug.cgi?id=103938
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:419s
fi-bdw-gvtdvm    total:288  pass:262  dwarn:2   dfail:0   fail:0   skip:24  time:425s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:490s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:279s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:485s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:470s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:457s
fi-cnl-y2        total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:528s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:277s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:512s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:400s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:462s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:410s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:469s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:500s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:456s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:509s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:576s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:429s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:516s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:527s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:491s
fi-skl-6770hq    total:288  pass:260  dwarn:0   dfail:0   fail:8   skip:20  time:488s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:536s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:399s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:574s
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:469s

254125b984264731491e1eafbe58bc50e84a032d drm-tip: 2018y-01m-15d-09h-31m-31s UTC integration manifest
40530efe1001 drm/i915: Move SST DP link retraining into the ->post_hotplug() hook
54738b5b5101 drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
523afd823a2c drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook

== Logs ==

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

Re: [PATCH 1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook

[Jani Nikula]

On Fri, 12 Jan 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Nice!

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/intel_crt.c     |  4 +++-
>  drivers/gpu/drm/i915/intel_ddi.c     |  1 +
>  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 | 24 ++++++++++++------------
>  drivers/gpu/drm/i915/intel_sdvo.c    | 12 ++++++++++--
>  7 files changed, 32 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 9f31aea51dff..9bc47cff5409 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -966,8 +966,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 6260a882fbe4..1aeae3e97013 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2866,6 +2866,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));
>  
> +	intel_encoder->hotplug = intel_encoder_hotplug;
>  	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
>  	intel_encoder->compute_config = intel_ddi_compute_config;
>  	intel_encoder->enable = intel_enable_ddi;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 68229f53d5b8..6bbf14410c2a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6400,6 +6400,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 731dc36d7129..7537b2d542fd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -214,7 +214,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);
>  	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
>  						      struct intel_crtc_state *,
>  						      struct drm_connector_state *);
> @@ -1690,7 +1691,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 691f15b59124..4a93cfd7a28e 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2348,6 +2348,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..0191c7831a06 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -263,24 +263,25 @@ 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 +371,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);
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 2b8764897d68..5b1ad42ec446 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1692,7 +1692,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
> @@ -2496,7 +2504,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;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook

[Ville Syrjälä]

On Mon, Jan 15, 2018 at 11:51:19AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
> URL   : https://patchwork.freedesktop.org/series/36431/
> State : failure
> 
> == Summary ==
> 
> Series 36431v1 series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
> https://patchwork.freedesktop.org/api/1.0/series/36431/revisions/1/mbox/
> 
> Test debugfs_test:
>         Subgroup read_all_entries:
>                 incomplete -> PASS       (fi-snb-2520m) fdo#103713
>                 pass       -> DMESG-WARN (fi-bdw-gvtdvm) fdo#103938 +1
> Test kms_cursor_legacy:
>         Subgroup basic-busy-flip-before-cursor-atomic:
>                 pass       -> FAIL       (fi-skl-6770hq)
>         Subgroup basic-busy-flip-before-cursor-legacy:
>                 pass       -> FAIL       (fi-skl-6770hq)
>         Subgroup basic-flip-after-cursor-atomic:
>                 pass       -> FAIL       (fi-skl-6770hq)
>         Subgroup basic-flip-after-cursor-legacy:
>                 pass       -> FAIL       (fi-skl-6770hq)
>         Subgroup basic-flip-after-cursor-varying-size:
>                 pass       -> FAIL       (fi-skl-6770hq)
>         Subgroup basic-flip-before-cursor-atomic:
>                 pass       -> FAIL       (fi-skl-6770hq)
>         Subgroup basic-flip-before-cursor-legacy:
>                 pass       -> FAIL       (fi-skl-6770hq)
>         Subgroup basic-flip-before-cursor-varying-size:
>                 pass       -> FAIL       (fi-skl-6770hq)

This is LSPCON being silly. It throws a short HPD during the enable
sequence before link training. The short hpd handler then thinks
the link has fallen over and schedules the hotplug work to retrain
the link. The hotplug work will wait for the modeset to finish and
won't actually retrain the link needlessly though.

But all this is apparently sufficient amount of extra work to
throw the test off.

Not quite sure what to do about this. The whole point here was to make
the short hpd handler not take the modeset locks, so we'd need some
other way to deal with the concurrent modeset. I guess I could just
add some kind of 'bool link_trained;' type of thing to intel_dp to go
alongside the link params we already store there. But I've been hoping
we could eliminate this extra state being tracked in intel_dp. But
I can immediately think of anything better that would avoid the modeset
locks in the short pulse handler.

> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-a:
>                 pass       -> DMESG-WARN (fi-kbl-r) fdo#104172
> 
> fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
> fdo#103938 https://bugs.freedesktop.org/show_bug.cgi?id=103938
> fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172
> 
> fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:419s
> fi-bdw-gvtdvm    total:288  pass:262  dwarn:2   dfail:0   fail:0   skip:24  time:425s
> fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
> fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:490s
> fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:279s
> fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:485s
> fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:470s
> fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:457s
> fi-cnl-y2        total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:528s
> fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
> fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:277s
> fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:512s
> fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:400s
> fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:462s
> fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:410s
> fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:469s
> fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:500s
> fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:456s
> fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:509s
> fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:576s
> fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:429s
> fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:516s
> fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:527s
> fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:491s
> fi-skl-6770hq    total:288  pass:260  dwarn:0   dfail:0   fail:8   skip:20  time:488s
> fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:536s
> fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:399s
> Blacklisted hosts:
> fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:574s
> fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:469s
> 
> 254125b984264731491e1eafbe58bc50e84a032d drm-tip: 2018y-01m-15d-09h-31m-31s UTC integration manifest
> 40530efe1001 drm/i915: Move SST DP link retraining into the ->post_hotplug() hook
> 54738b5b5101 drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
> 523afd823a2c drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7669/issues.html

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

Re: [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD

[Sharma, Shashank]

Regards

Shashank


On 1/13/2018 2:34 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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)
>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c | 146 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 145 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 1aeae3e97013..25793bdc692f 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"
>   
> @@ -2756,6 +2757,146 @@ 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)
Should this function go to intel_atomic.c or similar ? Also a little 
documentation about
usage will be helpful for others, who want to reuse this.
> +{
> +	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;
> +
As this is an internal modeset trigger, should we send an event to 
userspace about this ?
> +	return 0;
> +
> + out:
one debug message here telling us about if modeset was success/fail ?
> +	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;
> +	}
We can export this read as helper in scdc_helper.c, something like 
drm_scdc_get_tmds_clock_ratio ?
- Shashank
> +
> +	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)
>   {
> @@ -2866,7 +3007,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));
>   
> -	intel_encoder->hotplug = intel_encoder_hotplug;
> +	if (init_hdmi)
> +		intel_encoder->hotplug = intel_ddi_hotplug;
> +	else
> +		intel_encoder->hotplug = intel_encoder_hotplug;
>   	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
>   	intel_encoder->compute_config = intel_ddi_compute_config;
>   	intel_encoder->enable = intel_enable_ddi;

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

Re: [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD

[Ville Syrjälä]

On Mon, Jan 22, 2018 at 12:07:28PM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 1/13/2018 2:34 AM, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > 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)
> >
> > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_ddi.c | 146 ++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 145 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 1aeae3e97013..25793bdc692f 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"
> >   
> > @@ -2756,6 +2757,146 @@ 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)
> Should this function go to intel_atomic.c or similar ?

Do you have another user for it? If not I don't see a particularly
good reason for moving it out.

> Also a little 
> documentation about
> usage will be helpful for others, who want to reuse this.

What should it say? To me the function name is pretty clear.

> > +{
> > +	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;
> > +
> As this is an internal modeset trigger, should we send an event to 
> userspace about this ?

What would userspace do with such an event?

> > +	return 0;
> > +
> > + out:
> one debug message here telling us about if modeset was success/fail ?

There's a WARN already higher up.

> > +	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;
> > +	}
> We can export this read as helper in scdc_helper.c, something like 
> drm_scdc_get_tmds_clock_ratio ?

Feels like fairly pointless abstraction to me. And we'd either need
to do two SCDC reads instead of one, or return two things from said
function.

> - Shashank
> > +
> > +	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)
> >   {
> > @@ -2866,7 +3007,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));
> >   
> > -	intel_encoder->hotplug = intel_encoder_hotplug;
> > +	if (init_hdmi)
> > +		intel_encoder->hotplug = intel_ddi_hotplug;
> > +	else
> > +		intel_encoder->hotplug = intel_encoder_hotplug;
> >   	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
> >   	intel_encoder->compute_config = intel_ddi_compute_config;
> >   	intel_encoder->enable = intel_enable_ddi;

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