* [PATCH] drm/i915: Skip waking the device to service pwrite
@ 2017-08-30 17:48 Chris Wilson
2017-08-30 18:07 ` ✓ Fi.CI.BAT: success for " Patchwork
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Chris Wilson @ 2017-08-30 17:48 UTC (permalink / raw)
To: intel-gfx
If the device is in runtime suspend, resuming takes time and reduces our
powersaving. If this was for a small write into an object, that resume
will take longer than any savings in using the indirect GGTT access to
avoid the cpu cache.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 93dfa793975a..8940a6873ca5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1229,7 +1229,21 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
if (ret)
return ret;
- intel_runtime_pm_get(i915);
+ if (i915_gem_object_has_struct_page(obj)) {
+ /* Avoid waking the device up if we can fallback, as
+ * waking/resuming is very slow (10-100 ms depending
+ * on PCI sleeps and our own resume time). This easily
+ * dwarfs any performance advantage from using the
+ * cache bypass of indirect GGTT access.
+ */
+ if (!intel_runtime_pm_get_if_in_use(i915)) {
+ ret = -EFAULT;
+ goto out_unlock;
+ }
+ } else {
+ intel_runtime_pm_get(i915);
+ }
+
vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
PIN_MAPPABLE | PIN_NONBLOCK);
if (!IS_ERR(vma)) {
@@ -1244,7 +1258,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
if (IS_ERR(vma)) {
ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
if (ret)
- goto out_unlock;
+ goto out_rpm;
GEM_BUG_ON(!node.allocated);
}
@@ -1307,8 +1321,9 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
} else {
i915_vma_unpin(vma);
}
-out_unlock:
+out_rpm:
intel_runtime_pm_put(i915);
+out_unlock:
mutex_unlock(&i915->drm.struct_mutex);
return ret;
}
--
2.14.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* ✓ Fi.CI.BAT: success for drm/i915: Skip waking the device to service pwrite
2017-08-30 17:48 [PATCH] drm/i915: Skip waking the device to service pwrite Chris Wilson
@ 2017-08-30 18:07 ` Patchwork
2017-08-31 7:27 ` ✗ Fi.CI.IGT: warning " Patchwork
2017-09-04 8:12 ` [PATCH] " Daniel Vetter
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-08-30 18:07 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Skip waking the device to service pwrite
URL : https://patchwork.freedesktop.org/series/29560/
State : success
== Summary ==
Series 29560v1 drm/i915: Skip waking the device to service pwrite
https://patchwork.freedesktop.org/api/1.0/series/29560/revisions/1/mbox/
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> SKIP (fi-skl-x1585l) fdo#101781
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781
fi-bdw-5557u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:463s
fi-bdw-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:437s
fi-blb-e6850 total:288 pass:224 dwarn:1 dfail:0 fail:0 skip:63 time:362s
fi-bsw-n3050 total:288 pass:243 dwarn:0 dfail:0 fail:0 skip:45 time:560s
fi-bwr-2160 total:288 pass:184 dwarn:0 dfail:0 fail:0 skip:104 time:253s
fi-bxt-j4205 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:519s
fi-byt-j1900 total:288 pass:254 dwarn:1 dfail:0 fail:0 skip:33 time:524s
fi-elk-e7500 total:288 pass:230 dwarn:0 dfail:0 fail:0 skip:58 time:436s
fi-glk-2a total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:609s
fi-hsw-4770 total:288 pass:263 dwarn:0 dfail:0 fail:0 skip:25 time:445s
fi-hsw-4770r total:288 pass:263 dwarn:0 dfail:0 fail:0 skip:25 time:435s
fi-ilk-650 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:417s
fi-ivb-3520m total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:499s
fi-ivb-3770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:474s
fi-kbl-7500u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:479s
fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:597s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:597s
fi-pnv-d510 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:525s
fi-skl-6260u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:467s
fi-skl-6770hq total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:496s
fi-skl-gvtdvm total:288 pass:266 dwarn:0 dfail:0 fail:0 skip:22 time:449s
fi-skl-x1585l total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:486s
fi-snb-2520m total:288 pass:251 dwarn:0 dfail:0 fail:0 skip:37 time:548s
fi-snb-2600 total:288 pass:250 dwarn:0 dfail:0 fail:0 skip:38 time:405s
fi-byt-n2820 failed to connect after reboot
14d5b307c691a24460b991756dc2d501f853e0b3 drm-tip: 2017y-08m-30d-17h-07m-37s UTC integration manifest
e24a0b1b6b51 drm/i915: Skip waking the device to service pwrite
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5541/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* ✗ Fi.CI.IGT: warning for drm/i915: Skip waking the device to service pwrite
2017-08-30 17:48 [PATCH] drm/i915: Skip waking the device to service pwrite Chris Wilson
2017-08-30 18:07 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-08-31 7:27 ` Patchwork
2017-09-04 8:12 ` [PATCH] " Daniel Vetter
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-08-31 7:27 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Skip waking the device to service pwrite
URL : https://patchwork.freedesktop.org/series/29560/
State : warning
== Summary ==
Test kms_plane_multiple:
Subgroup legacy-pipe-C-tiling-none:
pass -> SKIP (shard-hsw)
Test kms_plane_lowres:
Subgroup pipe-A-tiling-x:
pass -> SKIP (shard-hsw)
Test kms_cursor_crc:
Subgroup cursor-256x85-offscreen:
pass -> SKIP (shard-hsw)
Test kms_concurrent:
Subgroup pipe-C:
pass -> SKIP (shard-hsw)
Test kms_chv_cursor_fail:
Subgroup pipe-B-128x128-top-edge:
pass -> SKIP (shard-hsw)
Test perf:
Subgroup blocking:
fail -> PASS (shard-hsw) fdo#102252 +1
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
shard-hsw total:2265 pass:1226 dwarn:0 dfail:0 fail:18 skip:1021 time:9631s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5541/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Skip waking the device to service pwrite
2017-08-30 17:48 [PATCH] drm/i915: Skip waking the device to service pwrite Chris Wilson
2017-08-30 18:07 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-08-31 7:27 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2017-09-04 8:12 ` Daniel Vetter
2017-09-04 10:18 ` Chris Wilson
2 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2017-09-04 8:12 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Wed, Aug 30, 2017 at 06:48:19PM +0100, Chris Wilson wrote:
> If the device is in runtime suspend, resuming takes time and reduces our
> powersaving. If this was for a small write into an object, that resume
> will take longer than any savings in using the indirect GGTT access to
> avoid the cpu cache.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 93dfa793975a..8940a6873ca5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1229,7 +1229,21 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
> if (ret)
> return ret;
>
> - intel_runtime_pm_get(i915);
> + if (i915_gem_object_has_struct_page(obj)) {
I don't really see why we need to check for has_struct_page here (we do
already outside the lock grabbing), and why if that's not the case we hit
the slow-path?
I'd have expected a simple s/pm_get/pm_get_if_in_use/ ...
-Daniel
> + /* Avoid waking the device up if we can fallback, as
> + * waking/resuming is very slow (10-100 ms depending
> + * on PCI sleeps and our own resume time). This easily
> + * dwarfs any performance advantage from using the
> + * cache bypass of indirect GGTT access.
> + */
> + if (!intel_runtime_pm_get_if_in_use(i915)) {
> + ret = -EFAULT;
> + goto out_unlock;
> + }
> + } else {
> + intel_runtime_pm_get(i915);
> + }
> +
> vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0,
> PIN_MAPPABLE | PIN_NONBLOCK);
> if (!IS_ERR(vma)) {
> @@ -1244,7 +1258,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
> if (IS_ERR(vma)) {
> ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
> if (ret)
> - goto out_unlock;
> + goto out_rpm;
> GEM_BUG_ON(!node.allocated);
> }
>
> @@ -1307,8 +1321,9 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
> } else {
> i915_vma_unpin(vma);
> }
> -out_unlock:
> +out_rpm:
> intel_runtime_pm_put(i915);
> +out_unlock:
> mutex_unlock(&i915->drm.struct_mutex);
> return ret;
> }
> --
> 2.14.1
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Skip waking the device to service pwrite
2017-09-04 8:12 ` [PATCH] " Daniel Vetter
@ 2017-09-04 10:18 ` Chris Wilson
2017-09-04 14:45 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2017-09-04 10:18 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
Quoting Daniel Vetter (2017-09-04 09:12:12)
> On Wed, Aug 30, 2017 at 06:48:19PM +0100, Chris Wilson wrote:
> > If the device is in runtime suspend, resuming takes time and reduces our
> > powersaving. If this was for a small write into an object, that resume
> > will take longer than any savings in using the indirect GGTT access to
> > avoid the cpu cache.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 21 ++++++++++++++++++---
> > 1 file changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 93dfa793975a..8940a6873ca5 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1229,7 +1229,21 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
> > if (ret)
> > return ret;
> >
> > - intel_runtime_pm_get(i915);
> > + if (i915_gem_object_has_struct_page(obj)) {
>
> I don't really see why we need to check for has_struct_page here (we do
> already outside the lock grabbing), and why if that's not the case we hit
> the slow-path?
We can't use the alternate paths if we don't have struct_page, hence we
have to force use of GTT if !i915_gem_object_has_struct_page. The
previous test is to also make sure we come down this path and not fail.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Skip waking the device to service pwrite
2017-09-04 10:18 ` Chris Wilson
@ 2017-09-04 14:45 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2017-09-04 14:45 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Mon, Sep 04, 2017 at 11:18:07AM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2017-09-04 09:12:12)
> > On Wed, Aug 30, 2017 at 06:48:19PM +0100, Chris Wilson wrote:
> > > If the device is in runtime suspend, resuming takes time and reduces our
> > > powersaving. If this was for a small write into an object, that resume
> > > will take longer than any savings in using the indirect GGTT access to
> > > avoid the cpu cache.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > > drivers/gpu/drm/i915/i915_gem.c | 21 ++++++++++++++++++---
> > > 1 file changed, 18 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index 93dfa793975a..8940a6873ca5 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -1229,7 +1229,21 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
> > > if (ret)
> > > return ret;
> > >
> > > - intel_runtime_pm_get(i915);
> > > + if (i915_gem_object_has_struct_page(obj)) {
> >
> > I don't really see why we need to check for has_struct_page here (we do
> > already outside the lock grabbing), and why if that's not the case we hit
> > the slow-path?
>
> We can't use the alternate paths if we don't have struct_page, hence we
> have to force use of GTT if !i915_gem_object_has_struct_page. The
> previous test is to also make sure we come down this path and not fail.
Ow, I've entirely misread all the code, I thought this checks for
obj->pages. With clue gained on my side:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-04 14:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-30 17:48 [PATCH] drm/i915: Skip waking the device to service pwrite Chris Wilson
2017-08-30 18:07 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-08-31 7:27 ` ✗ Fi.CI.IGT: warning " Patchwork
2017-09-04 8:12 ` [PATCH] " Daniel Vetter
2017-09-04 10:18 ` Chris Wilson
2017-09-04 14:45 ` Daniel Vetter
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.