[PATCH 1/2] drm/i915: Collect gtier properly on HSW.

[PATCH 1/2] drm/i915: Collect gtier properly on HSW.

[Rodrigo Vivi]

GTIER and DEIER doesn't have same interface on HSW so this "or" operation
makes the information provided useless.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  1 +
 drivers/gpu/drm/i915/i915_gpu_error.c | 16 ++++++++++------
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ef38c3b..ccb97f1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -314,6 +314,7 @@ struct drm_i915_error_state {
 	u32 eir;
 	u32 pgtbl_er;
 	u32 ier;
+	u32 gtier;
 	u32 ccid;
 	u32 derrmr;
 	u32 forcewake;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0b3f694..372fea3 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -359,6 +359,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 	err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
 	err_printf(m, "EIR: 0x%08x\n", error->eir);
 	err_printf(m, "IER: 0x%08x\n", error->ier);
+	if (IS_HASWELL(dev))
+		err_printf(m, "GTIER: 0x%08x\n", error->gtier);
 	err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
 	err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
 	err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
@@ -1135,13 +1137,15 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 	if (HAS_HW_CONTEXTS(dev))
 		error->ccid = I915_READ(CCID);
 
-	if (HAS_PCH_SPLIT(dev))
+	if (IS_HASWELL(dev)) {
+		error->ier = I915_READ(DEIER);
+		error->gtier = I915_READ(GTIER);
+	} else if (HAS_PCH_SPLIT(dev)) {
 		error->ier = I915_READ(DEIER) | I915_READ(GTIER);
-	else {
-		if (IS_GEN2(dev))
-			error->ier = I915_READ16(IER);
-		else
-			error->ier = I915_READ(IER);
+	} else if (IS_GEN2(dev)) {
+		error->ier = I915_READ16(IER);
+	} else {
+		error->ier = I915_READ(IER);
 	}
 
 	/* 4: Everything else */
-- 
1.9.3

[PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Rodrigo Vivi]

BDW has many other Display Engine interrupts and GT interrupts registers.
Collecting it properly on gpu_error_state.

On debugfs all was properly listed already but besides we were also listing old
DEIER and GTIER that doesn't exist on BDW anymore. This was causing
unclaimed register messages:

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

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
 drivers/gpu/drm/i915/i915_drv.h       |  4 +++-
 drivers/gpu/drm/i915/i915_gpu_error.c | 23 ++++++++++++++++++++---
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9e737b7..679cda6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -783,7 +783,7 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
 			seq_printf(m, "Pipe %c stat:         %08x\n",
 				   pipe_name(pipe),
 				   I915_READ(PIPESTAT(pipe)));
-	} else {
+	} else if (!IS_BROADWELL(dev)) {
 		seq_printf(m, "North Display Interrupt enable:		%08x\n",
 			   I915_READ(DEIER));
 		seq_printf(m, "North Display Interrupt identity:	%08x\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ccb97f1..ee28cd7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -314,7 +314,9 @@ struct drm_i915_error_state {
 	u32 eir;
 	u32 pgtbl_er;
 	u32 ier;
-	u32 gtier;
+	u32 gtier[4];
+	u32 deier[3];
+	u32 de_misc_ier;
 	u32 ccid;
 	u32 derrmr;
 	u32 forcewake;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 372fea3..f865d1d 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -358,9 +358,19 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 	err_printf(m, "Suspend count: %u\n", error->suspend_count);
 	err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
 	err_printf(m, "EIR: 0x%08x\n", error->eir);
-	err_printf(m, "IER: 0x%08x\n", error->ier);
+	if (IS_BROADWELL(dev)) {
+		for_each_pipe(i)
+			err_printf(m, "DEIER pipe %c: 0x%08x\n", pipe_name(i),
+				   error->deier[i]);
+		for (i = 0; i < 4; i++)
+			err_printf(m, "GTIER gt %d: 0x%08x\n", i,
+				   error->gtier[i]);
+		err_printf(m, "DE_MISC_IER: 0x%08x\n", error->de_misc_ier);
+	} else {
+		err_printf(m, "IER: 0x%08x\n", error->ier);
+	}
 	if (IS_HASWELL(dev))
-		err_printf(m, "GTIER: 0x%08x\n", error->gtier);
+		err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
 	err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
 	err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
 	err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
@@ -1093,6 +1103,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 				   struct drm_i915_error_state *error)
 {
 	struct drm_device *dev = dev_priv->dev;
+	int i;
 
 	/* General organization
 	 * 1. Registers specific to a single generation
@@ -1139,7 +1150,13 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 
 	if (IS_HASWELL(dev)) {
 		error->ier = I915_READ(DEIER);
-		error->gtier = I915_READ(GTIER);
+		error->gtier[0] = I915_READ(GTIER);
+	} else if (IS_BROADWELL(dev)) {
+		for_each_pipe(i)
+			error->deier[i] = I915_READ(GEN8_DE_PIPE_IER(i));
+		for (i = 0; i < 4; i++)
+			error->gtier[3] = I915_READ(GEN8_GT_IER(i));
+		error->de_misc_ier = I915_READ(GEN8_DE_MISC_IER);
 	} else if (HAS_PCH_SPLIT(dev)) {
 		error->ier = I915_READ(DEIER) | I915_READ(GTIER);
 	} else if (IS_GEN2(dev)) {
-- 
1.9.3

Re: [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Paulo Zanoni]

2014-07-28 12:19 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> BDW has many other Display Engine interrupts and GT interrupts registers.
> Collecting it properly on gpu_error_state.
>
> On debugfs all was properly listed already but besides we were also listing old
> DEIER and GTIER that doesn't exist on BDW anymore. This was causing
> unclaimed register messages:
>
> https://bugs.freedesktop.org/show_bug.cgi?id=81701
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h       |  4 +++-
>  drivers/gpu/drm/i915/i915_gpu_error.c | 23 ++++++++++++++++++++---
>  3 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9e737b7..679cda6 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -783,7 +783,7 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
>                         seq_printf(m, "Pipe %c stat:         %08x\n",
>                                    pipe_name(pipe),
>                                    I915_READ(PIPESTAT(pipe)));
> -       } else {
> +       } else if (!IS_BROADWELL(dev)) {
>                 seq_printf(m, "North Display Interrupt enable:          %08x\n",
>                            I915_READ(DEIER));
>                 seq_printf(m, "North Display Interrupt identity:        %08x\n",

This chunk is not needed since we already have a check for "gen >= 8"
at the top.


> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ccb97f1..ee28cd7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -314,7 +314,9 @@ struct drm_i915_error_state {
>         u32 eir;
>         u32 pgtbl_er;
>         u32 ier;
> -       u32 gtier;
> +       u32 gtier[4];
> +       u32 deier[3];
> +       u32 de_misc_ier;
>         u32 ccid;
>         u32 derrmr;
>         u32 forcewake;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 372fea3..f865d1d 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -358,9 +358,19 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>         err_printf(m, "Suspend count: %u\n", error->suspend_count);
>         err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
>         err_printf(m, "EIR: 0x%08x\n", error->eir);
> -       err_printf(m, "IER: 0x%08x\n", error->ier);
> +       if (IS_BROADWELL(dev)) {
> +               for_each_pipe(i)
> +                       err_printf(m, "DEIER pipe %c: 0x%08x\n", pipe_name(i),
> +                                  error->deier[i]);
> +               for (i = 0; i < 4; i++)
> +                       err_printf(m, "GTIER gt %d: 0x%08x\n", i,
> +                                  error->gtier[i]);
> +               err_printf(m, "DE_MISC_IER: 0x%08x\n", error->de_misc_ier);
> +       } else {
> +               err_printf(m, "IER: 0x%08x\n", error->ier);
> +       }
>         if (IS_HASWELL(dev))
> -               err_printf(m, "GTIER: 0x%08x\n", error->gtier);
> +               err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
>         err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
>         err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
>         err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
> @@ -1093,6 +1103,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>                                    struct drm_i915_error_state *error)
>  {
>         struct drm_device *dev = dev_priv->dev;
> +       int i;
>
>         /* General organization
>          * 1. Registers specific to a single generation
> @@ -1139,7 +1150,13 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>
>         if (IS_HASWELL(dev)) {
>                 error->ier = I915_READ(DEIER);
> -               error->gtier = I915_READ(GTIER);
> +               error->gtier[0] = I915_READ(GTIER);
> +       } else if (IS_BROADWELL(dev)) {
> +               for_each_pipe(i)
> +                       error->deier[i] = I915_READ(GEN8_DE_PIPE_IER(i));

for_each_pipe(i)
        if (intel_display_power_enabled(dev_priv, POWER_DOMAIN_PIPE(i)))
            error->deier[i] = I915_READ(GEN8_DE_PIPE_IER(i));

If the pipe's power well is disabled, the IER register is powered off
and we can get an unclaimed register errors too.

I also just noticed that this function contains the "/* 4: Everything
else */" comment twice. You could remove one :)


> +               for (i = 0; i < 4; i++)
> +                       error->gtier[3] = I915_READ(GEN8_GT_IER(i));
> +               error->de_misc_ier = I915_READ(GEN8_DE_MISC_IER);
>         } else if (HAS_PCH_SPLIT(dev)) {
>                 error->ier = I915_READ(DEIER) | I915_READ(GTIER);
>         } else if (IS_GEN2(dev)) {
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Rodrigo Vivi]

[-- Attachment #1.1: Type: text/plain, Size: 6093 bytes --]

On Wed, Jul 30, 2014 at 11:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:

> 2014-07-28 12:19 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> > BDW has many other Display Engine interrupts and GT interrupts registers.
> > Collecting it properly on gpu_error_state.
> >
> > On debugfs all was properly listed already but besides we were also
> listing old
> > DEIER and GTIER that doesn't exist on BDW anymore. This was causing
> > unclaimed register messages:
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=81701
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
> >  drivers/gpu/drm/i915/i915_drv.h       |  4 +++-
> >  drivers/gpu/drm/i915/i915_gpu_error.c | 23 ++++++++++++++++++++---
> >  3 files changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 9e737b7..679cda6 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -783,7 +783,7 @@ static int i915_interrupt_info(struct seq_file *m,
> void *data)
> >                         seq_printf(m, "Pipe %c stat:         %08x\n",
> >                                    pipe_name(pipe),
> >                                    I915_READ(PIPESTAT(pipe)));
> > -       } else {
> > +       } else if (!IS_BROADWELL(dev)) {
> >                 seq_printf(m, "North Display Interrupt enable:
>  %08x\n",
> >                            I915_READ(DEIER));
> >                 seq_printf(m, "North Display Interrupt identity:
>  %08x\n",
>
> This chunk is not needed since we already have a check for "gen >= 8"
> at the top.
>

oh, for a moment I thought it was messy as the other code and it was
writing twice...
mainly when you mentioned that you could reproduce the unclaimed when
reading debugfs...
but probably was related to the powerwell you mentioned below...


>
>
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> > index ccb97f1..ee28cd7 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -314,7 +314,9 @@ struct drm_i915_error_state {
> >         u32 eir;
> >         u32 pgtbl_er;
> >         u32 ier;
> > -       u32 gtier;
> > +       u32 gtier[4];
> > +       u32 deier[3];
> > +       u32 de_misc_ier;
> >         u32 ccid;
> >         u32 derrmr;
> >         u32 forcewake;
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 372fea3..f865d1d 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -358,9 +358,19 @@ int i915_error_state_to_str(struct
> drm_i915_error_state_buf *m,
> >         err_printf(m, "Suspend count: %u\n", error->suspend_count);
> >         err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
> >         err_printf(m, "EIR: 0x%08x\n", error->eir);
> > -       err_printf(m, "IER: 0x%08x\n", error->ier);
> > +       if (IS_BROADWELL(dev)) {
> > +               for_each_pipe(i)
> > +                       err_printf(m, "DEIER pipe %c: 0x%08x\n",
> pipe_name(i),
> > +                                  error->deier[i]);
> > +               for (i = 0; i < 4; i++)
> > +                       err_printf(m, "GTIER gt %d: 0x%08x\n", i,
> > +                                  error->gtier[i]);
> > +               err_printf(m, "DE_MISC_IER: 0x%08x\n",
> error->de_misc_ier);
> > +       } else {
> > +               err_printf(m, "IER: 0x%08x\n", error->ier);
> > +       }
> >         if (IS_HASWELL(dev))
> > -               err_printf(m, "GTIER: 0x%08x\n", error->gtier);
> > +               err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
> >         err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
> >         err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
> >         err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
> > @@ -1093,6 +1103,7 @@ static void i915_capture_reg_state(struct
> drm_i915_private *dev_priv,
> >                                    struct drm_i915_error_state *error)
> >  {
> >         struct drm_device *dev = dev_priv->dev;
> > +       int i;
> >
> >         /* General organization
> >          * 1. Registers specific to a single generation
> > @@ -1139,7 +1150,13 @@ static void i915_capture_reg_state(struct
> drm_i915_private *dev_priv,
> >
> >         if (IS_HASWELL(dev)) {
> >                 error->ier = I915_READ(DEIER);
> > -               error->gtier = I915_READ(GTIER);
> > +               error->gtier[0] = I915_READ(GTIER);
> > +       } else if (IS_BROADWELL(dev)) {
> > +               for_each_pipe(i)
> > +                       error->deier[i] = I915_READ(GEN8_DE_PIPE_IER(i));
>
> for_each_pipe(i)
>         if (intel_display_power_enabled(dev_priv, POWER_DOMAIN_PIPE(i)))
>             error->deier[i] = I915_READ(GEN8_DE_PIPE_IER(i));
>
> If the pipe's power well is disabled, the IER register is powered off
> and we can get an unclaimed register errors too.
>

thanks! I'll add it...
Also also need to be added to debugfs interface.


>
> I also just noticed that this function contains the "/* 4: Everything
> else */" comment twice. You could remove one :)
>

indeed.



>
>
> > +               for (i = 0; i < 4; i++)
> > +                       error->gtier[3] = I915_READ(GEN8_GT_IER(i));
> > +               error->de_misc_ier = I915_READ(GEN8_DE_MISC_IER);
> >         } else if (HAS_PCH_SPLIT(dev)) {
> >                 error->ier = I915_READ(DEIER) | I915_READ(GTIER);
> >         } else if (IS_GEN2(dev)) {
> > --
> > 1.9.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[-- Attachment #1.2: Type: text/html, Size: 9135 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Paulo Zanoni]

2014-07-30 15:20 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
>
>
>
> On Wed, Jul 30, 2014 at 11:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
>>
>> 2014-07-28 12:19 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
>> > BDW has many other Display Engine interrupts and GT interrupts
>> > registers.
>> > Collecting it properly on gpu_error_state.
>> >
>> > On debugfs all was properly listed already but besides we were also
>> > listing old
>> > DEIER and GTIER that doesn't exist on BDW anymore. This was causing
>> > unclaimed register messages:
>> >
>> > https://bugs.freedesktop.org/show_bug.cgi?id=81701
>> >
>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
>> >  drivers/gpu/drm/i915/i915_drv.h       |  4 +++-
>> >  drivers/gpu/drm/i915/i915_gpu_error.c | 23 ++++++++++++++++++++---
>> >  3 files changed, 24 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> > b/drivers/gpu/drm/i915/i915_debugfs.c
>> > index 9e737b7..679cda6 100644
>> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> > @@ -783,7 +783,7 @@ static int i915_interrupt_info(struct seq_file *m,
>> > void *data)
>> >                         seq_printf(m, "Pipe %c stat:         %08x\n",
>> >                                    pipe_name(pipe),
>> >                                    I915_READ(PIPESTAT(pipe)));
>> > -       } else {
>> > +       } else if (!IS_BROADWELL(dev)) {
>> >                 seq_printf(m, "North Display Interrupt enable:
>> > %08x\n",
>> >                            I915_READ(DEIER));
>> >                 seq_printf(m, "North Display Interrupt identity:
>> > %08x\n",
>>
>> This chunk is not needed since we already have a check for "gen >= 8"
>> at the top.
>
>
> oh, for a moment I thought it was messy as the other code and it was writing
> twice...
> mainly when you mentioned that you could reproduce the unclaimed when
> reading debugfs...
> but probably was related to the powerwell you mentioned below...

Yes, at i915_interrupt_info we also need to call intel_display_power_enabled().

I also won't complain if you fix i915_frequency_info on BDW :)

>
>>
>>
>>
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> > b/drivers/gpu/drm/i915/i915_drv.h
>> > index ccb97f1..ee28cd7 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -314,7 +314,9 @@ struct drm_i915_error_state {
>> >         u32 eir;
>> >         u32 pgtbl_er;
>> >         u32 ier;
>> > -       u32 gtier;
>> > +       u32 gtier[4];
>> > +       u32 deier[3];
>> > +       u32 de_misc_ier;
>> >         u32 ccid;
>> >         u32 derrmr;
>> >         u32 forcewake;
>> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
>> > b/drivers/gpu/drm/i915/i915_gpu_error.c
>> > index 372fea3..f865d1d 100644
>> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
>> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
>> > @@ -358,9 +358,19 @@ int i915_error_state_to_str(struct
>> > drm_i915_error_state_buf *m,
>> >         err_printf(m, "Suspend count: %u\n", error->suspend_count);
>> >         err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
>> >         err_printf(m, "EIR: 0x%08x\n", error->eir);
>> > -       err_printf(m, "IER: 0x%08x\n", error->ier);
>> > +       if (IS_BROADWELL(dev)) {
>> > +               for_each_pipe(i)
>> > +                       err_printf(m, "DEIER pipe %c: 0x%08x\n",
>> > pipe_name(i),
>> > +                                  error->deier[i]);
>> > +               for (i = 0; i < 4; i++)
>> > +                       err_printf(m, "GTIER gt %d: 0x%08x\n", i,
>> > +                                  error->gtier[i]);
>> > +               err_printf(m, "DE_MISC_IER: 0x%08x\n",
>> > error->de_misc_ier);
>> > +       } else {
>> > +               err_printf(m, "IER: 0x%08x\n", error->ier);
>> > +       }
>> >         if (IS_HASWELL(dev))
>> > -               err_printf(m, "GTIER: 0x%08x\n", error->gtier);
>> > +               err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
>> >         err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
>> >         err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
>> >         err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
>> > @@ -1093,6 +1103,7 @@ static void i915_capture_reg_state(struct
>> > drm_i915_private *dev_priv,
>> >                                    struct drm_i915_error_state *error)
>> >  {
>> >         struct drm_device *dev = dev_priv->dev;
>> > +       int i;
>> >
>> >         /* General organization
>> >          * 1. Registers specific to a single generation
>> > @@ -1139,7 +1150,13 @@ static void i915_capture_reg_state(struct
>> > drm_i915_private *dev_priv,
>> >
>> >         if (IS_HASWELL(dev)) {
>> >                 error->ier = I915_READ(DEIER);
>> > -               error->gtier = I915_READ(GTIER);
>> > +               error->gtier[0] = I915_READ(GTIER);
>> > +       } else if (IS_BROADWELL(dev)) {
>> > +               for_each_pipe(i)
>> > +                       error->deier[i] =
>> > I915_READ(GEN8_DE_PIPE_IER(i));
>>
>> for_each_pipe(i)
>>         if (intel_display_power_enabled(dev_priv, POWER_DOMAIN_PIPE(i)))
>>             error->deier[i] = I915_READ(GEN8_DE_PIPE_IER(i));
>>
>> If the pipe's power well is disabled, the IER register is powered off
>> and we can get an unclaimed register errors too.
>
>
> thanks! I'll add it...
> Also also need to be added to debugfs interface.
>
>>
>>
>> I also just noticed that this function contains the "/* 4: Everything
>> else */" comment twice. You could remove one :)
>
>
> indeed.
>
>
>>
>>
>>
>> > +               for (i = 0; i < 4; i++)
>> > +                       error->gtier[3] = I915_READ(GEN8_GT_IER(i));
>> > +               error->de_misc_ier = I915_READ(GEN8_DE_MISC_IER);
>> >         } else if (HAS_PCH_SPLIT(dev)) {
>> >                 error->ier = I915_READ(DEIER) | I915_READ(GTIER);
>> >         } else if (IS_GEN2(dev)) {
>> > --
>> > 1.9.3
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>> --
>> Paulo Zanoni
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
>



-- 
Paulo Zanoni

[PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Rodrigo Vivi]

BDW has many other Display Engine interrupts and GT interrupts registers.
Collecting it properly on gpu_error_state.

On debugfs all was properly listed already but besides we were also listing old
DEIER and GTIER that doesn't exist on BDW anymore. This was causing
unclaimed register messages:

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

v2: Fix small issues of first version and don't read DEIER regs when pipe's
    power well is disabled

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  4 ++++
 drivers/gpu/drm/i915/i915_drv.h       |  4 +++-
 drivers/gpu/drm/i915/i915_gpu_error.c | 29 +++++++++++++++++++++++++----
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9e737b7..b3493d3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -703,6 +703,10 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
 		}
 
 		for_each_pipe(pipe) {
+			if (!intel_display_power_enabled(dev_priv,
+							 POWER_DOMAIN_PIPE(pipe)))
+				continue;
+
 			seq_printf(m, "Pipe %c IMR:\t%08x\n",
 				   pipe_name(pipe),
 				   I915_READ(GEN8_DE_PIPE_IMR(pipe)));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 60227b2..d1ae952 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -317,7 +317,9 @@ struct drm_i915_error_state {
 	u32 eir;
 	u32 pgtbl_er;
 	u32 ier;
-	u32 gtier;
+	u32 gtier[4];
+	u32 deier[3];
+	u32 de_misc_ier;
 	u32 ccid;
 	u32 derrmr;
 	u32 forcewake;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 76c67dd..088b535 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -359,8 +359,19 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 	err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
 	err_printf(m, "EIR: 0x%08x\n", error->eir);
 	err_printf(m, "IER: 0x%08x\n", error->ier);
+	if (IS_BROADWELL(dev)) {
+		for_each_pipe(i)
+			err_printf(m, "DEIER pipe %c: 0x%08x\n", pipe_name(i),
+				   error->deier[i]);
+		for (i = 0; i < 4; i++)
+			err_printf(m, "GTIER gt %d: 0x%08x\n", i,
+				   error->gtier[i]);
+		err_printf(m, "DE_MISC_IER: 0x%08x\n", error->de_misc_ier);
+	} else {
+		err_printf(m, "IER: 0x%08x\n", error->ier);
+	}
 	if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
-		err_printf(m, "GTIER: 0x%08x\n", error->gtier);
+		err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
 	err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
 	err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
 	err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
@@ -1093,6 +1104,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 				   struct drm_i915_error_state *error)
 {
 	struct drm_device *dev = dev_priv->dev;
+	int i;
 
 	/* General organization
 	 * 1. Registers specific to a single generation
@@ -1104,7 +1116,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 
 	/* 1: Registers specific to a single generation */
 	if (IS_VALLEYVIEW(dev)) {
-		error->gtier = I915_READ(GTIER);
+		error->gtier[0] = I915_READ(GTIER);
 		error->ier = I915_READ(VLV_IER);
 		error->forcewake = I915_READ(FORCEWAKE_VLV);
 	}
@@ -1138,9 +1150,18 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 	if (HAS_HW_CONTEXTS(dev))
 		error->ccid = I915_READ(CCID);
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (IS_BROADWELL(dev)) {
+		for_each_pipe(i)
+			if (intel_display_power_enabled(dev_priv,
+							POWER_DOMAIN_PIPE(i)))
+				error->deier[i] =
+					I915_READ(GEN8_DE_PIPE_IER(i));
+		for (i = 0; i < 4; i++)
+			error->gtier[i] = I915_READ(GEN8_GT_IER(i));
+		error->de_misc_ier = I915_READ(GEN8_DE_MISC_IER);
+	} else if (HAS_PCH_SPLIT(dev)) {
 		error->ier = I915_READ(DEIER);
-		error->gtier = I915_READ(GTIER);
+		error->gtier[0] = I915_READ(GTIER);
 	} else if (IS_GEN2(dev)) {
 		error->ier = I915_READ16(IER);
 	} else {
-- 
1.9.1

Re: [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Paulo Zanoni]

2014-08-01 6:14 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> BDW has many other Display Engine interrupts and GT interrupts registers.
> Collecting it properly on gpu_error_state.
>
> On debugfs all was properly listed already but besides we were also listing old
> DEIER and GTIER that doesn't exist on BDW anymore. This was causing
> unclaimed register messages:
>
> https://bugs.freedesktop.org/show_bug.cgi?id=81701
>
> v2: Fix small issues of first version and don't read DEIER regs when pipe's
>     power well is disabled
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |  4 ++++
>  drivers/gpu/drm/i915/i915_drv.h       |  4 +++-
>  drivers/gpu/drm/i915/i915_gpu_error.c | 29 +++++++++++++++++++++++++----
>  3 files changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9e737b7..b3493d3 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -703,6 +703,10 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
>                 }
>
>                 for_each_pipe(pipe) {
> +                       if (!intel_display_power_enabled(dev_priv,
> +                                                        POWER_DOMAIN_PIPE(pipe)))
> +                               continue;

Bikeshed: print something here (e.g., "Pipe %c: power disabled\n").


> +
>                         seq_printf(m, "Pipe %c IMR:\t%08x\n",
>                                    pipe_name(pipe),
>                                    I915_READ(GEN8_DE_PIPE_IMR(pipe)));
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 60227b2..d1ae952 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -317,7 +317,9 @@ struct drm_i915_error_state {
>         u32 eir;
>         u32 pgtbl_er;
>         u32 ier;
> -       u32 gtier;
> +       u32 gtier[4];
> +       u32 deier[3];
> +       u32 de_misc_ier;
>         u32 ccid;
>         u32 derrmr;
>         u32 forcewake;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 76c67dd..088b535 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -359,8 +359,19 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>         err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
>         err_printf(m, "EIR: 0x%08x\n", error->eir);
>         err_printf(m, "IER: 0x%08x\n", error->ier);
> +       if (IS_BROADWELL(dev)) {
> +               for_each_pipe(i)
> +                       err_printf(m, "DEIER pipe %c: 0x%08x\n", pipe_name(i),
> +                                  error->deier[i]);
> +               for (i = 0; i < 4; i++)
> +                       err_printf(m, "GTIER gt %d: 0x%08x\n", i,
> +                                  error->gtier[i]);
> +               err_printf(m, "DE_MISC_IER: 0x%08x\n", error->de_misc_ier);
> +       } else {
> +               err_printf(m, "IER: 0x%08x\n", error->ier);
> +       }
>         if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
> -               err_printf(m, "GTIER: 0x%08x\n", error->gtier);
> +               err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
>         err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
>         err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
>         err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
> @@ -1093,6 +1104,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>                                    struct drm_i915_error_state *error)
>  {
>         struct drm_device *dev = dev_priv->dev;
> +       int i;

Bikeshed: also add "enum pipe pipe" and use it for the pipe iteration.

With or without these changes: Reviewed-by: Paulo Zanoni
<paulo.r.zanoni@intel.com>

>
>         /* General organization
>          * 1. Registers specific to a single generation
> @@ -1104,7 +1116,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>
>         /* 1: Registers specific to a single generation */
>         if (IS_VALLEYVIEW(dev)) {
> -               error->gtier = I915_READ(GTIER);
> +               error->gtier[0] = I915_READ(GTIER);
>                 error->ier = I915_READ(VLV_IER);
>                 error->forcewake = I915_READ(FORCEWAKE_VLV);
>         }
> @@ -1138,9 +1150,18 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>         if (HAS_HW_CONTEXTS(dev))
>                 error->ccid = I915_READ(CCID);
>
> -       if (HAS_PCH_SPLIT(dev)) {
> +       if (IS_BROADWELL(dev)) {
> +               for_each_pipe(i)
> +                       if (intel_display_power_enabled(dev_priv,
> +                                                       POWER_DOMAIN_PIPE(i)))
> +                               error->deier[i] =
> +                                       I915_READ(GEN8_DE_PIPE_IER(i));
> +               for (i = 0; i < 4; i++)
> +                       error->gtier[i] = I915_READ(GEN8_GT_IER(i));
> +               error->de_misc_ier = I915_READ(GEN8_DE_MISC_IER);
> +       } else if (HAS_PCH_SPLIT(dev)) {
>                 error->ier = I915_READ(DEIER);
> -               error->gtier = I915_READ(GTIER);
> +               error->gtier[0] = I915_READ(GTIER);
>         } else if (IS_GEN2(dev)) {
>                 error->ier = I915_READ16(IER);
>         } else {
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

[PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Rodrigo Vivi]

BDW has many other Display Engine interrupts and GT interrupts registers.
Collecting it properly on gpu_error_state.

On debugfs all was properly listed already but besides we were also listing old
DEIER and GTIER that doesn't exist on BDW anymore. This was causing
unclaimed register messages:

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

v2: Fix small issues of first version and don't read DEIER regs when pipe's
    power well is disabled
v3: bikeshed accepted: use enum pipe pipe instead of int i for pipe interection

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  4 ++++
 drivers/gpu/drm/i915/i915_drv.h       |  4 +++-
 drivers/gpu/drm/i915/i915_gpu_error.c | 32 ++++++++++++++++++++++++++++----
 3 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9e737b7..b3493d3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -703,6 +703,10 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
 		}
 
 		for_each_pipe(pipe) {
+			if (!intel_display_power_enabled(dev_priv,
+							 POWER_DOMAIN_PIPE(pipe)))
+				continue;
+
 			seq_printf(m, "Pipe %c IMR:\t%08x\n",
 				   pipe_name(pipe),
 				   I915_READ(GEN8_DE_PIPE_IMR(pipe)));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 60227b2..d1ae952 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -317,7 +317,9 @@ struct drm_i915_error_state {
 	u32 eir;
 	u32 pgtbl_er;
 	u32 ier;
-	u32 gtier;
+	u32 gtier[4];
+	u32 deier[3];
+	u32 de_misc_ier;
 	u32 ccid;
 	u32 derrmr;
 	u32 forcewake;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index c8f901f..402b621 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -328,6 +328,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_error_state *error = error_priv->error;
 	struct drm_i915_error_object *obj;
+	enum pipe pipe;
 	int i, j, offset, elt;
 	int max_hangcheck_score;
 
@@ -359,8 +360,20 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 	err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
 	err_printf(m, "EIR: 0x%08x\n", error->eir);
 	err_printf(m, "IER: 0x%08x\n", error->ier);
+	if (IS_BROADWELL(dev)) {
+		for_each_pipe(pipe)
+			err_printf(m, "DEIER pipe %c: 0x%08x\n",
+				   pipe_name(pipe),
+				   error->deier[pipe]);
+		for (i = 0; i < 4; i++)
+			err_printf(m, "GTIER gt %d: 0x%08x\n", i,
+				   error->gtier[i]);
+		err_printf(m, "DE_MISC_IER: 0x%08x\n", error->de_misc_ier);
+	} else {
+		err_printf(m, "IER: 0x%08x\n", error->ier);
+	}
 	if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
-		err_printf(m, "GTIER: 0x%08x\n", error->gtier);
+		err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
 	err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
 	err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
 	err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
@@ -1093,6 +1106,8 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 				   struct drm_i915_error_state *error)
 {
 	struct drm_device *dev = dev_priv->dev;
+	enum pipe pipe;
+	int i;
 
 	/* General organization
 	 * 1. Registers specific to a single generation
@@ -1104,7 +1119,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 
 	/* 1: Registers specific to a single generation */
 	if (IS_VALLEYVIEW(dev)) {
-		error->gtier = I915_READ(GTIER);
+		error->gtier[0] = I915_READ(GTIER);
 		error->ier = I915_READ(VLV_IER);
 		error->forcewake = I915_READ(FORCEWAKE_VLV);
 	}
@@ -1138,9 +1153,18 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 	if (HAS_HW_CONTEXTS(dev))
 		error->ccid = I915_READ(CCID);
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (IS_BROADWELL(dev)) {
+		for_each_pipe(pipe)
+			if (intel_display_power_enabled(dev_priv,
+							POWER_DOMAIN_PIPE(pipe)))
+				error->deier[pipe] =
+					I915_READ(GEN8_DE_PIPE_IER(pipe));
+		for (i = 0; i < 4; i++)
+			error->gtier[i] = I915_READ(GEN8_GT_IER(i));
+		error->de_misc_ier = I915_READ(GEN8_DE_MISC_IER);
+	} else if (HAS_PCH_SPLIT(dev)) {
 		error->ier = I915_READ(DEIER);
-		error->gtier = I915_READ(GTIER);
+		error->gtier[0] = I915_READ(GTIER);
 	} else if (IS_GEN2(dev)) {
 		error->ier = I915_READ16(IER);
 	} else if (!IS_VALLEYVIEW(dev)) {
-- 
1.9.1

Re: [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Paulo Zanoni]

2014-08-01 13:13 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> BDW has many other Display Engine interrupts and GT interrupts registers.
> Collecting it properly on gpu_error_state.
>
> On debugfs all was properly listed already but besides we were also listing old
> DEIER and GTIER that doesn't exist on BDW anymore. This was causing
> unclaimed register messages:
>
> https://bugs.freedesktop.org/show_bug.cgi?id=81701
>
> v2: Fix small issues of first version and don't read DEIER regs when pipe's
>     power well is disabled
> v3: bikeshed accepted: use enum pipe pipe instead of int i for pipe interection

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |  4 ++++
>  drivers/gpu/drm/i915/i915_drv.h       |  4 +++-
>  drivers/gpu/drm/i915/i915_gpu_error.c | 32 ++++++++++++++++++++++++++++----
>  3 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9e737b7..b3493d3 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -703,6 +703,10 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
>                 }
>
>                 for_each_pipe(pipe) {
> +                       if (!intel_display_power_enabled(dev_priv,
> +                                                        POWER_DOMAIN_PIPE(pipe)))
> +                               continue;
> +
>                         seq_printf(m, "Pipe %c IMR:\t%08x\n",
>                                    pipe_name(pipe),
>                                    I915_READ(GEN8_DE_PIPE_IMR(pipe)));
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 60227b2..d1ae952 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -317,7 +317,9 @@ struct drm_i915_error_state {
>         u32 eir;
>         u32 pgtbl_er;
>         u32 ier;
> -       u32 gtier;
> +       u32 gtier[4];
> +       u32 deier[3];
> +       u32 de_misc_ier;
>         u32 ccid;
>         u32 derrmr;
>         u32 forcewake;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index c8f901f..402b621 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -328,6 +328,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct drm_i915_error_state *error = error_priv->error;
>         struct drm_i915_error_object *obj;
> +       enum pipe pipe;
>         int i, j, offset, elt;
>         int max_hangcheck_score;
>
> @@ -359,8 +360,20 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>         err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
>         err_printf(m, "EIR: 0x%08x\n", error->eir);
>         err_printf(m, "IER: 0x%08x\n", error->ier);
> +       if (IS_BROADWELL(dev)) {
> +               for_each_pipe(pipe)
> +                       err_printf(m, "DEIER pipe %c: 0x%08x\n",
> +                                  pipe_name(pipe),
> +                                  error->deier[pipe]);
> +               for (i = 0; i < 4; i++)
> +                       err_printf(m, "GTIER gt %d: 0x%08x\n", i,
> +                                  error->gtier[i]);
> +               err_printf(m, "DE_MISC_IER: 0x%08x\n", error->de_misc_ier);
> +       } else {
> +               err_printf(m, "IER: 0x%08x\n", error->ier);
> +       }
>         if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
> -               err_printf(m, "GTIER: 0x%08x\n", error->gtier);
> +               err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
>         err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
>         err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
>         err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
> @@ -1093,6 +1106,8 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>                                    struct drm_i915_error_state *error)
>  {
>         struct drm_device *dev = dev_priv->dev;
> +       enum pipe pipe;
> +       int i;
>
>         /* General organization
>          * 1. Registers specific to a single generation
> @@ -1104,7 +1119,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>
>         /* 1: Registers specific to a single generation */
>         if (IS_VALLEYVIEW(dev)) {
> -               error->gtier = I915_READ(GTIER);
> +               error->gtier[0] = I915_READ(GTIER);
>                 error->ier = I915_READ(VLV_IER);
>                 error->forcewake = I915_READ(FORCEWAKE_VLV);
>         }
> @@ -1138,9 +1153,18 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>         if (HAS_HW_CONTEXTS(dev))
>                 error->ccid = I915_READ(CCID);
>
> -       if (HAS_PCH_SPLIT(dev)) {
> +       if (IS_BROADWELL(dev)) {
> +               for_each_pipe(pipe)
> +                       if (intel_display_power_enabled(dev_priv,
> +                                                       POWER_DOMAIN_PIPE(pipe)))
> +                               error->deier[pipe] =
> +                                       I915_READ(GEN8_DE_PIPE_IER(pipe));
> +               for (i = 0; i < 4; i++)
> +                       error->gtier[i] = I915_READ(GEN8_GT_IER(i));
> +               error->de_misc_ier = I915_READ(GEN8_DE_MISC_IER);
> +       } else if (HAS_PCH_SPLIT(dev)) {
>                 error->ier = I915_READ(DEIER);
> -               error->gtier = I915_READ(GTIER);
> +               error->gtier[0] = I915_READ(GTIER);
>         } else if (IS_GEN2(dev)) {
>                 error->ier = I915_READ16(IER);
>         } else if (!IS_VALLEYVIEW(dev)) {
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 2/2] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Daniel Vetter]

On Mon, Aug 04, 2014 at 11:44:10AM -0300, Paulo Zanoni wrote:
> 2014-08-01 13:13 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> > BDW has many other Display Engine interrupts and GT interrupts registers.
> > Collecting it properly on gpu_error_state.
> >
> > On debugfs all was properly listed already but besides we were also listing old
> > DEIER and GTIER that doesn't exist on BDW anymore. This was causing
> > unclaimed register messages:
> >
> > https://bugs.freedesktop.org/show_bug.cgi?id=81701
> >
> > v2: Fix small issues of first version and don't read DEIER regs when pipe's
> >     power well is disabled
> > v3: bikeshed accepted: use enum pipe pipe instead of int i for pipe interection
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Both patches merged to dinq, thanks.
-Daniel

> 
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c   |  4 ++++
> >  drivers/gpu/drm/i915/i915_drv.h       |  4 +++-
> >  drivers/gpu/drm/i915/i915_gpu_error.c | 32 ++++++++++++++++++++++++++++----
> >  3 files changed, 35 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 9e737b7..b3493d3 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -703,6 +703,10 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
> >                 }
> >
> >                 for_each_pipe(pipe) {
> > +                       if (!intel_display_power_enabled(dev_priv,
> > +                                                        POWER_DOMAIN_PIPE(pipe)))
> > +                               continue;
> > +
> >                         seq_printf(m, "Pipe %c IMR:\t%08x\n",
> >                                    pipe_name(pipe),
> >                                    I915_READ(GEN8_DE_PIPE_IMR(pipe)));
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 60227b2..d1ae952 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -317,7 +317,9 @@ struct drm_i915_error_state {
> >         u32 eir;
> >         u32 pgtbl_er;
> >         u32 ier;
> > -       u32 gtier;
> > +       u32 gtier[4];
> > +       u32 deier[3];
> > +       u32 de_misc_ier;
> >         u32 ccid;
> >         u32 derrmr;
> >         u32 forcewake;
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index c8f901f..402b621 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -328,6 +328,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> >         struct drm_i915_private *dev_priv = dev->dev_private;
> >         struct drm_i915_error_state *error = error_priv->error;
> >         struct drm_i915_error_object *obj;
> > +       enum pipe pipe;
> >         int i, j, offset, elt;
> >         int max_hangcheck_score;
> >
> > @@ -359,8 +360,20 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> >         err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
> >         err_printf(m, "EIR: 0x%08x\n", error->eir);
> >         err_printf(m, "IER: 0x%08x\n", error->ier);
> > +       if (IS_BROADWELL(dev)) {
> > +               for_each_pipe(pipe)
> > +                       err_printf(m, "DEIER pipe %c: 0x%08x\n",
> > +                                  pipe_name(pipe),
> > +                                  error->deier[pipe]);
> > +               for (i = 0; i < 4; i++)
> > +                       err_printf(m, "GTIER gt %d: 0x%08x\n", i,
> > +                                  error->gtier[i]);
> > +               err_printf(m, "DE_MISC_IER: 0x%08x\n", error->de_misc_ier);
> > +       } else {
> > +               err_printf(m, "IER: 0x%08x\n", error->ier);
> > +       }
> >         if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
> > -               err_printf(m, "GTIER: 0x%08x\n", error->gtier);
> > +               err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
> >         err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
> >         err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
> >         err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
> > @@ -1093,6 +1106,8 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> >                                    struct drm_i915_error_state *error)
> >  {
> >         struct drm_device *dev = dev_priv->dev;
> > +       enum pipe pipe;
> > +       int i;
> >
> >         /* General organization
> >          * 1. Registers specific to a single generation
> > @@ -1104,7 +1119,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> >
> >         /* 1: Registers specific to a single generation */
> >         if (IS_VALLEYVIEW(dev)) {
> > -               error->gtier = I915_READ(GTIER);
> > +               error->gtier[0] = I915_READ(GTIER);
> >                 error->ier = I915_READ(VLV_IER);
> >                 error->forcewake = I915_READ(FORCEWAKE_VLV);
> >         }
> > @@ -1138,9 +1153,18 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> >         if (HAS_HW_CONTEXTS(dev))
> >                 error->ccid = I915_READ(CCID);
> >
> > -       if (HAS_PCH_SPLIT(dev)) {
> > +       if (IS_BROADWELL(dev)) {
> > +               for_each_pipe(pipe)
> > +                       if (intel_display_power_enabled(dev_priv,
> > +                                                       POWER_DOMAIN_PIPE(pipe)))
> > +                               error->deier[pipe] =
> > +                                       I915_READ(GEN8_DE_PIPE_IER(pipe));
> > +               for (i = 0; i < 4; i++)
> > +                       error->gtier[i] = I915_READ(GEN8_GT_IER(i));
> > +               error->de_misc_ier = I915_READ(GEN8_DE_MISC_IER);
> > +       } else if (HAS_PCH_SPLIT(dev)) {
> >                 error->ier = I915_READ(DEIER);
> > -               error->gtier = I915_READ(GTIER);
> > +               error->gtier[0] = I915_READ(GTIER);
> >         } else if (IS_GEN2(dev)) {
> >                 error->ier = I915_READ16(IER);
> >         } else if (!IS_VALLEYVIEW(dev)) {
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Rodrigo Vivi]

BDW has many other Display Engine interrupts and GT interrupts registers.
Collecting it properly on gpu_error_state.

On debugfs all was properly listed already but besides we were also listing old
DEIER and GTIER that doesn't exist on BDW anymore. This was causing
unclaimed register messages:

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

v2: Fix small issues of first version and don't read DEIER regs when pipe's
    power well is disabled
v3: bikeshed accepted: use enum pipe pipe instead of int i for pipe interection
v4: Ben notice previous version was checking for display_power_enabled without
    using propper locks. Using _unlocked version isn't reliable and we cannot
    get this registers when power well is off. So let's avoid getting all DE_IER
    per pipe for now. If someone think this is an useful information it can be
    added later.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   | 12 ------------
 drivers/gpu/drm/i915/i915_drv.h       |  2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c | 19 ++++++++++++++-----
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index aea1a81..579fce6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -702,18 +702,6 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
 				   i, I915_READ(GEN8_GT_IER(i)));
 		}
 
-		for_each_pipe(pipe) {
-			seq_printf(m, "Pipe %c IMR:\t%08x\n",
-				   pipe_name(pipe),
-				   I915_READ(GEN8_DE_PIPE_IMR(pipe)));
-			seq_printf(m, "Pipe %c IIR:\t%08x\n",
-				   pipe_name(pipe),
-				   I915_READ(GEN8_DE_PIPE_IIR(pipe)));
-			seq_printf(m, "Pipe %c IER:\t%08x\n",
-				   pipe_name(pipe),
-				   I915_READ(GEN8_DE_PIPE_IER(pipe)));
-		}
-
 		seq_printf(m, "Display Engine port interrupt mask:\t%08x\n",
 			   I915_READ(GEN8_DE_PORT_IMR));
 		seq_printf(m, "Display Engine port interrupt identity:\t%08x\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 86c84a5..0d7e55f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -317,7 +317,7 @@ struct drm_i915_error_state {
 	u32 eir;
 	u32 pgtbl_er;
 	u32 ier;
-	u32 gtier;
+	u32 gtier[4];
 	u32 ccid;
 	u32 derrmr;
 	u32 forcewake;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0c945e9..ba2d463 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -359,8 +359,12 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 	err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
 	err_printf(m, "EIR: 0x%08x\n", error->eir);
 	err_printf(m, "IER: 0x%08x\n", error->ier);
-	if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
-		err_printf(m, "GTIER: 0x%08x\n", error->gtier);
+	if (IS_BROADWELL(dev)) {
+		for (i = 0; i < 4; i++)
+			err_printf(m, "GTIER gt %d: 0x%08x\n", i,
+				   error->gtier[i]);
+	} else if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
+		err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
 	err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
 	err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
 	err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
@@ -1094,6 +1098,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 				   struct drm_i915_error_state *error)
 {
 	struct drm_device *dev = dev_priv->dev;
+	int i;
 
 	/* General organization
 	 * 1. Registers specific to a single generation
@@ -1105,7 +1110,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 
 	/* 1: Registers specific to a single generation */
 	if (IS_VALLEYVIEW(dev)) {
-		error->gtier = I915_READ(GTIER);
+		error->gtier[0] = I915_READ(GTIER);
 		error->ier = I915_READ(VLV_IER);
 		error->forcewake = I915_READ(FORCEWAKE_VLV);
 	}
@@ -1139,9 +1144,13 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 	if (HAS_HW_CONTEXTS(dev))
 		error->ccid = I915_READ(CCID);
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (IS_BROADWELL(dev)) {
+		error->ier = I915_READ(GEN8_DE_MISC_IER);
+		for (i = 0; i < 4; i++)
+			error->gtier[i] = I915_READ(GEN8_GT_IER(i));
+	} else if (HAS_PCH_SPLIT(dev)) {
 		error->ier = I915_READ(DEIER);
-		error->gtier = I915_READ(GTIER);
+		error->gtier[0] = I915_READ(GTIER);
 	} else if (IS_GEN2(dev)) {
 		error->ier = I915_READ16(IER);
 	} else if (!IS_VALLEYVIEW(dev)) {
-- 
1.9.1

[PATCH] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Rodrigo Vivi]

BDW has many other Display Engine interrupts and GT interrupts registers.
Collecting it properly on gpu_error_state.

On debugfs all was properly listed already but besides we were also listing old
DEIER and GTIER that doesn't exist on BDW anymore. This was causing
unclaimed register messages:

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

v2: Fix small issues of first version and don't read DEIER regs when pipe's
    power well is disabled
v3: bikeshed accepted: use enum pipe pipe instead of int i for pipe interection
v4: Ben notice previous version was checking for display_power_enabled without
    using propper locks. Using _unlocked version isn't reliable and we cannot
    get this registers when power well is off. So let's avoid getting all DE_IER
    per pipe for now. If someone think this is an useful information it can be
    added later.
v5: Ben: put back debugfs stuff that might be coverred by pm_get and use
    	 gen >= 8 trying to predict future.

Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: (v3) Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c | 19 ++++++++++++++-----
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7c25345..73d2308 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -322,7 +322,7 @@ struct drm_i915_error_state {
 	u32 eir;
 	u32 pgtbl_er;
 	u32 ier;
-	u32 gtier;
+	u32 gtier[4];
 	u32 ccid;
 	u32 derrmr;
 	u32 forcewake;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 7c478e6..eab41f9 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -361,8 +361,12 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 	err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
 	err_printf(m, "EIR: 0x%08x\n", error->eir);
 	err_printf(m, "IER: 0x%08x\n", error->ier);
-	if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
-		err_printf(m, "GTIER: 0x%08x\n", error->gtier);
+	if (INTEL_INFO(dev)->gen >= 8) {
+		for (i = 0; i < 4; i++)
+			err_printf(m, "GTIER gt %d: 0x%08x\n", i,
+				   error->gtier[i]);
+	} else if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
+		err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
 	err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
 	err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
 	err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
@@ -1096,6 +1100,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 				   struct drm_i915_error_state *error)
 {
 	struct drm_device *dev = dev_priv->dev;
+	int i;
 
 	/* General organization
 	 * 1. Registers specific to a single generation
@@ -1107,7 +1112,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 
 	/* 1: Registers specific to a single generation */
 	if (IS_VALLEYVIEW(dev)) {
-		error->gtier = I915_READ(GTIER);
+		error->gtier[0] = I915_READ(GTIER);
 		error->ier = I915_READ(VLV_IER);
 		error->forcewake = I915_READ(FORCEWAKE_VLV);
 	}
@@ -1141,9 +1146,13 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 	if (HAS_HW_CONTEXTS(dev))
 		error->ccid = I915_READ(CCID);
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (INTEL_INFO(dev)->gen >= 8) {
+		error->ier = I915_READ(GEN8_DE_MISC_IER);
+		for (i = 0; i < 4; i++)
+			error->gtier[i] = I915_READ(GEN8_GT_IER(i));
+	} else if (HAS_PCH_SPLIT(dev)) {
 		error->ier = I915_READ(DEIER);
-		error->gtier = I915_READ(GTIER);
+		error->gtier[0] = I915_READ(GTIER);
 	} else if (IS_GEN2(dev)) {
 		error->ier = I915_READ16(IER);
 	} else if (!IS_VALLEYVIEW(dev)) {
-- 
1.9.3

Re: [PATCH] drm/i915: Fix DEIER and GTIER collecting for BDW.

[Daniel Vetter]

On Tue, Aug 05, 2014 at 10:07:13AM -0700, Rodrigo Vivi wrote:
> BDW has many other Display Engine interrupts and GT interrupts registers.
> Collecting it properly on gpu_error_state.
> 
> On debugfs all was properly listed already but besides we were also listing old
> DEIER and GTIER that doesn't exist on BDW anymore. This was causing
> unclaimed register messages:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=81701
> 
> v2: Fix small issues of first version and don't read DEIER regs when pipe's
>     power well is disabled
> v3: bikeshed accepted: use enum pipe pipe instead of int i for pipe interection
> v4: Ben notice previous version was checking for display_power_enabled without
>     using propper locks. Using _unlocked version isn't reliable and we cannot
>     get this registers when power well is off. So let's avoid getting all DE_IER
>     per pipe for now. If someone think this is an useful information it can be
>     added later.
> v5: Ben: put back debugfs stuff that might be coverred by pm_get and use
>     	 gen >= 8 trying to predict future.
> 
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
> Reviewed-by: (v3) Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Queued for -next, thanks for the patch.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  2 +-
>  drivers/gpu/drm/i915/i915_gpu_error.c | 19 ++++++++++++++-----
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7c25345..73d2308 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -322,7 +322,7 @@ struct drm_i915_error_state {
>  	u32 eir;
>  	u32 pgtbl_er;
>  	u32 ier;
> -	u32 gtier;
> +	u32 gtier[4];
>  	u32 ccid;
>  	u32 derrmr;
>  	u32 forcewake;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 7c478e6..eab41f9 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -361,8 +361,12 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>  	err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
>  	err_printf(m, "EIR: 0x%08x\n", error->eir);
>  	err_printf(m, "IER: 0x%08x\n", error->ier);
> -	if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
> -		err_printf(m, "GTIER: 0x%08x\n", error->gtier);
> +	if (INTEL_INFO(dev)->gen >= 8) {
> +		for (i = 0; i < 4; i++)
> +			err_printf(m, "GTIER gt %d: 0x%08x\n", i,
> +				   error->gtier[i]);
> +	} else if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
> +		err_printf(m, "GTIER: 0x%08x\n", error->gtier[0]);
>  	err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
>  	err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
>  	err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
> @@ -1096,6 +1100,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>  				   struct drm_i915_error_state *error)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> +	int i;
>  
>  	/* General organization
>  	 * 1. Registers specific to a single generation
> @@ -1107,7 +1112,7 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>  
>  	/* 1: Registers specific to a single generation */
>  	if (IS_VALLEYVIEW(dev)) {
> -		error->gtier = I915_READ(GTIER);
> +		error->gtier[0] = I915_READ(GTIER);
>  		error->ier = I915_READ(VLV_IER);
>  		error->forcewake = I915_READ(FORCEWAKE_VLV);
>  	}
> @@ -1141,9 +1146,13 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>  	if (HAS_HW_CONTEXTS(dev))
>  		error->ccid = I915_READ(CCID);
>  
> -	if (HAS_PCH_SPLIT(dev)) {
> +	if (INTEL_INFO(dev)->gen >= 8) {
> +		error->ier = I915_READ(GEN8_DE_MISC_IER);
> +		for (i = 0; i < 4; i++)
> +			error->gtier[i] = I915_READ(GEN8_GT_IER(i));
> +	} else if (HAS_PCH_SPLIT(dev)) {
>  		error->ier = I915_READ(DEIER);
> -		error->gtier = I915_READ(GTIER);
> +		error->gtier[0] = I915_READ(GTIER);
>  	} else if (IS_GEN2(dev)) {
>  		error->ier = I915_READ16(IER);
>  	} else if (!IS_VALLEYVIEW(dev)) {
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/2] drm/i915: Collect gtier properly on HSW.

[Paulo Zanoni]

2014-07-28 12:19 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> GTIER and DEIER doesn't have same interface on HSW so this "or" operation
> makes the information provided useless.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  1 +
>  drivers/gpu/drm/i915/i915_gpu_error.c | 16 ++++++++++------
>  2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ef38c3b..ccb97f1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -314,6 +314,7 @@ struct drm_i915_error_state {
>         u32 eir;
>         u32 pgtbl_er;
>         u32 ier;
> +       u32 gtier;
>         u32 ccid;
>         u32 derrmr;
>         u32 forcewake;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 0b3f694..372fea3 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -359,6 +359,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>         err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
>         err_printf(m, "EIR: 0x%08x\n", error->eir);
>         err_printf(m, "IER: 0x%08x\n", error->ier);
> +       if (IS_HASWELL(dev))
> +               err_printf(m, "GTIER: 0x%08x\n", error->gtier);
>         err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
>         err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
>         err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
> @@ -1135,13 +1137,15 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>         if (HAS_HW_CONTEXTS(dev))
>                 error->ccid = I915_READ(CCID);
>
> -       if (HAS_PCH_SPLIT(dev))
> +       if (IS_HASWELL(dev)) {
> +               error->ier = I915_READ(DEIER);
> +               error->gtier = I915_READ(GTIER);
> +       } else if (HAS_PCH_SPLIT(dev)) {
>                 error->ier = I915_READ(DEIER) | I915_READ(GTIER);

You did a change for HSW only, but we have these bits since Gen5. Why
don't you do this change for the whole HAS_PCH_SPLIT chunk instead of
adding a HSW-specific piece?

I am not a huge user of these error state files, I can't really think
why we would want to "or" the IER bits, so your patch looks correct to
me.


> -       else {
> -               if (IS_GEN2(dev))
> -                       error->ier = I915_READ16(IER);
> -               else
> -                       error->ier = I915_READ(IER);
> +       } else if (IS_GEN2(dev)) {
> +               error->ier = I915_READ16(IER);
> +       } else {
> +               error->ier = I915_READ(IER);

While reviewing your patch I also noticed that at the top of this
function we set error->ier for VLV, but then at this point we just
overwrite what was previously set. You could write another patch to
fix VLV too :)


>         }
>
>         /* 4: Everything else */
> --
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 1/2] drm/i915: Collect gtier properly on HSW.

[Rodrigo Vivi]

[-- Attachment #1.1: Type: text/plain, Size: 3952 bytes --]

On Wed, Jul 30, 2014 at 10:53 AM, Paulo Zanoni <przanoni@gmail.com> wrote:

> 2014-07-28 12:19 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> > GTIER and DEIER doesn't have same interface on HSW so this "or" operation
> > makes the information provided useless.
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h       |  1 +
> >  drivers/gpu/drm/i915/i915_gpu_error.c | 16 ++++++++++------
> >  2 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> > index ef38c3b..ccb97f1 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -314,6 +314,7 @@ struct drm_i915_error_state {
> >         u32 eir;
> >         u32 pgtbl_er;
> >         u32 ier;
> > +       u32 gtier;
> >         u32 ccid;
> >         u32 derrmr;
> >         u32 forcewake;
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
> b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 0b3f694..372fea3 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -359,6 +359,8 @@ int i915_error_state_to_str(struct
> drm_i915_error_state_buf *m,
> >         err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
> >         err_printf(m, "EIR: 0x%08x\n", error->eir);
> >         err_printf(m, "IER: 0x%08x\n", error->ier);
> > +       if (IS_HASWELL(dev))
> > +               err_printf(m, "GTIER: 0x%08x\n", error->gtier);
> >         err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
> >         err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
> >         err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
> > @@ -1135,13 +1137,15 @@ static void i915_capture_reg_state(struct
> drm_i915_private *dev_priv,
> >         if (HAS_HW_CONTEXTS(dev))
> >                 error->ccid = I915_READ(CCID);
> >
> > -       if (HAS_PCH_SPLIT(dev))
> > +       if (IS_HASWELL(dev)) {
> > +               error->ier = I915_READ(DEIER);
> > +               error->gtier = I915_READ(GTIER);
> > +       } else if (HAS_PCH_SPLIT(dev)) {
> >                 error->ier = I915_READ(DEIER) | I915_READ(GTIER);
>
> You did a change for HSW only, but we have these bits since Gen5. Why
> don't you do this change for the whole HAS_PCH_SPLIT chunk instead of
> adding a HSW-specific piece?
>
> I am not a huge user of these error state files, I can't really think
> why we would want to "or" the IER bits, so your patch looks correct to
> me.
>

I believe before HSW they had the same interface both DEIR and GTIER, but
since I'm splitting it for HSW you are right we can split everywhere else
with or without common interface.


>
>
> > -       else {
> > -               if (IS_GEN2(dev))
> > -                       error->ier = I915_READ16(IER);
> > -               else
> > -                       error->ier = I915_READ(IER);
> > +       } else if (IS_GEN2(dev)) {
> > +               error->ier = I915_READ16(IER);
> > +       } else {
> > +               error->ier = I915_READ(IER);
>
> While reviewing your patch I also noticed that at the top of this
> function we set error->ier for VLV, but then at this point we just
> overwrite what was previously set. You could write another patch to
> fix VLV too :)
>

Yeah, it is messy...  I'll also split for VLV and organize a bit to avoid
overwriting...


>
>
> >         }
> >
> >         /* 4: Everything else */
> > --
> > 1.9.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br

[-- Attachment #1.2: Type: text/html, Size: 6101 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 1/2] drm/i915: Collect gtier properly on HSW.

[Rodrigo Vivi]

GTIER and DEIER doesn't have same interface on HSW so this "or" operation
makes the information provided useless.

v2: since we have gtier variable already let's split for everybody
and avoid the strange | op.
    Also avoid overriding the value that was set for vlv. In this case I
    believe that we should reorganize the whole function, but I'll respect
    the comment that ask to not touch the order and let this organization
    work to be done later.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  1 +
 drivers/gpu/drm/i915/i915_gpu_error.c | 24 ++++++++++++++----------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d604f4f..60227b2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -317,6 +317,7 @@ struct drm_i915_error_state {
 	u32 eir;
 	u32 pgtbl_er;
 	u32 ier;
+	u32 gtier;
 	u32 ccid;
 	u32 derrmr;
 	u32 forcewake;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0b3f694..76c67dd 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -359,6 +359,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 	err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
 	err_printf(m, "EIR: 0x%08x\n", error->eir);
 	err_printf(m, "IER: 0x%08x\n", error->ier);
+	if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
+		err_printf(m, "GTIER: 0x%08x\n", error->gtier);
 	err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
 	err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
 	err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
@@ -1102,7 +1104,8 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 
 	/* 1: Registers specific to a single generation */
 	if (IS_VALLEYVIEW(dev)) {
-		error->ier = I915_READ(GTIER) | I915_READ(VLV_IER);
+		error->gtier = I915_READ(GTIER);
+		error->ier = I915_READ(VLV_IER);
 		error->forcewake = I915_READ(FORCEWAKE_VLV);
 	}
 
@@ -1135,17 +1138,18 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 	if (HAS_HW_CONTEXTS(dev))
 		error->ccid = I915_READ(CCID);
 
-	if (HAS_PCH_SPLIT(dev))
-		error->ier = I915_READ(DEIER) | I915_READ(GTIER);
-	else {
-		if (IS_GEN2(dev))
-			error->ier = I915_READ16(IER);
-		else
-			error->ier = I915_READ(IER);
+	if (HAS_PCH_SPLIT(dev)) {
+		error->ier = I915_READ(DEIER);
+		error->gtier = I915_READ(GTIER);
+	} else if (IS_GEN2(dev)) {
+		error->ier = I915_READ16(IER);
+	} else {
+		error->ier = I915_READ(IER);
 	}
 
-	/* 4: Everything else */
-	error->eir = I915_READ(EIR);
+	/* do not override what was set above for VLV */
+	if (!IS_VALLEYVIEW(dev))
+		error->eir = I915_READ(EIR);
 	error->pgtbl_er = I915_READ(PGTBL_ER);
 
 	i915_get_extra_instdone(dev, error->extra_instdone);
-- 
1.9.1

Re: [PATCH 1/2] drm/i915: Collect gtier properly on HSW.

[Paulo Zanoni]

2014-08-01 6:13 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> GTIER and DEIER doesn't have same interface on HSW so this "or" operation
> makes the information provided useless.
>
> v2: since we have gtier variable already let's split for everybody
> and avoid the strange | op.
>     Also avoid overriding the value that was set for vlv. In this case I
>     believe that we should reorganize the whole function, but I'll respect
>     the comment that ask to not touch the order and let this organization
>     work to be done later.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  1 +
>  drivers/gpu/drm/i915/i915_gpu_error.c | 24 ++++++++++++++----------
>  2 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d604f4f..60227b2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -317,6 +317,7 @@ struct drm_i915_error_state {
>         u32 eir;
>         u32 pgtbl_er;
>         u32 ier;
> +       u32 gtier;
>         u32 ccid;
>         u32 derrmr;
>         u32 forcewake;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 0b3f694..76c67dd 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -359,6 +359,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>         err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
>         err_printf(m, "EIR: 0x%08x\n", error->eir);
>         err_printf(m, "IER: 0x%08x\n", error->ier);
> +       if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
> +               err_printf(m, "GTIER: 0x%08x\n", error->gtier);

Optional bikeshed: you could just check for Gen >= 5 here.


>         err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
>         err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
>         err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
> @@ -1102,7 +1104,8 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>
>         /* 1: Registers specific to a single generation */
>         if (IS_VALLEYVIEW(dev)) {
> -               error->ier = I915_READ(GTIER) | I915_READ(VLV_IER);
> +               error->gtier = I915_READ(GTIER);
> +               error->ier = I915_READ(VLV_IER);
>                 error->forcewake = I915_READ(FORCEWAKE_VLV);
>         }
>
> @@ -1135,17 +1138,18 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>         if (HAS_HW_CONTEXTS(dev))
>                 error->ccid = I915_READ(CCID);
>
> -       if (HAS_PCH_SPLIT(dev))
> -               error->ier = I915_READ(DEIER) | I915_READ(GTIER);
> -       else {
> -               if (IS_GEN2(dev))
> -                       error->ier = I915_READ16(IER);
> -               else
> -                       error->ier = I915_READ(IER);
> +       if (HAS_PCH_SPLIT(dev)) {
> +               error->ier = I915_READ(DEIER);
> +               error->gtier = I915_READ(GTIER);
> +       } else if (IS_GEN2(dev)) {
> +               error->ier = I915_READ16(IER);
> +       } else {
> +               error->ier = I915_READ(IER);

We're still overwriting VLV's IER here.

>         }
>
> -       /* 4: Everything else */
> -       error->eir = I915_READ(EIR);
> +       /* do not override what was set above for VLV */
> +       if (!IS_VALLEYVIEW(dev))
> +               error->eir = I915_READ(EIR);

I don't see EIR being set above on VLV. It was IER, not EIR.


>         error->pgtbl_er = I915_READ(PGTBL_ER);
>
>         i915_get_extra_instdone(dev, error->extra_instdone);
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

[PATCH 1/2] drm/i915: Collect gtier properly on HSW.

[Rodrigo Vivi]

GTIER and DEIER doesn't have same interface on HSW so this "or" operation
makes the information provided useless.

v2: since we have gtier variable already let's split for everybody
and avoid the strange | op.
    Also avoid overriding the value that was set for vlv. In this case I
    believe that we should reorganize the whole function, but I'll respect
    the comment that ask to not touch the order and let this organization
    work to be done later.
v3: moving VLV check to the right place.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  1 +
 drivers/gpu/drm/i915/i915_gpu_error.c | 21 +++++++++++----------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d604f4f..60227b2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -317,6 +317,7 @@ struct drm_i915_error_state {
 	u32 eir;
 	u32 pgtbl_er;
 	u32 ier;
+	u32 gtier;
 	u32 ccid;
 	u32 derrmr;
 	u32 forcewake;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0b3f694..c8f901f 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -359,6 +359,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 	err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
 	err_printf(m, "EIR: 0x%08x\n", error->eir);
 	err_printf(m, "IER: 0x%08x\n", error->ier);
+	if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
+		err_printf(m, "GTIER: 0x%08x\n", error->gtier);
 	err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
 	err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
 	err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
@@ -1102,7 +1104,8 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 
 	/* 1: Registers specific to a single generation */
 	if (IS_VALLEYVIEW(dev)) {
-		error->ier = I915_READ(GTIER) | I915_READ(VLV_IER);
+		error->gtier = I915_READ(GTIER);
+		error->ier = I915_READ(VLV_IER);
 		error->forcewake = I915_READ(FORCEWAKE_VLV);
 	}
 
@@ -1135,16 +1138,14 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
 	if (HAS_HW_CONTEXTS(dev))
 		error->ccid = I915_READ(CCID);
 
-	if (HAS_PCH_SPLIT(dev))
-		error->ier = I915_READ(DEIER) | I915_READ(GTIER);
-	else {
-		if (IS_GEN2(dev))
-			error->ier = I915_READ16(IER);
-		else
-			error->ier = I915_READ(IER);
+	if (HAS_PCH_SPLIT(dev)) {
+		error->ier = I915_READ(DEIER);
+		error->gtier = I915_READ(GTIER);
+	} else if (IS_GEN2(dev)) {
+		error->ier = I915_READ16(IER);
+	} else if (!IS_VALLEYVIEW(dev)) {
+		error->ier = I915_READ(IER);
 	}
-
-	/* 4: Everything else */
 	error->eir = I915_READ(EIR);
 	error->pgtbl_er = I915_READ(PGTBL_ER);
 
-- 
1.9.1

Re: [PATCH 1/2] drm/i915: Collect gtier properly on HSW.

[Paulo Zanoni]

2014-08-01 13:12 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> GTIER and DEIER doesn't have same interface on HSW so this "or" operation
> makes the information provided useless.
>
> v2: since we have gtier variable already let's split for everybody
> and avoid the strange | op.
>     Also avoid overriding the value that was set for vlv. In this case I
>     believe that we should reorganize the whole function, but I'll respect
>     the comment that ask to not touch the order and let this organization
>     work to be done later.
> v3: moving VLV check to the right place.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  1 +
>  drivers/gpu/drm/i915/i915_gpu_error.c | 21 +++++++++++----------
>  2 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d604f4f..60227b2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -317,6 +317,7 @@ struct drm_i915_error_state {
>         u32 eir;
>         u32 pgtbl_er;
>         u32 ier;
> +       u32 gtier;
>         u32 ccid;
>         u32 derrmr;
>         u32 forcewake;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 0b3f694..c8f901f 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -359,6 +359,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>         err_printf(m, "PCI ID: 0x%04x\n", dev->pdev->device);
>         err_printf(m, "EIR: 0x%08x\n", error->eir);
>         err_printf(m, "IER: 0x%08x\n", error->ier);
> +       if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev))
> +               err_printf(m, "GTIER: 0x%08x\n", error->gtier);
>         err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
>         err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
>         err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
> @@ -1102,7 +1104,8 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>
>         /* 1: Registers specific to a single generation */
>         if (IS_VALLEYVIEW(dev)) {
> -               error->ier = I915_READ(GTIER) | I915_READ(VLV_IER);
> +               error->gtier = I915_READ(GTIER);
> +               error->ier = I915_READ(VLV_IER);
>                 error->forcewake = I915_READ(FORCEWAKE_VLV);
>         }
>
> @@ -1135,16 +1138,14 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
>         if (HAS_HW_CONTEXTS(dev))
>                 error->ccid = I915_READ(CCID);
>
> -       if (HAS_PCH_SPLIT(dev))
> -               error->ier = I915_READ(DEIER) | I915_READ(GTIER);
> -       else {
> -               if (IS_GEN2(dev))
> -                       error->ier = I915_READ16(IER);
> -               else
> -                       error->ier = I915_READ(IER);
> +       if (HAS_PCH_SPLIT(dev)) {
> +               error->ier = I915_READ(DEIER);
> +               error->gtier = I915_READ(GTIER);
> +       } else if (IS_GEN2(dev)) {
> +               error->ier = I915_READ16(IER);
> +       } else if (!IS_VALLEYVIEW(dev)) {
> +               error->ier = I915_READ(IER);
>         }
> -
> -       /* 4: Everything else */
>         error->eir = I915_READ(EIR);
>         error->pgtbl_er = I915_READ(PGTBL_ER);
>
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni