Quoting Janusz Krzysztofik (2019-11-04 09:39:32) > Hi Chris, > > On Friday, November 1, 2019 10:42:18 AM CET Chris Wilson wrote: > > Quoting Janusz Krzysztofik (2019-10-31 15:28:57) > > > exec-shared-gtt-* subtests use hardcoded values for object size and > > > softpin offset, based on 4kB GTT alignment assumption. That may result > > > in those subtests failing when run on future backing stores with > > > possibly larger minimum page sizes. > > > > > > Replace hardcoded constants with values calculated from minimum GTT > > > alignment of actual backing store the test is running on. > > > > > > v2: Update helper name, use 'minimum GTT alignment' term across the > > > change, adjust variable name. > > > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx> > > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > --- > > > tests/i915/gem_ctx_shared.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/tests/i915/gem_ctx_shared.c b/tests/i915/gem_ctx_shared.c > > > index 6d8cbcce..1e9c7f78 100644 > > > --- a/tests/i915/gem_ctx_shared.c > > > +++ b/tests/i915/gem_ctx_shared.c > > > @@ -195,6 +195,7 @@ static void exec_shared_gtt(int i915, unsigned int > ring) > > > uint32_t scratch, *s; > > > uint32_t batch, cs[16]; > > > uint64_t offset; > > > + uint64_t alignment; > > > int i; > > > > > > gem_require_ring(i915, ring); > > > @@ -203,7 +204,8 @@ static void exec_shared_gtt(int i915, unsigned int > ring) > > > clone = gem_context_clone(i915, 0, I915_CONTEXT_CLONE_VM, 0); > > > > > > /* Find a hole big enough for both objects later */ > > > - scratch = gem_create(i915, 16384); > > > + alignment = 2 * gem_gtt_min_alignment(i915); > > alignment = one page for an object + one page for stride > > > > + scratch = gem_create(i915, 2 * alignment); > > space reserved = 2 * (one page for an object + one page for stride) > > > > gem_write(i915, scratch, 0, &bbe, sizeof(bbe)); > > > obj.handle = scratch; > > > gem_execbuf(i915, &execbuf); > > > @@ -246,7 +248,7 @@ static void exec_shared_gtt(int i915, unsigned int > ring) > > > gem_write(i915, batch, 0, cs, sizeof(cs)); > > > > > > obj.handle = batch; > > > - obj.offset += 8192; /* make sure we don't cause an eviction! */ > > > + obj.offset += alignment; /* make sure we don't cause an eviction! > */ > > offset += (one page for an object + one page for stride) > > > It's 'stride' here. It's leaving a guard page in between, just in case > > page coloring demands it. > > Assuming I correctly understood the intentions here but my implementation > is not readable clearly enough, how do you think this should be fixed? How > about a comment '/* one page for an object + one page for stride */' above the > line where the 'alignment' variable is assigned the value of 2 * minimum GTT > alignment? I think "stride = 2 * min_alignment()" concisely describes the intent. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
