* [PATCH 1/3] drm/i915: Split encoder->hot_plug() into pre and post variants
@ 2017-12-22 18:28 Ville Syrjala
2017-12-22 18:28 ` [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD Ville Syrjala
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Ville Syrjala @ 2017-12-22 18:28 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
During hpd processing we may want to do things both before and
after the display detection. To that end split the encoder->hot_plug()
hooks.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_hotplug.c | 6 ++++--
drivers/gpu/drm/i915/intel_sdvo.c | 2 +-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 30f791f89d64..4a7e603ccc38 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -211,7 +211,8 @@ struct intel_encoder {
enum intel_output_type type;
enum port port;
unsigned int cloneable;
- void (*hot_plug)(struct intel_encoder *);
+ void (*pre_hotplug)(struct intel_encoder *encoder);
+ void (*post_hotplug)(struct intel_encoder *encoder);
enum intel_output_type (*compute_output_type)(struct intel_encoder *,
struct intel_crtc_state *,
struct drm_connector_state *);
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 875d5d218d5c..91abca3ae11a 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -370,10 +370,12 @@ 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_encoder->pre_hotplug)
+ intel_encoder->pre_hotplug(intel_encoder);
if (intel_hpd_irq_event(dev, connector))
changed = true;
+ if (intel_encoder->post_hotplug)
+ intel_encoder->post_hotplug(intel_encoder);
}
}
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..6de90a1fc553 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2496,7 +2496,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->pre_hotplug = intel_sdvo_enable_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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
2017-12-22 18:28 [PATCH 1/3] drm/i915: Split encoder->hot_plug() into pre and post variants Ville Syrjala
@ 2017-12-22 18:28 ` Ville Syrjala
2017-12-28 15:02 ` Sharma, Shashank
2017-12-22 18:28 ` [PATCH 3/3] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook Ville Syrjala
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjala @ 2017-12-22 18:28 UTC (permalink / raw)
To: intel-gfx
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->post_hotplug() hook.
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 | 75 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f51645a08dca..12da7024f01a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2720,6 +2720,79 @@ intel_ddi_init_dp_connector(struct intel_digital_port *intel_dig_port)
return connector;
}
+static int intel_ddi_hdmi_reset_scrambling(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 drm_connector_state *conn_state;
+ struct intel_crtc_state *crtc_state;
+ struct intel_crtc *crtc;
+ 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;
+
+ intel_hdmi_handle_sink_scrambling(encoder, &connector->base,
+ crtc_state->hdmi_high_tmds_clock_ratio,
+ crtc_state->hdmi_scrambling);
+
+ return 0;
+}
+
+static void intel_ddi_post_hotplug(struct intel_encoder *encoder)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ for (;;) {
+ ret = intel_ddi_hdmi_reset_scrambling(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);
+}
+
static struct intel_connector *
intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port)
{
@@ -2830,6 +2903,8 @@ 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->post_hotplug = intel_ddi_post_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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook
2017-12-22 18:28 [PATCH 1/3] drm/i915: Split encoder->hot_plug() into pre and post variants Ville Syrjala
2017-12-22 18:28 ` [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD Ville Syrjala
@ 2017-12-22 18:28 ` Ville Syrjala
2017-12-22 22:07 ` Manasi Navare
2017-12-22 19:52 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Split encoder->hot_plug() into pre and post variants Patchwork
2017-12-22 21:07 ` [PATCH 1/3] " Manasi Navare
3 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjala @ 2017-12-22 18:28 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
Doing link reatrining 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->post_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.
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 | 7 +-
drivers/gpu/drm/i915/intel_dp.c | 213 ++++++++++++++++++++++-----------------
drivers/gpu/drm/i915/intel_drv.h | 2 +
3 files changed, 125 insertions(+), 97 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 12da7024f01a..7e9964794efe 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2778,7 +2778,9 @@ static void intel_ddi_post_hotplug(struct intel_encoder *encoder)
drm_modeset_acquire_init(&ctx, 0);
for (;;) {
- ret = intel_ddi_hdmi_reset_scrambling(encoder, &ctx);
+ ret = intel_dp_retrain_link(encoder, &ctx);
+ if (!ret)
+ ret = intel_ddi_hdmi_reset_scrambling(encoder, &ctx);
if (ret == -EDEADLK) {
drm_modeset_backoff(&ctx);
@@ -2903,8 +2905,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->post_hotplug = intel_ddi_post_hotplug;
+ intel_encoder->post_hotplug = intel_ddi_post_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 35c5299feab6..72234c5dc15b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4251,74 +4251,137 @@ 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)
{
- 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);
-
- /* Suppress underruns caused by re-training */
- intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
- if (crtc->config->has_pch_encoder)
- intel_set_pch_fifo_underrun_reporting(dev_priv,
- intel_crtc_pch_transcoder(crtc), false);
-
- intel_dp_start_link_train(intel_dp);
- intel_dp_stop_link_train(intel_dp);
-
- /* Keep underrun reporting disabled until things are stable */
- intel_wait_for_vblank(dev_priv, crtc->pipe);
-
- intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true);
- if (crtc->config->has_pch_encoder)
- intel_set_pch_fifo_underrun_reporting(dev_priv,
- intel_crtc_pch_transcoder(crtc), true);
-}
-
-static void
-intel_dp_check_link_status(struct intel_dp *intel_dp)
-{
- 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;
+ return false;
}
- if (!conn_state->crtc)
- return;
-
- WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
-
- if (!conn_state->crtc->state->active)
- return;
-
- if (conn_state->commit &&
- !try_wait_for_completion(&conn_state->commit->hw_done))
- return;
-
/*
* 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;
+ return false;
/* 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);
+ return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
+}
- intel_dp_retrain_link(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.
+ */
+int intel_dp_retrain_link(struct intel_encoder *encoder,
+ struct drm_modeset_acquire_ctx *ctx)
+{
+ struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ 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);
+ if (crtc->config->has_pch_encoder)
+ intel_set_pch_fifo_underrun_reporting(dev_priv,
+ intel_crtc_pch_transcoder(crtc), false);
+
+ intel_dp_start_link_train(intel_dp);
+ intel_dp_stop_link_train(intel_dp);
+
+ /* Keep underrun reporting disabled until things are stable */
+ intel_wait_for_vblank(dev_priv, crtc->pipe);
+
+ intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true);
+ if (crtc->config->has_pch_encoder)
+ intel_set_pch_fifo_underrun_reporting(dev_priv,
+ intel_crtc_pch_transcoder(crtc), true);
+
+ return 0;
+}
+
+/*
+ * 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 void intel_dp_post_hotplug(struct intel_encoder *encoder)
+{
+ struct drm_modeset_acquire_ctx ctx;
+ int ret;
+
+ drm_modeset_acquire_init(&ctx, 0);
+
+ for (;;) {
+ ret = intel_dp_retrain_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);
}
/*
@@ -4376,7 +4439,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");
@@ -4761,20 +4826,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);
}
/*
@@ -5119,37 +5170,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);
-
if (!handled) {
intel_dp->detect_done = false;
goto put_power;
@@ -6170,6 +6194,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
"DP %c", port_name(port)))
goto err_encoder_init;
+ intel_encoder->post_hotplug = intel_dp_post_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 4a7e603ccc38..f95ac56784e6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1530,6 +1530,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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Split encoder->hot_plug() into pre and post variants
2017-12-22 18:28 [PATCH 1/3] drm/i915: Split encoder->hot_plug() into pre and post variants Ville Syrjala
2017-12-22 18:28 ` [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD Ville Syrjala
2017-12-22 18:28 ` [PATCH 3/3] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook Ville Syrjala
@ 2017-12-22 19:52 ` Patchwork
2017-12-22 21:07 ` [PATCH 1/3] " Manasi Navare
3 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-12-22 19:52 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/3] drm/i915: Split encoder->hot_plug() into pre and post variants
URL : https://patchwork.freedesktop.org/series/35732/
State : failure
== Summary ==
Series 35732v1 series starting with [1/3] drm/i915: Split encoder->hot_plug() into pre and post variants
https://patchwork.freedesktop.org/api/1.0/series/35732/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:
pass -> FAIL (fi-gdg-551) fdo#102575
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)
fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:436s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:443s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:383s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:493s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:275s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:497s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:493s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:477s
fi-elk-e7500 total:224 pass:163 dwarn:14 dfail:1 fail:0 skip:45
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:261s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:534s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:406s
fi-hsw-4770r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:412s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:425s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:476s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:427s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:479s
fi-kbl-7560u total:288 pass:268 dwarn:1 dfail:0 fail:0 skip:19 time:526s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:465s
fi-kbl-r total:288 pass:260 dwarn:1 dfail:0 fail:0 skip:27 time:522s
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:441s
fi-skl-6600u total:288 pass:260 dwarn:1 dfail:0 fail:0 skip:27 time:532s
fi-skl-6700hq total:288 pass:261 dwarn:1 dfail:0 fail:0 skip:26 time:555s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:506s
fi-skl-6770hq total:288 pass:260 dwarn:0 dfail:0 fail:8 skip:20 time:508s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:437s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:544s
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:420s
Blacklisted hosts:
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:591s
fi-cnl-y total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:625s
fi-byt-n2820 failed to collect. IGT log at Patchwork_7572/fi-byt-n2820/igt.log
fi-glk-dsi failed to collect. IGT log at Patchwork_7572/fi-glk-dsi/igt.log
b58679136779d486648809274b64d89fdafc79e7 drm-tip: 2017y-12m-22d-19h-04m-50s UTC integration manifest
b23451fe8d0f drm/i915: Move SST DP link retraining into the ->post_hotplug() hook
3f84c2d1f724 drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
460f4e0da62e drm/i915: Split encoder->hot_plug() into pre and post variants
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7572/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] drm/i915: Split encoder->hot_plug() into pre and post variants
2017-12-22 18:28 [PATCH 1/3] drm/i915: Split encoder->hot_plug() into pre and post variants Ville Syrjala
` (2 preceding siblings ...)
2017-12-22 19:52 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Split encoder->hot_plug() into pre and post variants Patchwork
@ 2017-12-22 21:07 ` Manasi Navare
3 siblings, 0 replies; 14+ messages in thread
From: Manasi Navare @ 2017-12-22 21:07 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
On Fri, Dec 22, 2017 at 08:28:55PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> During hpd processing we may want to do things both before and
> after the display detection. To that end split the encoder->hot_plug()
> hooks.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> drivers/gpu/drm/i915/intel_hotplug.c | 6 ++++--
> drivers/gpu/drm/i915/intel_sdvo.c | 2 +-
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 30f791f89d64..4a7e603ccc38 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -211,7 +211,8 @@ struct intel_encoder {
> enum intel_output_type type;
> enum port port;
> unsigned int cloneable;
> - void (*hot_plug)(struct intel_encoder *);
> + void (*pre_hotplug)(struct intel_encoder *encoder);
> + void (*post_hotplug)(struct intel_encoder *encoder);
Splitting into pre and post detect for the hotplug makes sense.
Although to be more intuitive of the order of things that happen on hotplug
the names could be hotplug_pre_detect() and hotplug_post_detect() or something
on those lines relative to the detect() call.
Looks good other than that.
Manasi
> enum intel_output_type (*compute_output_type)(struct intel_encoder *,
> struct intel_crtc_state *,
> struct drm_connector_state *);
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 875d5d218d5c..91abca3ae11a 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -370,10 +370,12 @@ 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_encoder->pre_hotplug)
> + intel_encoder->pre_hotplug(intel_encoder);
> if (intel_hpd_irq_event(dev, connector))
> changed = true;
> + if (intel_encoder->post_hotplug)
> + intel_encoder->post_hotplug(intel_encoder);
> }
> }
> 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..6de90a1fc553 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2496,7 +2496,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->pre_hotplug = intel_sdvo_enable_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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook
2017-12-22 18:28 ` [PATCH 3/3] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook Ville Syrjala
@ 2017-12-22 22:07 ` Manasi Navare
0 siblings, 0 replies; 14+ messages in thread
From: Manasi Navare @ 2017-12-22 22:07 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
On Fri, Dec 22, 2017 at 08:28:57PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Doing link reatrining 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->post_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.
>
> 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 | 7 +-
> drivers/gpu/drm/i915/intel_dp.c | 213 ++++++++++++++++++++++-----------------
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 3 files changed, 125 insertions(+), 97 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 12da7024f01a..7e9964794efe 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2778,7 +2778,9 @@ static void intel_ddi_post_hotplug(struct intel_encoder *encoder)
> drm_modeset_acquire_init(&ctx, 0);
>
> for (;;) {
> - ret = intel_ddi_hdmi_reset_scrambling(encoder, &ctx);
> + ret = intel_dp_retrain_link(encoder, &ctx);
> + if (!ret)
> + ret = intel_ddi_hdmi_reset_scrambling(encoder, &ctx);
>
> if (ret == -EDEADLK) {
> drm_modeset_backoff(&ctx);
> @@ -2903,8 +2905,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->post_hotplug = intel_ddi_post_hotplug;
> + intel_encoder->post_hotplug = intel_ddi_post_hotplug;
Same goes here for the name, may be call it intel_ddi_hotplug_post_detect()?
> 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 35c5299feab6..72234c5dc15b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4251,74 +4251,137 @@ 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)
> {
> - 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);
> -
> - /* Suppress underruns caused by re-training */
> - intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false);
> - if (crtc->config->has_pch_encoder)
> - intel_set_pch_fifo_underrun_reporting(dev_priv,
> - intel_crtc_pch_transcoder(crtc), false);
> -
> - intel_dp_start_link_train(intel_dp);
> - intel_dp_stop_link_train(intel_dp);
> -
> - /* Keep underrun reporting disabled until things are stable */
> - intel_wait_for_vblank(dev_priv, crtc->pipe);
> -
> - intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true);
> - if (crtc->config->has_pch_encoder)
> - intel_set_pch_fifo_underrun_reporting(dev_priv,
> - intel_crtc_pch_transcoder(crtc), true);
> -}
> -
> -static void
> -intel_dp_check_link_status(struct intel_dp *intel_dp)
> -{
> - 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;
> + return false;
> }
>
> - if (!conn_state->crtc)
> - return;
> -
> - WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex));
> -
> - if (!conn_state->crtc->state->active)
> - return;
> -
> - if (conn_state->commit &&
> - !try_wait_for_completion(&conn_state->commit->hw_done))
> - return;
> -
> /*
> * 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;
> + return false;
>
> /* 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);
> + return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
> +}
>
> - intel_dp_retrain_link(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.
> + */
> +int intel_dp_retrain_link(struct intel_encoder *encoder,
> + struct drm_modeset_acquire_ctx *ctx)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + 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);
> + if (crtc->config->has_pch_encoder)
> + intel_set_pch_fifo_underrun_reporting(dev_priv,
> + intel_crtc_pch_transcoder(crtc), false);
> +
> + intel_dp_start_link_train(intel_dp);
> + intel_dp_stop_link_train(intel_dp);
> +
> + /* Keep underrun reporting disabled until things are stable */
> + intel_wait_for_vblank(dev_priv, crtc->pipe);
> +
> + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, true);
> + if (crtc->config->has_pch_encoder)
> + intel_set_pch_fifo_underrun_reporting(dev_priv,
> + intel_crtc_pch_transcoder(crtc), true);
> +
> + return 0;
> +}
> +
> +/*
> + * 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 void intel_dp_post_hotplug(struct intel_encoder *encoder)
> +{
> + struct drm_modeset_acquire_ctx ctx;
> + int ret;
> +
> + drm_modeset_acquire_init(&ctx, 0);
> +
> + for (;;) {
> + ret = intel_dp_retrain_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);
> }
>
I like that now intel_dp_long_pulse() and intel_dp_short_pulse() dont need to call
link retraining separately. This post_hotplug will take care of retraining after long and short
pulse.
Acked-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> /*
> @@ -4376,7 +4439,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");
> @@ -4761,20 +4826,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);
> }
>
> /*
> @@ -5119,37 +5170,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);
> -
> if (!handled) {
> intel_dp->detect_done = false;
> goto put_power;
> @@ -6170,6 +6194,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> "DP %c", port_name(port)))
> goto err_encoder_init;
>
> + intel_encoder->post_hotplug = intel_dp_post_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 4a7e603ccc38..f95ac56784e6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1530,6 +1530,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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
2017-12-22 18:28 ` [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD Ville Syrjala
@ 2017-12-28 15:02 ` Sharma, Shashank
2018-01-09 18:01 ` Ville Syrjälä
0 siblings, 1 reply; 14+ messages in thread
From: Sharma, Shashank @ 2017-12-28 15:02 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
On 12/22/2017 11:58 PM, 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->post_hotplug() hook.
I am not sure if I understood the problem statement correctly. Even if
the TV triggers HPD line while turning it back, I would expect:
- EDID read for TV's detection, which will refresh SCDC and scrambling
capabilities
- A new modeset will be triggered, which will program the scrambling and
high tmds clock ratio again
- Once HDMI controller is programmed, it will generate scrambled signals
till next modeset / disable.
So why do we need to do this ? I might be missing something, but lets
discus about it.
- 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 | 75 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f51645a08dca..12da7024f01a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2720,6 +2720,79 @@ intel_ddi_init_dp_connector(struct intel_digital_port *intel_dig_port)
> return connector;
> }
>
> +static int intel_ddi_hdmi_reset_scrambling(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 drm_connector_state *conn_state;
> + struct intel_crtc_state *crtc_state;
> + struct intel_crtc *crtc;
> + 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;
> +
> + intel_hdmi_handle_sink_scrambling(encoder, &connector->base,
> + crtc_state->hdmi_high_tmds_clock_ratio,
> + crtc_state->hdmi_scrambling);
> +
> + return 0;
> +}
> +
> +static void intel_ddi_post_hotplug(struct intel_encoder *encoder)
> +{
> + struct drm_modeset_acquire_ctx ctx;
> + int ret;
> +
> + drm_modeset_acquire_init(&ctx, 0);
> +
> + for (;;) {
> + ret = intel_ddi_hdmi_reset_scrambling(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);
> +}
> +
> static struct intel_connector *
> intel_ddi_init_hdmi_connector(struct intel_digital_port *intel_dig_port)
> {
> @@ -2830,6 +2903,8 @@ 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->post_hotplug = intel_ddi_post_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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
2017-12-28 15:02 ` Sharma, Shashank
@ 2018-01-09 18:01 ` Ville Syrjälä
2018-01-10 4:37 ` Sharma, Shashank
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2018-01-09 18:01 UTC (permalink / raw)
To: Sharma, Shashank; +Cc: intel-gfx
On Thu, Dec 28, 2017 at 08:32:05PM +0530, Sharma, Shashank wrote:
>
>
> On 12/22/2017 11:58 PM, 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->post_hotplug() hook.
> I am not sure if I understood the problem statement correctly. Even if
> the TV triggers HPD line while turning it back, I would expect:
> - EDID read for TV's detection, which will refresh SCDC and scrambling
> capabilities
> - A new modeset will be triggered, which will program the scrambling and
> high tmds clock ratio again
> - Once HDMI controller is programmed, it will generate scrambled signals
> till next modeset / disable.
>
> So why do we need to do this ? I might be missing something, but lets
> discus about it.
The EDID is readable even when the HPD gets deasserted for a short
perios, hence we never consider the sink as being disconnected. Hence
there will be no modeset triggered by userspace.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
2018-01-09 18:01 ` Ville Syrjälä
@ 2018-01-10 4:37 ` Sharma, Shashank
2018-01-10 16:23 ` Ville Syrjälä
0 siblings, 1 reply; 14+ messages in thread
From: Sharma, Shashank @ 2018-01-10 4:37 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Regards
Shashank
On 1/9/2018 11:31 PM, Ville Syrjälä wrote:
> On Thu, Dec 28, 2017 at 08:32:05PM +0530, Sharma, Shashank wrote:
>>
>> On 12/22/2017 11:58 PM, 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->post_hotplug() hook.
>> I am not sure if I understood the problem statement correctly. Even if
>> the TV triggers HPD line while turning it back, I would expect:
>> - EDID read for TV's detection, which will refresh SCDC and scrambling
>> capabilities
>> - A new modeset will be triggered, which will program the scrambling and
>> high tmds clock ratio again
>> - Once HDMI controller is programmed, it will generate scrambled signals
>> till next modeset / disable.
>>
>> So why do we need to do this ? I might be missing something, but lets
>> discus about it.
> The EDID is readable even when the HPD gets deasserted for a short
> perios, hence we never consider the sink as being disconnected. Hence
> there will be no modeset triggered by userspace.
This is a bigger problem then, in that case, the pipe/port would be
enabled, and IMHO I don't think fixing just the scrambling status is
right thing to do.
Also, is this the right behavior from the monitor ? I am also worried if
we are trying to fix the monitor's fault in our driver.
- Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
2018-01-10 4:37 ` Sharma, Shashank
@ 2018-01-10 16:23 ` Ville Syrjälä
2018-01-11 13:16 ` Sharma, Shashank
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2018-01-10 16:23 UTC (permalink / raw)
To: Sharma, Shashank; +Cc: intel-gfx
On Wed, Jan 10, 2018 at 10:07:43AM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 1/9/2018 11:31 PM, Ville Syrjälä wrote:
> > On Thu, Dec 28, 2017 at 08:32:05PM +0530, Sharma, Shashank wrote:
> >>
> >> On 12/22/2017 11:58 PM, 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->post_hotplug() hook.
> >> I am not sure if I understood the problem statement correctly. Even if
> >> the TV triggers HPD line while turning it back, I would expect:
> >> - EDID read for TV's detection, which will refresh SCDC and scrambling
> >> capabilities
> >> - A new modeset will be triggered, which will program the scrambling and
> >> high tmds clock ratio again
> >> - Once HDMI controller is programmed, it will generate scrambled signals
> >> till next modeset / disable.
> >>
> >> So why do we need to do this ? I might be missing something, but lets
> >> discus about it.
> > The EDID is readable even when the HPD gets deasserted for a short
> > perios, hence we never consider the sink as being disconnected. Hence
> > there will be no modeset triggered by userspace.
> This is a bigger problem then, in that case, the pipe/port would be
> enabled, and IMHO I don't think fixing just the scrambling status is
> right thing to do.
Hmm. The spec does say "the Source shall not begin transmission of a
scrambled video signal before writing a 1 to the Scrambling_Enable bit".
So I guess you're right. We can't follow that rule 100% though because
we can't detect that the the sink has been turned off.
If we checked the live hpd status during hpd processing we should be
able to detect that the sink was logically disconnected for a short
period, but as we know the live hpd status is not exactly reliable
for HDMI. Also that would still be racy on account of hpd processing
delays.
When talking about the TMDS clock ratio the spec says that we have to
suspend TMDS clock/data transmission when we change the TMDS clock ratio
setting in the sink.
So I guess what we could do is force a full modeset if and when the sink
has become confused about these settings. Or if we want to optimize
things a bit I suppose we could just turn off DDI_BUF_CTL around the
operation. But probably no point in optimizing that part since it's a
rare event.
> Also, is this the right behavior from the monitor ? I am also worried if
> we are trying to fix the monitor's fault in our driver.
I don't think that's specified anywhere. Also doesn't matter because we
have to fix sink specific issues in the driver all the time. Otherwise
a lot of sinks would just fail to work.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
2018-01-10 16:23 ` Ville Syrjälä
@ 2018-01-11 13:16 ` Sharma, Shashank
0 siblings, 0 replies; 14+ messages in thread
From: Sharma, Shashank @ 2018-01-11 13:16 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
Regards
Shashank
On 1/10/2018 9:53 PM, Ville Syrjälä wrote:
> On Wed, Jan 10, 2018 at 10:07:43AM +0530, Sharma, Shashank wrote:
>> Regards
>>
>> Shashank
>>
>>
>> On 1/9/2018 11:31 PM, Ville Syrjälä wrote:
>>> On Thu, Dec 28, 2017 at 08:32:05PM +0530, Sharma, Shashank wrote:
>>>> On 12/22/2017 11:58 PM, 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->post_hotplug() hook.
>>>> I am not sure if I understood the problem statement correctly. Even if
>>>> the TV triggers HPD line while turning it back, I would expect:
>>>> - EDID read for TV's detection, which will refresh SCDC and scrambling
>>>> capabilities
>>>> - A new modeset will be triggered, which will program the scrambling and
>>>> high tmds clock ratio again
>>>> - Once HDMI controller is programmed, it will generate scrambled signals
>>>> till next modeset / disable.
>>>>
>>>> So why do we need to do this ? I might be missing something, but lets
>>>> discus about it.
>>> The EDID is readable even when the HPD gets deasserted for a short
>>> perios, hence we never consider the sink as being disconnected. Hence
>>> there will be no modeset triggered by userspace.
>> This is a bigger problem then, in that case, the pipe/port would be
>> enabled, and IMHO I don't think fixing just the scrambling status is
>> right thing to do.
> Hmm. The spec does say "the Source shall not begin transmission of a
> scrambled video signal before writing a 1 to the Scrambling_Enable bit".
> So I guess you're right. We can't follow that rule 100% though because
> we can't detect that the the sink has been turned off.
>
> If we checked the live hpd status during hpd processing we should be
> able to detect that the sink was logically disconnected for a short
> period, but as we know the live hpd status is not exactly reliable
> for HDMI. Also that would still be racy on account of hpd processing
> delays.
Agree. This inaccuracy of Live status HW has been a nightmare for us
since a long time,
it really cripples proper hotplug handling.
>
> When talking about the TMDS clock ratio the spec says that we have to
> suspend TMDS clock/data transmission when we change the TMDS clock ratio
> setting in the sink.
>
> So I guess what we could do is force a full modeset if and when the sink
> has become confused about these settings. Or if we want to optimize
> things a bit I suppose we could just turn off DDI_BUF_CTL around the
> operation. But probably no point in optimizing that part since it's a
> rare event.
Agree, again. Also it would be difficult to detect exactly when do we
have a genuine confusion :-)
- Shashank
>
>> Also, is this the right behavior from the monitor ? I am also worried if
>> we are trying to fix the monitor's fault in our driver.
> I don't think that's specified anywhere. Also doesn't matter because we
> have to fix sink specific issues in the driver all the time. Otherwise
> a lot of sinks would just fail to work.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
2018-01-22 6:37 ` Sharma, Shashank
@ 2018-01-22 19:15 ` Ville Syrjälä
0 siblings, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2018-01-22 19:15 UTC (permalink / raw)
To: Sharma, Shashank; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
2018-01-12 21:04 ` [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD Ville Syrjala
@ 2018-01-22 6:37 ` Sharma, Shashank
2018-01-22 19:15 ` Ville Syrjälä
0 siblings, 1 reply; 14+ messages in thread
From: Sharma, Shashank @ 2018-01-22 6:37 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD
2018-01-12 21:04 [PATCH 1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook Ville Syrjala
@ 2018-01-12 21:04 ` Ville Syrjala
2018-01-22 6:37 ` Sharma, Shashank
0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjala @ 2018-01-12 21:04 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-01-22 19:15 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-22 18:28 [PATCH 1/3] drm/i915: Split encoder->hot_plug() into pre and post variants Ville Syrjala
2017-12-22 18:28 ` [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD Ville Syrjala
2017-12-28 15:02 ` Sharma, Shashank
2018-01-09 18:01 ` Ville Syrjälä
2018-01-10 4:37 ` Sharma, Shashank
2018-01-10 16:23 ` Ville Syrjälä
2018-01-11 13:16 ` Sharma, Shashank
2017-12-22 18:28 ` [PATCH 3/3] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook Ville Syrjala
2017-12-22 22:07 ` Manasi Navare
2017-12-22 19:52 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Split encoder->hot_plug() into pre and post variants Patchwork
2017-12-22 21:07 ` [PATCH 1/3] " Manasi Navare
2018-01-12 21:04 [PATCH 1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook Ville Syrjala
2018-01-12 21:04 ` [PATCH 2/3] drm/i915: Reinitialize sink scrambling/TMDS clock ratio on HPD Ville Syrjala
2018-01-22 6:37 ` Sharma, Shashank
2018-01-22 19:15 ` Ville Syrjälä
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.