public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB
@ 2015-11-20 20:09 ville.syrjala
  2015-11-20 20:09 ` [PATCH 2/3] drm/i915: Disable CPU underruns around eDP port and vdd enable " ville.syrjala
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: ville.syrjala @ 2015-11-20 20:09 UTC (permalink / raw)
  To: intel-gfx

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

We still get spurious pipe underruns on ILK/SNB/IVB under two
circumstances when dealing with PCH ports:
* When the pipe has been disabled, but FDI RX/TX is still enabled
* During FDI link training

Both cases seem to happen at least when we do VGA+HDMI cloning
from the same pipe. I don't think I've seen them when not cloning,
but can't be 100% sure.

Disable underrun reporting around those places to eliminate the
dmesg errors.

Testcase: igt/kms_setmode/basic-clone-single-crtc
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 68fb449ded77..8a8104b7947d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4137,6 +4137,12 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
 	I915_WRITE(FDI_RX_TUSIZE1(pipe),
 		   I915_READ(PIPE_DATA_M1(pipe)) & TU_SIZE_MASK);
 
+	/*
+	 * Sometimes spurious CPU pipe underruns happen during FDI
+	 * training, at least with VGA+HDMI cloning. Suppress them.
+	 */
+	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
+
 	/* For PCH output, training FDI link */
 	dev_priv->display.fdi_link_train(crtc);
 
@@ -4170,6 +4176,8 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
 
 	intel_fdi_normal_train(crtc);
 
+	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
+
 	/* For PCH DP, enable TRANS_DP_CTL */
 	if (HAS_PCH_CPT(dev) && intel_crtc->config->has_dp_encoder) {
 		const struct drm_display_mode *adjusted_mode =
@@ -5062,12 +5070,22 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	drm_crtc_vblank_off(crtc);
 	assert_vblank_disabled(crtc);
 
+	/*
+	 * Sometimes spurious CPU pipe underruns happen when the
+	 * pipe is already disabled, but FDI RX/TX is still enabled.
+	 * Happens at least with VGA+HDMI cloning. Suppress them.
+	 */
+	if (intel_crtc->config->has_pch_encoder)
+		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
+
 	intel_disable_pipe(intel_crtc);
 
 	ironlake_pfit_disable(intel_crtc, false);
 
-	if (intel_crtc->config->has_pch_encoder)
+	if (intel_crtc->config->has_pch_encoder) {
 		ironlake_fdi_disable(crtc);
+		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
+	}
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->post_disable)
-- 
2.4.10

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] drm/i915: Disable CPU underruns around eDP port and vdd enable on ILK-IVB
  2015-11-20 20:09 [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB ville.syrjala
@ 2015-11-20 20:09 ` ville.syrjala
  2015-11-21 10:52   ` Chris Wilson
  2015-11-20 20:09 ` [PATCH 3/3] drm/i915: Check for underruns after crtc disable ville.syrjala
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: ville.syrjala @ 2015-11-20 20:09 UTC (permalink / raw)
  To: intel-gfx

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

We sometimes get a spurious CPU pipe underrun somewhere between
enabling port A and enabling vdd for the panel. Observed on both
ILK and IVB with port A eDP. Suppress FIFO underrun reporting
around the port and vdd enable to avoid the dmesg errors.

Not sure if port D eDP would suffer from the same issue, but assume
that it doesn't until proven differently.

Testcase: igt/kms_setmode
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bec443a629da..2805f0d4ba45 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2697,6 +2697,15 @@ static void intel_enable_dp(struct intel_encoder *encoder)
 	if (IS_VALLEYVIEW(dev))
 		vlv_init_panel_power_sequencer(intel_dp);
 
+	/*
+	 * We get an occasional spurious underrun between the port
+	 * enable and vdd enable, when enabling port A eDP.
+	 *
+	 * FIXME: Not sure if this applies to (PCH) port D eDP as well
+	 */
+	if (port == PORT_A)
+		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
+
 	intel_dp_enable_port(intel_dp);
 
 	if (port == PORT_A && IS_GEN5(dev_priv)) {
@@ -2714,6 +2723,9 @@ static void intel_enable_dp(struct intel_encoder *encoder)
 	edp_panel_on(intel_dp);
 	edp_panel_vdd_off(intel_dp, true);
 
+	if (port == PORT_A)
+		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
+
 	pps_unlock(intel_dp);
 
 	if (IS_VALLEYVIEW(dev)) {
-- 
2.4.10

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] drm/i915: Check for underruns after crtc disable
  2015-11-20 20:09 [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB ville.syrjala
  2015-11-20 20:09 ` [PATCH 2/3] drm/i915: Disable CPU underruns around eDP port and vdd enable " ville.syrjala
@ 2015-11-20 20:09 ` ville.syrjala
  2015-11-21 10:49   ` Chris Wilson
  2015-11-21 10:51 ` [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB Chris Wilson
  2015-11-24 14:16 ` Daniel Vetter
  3 siblings, 1 reply; 16+ messages in thread
From: ville.syrjala @ 2015-11-20 20:09 UTC (permalink / raw)
  To: intel-gfx

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

To get a better idea if underruns occurred during crtc disabling,
let's check for them explicitly. This helps in cases where the
error interrupt isn't active, or there is no underrun interrupt
support at all.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8a8104b7947d..21f703397f09 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13402,6 +13402,13 @@ static int intel_atomic_commit(struct drm_device *dev,
 			dev_priv->display.crtc_disable(crtc);
 			intel_crtc->active = false;
 			intel_disable_shared_dpll(intel_crtc);
+
+			/*
+			 * Underruns don't always raise
+			 * interrupts, so check manually.
+			 */
+			intel_check_cpu_fifo_underruns(dev_priv);
+			intel_check_pch_fifo_underruns(dev_priv);
 		}
 	}
 
-- 
2.4.10

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] drm/i915: Check for underruns after crtc disable
  2015-11-20 20:09 ` [PATCH 3/3] drm/i915: Check for underruns after crtc disable ville.syrjala
@ 2015-11-21 10:49   ` Chris Wilson
  2015-11-23 14:10     ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2015-11-21 10:49 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Fri, Nov 20, 2015 at 10:09:20PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> To get a better idea if underruns occurred during crtc disabling,
> let's check for them explicitly. This helps in cases where the
> error interrupt isn't active, or there is no underrun interrupt
> support at all.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Would this be better the vblank after enabling?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB
  2015-11-20 20:09 [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB ville.syrjala
  2015-11-20 20:09 ` [PATCH 2/3] drm/i915: Disable CPU underruns around eDP port and vdd enable " ville.syrjala
  2015-11-20 20:09 ` [PATCH 3/3] drm/i915: Check for underruns after crtc disable ville.syrjala
@ 2015-11-21 10:51 ` Chris Wilson
  2015-11-23 14:22   ` Ville Syrjälä
  2015-11-24 14:16 ` Daniel Vetter
  3 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2015-11-21 10:51 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Fri, Nov 20, 2015 at 10:09:18PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We still get spurious pipe underruns on ILK/SNB/IVB under two
> circumstances when dealing with PCH ports:
> * When the pipe has been disabled, but FDI RX/TX is still enabled
> * During FDI link training
> 
> Both cases seem to happen at least when we do VGA+HDMI cloning
> from the same pipe. I don't think I've seen them when not cloning,
> but can't be 100% sure.
> 
> Disable underrun reporting around those places to eliminate the
> dmesg errors.
> 
> Testcase: igt/kms_setmode/basic-clone-single-crtc
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>

I wondered if logging the suppressed errors would be of any use? Does
the check_cpu_fifo work if the reporting is disabled? Could we do a
manual check and DRM_DEBUG_KMS() if the enable did generate a failure.
If the check_cpu_fifo does work, won't we still get the error from the
added check in atomic_commit()?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] drm/i915: Disable CPU underruns around eDP port and vdd enable on ILK-IVB
  2015-11-20 20:09 ` [PATCH 2/3] drm/i915: Disable CPU underruns around eDP port and vdd enable " ville.syrjala
@ 2015-11-21 10:52   ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2015-11-21 10:52 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Fri, Nov 20, 2015 at 10:09:19PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We sometimes get a spurious CPU pipe underrun somewhere between
> enabling port A and enabling vdd for the panel. Observed on both
> ILK and IVB with port A eDP. Suppress FIFO underrun reporting
> around the port and vdd enable to avoid the dmesg errors.
> 
> Not sure if port D eDP would suffer from the same issue, but assume
> that it doesn't until proven differently.
> 
> Testcase: igt/kms_setmode
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>

Same query as before as to whether we could add some DBG spew.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] drm/i915: Check for underruns after crtc disable
  2015-11-21 10:49   ` Chris Wilson
@ 2015-11-23 14:10     ` Ville Syrjälä
  2015-11-23 14:42       ` Chris Wilson
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2015-11-23 14:10 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Sat, Nov 21, 2015 at 10:49:04AM +0000, Chris Wilson wrote:
> On Fri, Nov 20, 2015 at 10:09:20PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > To get a better idea if underruns occurred during crtc disabling,
> > let's check for them explicitly. This helps in cases where the
> > error interrupt isn't active, or there is no underrun interrupt
> > support at all.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Would this be better the vblank after enabling?

We do that too.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB
  2015-11-21 10:51 ` [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB Chris Wilson
@ 2015-11-23 14:22   ` Ville Syrjälä
  2015-11-23 14:28     ` Chris Wilson
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2015-11-23 14:22 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Sat, Nov 21, 2015 at 10:51:50AM +0000, Chris Wilson wrote:
> On Fri, Nov 20, 2015 at 10:09:18PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We still get spurious pipe underruns on ILK/SNB/IVB under two
> > circumstances when dealing with PCH ports:
> > * When the pipe has been disabled, but FDI RX/TX is still enabled
> > * During FDI link training
> > 
> > Both cases seem to happen at least when we do VGA+HDMI cloning
> > from the same pipe. I don't think I've seen them when not cloning,
> > but can't be 100% sure.
> > 
> > Disable underrun reporting around those places to eliminate the
> > dmesg errors.
> > 
> > Testcase: igt/kms_setmode/basic-clone-single-crtc
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> I wondered if logging the suppressed errors would be of any use?

Hmm. Maybe just to confirm that they still happen, and thus suppressing
is still neded.

> Does
> the check_cpu_fifo work if the reporting is disabled? Could we do a
> manual check and DRM_DEBUG_KMS() if the enable did generate a failure.
> If the check_cpu_fifo does work, won't we still get the error from the
> added check in atomic_commit()?

The check only looks at pipes that have underrun reporting enabled. I
suppose it might be possible to have it check all the pipes. At the
point where we call it no explicit suppression should be happening, so
the only reason why underrun reporting would be disabled on any pipe is
due to detecting a previous underrun via the interrupt.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB
  2015-11-23 14:22   ` Ville Syrjälä
@ 2015-11-23 14:28     ` Chris Wilson
  2015-11-23 14:55       ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2015-11-23 14:28 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Mon, Nov 23, 2015 at 04:22:08PM +0200, Ville Syrjälä wrote:
> On Sat, Nov 21, 2015 at 10:51:50AM +0000, Chris Wilson wrote:
> > On Fri, Nov 20, 2015 at 10:09:18PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We still get spurious pipe underruns on ILK/SNB/IVB under two
> > > circumstances when dealing with PCH ports:
> > > * When the pipe has been disabled, but FDI RX/TX is still enabled
> > > * During FDI link training
> > > 
> > > Both cases seem to happen at least when we do VGA+HDMI cloning
> > > from the same pipe. I don't think I've seen them when not cloning,
> > > but can't be 100% sure.
> > > 
> > > Disable underrun reporting around those places to eliminate the
> > > dmesg errors.
> > > 
> > > Testcase: igt/kms_setmode/basic-clone-single-crtc
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > I wondered if logging the suppressed errors would be of any use?
> 
> Hmm. Maybe just to confirm that they still happen, and thus suppressing
> is still neded.
> 
> > Does
> > the check_cpu_fifo work if the reporting is disabled? Could we do a
> > manual check and DRM_DEBUG_KMS() if the enable did generate a failure.
> > If the check_cpu_fifo does work, won't we still get the error from the
> > added check in atomic_commit()?
> 
> The check only looks at pipes that have underrun reporting enabled. I
> suppose it might be possible to have it check all the pipes. At the
> point where we call it no explicit suppression should be happening, so
> the only reason why underrun reporting would be disabled on any pipe is
> due to detecting a previous underrun via the interrupt.

Does the hw flag the underrun even if the interrupt is disabled?
(Playing games with PIPESTAT_IER?) Could we then report (debug) that an
underrun happened before we re-enable reporting?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] drm/i915: Check for underruns after crtc disable
  2015-11-23 14:10     ` Ville Syrjälä
@ 2015-11-23 14:42       ` Chris Wilson
  2015-11-24 14:14         ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2015-11-23 14:42 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Mon, Nov 23, 2015 at 04:10:32PM +0200, Ville Syrjälä wrote:
> On Sat, Nov 21, 2015 at 10:49:04AM +0000, Chris Wilson wrote:
> > On Fri, Nov 20, 2015 at 10:09:20PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > To get a better idea if underruns occurred during crtc disabling,
> > > let's check for them explicitly. This helps in cases where the
> > > error interrupt isn't active, or there is no underrun interrupt
> > > support at all.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Would this be better the vblank after enabling?
> 
> We do that too.

Oh, crtc disabling. I can't read.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB
  2015-11-23 14:28     ` Chris Wilson
@ 2015-11-23 14:55       ` Ville Syrjälä
  2015-11-24 14:09         ` Daniel Vetter
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2015-11-23 14:55 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On Mon, Nov 23, 2015 at 02:28:29PM +0000, Chris Wilson wrote:
> On Mon, Nov 23, 2015 at 04:22:08PM +0200, Ville Syrjälä wrote:
> > On Sat, Nov 21, 2015 at 10:51:50AM +0000, Chris Wilson wrote:
> > > On Fri, Nov 20, 2015 at 10:09:18PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > We still get spurious pipe underruns on ILK/SNB/IVB under two
> > > > circumstances when dealing with PCH ports:
> > > > * When the pipe has been disabled, but FDI RX/TX is still enabled
> > > > * During FDI link training
> > > > 
> > > > Both cases seem to happen at least when we do VGA+HDMI cloning
> > > > from the same pipe. I don't think I've seen them when not cloning,
> > > > but can't be 100% sure.
> > > > 
> > > > Disable underrun reporting around those places to eliminate the
> > > > dmesg errors.
> > > > 
> > > > Testcase: igt/kms_setmode/basic-clone-single-crtc
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > 
> > > I wondered if logging the suppressed errors would be of any use?
> > 
> > Hmm. Maybe just to confirm that they still happen, and thus suppressing
> > is still neded.
> > 
> > > Does
> > > the check_cpu_fifo work if the reporting is disabled? Could we do a
> > > manual check and DRM_DEBUG_KMS() if the enable did generate a failure.
> > > If the check_cpu_fifo does work, won't we still get the error from the
> > > added check in atomic_commit()?
> > 
> > The check only looks at pipes that have underrun reporting enabled. I
> > suppose it might be possible to have it check all the pipes. At the
> > point where we call it no explicit suppression should be happening, so
> > the only reason why underrun reporting would be disabled on any pipe is
> > due to detecting a previous underrun via the interrupt.
> 
> Does the hw flag the underrun even if the interrupt is disabled?
> (Playing games with PIPESTAT_IER?) Could we then report (debug) that an
> underrun happened before we re-enable reporting?

Hmm. We use the IMR to disable this stuff on many platforms, so we
do lose the IIR. So we'd need to frob IER instead to make this work
universally. Currently it would only work for GMCH, IVB/HSW,
CPT/PPT/LPT/WPT since PIPESTAT/ERR_INT/SERR_INT don't have IMR.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB
  2015-11-23 14:55       ` Ville Syrjälä
@ 2015-11-24 14:09         ` Daniel Vetter
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2015-11-24 14:09 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Mon, Nov 23, 2015 at 04:55:17PM +0200, Ville Syrjälä wrote:
> On Mon, Nov 23, 2015 at 02:28:29PM +0000, Chris Wilson wrote:
> > On Mon, Nov 23, 2015 at 04:22:08PM +0200, Ville Syrjälä wrote:
> > > On Sat, Nov 21, 2015 at 10:51:50AM +0000, Chris Wilson wrote:
> > > > On Fri, Nov 20, 2015 at 10:09:18PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > We still get spurious pipe underruns on ILK/SNB/IVB under two
> > > > > circumstances when dealing with PCH ports:
> > > > > * When the pipe has been disabled, but FDI RX/TX is still enabled
> > > > > * During FDI link training
> > > > > 
> > > > > Both cases seem to happen at least when we do VGA+HDMI cloning
> > > > > from the same pipe. I don't think I've seen them when not cloning,
> > > > > but can't be 100% sure.
> > > > > 
> > > > > Disable underrun reporting around those places to eliminate the
> > > > > dmesg errors.
> > > > > 
> > > > > Testcase: igt/kms_setmode/basic-clone-single-crtc
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > 
> > > > I wondered if logging the suppressed errors would be of any use?
> > > 
> > > Hmm. Maybe just to confirm that they still happen, and thus suppressing
> > > is still neded.
> > > 
> > > > Does
> > > > the check_cpu_fifo work if the reporting is disabled? Could we do a
> > > > manual check and DRM_DEBUG_KMS() if the enable did generate a failure.
> > > > If the check_cpu_fifo does work, won't we still get the error from the
> > > > added check in atomic_commit()?
> > > 
> > > The check only looks at pipes that have underrun reporting enabled. I
> > > suppose it might be possible to have it check all the pipes. At the
> > > point where we call it no explicit suppression should be happening, so
> > > the only reason why underrun reporting would be disabled on any pipe is
> > > due to detecting a previous underrun via the interrupt.
> > 
> > Does the hw flag the underrun even if the interrupt is disabled?
> > (Playing games with PIPESTAT_IER?) Could we then report (debug) that an
> > underrun happened before we re-enable reporting?
> 
> Hmm. We use the IMR to disable this stuff on many platforms, so we
> do lose the IIR. So we'd need to frob IER instead to make this work
> universally. Currently it would only work for GMCH, IVB/HSW,
> CPT/PPT/LPT/WPT since PIPESTAT/ERR_INT/SERR_INT don't have IMR.

Feels a bit too risky imo, changing all that code. Also, even in tests
where underruns reproduce readily it's not 100%, so the occasional debug
message won't be solid enough evidence.

For me I think as long as we hide underruns only while doing modesets
(where there's some expectation of them happening) we should be perfectly
fine wrt test coverage. The real benefit is when enabling/disabling
planes, fbc and all that stuff.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] drm/i915: Check for underruns after crtc disable
  2015-11-23 14:42       ` Chris Wilson
@ 2015-11-24 14:14         ` Daniel Vetter
  2015-11-24 14:21           ` Ville Syrjälä
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2015-11-24 14:14 UTC (permalink / raw)
  To: Chris Wilson, Ville Syrjälä, intel-gfx

On Mon, Nov 23, 2015 at 02:42:21PM +0000, Chris Wilson wrote:
> On Mon, Nov 23, 2015 at 04:10:32PM +0200, Ville Syrjälä wrote:
> > On Sat, Nov 21, 2015 at 10:49:04AM +0000, Chris Wilson wrote:
> > > On Fri, Nov 20, 2015 at 10:09:20PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > To get a better idea if underruns occurred during crtc disabling,
> > > > let's check for them explicitly. This helps in cases where the
> > > > error interrupt isn't active, or there is no underrun interrupt
> > > > support at all.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Would this be better the vblank after enabling?
> > 
> > We do that too.
> 
> Oh, crtc disabling. I can't read.

Hm, should we also double-check before disabling, to catch anything that
happened right before the modeset and avoid confusing it with underruns
happening during the modeset?

But this is a good idea already.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB
  2015-11-20 20:09 [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB ville.syrjala
                   ` (2 preceding siblings ...)
  2015-11-21 10:51 ` [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB Chris Wilson
@ 2015-11-24 14:16 ` Daniel Vetter
  2015-11-24 14:52   ` Ville Syrjälä
  3 siblings, 1 reply; 16+ messages in thread
From: Daniel Vetter @ 2015-11-24 14:16 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Fri, Nov 20, 2015 at 10:09:18PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We still get spurious pipe underruns on ILK/SNB/IVB under two
> circumstances when dealing with PCH ports:
> * When the pipe has been disabled, but FDI RX/TX is still enabled
> * During FDI link training
> 
> Both cases seem to happen at least when we do VGA+HDMI cloning
> from the same pipe. I don't think I've seen them when not cloning,
> but can't be 100% sure.
> 
> Disable underrun reporting around those places to eliminate the
> dmesg errors.
> 
> Testcase: igt/kms_setmode/basic-clone-single-crtc
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

hsw seems to have a similar problem when both vga and hdmi are used, even
when not cloned. Bad thing is that somehow the underrun reporting gets
into a non-recoverable state and stuck there, so that no underrun and also
nothing else (specifically crc irqs, that's why bat notices) work any
more.

Might be worth a shot to try the same trick there. Of course we still need
to figure out why it can't recover from this, too.

Anyway just an aside, ack on the entire series.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 68fb449ded77..8a8104b7947d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4137,6 +4137,12 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>  	I915_WRITE(FDI_RX_TUSIZE1(pipe),
>  		   I915_READ(PIPE_DATA_M1(pipe)) & TU_SIZE_MASK);
>  
> +	/*
> +	 * Sometimes spurious CPU pipe underruns happen during FDI
> +	 * training, at least with VGA+HDMI cloning. Suppress them.
> +	 */
> +	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
> +
>  	/* For PCH output, training FDI link */
>  	dev_priv->display.fdi_link_train(crtc);
>  
> @@ -4170,6 +4176,8 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
>  
>  	intel_fdi_normal_train(crtc);
>  
> +	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> +
>  	/* For PCH DP, enable TRANS_DP_CTL */
>  	if (HAS_PCH_CPT(dev) && intel_crtc->config->has_dp_encoder) {
>  		const struct drm_display_mode *adjusted_mode =
> @@ -5062,12 +5070,22 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
>  	drm_crtc_vblank_off(crtc);
>  	assert_vblank_disabled(crtc);
>  
> +	/*
> +	 * Sometimes spurious CPU pipe underruns happen when the
> +	 * pipe is already disabled, but FDI RX/TX is still enabled.
> +	 * Happens at least with VGA+HDMI cloning. Suppress them.
> +	 */
> +	if (intel_crtc->config->has_pch_encoder)
> +		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
> +
>  	intel_disable_pipe(intel_crtc);
>  
>  	ironlake_pfit_disable(intel_crtc, false);
>  
> -	if (intel_crtc->config->has_pch_encoder)
> +	if (intel_crtc->config->has_pch_encoder) {
>  		ironlake_fdi_disable(crtc);
> +		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> +	}
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
>  		if (encoder->post_disable)
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] drm/i915: Check for underruns after crtc disable
  2015-11-24 14:14         ` Daniel Vetter
@ 2015-11-24 14:21           ` Ville Syrjälä
  0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2015-11-24 14:21 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, Nov 24, 2015 at 03:14:23PM +0100, Daniel Vetter wrote:
> On Mon, Nov 23, 2015 at 02:42:21PM +0000, Chris Wilson wrote:
> > On Mon, Nov 23, 2015 at 04:10:32PM +0200, Ville Syrjälä wrote:
> > > On Sat, Nov 21, 2015 at 10:49:04AM +0000, Chris Wilson wrote:
> > > > On Fri, Nov 20, 2015 at 10:09:20PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > To get a better idea if underruns occurred during crtc disabling,
> > > > > let's check for them explicitly. This helps in cases where the
> > > > > error interrupt isn't active, or there is no underrun interrupt
> > > > > support at all.
> > > > > 
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Would this be better the vblank after enabling?
> > > 
> > > We do that too.
> > 
> > Oh, crtc disabling. I can't read.
> 
> Hm, should we also double-check before disabling, to catch anything that
> happened right before the modeset and avoid confusing it with underruns
> happening during the modeset?

Hmm. Yeah that might be a decent idea. Although we don't wait for the
planes to be disabled before we disable the pipe so in theory some weird
underrun stemming from the plane disable might still get mixed in there.

> 
> But this is a good idea already.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB
  2015-11-24 14:16 ` Daniel Vetter
@ 2015-11-24 14:52   ` Ville Syrjälä
  0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2015-11-24 14:52 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, Nov 24, 2015 at 03:16:24PM +0100, Daniel Vetter wrote:
> On Fri, Nov 20, 2015 at 10:09:18PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We still get spurious pipe underruns on ILK/SNB/IVB under two
> > circumstances when dealing with PCH ports:
> > * When the pipe has been disabled, but FDI RX/TX is still enabled
> > * During FDI link training
> > 
> > Both cases seem to happen at least when we do VGA+HDMI cloning
> > from the same pipe. I don't think I've seen them when not cloning,
> > but can't be 100% sure.
> > 
> > Disable underrun reporting around those places to eliminate the
> > dmesg errors.
> > 
> > Testcase: igt/kms_setmode/basic-clone-single-crtc
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> hsw seems to have a similar problem when both vga and hdmi are used, even
> when not cloned. Bad thing is that somehow the underrun reporting gets
> into a non-recoverable state and stuck there, so that no underrun and also
> nothing else (specifically crc irqs, that's why bat notices) work any
> more.
> 
> Might be worth a shot to try the same trick there. Of course we still need
> to figure out why it can't recover from this, too.

I don't have a HSW/BDW with VGA on me. I suppose I might need to go
find one and see what's what.

Series pushed to dinq. Thanks for the acks and reviews.

> 
> Anyway just an aside, ack on the entire series.
> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 20 +++++++++++++++++++-
> >  1 file changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 68fb449ded77..8a8104b7947d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4137,6 +4137,12 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
> >  	I915_WRITE(FDI_RX_TUSIZE1(pipe),
> >  		   I915_READ(PIPE_DATA_M1(pipe)) & TU_SIZE_MASK);
> >  
> > +	/*
> > +	 * Sometimes spurious CPU pipe underruns happen during FDI
> > +	 * training, at least with VGA+HDMI cloning. Suppress them.
> > +	 */
> > +	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
> > +
> >  	/* For PCH output, training FDI link */
> >  	dev_priv->display.fdi_link_train(crtc);
> >  
> > @@ -4170,6 +4176,8 @@ static void ironlake_pch_enable(struct drm_crtc *crtc)
> >  
> >  	intel_fdi_normal_train(crtc);
> >  
> > +	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> > +
> >  	/* For PCH DP, enable TRANS_DP_CTL */
> >  	if (HAS_PCH_CPT(dev) && intel_crtc->config->has_dp_encoder) {
> >  		const struct drm_display_mode *adjusted_mode =
> > @@ -5062,12 +5070,22 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
> >  	drm_crtc_vblank_off(crtc);
> >  	assert_vblank_disabled(crtc);
> >  
> > +	/*
> > +	 * Sometimes spurious CPU pipe underruns happen when the
> > +	 * pipe is already disabled, but FDI RX/TX is still enabled.
> > +	 * Happens at least with VGA+HDMI cloning. Suppress them.
> > +	 */
> > +	if (intel_crtc->config->has_pch_encoder)
> > +		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
> > +
> >  	intel_disable_pipe(intel_crtc);
> >  
> >  	ironlake_pfit_disable(intel_crtc, false);
> >  
> > -	if (intel_crtc->config->has_pch_encoder)
> > +	if (intel_crtc->config->has_pch_encoder) {
> >  		ironlake_fdi_disable(crtc);
> > +		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
> > +	}
> >  
> >  	for_each_encoder_on_crtc(dev, crtc, encoder)
> >  		if (encoder->post_disable)
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-11-24 14:52 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-20 20:09 [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB ville.syrjala
2015-11-20 20:09 ` [PATCH 2/3] drm/i915: Disable CPU underruns around eDP port and vdd enable " ville.syrjala
2015-11-21 10:52   ` Chris Wilson
2015-11-20 20:09 ` [PATCH 3/3] drm/i915: Check for underruns after crtc disable ville.syrjala
2015-11-21 10:49   ` Chris Wilson
2015-11-23 14:10     ` Ville Syrjälä
2015-11-23 14:42       ` Chris Wilson
2015-11-24 14:14         ` Daniel Vetter
2015-11-24 14:21           ` Ville Syrjälä
2015-11-21 10:51 ` [PATCH 1/3] drm/i915: Suppress spurious CPU FIFO underruns on ILK-IVB Chris Wilson
2015-11-23 14:22   ` Ville Syrjälä
2015-11-23 14:28     ` Chris Wilson
2015-11-23 14:55       ` Ville Syrjälä
2015-11-24 14:09         ` Daniel Vetter
2015-11-24 14:16 ` Daniel Vetter
2015-11-24 14:52   ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox