| Message ID | 20210317143935.2094831-1-jason@jlekstrand.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gem: Drop relocation support on all new hardware (v6) | expand |
I should probably have said that the reviews are on v5 and it's very different in v6 so they should probably be considered dropped until re-confirmed. On Wed, Mar 17, 2021 at 9:39 AM Jason Ekstrand <jason@jlekstrand.net> wrote: > > 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 > > v6 (Jason Ekstrand): > - Move the check to eb_validate_vma() with all the other exec_object > validation checks. > > Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> > Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com> > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index 99772f37bff60..c082fb0bef330 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -483,6 +483,13 @@ eb_validate_vma(struct i915_execbuffer *eb, > struct drm_i915_gem_exec_object2 *entry, > struct i915_vma *vma) > { > + /* Relocations are disallowed for all platforms after TGL-LP. This > + * also covers all platforms with local memory. > + */ > + if (entry->relocation_count && > + INTEL_GEN(eb->i915) >= 12 && !IS_TIGERLAKE(eb->i915)) > + return -EINVAL; > + > if (unlikely(entry->flags & eb->invalid_flags)) > return -EINVAL; > > -- > 2.29.2 >
On Wed, Mar 17, 2021 at 09:41:46AM -0500, Jason Ekstrand wrote: > I should probably have said that the reviews are on v5 and it's very > different in v6 so they should probably be considered dropped until > re-confirmed. You're checking relocation_count early in do_execbuffer() so imo there's no option to pass execbuf with relocations now. I keep my r-b. -- Zbigniew > > On Wed, Mar 17, 2021 at 9:39 AM Jason Ekstrand <jason@jlekstrand.net> wrote: > > > > 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 > > > > v6 (Jason Ekstrand): > > - Move the check to eb_validate_vma() with all the other exec_object > > validation checks. > > > > Signed-off-by: Jason Ekstrand <jason@jlekstrand.net> > > Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com> > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > --- > > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > > index 99772f37bff60..c082fb0bef330 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > > @@ -483,6 +483,13 @@ eb_validate_vma(struct i915_execbuffer *eb, > > struct drm_i915_gem_exec_object2 *entry, > > struct i915_vma *vma) > > { > > + /* Relocations are disallowed for all platforms after TGL-LP. This > > + * also covers all platforms with local memory. > > + */ > > + if (entry->relocation_count && > > + INTEL_GEN(eb->i915) >= 12 && !IS_TIGERLAKE(eb->i915)) > > + return -EINVAL; > > + > > if (unlikely(entry->flags & eb->invalid_flags)) > > return -EINVAL; > > > > -- > > 2.29.2 > >
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 99772f37bff60..c082fb0bef330 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -483,6 +483,13 @@ eb_validate_vma(struct i915_execbuffer *eb, struct drm_i915_gem_exec_object2 *entry, struct i915_vma *vma) { + /* Relocations are disallowed for all platforms after TGL-LP. This + * also covers all platforms with local memory. + */ + if (entry->relocation_count && + INTEL_GEN(eb->i915) >= 12 && !IS_TIGERLAKE(eb->i915)) + return -EINVAL; + if (unlikely(entry->flags & eb->invalid_flags)) return -EINVAL;