* [PATCH 1/4] drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link training
2016-06-16 13:37 [PATCH 0/4] drm/i915/bxt: Fix eDP link training after suspend Imre Deak
@ 2016-06-16 13:37 ` Imre Deak
2016-06-16 13:58 ` Ville Syrjälä
2016-06-16 13:37 ` [PATCH 2/4] drm/i915: Deduplicate PPS register retrieval Imre Deak
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Imre Deak @ 2016-06-16 13:37 UTC (permalink / raw)
To: intel-gfx
The PPS registers are backed by power well #0 and as such may be reset
after system or runtime suspend (both implying a possible DC9
transition). Fix this by reusing the VLV/CHV PPS pipe-reassignment
logic. The difference on BXT is that the PPS instances are not pipe but
port (or more accurately pin) specific, so we only need to care about
the lost HW state. As opposed to VLV/CHV the SW state is fixed and
initialized during connector init.
This also paves the way towards using the actual port->PPS instance
mapping based on VBT.
This fixes eDP link training errors on BXT after suspend, where we
started the link training too early due to an incorrect T3 (panel power
on) register value.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96436
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 71 +++++++++++++++++++++++----------
drivers/gpu/drm/i915/intel_drv.h | 7 +++-
drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-
3 files changed, 58 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index be08351..19a8bbe 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -426,6 +426,37 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
return intel_dp->pps_pipe;
}
+static int
+bxt_power_sequencer_idx(struct intel_dp *intel_dp)
+{
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ struct drm_device *dev = intel_dig_port->base.base.dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ lockdep_assert_held(&dev_priv->pps_mutex);
+
+ /* We should never land here with regular DP ports */
+ WARN_ON(!is_edp(intel_dp));
+
+ /*
+ * TODO: BXT has 2 PPS instances. The correct port->PPS instance
+ * mapping needs to be retrieved from VBT, for now just hard-code to
+ * use instance #0 always.
+ */
+ if (!intel_dp->pps_reset)
+ return 0;
+
+ intel_dp->pps_reset = false;
+
+ /*
+ * Only the HW needs to be reprogrammed, the SW state is fixed and
+ * has been setup during connector init.
+ */
+ intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
+
+ return 0;
+}
+
typedef bool (*vlv_pipe_check)(struct drm_i915_private *dev_priv,
enum pipe pipe);
@@ -507,12 +538,13 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
}
-void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
+void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = dev_priv->dev;
struct intel_encoder *encoder;
- if (WARN_ON(!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)))
+ if (WARN_ON(!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev) &&
+ !IS_BROXTON(dev)))
return;
/*
@@ -532,7 +564,10 @@ void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
continue;
intel_dp = enc_to_intel_dp(&encoder->base);
- intel_dp->pps_pipe = INVALID_PIPE;
+ if (IS_BROXTON(dev))
+ intel_dp->pps_reset = true;
+ else
+ intel_dp->pps_pipe = INVALID_PIPE;
}
}
@@ -542,7 +577,7 @@ _pp_ctrl_reg(struct intel_dp *intel_dp)
struct drm_device *dev = intel_dp_to_dev(intel_dp);
if (IS_BROXTON(dev))
- return BXT_PP_CONTROL(0);
+ return BXT_PP_CONTROL(bxt_power_sequencer_idx(intel_dp));
else if (HAS_PCH_SPLIT(dev))
return PCH_PP_CONTROL;
else
@@ -555,7 +590,7 @@ _pp_stat_reg(struct intel_dp *intel_dp)
struct drm_device *dev = intel_dp_to_dev(intel_dp);
if (IS_BROXTON(dev))
- return BXT_PP_STATUS(0);
+ return BXT_PP_STATUS(bxt_power_sequencer_idx(intel_dp));
else if (HAS_PCH_SPLIT(dev))
return PCH_PP_STATUS;
else
@@ -4722,14 +4757,11 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
return;
if (IS_BROXTON(dev)) {
- /*
- * TODO: BXT has 2 sets of PPS registers.
- * Correct Register for Broxton need to be identified
- * using VBT. hardcoding for now
- */
- pp_ctrl_reg = BXT_PP_CONTROL(0);
- pp_on_reg = BXT_PP_ON_DELAYS(0);
- pp_off_reg = BXT_PP_OFF_DELAYS(0);
+ int idx = bxt_power_sequencer_idx(intel_dp);
+
+ pp_ctrl_reg = BXT_PP_CONTROL(idx);
+ pp_on_reg = BXT_PP_ON_DELAYS(idx);
+ pp_off_reg = BXT_PP_OFF_DELAYS(idx);
} else if (HAS_PCH_SPLIT(dev)) {
pp_ctrl_reg = PCH_PP_CONTROL;
pp_on_reg = PCH_PP_ON_DELAYS;
@@ -4842,14 +4874,11 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
lockdep_assert_held(&dev_priv->pps_mutex);
if (IS_BROXTON(dev)) {
- /*
- * TODO: BXT has 2 sets of PPS registers.
- * Correct Register for Broxton need to be identified
- * using VBT. hardcoding for now
- */
- pp_ctrl_reg = BXT_PP_CONTROL(0);
- pp_on_reg = BXT_PP_ON_DELAYS(0);
- pp_off_reg = BXT_PP_OFF_DELAYS(0);
+ int idx = bxt_power_sequencer_idx(intel_dp);
+
+ pp_ctrl_reg = BXT_PP_CONTROL(idx);
+ pp_on_reg = BXT_PP_ON_DELAYS(idx);
+ pp_off_reg = BXT_PP_OFF_DELAYS(idx);
} else if (HAS_PCH_SPLIT(dev)) {
pp_on_reg = PCH_PP_ON_DELAYS;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8dc67ad..870849e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -869,6 +869,11 @@ struct intel_dp {
* this port. Only relevant on VLV/CHV.
*/
enum pipe pps_pipe;
+ /*
+ * Set if the sequencer may be reset due to a power transition,
+ * requiring a reinitialization. Only relevant on BXT.
+ */
+ bool pps_reset;
struct edp_power_seq pps_delays;
bool can_mst; /* this port supports mst */
@@ -1348,7 +1353,7 @@ void intel_dp_mst_resume(struct drm_device *dev);
int intel_dp_max_link_rate(struct intel_dp *intel_dp);
int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
-void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv);
+void intel_power_sequencer_reset(struct drm_i915_private *dev_priv);
uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes);
void intel_plane_destroy(struct drm_plane *plane);
void intel_edp_drrs_enable(struct intel_dp *intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index e856d49..22b46f5 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -578,6 +578,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
DRM_DEBUG_KMS("Enabling DC9\n");
+ intel_power_sequencer_reset(dev_priv);
gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
}
@@ -1112,7 +1113,7 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
/* make sure we're done processing display irqs */
synchronize_irq(dev_priv->dev->irq);
- vlv_power_sequencer_reset(dev_priv);
+ intel_power_sequencer_reset(dev_priv);
}
static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/4] drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link training
2016-06-16 13:37 ` [PATCH 1/4] drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link training Imre Deak
@ 2016-06-16 13:58 ` Ville Syrjälä
2016-06-16 14:39 ` Imre Deak
0 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2016-06-16 13:58 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
On Thu, Jun 16, 2016 at 04:37:20PM +0300, Imre Deak wrote:
> The PPS registers are backed by power well #0 and as such may be reset
> after system or runtime suspend (both implying a possible DC9
> transition). Fix this by reusing the VLV/CHV PPS pipe-reassignment
> logic. The difference on BXT is that the PPS instances are not pipe but
> port (or more accurately pin) specific, so we only need to care about
> the lost HW state. As opposed to VLV/CHV the SW state is fixed and
> initialized during connector init.
>
> This also paves the way towards using the actual port->PPS instance
> mapping based on VBT.
>
> This fixes eDP link training errors on BXT after suspend, where we
> started the link training too early due to an incorrect T3 (panel power
> on) register value.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96436
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 71 +++++++++++++++++++++++----------
> drivers/gpu/drm/i915/intel_drv.h | 7 +++-
> drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-
> 3 files changed, 58 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index be08351..19a8bbe 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -426,6 +426,37 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
> return intel_dp->pps_pipe;
> }
>
> +static int
> +bxt_power_sequencer_idx(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + lockdep_assert_held(&dev_priv->pps_mutex);
> +
> + /* We should never land here with regular DP ports */
> + WARN_ON(!is_edp(intel_dp));
> +
> + /*
> + * TODO: BXT has 2 PPS instances. The correct port->PPS instance
> + * mapping needs to be retrieved from VBT, for now just hard-code to
> + * use instance #0 always.
> + */
> + if (!intel_dp->pps_reset)
> + return 0;
> +
> + intel_dp->pps_reset = false;
> +
> + /*
> + * Only the HW needs to be reprogrammed, the SW state is fixed and
> + * has been setup during connector init.
> + */
> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> +
> + return 0;
> +}
> +
> typedef bool (*vlv_pipe_check)(struct drm_i915_private *dev_priv,
> enum pipe pipe);
>
> @@ -507,12 +538,13 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
> intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> }
>
> -void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
> +void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> {
> struct drm_device *dev = dev_priv->dev;
> struct intel_encoder *encoder;
>
> - if (WARN_ON(!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)))
> + if (WARN_ON(!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev) &&
> + !IS_BROXTON(dev)))
> return;
>
> /*
> @@ -532,7 +564,10 @@ void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
> continue;
>
> intel_dp = enc_to_intel_dp(&encoder->base);
> - intel_dp->pps_pipe = INVALID_PIPE;
> + if (IS_BROXTON(dev))
> + intel_dp->pps_reset = true;
> + else
> + intel_dp->pps_pipe = INVALID_PIPE;
> }
> }
>
> @@ -542,7 +577,7 @@ _pp_ctrl_reg(struct intel_dp *intel_dp)
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
>
> if (IS_BROXTON(dev))
> - return BXT_PP_CONTROL(0);
> + return BXT_PP_CONTROL(bxt_power_sequencer_idx(intel_dp));
> else if (HAS_PCH_SPLIT(dev))
> return PCH_PP_CONTROL;
> else
> @@ -555,7 +590,7 @@ _pp_stat_reg(struct intel_dp *intel_dp)
> struct drm_device *dev = intel_dp_to_dev(intel_dp);
>
> if (IS_BROXTON(dev))
> - return BXT_PP_STATUS(0);
> + return BXT_PP_STATUS(bxt_power_sequencer_idx(intel_dp));
> else if (HAS_PCH_SPLIT(dev))
> return PCH_PP_STATUS;
> else
> @@ -4722,14 +4757,11 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> return;
>
> if (IS_BROXTON(dev)) {
> - /*
> - * TODO: BXT has 2 sets of PPS registers.
> - * Correct Register for Broxton need to be identified
> - * using VBT. hardcoding for now
> - */
> - pp_ctrl_reg = BXT_PP_CONTROL(0);
> - pp_on_reg = BXT_PP_ON_DELAYS(0);
> - pp_off_reg = BXT_PP_OFF_DELAYS(0);
> + int idx = bxt_power_sequencer_idx(intel_dp);
> +
> + pp_ctrl_reg = BXT_PP_CONTROL(idx);
> + pp_on_reg = BXT_PP_ON_DELAYS(idx);
> + pp_off_reg = BXT_PP_OFF_DELAYS(idx);
> } else if (HAS_PCH_SPLIT(dev)) {
> pp_ctrl_reg = PCH_PP_CONTROL;
> pp_on_reg = PCH_PP_ON_DELAYS;
> @@ -4842,14 +4874,11 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> lockdep_assert_held(&dev_priv->pps_mutex);
>
> if (IS_BROXTON(dev)) {
> - /*
> - * TODO: BXT has 2 sets of PPS registers.
> - * Correct Register for Broxton need to be identified
> - * using VBT. hardcoding for now
> - */
> - pp_ctrl_reg = BXT_PP_CONTROL(0);
> - pp_on_reg = BXT_PP_ON_DELAYS(0);
> - pp_off_reg = BXT_PP_OFF_DELAYS(0);
> + int idx = bxt_power_sequencer_idx(intel_dp);
> +
> + pp_ctrl_reg = BXT_PP_CONTROL(idx);
> + pp_on_reg = BXT_PP_ON_DELAYS(idx);
> + pp_off_reg = BXT_PP_OFF_DELAYS(idx);
>
> } else if (HAS_PCH_SPLIT(dev)) {
> pp_on_reg = PCH_PP_ON_DELAYS;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8dc67ad..870849e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -869,6 +869,11 @@ struct intel_dp {
> * this port. Only relevant on VLV/CHV.
> */
> enum pipe pps_pipe;
> + /*
> + * Set if the sequencer may be reset due to a power transition,
> + * requiring a reinitialization. Only relevant on BXT.
> + */
> + bool pps_reset;
> struct edp_power_seq pps_delays;
>
> bool can_mst; /* this port supports mst */
> @@ -1348,7 +1353,7 @@ void intel_dp_mst_resume(struct drm_device *dev);
> int intel_dp_max_link_rate(struct intel_dp *intel_dp);
> int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
> void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
> -void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv);
> +void intel_power_sequencer_reset(struct drm_i915_private *dev_priv);
> uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes);
> void intel_plane_destroy(struct drm_plane *plane);
> void intel_edp_drrs_enable(struct intel_dp *intel_dp);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index e856d49..22b46f5 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -578,6 +578,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
>
> DRM_DEBUG_KMS("Enabling DC9\n");
>
> + intel_power_sequencer_reset(dev_priv);
> gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> }
I was wondering how we deal with coming out of S4 now, but I suppose we
currently enable DC9 in .freeze as well. When/if we remove that (to optimize
away the blinks during hibernation), I think we'll need something different
to deal with the unknown PPS state on .restore.
Are you going to do something about PCH platforms as well, or are you going
leave that for someone else?
>
> @@ -1112,7 +1113,7 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
> /* make sure we're done processing display irqs */
> synchronize_irq(dev_priv->dev->irq);
>
> - vlv_power_sequencer_reset(dev_priv);
> + intel_power_sequencer_reset(dev_priv);
> }
>
> static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> --
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] 15+ messages in thread* Re: [PATCH 1/4] drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link training
2016-06-16 13:58 ` Ville Syrjälä
@ 2016-06-16 14:39 ` Imre Deak
2016-06-16 14:48 ` Ville Syrjälä
0 siblings, 1 reply; 15+ messages in thread
From: Imre Deak @ 2016-06-16 14:39 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On to, 2016-06-16 at 16:58 +0300, Ville Syrjälä wrote:
> On Thu, Jun 16, 2016 at 04:37:20PM +0300, Imre Deak wrote:
> > The PPS registers are backed by power well #0 and as such may be reset
> > after system or runtime suspend (both implying a possible DC9
> > transition). Fix this by reusing the VLV/CHV PPS pipe-reassignment
> > logic. The difference on BXT is that the PPS instances are not pipe but
> > port (or more accurately pin) specific, so we only need to care about
> > the lost HW state. As opposed to VLV/CHV the SW state is fixed and
> > initialized during connector init.
> >
> > This also paves the way towards using the actual port->PPS instance
> > mapping based on VBT.
> >
> > This fixes eDP link training errors on BXT after suspend, where we
> > started the link training too early due to an incorrect T3 (panel power
> > on) register value.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96436
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 71 +++++++++++++++++++++++----------
> > drivers/gpu/drm/i915/intel_drv.h | 7 +++-
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-
> > 3 files changed, 58 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index be08351..19a8bbe 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -426,6 +426,37 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
> > return intel_dp->pps_pipe;
> > }
> >
> > +static int
> > +bxt_power_sequencer_idx(struct intel_dp *intel_dp)
> > +{
> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > + struct drm_device *dev = intel_dig_port->base.base.dev;
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > +
> > + lockdep_assert_held(&dev_priv->pps_mutex);
> > +
> > + /* We should never land here with regular DP ports */
> > + WARN_ON(!is_edp(intel_dp));
> > +
> > + /*
> > + * TODO: BXT has 2 PPS instances. The correct port->PPS instance
> > + * mapping needs to be retrieved from VBT, for now just hard-code to
> > + * use instance #0 always.
> > + */
> > + if (!intel_dp->pps_reset)
> > + return 0;
> > +
> > + intel_dp->pps_reset = false;
> > +
> > + /*
> > + * Only the HW needs to be reprogrammed, the SW state is fixed and
> > + * has been setup during connector init.
> > + */
> > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> > +
> > + return 0;
> > +}
> > +
> > typedef bool (*vlv_pipe_check)(struct drm_i915_private *dev_priv,
> > enum pipe pipe);
> >
> > @@ -507,12 +538,13 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
> > intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> > }
> >
> > -void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
> > +void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> > {
> > struct drm_device *dev = dev_priv->dev;
> > struct intel_encoder *encoder;
> >
> > - if (WARN_ON(!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)))
> > + if (WARN_ON(!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev) &&
> > + !IS_BROXTON(dev)))
> > return;
> >
> > /*
> > @@ -532,7 +564,10 @@ void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
> > continue;
> >
> > intel_dp = enc_to_intel_dp(&encoder->base);
> > - intel_dp->pps_pipe = INVALID_PIPE;
> > + if (IS_BROXTON(dev))
> > + intel_dp->pps_reset = true;
> > + else
> > + intel_dp->pps_pipe = INVALID_PIPE;
> > }
> > }
> >
> > @@ -542,7 +577,7 @@ _pp_ctrl_reg(struct intel_dp *intel_dp)
> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >
> > if (IS_BROXTON(dev))
> > - return BXT_PP_CONTROL(0);
> > + return BXT_PP_CONTROL(bxt_power_sequencer_idx(intel_dp));
> > else if (HAS_PCH_SPLIT(dev))
> > return PCH_PP_CONTROL;
> > else
> > @@ -555,7 +590,7 @@ _pp_stat_reg(struct intel_dp *intel_dp)
> > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> >
> > if (IS_BROXTON(dev))
> > - return BXT_PP_STATUS(0);
> > + return BXT_PP_STATUS(bxt_power_sequencer_idx(intel_dp));
> > else if (HAS_PCH_SPLIT(dev))
> > return PCH_PP_STATUS;
> > else
> > @@ -4722,14 +4757,11 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> > return;
> >
> > if (IS_BROXTON(dev)) {
> > - /*
> > - * TODO: BXT has 2 sets of PPS registers.
> > - * Correct Register for Broxton need to be identified
> > - * using VBT. hardcoding for now
> > - */
> > - pp_ctrl_reg = BXT_PP_CONTROL(0);
> > - pp_on_reg = BXT_PP_ON_DELAYS(0);
> > - pp_off_reg = BXT_PP_OFF_DELAYS(0);
> > + int idx = bxt_power_sequencer_idx(intel_dp);
> > +
> > + pp_ctrl_reg = BXT_PP_CONTROL(idx);
> > + pp_on_reg = BXT_PP_ON_DELAYS(idx);
> > + pp_off_reg = BXT_PP_OFF_DELAYS(idx);
> > } else if (HAS_PCH_SPLIT(dev)) {
> > pp_ctrl_reg = PCH_PP_CONTROL;
> > pp_on_reg = PCH_PP_ON_DELAYS;
> > @@ -4842,14 +4874,11 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> > lockdep_assert_held(&dev_priv->pps_mutex);
> >
> > if (IS_BROXTON(dev)) {
> > - /*
> > - * TODO: BXT has 2 sets of PPS registers.
> > - * Correct Register for Broxton need to be identified
> > - * using VBT. hardcoding for now
> > - */
> > - pp_ctrl_reg = BXT_PP_CONTROL(0);
> > - pp_on_reg = BXT_PP_ON_DELAYS(0);
> > - pp_off_reg = BXT_PP_OFF_DELAYS(0);
> > + int idx = bxt_power_sequencer_idx(intel_dp);
> > +
> > + pp_ctrl_reg = BXT_PP_CONTROL(idx);
> > + pp_on_reg = BXT_PP_ON_DELAYS(idx);
> > + pp_off_reg = BXT_PP_OFF_DELAYS(idx);
> >
> > } else if (HAS_PCH_SPLIT(dev)) {
> > pp_on_reg = PCH_PP_ON_DELAYS;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 8dc67ad..870849e 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -869,6 +869,11 @@ struct intel_dp {
> > * this port. Only relevant on VLV/CHV.
> > */
> > enum pipe pps_pipe;
> > + /*
> > + * Set if the sequencer may be reset due to a power transition,
> > + * requiring a reinitialization. Only relevant on BXT.
> > + */
> > + bool pps_reset;
> > struct edp_power_seq pps_delays;
> >
> > bool can_mst; /* this port supports mst */
> > @@ -1348,7 +1353,7 @@ void intel_dp_mst_resume(struct drm_device *dev);
> > int intel_dp_max_link_rate(struct intel_dp *intel_dp);
> > int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
> > void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
> > -void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv);
> > +void intel_power_sequencer_reset(struct drm_i915_private *dev_priv);
> > uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes);
> > void intel_plane_destroy(struct drm_plane *plane);
> > void intel_edp_drrs_enable(struct intel_dp *intel_dp);
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index e856d49..22b46f5 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -578,6 +578,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> >
> > DRM_DEBUG_KMS("Enabling DC9\n");
> >
> > + intel_power_sequencer_reset(dev_priv);
> > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> > }
>
> I was wondering how we deal with coming out of S4 now, but I suppose we
> currently enable DC9 in .freeze as well. When/if we remove that (to optimize
> away the blinks during hibernation), I think we'll need something different
> to deal with the unknown PPS state on .restore.
Hm yea, I assume we could reset the state in .restore then. But we'd
still need the DC9 time reset even in that case.
> Are you going to do something about PCH platforms as well, or are you going
> leave that for someone else?
I can take a look at that later if no one else does. AFAICS it would
amount to replacing the PPS save/restore in i915_suspend.c with a reset
during system suspend+register re-init based on pps_reset
in intel_pps_get_registers(). I'd have to also check how this would
affect LVDS.
> >
> > @@ -1112,7 +1113,7 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
> > /* make sure we're done processing display irqs */
> > synchronize_irq(dev_priv->dev->irq);
> >
> > - vlv_power_sequencer_reset(dev_priv);
> > + intel_power_sequencer_reset(dev_priv);
> > }
> >
> > static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> > --
> > 2.5.0
> >
> > _______________________________________________
> > 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] 15+ messages in thread* Re: [PATCH 1/4] drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link training
2016-06-16 14:39 ` Imre Deak
@ 2016-06-16 14:48 ` Ville Syrjälä
0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2016-06-16 14:48 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
On Thu, Jun 16, 2016 at 05:39:35PM +0300, Imre Deak wrote:
> On to, 2016-06-16 at 16:58 +0300, Ville Syrjälä wrote:
> > On Thu, Jun 16, 2016 at 04:37:20PM +0300, Imre Deak wrote:
> > > The PPS registers are backed by power well #0 and as such may be reset
> > > after system or runtime suspend (both implying a possible DC9
> > > transition). Fix this by reusing the VLV/CHV PPS pipe-reassignment
> > > logic. The difference on BXT is that the PPS instances are not pipe but
> > > port (or more accurately pin) specific, so we only need to care about
> > > the lost HW state. As opposed to VLV/CHV the SW state is fixed and
> > > initialized during connector init.
> > >
> > > This also paves the way towards using the actual port->PPS instance
> > > mapping based on VBT.
> > >
> > > This fixes eDP link training errors on BXT after suspend, where we
> > > started the link training too early due to an incorrect T3 (panel power
> > > on) register value.
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96436
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 71 +++++++++++++++++++++++----------
> > > drivers/gpu/drm/i915/intel_drv.h | 7 +++-
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-
> > > 3 files changed, 58 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index be08351..19a8bbe 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -426,6 +426,37 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
> > > return intel_dp->pps_pipe;
> > > }
> > >
> > > +static int
> > > +bxt_power_sequencer_idx(struct intel_dp *intel_dp)
> > > +{
> > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > + struct drm_device *dev = intel_dig_port->base.base.dev;
> > > + struct drm_i915_private *dev_priv = dev->dev_private;
> > > +
> > > + lockdep_assert_held(&dev_priv->pps_mutex);
> > > +
> > > + /* We should never land here with regular DP ports */
> > > + WARN_ON(!is_edp(intel_dp));
> > > +
> > > + /*
> > > + * TODO: BXT has 2 PPS instances. The correct port->PPS instance
> > > + * mapping needs to be retrieved from VBT, for now just hard-code to
> > > + * use instance #0 always.
> > > + */
> > > + if (!intel_dp->pps_reset)
> > > + return 0;
> > > +
> > > + intel_dp->pps_reset = false;
> > > +
> > > + /*
> > > + * Only the HW needs to be reprogrammed, the SW state is fixed and
> > > + * has been setup during connector init.
> > > + */
> > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > typedef bool (*vlv_pipe_check)(struct drm_i915_private *dev_priv,
> > > enum pipe pipe);
> > >
> > > @@ -507,12 +538,13 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
> > > intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
> > > }
> > >
> > > -void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
> > > +void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> > > {
> > > struct drm_device *dev = dev_priv->dev;
> > > struct intel_encoder *encoder;
> > >
> > > - if (WARN_ON(!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev)))
> > > + if (WARN_ON(!IS_VALLEYVIEW(dev) && !IS_CHERRYVIEW(dev) &&
> > > + !IS_BROXTON(dev)))
> > > return;
> > >
> > > /*
> > > @@ -532,7 +564,10 @@ void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
> > > continue;
> > >
> > > intel_dp = enc_to_intel_dp(&encoder->base);
> > > - intel_dp->pps_pipe = INVALID_PIPE;
> > > + if (IS_BROXTON(dev))
> > > + intel_dp->pps_reset = true;
> > > + else
> > > + intel_dp->pps_pipe = INVALID_PIPE;
> > > }
> > > }
> > >
> > > @@ -542,7 +577,7 @@ _pp_ctrl_reg(struct intel_dp *intel_dp)
> > > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > >
> > > if (IS_BROXTON(dev))
> > > - return BXT_PP_CONTROL(0);
> > > + return BXT_PP_CONTROL(bxt_power_sequencer_idx(intel_dp));
> > > else if (HAS_PCH_SPLIT(dev))
> > > return PCH_PP_CONTROL;
> > > else
> > > @@ -555,7 +590,7 @@ _pp_stat_reg(struct intel_dp *intel_dp)
> > > struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > >
> > > if (IS_BROXTON(dev))
> > > - return BXT_PP_STATUS(0);
> > > + return BXT_PP_STATUS(bxt_power_sequencer_idx(intel_dp));
> > > else if (HAS_PCH_SPLIT(dev))
> > > return PCH_PP_STATUS;
> > > else
> > > @@ -4722,14 +4757,11 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> > > return;
> > >
> > > if (IS_BROXTON(dev)) {
> > > - /*
> > > - * TODO: BXT has 2 sets of PPS registers.
> > > - * Correct Register for Broxton need to be identified
> > > - * using VBT. hardcoding for now
> > > - */
> > > - pp_ctrl_reg = BXT_PP_CONTROL(0);
> > > - pp_on_reg = BXT_PP_ON_DELAYS(0);
> > > - pp_off_reg = BXT_PP_OFF_DELAYS(0);
> > > + int idx = bxt_power_sequencer_idx(intel_dp);
> > > +
> > > + pp_ctrl_reg = BXT_PP_CONTROL(idx);
> > > + pp_on_reg = BXT_PP_ON_DELAYS(idx);
> > > + pp_off_reg = BXT_PP_OFF_DELAYS(idx);
> > > } else if (HAS_PCH_SPLIT(dev)) {
> > > pp_ctrl_reg = PCH_PP_CONTROL;
> > > pp_on_reg = PCH_PP_ON_DELAYS;
> > > @@ -4842,14 +4874,11 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
> > > lockdep_assert_held(&dev_priv->pps_mutex);
> > >
> > > if (IS_BROXTON(dev)) {
> > > - /*
> > > - * TODO: BXT has 2 sets of PPS registers.
> > > - * Correct Register for Broxton need to be identified
> > > - * using VBT. hardcoding for now
> > > - */
> > > - pp_ctrl_reg = BXT_PP_CONTROL(0);
> > > - pp_on_reg = BXT_PP_ON_DELAYS(0);
> > > - pp_off_reg = BXT_PP_OFF_DELAYS(0);
> > > + int idx = bxt_power_sequencer_idx(intel_dp);
> > > +
> > > + pp_ctrl_reg = BXT_PP_CONTROL(idx);
> > > + pp_on_reg = BXT_PP_ON_DELAYS(idx);
> > > + pp_off_reg = BXT_PP_OFF_DELAYS(idx);
> > >
> > > } else if (HAS_PCH_SPLIT(dev)) {
> > > pp_on_reg = PCH_PP_ON_DELAYS;
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 8dc67ad..870849e 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -869,6 +869,11 @@ struct intel_dp {
> > > * this port. Only relevant on VLV/CHV.
> > > */
> > > enum pipe pps_pipe;
> > > + /*
> > > + * Set if the sequencer may be reset due to a power transition,
> > > + * requiring a reinitialization. Only relevant on BXT.
> > > + */
> > > + bool pps_reset;
> > > struct edp_power_seq pps_delays;
> > >
> > > bool can_mst; /* this port supports mst */
> > > @@ -1348,7 +1353,7 @@ void intel_dp_mst_resume(struct drm_device *dev);
> > > int intel_dp_max_link_rate(struct intel_dp *intel_dp);
> > > int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
> > > void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
> > > -void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv);
> > > +void intel_power_sequencer_reset(struct drm_i915_private *dev_priv);
> > > uint32_t intel_dp_pack_aux(const uint8_t *src, int src_bytes);
> > > void intel_plane_destroy(struct drm_plane *plane);
> > > void intel_edp_drrs_enable(struct intel_dp *intel_dp);
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index e856d49..22b46f5 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -578,6 +578,7 @@ void bxt_enable_dc9(struct drm_i915_private *dev_priv)
> > >
> > > DRM_DEBUG_KMS("Enabling DC9\n");
> > >
> > > + intel_power_sequencer_reset(dev_priv);
> > > gen9_set_dc_state(dev_priv, DC_STATE_EN_DC9);
> > > }
> >
> > I was wondering how we deal with coming out of S4 now, but I suppose we
> > currently enable DC9 in .freeze as well. When/if we remove that (to optimize
> > away the blinks during hibernation), I think we'll need something different
> > to deal with the unknown PPS state on .restore.
>
> Hm yea, I assume we could reset the state in .restore then. But we'd
> still need the DC9 time reset even in that case.
Yeah.
>
> > Are you going to do something about PCH platforms as well, or are you going
> > leave that for someone else?
>
> I can take a look at that later if no one else does. AFAICS it would
> amount to replacing the PPS save/restore in i915_suspend.c with a reset
> during system suspend+register re-init based on pps_reset
> in intel_pps_get_registers(). I'd have to also check how this would
> affect LVDS.
I was thinking that might simply move the current save code to
LVDS init, and then add a .reset hook for LVDS to restore the
registers at resume time. But I msut admit that I didn't spend
much time thinking about it.
>
> > >
> > > @@ -1112,7 +1113,7 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
> > > /* make sure we're done processing display irqs */
> > > synchronize_irq(dev_priv->dev->irq);
> > >
> > > - vlv_power_sequencer_reset(dev_priv);
> > > + intel_power_sequencer_reset(dev_priv);
> > > }
> > >
> > > static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> > > --
> > > 2.5.0
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
--
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] 15+ messages in thread
* [PATCH 2/4] drm/i915: Deduplicate PPS register retrieval
2016-06-16 13:37 [PATCH 0/4] drm/i915/bxt: Fix eDP link training after suspend Imre Deak
2016-06-16 13:37 ` [PATCH 1/4] drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link training Imre Deak
@ 2016-06-16 13:37 ` Imre Deak
2016-06-16 13:37 ` [PATCH 3/4] drm/i915: Factor out helper to read out PPS HW state Imre Deak
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Imre Deak @ 2016-06-16 13:37 UTC (permalink / raw)
To: intel-gfx
No functional change.
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 131 ++++++++++++++++++++--------------------
1 file changed, 65 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 19a8bbe..3e875ff 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -571,30 +571,64 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
}
}
+struct pps_registers {
+ i915_reg_t pp_ctrl;
+ i915_reg_t pp_stat;
+ i915_reg_t pp_on;
+ i915_reg_t pp_off;
+ i915_reg_t pp_div;
+};
+
+static void intel_pps_get_registers(struct drm_i915_private *dev_priv,
+ struct intel_dp *intel_dp,
+ struct pps_registers *regs)
+{
+ memset(regs, 0, sizeof(*regs));
+
+ if (IS_BROXTON(dev_priv)) {
+ int idx = bxt_power_sequencer_idx(intel_dp);
+
+ regs->pp_ctrl = BXT_PP_CONTROL(idx);
+ regs->pp_stat = BXT_PP_STATUS(idx);
+ regs->pp_on = BXT_PP_ON_DELAYS(idx);
+ regs->pp_off = BXT_PP_OFF_DELAYS(idx);
+ } else if (HAS_PCH_SPLIT(dev_priv)) {
+ regs->pp_ctrl = PCH_PP_CONTROL;
+ regs->pp_stat = PCH_PP_STATUS;
+ regs->pp_on = PCH_PP_ON_DELAYS;
+ regs->pp_off = PCH_PP_OFF_DELAYS;
+ regs->pp_div = PCH_PP_DIVISOR;
+ } else {
+ enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+ regs->pp_ctrl = VLV_PIPE_PP_CONTROL(pipe);
+ regs->pp_stat = VLV_PIPE_PP_STATUS(pipe);
+ regs->pp_on = VLV_PIPE_PP_ON_DELAYS(pipe);
+ regs->pp_off = VLV_PIPE_PP_OFF_DELAYS(pipe);
+ regs->pp_div = VLV_PIPE_PP_DIVISOR(pipe);
+ }
+}
+
static i915_reg_t
_pp_ctrl_reg(struct intel_dp *intel_dp)
{
- struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct pps_registers regs;
- if (IS_BROXTON(dev))
- return BXT_PP_CONTROL(bxt_power_sequencer_idx(intel_dp));
- else if (HAS_PCH_SPLIT(dev))
- return PCH_PP_CONTROL;
- else
- return VLV_PIPE_PP_CONTROL(vlv_power_sequencer_pipe(intel_dp));
+ intel_pps_get_registers(to_i915(intel_dp_to_dev(intel_dp)), intel_dp,
+ ®s);
+
+ return regs.pp_ctrl;
}
static i915_reg_t
_pp_stat_reg(struct intel_dp *intel_dp)
{
- struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct pps_registers regs;
- if (IS_BROXTON(dev))
- return BXT_PP_STATUS(bxt_power_sequencer_idx(intel_dp));
- else if (HAS_PCH_SPLIT(dev))
- return PCH_PP_STATUS;
- else
- return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
+ intel_pps_get_registers(to_i915(intel_dp_to_dev(intel_dp)), intel_dp,
+ ®s);
+
+ return regs.pp_stat;
}
/* Reboot notifier handler to shutdown panel power to guarantee T12 timing
@@ -4748,7 +4782,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
struct edp_power_seq cur, vbt, spec,
*final = &intel_dp->pps_delays;
u32 pp_on, pp_off, pp_div = 0, pp_ctl = 0;
- i915_reg_t pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg;
+ struct pps_registers regs;
lockdep_assert_held(&dev_priv->pps_mutex);
@@ -4756,35 +4790,17 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
if (final->t11_t12 != 0)
return;
- if (IS_BROXTON(dev)) {
- int idx = bxt_power_sequencer_idx(intel_dp);
-
- pp_ctrl_reg = BXT_PP_CONTROL(idx);
- pp_on_reg = BXT_PP_ON_DELAYS(idx);
- pp_off_reg = BXT_PP_OFF_DELAYS(idx);
- } else if (HAS_PCH_SPLIT(dev)) {
- pp_ctrl_reg = PCH_PP_CONTROL;
- pp_on_reg = PCH_PP_ON_DELAYS;
- pp_off_reg = PCH_PP_OFF_DELAYS;
- pp_div_reg = PCH_PP_DIVISOR;
- } else {
- enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
-
- pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
- pp_on_reg = VLV_PIPE_PP_ON_DELAYS(pipe);
- pp_off_reg = VLV_PIPE_PP_OFF_DELAYS(pipe);
- pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
- }
+ intel_pps_get_registers(dev_priv, intel_dp, ®s);
/* Workaround: Need to write PP_CONTROL with the unlock key as
* the very first thing. */
pp_ctl = ironlake_get_pp_control(intel_dp);
- pp_on = I915_READ(pp_on_reg);
- pp_off = I915_READ(pp_off_reg);
+ pp_on = I915_READ(regs.pp_on);
+ pp_off = I915_READ(regs.pp_off);
if (!IS_BROXTON(dev)) {
- I915_WRITE(pp_ctrl_reg, pp_ctl);
- pp_div = I915_READ(pp_div_reg);
+ I915_WRITE(regs.pp_ctrl, pp_ctl);
+ pp_div = I915_READ(regs.pp_div);
}
/* Pull timing values out of registers */
@@ -4867,30 +4883,13 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
struct drm_i915_private *dev_priv = dev->dev_private;
u32 pp_on, pp_off, pp_div, port_sel = 0;
int div = dev_priv->rawclk_freq / 1000;
- i915_reg_t pp_on_reg, pp_off_reg, pp_div_reg, pp_ctrl_reg;
+ struct pps_registers regs;
enum port port = dp_to_dig_port(intel_dp)->port;
const struct edp_power_seq *seq = &intel_dp->pps_delays;
lockdep_assert_held(&dev_priv->pps_mutex);
- if (IS_BROXTON(dev)) {
- int idx = bxt_power_sequencer_idx(intel_dp);
-
- pp_ctrl_reg = BXT_PP_CONTROL(idx);
- pp_on_reg = BXT_PP_ON_DELAYS(idx);
- pp_off_reg = BXT_PP_OFF_DELAYS(idx);
-
- } else if (HAS_PCH_SPLIT(dev)) {
- pp_on_reg = PCH_PP_ON_DELAYS;
- pp_off_reg = PCH_PP_OFF_DELAYS;
- pp_div_reg = PCH_PP_DIVISOR;
- } else {
- enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
-
- pp_on_reg = VLV_PIPE_PP_ON_DELAYS(pipe);
- pp_off_reg = VLV_PIPE_PP_OFF_DELAYS(pipe);
- pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
- }
+ intel_pps_get_registers(dev_priv, intel_dp, ®s);
/*
* And finally store the new values in the power sequencer. The
@@ -4907,7 +4906,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
/* Compute the divisor for the pp clock, simply match the Bspec
* formula. */
if (IS_BROXTON(dev)) {
- pp_div = I915_READ(pp_ctrl_reg);
+ pp_div = I915_READ(regs.pp_ctrl);
pp_div &= ~BXT_POWER_CYCLE_DELAY_MASK;
pp_div |= (DIV_ROUND_UP((seq->t11_t12 + 1), 1000)
<< BXT_POWER_CYCLE_DELAY_SHIFT);
@@ -4930,19 +4929,19 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
pp_on |= port_sel;
- I915_WRITE(pp_on_reg, pp_on);
- I915_WRITE(pp_off_reg, pp_off);
+ I915_WRITE(regs.pp_on, pp_on);
+ I915_WRITE(regs.pp_off, pp_off);
if (IS_BROXTON(dev))
- I915_WRITE(pp_ctrl_reg, pp_div);
+ I915_WRITE(regs.pp_ctrl, pp_div);
else
- I915_WRITE(pp_div_reg, pp_div);
+ I915_WRITE(regs.pp_div, pp_div);
DRM_DEBUG_KMS("panel power sequencer register settings: PP_ON %#x, PP_OFF %#x, PP_DIV %#x\n",
- I915_READ(pp_on_reg),
- I915_READ(pp_off_reg),
+ I915_READ(regs.pp_on),
+ I915_READ(regs.pp_off),
IS_BROXTON(dev) ?
- (I915_READ(pp_ctrl_reg) & BXT_POWER_CYCLE_DELAY_MASK) :
- I915_READ(pp_div_reg));
+ (I915_READ(regs.pp_ctrl) & BXT_POWER_CYCLE_DELAY_MASK) :
+ I915_READ(regs.pp_div));
}
/**
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/4] drm/i915: Factor out helper to read out PPS HW state
2016-06-16 13:37 [PATCH 0/4] drm/i915/bxt: Fix eDP link training after suspend Imre Deak
2016-06-16 13:37 ` [PATCH 1/4] drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link training Imre Deak
2016-06-16 13:37 ` [PATCH 2/4] drm/i915: Deduplicate PPS register retrieval Imre Deak
@ 2016-06-16 13:37 ` Imre Deak
2016-06-16 13:37 ` [PATCH 4/4] drm/i915: Sanity check " Imre Deak
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Imre Deak @ 2016-06-16 13:37 UTC (permalink / raw)
To: intel-gfx
This will be needed by the next patch too so factor it out.
No functional change.
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 56 +++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3e875ff..caad825 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4775,21 +4775,12 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
}
static void
-intel_dp_init_panel_power_sequencer(struct drm_device *dev,
- struct intel_dp *intel_dp)
+intel_pps_readout_hw_state(struct drm_i915_private *dev_priv,
+ struct intel_dp *intel_dp, struct edp_power_seq *seq)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct edp_power_seq cur, vbt, spec,
- *final = &intel_dp->pps_delays;
u32 pp_on, pp_off, pp_div = 0, pp_ctl = 0;
struct pps_registers regs;
- lockdep_assert_held(&dev_priv->pps_mutex);
-
- /* already initialized? */
- if (final->t11_t12 != 0)
- return;
-
intel_pps_get_registers(dev_priv, intel_dp, ®s);
/* Workaround: Need to write PP_CONTROL with the unlock key as
@@ -4798,35 +4789,52 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
pp_on = I915_READ(regs.pp_on);
pp_off = I915_READ(regs.pp_off);
- if (!IS_BROXTON(dev)) {
+ if (!IS_BROXTON(dev_priv)) {
I915_WRITE(regs.pp_ctrl, pp_ctl);
pp_div = I915_READ(regs.pp_div);
}
/* Pull timing values out of registers */
- cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
- PANEL_POWER_UP_DELAY_SHIFT;
+ seq->t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
+ PANEL_POWER_UP_DELAY_SHIFT;
- cur.t8 = (pp_on & PANEL_LIGHT_ON_DELAY_MASK) >>
- PANEL_LIGHT_ON_DELAY_SHIFT;
+ seq->t8 = (pp_on & PANEL_LIGHT_ON_DELAY_MASK) >>
+ PANEL_LIGHT_ON_DELAY_SHIFT;
- cur.t9 = (pp_off & PANEL_LIGHT_OFF_DELAY_MASK) >>
- PANEL_LIGHT_OFF_DELAY_SHIFT;
+ seq->t9 = (pp_off & PANEL_LIGHT_OFF_DELAY_MASK) >>
+ PANEL_LIGHT_OFF_DELAY_SHIFT;
- cur.t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >>
- PANEL_POWER_DOWN_DELAY_SHIFT;
+ seq->t10 = (pp_off & PANEL_POWER_DOWN_DELAY_MASK) >>
+ PANEL_POWER_DOWN_DELAY_SHIFT;
- if (IS_BROXTON(dev)) {
+ if (IS_BROXTON(dev_priv)) {
u16 tmp = (pp_ctl & BXT_POWER_CYCLE_DELAY_MASK) >>
BXT_POWER_CYCLE_DELAY_SHIFT;
if (tmp > 0)
- cur.t11_t12 = (tmp - 1) * 1000;
+ seq->t11_t12 = (tmp - 1) * 1000;
else
- cur.t11_t12 = 0;
+ seq->t11_t12 = 0;
} else {
- cur.t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
+ seq->t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
}
+}
+
+static void
+intel_dp_init_panel_power_sequencer(struct drm_device *dev,
+ struct intel_dp *intel_dp)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct edp_power_seq cur, vbt, spec,
+ *final = &intel_dp->pps_delays;
+
+ lockdep_assert_held(&dev_priv->pps_mutex);
+
+ /* already initialized? */
+ if (final->t11_t12 != 0)
+ return;
+
+ intel_pps_readout_hw_state(dev_priv, intel_dp, &cur);
DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/4] drm/i915: Sanity check PPS HW state
2016-06-16 13:37 [PATCH 0/4] drm/i915/bxt: Fix eDP link training after suspend Imre Deak
` (2 preceding siblings ...)
2016-06-16 13:37 ` [PATCH 3/4] drm/i915: Factor out helper to read out PPS HW state Imre Deak
@ 2016-06-16 13:37 ` Imre Deak
2016-06-16 14:01 ` Ville Syrjälä
2016-06-16 17:01 ` [PATCH v2 " Imre Deak
2016-06-16 13:56 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix eDP link training after suspend Patchwork
` (2 subsequent siblings)
6 siblings, 2 replies; 15+ messages in thread
From: Imre Deak @ 2016-06-16 13:37 UTC (permalink / raw)
To: intel-gfx
The wait for panel status helper will only function correctly if the
HW panel timings are programmed correctly. Returning prematurely from
this helper may lead to obscure bugs later, so sanity check the HW
timing registers.
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 40 ++++++++++++++++++++++++++++++++++++----
1 file changed, 36 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index caad825..163dcad 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1772,6 +1772,9 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
#define IDLE_CYCLE_MASK (PP_ON | PP_SEQUENCE_MASK | PP_CYCLE_DELAY_ACTIVE | PP_SEQUENCE_STATE_MASK)
#define IDLE_CYCLE_VALUE (0 | PP_SEQUENCE_NONE | 0 | PP_SEQUENCE_STATE_OFF_IDLE)
+static void intel_pps_verify_state(struct drm_i915_private *dev_priv,
+ struct intel_dp *intel_dp);
+
static void wait_panel_status(struct intel_dp *intel_dp,
u32 mask,
u32 value)
@@ -1782,6 +1785,8 @@ static void wait_panel_status(struct intel_dp *intel_dp,
lockdep_assert_held(&dev_priv->pps_mutex);
+ intel_pps_verify_state(dev_priv, intel_dp);
+
pp_stat_reg = _pp_stat_reg(intel_dp);
pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
@@ -4821,6 +4826,35 @@ intel_pps_readout_hw_state(struct drm_i915_private *dev_priv,
}
static void
+intel_pps_dump_state(const char *state_name, const struct edp_power_seq *seq)
+{
+ DRM_DEBUG_KMS("%s t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
+ state_name,
+ seq->t1_t3, seq->t8, seq->t9, seq->t10, seq->t11_t12);
+}
+
+static void
+intel_pps_verify_state(struct drm_i915_private *dev_priv,
+ struct intel_dp *intel_dp)
+{
+ struct edp_power_seq hw;
+ struct edp_power_seq *sw = &intel_dp->pps_delays;
+
+ intel_pps_readout_hw_state(dev_priv, intel_dp, &hw);
+
+ /*
+ * We don't use/program the HW T8 and T9 timings as we use SW based
+ * delays for these, so the HW state of these fields are dont-care.
+ */
+ if (hw.t1_t3 != sw->t1_t3 || hw.t10 != sw->t10 ||
+ hw.t11_t12 != sw->t11_t12) {
+ DRM_ERROR("PPS state mismatch\n");
+ intel_pps_dump_state("sw", sw);
+ intel_pps_dump_state("hw", &hw);
+ }
+}
+
+static void
intel_dp_init_panel_power_sequencer(struct drm_device *dev,
struct intel_dp *intel_dp)
{
@@ -4836,8 +4870,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
intel_pps_readout_hw_state(dev_priv, intel_dp, &cur);
- DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
- cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
+ intel_pps_dump_state("cur", &cur);
vbt = dev_priv->vbt.edp.pps;
@@ -4853,8 +4886,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
* too. */
spec.t11_t12 = (510 + 100) * 10;
- DRM_DEBUG_KMS("vbt t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
- vbt.t1_t3, vbt.t8, vbt.t9, vbt.t10, vbt.t11_t12);
+ intel_pps_dump_state("vbt", &vbt);
/* Use the max of the register settings and vbt. If both are
* unset, fall back to the spec limits. */
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/4] drm/i915: Sanity check PPS HW state
2016-06-16 13:37 ` [PATCH 4/4] drm/i915: Sanity check " Imre Deak
@ 2016-06-16 14:01 ` Ville Syrjälä
2016-06-16 15:56 ` Imre Deak
2016-06-16 17:01 ` [PATCH v2 " Imre Deak
1 sibling, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2016-06-16 14:01 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
On Thu, Jun 16, 2016 at 04:37:23PM +0300, Imre Deak wrote:
> The wait for panel status helper will only function correctly if the
> HW panel timings are programmed correctly. Returning prematurely from
> this helper may lead to obscure bugs later, so sanity check the HW
> timing registers.
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 40 ++++++++++++++++++++++++++++++++++++----
> 1 file changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index caad825..163dcad 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1772,6 +1772,9 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
> #define IDLE_CYCLE_MASK (PP_ON | PP_SEQUENCE_MASK | PP_CYCLE_DELAY_ACTIVE | PP_SEQUENCE_STATE_MASK)
> #define IDLE_CYCLE_VALUE (0 | PP_SEQUENCE_NONE | 0 | PP_SEQUENCE_STATE_OFF_IDLE)
>
> +static void intel_pps_verify_state(struct drm_i915_private *dev_priv,
> + struct intel_dp *intel_dp);
> +
> static void wait_panel_status(struct intel_dp *intel_dp,
> u32 mask,
> u32 value)
> @@ -1782,6 +1785,8 @@ static void wait_panel_status(struct intel_dp *intel_dp,
>
> lockdep_assert_held(&dev_priv->pps_mutex);
>
> + intel_pps_verify_state(dev_priv, intel_dp);
> +
> pp_stat_reg = _pp_stat_reg(intel_dp);
> pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
>
> @@ -4821,6 +4826,35 @@ intel_pps_readout_hw_state(struct drm_i915_private *dev_priv,
> }
>
> static void
> +intel_pps_dump_state(const char *state_name, const struct edp_power_seq *seq)
> +{
> + DRM_DEBUG_KMS("%s t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
> + state_name,
> + seq->t1_t3, seq->t8, seq->t9, seq->t10, seq->t11_t12);
> +}
> +
> +static void
> +intel_pps_verify_state(struct drm_i915_private *dev_priv,
> + struct intel_dp *intel_dp)
> +{
> + struct edp_power_seq hw;
> + struct edp_power_seq *sw = &intel_dp->pps_delays;
> +
> + intel_pps_readout_hw_state(dev_priv, intel_dp, &hw);
> +
> + /*
> + * We don't use/program the HW T8 and T9 timings as we use SW based
> + * delays for these, so the HW state of these fields are dont-care.
> + */
I don't think they should be treated as "don't care". We want them to
be 1 to avoid needless delays.
> + if (hw.t1_t3 != sw->t1_t3 || hw.t10 != sw->t10 ||
> + hw.t11_t12 != sw->t11_t12) {
> + DRM_ERROR("PPS state mismatch\n");
> + intel_pps_dump_state("sw", sw);
> + intel_pps_dump_state("hw", &hw);
> + }
> +}
> +
> +static void
> intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> struct intel_dp *intel_dp)
> {
> @@ -4836,8 +4870,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
>
> intel_pps_readout_hw_state(dev_priv, intel_dp, &cur);
>
> - DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
> - cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
> + intel_pps_dump_state("cur", &cur);
>
> vbt = dev_priv->vbt.edp.pps;
>
> @@ -4853,8 +4886,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> * too. */
> spec.t11_t12 = (510 + 100) * 10;
>
> - DRM_DEBUG_KMS("vbt t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
> - vbt.t1_t3, vbt.t8, vbt.t9, vbt.t10, vbt.t11_t12);
> + intel_pps_dump_state("vbt", &vbt);
>
> /* Use the max of the register settings and vbt. If both are
> * unset, fall back to the spec limits. */
> --
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] 15+ messages in thread* Re: [PATCH 4/4] drm/i915: Sanity check PPS HW state
2016-06-16 14:01 ` Ville Syrjälä
@ 2016-06-16 15:56 ` Imre Deak
0 siblings, 0 replies; 15+ messages in thread
From: Imre Deak @ 2016-06-16 15:56 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On to, 2016-06-16 at 17:01 +0300, Ville Syrjälä wrote:
> On Thu, Jun 16, 2016 at 04:37:23PM +0300, Imre Deak wrote:
> > The wait for panel status helper will only function correctly if the
> > HW panel timings are programmed correctly. Returning prematurely from
> > this helper may lead to obscure bugs later, so sanity check the HW
> > timing registers.
> >
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 40 ++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 36 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index caad825..163dcad 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1772,6 +1772,9 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
> > #define IDLE_CYCLE_MASK (PP_ON | PP_SEQUENCE_MASK | PP_CYCLE_DELAY_ACTIVE | PP_SEQUENCE_STATE_MASK)
> > #define IDLE_CYCLE_VALUE (0 | PP_SEQUENCE_NONE | 0 | PP_SEQUENCE_STATE_OFF_IDLE)
> >
> > +static void intel_pps_verify_state(struct drm_i915_private *dev_priv,
> > + struct intel_dp *intel_dp);
> > +
> > static void wait_panel_status(struct intel_dp *intel_dp,
> > u32 mask,
> > u32 value)
> > @@ -1782,6 +1785,8 @@ static void wait_panel_status(struct intel_dp *intel_dp,
> >
> > lockdep_assert_held(&dev_priv->pps_mutex);
> >
> > + intel_pps_verify_state(dev_priv, intel_dp);
> > +
> > pp_stat_reg = _pp_stat_reg(intel_dp);
> > pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
> >
> > @@ -4821,6 +4826,35 @@ intel_pps_readout_hw_state(struct drm_i915_private *dev_priv,
> > }
> >
> > static void
> > +intel_pps_dump_state(const char *state_name, const struct edp_power_seq *seq)
> > +{
> > + DRM_DEBUG_KMS("%s t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
> > + state_name,
> > + seq->t1_t3, seq->t8, seq->t9, seq->t10, seq->t11_t12);
> > +}
> > +
> > +static void
> > +intel_pps_verify_state(struct drm_i915_private *dev_priv,
> > + struct intel_dp *intel_dp)
> > +{
> > + struct edp_power_seq hw;
> > + struct edp_power_seq *sw = &intel_dp->pps_delays;
> > +
> > + intel_pps_readout_hw_state(dev_priv, intel_dp, &hw);
> > +
> > + /*
> > + * We don't use/program the HW T8 and T9 timings as we use SW based
> > + * delays for these, so the HW state of these fields are dont-care.
> > + */
>
> I don't think they should be treated as "don't care". We want them to
> be 1 to avoid needless delays.
Ah right, didn't notice that we program these. I'll fix this.
>
> > + if (hw.t1_t3 != sw->t1_t3 || hw.t10 != sw->t10 ||
> > + hw.t11_t12 != sw->t11_t12) {
> > + DRM_ERROR("PPS state mismatch\n");
> > + intel_pps_dump_state("sw", sw);
> > + intel_pps_dump_state("hw", &hw);
> > + }
> > +}
> > +
> > +static void
> > intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> > struct intel_dp *intel_dp)
> > {
> > @@ -4836,8 +4870,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> >
> > intel_pps_readout_hw_state(dev_priv, intel_dp, &cur);
> >
> > - DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
> > - cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
> > + intel_pps_dump_state("cur", &cur);
> >
> > vbt = dev_priv->vbt.edp.pps;
> >
> > @@ -4853,8 +4886,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
> > * too. */
> > spec.t11_t12 = (510 + 100) * 10;
> >
> > - DRM_DEBUG_KMS("vbt t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
> > - vbt.t1_t3, vbt.t8, vbt.t9, vbt.t10, vbt.t11_t12);
> > + intel_pps_dump_state("vbt", &vbt);
> >
> > /* Use the max of the register settings and vbt. If both are
> > * unset, fall back to the spec limits. */
> > --
> > 2.5.0
> >
> > _______________________________________________
> > 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] 15+ messages in thread
* [PATCH v2 4/4] drm/i915: Sanity check PPS HW state
2016-06-16 13:37 ` [PATCH 4/4] drm/i915: Sanity check " Imre Deak
2016-06-16 14:01 ` Ville Syrjälä
@ 2016-06-16 17:01 ` Imre Deak
1 sibling, 0 replies; 15+ messages in thread
From: Imre Deak @ 2016-06-16 17:01 UTC (permalink / raw)
To: intel-gfx
The wait for panel status helper will only function correctly if the
HW panel timings are programmed correctly. Returning prematurely from
this helper may lead to obscure bugs later, so sanity check the HW
timing registers.
v2:
- Check the T8, T9 fields too, we do program them (Ville)
CC: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 58 +++++++++++++++++++++++++++++++----------
1 file changed, 44 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index caad825..75d07d8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1772,6 +1772,9 @@ static void intel_dp_prepare(struct intel_encoder *encoder)
#define IDLE_CYCLE_MASK (PP_ON | PP_SEQUENCE_MASK | PP_CYCLE_DELAY_ACTIVE | PP_SEQUENCE_STATE_MASK)
#define IDLE_CYCLE_VALUE (0 | PP_SEQUENCE_NONE | 0 | PP_SEQUENCE_STATE_OFF_IDLE)
+static void intel_pps_verify_state(struct drm_i915_private *dev_priv,
+ struct intel_dp *intel_dp);
+
static void wait_panel_status(struct intel_dp *intel_dp,
u32 mask,
u32 value)
@@ -1782,6 +1785,8 @@ static void wait_panel_status(struct intel_dp *intel_dp,
lockdep_assert_held(&dev_priv->pps_mutex);
+ intel_pps_verify_state(dev_priv, intel_dp);
+
pp_stat_reg = _pp_stat_reg(intel_dp);
pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
@@ -4821,6 +4826,31 @@ intel_pps_readout_hw_state(struct drm_i915_private *dev_priv,
}
static void
+intel_pps_dump_state(const char *state_name, const struct edp_power_seq *seq)
+{
+ DRM_DEBUG_KMS("%s t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
+ state_name,
+ seq->t1_t3, seq->t8, seq->t9, seq->t10, seq->t11_t12);
+}
+
+static void
+intel_pps_verify_state(struct drm_i915_private *dev_priv,
+ struct intel_dp *intel_dp)
+{
+ struct edp_power_seq hw;
+ struct edp_power_seq *sw = &intel_dp->pps_delays;
+
+ intel_pps_readout_hw_state(dev_priv, intel_dp, &hw);
+
+ if (hw.t1_t3 != sw->t1_t3 || hw.t8 != sw->t8 || hw.t9 != sw->t9 ||
+ hw.t10 != sw->t10 || hw.t11_t12 != sw->t11_t12) {
+ DRM_ERROR("PPS state mismatch\n");
+ intel_pps_dump_state("sw", sw);
+ intel_pps_dump_state("hw", &hw);
+ }
+}
+
+static void
intel_dp_init_panel_power_sequencer(struct drm_device *dev,
struct intel_dp *intel_dp)
{
@@ -4836,8 +4866,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
intel_pps_readout_hw_state(dev_priv, intel_dp, &cur);
- DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
- cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
+ intel_pps_dump_state("cur", &cur);
vbt = dev_priv->vbt.edp.pps;
@@ -4853,8 +4882,7 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
* too. */
spec.t11_t12 = (510 + 100) * 10;
- DRM_DEBUG_KMS("vbt t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
- vbt.t1_t3, vbt.t8, vbt.t9, vbt.t10, vbt.t11_t12);
+ intel_pps_dump_state("vbt", &vbt);
/* Use the max of the register settings and vbt. If both are
* unset, fall back to the spec limits. */
@@ -4882,6 +4910,16 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev,
DRM_DEBUG_KMS("backlight on delay %d, off delay %d\n",
intel_dp->backlight_on_delay, intel_dp->backlight_off_delay);
+
+ /*
+ * We override the HW backlight delays to 1 because we do manual waits
+ * on them. For T8, even BSpec recommends doing it. For T9, if we
+ * don't do this, we'll end up waiting for the backlight off delay
+ * twice: once when we do the manual sleep, and once when we disable
+ * the panel and wait for the PP_STATUS bit to become zero.
+ */
+ final->t8 = 1;
+ final->t9 = 1;
}
static void
@@ -4899,17 +4937,9 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
intel_pps_get_registers(dev_priv, intel_dp, ®s);
- /*
- * And finally store the new values in the power sequencer. The
- * backlight delays are set to 1 because we do manual waits on them. For
- * T8, even BSpec recommends doing it. For T9, if we don't do this,
- * we'll end up waiting for the backlight off delay twice: once when we
- * do the manual sleep, and once when we disable the panel and wait for
- * the PP_STATUS bit to become zero.
- */
pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) |
- (1 << PANEL_LIGHT_ON_DELAY_SHIFT);
- pp_off = (1 << PANEL_LIGHT_OFF_DELAY_SHIFT) |
+ (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT);
+ pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) |
(seq->t10 << PANEL_POWER_DOWN_DELAY_SHIFT);
/* Compute the divisor for the pp clock, simply match the Bspec
* formula. */
--
2.5.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix eDP link training after suspend
2016-06-16 13:37 [PATCH 0/4] drm/i915/bxt: Fix eDP link training after suspend Imre Deak
` (3 preceding siblings ...)
2016-06-16 13:37 ` [PATCH 4/4] drm/i915: Sanity check " Imre Deak
@ 2016-06-16 13:56 ` Patchwork
2016-06-16 18:37 ` [PATCH 0/4] " Ville Syrjälä
2016-06-17 5:21 ` ✗ Ro.CI.BAT: failure for drm/i915/bxt: Fix eDP link training after suspend (rev2) Patchwork
6 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2016-06-16 13:56 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/bxt: Fix eDP link training after suspend
URL : https://patchwork.freedesktop.org/series/8783/
State : warning
== Summary ==
Series 8783v1 drm/i915/bxt: Fix eDP link training after suspend
http://patchwork.freedesktop.org/api/1.0/series/8783/revisions/1/mbox
Test drv_module_reload_basic:
pass -> SKIP (ro-hsw-i3-4010u)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup basic-flip-vs-modeset:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup basic-flip-vs-wf_vblank:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup basic-plain-flip:
pass -> SKIP (fi-bdw-i7-5557u)
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> SKIP (fi-bdw-i7-5557u)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup hang-read-crc-pipe-b:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup hang-read-crc-pipe-c:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-a:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-a-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-b:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-b-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-c:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-c-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-a:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-a-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-b:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-b-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-c:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-c-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup suspend-read-crc-pipe-a:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup suspend-read-crc-pipe-b:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup suspend-read-crc-pipe-c:
pass -> SKIP (fi-bdw-i7-5557u)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup basic-rte:
pass -> SKIP (fi-bdw-i7-5557u)
fi-bdw-i7-5557u total:213 pass:176 dwarn:0 dfail:0 fail:0 skip:37
fi-skl-i7-6700k total:213 pass:188 dwarn:0 dfail:0 fail:0 skip:25
ro-bdw-i5-5250u total:213 pass:197 dwarn:1 dfail:0 fail:0 skip:15
ro-bdw-i7-5557U total:213 pass:198 dwarn:0 dfail:0 fail:0 skip:15
ro-bdw-i7-5600u total:213 pass:185 dwarn:0 dfail:0 fail:0 skip:28
ro-bsw-n3050 total:213 pass:172 dwarn:0 dfail:0 fail:2 skip:39
ro-byt-n2820 total:213 pass:173 dwarn:0 dfail:0 fail:3 skip:37
ro-hsw-i3-4010u total:213 pass:189 dwarn:0 dfail:0 fail:0 skip:24
ro-hsw-i7-4770r total:213 pass:190 dwarn:0 dfail:0 fail:0 skip:23
ro-ilk-i7-620lm total:213 pass:150 dwarn:0 dfail:0 fail:1 skip:62
ro-ilk1-i5-650 total:208 pass:150 dwarn:0 dfail:0 fail:1 skip:57
ro-ivb-i7-3770 total:213 pass:181 dwarn:0 dfail:0 fail:0 skip:32
ro-ivb2-i7-3770 total:213 pass:185 dwarn:0 dfail:0 fail:0 skip:28
ro-skl3-i5-6260u total:213 pass:201 dwarn:1 dfail:0 fail:0 skip:11
ro-snb-i7-2620M total:213 pass:174 dwarn:0 dfail:0 fail:1 skip:38
fi-hsw-i7-4770k failed to connect after reboot
fi-skl-i5-6260u failed to connect after reboot
fi-snb-i7-2600 failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_1195/
3eb202e drm-intel-nightly: 2016y-06m-16d-12h-38m-37s UTC integration manifest
eaf948d drm/i915: Sanity check PPS HW state
bb833e0 drm/i915: Factor out helper to read out PPS HW state
93a875d drm/i915: Deduplicate PPS register retrieval
e137c10 drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link training
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/4] drm/i915/bxt: Fix eDP link training after suspend
2016-06-16 13:37 [PATCH 0/4] drm/i915/bxt: Fix eDP link training after suspend Imre Deak
` (4 preceding siblings ...)
2016-06-16 13:56 ` ✗ Ro.CI.BAT: warning for drm/i915/bxt: Fix eDP link training after suspend Patchwork
@ 2016-06-16 18:37 ` Ville Syrjälä
2016-06-17 5:21 ` ✗ Ro.CI.BAT: failure for drm/i915/bxt: Fix eDP link training after suspend (rev2) Patchwork
6 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2016-06-16 18:37 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
On Thu, Jun 16, 2016 at 04:37:19PM +0300, Imre Deak wrote:
> This fixes eDP link training errors I noticed on BXT after suspend to
> ram and adds some HW sanity check to catch similar issues in the future.
>
> Imre Deak (4):
> drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link
> training
> drm/i915: Deduplicate PPS register retrieval
> drm/i915: Factor out helper to read out PPS HW state
> drm/i915: Sanity check PPS HW state
Series lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> drivers/gpu/drm/i915/intel_dp.c | 274 ++++++++++++++++++++------------
> drivers/gpu/drm/i915/intel_drv.h | 7 +-
> drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +-
> 3 files changed, 179 insertions(+), 105 deletions(-)
>
> --
> 2.5.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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] 15+ messages in thread* ✗ Ro.CI.BAT: failure for drm/i915/bxt: Fix eDP link training after suspend (rev2)
2016-06-16 13:37 [PATCH 0/4] drm/i915/bxt: Fix eDP link training after suspend Imre Deak
` (5 preceding siblings ...)
2016-06-16 18:37 ` [PATCH 0/4] " Ville Syrjälä
@ 2016-06-17 5:21 ` Patchwork
2016-06-22 16:45 ` Imre Deak
6 siblings, 1 reply; 15+ messages in thread
From: Patchwork @ 2016-06-17 5:21 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/bxt: Fix eDP link training after suspend (rev2)
URL : https://patchwork.freedesktop.org/series/8783/
State : failure
== Summary ==
Series 8783v2 drm/i915/bxt: Fix eDP link training after suspend
http://patchwork.freedesktop.org/api/1.0/series/8783/revisions/2/mbox
Test drv_module_reload_basic:
pass -> DMESG-WARN (fi-skl-i5-6260u)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup basic-flip-vs-modeset:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup basic-flip-vs-wf_vblank:
pass -> SKIP (fi-bdw-i7-5557u)
pass -> DMESG-FAIL (ro-bdw-i5-5250u)
Subgroup basic-plain-flip:
pass -> SKIP (fi-bdw-i7-5557u)
Test kms_frontbuffer_tracking:
Subgroup basic:
pass -> SKIP (fi-bdw-i7-5557u)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup hang-read-crc-pipe-b:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup hang-read-crc-pipe-c:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-a:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-a-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-b:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-b-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-c:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup nonblocking-crc-pipe-c-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-a:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-a-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-b:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-b-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup read-crc-pipe-c:
pass -> SKIP (fi-bdw-i7-5557u)
pass -> SKIP (fi-skl-i5-6260u)
Subgroup read-crc-pipe-c-frame-sequence:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup suspend-read-crc-pipe-a:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup suspend-read-crc-pipe-b:
pass -> SKIP (fi-bdw-i7-5557u)
Subgroup suspend-read-crc-pipe-c:
pass -> SKIP (fi-bdw-i7-5557u)
skip -> DMESG-WARN (ro-bdw-i5-5250u)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass -> SKIP (fi-bdw-i7-5557u)
pass -> SKIP (fi-skl-i5-6260u)
Subgroup basic-rte:
pass -> SKIP (fi-bdw-i7-5557u)
fi-bdw-i7-5557u total:213 pass:176 dwarn:0 dfail:0 fail:0 skip:37
fi-skl-i5-6260u total:213 pass:199 dwarn:1 dfail:0 fail:0 skip:13
fi-skl-i7-6700k total:213 pass:188 dwarn:0 dfail:0 fail:0 skip:25
fi-snb-i7-2600 total:213 pass:174 dwarn:0 dfail:0 fail:0 skip:39
ro-bdw-i5-5250u total:213 pass:196 dwarn:2 dfail:1 fail:0 skip:14
ro-bdw-i7-5557U total:213 pass:198 dwarn:0 dfail:0 fail:0 skip:15
ro-bdw-i7-5600u total:213 pass:185 dwarn:0 dfail:0 fail:0 skip:28
ro-bsw-n3050 total:213 pass:172 dwarn:0 dfail:0 fail:2 skip:39
ro-byt-n2820 total:213 pass:173 dwarn:0 dfail:0 fail:3 skip:37
ro-hsw-i3-4010u total:213 pass:190 dwarn:0 dfail:0 fail:0 skip:23
ro-hsw-i7-4770r total:213 pass:190 dwarn:0 dfail:0 fail:0 skip:23
ro-ilk-i7-620lm total:213 pass:150 dwarn:0 dfail:0 fail:1 skip:62
ro-ilk1-i5-650 total:208 pass:150 dwarn:0 dfail:0 fail:1 skip:57
ro-ivb-i7-3770 total:213 pass:181 dwarn:0 dfail:0 fail:0 skip:32
ro-ivb2-i7-3770 total:213 pass:185 dwarn:0 dfail:0 fail:0 skip:28
ro-skl3-i5-6260u total:213 pass:201 dwarn:1 dfail:0 fail:0 skip:11
ro-snb-i7-2620M total:213 pass:174 dwarn:0 dfail:0 fail:1 skip:38
fi-hsw-i7-4770k failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_1199/
3eb202e drm-intel-nightly: 2016y-06m-16d-12h-38m-37s UTC integration manifest
e064ce6 drm/i915: Sanity check PPS HW state
fbe8754 drm/i915: Factor out helper to read out PPS HW state
0de3cc5 drm/i915: Deduplicate PPS register retrieval
49bba19 drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link training
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: ✗ Ro.CI.BAT: failure for drm/i915/bxt: Fix eDP link training after suspend (rev2)
2016-06-17 5:21 ` ✗ Ro.CI.BAT: failure for drm/i915/bxt: Fix eDP link training after suspend (rev2) Patchwork
@ 2016-06-22 16:45 ` Imre Deak
0 siblings, 0 replies; 15+ messages in thread
From: Imre Deak @ 2016-06-22 16:45 UTC (permalink / raw)
To: intel-gfx, Ville Syrjälä
On pe, 2016-06-17 at 05:21 +0000, Patchwork wrote:
> == Series Details ==
>
> Series: drm/i915/bxt: Fix eDP link training after suspend (rev2)
> URL : https://patchwork.freedesktop.org/series/8783/
> State : failure
>
> == Summary ==
>
> Series 8783v2 drm/i915/bxt: Fix eDP link training after suspend
> http://patchwork.freedesktop.org/api/1.0/series/8783/revisions/2/mbox
>
> Test drv_module_reload_basic:
> pass -> DMESG-WARN (fi-skl-i5-6260u)
[drm:wait_panel_status [i915]] *ERROR* PPS state mismatch,
fixed already by
drm/i915: Initialize the PPS HW before its first use
> Test kms_flip:
> Subgroup basic-flip-vs-dpms:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup basic-flip-vs-modeset:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup basic-flip-vs-wf_vblank:
> pass -> SKIP (fi-bdw-i7-5557u)
> pass -> DMESG-FAIL (ro-bdw-i5-5250u)
Looks like new, but this machine doesn't have eDP so shouldn't be
affected by these patches:
(kms_flip:8270) DEBUG: name = flip
last_ts = 391.822981 usec
last_received_ts = 391.822434 usec
last_seq = 1076
current_ts = 392.39532 usec
current_received_ts = 392.42537 usec
current_seq = 1089
count = 13
seq_step = 1
(kms_flip:8270) CRITICAL: Test assertion failure function check_state, file kms_flip.c:698:
(kms_flip:8270) CRITICAL: Failed assertion: fabs((((double) diff.tv_usec) - usec_interflip) / usec_interflip) <= 0.005
(kms_flip:8270) CRITICAL: Last errno: 25, Inappropriate ioctl for device
(kms_flip:8270) CRITICAL: inter-flip ts jitter: 0s, 216551usec
**** END ****
[ 392.247250] WARNING: CPU: 1 PID: 7990 at drivers/gpu/drm/i915/intel_display.c:13666 intel_atomic_commit_tail+0x1171/0x1180 [i915]
[ 392.247253] pipe C vblank wait timed out
[ 392.247255] Modules linked in: snd_hda_intel i915 drm_kms_helper drm x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul snd_hda_codec_realtek crc32_pclmul snd_hda_codec_generic ghash_clmulni_intel snd_hda_codec_hdmi snd_hda_codec mei_me snd_hwdep mei snd_hda_core lpc_ich snd_pcm i2c_designware_platform i2c_designware_core i2c_algo_bit syscopyarea sysfillrect sysimgblt fb_sys_fops e1000e ptp pps_core sdhci_acpi sdhci mmc_core i2c_hid [last unloaded: drm]
[ 392.247299] CPU: 1 PID: 7990 Comm: kworker/u8:12 Tainted: G U 4.7.0-rc2-gfxbench-RO_Patchwork_1199+ #1
[ 392.247301] Hardware name: /NUC5i5RYB, BIOS RYBDWi35.86A.0350.2015.0812.1722 08/12/2015
[ 392.247330] Workqueue: events_unbound intel_atomic_commit_work [i915]
[ 392.247339] 0000000000000000 ffff8803f8d6bc10 ffffffff8140e085 ffff8803f8d6bc60
[ 392.247354] 0000000000000000 ffff8803f8d6bc50 ffffffff8107a996 0000356200000292
[ 392.247368] 00000000000002b0 ffff88043cbc0008 0000000000000008 0000000000000002
[ 392.247381] Call Trace:
[ 392.247390] [<ffffffff8140e085>] dump_stack+0x67/0x92
[ 392.247397] [<ffffffff8107a996>] __warn+0xc6/0xe0
[ 392.247403] [<ffffffff8107a9fa>] warn_slowpath_fmt+0x4a/0x50
[ 392.247410] [<ffffffff810c45c9>] ? finish_wait+0x59/0x70
[ 392.247439] [<ffffffffa01ec4d1>] intel_atomic_commit_tail+0x1171/0x1180 [i915]
[ 392.247446] [<ffffffff810c4790>] ? wake_atomic_t_function+0x60/0x60
[ 392.247474] [<ffffffffa01eca2d>] intel_atomic_commit_work+0xd/0x10 [i915]
[ 392.247480] [<ffffffff810975eb>] process_one_work+0x1eb/0x6c0
[ 392.247485] [<ffffffff81097566>] ? process_one_work+0x166/0x6c0
[ 392.247490] [<ffffffff81097b09>] worker_thread+0x49/0x490
[ 392.247493] [<ffffffff81097ac0>] ? process_one_work+0x6c0/0x6c0
[ 392.247496] [<ffffffff8109ddba>] kthread+0xea/0x100
[ 392.247503] [<ffffffff817acd2f>] ret_from_fork+0x1f/0x40
[ 392.247506] [<ffffffff8109dcd0>] ? kthread_create_on_node+0x1f0/0x1f0
[ 392.247509] ---[ end trace 2dcf267c73aeb528 ]---
> Subgroup basic-plain-flip:
> pass -> SKIP (fi-bdw-i7-5557u)
> Test kms_frontbuffer_tracking:
> Subgroup basic:
> pass -> SKIP (fi-bdw-i7-5557u)
> Test kms_pipe_crc_basic:
> Subgroup hang-read-crc-pipe-a:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup hang-read-crc-pipe-b:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup hang-read-crc-pipe-c:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup nonblocking-crc-pipe-a:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup nonblocking-crc-pipe-a-frame-sequence:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup nonblocking-crc-pipe-b:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup nonblocking-crc-pipe-b-frame-sequence:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup nonblocking-crc-pipe-c:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup nonblocking-crc-pipe-c-frame-sequence:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup read-crc-pipe-a:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup read-crc-pipe-a-frame-sequence:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup read-crc-pipe-b:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup read-crc-pipe-b-frame-sequence:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup read-crc-pipe-c:
> pass -> SKIP (fi-bdw-i7-5557u)
> pass -> SKIP (fi-skl-i5-6260u)
> Subgroup read-crc-pipe-c-frame-sequence:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup suspend-read-crc-pipe-a:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup suspend-read-crc-pipe-b:
> pass -> SKIP (fi-bdw-i7-5557u)
> Subgroup suspend-read-crc-pipe-c:
> pass -> SKIP (fi-bdw-i7-5557u)
> skip -> DMESG-WARN (ro-bdw-i5-5250u)
Pre-existing bug:
https://bugs.freedesktop.org/show_bug.cgi?id=96614
After suspend-to-ram/resume:
[ 430.375822] [drm:intel_dp_link_training_clock_recovery [i915]] *ERROR* failed to enable link training
[ 430.460710] [drm:intel_dp_start_link_train [i915]] *ERROR* failed to start channel equalization
I also sent the patches to trybot due to all the rest of the SKIPs, I
haven't seen those anymore.
Thanks for the review, I pushed the patchset to -dinq.
--Imre
> Test pm_rpm:
> Subgroup basic-pci-d3-state:
> pass -> SKIP (fi-bdw-i7-5557u)
> pass -> SKIP (fi-skl-i5-6260u)
> Subgroup basic-rte:
> pass -> SKIP (fi-bdw-i7-5557u)
>
> fi-bdw-i7-5557u total:213 pass:176 dwarn:0 dfail:0 fail:0 skip:37
> fi-skl-i5-6260u total:213 pass:199 dwarn:1 dfail:0 fail:0 skip:13
> fi-skl-i7-6700k total:213 pass:188 dwarn:0 dfail:0 fail:0 skip:25
> fi-snb-i7-2600 total:213 pass:174 dwarn:0 dfail:0 fail:0 skip:39
> ro-bdw-i5-5250u total:213 pass:196 dwarn:2 dfail:1 fail:0 skip:14
> ro-bdw-i7-5557U total:213 pass:198 dwarn:0 dfail:0 fail:0 skip:15
> ro-bdw-i7-5600u total:213 pass:185 dwarn:0 dfail:0 fail:0 skip:28
> ro-bsw-n3050 total:213 pass:172 dwarn:0 dfail:0 fail:2 skip:39
> ro-byt-n2820 total:213 pass:173 dwarn:0 dfail:0 fail:3 skip:37
> ro-hsw-i3-4010u total:213 pass:190 dwarn:0 dfail:0 fail:0 skip:23
> ro-hsw-i7-4770r total:213 pass:190 dwarn:0 dfail:0 fail:0 skip:23
> ro-ilk-i7-620lm total:213 pass:150 dwarn:0 dfail:0 fail:1 skip:62
> ro-ilk1-i5-650 total:208 pass:150 dwarn:0 dfail:0 fail:1 skip:57
> ro-ivb-i7-3770 total:213 pass:181 dwarn:0 dfail:0 fail:0 skip:32
> ro-ivb2-i7-3770 total:213 pass:185 dwarn:0 dfail:0 fail:0 skip:28
> ro-skl3-i5-6260u total:213 pass:201 dwarn:1 dfail:0 fail:0 skip:11
> ro-snb-i7-2620M total:213 pass:174 dwarn:0 dfail:0 fail:1 skip:38
> fi-hsw-i7-4770k failed to connect after reboot
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1199/
>
> 3eb202e drm-intel-nightly: 2016y-06m-16d-12h-38m-37s UTC integration manifest
> e064ce6 drm/i915: Sanity check PPS HW state
> fbe8754 drm/i915: Factor out helper to read out PPS HW state
> 0de3cc5 drm/i915: Deduplicate PPS register retrieval
> 49bba19 drm/i915/bxt: Fix PPS lost state after suspend breaking eDP link training
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread