[PATCH] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

[PATCH] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

[deepak.s]

From: Deepak S <deepak.s@linux.intel.com>

Programing GT IER interrupts was fumbled while enabling Interrupts for
gen8

This is a regression from
    commit abd58f0175915bed644aa67c8f69dc571b8280e0
    Author: Ben Widawsky <benjamin.widawsky@intel.com>
    Date:   Sat Nov 2 21:07:09 2013 -0700

	drm/i915/bdw: Implement interrupt changes

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d5445e7..48c02bc 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3812,7 +3812,7 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VCS1_IRQ_SHIFT |
 			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT |
 			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VCS2_IRQ_SHIFT,
-		0,
+		dev_priv->pm_rps_events,
 		GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT |
 			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT
 		};
-- 
1.9.1

Re: [PATCH] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

[Ville Syrjälä]

On Thu, Aug 21, 2014 at 01:37:09PM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> Programing GT IER interrupts was fumbled while enabling Interrupts for
> gen8

True, but...

> 
> This is a regression from
>     commit abd58f0175915bed644aa67c8f69dc571b8280e0
>     Author: Ben Widawsky <benjamin.widawsky@intel.com>
>     Date:   Sat Nov 2 21:07:09 2013 -0700
> 
> 	drm/i915/bdw: Implement interrupt changes
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d5445e7..48c02bc 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3812,7 +3812,7 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>  			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VCS1_IRQ_SHIFT |
>  			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT |
>  			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VCS2_IRQ_SHIFT,
> -		0,
> +		dev_priv->pm_rps_events,

.. this would now unmask the rps interrupts already in postinstall which
isn't quite what we want. I think the best solution might be to just
kill the loop below and just init each GT interrupt register set indivually
so that you can pass the correct mask to GT_IMR(2). So I'm thinking simply
something like this:

dev_priv->pm_irq_mask = 0xffffffff;
GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_irq_mask, dev_priv->pm_rps_events);
GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);

>  		GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT |
>  			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT
>  		};
> -- 
> 1.9.1

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

[Deepak S]

On Wednesday 20 August 2014 04:26 PM, Ville Syrjälä wrote:
> On Thu, Aug 21, 2014 at 01:37:09PM +0530, deepak.s@linux.intel.com wrote:
>> From: Deepak S <deepak.s@linux.intel.com>
>>
>> Programing GT IER interrupts was fumbled while enabling Interrupts for
>> gen8
> True, but...
>
>> This is a regression from
>>      commit abd58f0175915bed644aa67c8f69dc571b8280e0
>>      Author: Ben Widawsky <benjamin.widawsky@intel.com>
>>      Date:   Sat Nov 2 21:07:09 2013 -0700
>>
>> 	drm/i915/bdw: Implement interrupt changes
>>
>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_irq.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index d5445e7..48c02bc 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -3812,7 +3812,7 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>>   			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VCS1_IRQ_SHIFT |
>>   			GT_RENDER_USER_INTERRUPT << GEN8_VCS2_IRQ_SHIFT |
>>   			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VCS2_IRQ_SHIFT,
>> -		0,
>> +		dev_priv->pm_rps_events,
> .. this would now unmask the rps interrupts already in postinstall which
> isn't quite what we want. I think the best solution might be to just
> kill the loop below and just init each GT interrupt register set indivually
> so that you can pass the correct mask to GT_IMR(2). So I'm thinking simply
> something like this:
>
> dev_priv->pm_irq_mask = 0xffffffff;
> GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
> GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
> GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_irq_mask, dev_priv->pm_rps_events);
> GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);

Thanks for the review. I will address this.

>>   		GT_RENDER_USER_INTERRUPT << GEN8_VECS_IRQ_SHIFT |
>>   			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT
>>   		};
>> -- 
>> 1.9.1

[PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

[deepak.s]

From: Deepak S <deepak.s@linux.intel.com>

Programing GT IER interrupts was fumbled while enabling Interrupts for
gen8

This is a regression from
    commit abd58f0175915bed644aa67c8f69dc571b8280e0
    Author: Ben Widawsky <benjamin.widawsky@intel.com>
    Date:   Sat Nov 2 21:07:09 2013 -0700

	drm/i915/bdw: Implement interrupt changes

v2: Kill the loop and init GT interrupts (Ville)

Signed-off-by: Deepak S <deepak.s@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d5445e7..c33cf89 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -3799,8 +3799,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
 
 static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	int i;
-
 	/* These are interrupts we'll toggle with the ring mask register */
 	uint32_t gt_interrupts[] = {
 		GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
@@ -3817,10 +3815,11 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT
 		};
 
-	for (i = 0; i < ARRAY_SIZE(gt_interrupts); i++)
-		GEN8_IRQ_INIT_NDX(GT, i, ~gt_interrupts[i], gt_interrupts[i]);
-
 	dev_priv->pm_irq_mask = 0xffffffff;
+	GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
+	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
+	GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_irq_mask, dev_priv->pm_rps_events);
+	GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
 }
 
 static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
-- 
1.9.1

Re: [PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

[Ville Syrjälä]

On Fri, Aug 22, 2014 at 08:32:40AM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> Programing GT IER interrupts was fumbled while enabling Interrupts for
> gen8
> 
> This is a regression from
>     commit abd58f0175915bed644aa67c8f69dc571b8280e0
>     Author: Ben Widawsky <benjamin.widawsky@intel.com>
>     Date:   Sat Nov 2 21:07:09 2013 -0700
> 
> 	drm/i915/bdw: Implement interrupt changes
> 
> v2: Kill the loop and init GT interrupts (Ville)
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d5445e7..c33cf89 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3799,8 +3799,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
>  
>  static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>  {
> -	int i;
> -
>  	/* These are interrupts we'll toggle with the ring mask register */
>  	uint32_t gt_interrupts[] = {
>  		GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
> @@ -3817,10 +3815,11 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>  			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT
>  		};
>  
> -	for (i = 0; i < ARRAY_SIZE(gt_interrupts); i++)
> -		GEN8_IRQ_INIT_NDX(GT, i, ~gt_interrupts[i], gt_interrupts[i]);
> -
>  	dev_priv->pm_irq_mask = 0xffffffff;
> +	GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
> +	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
> +	GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_irq_mask, dev_priv->pm_rps_events);
> +	GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
>  }
>  
>  static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> -- 
> 1.9.1

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

[Daniel Vetter]

On Fri, Aug 22, 2014 at 08:32:40AM +0530, deepak.s@linux.intel.com wrote:
> From: Deepak S <deepak.s@linux.intel.com>
> 
> Programing GT IER interrupts was fumbled while enabling Interrupts for
> gen8
> 
> This is a regression from
>     commit abd58f0175915bed644aa67c8f69dc571b8280e0
>     Author: Ben Widawsky <benjamin.widawsky@intel.com>
>     Date:   Sat Nov 2 21:07:09 2013 -0700
> 
> 	drm/i915/bdw: Implement interrupt changes

_Really_ unlikely that this is a regression from this commit, since that
introduced all the gen8 interrupt handling in the first place. I think
this is because of the reworked interrupt handling over gpu resets, where
we want to keep rps interrupts enabled. But I'm not terribly sure.

I think a more convincing story is that this is an oversight from

commit a6706b45a57a23a613b34793e1414991b60a09c1
Author: Deepak S <deepak.s@linux.intel.com>
Date:   Sat Mar 15 20:23:22 2014 +0530

    drm/i915: Track the enabled PM interrupts in dev_priv

or more precisely (since chv wasn't public back then iirc) we forgot to
add the corresponding patch to -internal.

In any case please clarify the commit message and please also add a few
words about why exactly we need this - I had to dig through git history to
figure this all out.

I've applied the patch already, so you can just reply with the revised
commit message that I should put in.

Thanks, Daniel

> v2: Kill the loop and init GT interrupts (Ville)
> 
> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d5445e7..c33cf89 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -3799,8 +3799,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
>  
>  static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>  {
> -	int i;
> -
>  	/* These are interrupts we'll toggle with the ring mask register */
>  	uint32_t gt_interrupts[] = {
>  		GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
> @@ -3817,10 +3815,11 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>  			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT
>  		};
>  
> -	for (i = 0; i < ARRAY_SIZE(gt_interrupts); i++)
> -		GEN8_IRQ_INIT_NDX(GT, i, ~gt_interrupts[i], gt_interrupts[i]);
> -
>  	dev_priv->pm_irq_mask = 0xffffffff;
> +	GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
> +	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
> +	GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_irq_mask, dev_priv->pm_rps_events);
> +	GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
>  }
>  
>  static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> -- 
> 1.9.1
> 
> _______________________________________________
> 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 v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

[Deepak S]

On Tuesday 26 August 2014 07:24 PM, Daniel Vetter wrote:
> On Fri, Aug 22, 2014 at 08:32:40AM +0530, deepak.s@linux.intel.com wrote:
>> From: Deepak S <deepak.s@linux.intel.com>
>>
>> Programing GT IER interrupts was fumbled while enabling Interrupts for
>> gen8
>>
>> This is a regression from
>>      commit abd58f0175915bed644aa67c8f69dc571b8280e0
>>      Author: Ben Widawsky <benjamin.widawsky@intel.com>
>>      Date:   Sat Nov 2 21:07:09 2013 -0700
>>
>> 	drm/i915/bdw: Implement interrupt changes
> _Really_ unlikely that this is a regression from this commit, since that
> introduced all the gen8 interrupt handling in the first place. I think
> this is because of the reworked interrupt handling over gpu resets, where
> we want to keep rps interrupts enabled. But I'm not terribly sure.
>
> I think a more convincing story is that this is an oversight from
>
> commit a6706b45a57a23a613b34793e1414991b60a09c1
> Author: Deepak S <deepak.s@linux.intel.com>
> Date:   Sat Mar 15 20:23:22 2014 +0530
>
>      drm/i915: Track the enabled PM interrupts in dev_priv
>
> or more precisely (since chv wasn't public back then iirc) we forgot to
> add the corresponding patch to -internal.
>
> In any case please clarify the commit message and please also add a few
> words about why exactly we need this - I had to dig through git history to
> figure this all out.
>
> I've applied the patch already, so you can just reply with the revised
> commit message that I should put in.
>
> Thanks, Daniel

Daniel, This patch is not just for chv right. it affects the gen8(BDW).
Your right we might have missed in (drm/i915: Track the enabled PM interrupts in dev_priv).
In -internal, for chv, We used to program the IER in enable_interrupts routing so it was already taken care.
After the interrupt re-work, we are supposed to add IER in post_irq handler which we missed it.

New commit msg:  Is this fine? Do i need to resubmit the patch?
------------------------------------------------------------------

Programing GT IER interrupts was fumbled while enabling Interrupts for
gen8

We forgot to program PM IER interrupt in gen8_gt_irq_postinstall based on the new  re-worked interrupt
routines.

>> v2: Kill the loop and init GT interrupts (Ville)
>>
>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_irq.c | 9 ++++-----
>>   1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index d5445e7..c33cf89 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -3799,8 +3799,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
>>   
>>   static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>>   {
>> -	int i;
>> -
>>   	/* These are interrupts we'll toggle with the ring mask register */
>>   	uint32_t gt_interrupts[] = {
>>   		GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
>> @@ -3817,10 +3815,11 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>>   			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT
>>   		};
>>   
>> -	for (i = 0; i < ARRAY_SIZE(gt_interrupts); i++)
>> -		GEN8_IRQ_INIT_NDX(GT, i, ~gt_interrupts[i], gt_interrupts[i]);
>> -
>>   	dev_priv->pm_irq_mask = 0xffffffff;
>> +	GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
>> +	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
>> +	GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_irq_mask, dev_priv->pm_rps_events);
>> +	GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
>>   }
>>   
>>   static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

[Daniel Vetter]

On Fri, Aug 29, 2014 at 08:45:21AM +0530, Deepak S wrote:
> 
> On Tuesday 26 August 2014 07:24 PM, Daniel Vetter wrote:
> >On Fri, Aug 22, 2014 at 08:32:40AM +0530, deepak.s@linux.intel.com wrote:
> >>From: Deepak S <deepak.s@linux.intel.com>
> >>
> >>Programing GT IER interrupts was fumbled while enabling Interrupts for
> >>gen8
> >>
> >>This is a regression from
> >>     commit abd58f0175915bed644aa67c8f69dc571b8280e0
> >>     Author: Ben Widawsky <benjamin.widawsky@intel.com>
> >>     Date:   Sat Nov 2 21:07:09 2013 -0700
> >>
> >>	drm/i915/bdw: Implement interrupt changes
> >_Really_ unlikely that this is a regression from this commit, since that
> >introduced all the gen8 interrupt handling in the first place. I think
> >this is because of the reworked interrupt handling over gpu resets, where
> >we want to keep rps interrupts enabled. But I'm not terribly sure.
> >
> >I think a more convincing story is that this is an oversight from
> >
> >commit a6706b45a57a23a613b34793e1414991b60a09c1
> >Author: Deepak S <deepak.s@linux.intel.com>
> >Date:   Sat Mar 15 20:23:22 2014 +0530
> >
> >     drm/i915: Track the enabled PM interrupts in dev_priv
> >
> >or more precisely (since chv wasn't public back then iirc) we forgot to
> >add the corresponding patch to -internal.
> >
> >In any case please clarify the commit message and please also add a few
> >words about why exactly we need this - I had to dig through git history to
> >figure this all out.
> >
> >I've applied the patch already, so you can just reply with the revised
> >commit message that I should put in.
> >
> >Thanks, Daniel
> 
> Daniel, This patch is not just for chv right. it affects the gen8(BDW).
> Your right we might have missed in (drm/i915: Track the enabled PM interrupts in dev_priv).
> In -internal, for chv, We used to program the IER in enable_interrupts routing so it was already taken care.
> After the interrupt re-work, we are supposed to add IER in post_irq handler which we missed it.
> 
> New commit msg:  Is this fine? Do i need to resubmit the patch?

Yeah fine and thanks for confirming. I've adjusted the commit message.
-Daniel

> ------------------------------------------------------------------
> 
> Programing GT IER interrupts was fumbled while enabling Interrupts for
> gen8
> 
> We forgot to program PM IER interrupt in gen8_gt_irq_postinstall based on the new  re-worked interrupt
> routines.
> 
> >>v2: Kill the loop and init GT interrupts (Ville)
> >>
> >>Signed-off-by: Deepak S <deepak.s@linux.intel.com>
> >>---
> >>  drivers/gpu/drm/i915/i915_irq.c | 9 ++++-----
> >>  1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >>index d5445e7..c33cf89 100644
> >>--- a/drivers/gpu/drm/i915/i915_irq.c
> >>+++ b/drivers/gpu/drm/i915/i915_irq.c
> >>@@ -3799,8 +3799,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
> >>  static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
> >>  {
> >>-	int i;
> >>-
> >>  	/* These are interrupts we'll toggle with the ring mask register */
> >>  	uint32_t gt_interrupts[] = {
> >>  		GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
> >>@@ -3817,10 +3815,11 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
> >>  			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT
> >>  		};
> >>-	for (i = 0; i < ARRAY_SIZE(gt_interrupts); i++)
> >>-		GEN8_IRQ_INIT_NDX(GT, i, ~gt_interrupts[i], gt_interrupts[i]);
> >>-
> >>  	dev_priv->pm_irq_mask = 0xffffffff;
> >>+	GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
> >>+	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
> >>+	GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_irq_mask, dev_priv->pm_rps_events);
> >>+	GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
> >>  }
> >>  static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
> >>-- 
> >>1.9.1
> >>
> >>_______________________________________________
> >>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 v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.

[Deepak S]

On Thursday 28 August 2014 11:00 AM, Daniel Vetter wrote:
> On Fri, Aug 29, 2014 at 08:45:21AM +0530, Deepak S wrote:
>> On Tuesday 26 August 2014 07:24 PM, Daniel Vetter wrote:
>>> On Fri, Aug 22, 2014 at 08:32:40AM +0530, deepak.s@linux.intel.com wrote:
>>>> From: Deepak S <deepak.s@linux.intel.com>
>>>>
>>>> Programing GT IER interrupts was fumbled while enabling Interrupts for
>>>> gen8
>>>>
>>>> This is a regression from
>>>>      commit abd58f0175915bed644aa67c8f69dc571b8280e0
>>>>      Author: Ben Widawsky <benjamin.widawsky@intel.com>
>>>>      Date:   Sat Nov 2 21:07:09 2013 -0700
>>>>
>>>> 	drm/i915/bdw: Implement interrupt changes
>>> _Really_ unlikely that this is a regression from this commit, since that
>>> introduced all the gen8 interrupt handling in the first place. I think
>>> this is because of the reworked interrupt handling over gpu resets, where
>>> we want to keep rps interrupts enabled. But I'm not terribly sure.
>>>
>>> I think a more convincing story is that this is an oversight from
>>>
>>> commit a6706b45a57a23a613b34793e1414991b60a09c1
>>> Author: Deepak S <deepak.s@linux.intel.com>
>>> Date:   Sat Mar 15 20:23:22 2014 +0530
>>>
>>>      drm/i915: Track the enabled PM interrupts in dev_priv
>>>
>>> or more precisely (since chv wasn't public back then iirc) we forgot to
>>> add the corresponding patch to -internal.
>>>
>>> In any case please clarify the commit message and please also add a few
>>> words about why exactly we need this - I had to dig through git history to
>>> figure this all out.
>>>
>>> I've applied the patch already, so you can just reply with the revised
>>> commit message that I should put in.
>>>
>>> Thanks, Daniel
>> Daniel, This patch is not just for chv right. it affects the gen8(BDW).
>> Your right we might have missed in (drm/i915: Track the enabled PM interrupts in dev_priv).
>> In -internal, for chv, We used to program the IER in enable_interrupts routing so it was already taken care.
>> After the interrupt re-work, we are supposed to add IER in post_irq handler which we missed it.
>>
>> New commit msg:  Is this fine? Do i need to resubmit the patch?
> Yeah fine and thanks for confirming. I've adjusted the commit message.
> -Daniel

Thanks Daniel.

>> ------------------------------------------------------------------
>>
>> Programing GT IER interrupts was fumbled while enabling Interrupts for
>> gen8
>>
>> We forgot to program PM IER interrupt in gen8_gt_irq_postinstall based on the new  re-worked interrupt
>> routines.
>>
>>>> v2: Kill the loop and init GT interrupts (Ville)
>>>>
>>>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/i915_irq.c | 9 ++++-----
>>>>   1 file changed, 4 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>>>> index d5445e7..c33cf89 100644
>>>> --- a/drivers/gpu/drm/i915/i915_irq.c
>>>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>>>> @@ -3799,8 +3799,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
>>>>   static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>>>>   {
>>>> -	int i;
>>>> -
>>>>   	/* These are interrupts we'll toggle with the ring mask register */
>>>>   	uint32_t gt_interrupts[] = {
>>>>   		GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
>>>> @@ -3817,10 +3815,11 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
>>>>   			GT_CONTEXT_SWITCH_INTERRUPT << GEN8_VECS_IRQ_SHIFT
>>>>   		};
>>>> -	for (i = 0; i < ARRAY_SIZE(gt_interrupts); i++)
>>>> -		GEN8_IRQ_INIT_NDX(GT, i, ~gt_interrupts[i], gt_interrupts[i]);
>>>> -
>>>>   	dev_priv->pm_irq_mask = 0xffffffff;
>>>> +	GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]);
>>>> +	GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]);
>>>> +	GEN8_IRQ_INIT_NDX(GT, 2, dev_priv->pm_irq_mask, dev_priv->pm_rps_events);
>>>> +	GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]);
>>>>   }
>>>>   static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
>>>> -- 
>>>> 1.9.1
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx