| Message ID | 20180706171537.32307-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 06, 2018 at 06:15:37PM +0100, Chris Wilson wrote: > Older machines do not have the 128-byte tile width format for > I915_TILING_Y and so we must adapt our reference swizzle. > > Testcase: igt/drv_selftest/live_objects #gdg The change below itself makes sense to me, but I'm trying to understand where this came from.... Looking to https://intel-gfx-ci.01.org/tree/drm-tip/igt@drv_selftest@live_objects.html is this related to issues on fi-gdg-551? Or is this related to that APL bugzilla entry? > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/selftests/i915_gem_object.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c > index 6fe71865b710..8a35d2f70671 100644 > --- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c > @@ -171,7 +171,7 @@ static u64 tiled_offset(const struct tile *tile, u64 v) > v += x; > } else { > const unsigned int ytile_span = 16; could we also figure this value from somewhere else instead of leaving it hardcoded for all platforms here? > - const unsigned int ytile_height = 32 * ytile_span; > + const unsigned int ytile_height = tile->height * ytile_span; > > v += y * ytile_span; > v += div64_u64_rem(x, ytile_span, &x) * ytile_height; > -- > 2.18.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Rodrigo Vivi (2018-07-06 21:27:52) > On Fri, Jul 06, 2018 at 06:15:37PM +0100, Chris Wilson wrote: > > Older machines do not have the 128-byte tile width format for > > I915_TILING_Y and so we must adapt our reference swizzle. > > > > Testcase: igt/drv_selftest/live_objects #gdg > > The change below itself makes sense to me, but I'm trying to understand > where this came from.... The result doesn't look right, so scrap it. > Looking to https://intel-gfx-ci.01.org/tree/drm-tip/igt@drv_selftest@live_objects.html > is this related to issues on fi-gdg-551? Would only apply to gdg in the farm. > Or is this related to that APL bugzilla entry? Which? > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/selftests/i915_gem_object.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c > > index 6fe71865b710..8a35d2f70671 100644 > > --- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c > > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c > > @@ -171,7 +171,7 @@ static u64 tiled_offset(const struct tile *tile, u64 v) > > v += x; > > } else { > > const unsigned int ytile_span = 16; > > could we also figure this value from somewhere else instead of > leaving it hardcoded for all platforms here? The only place where manual detiling is used inside the kernel. And if we were, it would be a lot of specialised code, where obfuscation of magic macros is unlikely to help (careful handling of cachelines being at the forefront). Interesting question as to whether we do provide a bounce buffer mmap to replace GTT mmap? Just say no. -Chris
On Fri, Jul 06, 2018 at 09:39:14PM +0100, Chris Wilson wrote: > Quoting Rodrigo Vivi (2018-07-06 21:27:52) > > On Fri, Jul 06, 2018 at 06:15:37PM +0100, Chris Wilson wrote: > > > Older machines do not have the 128-byte tile width format for > > > I915_TILING_Y and so we must adapt our reference swizzle. > > > > > > Testcase: igt/drv_selftest/live_objects #gdg > > > > The change below itself makes sense to me, but I'm trying to understand > > where this came from.... > > The result doesn't look right, so scrap it. > > > Looking to https://intel-gfx-ci.01.org/tree/drm-tip/igt@drv_selftest@live_objects.html > > is this related to issues on fi-gdg-551? > > Would only apply to gdg in the farm. > > > Or is this related to that APL bugzilla entry? that flash of a moment when I wondered you could be actually targeting this: https://bugs.freedesktop.org/show_bug.cgi?id=107113 > > Which? > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > --- > > > drivers/gpu/drm/i915/selftests/i915_gem_object.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c > > > index 6fe71865b710..8a35d2f70671 100644 > > > --- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c > > > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c > > > @@ -171,7 +171,7 @@ static u64 tiled_offset(const struct tile *tile, u64 v) > > > v += x; > > > } else { > > > const unsigned int ytile_span = 16; > > > > could we also figure this value from somewhere else instead of > > leaving it hardcoded for all platforms here? > > The only place where manual detiling is used inside the kernel. And if > we were, it would be a lot of specialised code, where obfuscation of > magic macros is unlikely to help (careful handling of cachelines being > at the forefront). Interesting question as to whether we do provide a > bounce buffer mmap to replace GTT mmap? Just say no. > -Chris
Quoting Rodrigo Vivi (2018-07-06 21:55:37) > On Fri, Jul 06, 2018 at 09:39:14PM +0100, Chris Wilson wrote: > > Quoting Rodrigo Vivi (2018-07-06 21:27:52) > > > On Fri, Jul 06, 2018 at 06:15:37PM +0100, Chris Wilson wrote: > > > > Older machines do not have the 128-byte tile width format for > > > > I915_TILING_Y and so we must adapt our reference swizzle. > > > > > > > > Testcase: igt/drv_selftest/live_objects #gdg > > > > > > The change below itself makes sense to me, but I'm trying to understand > > > where this came from.... > > > > The result doesn't look right, so scrap it. > > > > > Looking to https://intel-gfx-ci.01.org/tree/drm-tip/igt@drv_selftest@live_objects.html > > > is this related to issues on fi-gdg-551? > > > > Would only apply to gdg in the farm. > > > > > Or is this related to that APL bugzilla entry? > > that flash of a moment when I wondered you could be actually targeting this: > https://bugs.freedesktop.org/show_bug.cgi?id=107113 Ah, totally unrelated problem :) Fwiw, I can reproduce this on my i915gm so hopefully should be able to resolve it without spamming the lists. -Chris
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_object.c b/drivers/gpu/drm/i915/selftests/i915_gem_object.c index 6fe71865b710..8a35d2f70671 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_object.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_object.c @@ -171,7 +171,7 @@ static u64 tiled_offset(const struct tile *tile, u64 v) v += x; } else { const unsigned int ytile_span = 16; - const unsigned int ytile_height = 32 * ytile_span; + const unsigned int ytile_height = tile->height * ytile_span; v += y * ytile_span; v += div64_u64_rem(x, ytile_span, &x) * ytile_height;
Older machines do not have the 128-byte tile width format for I915_TILING_Y and so we must adapt our reference swizzle. Testcase: igt/drv_selftest/live_objects #gdg Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/selftests/i915_gem_object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)