* [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND.
2019-03-12 3:31 [igt-dev] [PATCH i-g-t v2 1/1] i915_pm_rpm: gem-execbuf-stress-extra-wait faster Caz Yokoyama
@ 2019-03-12 3:31 ` Caz Yokoyama
2019-03-12 15:30 ` Chris Wilson
2019-03-12 15:14 ` [igt-dev] [PATCH i-g-t v2 1/1] i915_pm_rpm: gem-execbuf-stress-extra-wait faster Caz Yokoyama
2019-03-12 15:24 ` Chris Wilson
2 siblings, 1 reply; 12+ messages in thread
From: Caz Yokoyama @ 2019-03-12 3:31 UTC (permalink / raw)
To: igt-dev
From: "Yokoyama, Caz" <caz.yokoyama@intel.com>
Confirm there is no additional work needed when
GPU is in suspended state.
Change-Id: I0775d784f1485015304261309652de084280bf82
Signed-off-by: Yokoyama, Caz <caz.yokoyama@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0a6348ad7c98..ca0ba1f0d89e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -28,6 +28,7 @@
#include <linux/sort.h>
#include <linux/sched/mm.h>
+#include <linux/pm_runtime.h>
#include <drm/drm_debugfs.h>
#include <drm/drm_fourcc.h>
#include "intel_drv.h"
@@ -3873,6 +3874,7 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_wedged_fops,
#define DROP_IDLE BIT(6)
#define DROP_RESET_ACTIVE BIT(7)
#define DROP_RESET_SEQNO BIT(8)
+#define DROP_SUSPEND BIT(9)
#define DROP_ALL (DROP_UNBOUND | \
DROP_BOUND | \
DROP_RETIRE | \
@@ -3899,7 +3901,9 @@ i915_drop_caches_set(void *data, u64 val)
DRM_DEBUG("Dropping caches: 0x%08llx [0x%08llx]\n",
val, val & DROP_ALL);
- wakeref = intel_runtime_pm_get(i915);
+ if (!(val & DROP_SUSPEND)) {
+ wakeref = intel_runtime_pm_get(i915);
+ }
if (val & DROP_RESET_ACTIVE &&
wait_for(intel_engines_are_idle(i915), I915_IDLE_ENGINES_TIMEOUT))
@@ -3949,8 +3953,23 @@ i915_drop_caches_set(void *data, u64 val)
if (val & DROP_FREED)
i915_gem_drain_freed_objects(i915);
+ if (val & DROP_SUSPEND) {
+ struct device *dev = i915->drm.dev;
+
+ dev_info(dev, "%s() %d: runtime_status: %d\n", __func__, __LINE__,
+ dev->power.runtime_status);
+ pm_runtime_disable(dev);
+ if (!pm_runtime_status_suspended(dev)) {
+ dev_warn(dev, "%s() %d: runtime_status: %d\n", __func__, __LINE__,
+ dev->power.runtime_status);
+ }
+ pm_runtime_enable(dev);
+ }
+
out:
- intel_runtime_pm_put(i915, wakeref);
+ if (!(val & DROP_SUSPEND)) {
+ intel_runtime_pm_put(i915, wakeref);
+ }
return ret;
}
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND.
2019-03-12 3:31 ` [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND Caz Yokoyama
@ 2019-03-12 15:30 ` Chris Wilson
2019-03-12 15:52 ` Caz Yokoyama
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2019-03-12 15:30 UTC (permalink / raw)
To: Caz Yokoyama, igt-dev
Quoting Caz Yokoyama (2019-03-12 03:31:32)
> + if (val & DROP_SUSPEND) {
> + struct device *dev = i915->drm.dev;
> +
> + dev_info(dev, "%s() %d: runtime_status: %d\n", __func__, __LINE__,
> + dev->power.runtime_status);
> + pm_runtime_disable(dev);
> + if (!pm_runtime_status_suspended(dev)) {
> + dev_warn(dev, "%s() %d: runtime_status: %d\n", __func__, __LINE__,
> + dev->power.runtime_status);
> + }
> + pm_runtime_enable(dev);
ret = pm_runtime_force_suspend(i915->drm.dev);
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND.
2019-03-12 15:30 ` Chris Wilson
@ 2019-03-12 15:52 ` Caz Yokoyama
2019-03-12 15:51 ` Chris Wilson
2019-03-12 22:36 ` Caz Yokoyama
0 siblings, 2 replies; 12+ messages in thread
From: Caz Yokoyama @ 2019-03-12 15:52 UTC (permalink / raw)
To: Chris Wilson, igt-dev
On Tue, 2019-03-12 at 15:30 +0000, Chris Wilson wrote:
> Quoting Caz Yokoyama (2019-03-12 03:31:32)
> > + if (val & DROP_SUSPEND) {
> > + struct device *dev = i915->drm.dev;
> > +
> > + dev_info(dev, "%s() %d: runtime_status: %d\n",
> > __func__, __LINE__,
> > + dev->power.runtime_status);
>
>
> > + pm_runtime_disable(dev);
> > + if (!pm_runtime_status_suspended(dev)) {
> > + dev_warn(dev, "%s() %d: runtime_status:
> > %d\n", __func__, __LINE__,
> > + dev->power.runtime_status);
> > + }
> > + pm_runtime_enable(dev);
>
> ret = pm_runtime_force_suspend(i915->drm.dev);
>
Yes, this works. However, pm_runtime_force_suspend() always exits on
if (pm_runtime_status_suspended(dev))
return 0;
Therefore, this patch is meaningless after suspended state.
-caz
> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND.
2019-03-12 15:52 ` Caz Yokoyama
@ 2019-03-12 15:51 ` Chris Wilson
2019-03-12 16:11 ` Caz Yokoyama
2019-03-12 22:36 ` Caz Yokoyama
1 sibling, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2019-03-12 15:51 UTC (permalink / raw)
To: Caz Yokoyama, igt-dev
Quoting Caz Yokoyama (2019-03-12 15:52:09)
> On Tue, 2019-03-12 at 15:30 +0000, Chris Wilson wrote:
> > Quoting Caz Yokoyama (2019-03-12 03:31:32)
> > > + if (val & DROP_SUSPEND) {
> > > + struct device *dev = i915->drm.dev;
> > > +
> > > + dev_info(dev, "%s() %d: runtime_status: %d\n",
> > > __func__, __LINE__,
> > > + dev->power.runtime_status);
> >
> >
> > > + pm_runtime_disable(dev);
> > > + if (!pm_runtime_status_suspended(dev)) {
> > > + dev_warn(dev, "%s() %d: runtime_status:
> > > %d\n", __func__, __LINE__,
> > > + dev->power.runtime_status);
> > > + }
> > > + pm_runtime_enable(dev);
> >
> > ret = pm_runtime_force_suspend(i915->drm.dev);
> >
> Yes, this works. However, pm_runtime_force_suspend() always exits on
> if (pm_runtime_status_suspended(dev))
> return 0;
> Therefore, this patch is meaningless after suspended state.
Which would be fine for DROP_SUSPEND. Ah, so your intent is to do a
DROP_WAKE as well :)
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND.
2019-03-12 15:51 ` Chris Wilson
@ 2019-03-12 16:11 ` Caz Yokoyama
2019-03-12 16:09 ` Chris Wilson
0 siblings, 1 reply; 12+ messages in thread
From: Caz Yokoyama @ 2019-03-12 16:11 UTC (permalink / raw)
To: Chris Wilson, igt-dev
On Tue, 2019-03-12 at 15:51 +0000, Chris Wilson wrote:
> Quoting Caz Yokoyama (2019-03-12 15:52:09)
> > On Tue, 2019-03-12 at 15:30 +0000, Chris Wilson wrote:
> > > Quoting Caz Yokoyama (2019-03-12 03:31:32)
> > > > + if (val & DROP_SUSPEND) {
> > > > + struct device *dev = i915->drm.dev;
> > > > +
> > > > + dev_info(dev, "%s() %d: runtime_status: %d\n",
> > > > __func__, __LINE__,
> > > > + dev->power.runtime_status);
> > >
> > >
> > > > + pm_runtime_disable(dev);
> > > > + if (!pm_runtime_status_suspended(dev)) {
> > > > + dev_warn(dev, "%s() %d: runtime_status:
> > > > %d\n", __func__, __LINE__,
> > > > + dev->power.runtime_status);
> > > > + }
> > > > + pm_runtime_enable(dev);
> > >
> > > ret = pm_runtime_force_suspend(i915->drm.dev);
> > >
> >
> > Yes, this works. However, pm_runtime_force_suspend() always exits
> > on
> > if (pm_runtime_status_suspended(dev))
> > return 0;
> > Therefore, this patch is meaningless after suspended state.
>
> Which would be fine for DROP_SUSPEND. Ah, so your intent is to do a
> DROP_WAKE as well :)
My intention is to remove gem-execbuf-stress-extra-wait subtest
because it does same as gem-execbuf-stress except for meaningless (10 x
5 sec) delay.
DROP_WAKE is a joke, correct? The runtime of gem-execbuf-stress is 1.5
sec. or less. It is quick enough.
-caz
> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND.
2019-03-12 16:11 ` Caz Yokoyama
@ 2019-03-12 16:09 ` Chris Wilson
2019-03-12 17:14 ` Caz Yokoyama
0 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2019-03-12 16:09 UTC (permalink / raw)
To: Caz Yokoyama, igt-dev
Quoting Caz Yokoyama (2019-03-12 16:11:14)
> On Tue, 2019-03-12 at 15:51 +0000, Chris Wilson wrote:
> > Quoting Caz Yokoyama (2019-03-12 15:52:09)
> > > On Tue, 2019-03-12 at 15:30 +0000, Chris Wilson wrote:
> > > > Quoting Caz Yokoyama (2019-03-12 03:31:32)
> > > > > + if (val & DROP_SUSPEND) {
> > > > > + struct device *dev = i915->drm.dev;
> > > > > +
> > > > > + dev_info(dev, "%s() %d: runtime_status: %d\n",
> > > > > __func__, __LINE__,
> > > > > + dev->power.runtime_status);
> > > >
> > > >
> > > > > + pm_runtime_disable(dev);
> > > > > + if (!pm_runtime_status_suspended(dev)) {
> > > > > + dev_warn(dev, "%s() %d: runtime_status:
> > > > > %d\n", __func__, __LINE__,
> > > > > + dev->power.runtime_status);
> > > > > + }
> > > > > + pm_runtime_enable(dev);
> > > >
> > > > ret = pm_runtime_force_suspend(i915->drm.dev);
> > > >
> > >
> > > Yes, this works. However, pm_runtime_force_suspend() always exits
> > > on
> > > if (pm_runtime_status_suspended(dev))
> > > return 0;
> > > Therefore, this patch is meaningless after suspended state.
> >
> > Which would be fine for DROP_SUSPEND. Ah, so your intent is to do a
> > DROP_WAKE as well :)
>
> My intention is to remove gem-execbuf-stress-extra-wait subtest
> because it does same as gem-execbuf-stress except for meaningless (10 x
> 5 sec) delay.
Then why complain that pm_runtime_force_suspend() is a no-op starting
from a suspended state?
WAIT_EXTRA is after WAIT_STATUS, so it will always be in a suspended
state. If your intent then is to bolster the test by forcing a fresh
suspend cycle, make that explicit and have both DROP_WAKE |
DROP_SUSPEND. As it stands we can use DROP_SUSPEND as a faster
WAIT_STATUS.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND.
2019-03-12 16:09 ` Chris Wilson
@ 2019-03-12 17:14 ` Caz Yokoyama
0 siblings, 0 replies; 12+ messages in thread
From: Caz Yokoyama @ 2019-03-12 17:14 UTC (permalink / raw)
To: Chris Wilson, igt-dev
On Tue, 2019-03-12 at 16:09 +0000, Chris Wilson wrote:
> Quoting Caz Yokoyama (2019-03-12 16:11:14)
> > On Tue, 2019-03-12 at 15:51 +0000, Chris Wilson wrote:
> > > Quoting Caz Yokoyama (2019-03-12 15:52:09)
> > > > On Tue, 2019-03-12 at 15:30 +0000, Chris Wilson wrote:
> > > > > Quoting Caz Yokoyama (2019-03-12 03:31:32)
> > > > > > + if (val & DROP_SUSPEND) {
> > > > > > + struct device *dev = i915->drm.dev;
> > > > > > +
> > > > > > + dev_info(dev, "%s() %d: runtime_status:
> > > > > > %d\n",
> > > > > > __func__, __LINE__,
> > > > > > + dev->power.runtime_status);
> > > > >
> > > > >
> > > > > > + pm_runtime_disable(dev);
> > > > > > + if (!pm_runtime_status_suspended(dev)) {
> > > > > > + dev_warn(dev, "%s() %d:
> > > > > > runtime_status:
> > > > > > %d\n", __func__, __LINE__,
> > > > > > + dev-
> > > > > > >power.runtime_status);
> > > > > > + }
> > > > > > + pm_runtime_enable(dev);
> > > > >
> > > > > ret = pm_runtime_force_suspend(i915->drm.dev);
> > > > >
> > > >
> > > > Yes, this works. However, pm_runtime_force_suspend() always
> > > > exits
> > > > on
> > > > if (pm_runtime_status_suspended(dev))
> > > > return 0;
> > > > Therefore, this patch is meaningless after suspended state.
> > >
> > > Which would be fine for DROP_SUSPEND. Ah, so your intent is to do
> > > a
> > > DROP_WAKE as well :)
> >
> > My intention is to remove gem-execbuf-stress-extra-wait subtest
> > because it does same as gem-execbuf-stress except for meaningless
> > (10 x
> > 5 sec) delay.
>
> Then why complain that pm_runtime_force_suspend() is a no-op starting
> from a suspended state?
No, I am not complaining. The patch above is to confirm the patch is
meaningless on suspended state.
>
> WAIT_EXTRA is after WAIT_STATUS, so it will always be in a suspended
> state. If your intent then is to bolster the test by forcing a fresh
> suspend cycle, make that explicit and have both DROP_WAKE |
> DROP_SUSPEND. As it stands we can use DROP_SUSPEND as a faster
> WAIT_STATUS.
Let me update the patch.
-caz
> -Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND.
2019-03-12 15:52 ` Caz Yokoyama
2019-03-12 15:51 ` Chris Wilson
@ 2019-03-12 22:36 ` Caz Yokoyama
2019-03-12 22:42 ` Chris Wilson
1 sibling, 1 reply; 12+ messages in thread
From: Caz Yokoyama @ 2019-03-12 22:36 UTC (permalink / raw)
To: Chris Wilson, igt-dev
On Tue, 2019-03-12 at 08:52 -0700, Caz Yokoyama wrote:
> On Tue, 2019-03-12 at 15:30 +0000, Chris Wilson wrote:
> > Quoting Caz Yokoyama (2019-03-12 03:31:32)
> > > + if (val & DROP_SUSPEND) {
> > > + struct device *dev = i915->drm.dev;
> > > +
> > > + dev_info(dev, "%s() %d: runtime_status: %d\n",
> > > __func__, __LINE__,
> > > + dev->power.runtime_status);
> >
> >
> > > + pm_runtime_disable(dev);
> > > + if (!pm_runtime_status_suspended(dev)) {
> > > + dev_warn(dev, "%s() %d: runtime_status:
> > > %d\n", __func__, __LINE__,
> > > + dev->power.runtime_status);
> > > + }
> > > + pm_runtime_enable(dev);
> >
> > ret = pm_runtime_force_suspend(i915->drm.dev);
> >
>
> Yes, this works. However, pm_runtime_force_suspend() always exits on
> if (pm_runtime_status_suspended(dev))
> return 0;
Correction. This does not work because of kernel crash. When exiting
from pm_runtime_force_suspend() by pm_runtime_status_suspended(dev) ==
true, pm_runtime_enable(dev) is not called. It looks a bug for me.
-caz
> Therefore, this patch is meaningless after suspended state.
> -caz
>
> > -Chris
>
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND.
2019-03-12 22:36 ` Caz Yokoyama
@ 2019-03-12 22:42 ` Chris Wilson
0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-03-12 22:42 UTC (permalink / raw)
To: Caz Yokoyama, igt-dev
Quoting Caz Yokoyama (2019-03-12 22:36:49)
> On Tue, 2019-03-12 at 08:52 -0700, Caz Yokoyama wrote:
> > On Tue, 2019-03-12 at 15:30 +0000, Chris Wilson wrote:
> > > Quoting Caz Yokoyama (2019-03-12 03:31:32)
> > > > + if (val & DROP_SUSPEND) {
> > > > + struct device *dev = i915->drm.dev;
> > > > +
> > > > + dev_info(dev, "%s() %d: runtime_status: %d\n",
> > > > __func__, __LINE__,
> > > > + dev->power.runtime_status);
> > >
> > >
> > > > + pm_runtime_disable(dev);
> > > > + if (!pm_runtime_status_suspended(dev)) {
> > > > + dev_warn(dev, "%s() %d: runtime_status:
> > > > %d\n", __func__, __LINE__,
> > > > + dev->power.runtime_status);
> > > > + }
> > > > + pm_runtime_enable(dev);
> > >
> > > ret = pm_runtime_force_suspend(i915->drm.dev);
> > >
> >
> > Yes, this works. However, pm_runtime_force_suspend() always exits on
> > if (pm_runtime_status_suspended(dev))
> > return 0;
> Correction. This does not work because of kernel crash. When exiting
> from pm_runtime_force_suspend() by pm_runtime_status_suspended(dev) ==
> true, pm_runtime_enable(dev) is not called. It looks a bug for me.
Bah, just that I'm suggesting to use them outside of their expected
context. They should only be used during system-wide suspend/resume
apparently, which is annoying.
However, without such a call, we do not guarantee we suspend (skipping a
call to intel_runtime_suspend).
Maybe...
pm_runtime_disable();
pm_runtime_suspend();
pm_runtime_enable();
If a plain pm_runtime_suspend() isn't enough by itself.
Fun experiments,
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 1/1] i915_pm_rpm: gem-execbuf-stress-extra-wait faster
2019-03-12 3:31 [igt-dev] [PATCH i-g-t v2 1/1] i915_pm_rpm: gem-execbuf-stress-extra-wait faster Caz Yokoyama
2019-03-12 3:31 ` [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND Caz Yokoyama
@ 2019-03-12 15:14 ` Caz Yokoyama
2019-03-12 15:24 ` Chris Wilson
2 siblings, 0 replies; 12+ messages in thread
From: Caz Yokoyama @ 2019-03-12 15:14 UTC (permalink / raw)
To: igt-dev
The corresponding patch is subjected as "[PATCH v2 2/2] add
DROP_SUSPEND".
The patch in i915_drop_caches_set() is the first part
of pm_runtime_force_suspend(). After GPU becomes suspended state,
pm_runtime_status_suspended(dev) is always true and get out. In other
words, the patch and pm_runtime_force_suspend() are equivalent on
suspended state.
As a result, I prefer to remove gem-execbuf-stress-extra-wait subtest
because it does same as gem-execbuf-stress except for meaningless (10 x
5 sec) delay.
Chris, how do you think?
-caz
On Tue, 2019-03-12 at 03:31 +0000, Caz Yokoyama wrote:
> With the corresponding patch in drm-tip,
> confirm there is no additional work needed when
> GPU is in suspended state.
>
> Signed-off-by: Caz Yokoyama <caz.yokoyama@intel.com>
> ---
> lib/igt_debugfs.h | 7 +++++++
> tests/i915/i915_pm_rpm.c | 7 +++++--
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index b9cf0f81..9fa38b42 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -216,6 +216,13 @@ void igt_require_hpd_storm_ctl(int fd);
> DROP_FREED | \
> DROP_IDLE)
>
> +/**
> + * DROP_SUSPEND:
> + *
> + * force to suspended state.
> + */
> +#define DROP_SUSPEND 0x200
> +
> void igt_reset_fifo_underrun_reporting(int drm_fd);
>
> bool igt_drop_caches_has(int fd, uint64_t val);
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index be296f52..79528d18 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -1338,12 +1338,15 @@ static void gem_execbuf_stress_subtest(int
> rounds, int wait_flags)
> for (i = 0; i < rounds; i++) {
> gem_execbuf(drm_fd, &execbuf);
>
> + igt_drop_caches_set(drm_fd, DROP_IDLE); /* clean up
> idle work */
> if (wait_flags & WAIT_STATUS)
> igt_assert(wait_for_suspended());
> if (wait_flags & WAIT_PC8_RES)
> igt_assert(pc8_plus_residency_changed(30));
> - if (wait_flags & WAIT_EXTRA)
> - sleep(5);
> + if (wait_flags & WAIT_EXTRA) {
> + /* force to suspend mode */
> + igt_drop_caches_set(drm_fd, DROP_SUSPEND);
> + }
> }
>
> gem_close(drm_fd, handle);
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [igt-dev] [PATCH i-g-t v2 1/1] i915_pm_rpm: gem-execbuf-stress-extra-wait faster
2019-03-12 3:31 [igt-dev] [PATCH i-g-t v2 1/1] i915_pm_rpm: gem-execbuf-stress-extra-wait faster Caz Yokoyama
2019-03-12 3:31 ` [igt-dev] [PATCH v2 2/2] add DROP_SUSPEND Caz Yokoyama
2019-03-12 15:14 ` [igt-dev] [PATCH i-g-t v2 1/1] i915_pm_rpm: gem-execbuf-stress-extra-wait faster Caz Yokoyama
@ 2019-03-12 15:24 ` Chris Wilson
2 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2019-03-12 15:24 UTC (permalink / raw)
To: Caz Yokoyama, igt-dev
Quoting Caz Yokoyama (2019-03-12 03:31:31)
> With the corresponding patch in drm-tip,
> confirm there is no additional work needed when
> GPU is in suspended state.
>
> Signed-off-by: Caz Yokoyama <caz.yokoyama@intel.com>
> ---
> lib/igt_debugfs.h | 7 +++++++
> tests/i915/i915_pm_rpm.c | 7 +++++--
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index b9cf0f81..9fa38b42 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -216,6 +216,13 @@ void igt_require_hpd_storm_ctl(int fd);
> DROP_FREED | \
> DROP_IDLE)
>
> +/**
> + * DROP_SUSPEND:
> + *
> + * force to suspended state.
> + */
> +#define DROP_SUSPEND 0x200
> +
> void igt_reset_fifo_underrun_reporting(int drm_fd);
>
> bool igt_drop_caches_has(int fd, uint64_t val);
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index be296f52..79528d18 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -1338,12 +1338,15 @@ static void gem_execbuf_stress_subtest(int rounds, int wait_flags)
> for (i = 0; i < rounds; i++) {
> gem_execbuf(drm_fd, &execbuf);
>
> + igt_drop_caches_set(drm_fd, DROP_IDLE); /* clean up idle work */
> if (wait_flags & WAIT_STATUS)
> igt_assert(wait_for_suspended());
> if (wait_flags & WAIT_PC8_RES)
> igt_assert(pc8_plus_residency_changed(30));
> - if (wait_flags & WAIT_EXTRA)
> - sleep(5);
> + if (wait_flags & WAIT_EXTRA) {
> + /* force to suspend mode */
> + igt_drop_caches_set(drm_fd, DROP_SUSPEND);
> + }
Hmm, this would ostensibly be a quicker WAIT_STATUS. So the meaning of
WAIT_EXTRA (whatever that meaning was) is now tautological.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread