Re: [Intel-gfx] [PATCH] drm/i915/display: Add a debugfs entry for fifo underruns

2023-03-14 Thread Swati Sharma

Thanks Andi and Jani N for the review comments.
Sorry for the delay.
Will send the next rev soon.

On 14-Feb-23 5:55 PM, Jani Nikula wrote:

On Wed, 08 Feb 2023, Andi Shyti  wrote:

Hi Swati,

[...]


+static void intel_fifo_underrun_inc_count(struct intel_crtc *crtc,
+ bool is_cpu_fifo)


I'm not a big fan of the true/false parameters in functions. I
actually hate them because it's never clear from the caller what
the true/false means.

Isn't it clear to just have some wrappers

#define intel_fifo_underrun_inc_cpu_count(...)
#define intel_fifo_underrun_inc_cph_count(...)

or similar?


+{
+#ifdef CONFIG_DEBUG_FS
+   if (is_cpu_fifo)
+   crtc->cpu_fifo_underrun_count++;
+   else
+   crtc->pch_fifo_underrun_count++;
+#endif
+}
+
  static void i9xx_check_fifo_underruns(struct intel_crtc *crtc)
  {
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -103,6 +114,7 @@ static void i9xx_check_fifo_underruns(struct intel_crtc 
*crtc)
intel_de_write(dev_priv, reg, enable_mask | PIPE_FIFO_UNDERRUN_STATUS);
intel_de_posting_read(dev_priv, reg);
  
+	intel_fifo_underrun_inc_count(crtc, true);

trace_intel_cpu_fifo_underrun(dev_priv, crtc->pipe);
drm_err(_priv->drm, "pipe %c underrun\n", pipe_name(crtc->pipe));
  }
@@ -156,6 +168,7 @@ static void ivb_check_fifo_underruns(struct intel_crtc 
*crtc)
intel_de_write(dev_priv, GEN7_ERR_INT, ERR_INT_FIFO_UNDERRUN(pipe));
intel_de_posting_read(dev_priv, GEN7_ERR_INT);
  
+	intel_fifo_underrun_inc_count(crtc, true);

trace_intel_cpu_fifo_underrun(dev_priv, pipe);
drm_err(_priv->drm, "fifo underrun on pipe %c\n", pipe_name(pipe));
  }
@@ -244,6 +257,7 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc 
*crtc)
   SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder));
intel_de_posting_read(dev_priv, SERR_INT);
  
+	intel_fifo_underrun_inc_count(crtc, false);

trace_intel_pch_fifo_underrun(dev_priv, pch_transcoder);
drm_err(_priv->drm, "pch fifo underrun on pch transcoder %c\n",
pipe_name(pch_transcoder));
@@ -286,6 +300,11 @@ static bool __intel_set_cpu_fifo_underrun_reporting(struct 
drm_device *dev,
  
  	old = !crtc->cpu_fifo_underrun_disabled;

crtc->cpu_fifo_underrun_disabled = !enable;
+#ifdef CONFIG_DEBUG_FS
+   /* don't reset count in fifo underrun irq path */
+   if (!in_irq() && !enable)
+   crtc->cpu_fifo_underrun_count = 0;
+#endif
  
  	if (HAS_GMCH(dev_priv))

i9xx_set_fifo_underrun_reporting(dev, pipe, enable, old);
@@ -365,6 +384,11 @@ bool intel_set_pch_fifo_underrun_reporting(struct 
drm_i915_private *dev_priv,
  
  	old = !crtc->pch_fifo_underrun_disabled;

crtc->pch_fifo_underrun_disabled = !enable;
+#ifdef CONFIG_DEBUG_FS
+   /* don't reset count in fifo underrun irq path */
+   if (!in_irq() && !enable)
+   crtc->pch_fifo_underrun_count = 0;
+#endif


All these ifdefs are a bit ugly. Can we put all these stuff
inside the debugfs.c file that is compiled only if DEBUG_FS is
configured?


The opposite, the debugfs should be added in this file instead. :)

See my reply.

BR,
Jani.






Andi

  
  	if (HAS_PCH_IBX(dev_priv))

ibx_set_fifo_underrun_reporting(_priv->drm,
@@ -434,6 +458,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct 
drm_i915_private *dev_priv,
drm_err(_priv->drm, "CPU pipe %c FIFO underrun\n", 
pipe_name(pipe));
}
  
+	intel_fifo_underrun_inc_count(crtc, true);

intel_fbc_handle_fifo_underrun_irq(dev_priv);
  }




--
~Swati Sharma


Re: [Intel-gfx] [PATCH] drm/i915/display: Add a debugfs entry for fifo underruns

2023-02-14 Thread Jani Nikula
On Wed, 08 Feb 2023, Andi Shyti  wrote:
> Hi Swati,
>
> [...]
>
>> +static void intel_fifo_underrun_inc_count(struct intel_crtc *crtc,
>> +  bool is_cpu_fifo)
>
> I'm not a big fan of the true/false parameters in functions. I
> actually hate them because it's never clear from the caller what
> the true/false means.
>
> Isn't it clear to just have some wrappers
>
> #define intel_fifo_underrun_inc_cpu_count(...)
> #define intel_fifo_underrun_inc_cph_count(...)
>
> or similar?
>
>> +{
>> +#ifdef CONFIG_DEBUG_FS
>> +if (is_cpu_fifo)
>> +crtc->cpu_fifo_underrun_count++;
>> +else
>> +crtc->pch_fifo_underrun_count++;
>> +#endif
>> +}
>> +
>>  static void i9xx_check_fifo_underruns(struct intel_crtc *crtc)
>>  {
>>  struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> @@ -103,6 +114,7 @@ static void i9xx_check_fifo_underruns(struct intel_crtc 
>> *crtc)
>>  intel_de_write(dev_priv, reg, enable_mask | PIPE_FIFO_UNDERRUN_STATUS);
>>  intel_de_posting_read(dev_priv, reg);
>>  
>> +intel_fifo_underrun_inc_count(crtc, true);
>>  trace_intel_cpu_fifo_underrun(dev_priv, crtc->pipe);
>>  drm_err(_priv->drm, "pipe %c underrun\n", pipe_name(crtc->pipe));
>>  }
>> @@ -156,6 +168,7 @@ static void ivb_check_fifo_underruns(struct intel_crtc 
>> *crtc)
>>  intel_de_write(dev_priv, GEN7_ERR_INT, ERR_INT_FIFO_UNDERRUN(pipe));
>>  intel_de_posting_read(dev_priv, GEN7_ERR_INT);
>>  
>> +intel_fifo_underrun_inc_count(crtc, true);
>>  trace_intel_cpu_fifo_underrun(dev_priv, pipe);
>>  drm_err(_priv->drm, "fifo underrun on pipe %c\n", pipe_name(pipe));
>>  }
>> @@ -244,6 +257,7 @@ static void cpt_check_pch_fifo_underruns(struct 
>> intel_crtc *crtc)
>> SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder));
>>  intel_de_posting_read(dev_priv, SERR_INT);
>>  
>> +intel_fifo_underrun_inc_count(crtc, false);
>>  trace_intel_pch_fifo_underrun(dev_priv, pch_transcoder);
>>  drm_err(_priv->drm, "pch fifo underrun on pch transcoder %c\n",
>>  pipe_name(pch_transcoder));
>> @@ -286,6 +300,11 @@ static bool 
>> __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
>>  
>>  old = !crtc->cpu_fifo_underrun_disabled;
>>  crtc->cpu_fifo_underrun_disabled = !enable;
>> +#ifdef CONFIG_DEBUG_FS
>> +/* don't reset count in fifo underrun irq path */
>> +if (!in_irq() && !enable)
>> +crtc->cpu_fifo_underrun_count = 0;
>> +#endif
>>  
>>  if (HAS_GMCH(dev_priv))
>>  i9xx_set_fifo_underrun_reporting(dev, pipe, enable, old);
>> @@ -365,6 +384,11 @@ bool intel_set_pch_fifo_underrun_reporting(struct 
>> drm_i915_private *dev_priv,
>>  
>>  old = !crtc->pch_fifo_underrun_disabled;
>>  crtc->pch_fifo_underrun_disabled = !enable;
>> +#ifdef CONFIG_DEBUG_FS
>> +/* don't reset count in fifo underrun irq path */
>> +if (!in_irq() && !enable)
>> +crtc->pch_fifo_underrun_count = 0;
>> +#endif
>
> All these ifdefs are a bit ugly. Can we put all these stuff
> inside the debugfs.c file that is compiled only if DEBUG_FS is
> configured?

The opposite, the debugfs should be added in this file instead. :)

See my reply.

BR,
Jani.




>
> Andi
>
>>  
>>  if (HAS_PCH_IBX(dev_priv))
>>  ibx_set_fifo_underrun_reporting(_priv->drm,
>> @@ -434,6 +458,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct 
>> drm_i915_private *dev_priv,
>>  drm_err(_priv->drm, "CPU pipe %c FIFO underrun\n", 
>> pipe_name(pipe));
>>  }
>>  
>> +intel_fifo_underrun_inc_count(crtc, true);
>>  intel_fbc_handle_fifo_underrun_irq(dev_priv);
>>  }

-- 
Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [PATCH] drm/i915/display: Add a debugfs entry for fifo underruns

2023-02-14 Thread Jani Nikula
On Wed, 08 Feb 2023, Swati Sharma  wrote:
> From: Mohammed Khajapasha 
>
> Add a debugfs entry i915_fifo_underruns to indicate the count of
> fifo underruns for each pipe.
>
> Cc: Stanislav Lisovskiy 
> Signed-off-by: Mohammed Khajapasha 
> Signed-off-by: Swati Sharma 
> ---
>  .../drm/i915/display/intel_display_debugfs.c  | 28 ++
>  .../drm/i915/display/intel_display_types.h|  2 ++
>  .../drm/i915/display/intel_fifo_underrun.c| 29 +++
>  3 files changed, 59 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 9e2fb8626c96..d64b4675766c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -9,6 +9,7 @@
>  #include 
>  
>  #include "i915_debugfs.h"
> +#include "intel_crtc.h"
>  #include "i915_irq.h"
>  #include "i915_reg.h"
>  #include "intel_de.h"
> @@ -1057,6 +1058,32 @@ static int i915_lpsp_status(struct seq_file *m, void 
> *unused)
>   return 0;
>  }
>  
> +static int i915_fifo_underruns(struct seq_file *m, void *unused)
> +{
> + struct drm_i915_private *dev_priv = node_to_i915(m->private);
> + struct intel_crtc *crtc;
> + enum pipe pipe;
> + unsigned long flags;
> + u32 cpu_fifo_underrun_count[I915_MAX_PIPES];
> + u32 pch_fifo_underrun_count[I915_MAX_PIPES];
> +
> + spin_lock_irqsave(_priv->irq_lock, flags);
> + for_each_pipe(dev_priv, pipe) {
> + crtc = intel_crtc_for_pipe(dev_priv, pipe);

See the implementation of intel_crtc_for_pipe(). Looping over pipes and
then converting to crtcs is not great.

> + cpu_fifo_underrun_count[pipe] = crtc->cpu_fifo_underrun_count;
> + pch_fifo_underrun_count[pipe] = crtc->pch_fifo_underrun_count;
> + }
> + spin_unlock_irqrestore(_priv->irq_lock, flags);
> +
> + seq_printf(m, "\t%-10s \t%10s\n", "cpu fifounderruns", "pch 
> fifounderruns");
> + for_each_pipe(dev_priv, pipe)
> + seq_printf(m, "pipe:%c %10u %20u\n", pipe_name(pipe),
> +cpu_fifo_underrun_count[pipe],
> +pch_fifo_underrun_count[pipe]);

I would replace all of the above with a single for_each_intel_crtc()
loop, and ditch the local count arrays here. I'm not sure we care about
the counts being in sync across crtcs i.e. no need to take the irq lock
across the whole loop.

...

No wait. I think I'd actually replace all of that with a *crtc* specific
debugfs file instead. See below.

> +
> + return 0;
> +}
> +
>  static int i915_dp_mst_info(struct seq_file *m, void *unused)
>  {
>   struct drm_i915_private *dev_priv = node_to_i915(m->private);
> @@ -1586,6 +1613,7 @@ static const struct drm_info_list 
> intel_display_debugfs_list[] = {
>   {"i915_dp_mst_info", i915_dp_mst_info, 0},
>   {"i915_ddb_info", i915_ddb_info, 0},
>   {"i915_lpsp_status", i915_lpsp_status, 0},
> + {"i915_fifo_underruns", i915_fifo_underruns, 0},

The direction now is to add debugfs files next to the implementation.

So with the crtc specific files, you'd add

void intel_fifo_underrun_debugfs_add(struct intel_crtc *crtc)

in intel_fifo_underrun.[ch] and call that from intel_crtc_debugfs_add().

It handles exactly one crtc. See for example
intel_drrs_crtc_debugfs_add().

>  };
>  
>  static const struct {
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9ccae7a46020..b0468ac70059 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1438,6 +1438,8 @@ struct intel_crtc {
>  
>  #ifdef CONFIG_DEBUG_FS
>   struct intel_pipe_crc pipe_crc;
> + u32 cpu_fifo_underrun_count;
> + u32 pch_fifo_underrun_count;
>  #endif
>  };
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c 
> b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> index d636d21fa9ce..7131dd400ac3 100644
> --- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> +++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
> @@ -88,6 +88,17 @@ static bool cpt_can_enable_serr_int(struct drm_device *dev)
>   return true;
>  }
>  
> +static void intel_fifo_underrun_inc_count(struct intel_crtc *crtc,
> +   bool is_cpu_fifo)

What Andy said, but please don't add #defines, just add two separate
functions like:

intel_cpu_fifo_underrun_count_inc()
intel_pch_fifo_underrun_count_inc()

Those go hand in hand with:

intel_cpu_fifo_underrun_count_reset()
intel_pch_fifo_underrun_count_reset()

which we'll also need instead of messing with the counts directly in
some places and via accessors in some others.

> +{
> +#ifdef CONFIG_DEBUG_FS

The #ifdefs go outside the function. See coding-style.rst.

> + if (is_cpu_fifo)
> + crtc->cpu_fifo_underrun_count++;
> + else
> +  

Re: [Intel-gfx] [PATCH] drm/i915/display: Add a debugfs entry for fifo underruns

2023-02-08 Thread Andi Shyti
Hi Swati,

[...]

> +static void intel_fifo_underrun_inc_count(struct intel_crtc *crtc,
> +   bool is_cpu_fifo)

I'm not a big fan of the true/false parameters in functions. I
actually hate them because it's never clear from the caller what
the true/false means.

Isn't it clear to just have some wrappers

#define intel_fifo_underrun_inc_cpu_count(...)
#define intel_fifo_underrun_inc_cph_count(...)

or similar?

> +{
> +#ifdef CONFIG_DEBUG_FS
> + if (is_cpu_fifo)
> + crtc->cpu_fifo_underrun_count++;
> + else
> + crtc->pch_fifo_underrun_count++;
> +#endif
> +}
> +
>  static void i9xx_check_fifo_underruns(struct intel_crtc *crtc)
>  {
>   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -103,6 +114,7 @@ static void i9xx_check_fifo_underruns(struct intel_crtc 
> *crtc)
>   intel_de_write(dev_priv, reg, enable_mask | PIPE_FIFO_UNDERRUN_STATUS);
>   intel_de_posting_read(dev_priv, reg);
>  
> + intel_fifo_underrun_inc_count(crtc, true);
>   trace_intel_cpu_fifo_underrun(dev_priv, crtc->pipe);
>   drm_err(_priv->drm, "pipe %c underrun\n", pipe_name(crtc->pipe));
>  }
> @@ -156,6 +168,7 @@ static void ivb_check_fifo_underruns(struct intel_crtc 
> *crtc)
>   intel_de_write(dev_priv, GEN7_ERR_INT, ERR_INT_FIFO_UNDERRUN(pipe));
>   intel_de_posting_read(dev_priv, GEN7_ERR_INT);
>  
> + intel_fifo_underrun_inc_count(crtc, true);
>   trace_intel_cpu_fifo_underrun(dev_priv, pipe);
>   drm_err(_priv->drm, "fifo underrun on pipe %c\n", pipe_name(pipe));
>  }
> @@ -244,6 +257,7 @@ static void cpt_check_pch_fifo_underruns(struct 
> intel_crtc *crtc)
>  SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder));
>   intel_de_posting_read(dev_priv, SERR_INT);
>  
> + intel_fifo_underrun_inc_count(crtc, false);
>   trace_intel_pch_fifo_underrun(dev_priv, pch_transcoder);
>   drm_err(_priv->drm, "pch fifo underrun on pch transcoder %c\n",
>   pipe_name(pch_transcoder));
> @@ -286,6 +300,11 @@ static bool 
> __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
>  
>   old = !crtc->cpu_fifo_underrun_disabled;
>   crtc->cpu_fifo_underrun_disabled = !enable;
> +#ifdef CONFIG_DEBUG_FS
> + /* don't reset count in fifo underrun irq path */
> + if (!in_irq() && !enable)
> + crtc->cpu_fifo_underrun_count = 0;
> +#endif
>  
>   if (HAS_GMCH(dev_priv))
>   i9xx_set_fifo_underrun_reporting(dev, pipe, enable, old);
> @@ -365,6 +384,11 @@ bool intel_set_pch_fifo_underrun_reporting(struct 
> drm_i915_private *dev_priv,
>  
>   old = !crtc->pch_fifo_underrun_disabled;
>   crtc->pch_fifo_underrun_disabled = !enable;
> +#ifdef CONFIG_DEBUG_FS
> + /* don't reset count in fifo underrun irq path */
> + if (!in_irq() && !enable)
> + crtc->pch_fifo_underrun_count = 0;
> +#endif

All these ifdefs are a bit ugly. Can we put all these stuff
inside the debugfs.c file that is compiled only if DEBUG_FS is
configured?

Andi

>  
>   if (HAS_PCH_IBX(dev_priv))
>   ibx_set_fifo_underrun_reporting(_priv->drm,
> @@ -434,6 +458,7 @@ void intel_cpu_fifo_underrun_irq_handler(struct 
> drm_i915_private *dev_priv,
>   drm_err(_priv->drm, "CPU pipe %c FIFO underrun\n", 
> pipe_name(pipe));
>   }
>  
> + intel_fifo_underrun_inc_count(crtc, true);
>   intel_fbc_handle_fifo_underrun_irq(dev_priv);
>  }


[Intel-gfx] [PATCH] drm/i915/display: Add a debugfs entry for fifo underruns

2023-02-08 Thread Swati Sharma
From: Mohammed Khajapasha 

Add a debugfs entry i915_fifo_underruns to indicate the count of
fifo underruns for each pipe.

Cc: Stanislav Lisovskiy 
Signed-off-by: Mohammed Khajapasha 
Signed-off-by: Swati Sharma 
---
 .../drm/i915/display/intel_display_debugfs.c  | 28 ++
 .../drm/i915/display/intel_display_types.h|  2 ++
 .../drm/i915/display/intel_fifo_underrun.c| 29 +++
 3 files changed, 59 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c 
b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 9e2fb8626c96..d64b4675766c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -9,6 +9,7 @@
 #include 
 
 #include "i915_debugfs.h"
+#include "intel_crtc.h"
 #include "i915_irq.h"
 #include "i915_reg.h"
 #include "intel_de.h"
@@ -1057,6 +1058,32 @@ static int i915_lpsp_status(struct seq_file *m, void 
*unused)
return 0;
 }
 
+static int i915_fifo_underruns(struct seq_file *m, void *unused)
+{
+   struct drm_i915_private *dev_priv = node_to_i915(m->private);
+   struct intel_crtc *crtc;
+   enum pipe pipe;
+   unsigned long flags;
+   u32 cpu_fifo_underrun_count[I915_MAX_PIPES];
+   u32 pch_fifo_underrun_count[I915_MAX_PIPES];
+
+   spin_lock_irqsave(_priv->irq_lock, flags);
+   for_each_pipe(dev_priv, pipe) {
+   crtc = intel_crtc_for_pipe(dev_priv, pipe);
+   cpu_fifo_underrun_count[pipe] = crtc->cpu_fifo_underrun_count;
+   pch_fifo_underrun_count[pipe] = crtc->pch_fifo_underrun_count;
+   }
+   spin_unlock_irqrestore(_priv->irq_lock, flags);
+
+   seq_printf(m, "\t%-10s \t%10s\n", "cpu fifounderruns", "pch 
fifounderruns");
+   for_each_pipe(dev_priv, pipe)
+   seq_printf(m, "pipe:%c %10u %20u\n", pipe_name(pipe),
+  cpu_fifo_underrun_count[pipe],
+  pch_fifo_underrun_count[pipe]);
+
+   return 0;
+}
+
 static int i915_dp_mst_info(struct seq_file *m, void *unused)
 {
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -1586,6 +1613,7 @@ static const struct drm_info_list 
intel_display_debugfs_list[] = {
{"i915_dp_mst_info", i915_dp_mst_info, 0},
{"i915_ddb_info", i915_ddb_info, 0},
{"i915_lpsp_status", i915_lpsp_status, 0},
+   {"i915_fifo_underruns", i915_fifo_underruns, 0},
 };
 
 static const struct {
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h 
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 9ccae7a46020..b0468ac70059 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1438,6 +1438,8 @@ struct intel_crtc {
 
 #ifdef CONFIG_DEBUG_FS
struct intel_pipe_crc pipe_crc;
+   u32 cpu_fifo_underrun_count;
+   u32 pch_fifo_underrun_count;
 #endif
 };
 
diff --git a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c 
b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
index d636d21fa9ce..7131dd400ac3 100644
--- a/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/display/intel_fifo_underrun.c
@@ -88,6 +88,17 @@ static bool cpt_can_enable_serr_int(struct drm_device *dev)
return true;
 }
 
+static void intel_fifo_underrun_inc_count(struct intel_crtc *crtc,
+ bool is_cpu_fifo)
+{
+#ifdef CONFIG_DEBUG_FS
+   if (is_cpu_fifo)
+   crtc->cpu_fifo_underrun_count++;
+   else
+   crtc->pch_fifo_underrun_count++;
+#endif
+}
+
 static void i9xx_check_fifo_underruns(struct intel_crtc *crtc)
 {
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -103,6 +114,7 @@ static void i9xx_check_fifo_underruns(struct intel_crtc 
*crtc)
intel_de_write(dev_priv, reg, enable_mask | PIPE_FIFO_UNDERRUN_STATUS);
intel_de_posting_read(dev_priv, reg);
 
+   intel_fifo_underrun_inc_count(crtc, true);
trace_intel_cpu_fifo_underrun(dev_priv, crtc->pipe);
drm_err(_priv->drm, "pipe %c underrun\n", pipe_name(crtc->pipe));
 }
@@ -156,6 +168,7 @@ static void ivb_check_fifo_underruns(struct intel_crtc 
*crtc)
intel_de_write(dev_priv, GEN7_ERR_INT, ERR_INT_FIFO_UNDERRUN(pipe));
intel_de_posting_read(dev_priv, GEN7_ERR_INT);
 
+   intel_fifo_underrun_inc_count(crtc, true);
trace_intel_cpu_fifo_underrun(dev_priv, pipe);
drm_err(_priv->drm, "fifo underrun on pipe %c\n", pipe_name(pipe));
 }
@@ -244,6 +257,7 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc 
*crtc)
   SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder));
intel_de_posting_read(dev_priv, SERR_INT);
 
+   intel_fifo_underrun_inc_count(crtc, false);
trace_intel_pch_fifo_underrun(dev_priv, pch_transcoder);
drm_err(_priv->drm, "pch fifo underrun on pch transcoder %c\n",