[RFC 0/3] drm/i915: expose fifo pipe underrun counts

[RFC 0/3] drm/i915: expose fifo pipe underrun counts

[Joe Konno]

From: Joe Konno <joe.konno@intel.com>

In tracking down a watermark bug, I discovered the pch and cpu underrun
interrupt handlers would disable themselves after initial reports to prevent an
interrupt/dmesg storm. Storms are bad, but underrun interrupt handling should
not cease. For my case, I need to be able to count pch and cpu underruns for
each pipe or transcoder. Displaying this information in the 'i915_display_info'
node seemed the best course of action.

In order to do this, however, I had to revisit some long-standing behaviors in
the underrun interrupt handlers. One problem became three. Thanks in advance
for your review and feedback.

Requesting comment on the following solutions I came up with (corresponding to
each patch in the series):

  1. provide simple 'getter' mechanisms for pch and cpu underrun reporting
     ("is it enabled?")-- and base dmesg output on the answer to that question;

  2. don't allow the interrupt handlers to disable or filter themselves (and
     prevent accurate counting); and finally

  3. atomically-incremented pch and cpu underrun counters, with those counters
     displayed in debugfs i915_display_info per-pipe, per-transcoder

For: https://bugs.freedesktop.org/show_bug.cgi?id=93865


Joe Konno (3):
  drm/i915: get cpu, pch underrun reporting state
  drm/i915: do not disable handler after underrun
  drm/i915: add underrun counts to i915_display_info

 drivers/gpu/drm/i915/i915_debugfs.c        |  6 ++-
 drivers/gpu/drm/i915/intel_drv.h           |  4 ++
 drivers/gpu/drm/i915/intel_fifo_underrun.c | 78 ++++++++++++++++++++++++------
 3 files changed, 71 insertions(+), 17 deletions(-)

-- 
2.6.4

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

[RFC 1/3] drm/i915: get cpu, pch underrun reporting state

[Joe Konno]

From: Joe Konno <joe.konno@intel.com>

There are mechanisms for "set and return previous" underrun reporting
state, but no convenience functions for simply getting the underrun
reporting state for a particular pipe or pch transcoder.

Signed-off-by: Joe Konno <joe.konno@intel.com>
---
 drivers/gpu/drm/i915/intel_fifo_underrun.c | 43 ++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index bda526660e20..9516bd416226 100644
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -297,6 +297,27 @@ bool intel_set_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
 }
 
 /**
+ * intel_get_cpu_fifo_underrun_reporting - get cpu fifo underrrun reporting state
+ * @dev_priv: i915 device instance
+ * @pipe: (CPU) pipe to get state for
+ *
+ * This function gets the fifo underrun reporting state for @pipe.
+ *
+ * Returns true if underrun reporting is enabled for @pipe.
+ */
+bool intel_get_cpu_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
+					   enum pipe pipe)
+{
+	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	bool ret;
+
+	ret = !intel_crtc->cpu_fifo_underrun_disabled;
+
+	return ret;
+}
+
+/**
  * intel_set_pch_fifo_underrun_reporting - set PCH fifo underrun reporting state
  * @dev_priv: i915 device instance
  * @pch_transcoder: the PCH transcoder (same as pipe on IVB and older)
@@ -345,6 +366,28 @@ bool intel_set_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
 }
 
 /**
+ * intel_get_pch_fifo_underrun_reporting - get PCH fifo underrun reporting state
+ * @dev_priv: i915 device instance
+ * @pch_transcoder: the PCH transcoder (same as pipe on IVB and older)
+ *
+ * This function shows whether PCH fifo underrun reporting is enabled for
+ * @pch_transcoder.
+ *
+ * Returns true if PCH underrun reporting is enabled for @pch_transcoder.
+ */
+bool intel_get_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
+					   enum transcoder pch_transcoder)
+{
+	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pch_transcoder];
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	bool ret;
+
+	ret = !intel_crtc->pch_fifo_underrun_disabled;
+
+	return ret;
+}
+
+/**
  * intel_cpu_fifo_underrun_irq_handler - handle CPU fifo underrun interrupt
  * @dev_priv: i915 device instance
  * @pipe: (CPU) pipe to set state for
-- 
2.6.4

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

[RFC 2/3] drm/i915: do not disable handler after underrun

[Joe Konno]

From: Joe Konno <joe.konno@intel.com>

Previously, cpu and pch underrun reporting was done on a "report once
and disable" basis to prevent interrupt/dmesg floods.

With this change, do not disable the underrun interrupt handlers.
Instead, use throttled (DRM_ERROR_RATELIMITED) dmesg output if dmesg
underrun reporting enabled.

Signed-off-by: Joe Konno <joe.konno@intel.com>
---
 drivers/gpu/drm/i915/intel_fifo_underrun.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index 9516bd416226..4a1a2781fc5e 100644
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -393,26 +393,23 @@ bool intel_get_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
  * @pipe: (CPU) pipe to set state for
  *
  * This handles a CPU fifo underrun interrupt, generating an underrun warning
- * into dmesg if underrun reporting is enabled and then disables the underrun
- * interrupt to avoid an irq storm.
+ * into dmesg if underrun reporting is enabled.
  */
 void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
 					 enum pipe pipe)
 {
 	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+	bool report;
 
 	/* We may be called too early in init, thanks BIOS! */
 	if (crtc == NULL)
 		return;
 
-	/* GMCH can't disable fifo underruns, filter them. */
-	if (HAS_GMCH_DISPLAY(dev_priv->dev) &&
-	    to_intel_crtc(crtc)->cpu_fifo_underrun_disabled)
-		return;
+	report = intel_get_cpu_fifo_underrun_reporting(dev_priv, pipe);
 
-	if (intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false))
-		DRM_ERROR("CPU pipe %c FIFO underrun\n",
-			  pipe_name(pipe));
+	if (report)
+		DRM_ERROR_RATELIMITED("CPU pipe %c FIFO underrun\n",
+				      pipe_name(pipe));
 }
 
 /**
@@ -421,16 +418,18 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
  * @pch_transcoder: the PCH transcoder (same as pipe on IVB and older)
  *
  * This handles a PCH fifo underrun interrupt, generating an underrun warning
- * into dmesg if underrun reporting is enabled and then disables the underrun
- * interrupt to avoid an irq storm.
+ * into dmesg if underrun reporting is enabled.
  */
 void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
 					 enum transcoder pch_transcoder)
 {
-	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
-						  false))
-		DRM_ERROR("PCH transcoder %c FIFO underrun\n",
-			  transcoder_name(pch_transcoder));
+	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pch_transcoder];
+	bool report = intel_get_pch_fifo_underrun_reporting(dev_priv,
+							    pch_transcoder);
+
+	if (report)
+		DRM_ERROR_RATELIMITED("PCH transcoder %c FIFO underrun\n",
+				      transcoder_name(pch_transcoder));
 }
 
 /**
-- 
2.6.4

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

Re: [RFC 2/3] drm/i915: do not disable handler after underrun

[Daniel Vetter]

On Tue, Jan 26, 2016 at 11:36:56AM -0800, Joe Konno wrote:
> From: Joe Konno <joe.konno@intel.com>
> 
> Previously, cpu and pch underrun reporting was done on a "report once
> and disable" basis to prevent interrupt/dmesg floods.
> 
> With this change, do not disable the underrun interrupt handlers.
> Instead, use throttled (DRM_ERROR_RATELIMITED) dmesg output if dmesg
> underrun reporting enabled.
> 
> Signed-off-by: Joe Konno <joe.konno@intel.com>

This is too simple unfortunately. In many cases the underrun is a one-off
right when scan-out starts and then stops again, until something changes
again. This is the typical failure mode for non-atomic wm update and
similar stuff.

But it's also possible that the pipe wedges entirely and the underrun is
stuck. Then you'll end up doing nothing else than serving fifo underruns,
with no cpu time left for anything else. This kills your box.

So if you want to keep track of a count of underruns the right way is to
disable it when it happens, but then schedule re-enabling for the next
vblank. Also I think we shouldn't spam dmesg more than we do already, so
instead of ratelimiting I'd just keep a counter after the first one
happened, before the next modeset.

Another really annoying thing with underruns: On some platforms the irq
control bits are shared between them all, which means you can't disable
them individually. If you underrun on one pipe you also have to disable it
for all the others.

In short: This is all a pretty big mess :(
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_fifo_underrun.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> index 9516bd416226..4a1a2781fc5e 100644
> --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> @@ -393,26 +393,23 @@ bool intel_get_pch_fifo_underrun_reporting(struct drm_i915_private *dev_priv,
>   * @pipe: (CPU) pipe to set state for
>   *
>   * This handles a CPU fifo underrun interrupt, generating an underrun warning
> - * into dmesg if underrun reporting is enabled and then disables the underrun
> - * interrupt to avoid an irq storm.
> + * into dmesg if underrun reporting is enabled.
>   */
>  void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
>  					 enum pipe pipe)
>  {
>  	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> +	bool report;
>  
>  	/* We may be called too early in init, thanks BIOS! */
>  	if (crtc == NULL)
>  		return;
>  
> -	/* GMCH can't disable fifo underruns, filter them. */
> -	if (HAS_GMCH_DISPLAY(dev_priv->dev) &&
> -	    to_intel_crtc(crtc)->cpu_fifo_underrun_disabled)
> -		return;
> +	report = intel_get_cpu_fifo_underrun_reporting(dev_priv, pipe);
>  
> -	if (intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false))
> -		DRM_ERROR("CPU pipe %c FIFO underrun\n",
> -			  pipe_name(pipe));
> +	if (report)
> +		DRM_ERROR_RATELIMITED("CPU pipe %c FIFO underrun\n",
> +				      pipe_name(pipe));
>  }
>  
>  /**
> @@ -421,16 +418,18 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
>   * @pch_transcoder: the PCH transcoder (same as pipe on IVB and older)
>   *
>   * This handles a PCH fifo underrun interrupt, generating an underrun warning
> - * into dmesg if underrun reporting is enabled and then disables the underrun
> - * interrupt to avoid an irq storm.
> + * into dmesg if underrun reporting is enabled.
>   */
>  void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
>  					 enum transcoder pch_transcoder)
>  {
> -	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
> -						  false))
> -		DRM_ERROR("PCH transcoder %c FIFO underrun\n",
> -			  transcoder_name(pch_transcoder));
> +	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pch_transcoder];
> +	bool report = intel_get_pch_fifo_underrun_reporting(dev_priv,
> +							    pch_transcoder);
> +
> +	if (report)
> +		DRM_ERROR_RATELIMITED("PCH transcoder %c FIFO underrun\n",
> +				      transcoder_name(pch_transcoder));
>  }
>  
>  /**
> -- 
> 2.6.4
> 
> _______________________________________________
> 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

[RFC 3/3] drm/i915: add underrun counts to i915_display_info

[Joe Konno]

From: Joe Konno <joe.konno@intel.com>

Add atomic cpu and pch underrun counters to struct intel_crtc, and
increment them each time their respective interrupt handler executes.

Whether dmesg underrun reporting is disabled or not, keep a count of cpu
and pch underrun interrupts. Display these counts in i915_display_info
on a per-pipe. per-transcoder basis to assist debugging and testing.

In debugfs, the i915_display_info output for each pipe changes from
this:
    underrun reporting: cpu=%s pch=%s

to this, where the number of underruns are reported in parentheses:
    underrun reporting: cpu=%s (%d) pch=%s (%d)

Signed-off-by: Joe Konno <joe.konno@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 6 ++++--
 drivers/gpu/drm/i915/intel_drv.h           | 4 ++++
 drivers/gpu/drm/i915/intel_fifo_underrun.c | 6 ++++++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 863012a2602e..aa47987bb13b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3101,9 +3101,11 @@ static int i915_display_info(struct seq_file *m, void *unused)
 			intel_plane_info(m, crtc);
 		}
 
-		seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s \n",
+		seq_printf(m, "\tunderrun reporting: cpu=%s (%d) pch=%s (%d) \n",
 			   yesno(!crtc->cpu_fifo_underrun_disabled),
-			   yesno(!crtc->pch_fifo_underrun_disabled));
+			   atomic_read(&crtc->cpu_fifo_underrun_count),
+			   yesno(!crtc->pch_fifo_underrun_disabled),
+			   atomic_read(&crtc->pch_fifo_underrun_count));
 	}
 
 	seq_printf(m, "\n");
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bc970125ec76..efc6eb0a517f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -589,6 +589,10 @@ struct intel_crtc {
 	/* reset counter value when the last flip was submitted */
 	unsigned int reset_counter;
 
+	/* fifo_underrun counters, used in debugfs only */
+	atomic_t cpu_fifo_underrun_count;
+	atomic_t pch_fifo_underrun_count;
+
 	/* Access to these should be protected by dev_priv->irq_lock. */
 	bool cpu_fifo_underrun_disabled;
 	bool pch_fifo_underrun_disabled;
diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index 4a1a2781fc5e..d7b355b831e1 100644
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -407,6 +407,9 @@ void intel_cpu_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
 
 	report = intel_get_cpu_fifo_underrun_reporting(dev_priv, pipe);
 
+	/* Increment the debugfs underrun counter */
+	atomic_inc(&to_intel_crtc(crtc)->cpu_fifo_underrun_count);
+
 	if (report)
 		DRM_ERROR_RATELIMITED("CPU pipe %c FIFO underrun\n",
 				      pipe_name(pipe));
@@ -427,6 +430,9 @@ void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
 	bool report = intel_get_pch_fifo_underrun_reporting(dev_priv,
 							    pch_transcoder);
 
+	/* Increment the debugfs underrun counter */
+	atomic_inc(&to_intel_crtc(crtc)->pch_fifo_underrun_count);
+
 	if (report)
 		DRM_ERROR_RATELIMITED("PCH transcoder %c FIFO underrun\n",
 				      transcoder_name(pch_transcoder));
-- 
2.6.4

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

Re: [RFC 0/3] drm/i915: expose fifo pipe underrun counts

[Daniel Vetter]

On Tue, Jan 26, 2016 at 11:36:54AM -0800, Joe Konno wrote:
> From: Joe Konno <joe.konno@intel.com>
> 
> In tracking down a watermark bug, I discovered the pch and cpu underrun
> interrupt handlers would disable themselves after initial reports to prevent an
> interrupt/dmesg storm. Storms are bad, but underrun interrupt handling should
> not cease. For my case, I need to be able to count pch and cpu underruns for
> each pipe or transcoder. Displaying this information in the 'i915_display_info'
> node seemed the best course of action.
> 
> In order to do this, however, I had to revisit some long-standing behaviors in
> the underrun interrupt handlers. One problem became three. Thanks in advance
> for your review and feedback.
> 
> Requesting comment on the following solutions I came up with (corresponding to
> each patch in the series):
> 
>   1. provide simple 'getter' mechanisms for pch and cpu underrun reporting
>      ("is it enabled?")-- and base dmesg output on the answer to that question;
> 
>   2. don't allow the interrupt handlers to disable or filter themselves (and
>      prevent accurate counting); and finally
> 
>   3. atomically-incremented pch and cpu underrun counters, with those counters
>      displayed in debugfs i915_display_info per-pipe, per-transcoder
> 
> For: https://bugs.freedesktop.org/show_bug.cgi?id=93865

It's more complicated than this, replied with the technicalities to patch
2. But what I've forgotten to ask: What do you want to use this for? We
make sure that after a full modeset underrun reporting state is restored,
so you can retest for a given bug essentially forever, with no need to
reboot.

And we've also become a lot better at fixing the existing underruns, so
nowadays fifo reporting won't be disable right away even before you
managed to display the very first frame.
-Daniel

> 
> 
> Joe Konno (3):
>   drm/i915: get cpu, pch underrun reporting state
>   drm/i915: do not disable handler after underrun
>   drm/i915: add underrun counts to i915_display_info
> 
>  drivers/gpu/drm/i915/i915_debugfs.c        |  6 ++-
>  drivers/gpu/drm/i915/intel_drv.h           |  4 ++
>  drivers/gpu/drm/i915/intel_fifo_underrun.c | 78 ++++++++++++++++++++++++------
>  3 files changed, 71 insertions(+), 17 deletions(-)
> 
> -- 
> 2.6.4
> 
> _______________________________________________
> 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

Re: [RFC 0/3] drm/i915: expose fifo pipe underrun counts

[Joe Konno]

On 01/26/2016 12:51 PM, Daniel Vetter wrote:
> On Tue, Jan 26, 2016 at 11:36:54AM -0800, Joe Konno wrote:
>> From: Joe Konno <joe.konno@intel.com>
>>
>> In tracking down a watermark bug, I discovered the pch and cpu underrun
>> interrupt handlers would disable themselves after initial reports to prevent an
>> interrupt/dmesg storm. Storms are bad, but underrun interrupt handling should
>> not cease. For my case, I need to be able to count pch and cpu underruns for
>> each pipe or transcoder. Displaying this information in the 'i915_display_info'
>> node seemed the best course of action.
>>
>> In order to do this, however, I had to revisit some long-standing behaviors in
>> the underrun interrupt handlers. One problem became three. Thanks in advance
>> for your review and feedback.
>>
>> Requesting comment on the following solutions I came up with (corresponding to
>> each patch in the series):
>>
>>   1. provide simple 'getter' mechanisms for pch and cpu underrun reporting
>>      ("is it enabled?")-- and base dmesg output on the answer to that question;
>>
>>   2. don't allow the interrupt handlers to disable or filter themselves (and
>>      prevent accurate counting); and finally
>>
>>   3. atomically-incremented pch and cpu underrun counters, with those counters
>>      displayed in debugfs i915_display_info per-pipe, per-transcoder
>>
>> For: https://bugs.freedesktop.org/show_bug.cgi?id=93865
> 
> It's more complicated than this, replied with the technicalities to patch
> 2. But what I've forgotten to ask: What do you want to use this for? We
> make sure that after a full modeset underrun reporting state is restored,
> so you can retest for a given bug essentially forever, with no need to
> reboot.

I see a correlation between pipe underruns and display flicker-- that's
the particular case I'm working presently, so an underrun counter (in
whatever form) is extremely useful. Assuming, of course, I'm barking up
the right tree.

Such a counter seems a reliable metric for expressing "how bad"
insert-display-issue-here is. Or, "how much better/worse" said issue is
after hacking or applying patch(es).

> And we've also become a lot better at fixing the existing underruns, so
> nowadays fifo reporting won't be disable right away even before you
> managed to display the very first frame.
> -Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC 0/3] drm/i915: expose fifo pipe underrun counts

[Daniel Vetter]

On Tue, Jan 26, 2016 at 01:05:28PM -0800, Joe Konno wrote:
> On 01/26/2016 12:51 PM, Daniel Vetter wrote:
> > On Tue, Jan 26, 2016 at 11:36:54AM -0800, Joe Konno wrote:
> >> From: Joe Konno <joe.konno@intel.com>
> >>
> >> In tracking down a watermark bug, I discovered the pch and cpu underrun
> >> interrupt handlers would disable themselves after initial reports to prevent an
> >> interrupt/dmesg storm. Storms are bad, but underrun interrupt handling should
> >> not cease. For my case, I need to be able to count pch and cpu underruns for
> >> each pipe or transcoder. Displaying this information in the 'i915_display_info'
> >> node seemed the best course of action.
> >>
> >> In order to do this, however, I had to revisit some long-standing behaviors in
> >> the underrun interrupt handlers. One problem became three. Thanks in advance
> >> for your review and feedback.
> >>
> >> Requesting comment on the following solutions I came up with (corresponding to
> >> each patch in the series):
> >>
> >>   1. provide simple 'getter' mechanisms for pch and cpu underrun reporting
> >>      ("is it enabled?")-- and base dmesg output on the answer to that question;
> >>
> >>   2. don't allow the interrupt handlers to disable or filter themselves (and
> >>      prevent accurate counting); and finally
> >>
> >>   3. atomically-incremented pch and cpu underrun counters, with those counters
> >>      displayed in debugfs i915_display_info per-pipe, per-transcoder
> >>
> >> For: https://bugs.freedesktop.org/show_bug.cgi?id=93865
> > 
> > It's more complicated than this, replied with the technicalities to patch
> > 2. But what I've forgotten to ask: What do you want to use this for? We
> > make sure that after a full modeset underrun reporting state is restored,
> > so you can retest for a given bug essentially forever, with no need to
> > reboot.
> 
> I see a correlation between pipe underruns and display flicker-- that's
> the particular case I'm working presently, so an underrun counter (in
> whatever form) is extremely useful. Assuming, of course, I'm barking up
> the right tree.
> 
> Such a counter seems a reliable metric for expressing "how bad"
> insert-display-issue-here is. Or, "how much better/worse" said issue is
> after hacking or applying patch(es).

Yeah, display underruns usually cause flicker (sometimes just so small you
can't see it any more), in bad cases also complete loss of link sync and
resulting bad screen. Any kind of fifo underrun is a bug really, and CI
treats them as such.

Imo the right approach here would be to periodically re-enable underruns.
The problem with that is synchronizing with modesets - enabling/disabling
the display pipe is known/expected to cause underruns, so that re-enable
needs to sync with modesets. And with atomic those can be async, which
will make this all a pain. But I think even that should be opt-in to avoid
spamming dmesg too badly.

Another option would be to add a debugfs interface to re-enable fifo
reporting for every pipe. Would that work for your use-case of
experimenting around with watermark issues?

Or do you really want to have a measure for how badly you're underrunning
(continuously, or only sporadically), which your patch here gives? If
once per vblank isn't good enough, or re-arming isn't good enough then
we'd indeed need something like your patch series does, but it must be
opt-in somehow. Also we need to make sure that we don't accidentally
ratelimit underruns right after modesets (where they tend to happen most
often). Doing that means tests fail less often, which causes lots of
trouble with unstable tests in CI.
-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

✗ Fi.CI.BAT: failure for drm/i915: expose fifo pipe underrun counts

[Patchwork]

== Summary ==

Built on 430706bace599ea1a908b9a7c6b7ea17535fe17f drm-intel-nightly: 2016y-01m-27d-16h-33m-06s UTC integration manifest

Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (skl-i5k-2)
        Subgroup read-crc-pipe-b:
                dmesg-warn -> PASS       (ilk-hp8440p)

bdw-nuci7        total:141  pass:132  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:144  pass:138  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:144  pass:120  dwarn:0   dfail:0   fail:0   skip:24 
byt-nuc          total:144  pass:129  dwarn:0   dfail:0   fail:0   skip:15 
hsw-brixbox      total:144  pass:137  dwarn:0   dfail:0   fail:0   skip:7  
ilk-hp8440p      total:144  pass:105  dwarn:0   dfail:0   fail:1   skip:38 
ivb-t430s        total:144  pass:138  dwarn:0   dfail:0   fail:0   skip:6  
skl-i5k-2        total:144  pass:134  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps      total:144  pass:130  dwarn:0   dfail:0   fail:0   skip:14 
snb-x220t        total:144  pass:130  dwarn:0   dfail:0   fail:1   skip:13 

HANGED hsw-gt2 in igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c

Results at /archive/results/CI_IGT_test/Patchwork_1270/

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

✓ Fi.CI.BAT: success for drm/i915: expose fifo pipe underrun counts

[Patchwork]

== Summary ==

Series 2852v1 drm/i915: expose fifo pipe underrun counts
http://patchwork.freedesktop.org/api/1.0/series/2852/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE

bdw-nuci7        total:156  pass:147  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:159  pass:153  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:159  pass:135  dwarn:0   dfail:0   fail:0   skip:24 
byt-nuc          total:159  pass:142  dwarn:0   dfail:0   fail:0   skip:17 
hsw-brixbox      total:159  pass:152  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:159  pass:155  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:159  pass:113  dwarn:1   dfail:0   fail:1   skip:44 
ivb-t430s        total:159  pass:151  dwarn:0   dfail:0   fail:0   skip:8  
skl-i5k-2        total:159  pass:150  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps      total:159  pass:141  dwarn:0   dfail:0   fail:0   skip:18 
snb-x220t        total:159  pass:141  dwarn:0   dfail:0   fail:1   skip:17 

Results at /archive/results/CI_IGT_test/Patchwork_1299/

b3f8ad64bc71f6236f05c2e9f4ad49a61745869a drm-intel-nightly: 2016y-01m-28d-10h-26m-23s UTC integration manifest
0e21d40055530fd0784652968c23b8ac0f84462d drm/i915: add underrun counts to i915_display_info
9656e9897f3561a92ad00908ee3144c5a46bb5ae drm/i915: do not disable handler after underrun
50805ac7a71a7d6fc40ccf5dd6c9dbfeb84db3ac drm/i915: get cpu, pch underrun reporting state

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