* [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
2018-01-12 21:04 ` [PATCH 3/3] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook Ville Syrjala
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ messages in thread
* [PATCH 3/3] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook
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-12 21:04 ` Ville Syrjala
2018-01-12 21:27 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook Patchwork
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ 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>
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
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-12 21:04 ` [PATCH 3/3] drm/i915: Move SST DP link retraining into the ->post_hotplug() hook Ville Syrjala
@ 2018-01-12 21:27 ` Patchwork
2018-01-15 11:51 ` Patchwork
2018-01-16 10:40 ` [PATCH 1/3] " Jani Nikula
4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-01-12 21:27 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 9+ messages in thread* ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
2018-01-12 21:04 [PATCH 1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook Ville Syrjala
` (2 preceding siblings ...)
2018-01-12 21:27 ` ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook Patchwork
@ 2018-01-15 11:51 ` Patchwork
2018-01-17 17:11 ` Ville Syrjälä
2018-01-16 10:40 ` [PATCH 1/3] " Jani Nikula
4 siblings, 1 reply; 9+ messages in thread
From: Patchwork @ 2018-01-15 11:51 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
2018-01-15 11:51 ` Patchwork
@ 2018-01-17 17:11 ` Ville Syrjälä
0 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2018-01-17 17:11 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook
2018-01-12 21:04 [PATCH 1/3] drm/i915: Convert intel_hpd_irq_event() into an encoder hotplug hook Ville Syrjala
` (3 preceding siblings ...)
2018-01-15 11:51 ` Patchwork
@ 2018-01-16 10:40 ` Jani Nikula
4 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2018-01-16 10:40 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread