[PATCH] drm/i915/guc: make forwarding of VBlanks conditional

[PATCH] drm/i915/guc: make forwarding of VBlanks conditional

[Dave Gordon]

Apparently we shouldn't forward VBlanks to the GuC unconditionally,
as it may trigger a race condition in the GuC's internal dispatcher.

From an internal message from Harsh Chheda <harsh.j.chheda@intel.com>
>
> If the context has switched out the [GuC] scheduler should know so
> that it can  put the context in wait state and schedule something
> else.  If the context which is waiting on Vblank is still on the CS
> then [the GuC] scheduler does not need to know. If in this case wait
> on Vblank is sent to Guc while the context is still on CS there is
> an edge condition when the Vblank may get satisfied right after
> sending interrupt to guc and context may complete. Scheduler will
> then incorrectly mark the context to be in wait state.

So we need to change this register setting to GFX_FORWARD_VBLANK_COND.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Cc: Harsh Chheda <harsh.j.chheda@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 8fe96a2..af9f51e 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -106,7 +106,7 @@ static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv)
 	u32 tmp;
 
 	/* tell all command streamers to forward interrupts and vblank to GuC */
-	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS);
+	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_COND);
 	irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
 	for_each_engine(engine, dev_priv)
 		I915_WRITE(RING_MODE_GEN7(engine), irqs);
-- 
1.9.1

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

Re: [PATCH] drm/i915/guc: make forwarding of VBlanks conditional

[Ville Syrjälä]

On Fri, Jun 17, 2016 at 02:42:39PM +0100, Dave Gordon wrote:
> Apparently we shouldn't forward VBlanks to the GuC unconditionally,
> as it may trigger a race condition in the GuC's internal dispatcher.
> 
> From an internal message from Harsh Chheda <harsh.j.chheda@intel.com>
> >
> > If the context has switched out the [GuC] scheduler should know so
> > that it can  put the context in wait state and schedule something
> > else.  If the context which is waiting on Vblank is still on the CS
> > then [the GuC] scheduler does not need to know. If in this case wait
> > on Vblank is sent to Guc while the context is still on CS there is
> > an edge condition when the Vblank may get satisfied right after
> > sending interrupt to guc and context may complete. Scheduler will
> > then incorrectly mark the context to be in wait state.
> 
> So we need to change this register setting to GFX_FORWARD_VBLANK_COND.

No one has ever managed to explain why the guc needs to get vblanks
in the first place. The original commit message adding this stuff is
not helpful at all.

> 
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> Cc: Harsh Chheda <harsh.j.chheda@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 8fe96a2..af9f51e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -106,7 +106,7 @@ static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv)
>  	u32 tmp;
>  
>  	/* tell all command streamers to forward interrupts and vblank to GuC */
> -	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS);
> +	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_COND);
>  	irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
>  	for_each_engine(engine, dev_priv)
>  		I915_WRITE(RING_MODE_GEN7(engine), irqs);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/guc: make forwarding of VBlanks conditional

[Daniel Vetter]

On Fri, Jun 17, 2016 at 04:51:34PM +0300, Ville Syrjälä wrote:
> On Fri, Jun 17, 2016 at 02:42:39PM +0100, Dave Gordon wrote:
> > Apparently we shouldn't forward VBlanks to the GuC unconditionally,
> > as it may trigger a race condition in the GuC's internal dispatcher.
> > 
> > From an internal message from Harsh Chheda <harsh.j.chheda@intel.com>
> > >
> > > If the context has switched out the [GuC] scheduler should know so
> > > that it can  put the context in wait state and schedule something
> > > else.  If the context which is waiting on Vblank is still on the CS
> > > then [the GuC] scheduler does not need to know. If in this case wait
> > > on Vblank is sent to Guc while the context is still on CS there is
> > > an edge condition when the Vblank may get satisfied right after
> > > sending interrupt to guc and context may complete. Scheduler will
> > > then incorrectly mark the context to be in wait state.
> > 
> > So we need to change this register setting to GFX_FORWARD_VBLANK_COND.
> 
> No one has ever managed to explain why the guc needs to get vblanks
> in the first place. The original commit message adding this stuff is
> not helpful at all.

It's used for the guc's magic slpc so that it can try to hit flips at
vrefresh. I maintain that this design is bonghits, and we should forward
_any_ display interrupts to guc. Instead the kernel should tell guc when
it's doing a sloppy job and missing deadlines. Because the kernel actually
knows, and doesn't have to infer what might be going on by watching
vblanks and flip_complete interrupts.

Also, with atomic we won't be using cs flips any more, who knows whether
that guc magic even still works with that ...

So I'd vote to just stop telling guc this stuff and be done with it.
-Daniel

> 
> > 
> > Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> > Cc: Harsh Chheda <harsh.j.chheda@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_guc_loader.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> > index 8fe96a2..af9f51e 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> > @@ -106,7 +106,7 @@ static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv)
> >  	u32 tmp;
> >  
> >  	/* tell all command streamers to forward interrupts and vblank to GuC */
> > -	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS);
> > +	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_COND);
> >  	irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
> >  	for_each_engine(engine, dev_priv)
> >  		I915_WRITE(RING_MODE_GEN7(engine), irqs);
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for drm/i915/guc: make forwarding of VBlanks conditional

[Patchwork]

== Series Details ==

Series: drm/i915/guc: make forwarding of VBlanks conditional
URL   : https://patchwork.freedesktop.org/series/8835/
State : failure

== Summary ==

Series 8835v1 drm/i915/guc: make forwarding of VBlanks conditional
http://patchwork.freedesktop.org/api/1.0/series/8835/revisions/1/mbox

Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> INCOMPLETE (fi-snb-i7-2600)
        Subgroup suspend-read-crc-pipe-a:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)
        Subgroup suspend-read-crc-pipe-c:
                skip       -> DMESG-WARN (ro-bdw-i5-5250u)

fi-snb-i7-2600   total:189  pass:156  dwarn:1   dfail:0   fail:0   skip:31 
ro-bdw-i5-5250u  total:213  pass:196  dwarn:4   dfail:0   fail:0   skip:13 
ro-bdw-i7-5557U  total:213  pass:197  dwarn:1   dfail:0   fail:0   skip:15 
ro-bdw-i7-5600u  total:213  pass:184  dwarn:1   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:213  pass:171  dwarn:1   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:213  pass:172  dwarn:1   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:213  pass:189  dwarn:1   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:213  pass:189  dwarn:1   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:213  pass:149  dwarn:1   dfail:0   fail:1   skip:62 
ro-ilk1-i5-650   total:208  pass:149  dwarn:1   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:213  pass:180  dwarn:1   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:213  pass:184  dwarn:1   dfail:0   fail:0   skip:28 
ro-skl3-i5-6260u total:213  pass:201  dwarn:1   dfail:0   fail:0   skip:11 
ro-snb-i7-2620M  total:213  pass:173  dwarn:1   dfail:0   fail:1   skip:38 
fi-hsw-i7-4770k failed to connect after reboot
fi-skl-i5-6260u failed to connect after reboot
fi-skl-i7-6700k failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1213/

011c1da drm-intel-nightly: 2016y-06m-17d-13h-33m-01s UTC integration manifest
86477af drm/i915/guc: make forwarding of VBlanks conditional

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

Re: [PATCH] drm/i915/guc: make forwarding of VBlanks conditional

[Ville Syrjälä]

On Fri, Jun 17, 2016 at 04:14:48PM +0200, Daniel Vetter wrote:
> On Fri, Jun 17, 2016 at 04:51:34PM +0300, Ville Syrjälä wrote:
> > On Fri, Jun 17, 2016 at 02:42:39PM +0100, Dave Gordon wrote:
> > > Apparently we shouldn't forward VBlanks to the GuC unconditionally,
> > > as it may trigger a race condition in the GuC's internal dispatcher.
> > > 
> > > From an internal message from Harsh Chheda <harsh.j.chheda@intel.com>
> > > >
> > > > If the context has switched out the [GuC] scheduler should know so
> > > > that it can  put the context in wait state and schedule something
> > > > else.  If the context which is waiting on Vblank is still on the CS
> > > > then [the GuC] scheduler does not need to know. If in this case wait
> > > > on Vblank is sent to Guc while the context is still on CS there is
> > > > an edge condition when the Vblank may get satisfied right after
> > > > sending interrupt to guc and context may complete. Scheduler will
> > > > then incorrectly mark the context to be in wait state.
> > > 
> > > So we need to change this register setting to GFX_FORWARD_VBLANK_COND.
> > 
> > No one has ever managed to explain why the guc needs to get vblanks
> > in the first place. The original commit message adding this stuff is
> > not helpful at all.
> 
> It's used for the guc's magic slpc so that it can try to hit flips at
> vrefresh.

Is that a documented fact, or just an assumption? There's nothing
in the commit message, there's no comment in the code, and I don't
see anything in the spec either.

> I maintain that this design is bonghits, and we should forward

I assume you mean "should not".

> _any_ display interrupts to guc. Instead the kernel should tell guc when
> it's doing a sloppy job and missing deadlines. Because the kernel actually
> knows, and doesn't have to infer what might be going on by watching
> vblanks and flip_complete interrupts.
> 
> Also, with atomic we won't be using cs flips any more, who knows whether
> that guc magic even still works with that ...
> 
> So I'd vote to just stop telling guc this stuff and be done with it.

I would have to agree.

> -Daniel
> 
> > 
> > > 
> > > Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> > > Cc: Harsh Chheda <harsh.j.chheda@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_guc_loader.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> > > index 8fe96a2..af9f51e 100644
> > > --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> > > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> > > @@ -106,7 +106,7 @@ static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv)
> > >  	u32 tmp;
> > >  
> > >  	/* tell all command streamers to forward interrupts and vblank to GuC */
> > > -	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS);
> > > +	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_COND);
> > >  	irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
> > >  	for_each_engine(engine, dev_priv)
> > >  		I915_WRITE(RING_MODE_GEN7(engine), irqs);
> > > -- 
> > > 1.9.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/guc: don't ever forward VBlank to the GuC

[Dave Gordon]

If a context waiting for VBlank were switched out, the GuC would
have to receive the VBlank interrupt so that it could resubmit
the context. However, we don't use the GuC internal scheduler,
and we always set the CTX_CTRL_INHIBIT_SYN_CTX_SWITCH bit in the
RING_CONTEXT_CONTROL register, so this case cannot arise.

Consequently, the GuC doesn't need to see VBlanks, and we may be
waking it up unnecessarily by sending them. So let's not ...

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 8fe96a2..5d989d2 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -105,8 +105,8 @@ static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv)
 	int irqs;
 	u32 tmp;
 
-	/* tell all command streamers to forward interrupts and vblank to GuC */
-	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS);
+	/* tell all command streamers to forward interrupts (but not vblank) to GuC */
+	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_NEVER);
 	irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
 	for_each_engine(engine, dev_priv)
 		I915_WRITE(RING_MODE_GEN7(engine), irqs);
-- 
1.9.1

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

✓ Ro.CI.BAT: success for drm/i915/guc: make forwarding of VBlanks conditional (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/guc: make forwarding of VBlanks conditional (rev2)
URL   : https://patchwork.freedesktop.org/series/8835/
State : success

== Summary ==

Series 8835v2 drm/i915/guc: make forwarding of VBlanks conditional
http://patchwork.freedesktop.org/api/1.0/series/8835/revisions/2/mbox


fi-skl-i7-6700k  total:223  pass:18   dwarn:0   dfail:0   fail:0   skip:205
ro-bsw-n3050     total:223  pass:18   dwarn:0   dfail:0   fail:0   skip:205
ro-byt-n2820     total:223  pass:17   dwarn:0   dfail:0   fail:0   skip:206
ro-ilk1-i5-650   total:218  pass:11   dwarn:0   dfail:0   fail:0   skip:207
ro-ivb-i7-3770   total:223  pass:17   dwarn:0   dfail:0   fail:0   skip:206
ro-ivb2-i7-3770  total:223  pass:21   dwarn:0   dfail:0   fail:0   skip:202
ro-snb-i7-2620M  total:223  pass:20   dwarn:0   dfail:0   fail:0   skip:203
fi-hsw-i7-4770k failed to connect after reboot
fi-skl-i5-6260u failed to connect after reboot
fi-snb-i7-2600 failed to connect after reboot
ro-bdw-i5-5250u failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot
ro-hsw-i3-4010u failed to connect after reboot
ro-hsw-i7-4770r failed to connect after reboot
ro-ilk-i7-620lm failed to connect after reboot
ro-skl3-i5-6260u failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1235/

edc0e9e drm-intel-nightly: 2016y-06m-20d-09h-47m-30s UTC integration manifest
b4e6e1c drm/i915/guc: don't ever forward VBlank to the GuC

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

Re: [PATCH] drm/i915/guc: make forwarding of VBlanks conditional

[Daniel Vetter]

On Fri, Jun 17, 2016 at 06:00:15PM +0300, Ville Syrjälä wrote:
> On Fri, Jun 17, 2016 at 04:14:48PM +0200, Daniel Vetter wrote:
> > On Fri, Jun 17, 2016 at 04:51:34PM +0300, Ville Syrjälä wrote:
> > > On Fri, Jun 17, 2016 at 02:42:39PM +0100, Dave Gordon wrote:
> > > > Apparently we shouldn't forward VBlanks to the GuC unconditionally,
> > > > as it may trigger a race condition in the GuC's internal dispatcher.
> > > > 
> > > > From an internal message from Harsh Chheda <harsh.j.chheda@intel.com>
> > > > >
> > > > > If the context has switched out the [GuC] scheduler should know so
> > > > > that it can  put the context in wait state and schedule something
> > > > > else.  If the context which is waiting on Vblank is still on the CS
> > > > > then [the GuC] scheduler does not need to know. If in this case wait
> > > > > on Vblank is sent to Guc while the context is still on CS there is
> > > > > an edge condition when the Vblank may get satisfied right after
> > > > > sending interrupt to guc and context may complete. Scheduler will
> > > > > then incorrectly mark the context to be in wait state.
> > > > 
> > > > So we need to change this register setting to GFX_FORWARD_VBLANK_COND.
> > > 
> > > No one has ever managed to explain why the guc needs to get vblanks
> > > in the first place. The original commit message adding this stuff is
> > > not helpful at all.
> > 
> > It's used for the guc's magic slpc so that it can try to hit flips at
> > vrefresh.
> 
> Is that a documented fact, or just an assumption? There's nothing
> in the commit message, there's no comment in the code, and I don't
> see anything in the spec either.

Well there's a magic mode where we tell guc to hit a vrefresh and it does
magic. Crucially we don't tell guc when it missed or when we flip, it
seems to observe all that through interrupts it gets from the display
block. But that's all inferences indeed, I have know idea what it does for
real.

> > I maintain that this design is bonghits, and we should forward
> 
> I assume you mean "should not".

yup.
-Daniel

> > _any_ display interrupts to guc. Instead the kernel should tell guc when
> > it's doing a sloppy job and missing deadlines. Because the kernel actually
> > knows, and doesn't have to infer what might be going on by watching
> > vblanks and flip_complete interrupts.
> > 
> > Also, with atomic we won't be using cs flips any more, who knows whether
> > that guc magic even still works with that ...
> > 
> > So I'd vote to just stop telling guc this stuff and be done with it.
> 
> I would have to agree.
> 
> > -Daniel
> > 
> > > 
> > > > 
> > > > Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> > > > Cc: Harsh Chheda <harsh.j.chheda@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_guc_loader.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> > > > index 8fe96a2..af9f51e 100644
> > > > --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> > > > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> > > > @@ -106,7 +106,7 @@ static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv)
> > > >  	u32 tmp;
> > > >  
> > > >  	/* tell all command streamers to forward interrupts and vblank to GuC */
> > > > -	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS);
> > > > +	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_COND);
> > > >  	irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
> > > >  	for_each_engine(engine, dev_priv)
> > > >  		I915_WRITE(RING_MODE_GEN7(engine), irqs);
> > > > -- 
> > > > 1.9.1
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel OTC
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/guc: don't ever forward VBlank to the GuC

[Daniel Vetter]

On Mon, Jun 20, 2016 at 11:23:31AM +0100, Dave Gordon wrote:
> If a context waiting for VBlank were switched out, the GuC would
> have to receive the VBlank interrupt so that it could resubmit
> the context. However, we don't use the GuC internal scheduler,
> and we always set the CTX_CTRL_INHIBIT_SYN_CTX_SWITCH bit in the
> RING_CONTEXT_CONTROL register, so this case cannot arise.
> 
> Consequently, the GuC doesn't need to see VBlanks, and we may be
> waking it up unnecessarily by sending them. So let's not ...
> 
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>

Missing v2, and commit message should mention why we shut vblanks down
entirely for the guc. With that fixed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'm soonish on vacations, so pls ask Tvrkto or someone else to push.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_guc_loader.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index 8fe96a2..5d989d2 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -105,8 +105,8 @@ static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv)
>  	int irqs;
>  	u32 tmp;
>  
> -	/* tell all command streamers to forward interrupts and vblank to GuC */
> -	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS);
> +	/* tell all command streamers to forward interrupts (but not vblank) to GuC */
> +	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_NEVER);
>  	irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
>  	for_each_engine(engine, dev_priv)
>  		I915_WRITE(RING_MODE_GEN7(engine), irqs);
> -- 
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx