diff mbox series

drm/i915/gem: Drop relocation support on all new hardware (v6)

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

Commit Message

Jason Ekstrand March 17, 2021, 2:39 p.m. UTC
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(+)

Comments

Jason Ekstrand March 17, 2021, 2:41 p.m. UTC | #1
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
>
Zbigniew Kempczyński March 17, 2021, 3:06 p.m. UTC | #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 mbox series

Patch

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;