On Wed, Mar 17, 2021 at 2:55 AM Petri Latvala <petri.latvala@xxxxxxxxx> wrote:
>
> On Mon, Mar 15, 2021 at 10:29:20PM -0700, Ashutosh Dixit wrote:
> > From: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
> >
> > The Vulkan driver in Mesa for Intel hardware never uses relocations if
> > it's running on a version of i915 that supports at least softpin which
> > all versions of i915 supporting Gen12 do. On the OpenGL side, Gen12+ is
> > only supported by iris which never uses relocations. The older i965
> > driver in Mesa does use relocations but it only supports Intel hardware
> > through Gen11 and has been deprecated for all hardware Gen9+. The
> > compute driver also never uses relocations. This only leaves the media
> > driver which is supposed to be switching to softpin going forward.
> > Making softpin a requirement for all future hardware seems reasonable.
> >
> > There is one piece of hardware enabled by default in i915: RKL which was
> > enabled by e22fa6f0a976 which has not yet landed in drm-next so this
> > almost but not really a userspace API change for RKL. If it becomes a
> > problem, we can always add !IS_ROCKETLAKE(eb->i915) to the condition.
> >
> > Rejecting relocations starting with newer Gen12 platforms has the
> > benefit that we don't have to bother supporting it on platforms with
> > local memory. Given how much CPU touching of memory is required for
> > relocations, not having to do so on platforms where not all memory is
> > directly CPU-accessible carries significant advantages.
> >
> > v2 (Jason Ekstrand):
> > - Allow TGL-LP platforms as they've already shipped
> >
> > v3 (Jason Ekstrand):
> > - WARN_ON platforms with LMEM support in case the check is wrong
> >
> > v4 (Jason Ekstrand):
> > - Call out Rocket Lake in the commit message
> >
> > v5 (Jason Ekstrand):
> > - Drop the HAS_LMEM check as it's already covered by the version check
> >
> > Signed-off-by: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
> > Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@xxxxxxxxx>
> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > index 99772f37bff60..f66cff2943baa 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > @@ -1764,7 +1764,8 @@ eb_relocate_vma_slow(struct i915_execbuffer *eb, struct eb_vma *ev)
> > return err;
> > }
> >
> > -static int check_relocations(const struct drm_i915_gem_exec_object2 *entry)
> > +static int check_relocations(const struct i915_execbuffer *eb,
> > + const struct drm_i915_gem_exec_object2 *entry)
> > {
> > const char __user *addr, *end;
> > unsigned long size;
> > @@ -1774,6 +1775,12 @@ static int check_relocations(const struct drm_i915_gem_exec_object2 *entry)
> > if (size == 0)
> > return 0;
> >
> > + /* Relocations are disallowed for all platforms after TGL-LP. This
> > + * also covers all platforms with local memory.
> > + */
> > + if (INTEL_GEN(eb->i915) >= 12 && !IS_TIGERLAKE(eb->i915))
> > + return -EINVAL;
> > +
> > if (size > N_RELOC(ULONG_MAX))
> > return -EINVAL;
> >
> > @@ -1807,7 +1814,7 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
> > if (nreloc == 0)
> > continue;
> >
> > - err = check_relocations(&eb->exec[i]);
> > + err = check_relocations(eb, &eb->exec[i]);
> > if (err)
> > goto err;
> >
> > @@ -1880,7 +1887,7 @@ static int eb_prefault_relocations(const struct i915_execbuffer *eb)
> > for (i = 0; i < count; i++) {
> > int err;
> >
> > - err = check_relocations(&eb->exec[i]);
> > + err = check_relocations(eb, &eb->exec[i]);
> > if (err)
> > return err;
> > }
>
>
> Disclaimer: I don't know deeply how any of this works.
>
> check_relocations() is only called if eb_relocate_parse goes on its
> slowpath, fast path doesn't check for gen. Should it, or am I
> misunderstanding something?
Good eye! Yeah, this doesn't work for the reason you just said. I
was trying to avoid an up-front list walk of the object list but if we
want to be very sure it's correct, we probably have to.
--Jason
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
