[PATCH] drm/i915: Always wake the device to flush the GTT

[PATCH] drm/i915: Always wake the device to flush the GTT

[Chris Wilson]

Experimental patch for bxt/gem_pwrite.
---
 drivers/gpu/drm/i915/i915_gem.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 37fbc64d9ffe..4f6af401320c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -695,12 +695,11 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 	switch (obj->base.write_domain) {
 	case I915_GEM_DOMAIN_GTT:
 		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
-			if (intel_runtime_pm_get_if_in_use(dev_priv)) {
-				spin_lock_irq(&dev_priv->uncore.lock);
-				POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
-				spin_unlock_irq(&dev_priv->uncore.lock);
-				intel_runtime_pm_put(dev_priv);
-			}
+			intel_runtime_pm_get(dev_priv);
+			spin_lock_irq(&dev_priv->uncore.lock);
+			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
+			spin_unlock_irq(&dev_priv->uncore.lock);
+			intel_runtime_pm_put(dev_priv);
 		}
 
 		intel_fb_obj_flush(obj,
-- 
2.14.1

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

✓ Fi.CI.BAT: success for drm/i915: Always wake the device to flush the GTT

[Patchwork]

== Series Details ==

Series: drm/i915: Always wake the device to flush the GTT
URL   : https://patchwork.freedesktop.org/series/29436/
State : success

== Summary ==

Series 29436v1 drm/i915: Always wake the device to flush the GTT
https://patchwork.freedesktop.org/api/1.0/series/29436/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2600) fdo#100215 +1
        Subgroup basic-flip-after-cursor-varying-size:
                fail       -> PASS       (fi-hsw-4770) fdo#102402 +1

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#102402 https://bugs.freedesktop.org/show_bug.cgi?id=102402

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:451s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:438s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:361s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:539s
fi-bwr-2160      total:279  pass:184  dwarn:0   dfail:0   fail:0   skip:95  time:252s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:524s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:521s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:512s
fi-elk-e7500     total:279  pass:230  dwarn:0   dfail:0   fail:0   skip:49  time:436s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:611s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:443s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:427s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:507s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:469s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:478s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:597s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:596s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:524s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:473s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:479s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:487s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:434s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:488s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:547s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:408s

ee53909d971df42daac0b870cf7c091f45f1f6b9 drm-tip: 2017y-08m-28d-15h-03m-59s UTC integration manifest
2dc5cdc900aa drm/i915: Always wake the device to flush the GTT

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5511/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Always wake the device to flush the GTT

[Patchwork]

== Series Details ==

Series: drm/i915: Always wake the device to flush the GTT
URL   : https://patchwork.freedesktop.org/series/29436/
State : success

== Summary ==

Test kms_flip:
        Subgroup plain-flip-ts-check:
                fail       -> PASS       (shard-hsw)
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2230 pass:1230 dwarn:0   dfail:0   fail:18  skip:982 time:9646s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5511/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for drm/i915: Always wake the device to flush the GTT

[Chris Wilson]

Quoting Patchwork (2017-08-28 21:15:28)
> == Series Details ==
> 
> Series: drm/i915: Always wake the device to flush the GTT
> URL   : https://patchwork.freedesktop.org/series/29436/
> State : success
> 
> == Summary ==
> 
> Test kms_flip:
>         Subgroup plain-flip-ts-check:
>                 fail       -> PASS       (shard-hsw)
> Test kms_setmode:
>         Subgroup basic:
>                 pass       -> FAIL       (shard-hsw) fdo#99912
> 
> fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
> 
> shard-hsw        total:2230 pass:1230 dwarn:0   dfail:0   fail:18  skip:982 time:9646s
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5511/shards.html

Hard to tell if the flips are fixed since this only shows the changes,
but I need to see the full list. Anyone know how to get the actual
results?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for drm/i915: Always wake the device to flush the GTT

[Saarinen, Jani]

HI, 
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Chris Wilson
> Sent: Tuesday, August 29, 2017 12:59 PM
> To: intel-gfx@lists.freedesktop.org; Patchwork
> <patchwork@emeril.freedesktop.org>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Always wake the
> device to flush the GTT
> 
> Quoting Patchwork (2017-08-28 21:15:28)
> > == Series Details ==
> >
> > Series: drm/i915: Always wake the device to flush the GTT
> > URL   : https://patchwork.freedesktop.org/series/29436/
> > State : success
> >
> > == Summary ==
> >
> > Test kms_flip:
> >         Subgroup plain-flip-ts-check:
> >                 fail       -> PASS       (shard-hsw)
> > Test kms_setmode:
> >         Subgroup basic:
> >                 pass       -> FAIL       (shard-hsw) fdo#99912
> >
> > fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
> >
> > shard-hsw        total:2230 pass:1230 dwarn:0   dfail:0   fail:18  skip:982
> time:9646s
> >
> > == Logs ==
> >
> > For more details see:
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5511/shards.html
You mean https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5511/shards-all.html
> 
> Hard to tell if the flips are fixed since this only shows the changes, but I need
> to see the full list. Anyone know how to get the actual results?
> -Chris

Br,
Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo


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

[PATCH] drm/i915: Always wake the device to flush the GTT

[Chris Wilson]

Since we hold the device wakeref when writing through the GTT (otherwise
the writes would fail), we presumed that before the device sleeps those
writes would naturally be flushed and that we wouldn't need our mmio
read trick. However, that presumption seems false and a sleepy bxt seems
to require us to always manually flush the GTT writes prior to direct
access.

Fixes: e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 43834dee4e8d..890fe2802973 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -695,12 +695,11 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 	switch (obj->base.write_domain) {
 	case I915_GEM_DOMAIN_GTT:
 		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
-			if (intel_runtime_pm_get_if_in_use(dev_priv)) {
-				spin_lock_irq(&dev_priv->uncore.lock);
-				POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
-				spin_unlock_irq(&dev_priv->uncore.lock);
-				intel_runtime_pm_put(dev_priv);
-			}
+			intel_runtime_pm_get(dev_priv);
+			spin_lock_irq(&dev_priv->uncore.lock);
+			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
+			spin_unlock_irq(&dev_priv->uncore.lock);
+			intel_runtime_pm_put(dev_priv);
 		}
 
 		intel_fb_obj_flush(obj,
-- 
2.14.1

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

Re: [PATCH] drm/i915: Always wake the device to flush the GTT

[Joonas Lahtinen]

On Tue, 2017-08-29 at 11:33 +0100, Chris Wilson wrote:
> Since we hold the device wakeref when writing through the GTT (otherwise
> the writes would fail), we presumed that before the device sleeps those
> writes would naturally be flushed and that we wouldn't need our mmio
> read trick. However, that presumption seems false and a sleepy bxt seems
> to require us to always manually flush the GTT writes prior to direct
> access.
> 
> Fixes: e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Got any Bugzilla, Testcase, Tested-by?

Does what the message describes.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Always wake the device to flush the GTT

[Chris Wilson]

Quoting Joonas Lahtinen (2017-08-29 15:54:06)
> On Tue, 2017-08-29 at 11:33 +0100, Chris Wilson wrote:
> > Since we hold the device wakeref when writing through the GTT (otherwise
> > the writes would fail), we presumed that before the device sleeps those
> > writes would naturally be flushed and that we wouldn't need our mmio
> > read trick. However, that presumption seems false and a sleepy bxt seems
> > to require us to always manually flush the GTT writes prior to direct
> > access.
> > 
> > Fixes: e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Got any Bugzilla, Testcase, Tested-by?

Original bugzilla hasn't been reopened, so I its looks like they were
happy enough with the original patches that fixed the problem on my bxt.
The testcase seems to be very system dependent, my suspicion is that it
has to do with the wacky runtime pm exhibited by CI bxt.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Always wake the device to flush the GTT

[Daniel Vetter]

On Tue, Aug 29, 2017 at 03:59:36PM +0100, Chris Wilson wrote:
> Quoting Joonas Lahtinen (2017-08-29 15:54:06)
> > On Tue, 2017-08-29 at 11:33 +0100, Chris Wilson wrote:
> > > Since we hold the device wakeref when writing through the GTT (otherwise
> > > the writes would fail), we presumed that before the device sleeps those
> > > writes would naturally be flushed and that we wouldn't need our mmio
> > > read trick. However, that presumption seems false and a sleepy bxt seems
> > > to require us to always manually flush the GTT writes prior to direct
> > > access.
> > > 
> > > Fixes: e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > 
> > Got any Bugzilla, Testcase, Tested-by?
> 
> Original bugzilla hasn't been reopened, so I its looks like they were
> happy enough with the original patches that fixed the problem on my bxt.
> The testcase seems to be very system dependent, my suspicion is that it
> has to do with the wacky runtime pm exhibited by CI bxt.

CI bxt doesn't have displays, which means we shut down a lot more when
it's running. Does this indicate a huge gem test gap where we should run
plenty of gem testcases with all the outputs shut down?

Or just the need to add a pile more tests to pm_rpm?

Would be good if testcase review is a part of review, and not just "code
does what the commit message says" ... The latter should be the
trivial-most part of review really.
-Daniel
-- 
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: Always wake the device to flush the GTT

[Chris Wilson]

Quoting Daniel Vetter (2017-08-30 13:23:56)
> On Tue, Aug 29, 2017 at 03:59:36PM +0100, Chris Wilson wrote:
> > Quoting Joonas Lahtinen (2017-08-29 15:54:06)
> > > On Tue, 2017-08-29 at 11:33 +0100, Chris Wilson wrote:
> > > > Since we hold the device wakeref when writing through the GTT (otherwise
> > > > the writes would fail), we presumed that before the device sleeps those
> > > > writes would naturally be flushed and that we wouldn't need our mmio
> > > > read trick. However, that presumption seems false and a sleepy bxt seems
> > > > to require us to always manually flush the GTT writes prior to direct
> > > > access.
> > > > 
> > > > Fixes: e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > 
> > > Got any Bugzilla, Testcase, Tested-by?
> > 
> > Original bugzilla hasn't been reopened, so I its looks like they were
> > happy enough with the original patches that fixed the problem on my bxt.
> > The testcase seems to be very system dependent, my suspicion is that it
> > has to do with the wacky runtime pm exhibited by CI bxt.
> 
> CI bxt doesn't have displays, which means we shut down a lot more when
> it's running. Does this indicate a huge gem test gap where we should run
> plenty of gem testcases with all the outputs shut down?

This one is hard to tell since we are guessing at how the hw actually
works. Strong PCI ordering it is not.

If we take this example at face value, the key point of failure was
rpm_get_if_in_use, so we could simply say that we need to ensure that
all such branches are exercised, with varying amounts of stress since we
are looking for a random hw delay.

At the moment that boils down to the shrinker being avoiding unbinding
anything whilst the device is idle, pushing us closer to oom (with
kswapd hopefully riding to the rescue, and we all know how unreliable
kswapd is).

The wacky part of CI suspend seems to be that there's no reason for the
device to wake at times, quite a few of the tests are just burning
cycles without touching the hw and we still have the constant stream of
suspend/resume. I'm very suspicious that we are waking up too often (and
that it takes too long, about 28ms including the hpd of a headless
machine).

> Or just the need to add a pile more tests to pm_rpm?

No. It's just your regular combinatorial explosion. The approach I would
take here would be to register a sysenter callback that attempted to do a
rpm suspend (i.e. so ~every ioctl would start from idle, and controlled
via the faultinjection framework) and then run the minimal test set that
exercises all ioctl paths, and then expand to all driver branches.

First we need coverage feedback.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Always wake the device to flush the GTT

[Chris Wilson]

Quoting Chris Wilson (2017-08-30 13:56:40)
> Quoting Daniel Vetter (2017-08-30 13:23:56)
> > Or just the need to add a pile more tests to pm_rpm?
> 
> No. It's just your regular combinatorial explosion. The approach I would
> take here would be to register a sysenter callback that attempted to do a
> rpm suspend (i.e. so ~every ioctl would start from idle, and controlled
> via the faultinjection framework) and then run the minimal test set that
> exercises all ioctl paths, and then expand to all driver branches.

We also need the corollary that without rpm it works.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Always wake the device to flush the GTT

[Chris Wilson]

Quoting Chris Wilson (2017-08-30 13:56:40)
> No. It's just your regular combinatorial explosion. The approach I would
> take here would be to register a sysenter callback that attempted to do a
> rpm suspend (i.e. so ~every ioctl would start from idle, and controlled
> via the faultinjection framework) and then run the minimal test set that
> exercises all ioctl paths, and then expand to all driver branches.

The simpler option is to apply the faultinjection to the rpm_put.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Always wake the device to flush the GTT

[Daniel Vetter]

On Wed, Aug 30, 2017 at 01:56:40PM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2017-08-30 13:23:56)
> > Or just the need to add a pile more tests to pm_rpm?
> 
> No. It's just your regular combinatorial explosion. The approach I would
> take here would be to register a sysenter callback that attempted to do a
> rpm suspend (i.e. so ~every ioctl would start from idle, and controlled
> via the faultinjection framework) and then run the minimal test set that
> exercises all ioctl paths, and then expand to all driver branches.
> 
> First we need coverage feedback.

What I meant to imply: As long as any display is on we will never rpm
suspend. Mostly this is the case for CI machines.

The new testcases I've had in mind would explicitly dpms off the display
before running a set of gem testcases. We don't want to do that everywhere
though, because a dpms on/off is very costly.

I guess once we switch (eventually, hopefully) to a binary-at-time model
for full igt CI we could amortize that over a pile of substests and do it
almost everywhere.

So I think adding a force rpm suspend won't help, because under normal CI
runs we can't ever get there becaus of the active display. And that's why
we're not hitting all these tons of problems anywhere else.

This might also be good reason for more server chips, or at least fake
server chips, where we disable the display entirely.
-Daniel
-- 
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: Always wake the device to flush the GTT

[Chris Wilson]

Quoting Daniel Vetter (2017-08-30 14:59:32)
> On Wed, Aug 30, 2017 at 01:56:40PM +0100, Chris Wilson wrote:
> > Quoting Daniel Vetter (2017-08-30 13:23:56)
> > > Or just the need to add a pile more tests to pm_rpm?
> > 
> > No. It's just your regular combinatorial explosion. The approach I would
> > take here would be to register a sysenter callback that attempted to do a
> > rpm suspend (i.e. so ~every ioctl would start from idle, and controlled
> > via the faultinjection framework) and then run the minimal test set that
> > exercises all ioctl paths, and then expand to all driver branches.
> > 
> > First we need coverage feedback.
> 
> What I meant to imply: As long as any display is on we will never rpm
> suspend. Mostly this is the case for CI machines.
> 
> The new testcases I've had in mind would explicitly dpms off the display
> before running a set of gem testcases. We don't want to do that everywhere
> though, because a dpms on/off is very costly.

If no userspace is using the display and we remove fbcon, shouldn't the
kernel be disabling the outputs anyway? There's literally nothing there
to provide display continuity.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Always wake the device to flush the GTT

[Daniel Vetter]

On Wed, Aug 30, 2017 at 04:13:41PM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2017-08-30 14:59:32)
> > On Wed, Aug 30, 2017 at 01:56:40PM +0100, Chris Wilson wrote:
> > > Quoting Daniel Vetter (2017-08-30 13:23:56)
> > > > Or just the need to add a pile more tests to pm_rpm?
> > > 
> > > No. It's just your regular combinatorial explosion. The approach I would
> > > take here would be to register a sysenter callback that attempted to do a
> > > rpm suspend (i.e. so ~every ioctl would start from idle, and controlled
> > > via the faultinjection framework) and then run the minimal test set that
> > > exercises all ioctl paths, and then expand to all driver branches.
> > > 
> > > First we need coverage feedback.
> > 
> > What I meant to imply: As long as any display is on we will never rpm
> > suspend. Mostly this is the case for CI machines.
> > 
> > The new testcases I've had in mind would explicitly dpms off the display
> > before running a set of gem testcases. We don't want to do that everywhere
> > though, because a dpms on/off is very costly.
> 
> If no userspace is using the display and we remove fbcon, shouldn't the
> kernel be disabling the outputs anyway? There's literally nothing there
> to provide display continuity.

Fastboot will take over boot-up state and keep it running until a kms
client takes over. Once that happened, we'll have dropped the bios fb on
the floor and will shut down all outputs on close (due to removal of the
userspace fbs).

Which means if you want to rely on that, then how your test behaves
depends upon when we last had to reboot, and whether a kms test has run
before you. That's really bad from a "too much noise in CI" pov (because
with the full sharded run it's somewhat randomized each time).

Also, we have fbcon tests in igt, so disabling fbcon isn't an option
really.
-Daniel
-- 
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

[Resend for flip-flops] drm/i915: Always wake the device to flush the GTT

[Chris Wilson]

Since we hold the device wakeref when writing through the GTT (otherwise
the writes would fail), we presumed that before the device sleeps those
writes would naturally be flushed and that we wouldn't need our mmio
read trick. However, that presumption seems false and a sleepy bxt seems
to require us to always manually flush the GTT writes prior to direct
access.

Fixes: e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 44a7cb0e8bad..e4cc08bc518c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -695,12 +695,11 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
 	switch (obj->base.write_domain) {
 	case I915_GEM_DOMAIN_GTT:
 		if (INTEL_GEN(dev_priv) >= 6 && !HAS_LLC(dev_priv)) {
-			if (intel_runtime_pm_get_if_in_use(dev_priv)) {
-				spin_lock_irq(&dev_priv->uncore.lock);
-				POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
-				spin_unlock_irq(&dev_priv->uncore.lock);
-				intel_runtime_pm_put(dev_priv);
-			}
+			intel_runtime_pm_get(dev_priv);
+			spin_lock_irq(&dev_priv->uncore.lock);
+			POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
+			spin_unlock_irq(&dev_priv->uncore.lock);
+			intel_runtime_pm_put(dev_priv);
 		}
 
 		intel_fb_obj_flush(obj,
-- 
2.14.1

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

Re: [Resend for flip-flops] drm/i915: Always wake the device to flush the GTT

[Chris Wilson]

Quoting Chris Wilson (2017-08-29 20:25:46)
> Since we hold the device wakeref when writing through the GTT (otherwise
> the writes would fail), we presumed that before the device sleeps those
> writes would naturally be flushed and that we wouldn't need our mmio
> read trick. However, that presumption seems false and a sleepy bxt seems
> to require us to always manually flush the GTT writes prior to direct
> access.
> 
> Fixes: e2a2aa36a509 ("drm/i915: Check we have an wake device before flushing GTT writes")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

On the basis of two samples, this seems to fix the pwrite flip-flops,
so pushed for a wider sampling.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for drm/i915: Always wake the device to flush the GTT (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Always wake the device to flush the GTT (rev2)
URL   : https://patchwork.freedesktop.org/series/29436/
State : warning

== Summary ==

Series 29436v2 drm/i915: Always wake the device to flush the GTT
https://patchwork.freedesktop.org/api/1.0/series/29436/revisions/2/mbox/

Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-bdw-5557u)

fi-bdw-5557u     total:279  pass:267  dwarn:1   dfail:0   fail:0   skip:11  time:461s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:445s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:360s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:547s
fi-bwr-2160      total:279  pass:184  dwarn:0   dfail:0   fail:0   skip:95  time:253s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:518s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:521s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:511s
fi-elk-e7500     total:279  pass:230  dwarn:0   dfail:0   fail:0   skip:49  time:439s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:614s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:444s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:425s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:508s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:474s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:476s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:595s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:598s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:529s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:471s
fi-skl-6700k     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:481s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:487s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:449s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:486s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:548s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:402s

627598734e5ed449b1173ff8158126c57c361a40 drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest
2a4825b281be drm/i915: Always wake the device to flush the GTT

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5518/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Flush indirect GTT writes upon runtime-suspend

[Chris Wilson]

Our assumption is that indirect writes via the GTT are naturally flushed
when we enter runtime suspend. However, from the look of bxt in our CI,
this is not true and so we must apply our trick of doing a mmio to
serialise the writes.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 43834dee4e8d..6b9352248925 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2023,6 +2023,7 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
 void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
 {
 	struct drm_i915_gem_object *obj, *on;
+	unsigned long flags;
 	int i;
 
 	/*
@@ -2063,6 +2064,17 @@ void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
 		GEM_BUG_ON(!list_empty(&reg->vma->obj->userfault_link));
 		reg->dirty = true;
 	}
+
+	spin_lock_irqsave(&dev_priv->uncore.lock, flags);
+	POSTING_READ_FW(RING_ACTHD(dev_priv->engine[RCS]->mmio_base));
+	spin_unlock_irqrestore(&dev_priv->uncore.lock, flags);
+
+	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+		if (obj->base.read_domains & I915_GEM_DOMAIN_GTT) {
+			obj->base.read_domains &= ~I915_GEM_DOMAIN_GTT;
+			obj->base.write_domain &= ~I915_GEM_DOMAIN_GTT;
+		}
+	}
 }
 
 static int i915_gem_object_create_mmap_offset(struct drm_i915_gem_object *obj)
-- 
2.14.1

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

✓ Fi.CI.BAT: success for drm/i915: Always wake the device to flush the GTT (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Always wake the device to flush the GTT (rev3)
URL   : https://patchwork.freedesktop.org/series/29436/
State : success

== Summary ==

Series 29436v3 drm/i915: Always wake the device to flush the GTT
https://patchwork.freedesktop.org/api/1.0/series/29436/revisions/3/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215 +1

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:458s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:445s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:359s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:553s
fi-bwr-2160      total:279  pass:184  dwarn:0   dfail:0   fail:0   skip:95  time:253s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:522s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:527s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:518s
fi-elk-e7500     total:279  pass:230  dwarn:0   dfail:0   fail:0   skip:49  time:435s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:620s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:442s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:421s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:423s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:508s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:471s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:474s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:591s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:599s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:526s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:471s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:483s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:446s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:481s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:547s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:403s
fi-skl-6700k failed to connect after reboot

627598734e5ed449b1173ff8158126c57c361a40 drm-tip: 2017y-08m-29d-09h-52m-12s UTC integration manifest
aee73c805b40 drm/i915: Flush indirect GTT writes upon runtime-suspend

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5522/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Always wake the device to flush the GTT (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Always wake the device to flush the GTT (rev3)
URL   : https://patchwork.freedesktop.org/series/29436/
State : success

== Summary ==

Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-C-planes:
                skip       -> PASS       (shard-hsw)
        Subgroup plane-position-hole-dpms-pipe-C-planes:
                skip       -> PASS       (shard-hsw)
Test kms_atomic_transition:
        Subgroup plane-all-transition-fencing:
                skip       -> PASS       (shard-hsw)
Test kms_properties:
        Subgroup plane-properties-legacy:
                skip       -> PASS       (shard-hsw)

shard-hsw        total:2230 pass:1230 dwarn:0   dfail:0   fail:18  skip:982 time:9625s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5522/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for drm/i915: Always wake the device to flush the GTT (rev3)

[Chris Wilson]

Quoting Patchwork (2017-08-29 19:50:02)
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5522/shards.html

Hmm, bxt pwrite issues still remain in this one. So either the theory is
bogus or we have a hole...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for drm/i915: Always wake the device to flush the GTT (rev3)

[Chris Wilson]

Quoting Chris Wilson (2017-08-29 20:21:36)
> Quoting Patchwork (2017-08-29 19:50:02)
> > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5522/shards.html
> 
> Hmm, bxt pwrite issues still remain in this one. So either the theory is
> bogus or we have a hole...

Most obvious hole being the delay being rpm being not-in-use and
autosuspend.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Always wake the device to flush the GTT (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Always wake the device to flush the GTT (rev4)
URL   : https://patchwork.freedesktop.org/series/29436/
State : success

== Summary ==

Series 29436v4 drm/i915: Always wake the device to flush the GTT
https://patchwork.freedesktop.org/api/1.0/series/29436/revisions/4/mbox/

Test gem_ringfill:
        Subgroup basic-default-hang:
                incomplete -> DMESG-WARN (fi-pnv-d510) fdo#101600
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-skl-x1585l) fdo#101781
Test kms_frontbuffer_tracking:
        Subgroup basic:
                dmesg-warn -> PASS       (fi-bdw-5557u)

fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:458s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:439s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:358s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:548s
fi-bwr-2160      total:279  pass:184  dwarn:0   dfail:0   fail:0   skip:95  time:253s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:524s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:515s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:514s
fi-elk-e7500     total:279  pass:230  dwarn:0   dfail:0   fail:0   skip:49  time:436s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:611s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:445s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:422s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:425s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:498s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:476s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:480s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:590s
fi-kbl-r         total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:601s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:532s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:463s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:488s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:444s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:470s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:549s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:407s
fi-skl-6700k failed to connect after reboot

428ed27345fbf9be530d01ca6dc862eb5895db81 drm-tip: 2017y-08m-29d-17h-43m-11s UTC integration manifest
26e7bf1f36fb drm/i915: Always wake the device to flush the GTT

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5528/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for drm/i915: Always wake the device to flush the GTT (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Always wake the device to flush the GTT (rev4)
URL   : https://patchwork.freedesktop.org/series/29436/
State : failure

== Summary ==

Test kms_cursor_legacy:
        Subgroup short-flip-before-cursor-atomic-transitions-varying-size:
                skip       -> PASS       (shard-hsw)
Test kms_flip:
        Subgroup dpms-vs-vblank-race:
                pass       -> FAIL       (shard-hsw)

shard-hsw        total:2230 pass:1229 dwarn:0   dfail:0   fail:19  skip:982 time:9668s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5528/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx