Re: [PATCH] drm/i915: Actually delete gpu reloc selftests
On Fri, Aug 20, 2021 at 07:59:04PM +0200, Daniel Vetter wrote:
> On Fri, Aug 20, 2021 at 7:00 PM Rodrigo Vivi wrote:
> >
> > On Fri, Aug 20, 2021 at 05:49:32PM +0200, Daniel Vetter wrote:
> > > In
> > >
> > > commit 8e02cceb1f1f4f254625e5338dd997ff61ab40d7
> > > Author: Daniel Vetter
> > > Date: Tue Aug 3 14:48:33 2021 +0200
> > >
> > > drm/i915: delete gpu reloc code
> >
> > it would be better with dim cite format...
> >
> > do we need the Fixes: tag?
>
> I did delete the selftest, I just forgot to delete the code. So no
> Fixes: imo. I'll bikeshed the commit citation.
>
> > anyway:
> >
> > Reviewed-by: Rodrigo Vivi
>
> Thanks for the review, will merge when CI approves too, one never knows.
Well CI fell over in noise, but it builds still, so good enough for this
one :-)
-Daniel
> -Daniel
>
> >
> >
> > >
> > > I deleted the gpu relocation code and the selftest include and
> > > enabling, but accidentally forgot about the selftest source code.
> > >
> > > Fix this oversight.
> > >
> > > Signed-off-by: Daniel Vetter
> > > Cc: Jon Bloomfield
> > > Cc: Chris Wilson
> > > Cc: Maarten Lankhorst
> > > Cc: Daniel Vetter
> > > Cc: Joonas Lahtinen
> > > Cc: "Thomas Hellström"
> > > Cc: Matthew Auld
> > > Cc: Lionel Landwerlin
> > > Cc: Dave Airlie
> > > Cc: Jason Ekstrand
> > > ---
> > > .../i915/gem/selftests/i915_gem_execbuffer.c | 190 --
> > > 1 file changed, 190 deletions(-)
> > > delete mode 100644
> > > drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> > > b/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> > > deleted file mode 100644
> > > index 16162fc2782d..
> > > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> > > +++ /dev/null
> > > @@ -1,190 +0,0 @@
> > > -// SPDX-License-Identifier: MIT
> > > -/*
> > > - * Copyright © 2020 Intel Corporation
> > > - */
> > > -
> > > -#include "i915_selftest.h"
> > > -
> > > -#include "gt/intel_engine_pm.h"
> > > -#include "selftests/igt_flush_test.h"
> > > -
> > > -static u64 read_reloc(const u32 *map, int x, const u64 mask)
> > > -{
> > > - u64 reloc;
> > > -
> > > - memcpy(, [x], sizeof(reloc));
> > > - return reloc & mask;
> > > -}
> > > -
> > > -static int __igt_gpu_reloc(struct i915_execbuffer *eb,
> > > -struct drm_i915_gem_object *obj)
> > > -{
> > > - const unsigned int offsets[] = { 8, 3, 0 };
> > > - const u64 mask =
> > > - GENMASK_ULL(eb->reloc_cache.use_64bit_reloc ? 63 : 31, 0);
> > > - const u32 *map = page_mask_bits(obj->mm.mapping);
> > > - struct i915_request *rq;
> > > - struct i915_vma *vma;
> > > - int err;
> > > - int i;
> > > -
> > > - vma = i915_vma_instance(obj, eb->context->vm, NULL);
> > > - if (IS_ERR(vma))
> > > - return PTR_ERR(vma);
> > > -
> > > - err = i915_gem_object_lock(obj, >ww);
> > > - if (err)
> > > - return err;
> > > -
> > > - err = i915_vma_pin_ww(vma, >ww, 0, 0, PIN_USER | PIN_HIGH);
> > > - if (err)
> > > - return err;
> > > -
> > > - /* 8-Byte aligned */
> > > - err = __reloc_entry_gpu(eb, vma, offsets[0] * sizeof(u32), 0);
> > > - if (err <= 0)
> > > - goto reloc_err;
> > > -
> > > - /* !8-Byte aligned */
> > > - err = __reloc_entry_gpu(eb, vma, offsets[1] * sizeof(u32), 1);
> > > - if (err <= 0)
> > > - goto reloc_err;
> > > -
> > > - /* Skip to the end of the cmd page */
> > > - i = PAGE_SIZE / sizeof(u32) - 1;
> > > - i -= eb->reloc_cache.rq_size;
> > > - memset32(eb->reloc_cache.rq_cmd + eb->reloc_cache.rq_size,
> > > - MI_NOOP, i);
> > > - eb->reloc_cache.rq_size += i;
> > > -
> > > - /* Force next batch */
> > > - err = __reloc_entry_gpu(eb, vma, offsets[2] * sizeof(u32), 2);
> > > - if (err <= 0)
> > > - goto reloc_err;
> > > -
> > > - GEM_BUG_ON(!eb->reloc_cache.rq);
> > > - rq = i915_request_get(eb->reloc_cache.rq);
> > > - reloc_gpu_flush(eb, >reloc_cache);
> > > - GEM_BUG_ON(eb->reloc_cache.rq);
> > > -
> > > - err = i915_gem_object_wait(obj, I915_WAIT_INTERRUPTIBLE, HZ / 2);
> > > - if (err) {
> > > - intel_gt_set_wedged(eb->engine->gt);
> > > - goto put_rq;
> > > - }
> > > -
> > > - if (!i915_request_completed(rq)) {
> > > - pr_err("%s: did not wait for relocations!\n",
> > > eb->engine->name);
> > > - err = -EINVAL;
> > > - goto put_rq;
> > > - }
> > > -
> > > - for (i = 0; i < ARRAY_SIZE(offsets); i++) {
> > > - u64 reloc = read_reloc(map, offsets[i], mask);
> > > -
> > > - if (reloc != i) {
> > > - pr_err("%s[%d]: map[%d] %llx != %x\n",
> > > -eb->engine->name, i, offsets[i], reloc, i);
> > > - err =
Re: [PATCH] drm/i915: Actually delete gpu reloc selftests
On Fri, Aug 20, 2021 at 7:00 PM Rodrigo Vivi wrote:
>
> On Fri, Aug 20, 2021 at 05:49:32PM +0200, Daniel Vetter wrote:
> > In
> >
> > commit 8e02cceb1f1f4f254625e5338dd997ff61ab40d7
> > Author: Daniel Vetter
> > Date: Tue Aug 3 14:48:33 2021 +0200
> >
> > drm/i915: delete gpu reloc code
>
> it would be better with dim cite format...
>
> do we need the Fixes: tag?
I did delete the selftest, I just forgot to delete the code. So no
Fixes: imo. I'll bikeshed the commit citation.
> anyway:
>
> Reviewed-by: Rodrigo Vivi
Thanks for the review, will merge when CI approves too, one never knows.
-Daniel
>
>
> >
> > I deleted the gpu relocation code and the selftest include and
> > enabling, but accidentally forgot about the selftest source code.
> >
> > Fix this oversight.
> >
> > Signed-off-by: Daniel Vetter
> > Cc: Jon Bloomfield
> > Cc: Chris Wilson
> > Cc: Maarten Lankhorst
> > Cc: Daniel Vetter
> > Cc: Joonas Lahtinen
> > Cc: "Thomas Hellström"
> > Cc: Matthew Auld
> > Cc: Lionel Landwerlin
> > Cc: Dave Airlie
> > Cc: Jason Ekstrand
> > ---
> > .../i915/gem/selftests/i915_gem_execbuffer.c | 190 --
> > 1 file changed, 190 deletions(-)
> > delete mode 100644 drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> >
> > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> > b/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> > deleted file mode 100644
> > index 16162fc2782d..
> > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> > +++ /dev/null
> > @@ -1,190 +0,0 @@
> > -// SPDX-License-Identifier: MIT
> > -/*
> > - * Copyright © 2020 Intel Corporation
> > - */
> > -
> > -#include "i915_selftest.h"
> > -
> > -#include "gt/intel_engine_pm.h"
> > -#include "selftests/igt_flush_test.h"
> > -
> > -static u64 read_reloc(const u32 *map, int x, const u64 mask)
> > -{
> > - u64 reloc;
> > -
> > - memcpy(, [x], sizeof(reloc));
> > - return reloc & mask;
> > -}
> > -
> > -static int __igt_gpu_reloc(struct i915_execbuffer *eb,
> > -struct drm_i915_gem_object *obj)
> > -{
> > - const unsigned int offsets[] = { 8, 3, 0 };
> > - const u64 mask =
> > - GENMASK_ULL(eb->reloc_cache.use_64bit_reloc ? 63 : 31, 0);
> > - const u32 *map = page_mask_bits(obj->mm.mapping);
> > - struct i915_request *rq;
> > - struct i915_vma *vma;
> > - int err;
> > - int i;
> > -
> > - vma = i915_vma_instance(obj, eb->context->vm, NULL);
> > - if (IS_ERR(vma))
> > - return PTR_ERR(vma);
> > -
> > - err = i915_gem_object_lock(obj, >ww);
> > - if (err)
> > - return err;
> > -
> > - err = i915_vma_pin_ww(vma, >ww, 0, 0, PIN_USER | PIN_HIGH);
> > - if (err)
> > - return err;
> > -
> > - /* 8-Byte aligned */
> > - err = __reloc_entry_gpu(eb, vma, offsets[0] * sizeof(u32), 0);
> > - if (err <= 0)
> > - goto reloc_err;
> > -
> > - /* !8-Byte aligned */
> > - err = __reloc_entry_gpu(eb, vma, offsets[1] * sizeof(u32), 1);
> > - if (err <= 0)
> > - goto reloc_err;
> > -
> > - /* Skip to the end of the cmd page */
> > - i = PAGE_SIZE / sizeof(u32) - 1;
> > - i -= eb->reloc_cache.rq_size;
> > - memset32(eb->reloc_cache.rq_cmd + eb->reloc_cache.rq_size,
> > - MI_NOOP, i);
> > - eb->reloc_cache.rq_size += i;
> > -
> > - /* Force next batch */
> > - err = __reloc_entry_gpu(eb, vma, offsets[2] * sizeof(u32), 2);
> > - if (err <= 0)
> > - goto reloc_err;
> > -
> > - GEM_BUG_ON(!eb->reloc_cache.rq);
> > - rq = i915_request_get(eb->reloc_cache.rq);
> > - reloc_gpu_flush(eb, >reloc_cache);
> > - GEM_BUG_ON(eb->reloc_cache.rq);
> > -
> > - err = i915_gem_object_wait(obj, I915_WAIT_INTERRUPTIBLE, HZ / 2);
> > - if (err) {
> > - intel_gt_set_wedged(eb->engine->gt);
> > - goto put_rq;
> > - }
> > -
> > - if (!i915_request_completed(rq)) {
> > - pr_err("%s: did not wait for relocations!\n",
> > eb->engine->name);
> > - err = -EINVAL;
> > - goto put_rq;
> > - }
> > -
> > - for (i = 0; i < ARRAY_SIZE(offsets); i++) {
> > - u64 reloc = read_reloc(map, offsets[i], mask);
> > -
> > - if (reloc != i) {
> > - pr_err("%s[%d]: map[%d] %llx != %x\n",
> > -eb->engine->name, i, offsets[i], reloc, i);
> > - err = -EINVAL;
> > - }
> > - }
> > - if (err)
> > - igt_hexdump(map, 4096);
> > -
> > -put_rq:
> > - i915_request_put(rq);
> > -unpin_vma:
> > - i915_vma_unpin(vma);
> > - return err;
> > -
> > -reloc_err:
> > - if (!err)
> > - err = -EIO;
> > - goto unpin_vma;
> > -}
> > -
> > -static int igt_gpu_reloc(void *arg)
> > -{
> > - struct i915_execbuffer eb;
> > - struct drm_i915_gem_object
Re: [PATCH] drm/i915: Actually delete gpu reloc selftests
On Fri, Aug 20, 2021 at 05:49:32PM +0200, Daniel Vetter wrote:
> In
>
> commit 8e02cceb1f1f4f254625e5338dd997ff61ab40d7
> Author: Daniel Vetter
> Date: Tue Aug 3 14:48:33 2021 +0200
>
> drm/i915: delete gpu reloc code
it would be better with dim cite format...
do we need the Fixes: tag?
anyway:
Reviewed-by: Rodrigo Vivi
>
> I deleted the gpu relocation code and the selftest include and
> enabling, but accidentally forgot about the selftest source code.
>
> Fix this oversight.
>
> Signed-off-by: Daniel Vetter
> Cc: Jon Bloomfield
> Cc: Chris Wilson
> Cc: Maarten Lankhorst
> Cc: Daniel Vetter
> Cc: Joonas Lahtinen
> Cc: "Thomas Hellström"
> Cc: Matthew Auld
> Cc: Lionel Landwerlin
> Cc: Dave Airlie
> Cc: Jason Ekstrand
> ---
> .../i915/gem/selftests/i915_gem_execbuffer.c | 190 --
> 1 file changed, 190 deletions(-)
> delete mode 100644 drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
>
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> deleted file mode 100644
> index 16162fc2782d..
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_execbuffer.c
> +++ /dev/null
> @@ -1,190 +0,0 @@
> -// SPDX-License-Identifier: MIT
> -/*
> - * Copyright © 2020 Intel Corporation
> - */
> -
> -#include "i915_selftest.h"
> -
> -#include "gt/intel_engine_pm.h"
> -#include "selftests/igt_flush_test.h"
> -
> -static u64 read_reloc(const u32 *map, int x, const u64 mask)
> -{
> - u64 reloc;
> -
> - memcpy(, [x], sizeof(reloc));
> - return reloc & mask;
> -}
> -
> -static int __igt_gpu_reloc(struct i915_execbuffer *eb,
> -struct drm_i915_gem_object *obj)
> -{
> - const unsigned int offsets[] = { 8, 3, 0 };
> - const u64 mask =
> - GENMASK_ULL(eb->reloc_cache.use_64bit_reloc ? 63 : 31, 0);
> - const u32 *map = page_mask_bits(obj->mm.mapping);
> - struct i915_request *rq;
> - struct i915_vma *vma;
> - int err;
> - int i;
> -
> - vma = i915_vma_instance(obj, eb->context->vm, NULL);
> - if (IS_ERR(vma))
> - return PTR_ERR(vma);
> -
> - err = i915_gem_object_lock(obj, >ww);
> - if (err)
> - return err;
> -
> - err = i915_vma_pin_ww(vma, >ww, 0, 0, PIN_USER | PIN_HIGH);
> - if (err)
> - return err;
> -
> - /* 8-Byte aligned */
> - err = __reloc_entry_gpu(eb, vma, offsets[0] * sizeof(u32), 0);
> - if (err <= 0)
> - goto reloc_err;
> -
> - /* !8-Byte aligned */
> - err = __reloc_entry_gpu(eb, vma, offsets[1] * sizeof(u32), 1);
> - if (err <= 0)
> - goto reloc_err;
> -
> - /* Skip to the end of the cmd page */
> - i = PAGE_SIZE / sizeof(u32) - 1;
> - i -= eb->reloc_cache.rq_size;
> - memset32(eb->reloc_cache.rq_cmd + eb->reloc_cache.rq_size,
> - MI_NOOP, i);
> - eb->reloc_cache.rq_size += i;
> -
> - /* Force next batch */
> - err = __reloc_entry_gpu(eb, vma, offsets[2] * sizeof(u32), 2);
> - if (err <= 0)
> - goto reloc_err;
> -
> - GEM_BUG_ON(!eb->reloc_cache.rq);
> - rq = i915_request_get(eb->reloc_cache.rq);
> - reloc_gpu_flush(eb, >reloc_cache);
> - GEM_BUG_ON(eb->reloc_cache.rq);
> -
> - err = i915_gem_object_wait(obj, I915_WAIT_INTERRUPTIBLE, HZ / 2);
> - if (err) {
> - intel_gt_set_wedged(eb->engine->gt);
> - goto put_rq;
> - }
> -
> - if (!i915_request_completed(rq)) {
> - pr_err("%s: did not wait for relocations!\n", eb->engine->name);
> - err = -EINVAL;
> - goto put_rq;
> - }
> -
> - for (i = 0; i < ARRAY_SIZE(offsets); i++) {
> - u64 reloc = read_reloc(map, offsets[i], mask);
> -
> - if (reloc != i) {
> - pr_err("%s[%d]: map[%d] %llx != %x\n",
> -eb->engine->name, i, offsets[i], reloc, i);
> - err = -EINVAL;
> - }
> - }
> - if (err)
> - igt_hexdump(map, 4096);
> -
> -put_rq:
> - i915_request_put(rq);
> -unpin_vma:
> - i915_vma_unpin(vma);
> - return err;
> -
> -reloc_err:
> - if (!err)
> - err = -EIO;
> - goto unpin_vma;
> -}
> -
> -static int igt_gpu_reloc(void *arg)
> -{
> - struct i915_execbuffer eb;
> - struct drm_i915_gem_object *scratch;
> - int err = 0;
> - u32 *map;
> -
> - eb.i915 = arg;
> -
> - scratch = i915_gem_object_create_internal(eb.i915, 4096);
> - if (IS_ERR(scratch))
> - return PTR_ERR(scratch);
> -
> - map = i915_gem_object_pin_map_unlocked(scratch, I915_MAP_WC);
> - if (IS_ERR(map)) {
> - err = PTR_ERR(map);
> - goto err_scratch;
> - }
> -
> - intel_gt_pm_get(>gt);
> -
> - for_each_uabi_engine(eb.engine, eb.i915) {
> - if (intel_engine_requires_cmd_parser(eb.engine) ||
> -
