* [PATCH] drm/i915: Skip clearing the GGTT on full-ppgtt systems
@ 2016-05-12 11:19 Chris Wilson
2016-05-12 12:11 ` ✓ Ro.CI.BAT: success for " Patchwork
2016-05-13 6:22 ` [PATCH] " Joonas Lahtinen
0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2016-05-12 11:19 UTC (permalink / raw)
To: intel-gfx; +Cc: David Weinehall
Under full-ppgtt, access to the global GTT is carefully regulated
through hardware functions (i.e. userspace cannot read and write to
arbitrary locations in the GGTT via the GPU). With this restriction in
place, we can forgo clearing stale entries from the GGTT as they will
not be accessed.
For aliasing-ppgtt, we could almost do the same except that we do allow
userspace access to the global-GTT via execbuf in order to workraound
some quirks of certain instructions. (This execbuf path is filtered out
with EINVAL on full-ppgtt.)
The most dramatic effect this will have will be during resume, as with
full-ppgtt the GGTT is only used sparingly.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Weinehall <david.weinehall@intel.com>
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 319f3b459b3e..7eab619a3eb2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2477,6 +2477,13 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
}
+static void nop_clear_range(struct i915_address_space *vm,
+ uint64_t start,
+ uint64_t length,
+ bool use_scratch)
+{
+}
+
static void gen8_ggtt_clear_range(struct i915_address_space *vm,
uint64_t start,
uint64_t length,
@@ -3072,14 +3079,17 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
ret = ggtt_probe_common(dev, ggtt->size);
- ggtt->base.clear_range = gen8_ggtt_clear_range;
- if (IS_CHERRYVIEW(dev_priv))
- ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
- else
- ggtt->base.insert_entries = gen8_ggtt_insert_entries;
ggtt->base.bind_vma = ggtt_bind_vma;
ggtt->base.unbind_vma = ggtt_unbind_vma;
+ ggtt->base.clear_range = nop_clear_range;
+ if (!USES_FULL_PPGTT(dev_priv))
+ ggtt->base.clear_range = gen8_ggtt_clear_range;
+
+ ggtt->base.insert_entries = gen8_ggtt_insert_entries;
+ if (IS_CHERRYVIEW(dev_priv))
+ ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
+
return ret;
}
--
2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread* ✓ Ro.CI.BAT: success for drm/i915: Skip clearing the GGTT on full-ppgtt systems
2016-05-12 11:19 [PATCH] drm/i915: Skip clearing the GGTT on full-ppgtt systems Chris Wilson
@ 2016-05-12 12:11 ` Patchwork
2016-05-13 6:22 ` [PATCH] " Joonas Lahtinen
1 sibling, 0 replies; 4+ messages in thread
From: Patchwork @ 2016-05-12 12:11 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Skip clearing the GGTT on full-ppgtt systems
URL : https://patchwork.freedesktop.org/series/7061/
State : success
== Summary ==
Series 7061v1 drm/i915: Skip clearing the GGTT on full-ppgtt systems
http://patchwork.freedesktop.org/api/1.0/series/7061/revisions/1/mbox
fi-bsw-n3050 total:218 pass:174 dwarn:0 dfail:0 fail:2 skip:42
fi-byt-n2820 total:218 pass:174 dwarn:0 dfail:0 fail:3 skip:41
fi-hsw-i7-4770k total:219 pass:198 dwarn:0 dfail:0 fail:0 skip:21
fi-hsw-i7-4770r total:219 pass:193 dwarn:0 dfail:0 fail:0 skip:26
fi-skl-i5-6260u total:219 pass:207 dwarn:0 dfail:0 fail:0 skip:12
fi-skl-i7-6700k total:219 pass:191 dwarn:0 dfail:0 fail:0 skip:28
fi-kbl-y 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-bdw-i7-5600u failed to connect after reboot
ro-bsw-n3050 failed to connect after reboot
ro-byt-n2820 failed to connect after reboot
ro-hsw-i3-4010u failed to connect after reboot
ro-hsw-i7-4770r failed to connect after reboot
ro-ilk1-i5-650 failed to connect after reboot
ro-ilk-i7-620lm failed to connect after reboot
ro-ivb2-i7-3770 failed to connect after reboot
ro-ivb-i7-3770 failed to connect after reboot
ro-skl-i7-6700hq failed to connect after reboot
ro-snb-i7-2620M failed to connect after reboot
Results at /archive/results/CI_IGT_test/RO_Patchwork_859/
dd7de1c drm-intel-nightly: 2016y-05m-12d-11h-29m-40s UTC integration manifest
0f38f62 drm/i915: Skip clearing the GGTT on full-ppgtt systems
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Skip clearing the GGTT on full-ppgtt systems
2016-05-12 11:19 [PATCH] drm/i915: Skip clearing the GGTT on full-ppgtt systems Chris Wilson
2016-05-12 12:11 ` ✓ Ro.CI.BAT: success for " Patchwork
@ 2016-05-13 6:22 ` Joonas Lahtinen
2016-05-13 7:10 ` Chris Wilson
1 sibling, 1 reply; 4+ messages in thread
From: Joonas Lahtinen @ 2016-05-13 6:22 UTC (permalink / raw)
To: Chris Wilson, intel-gfx; +Cc: David Weinehall
On to, 2016-05-12 at 12:19 +0100, Chris Wilson wrote:
> Under full-ppgtt, access to the global GTT is carefully regulated
> through hardware functions (i.e. userspace cannot read and write to
> arbitrary locations in the GGTT via the GPU). With this restriction in
> place, we can forgo clearing stale entries from the GGTT as they will
> not be accessed.
>
> For aliasing-ppgtt, we could almost do the same except that we do allow
> userspace access to the global-GTT via execbuf in order to workraound
> some quirks of certain instructions. (This execbuf path is filtered out
> with EINVAL on full-ppgtt.)
>
> The most dramatic effect this will have will be during resume, as with
> full-ppgtt the GGTT is only used sparingly.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: David Weinehall <david.weinehall@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 319f3b459b3e..7eab619a3eb2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2477,6 +2477,13 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
> assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
> }
>
> +static void nop_clear_range(struct i915_address_space *vm,
> + uint64_t start,
> + uint64_t length,
> + bool use_scratch)
> +{
> +}
> +
> static void gen8_ggtt_clear_range(struct i915_address_space *vm,
> uint64_t start,
> uint64_t length,
> @@ -3072,14 +3079,17 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>
> ret = ggtt_probe_common(dev, ggtt->size);
>
> - ggtt->base.clear_range = gen8_ggtt_clear_range;
> - if (IS_CHERRYVIEW(dev_priv))
> - ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
> - else
> - ggtt->base.insert_entries = gen8_ggtt_insert_entries;
This form looks better to my eyes, especially once you start adding a
couple of other if ()'s for same callback.
BAT seems to have gone crazy with this patch, too.
> ggtt->base.bind_vma = ggtt_bind_vma;
> ggtt->base.unbind_vma = ggtt_unbind_vma;
>
> + ggtt->base.clear_range = nop_clear_range;
> + if (!USES_FULL_PPGTT(dev_priv))
> + ggtt->base.clear_range = gen8_ggtt_clear_range;
> +
> + ggtt->base.insert_entries = gen8_ggtt_insert_entries;
> + if (IS_CHERRYVIEW(dev_priv))
> + ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
> +
> return ret;
> }
>
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/i915: Skip clearing the GGTT on full-ppgtt systems
2016-05-13 6:22 ` [PATCH] " Joonas Lahtinen
@ 2016-05-13 7:10 ` Chris Wilson
0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2016-05-13 7:10 UTC (permalink / raw)
To: Joonas Lahtinen; +Cc: intel-gfx, David Weinehall
On Fri, May 13, 2016 at 09:22:03AM +0300, Joonas Lahtinen wrote:
> On to, 2016-05-12 at 12:19 +0100, Chris Wilson wrote:
> > Under full-ppgtt, access to the global GTT is carefully regulated
> > through hardware functions (i.e. userspace cannot read and write to
> > arbitrary locations in the GGTT via the GPU). With this restriction in
> > place, we can forgo clearing stale entries from the GGTT as they will
> > not be accessed.
> >
> > For aliasing-ppgtt, we could almost do the same except that we do allow
> > userspace access to the global-GTT via execbuf in order to workraound
> > some quirks of certain instructions. (This execbuf path is filtered out
> > with EINVAL on full-ppgtt.)
> >
> > The most dramatic effect this will have will be during resume, as with
> > full-ppgtt the GGTT is only used sparingly.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: David Weinehall <david.weinehall@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_gem_gtt.c | 20 +++++++++++++++-----
> > 1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 319f3b459b3e..7eab619a3eb2 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -2477,6 +2477,13 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
> > assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
> > }
> >
> > +static void nop_clear_range(struct i915_address_space *vm,
> > + uint64_t start,
> > + uint64_t length,
> > + bool use_scratch)
> > +{
> > +}
> > +
> > static void gen8_ggtt_clear_range(struct i915_address_space *vm,
> > uint64_t start,
> > uint64_t length,
> > @@ -3072,14 +3079,17 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
> >
> > ret = ggtt_probe_common(dev, ggtt->size);
> >
> > - ggtt->base.clear_range = gen8_ggtt_clear_range;
> > - if (IS_CHERRYVIEW(dev_priv))
> > - ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
> > - else
> > - ggtt->base.insert_entries = gen8_ggtt_insert_entries;
>
> This form looks better to my eyes, especially once you start adding a
> couple of other if ()'s for same callback.
This form is actually unusual, the idiom is
func = default;
if (unusual)
func = bar;
which is meant to highlight the normal vs exceptional paths.
> BAT seems to have gone crazy with this patch, too.
There's no reason for this patch to make anything go crazy. The danger
is of course that we don't clear PTEs and so if you access unassigned
ranges, you write into pages owned by the system and not us. That would
be a major bug in our handling of the GGTT - atm the only known bugs
relate to partial vma (afaik).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-05-13 7:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-12 11:19 [PATCH] drm/i915: Skip clearing the GGTT on full-ppgtt systems Chris Wilson
2016-05-12 12:11 ` ✓ Ro.CI.BAT: success for " Patchwork
2016-05-13 6:22 ` [PATCH] " Joonas Lahtinen
2016-05-13 7:10 ` Chris Wilson
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.