* Re: [PATCH] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.
2014-08-21 8:07 [PATCH] drm/i915: Fix to Enable GT/PM Interrupts for cherryview deepak.s
@ 2014-08-20 10:56 ` Ville Syrjälä
2014-08-22 2:13 ` Deepak S
2014-08-22 3:02 ` [PATCH v2] " deepak.s
0 siblings, 2 replies; 9+ messages in thread
From: Ville Syrjälä @ 2014-08-20 10:56 UTC (permalink / raw)
To: deepak.s; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.
2014-08-22 3:02 ` [PATCH v2] " deepak.s
@ 2014-08-21 7:54 ` Ville Syrjälä
2014-08-26 13:54 ` Daniel Vetter
1 sibling, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2014-08-21 7:54 UTC (permalink / raw)
To: deepak.s; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.
@ 2014-08-21 8:07 deepak.s
2014-08-20 10:56 ` Ville Syrjälä
0 siblings, 1 reply; 9+ messages in thread
From: deepak.s @ 2014-08-21 8:07 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.
2014-08-20 10:56 ` Ville Syrjälä
@ 2014-08-22 2:13 ` Deepak S
2014-08-22 3:02 ` [PATCH v2] " deepak.s
1 sibling, 0 replies; 9+ messages in thread
From: Deepak S @ 2014-08-22 2:13 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.
2014-08-20 10:56 ` Ville Syrjälä
2014-08-22 2:13 ` Deepak S
@ 2014-08-22 3:02 ` deepak.s
2014-08-21 7:54 ` Ville Syrjälä
2014-08-26 13:54 ` Daniel Vetter
1 sibling, 2 replies; 9+ messages in thread
From: deepak.s @ 2014-08-22 3:02 UTC (permalink / raw)
To: intel-gfx
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.
2014-08-22 3:02 ` [PATCH v2] " deepak.s
2014-08-21 7:54 ` Ville Syrjälä
@ 2014-08-26 13:54 ` Daniel Vetter
2014-08-29 3:15 ` Deepak S
1 sibling, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2014-08-26 13:54 UTC (permalink / raw)
To: deepak.s; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.
2014-08-29 3:15 ` Deepak S
@ 2014-08-28 5:30 ` Daniel Vetter
2014-09-02 8:23 ` Deepak S
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2014-08-28 5:30 UTC (permalink / raw)
To: Deepak S; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.
2014-08-26 13:54 ` Daniel Vetter
@ 2014-08-29 3:15 ` Deepak S
2014-08-28 5:30 ` Daniel Vetter
0 siblings, 1 reply; 9+ messages in thread
From: Deepak S @ 2014-08-29 3:15 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.
2014-08-28 5:30 ` Daniel Vetter
@ 2014-09-02 8:23 ` Deepak S
0 siblings, 0 replies; 9+ messages in thread
From: Deepak S @ 2014-09-02 8:23 UTC (permalink / raw)
To: Daniel Vetter, Deepak S; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-09-01 8:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-21 8:07 [PATCH] drm/i915: Fix to Enable GT/PM Interrupts for cherryview deepak.s
2014-08-20 10:56 ` Ville Syrjälä
2014-08-22 2:13 ` Deepak S
2014-08-22 3:02 ` [PATCH v2] " deepak.s
2014-08-21 7:54 ` Ville Syrjälä
2014-08-26 13:54 ` Daniel Vetter
2014-08-29 3:15 ` Deepak S
2014-08-28 5:30 ` Daniel Vetter
2014-09-02 8:23 ` Deepak S
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.