Re: [Intel-gfx] [PATCH] drm/i915/selftests: Drop igt_cs_tlb

2023-03-23 Thread Andi Shyti
Hi,

On Mon, Mar 20, 2023 at 08:21:17PM +0100, Andi Shyti wrote:
> From: Jonathan Cavitt 
> 
> The gt_tlb live selftest has the same code coverage as the
> igt_cs_tlb subtest of gtt, except it is better at detecting
> TLB bugs.  Furthermore, while igt_cs_tlb is hitting some
> unforeseen issues, these issues are either false positives
> due to the test being poorly formatted, or are true
> positives that can be more easily diagnosed with smaller
> tests.  As such, igt_cs_tlb is superceded by and obsoleted
> by gt_tlb, meaning it can be removed.
> 
> Signed-off-by: Jonathan Cavitt 
> Reviewed-by: Andrzej Hajda 
> Acked-by: Nirmoy Das 
> Signed-off-by: Andi Shyti 

Reviewed-by: Andi Shyti 

This patch, anyway needs some follow up for adding the range
TLB invalidation part.

Andi


[Intel-gfx] [PATCH] drm/i915/selftests: Drop igt_cs_tlb

2023-03-20 Thread Andi Shyti
From: Jonathan Cavitt 

The gt_tlb live selftest has the same code coverage as the
igt_cs_tlb subtest of gtt, except it is better at detecting
TLB bugs.  Furthermore, while igt_cs_tlb is hitting some
unforeseen issues, these issues are either false positives
due to the test being poorly formatted, or are true
positives that can be more easily diagnosed with smaller
tests.  As such, igt_cs_tlb is superceded by and obsoleted
by gt_tlb, meaning it can be removed.

Signed-off-by: Jonathan Cavitt 
Reviewed-by: Andrzej Hajda 
Acked-by: Nirmoy Das 
Signed-off-by: Andi Shyti 
---
Hi,

just respinning this patch that has been sent some times ago.
It should be ready to be pushed.

Andi

 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 356 --
 1 file changed, 356 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 01e75160a84ab..5361ce70d3f29 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1940,361 +1940,6 @@ int i915_gem_gtt_mock_selftests(void)
return err;
 }
 
-static int context_sync(struct intel_context *ce)
-{
-   struct i915_request *rq;
-   long timeout;
-
-   rq = intel_context_create_request(ce);
-   if (IS_ERR(rq))
-   return PTR_ERR(rq);
-
-   i915_request_get(rq);
-   i915_request_add(rq);
-
-   timeout = i915_request_wait(rq, 0, HZ / 5);
-   i915_request_put(rq);
-
-   return timeout < 0 ? -EIO : 0;
-}
-
-static struct i915_request *
-submit_batch(struct intel_context *ce, u64 addr)
-{
-   struct i915_request *rq;
-   int err;
-
-   rq = intel_context_create_request(ce);
-   if (IS_ERR(rq))
-   return rq;
-
-   err = 0;
-   if (rq->engine->emit_init_breadcrumb) /* detect a hang */
-   err = rq->engine->emit_init_breadcrumb(rq);
-   if (err == 0)
-   err = rq->engine->emit_bb_start(rq, addr, 0, 0);
-
-   if (err == 0)
-   i915_request_get(rq);
-   i915_request_add(rq);
-
-   return err ? ERR_PTR(err) : rq;
-}
-
-static u32 *spinner(u32 *batch, int i)
-{
-   return batch + i * 64 / sizeof(*batch) + 4;
-}
-
-static void end_spin(u32 *batch, int i)
-{
-   *spinner(batch, i) = MI_BATCH_BUFFER_END;
-   wmb();
-}
-
-static int igt_cs_tlb(void *arg)
-{
-   const unsigned int count = PAGE_SIZE / 64;
-   const unsigned int chunk_size = count * PAGE_SIZE;
-   struct drm_i915_private *i915 = arg;
-   struct drm_i915_gem_object *bbe, *act, *out;
-   struct i915_gem_engines_iter it;
-   struct i915_address_space *vm;
-   struct i915_gem_context *ctx;
-   struct intel_context *ce;
-   struct i915_vma *vma;
-   I915_RND_STATE(prng);
-   struct file *file;
-   unsigned int i;
-   u32 *result;
-   u32 *batch;
-   int err = 0;
-
-   /*
-* Our mission here is to fool the hardware to execute something
-* from scratch as it has not seen the batch move (due to missing
-* the TLB invalidate).
-*/
-
-   file = mock_file(i915);
-   if (IS_ERR(file))
-   return PTR_ERR(file);
-
-   ctx = live_context(i915, file);
-   if (IS_ERR(ctx)) {
-   err = PTR_ERR(ctx);
-   goto out_unlock;
-   }
-
-   vm = i915_gem_context_get_eb_vm(ctx);
-   if (i915_is_ggtt(vm))
-   goto out_vm;
-
-   /* Create two pages; dummy we prefill the TLB, and intended */
-   bbe = i915_gem_object_create_internal(i915, PAGE_SIZE);
-   if (IS_ERR(bbe)) {
-   err = PTR_ERR(bbe);
-   goto out_vm;
-   }
-
-   batch = i915_gem_object_pin_map_unlocked(bbe, I915_MAP_WC);
-   if (IS_ERR(batch)) {
-   err = PTR_ERR(batch);
-   goto out_put_bbe;
-   }
-   memset32(batch, MI_BATCH_BUFFER_END, PAGE_SIZE / sizeof(u32));
-   i915_gem_object_flush_map(bbe);
-   i915_gem_object_unpin_map(bbe);
-
-   act = i915_gem_object_create_internal(i915, PAGE_SIZE);
-   if (IS_ERR(act)) {
-   err = PTR_ERR(act);
-   goto out_put_bbe;
-   }
-
-   /* Track the execution of each request by writing into different slot */
-   batch = i915_gem_object_pin_map_unlocked(act, I915_MAP_WC);
-   if (IS_ERR(batch)) {
-   err = PTR_ERR(batch);
-   goto out_put_act;
-   }
-   for (i = 0; i < count; i++) {
-   u32 *cs = batch + i * 64 / sizeof(*cs);
-   u64 addr = (vm->total - PAGE_SIZE) + i * sizeof(u32);
-
-   GEM_BUG_ON(GRAPHICS_VER(i915) < 6);
-   cs[0] = MI_STORE_DWORD_IMM_GEN4;
-   if (GRAPHICS_VER(i915) >= 8) {
-   cs[1] = lower_32_bits(addr);
-   cs[2] = upper_32_bits(addr);
-   cs[3] = i;
-   cs[4] = MI_NOOP;
-   

Re: [Intel-gfx] [PATCH] drm/i915/selftests: Drop igt_cs_tlb

2023-03-20 Thread Das, Nirmoy



On 2/17/2023 11:33 PM, Jonathan Cavitt wrote:

The gt_tlb live selftest has the same code coverage as the
igt_cs_tlb subtest of gtt, except it is better at detecting
TLB bugs.  Furthermore, while igt_cs_tlb is hitting some
unforeseen issues, these issues are either false positives
due to the test being poorly formatted, or are true
positives that can be more easily diagnosed with smaller
tests.  As such, igt_cs_tlb is superceded by and obsoleted
by gt_tlb, meaning it can be removed.

Signed-off-by: Jonathan Cavitt 


Acked-by: Nirmoy Das 



---
  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 356 --
  1 file changed, 356 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 01e75160a84a..5361ce70d3f2 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1940,361 +1940,6 @@ int i915_gem_gtt_mock_selftests(void)
return err;
  }
  
-static int context_sync(struct intel_context *ce)

-{
-   struct i915_request *rq;
-   long timeout;
-
-   rq = intel_context_create_request(ce);
-   if (IS_ERR(rq))
-   return PTR_ERR(rq);
-
-   i915_request_get(rq);
-   i915_request_add(rq);
-
-   timeout = i915_request_wait(rq, 0, HZ / 5);
-   i915_request_put(rq);
-
-   return timeout < 0 ? -EIO : 0;
-}
-
-static struct i915_request *
-submit_batch(struct intel_context *ce, u64 addr)
-{
-   struct i915_request *rq;
-   int err;
-
-   rq = intel_context_create_request(ce);
-   if (IS_ERR(rq))
-   return rq;
-
-   err = 0;
-   if (rq->engine->emit_init_breadcrumb) /* detect a hang */
-   err = rq->engine->emit_init_breadcrumb(rq);
-   if (err == 0)
-   err = rq->engine->emit_bb_start(rq, addr, 0, 0);
-
-   if (err == 0)
-   i915_request_get(rq);
-   i915_request_add(rq);
-
-   return err ? ERR_PTR(err) : rq;
-}
-
-static u32 *spinner(u32 *batch, int i)
-{
-   return batch + i * 64 / sizeof(*batch) + 4;
-}
-
-static void end_spin(u32 *batch, int i)
-{
-   *spinner(batch, i) = MI_BATCH_BUFFER_END;
-   wmb();
-}
-
-static int igt_cs_tlb(void *arg)
-{
-   const unsigned int count = PAGE_SIZE / 64;
-   const unsigned int chunk_size = count * PAGE_SIZE;
-   struct drm_i915_private *i915 = arg;
-   struct drm_i915_gem_object *bbe, *act, *out;
-   struct i915_gem_engines_iter it;
-   struct i915_address_space *vm;
-   struct i915_gem_context *ctx;
-   struct intel_context *ce;
-   struct i915_vma *vma;
-   I915_RND_STATE(prng);
-   struct file *file;
-   unsigned int i;
-   u32 *result;
-   u32 *batch;
-   int err = 0;
-
-   /*
-* Our mission here is to fool the hardware to execute something
-* from scratch as it has not seen the batch move (due to missing
-* the TLB invalidate).
-*/
-
-   file = mock_file(i915);
-   if (IS_ERR(file))
-   return PTR_ERR(file);
-
-   ctx = live_context(i915, file);
-   if (IS_ERR(ctx)) {
-   err = PTR_ERR(ctx);
-   goto out_unlock;
-   }
-
-   vm = i915_gem_context_get_eb_vm(ctx);
-   if (i915_is_ggtt(vm))
-   goto out_vm;
-
-   /* Create two pages; dummy we prefill the TLB, and intended */
-   bbe = i915_gem_object_create_internal(i915, PAGE_SIZE);
-   if (IS_ERR(bbe)) {
-   err = PTR_ERR(bbe);
-   goto out_vm;
-   }
-
-   batch = i915_gem_object_pin_map_unlocked(bbe, I915_MAP_WC);
-   if (IS_ERR(batch)) {
-   err = PTR_ERR(batch);
-   goto out_put_bbe;
-   }
-   memset32(batch, MI_BATCH_BUFFER_END, PAGE_SIZE / sizeof(u32));
-   i915_gem_object_flush_map(bbe);
-   i915_gem_object_unpin_map(bbe);
-
-   act = i915_gem_object_create_internal(i915, PAGE_SIZE);
-   if (IS_ERR(act)) {
-   err = PTR_ERR(act);
-   goto out_put_bbe;
-   }
-
-   /* Track the execution of each request by writing into different slot */
-   batch = i915_gem_object_pin_map_unlocked(act, I915_MAP_WC);
-   if (IS_ERR(batch)) {
-   err = PTR_ERR(batch);
-   goto out_put_act;
-   }
-   for (i = 0; i < count; i++) {
-   u32 *cs = batch + i * 64 / sizeof(*cs);
-   u64 addr = (vm->total - PAGE_SIZE) + i * sizeof(u32);
-
-   GEM_BUG_ON(GRAPHICS_VER(i915) < 6);
-   cs[0] = MI_STORE_DWORD_IMM_GEN4;
-   if (GRAPHICS_VER(i915) >= 8) {
-   cs[1] = lower_32_bits(addr);
-   cs[2] = upper_32_bits(addr);
-   cs[3] = i;
-   cs[4] = MI_NOOP;
-   cs[5] = MI_BATCH_BUFFER_START_GEN8;
-   } else {
-   cs[1] = 0;
-   

Re: [Intel-gfx] [PATCH] drm/i915/selftests: Drop igt_cs_tlb

2023-02-22 Thread Andrzej Hajda

On 17.02.2023 23:33, Jonathan Cavitt wrote:

The gt_tlb live selftest has the same code coverage as the
igt_cs_tlb subtest of gtt, except it is better at detecting
TLB bugs.  Furthermore, while igt_cs_tlb is hitting some
unforeseen issues, these issues are either false positives
due to the test being poorly formatted, or are true
positives that can be more easily diagnosed with smaller
tests.  As such, igt_cs_tlb is superceded by and obsoleted
by gt_tlb, meaning it can be removed.

Signed-off-by: Jonathan Cavitt 


Reviewed-by: Andrzej Hajda 

Regards
Andrzej


---
  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 356 --
  1 file changed, 356 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 01e75160a84a..5361ce70d3f2 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1940,361 +1940,6 @@ int i915_gem_gtt_mock_selftests(void)
return err;
  }
  
-static int context_sync(struct intel_context *ce)

-{
-   struct i915_request *rq;
-   long timeout;
-
-   rq = intel_context_create_request(ce);
-   if (IS_ERR(rq))
-   return PTR_ERR(rq);
-
-   i915_request_get(rq);
-   i915_request_add(rq);
-
-   timeout = i915_request_wait(rq, 0, HZ / 5);
-   i915_request_put(rq);
-
-   return timeout < 0 ? -EIO : 0;
-}
-
-static struct i915_request *
-submit_batch(struct intel_context *ce, u64 addr)
-{
-   struct i915_request *rq;
-   int err;
-
-   rq = intel_context_create_request(ce);
-   if (IS_ERR(rq))
-   return rq;
-
-   err = 0;
-   if (rq->engine->emit_init_breadcrumb) /* detect a hang */
-   err = rq->engine->emit_init_breadcrumb(rq);
-   if (err == 0)
-   err = rq->engine->emit_bb_start(rq, addr, 0, 0);
-
-   if (err == 0)
-   i915_request_get(rq);
-   i915_request_add(rq);
-
-   return err ? ERR_PTR(err) : rq;
-}
-
-static u32 *spinner(u32 *batch, int i)
-{
-   return batch + i * 64 / sizeof(*batch) + 4;
-}
-
-static void end_spin(u32 *batch, int i)
-{
-   *spinner(batch, i) = MI_BATCH_BUFFER_END;
-   wmb();
-}
-
-static int igt_cs_tlb(void *arg)
-{
-   const unsigned int count = PAGE_SIZE / 64;
-   const unsigned int chunk_size = count * PAGE_SIZE;
-   struct drm_i915_private *i915 = arg;
-   struct drm_i915_gem_object *bbe, *act, *out;
-   struct i915_gem_engines_iter it;
-   struct i915_address_space *vm;
-   struct i915_gem_context *ctx;
-   struct intel_context *ce;
-   struct i915_vma *vma;
-   I915_RND_STATE(prng);
-   struct file *file;
-   unsigned int i;
-   u32 *result;
-   u32 *batch;
-   int err = 0;
-
-   /*
-* Our mission here is to fool the hardware to execute something
-* from scratch as it has not seen the batch move (due to missing
-* the TLB invalidate).
-*/
-
-   file = mock_file(i915);
-   if (IS_ERR(file))
-   return PTR_ERR(file);
-
-   ctx = live_context(i915, file);
-   if (IS_ERR(ctx)) {
-   err = PTR_ERR(ctx);
-   goto out_unlock;
-   }
-
-   vm = i915_gem_context_get_eb_vm(ctx);
-   if (i915_is_ggtt(vm))
-   goto out_vm;
-
-   /* Create two pages; dummy we prefill the TLB, and intended */
-   bbe = i915_gem_object_create_internal(i915, PAGE_SIZE);
-   if (IS_ERR(bbe)) {
-   err = PTR_ERR(bbe);
-   goto out_vm;
-   }
-
-   batch = i915_gem_object_pin_map_unlocked(bbe, I915_MAP_WC);
-   if (IS_ERR(batch)) {
-   err = PTR_ERR(batch);
-   goto out_put_bbe;
-   }
-   memset32(batch, MI_BATCH_BUFFER_END, PAGE_SIZE / sizeof(u32));
-   i915_gem_object_flush_map(bbe);
-   i915_gem_object_unpin_map(bbe);
-
-   act = i915_gem_object_create_internal(i915, PAGE_SIZE);
-   if (IS_ERR(act)) {
-   err = PTR_ERR(act);
-   goto out_put_bbe;
-   }
-
-   /* Track the execution of each request by writing into different slot */
-   batch = i915_gem_object_pin_map_unlocked(act, I915_MAP_WC);
-   if (IS_ERR(batch)) {
-   err = PTR_ERR(batch);
-   goto out_put_act;
-   }
-   for (i = 0; i < count; i++) {
-   u32 *cs = batch + i * 64 / sizeof(*cs);
-   u64 addr = (vm->total - PAGE_SIZE) + i * sizeof(u32);
-
-   GEM_BUG_ON(GRAPHICS_VER(i915) < 6);
-   cs[0] = MI_STORE_DWORD_IMM_GEN4;
-   if (GRAPHICS_VER(i915) >= 8) {
-   cs[1] = lower_32_bits(addr);
-   cs[2] = upper_32_bits(addr);
-   cs[3] = i;
-   cs[4] = MI_NOOP;
-   cs[5] = MI_BATCH_BUFFER_START_GEN8;
-   } else {
-   cs[1] = 

[Intel-gfx] [PATCH] drm/i915/selftests: Drop igt_cs_tlb

2023-02-17 Thread Jonathan Cavitt
The gt_tlb live selftest has the same code coverage as the
igt_cs_tlb subtest of gtt, except it is better at detecting
TLB bugs.  Furthermore, while igt_cs_tlb is hitting some
unforeseen issues, these issues are either false positives
due to the test being poorly formatted, or are true
positives that can be more easily diagnosed with smaller
tests.  As such, igt_cs_tlb is superceded by and obsoleted
by gt_tlb, meaning it can be removed.

Signed-off-by: Jonathan Cavitt 
---
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 356 --
 1 file changed, 356 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 01e75160a84a..5361ce70d3f2 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1940,361 +1940,6 @@ int i915_gem_gtt_mock_selftests(void)
return err;
 }
 
-static int context_sync(struct intel_context *ce)
-{
-   struct i915_request *rq;
-   long timeout;
-
-   rq = intel_context_create_request(ce);
-   if (IS_ERR(rq))
-   return PTR_ERR(rq);
-
-   i915_request_get(rq);
-   i915_request_add(rq);
-
-   timeout = i915_request_wait(rq, 0, HZ / 5);
-   i915_request_put(rq);
-
-   return timeout < 0 ? -EIO : 0;
-}
-
-static struct i915_request *
-submit_batch(struct intel_context *ce, u64 addr)
-{
-   struct i915_request *rq;
-   int err;
-
-   rq = intel_context_create_request(ce);
-   if (IS_ERR(rq))
-   return rq;
-
-   err = 0;
-   if (rq->engine->emit_init_breadcrumb) /* detect a hang */
-   err = rq->engine->emit_init_breadcrumb(rq);
-   if (err == 0)
-   err = rq->engine->emit_bb_start(rq, addr, 0, 0);
-
-   if (err == 0)
-   i915_request_get(rq);
-   i915_request_add(rq);
-
-   return err ? ERR_PTR(err) : rq;
-}
-
-static u32 *spinner(u32 *batch, int i)
-{
-   return batch + i * 64 / sizeof(*batch) + 4;
-}
-
-static void end_spin(u32 *batch, int i)
-{
-   *spinner(batch, i) = MI_BATCH_BUFFER_END;
-   wmb();
-}
-
-static int igt_cs_tlb(void *arg)
-{
-   const unsigned int count = PAGE_SIZE / 64;
-   const unsigned int chunk_size = count * PAGE_SIZE;
-   struct drm_i915_private *i915 = arg;
-   struct drm_i915_gem_object *bbe, *act, *out;
-   struct i915_gem_engines_iter it;
-   struct i915_address_space *vm;
-   struct i915_gem_context *ctx;
-   struct intel_context *ce;
-   struct i915_vma *vma;
-   I915_RND_STATE(prng);
-   struct file *file;
-   unsigned int i;
-   u32 *result;
-   u32 *batch;
-   int err = 0;
-
-   /*
-* Our mission here is to fool the hardware to execute something
-* from scratch as it has not seen the batch move (due to missing
-* the TLB invalidate).
-*/
-
-   file = mock_file(i915);
-   if (IS_ERR(file))
-   return PTR_ERR(file);
-
-   ctx = live_context(i915, file);
-   if (IS_ERR(ctx)) {
-   err = PTR_ERR(ctx);
-   goto out_unlock;
-   }
-
-   vm = i915_gem_context_get_eb_vm(ctx);
-   if (i915_is_ggtt(vm))
-   goto out_vm;
-
-   /* Create two pages; dummy we prefill the TLB, and intended */
-   bbe = i915_gem_object_create_internal(i915, PAGE_SIZE);
-   if (IS_ERR(bbe)) {
-   err = PTR_ERR(bbe);
-   goto out_vm;
-   }
-
-   batch = i915_gem_object_pin_map_unlocked(bbe, I915_MAP_WC);
-   if (IS_ERR(batch)) {
-   err = PTR_ERR(batch);
-   goto out_put_bbe;
-   }
-   memset32(batch, MI_BATCH_BUFFER_END, PAGE_SIZE / sizeof(u32));
-   i915_gem_object_flush_map(bbe);
-   i915_gem_object_unpin_map(bbe);
-
-   act = i915_gem_object_create_internal(i915, PAGE_SIZE);
-   if (IS_ERR(act)) {
-   err = PTR_ERR(act);
-   goto out_put_bbe;
-   }
-
-   /* Track the execution of each request by writing into different slot */
-   batch = i915_gem_object_pin_map_unlocked(act, I915_MAP_WC);
-   if (IS_ERR(batch)) {
-   err = PTR_ERR(batch);
-   goto out_put_act;
-   }
-   for (i = 0; i < count; i++) {
-   u32 *cs = batch + i * 64 / sizeof(*cs);
-   u64 addr = (vm->total - PAGE_SIZE) + i * sizeof(u32);
-
-   GEM_BUG_ON(GRAPHICS_VER(i915) < 6);
-   cs[0] = MI_STORE_DWORD_IMM_GEN4;
-   if (GRAPHICS_VER(i915) >= 8) {
-   cs[1] = lower_32_bits(addr);
-   cs[2] = upper_32_bits(addr);
-   cs[3] = i;
-   cs[4] = MI_NOOP;
-   cs[5] = MI_BATCH_BUFFER_START_GEN8;
-   } else {
-   cs[1] = 0;
-   cs[2] = lower_32_bits(addr);
-   cs[3] = i;
- 

Re: [Intel-gfx] [PATCH] drm/i915/selftests: Drop igt_cs_tlb

2023-02-17 Thread Chris Wilson
Quoting Jonathan Cavitt (2023-02-17 17:20:19)
> The gt_tlb live selftest has the same code coverage as the
> igt_cs_tlb subtest of gtt.

True, the intent of the code is the same, but gt_tlb has had a much high
success rate at hitting TLB bugs, so is my preferred test.

> However, igt_cs_tlb is hitting
> an issue in that we are updating PTE in gt0->vm but
> executing on the media tile without updating gt1->vm.

I'm no longer convinced this a good explanation of the issue, as unlike
the i915_requests selftest this is using a GEM context and not the local
kernel contexts. The GEM context->vm should work equally on the mtl
render and media tiles, afaict.

The failure is very early in running on media tile (after running on the
render tile) so I think it should be easy enough to reproduce in a
simpler test to narrow down the cause.

> This issue is corrected in gt_tlb, and thus igt_cs_tlb
> is obsolete and should be removed.

gt_tlb supersedes igt_cs_tlb, that I can agree on,
Acked-by: Chris Wilson 
-Chris


[Intel-gfx] [PATCH] drm/i915/selftests: Drop igt_cs_tlb

2023-02-17 Thread Jonathan Cavitt
The gt_tlb live selftest has the same code coverage as the
igt_cs_tlb subtest of gtt.  However, igt_cs_tlb is hitting
an issue in that we are updating PTE in gt0->vm but
executing on the media tile without updating gt1->vm.
This issue is corrected in gt_tlb, and thus igt_cs_tlb
is obsolete and should be removed.

Signed-off-by: Jonathan Cavitt 
---
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 356 --
 1 file changed, 356 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index 01e75160a84a..5361ce70d3f2 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1940,361 +1940,6 @@ int i915_gem_gtt_mock_selftests(void)
return err;
 }
 
-static int context_sync(struct intel_context *ce)
-{
-   struct i915_request *rq;
-   long timeout;
-
-   rq = intel_context_create_request(ce);
-   if (IS_ERR(rq))
-   return PTR_ERR(rq);
-
-   i915_request_get(rq);
-   i915_request_add(rq);
-
-   timeout = i915_request_wait(rq, 0, HZ / 5);
-   i915_request_put(rq);
-
-   return timeout < 0 ? -EIO : 0;
-}
-
-static struct i915_request *
-submit_batch(struct intel_context *ce, u64 addr)
-{
-   struct i915_request *rq;
-   int err;
-
-   rq = intel_context_create_request(ce);
-   if (IS_ERR(rq))
-   return rq;
-
-   err = 0;
-   if (rq->engine->emit_init_breadcrumb) /* detect a hang */
-   err = rq->engine->emit_init_breadcrumb(rq);
-   if (err == 0)
-   err = rq->engine->emit_bb_start(rq, addr, 0, 0);
-
-   if (err == 0)
-   i915_request_get(rq);
-   i915_request_add(rq);
-
-   return err ? ERR_PTR(err) : rq;
-}
-
-static u32 *spinner(u32 *batch, int i)
-{
-   return batch + i * 64 / sizeof(*batch) + 4;
-}
-
-static void end_spin(u32 *batch, int i)
-{
-   *spinner(batch, i) = MI_BATCH_BUFFER_END;
-   wmb();
-}
-
-static int igt_cs_tlb(void *arg)
-{
-   const unsigned int count = PAGE_SIZE / 64;
-   const unsigned int chunk_size = count * PAGE_SIZE;
-   struct drm_i915_private *i915 = arg;
-   struct drm_i915_gem_object *bbe, *act, *out;
-   struct i915_gem_engines_iter it;
-   struct i915_address_space *vm;
-   struct i915_gem_context *ctx;
-   struct intel_context *ce;
-   struct i915_vma *vma;
-   I915_RND_STATE(prng);
-   struct file *file;
-   unsigned int i;
-   u32 *result;
-   u32 *batch;
-   int err = 0;
-
-   /*
-* Our mission here is to fool the hardware to execute something
-* from scratch as it has not seen the batch move (due to missing
-* the TLB invalidate).
-*/
-
-   file = mock_file(i915);
-   if (IS_ERR(file))
-   return PTR_ERR(file);
-
-   ctx = live_context(i915, file);
-   if (IS_ERR(ctx)) {
-   err = PTR_ERR(ctx);
-   goto out_unlock;
-   }
-
-   vm = i915_gem_context_get_eb_vm(ctx);
-   if (i915_is_ggtt(vm))
-   goto out_vm;
-
-   /* Create two pages; dummy we prefill the TLB, and intended */
-   bbe = i915_gem_object_create_internal(i915, PAGE_SIZE);
-   if (IS_ERR(bbe)) {
-   err = PTR_ERR(bbe);
-   goto out_vm;
-   }
-
-   batch = i915_gem_object_pin_map_unlocked(bbe, I915_MAP_WC);
-   if (IS_ERR(batch)) {
-   err = PTR_ERR(batch);
-   goto out_put_bbe;
-   }
-   memset32(batch, MI_BATCH_BUFFER_END, PAGE_SIZE / sizeof(u32));
-   i915_gem_object_flush_map(bbe);
-   i915_gem_object_unpin_map(bbe);
-
-   act = i915_gem_object_create_internal(i915, PAGE_SIZE);
-   if (IS_ERR(act)) {
-   err = PTR_ERR(act);
-   goto out_put_bbe;
-   }
-
-   /* Track the execution of each request by writing into different slot */
-   batch = i915_gem_object_pin_map_unlocked(act, I915_MAP_WC);
-   if (IS_ERR(batch)) {
-   err = PTR_ERR(batch);
-   goto out_put_act;
-   }
-   for (i = 0; i < count; i++) {
-   u32 *cs = batch + i * 64 / sizeof(*cs);
-   u64 addr = (vm->total - PAGE_SIZE) + i * sizeof(u32);
-
-   GEM_BUG_ON(GRAPHICS_VER(i915) < 6);
-   cs[0] = MI_STORE_DWORD_IMM_GEN4;
-   if (GRAPHICS_VER(i915) >= 8) {
-   cs[1] = lower_32_bits(addr);
-   cs[2] = upper_32_bits(addr);
-   cs[3] = i;
-   cs[4] = MI_NOOP;
-   cs[5] = MI_BATCH_BUFFER_START_GEN8;
-   } else {
-   cs[1] = 0;
-   cs[2] = lower_32_bits(addr);
-   cs[3] = i;
-   cs[4] = MI_NOOP;
-   cs[5] = MI_BATCH_BUFFER_START;
-   }
-   }
-
-