| Message ID | 20180702090727.7721-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/07/2018 10:07, Chris Wilson wrote: > As we want to compare a templated tiling pattern against the target_bo, > we need to know that the swizzling is compatible. Or else the two > tiling pattern may differ due to underlying page address that we cannot > know, and so the test may sporadically fail. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=102575 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > tests/gem_tiled_partial_pwrite_pread.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/tests/gem_tiled_partial_pwrite_pread.c b/tests/gem_tiled_partial_pwrite_pread.c > index fe573c37c..83c57c07d 100644 > --- a/tests/gem_tiled_partial_pwrite_pread.c > +++ b/tests/gem_tiled_partial_pwrite_pread.c > @@ -249,6 +249,24 @@ static void test_partial_read_writes(void) > } > } > > +static bool known_swizzling(uint32_t handle) > +{ > + struct drm_i915_gem_get_tiling2 { > + uint32_t handle; > + uint32_t tiling_mode; > + uint32_t swizzle_mode; > + uint32_t phys_swizzle_mode; > + } arg = { > + .handle = handle, > + }; > +#define DRM_IOCTL_I915_GEM_GET_TILING2 DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct Can't we rely on this being in system headers by now? drm_i915_gem_get_tiling2) > + > + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg)) > + return false; > + > + return arg.phys_swizzle_mode == arg.swizzle_mode; > +} > + > igt_main > { > uint32_t tiling_mode = I915_TILING_X; > @@ -271,6 +289,12 @@ igt_main > &tiling_mode, &scratch_pitch, 0); > igt_assert(tiling_mode == I915_TILING_X); > igt_assert(scratch_pitch == 4096); > + > + /* > + * As we want to compare our template tiled pattern against > + * the target bo, we need consistent swizzling on both. > + */ > + igt_require(known_swizzling(scratch_bo->handle)); > staging_bo = drm_intel_bo_alloc(bufmgr, "staging bo", BO_SIZE, 4096); > tiled_staging_bo = drm_intel_bo_alloc_tiled(bufmgr, "scratch bo", 1024, > BO_SIZE/4096, 4, > Another option could be to keep allocating until we found one in the memory area with compatible swizzling? Like this it may be some noise in the test pass<->skip transitions. Regards, Tvrtko
Quoting Tvrtko Ursulin (2018-07-02 13:00:07) > > On 02/07/2018 10:07, Chris Wilson wrote: > > As we want to compare a templated tiling pattern against the target_bo, > > we need to know that the swizzling is compatible. Or else the two > > tiling pattern may differ due to underlying page address that we cannot > > know, and so the test may sporadically fail. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=102575 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > tests/gem_tiled_partial_pwrite_pread.c | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/tests/gem_tiled_partial_pwrite_pread.c b/tests/gem_tiled_partial_pwrite_pread.c > > index fe573c37c..83c57c07d 100644 > > --- a/tests/gem_tiled_partial_pwrite_pread.c > > +++ b/tests/gem_tiled_partial_pwrite_pread.c > > @@ -249,6 +249,24 @@ static void test_partial_read_writes(void) > > } > > } > > > > +static bool known_swizzling(uint32_t handle) > > +{ > > + struct drm_i915_gem_get_tiling2 { > > + uint32_t handle; > > + uint32_t tiling_mode; > > + uint32_t swizzle_mode; > > + uint32_t phys_swizzle_mode; > > + } arg = { > > + .handle = handle, > > + }; > > +#define DRM_IOCTL_I915_GEM_GET_TILING2 DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct > > Can't we rely on this being in system headers by now? > > drm_i915_gem_get_tiling2) > > + > > + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg)) > > + return false; > > + > > + return arg.phys_swizzle_mode == arg.swizzle_mode; > > +} > > + > > igt_main > > { > > uint32_t tiling_mode = I915_TILING_X; > > @@ -271,6 +289,12 @@ igt_main > > &tiling_mode, &scratch_pitch, 0); > > igt_assert(tiling_mode == I915_TILING_X); > > igt_assert(scratch_pitch == 4096); > > + > > + /* > > + * As we want to compare our template tiled pattern against > > + * the target bo, we need consistent swizzling on both. > > + */ > > + igt_require(known_swizzling(scratch_bo->handle)); > > staging_bo = drm_intel_bo_alloc(bufmgr, "staging bo", BO_SIZE, 4096); > > tiled_staging_bo = drm_intel_bo_alloc_tiled(bufmgr, "scratch bo", 1024, > > BO_SIZE/4096, 4, > > > > Another option could be to keep allocating until we found one in the > memory area with compatible swizzling? Like this it may be some noise in > the test pass<->skip transitions. It depends on physical layout which the kernel keeps hidden (for understandable reasons). -Chris
On 05/07/2018 12:14, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-07-02 13:00:07) >> >> On 02/07/2018 10:07, Chris Wilson wrote: >>> As we want to compare a templated tiling pattern against the target_bo, >>> we need to know that the swizzling is compatible. Or else the two >>> tiling pattern may differ due to underlying page address that we cannot >>> know, and so the test may sporadically fail. >>> >>> References: https://bugs.freedesktop.org/show_bug.cgi?id=102575 >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> --- >>> tests/gem_tiled_partial_pwrite_pread.c | 24 ++++++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> >>> diff --git a/tests/gem_tiled_partial_pwrite_pread.c b/tests/gem_tiled_partial_pwrite_pread.c >>> index fe573c37c..83c57c07d 100644 >>> --- a/tests/gem_tiled_partial_pwrite_pread.c >>> +++ b/tests/gem_tiled_partial_pwrite_pread.c >>> @@ -249,6 +249,24 @@ static void test_partial_read_writes(void) >>> } >>> } >>> >>> +static bool known_swizzling(uint32_t handle) >>> +{ >>> + struct drm_i915_gem_get_tiling2 { >>> + uint32_t handle; >>> + uint32_t tiling_mode; >>> + uint32_t swizzle_mode; >>> + uint32_t phys_swizzle_mode; >>> + } arg = { >>> + .handle = handle, >>> + }; >>> +#define DRM_IOCTL_I915_GEM_GET_TILING2 DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct >> >> Can't we rely on this being in system headers by now? >> >> drm_i915_gem_get_tiling2) >>> + >>> + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg)) >>> + return false; >>> + >>> + return arg.phys_swizzle_mode == arg.swizzle_mode; >>> +} >>> + >>> igt_main >>> { >>> uint32_t tiling_mode = I915_TILING_X; >>> @@ -271,6 +289,12 @@ igt_main >>> &tiling_mode, &scratch_pitch, 0); >>> igt_assert(tiling_mode == I915_TILING_X); >>> igt_assert(scratch_pitch == 4096); >>> + >>> + /* >>> + * As we want to compare our template tiled pattern against >>> + * the target bo, we need consistent swizzling on both. >>> + */ >>> + igt_require(known_swizzling(scratch_bo->handle)); >>> staging_bo = drm_intel_bo_alloc(bufmgr, "staging bo", BO_SIZE, 4096); >>> tiled_staging_bo = drm_intel_bo_alloc_tiled(bufmgr, "scratch bo", 1024, >>> BO_SIZE/4096, 4, >>> >> >> Another option could be to keep allocating until we found one in the >> memory area with compatible swizzling? Like this it may be some noise in >> the test pass<->skip transitions. > > It depends on physical layout which the kernel keeps hidden (for > understandable reasons). Yeah, but we could allocate more and more until we end up in the area where args.phys_swizzle_mode == args.swizzle_mode. Might be to heavy approach. But then this skip can be random depending on what physical memory gets allocated in each test run. Regards, Tvrtko
Quoting Tvrtko Ursulin (2018-07-05 13:30:57) > > On 05/07/2018 12:14, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-07-02 13:00:07) > >> > >> On 02/07/2018 10:07, Chris Wilson wrote: > >>> As we want to compare a templated tiling pattern against the target_bo, > >>> we need to know that the swizzling is compatible. Or else the two > >>> tiling pattern may differ due to underlying page address that we cannot > >>> know, and so the test may sporadically fail. > >>> > >>> References: https://bugs.freedesktop.org/show_bug.cgi?id=102575 > >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > >>> --- > >>> tests/gem_tiled_partial_pwrite_pread.c | 24 ++++++++++++++++++++++++ > >>> 1 file changed, 24 insertions(+) > >>> > >>> diff --git a/tests/gem_tiled_partial_pwrite_pread.c b/tests/gem_tiled_partial_pwrite_pread.c > >>> index fe573c37c..83c57c07d 100644 > >>> --- a/tests/gem_tiled_partial_pwrite_pread.c > >>> +++ b/tests/gem_tiled_partial_pwrite_pread.c > >>> @@ -249,6 +249,24 @@ static void test_partial_read_writes(void) > >>> } > >>> } > >>> > >>> +static bool known_swizzling(uint32_t handle) > >>> +{ > >>> + struct drm_i915_gem_get_tiling2 { > >>> + uint32_t handle; > >>> + uint32_t tiling_mode; > >>> + uint32_t swizzle_mode; > >>> + uint32_t phys_swizzle_mode; > >>> + } arg = { > >>> + .handle = handle, > >>> + }; > >>> +#define DRM_IOCTL_I915_GEM_GET_TILING2 DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct > >> > >> Can't we rely on this being in system headers by now? > >> > >> drm_i915_gem_get_tiling2) > >>> + > >>> + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg)) > >>> + return false; > >>> + > >>> + return arg.phys_swizzle_mode == arg.swizzle_mode; > >>> +} > >>> + > >>> igt_main > >>> { > >>> uint32_t tiling_mode = I915_TILING_X; > >>> @@ -271,6 +289,12 @@ igt_main > >>> &tiling_mode, &scratch_pitch, 0); > >>> igt_assert(tiling_mode == I915_TILING_X); > >>> igt_assert(scratch_pitch == 4096); > >>> + > >>> + /* > >>> + * As we want to compare our template tiled pattern against > >>> + * the target bo, we need consistent swizzling on both. > >>> + */ > >>> + igt_require(known_swizzling(scratch_bo->handle)); > >>> staging_bo = drm_intel_bo_alloc(bufmgr, "staging bo", BO_SIZE, 4096); > >>> tiled_staging_bo = drm_intel_bo_alloc_tiled(bufmgr, "scratch bo", 1024, > >>> BO_SIZE/4096, 4, > >>> > >> > >> Another option could be to keep allocating until we found one in the > >> memory area with compatible swizzling? Like this it may be some noise in > >> the test pass<->skip transitions. > > > > It depends on physical layout which the kernel keeps hidden (for > > understandable reasons). > > Yeah, but we could allocate more and more until we end up in the area > where args.phys_swizzle_mode == args.swizzle_mode. Might be to heavy > approach. But then this skip can be random depending on what physical > memory gets allocated in each test run. Ah, but phys_swizzle_mode and swizzle_mode are invariants determined during boot for each fence-tiling type. -Chris
On 05/07/2018 13:35, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-07-05 13:30:57) >> >> On 05/07/2018 12:14, Chris Wilson wrote: >>> Quoting Tvrtko Ursulin (2018-07-02 13:00:07) >>>> >>>> On 02/07/2018 10:07, Chris Wilson wrote: >>>>> As we want to compare a templated tiling pattern against the target_bo, >>>>> we need to know that the swizzling is compatible. Or else the two >>>>> tiling pattern may differ due to underlying page address that we cannot >>>>> know, and so the test may sporadically fail. >>>>> >>>>> References: https://bugs.freedesktop.org/show_bug.cgi?id=102575 >>>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>>>> --- >>>>> tests/gem_tiled_partial_pwrite_pread.c | 24 ++++++++++++++++++++++++ >>>>> 1 file changed, 24 insertions(+) >>>>> >>>>> diff --git a/tests/gem_tiled_partial_pwrite_pread.c b/tests/gem_tiled_partial_pwrite_pread.c >>>>> index fe573c37c..83c57c07d 100644 >>>>> --- a/tests/gem_tiled_partial_pwrite_pread.c >>>>> +++ b/tests/gem_tiled_partial_pwrite_pread.c >>>>> @@ -249,6 +249,24 @@ static void test_partial_read_writes(void) >>>>> } >>>>> } >>>>> >>>>> +static bool known_swizzling(uint32_t handle) >>>>> +{ >>>>> + struct drm_i915_gem_get_tiling2 { >>>>> + uint32_t handle; >>>>> + uint32_t tiling_mode; >>>>> + uint32_t swizzle_mode; >>>>> + uint32_t phys_swizzle_mode; >>>>> + } arg = { >>>>> + .handle = handle, >>>>> + }; >>>>> +#define DRM_IOCTL_I915_GEM_GET_TILING2 DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct >>>> >>>> Can't we rely on this being in system headers by now? >>>> >>>> drm_i915_gem_get_tiling2) >>>>> + >>>>> + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg)) >>>>> + return false; >>>>> + >>>>> + return arg.phys_swizzle_mode == arg.swizzle_mode; >>>>> +} >>>>> + >>>>> igt_main >>>>> { >>>>> uint32_t tiling_mode = I915_TILING_X; >>>>> @@ -271,6 +289,12 @@ igt_main >>>>> &tiling_mode, &scratch_pitch, 0); >>>>> igt_assert(tiling_mode == I915_TILING_X); >>>>> igt_assert(scratch_pitch == 4096); >>>>> + >>>>> + /* >>>>> + * As we want to compare our template tiled pattern against >>>>> + * the target bo, we need consistent swizzling on both. >>>>> + */ >>>>> + igt_require(known_swizzling(scratch_bo->handle)); >>>>> staging_bo = drm_intel_bo_alloc(bufmgr, "staging bo", BO_SIZE, 4096); >>>>> tiled_staging_bo = drm_intel_bo_alloc_tiled(bufmgr, "scratch bo", 1024, >>>>> BO_SIZE/4096, 4, >>>>> >>>> >>>> Another option could be to keep allocating until we found one in the >>>> memory area with compatible swizzling? Like this it may be some noise in >>>> the test pass<->skip transitions. >>> >>> It depends on physical layout which the kernel keeps hidden (for >>> understandable reasons). >> >> Yeah, but we could allocate more and more until we end up in the area >> where args.phys_swizzle_mode == args.swizzle_mode. Might be to heavy >> approach. But then this skip can be random depending on what physical >> memory gets allocated in each test run. > > Ah, but phys_swizzle_mode and swizzle_mode are invariants determined > during boot for each fence-tiling type. True, get_tiling ioctl has nothing on that level. I dont' know where was I. Had this ideas about multi-bank RAM with one bank single channel, another multi-channel and then swizzling differing between the two. Okay, then question remains if you just want to drop the local struct and ioctl definition since AFAICS the same definitions are in the kernel since 2012. Regards, Tvrtko
Quoting Tvrtko Ursulin (2018-07-05 16:26:51) > > Okay, then question remains if you just want to drop the local struct > and ioctl definition since AFAICS the same definitions are in the kernel > since 2012. I haven't commented on that as there's no argument here. I was lazy and copied existing code, and will always be as lazy as I can get away with. -Chris
diff --git a/tests/gem_tiled_partial_pwrite_pread.c b/tests/gem_tiled_partial_pwrite_pread.c index fe573c37c..83c57c07d 100644 --- a/tests/gem_tiled_partial_pwrite_pread.c +++ b/tests/gem_tiled_partial_pwrite_pread.c @@ -249,6 +249,24 @@ static void test_partial_read_writes(void) } } +static bool known_swizzling(uint32_t handle) +{ + struct drm_i915_gem_get_tiling2 { + uint32_t handle; + uint32_t tiling_mode; + uint32_t swizzle_mode; + uint32_t phys_swizzle_mode; + } arg = { + .handle = handle, + }; +#define DRM_IOCTL_I915_GEM_GET_TILING2 DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct drm_i915_gem_get_tiling2) + + if (igt_ioctl(fd, DRM_IOCTL_I915_GEM_GET_TILING2, &arg)) + return false; + + return arg.phys_swizzle_mode == arg.swizzle_mode; +} + igt_main { uint32_t tiling_mode = I915_TILING_X; @@ -271,6 +289,12 @@ igt_main &tiling_mode, &scratch_pitch, 0); igt_assert(tiling_mode == I915_TILING_X); igt_assert(scratch_pitch == 4096); + + /* + * As we want to compare our template tiled pattern against + * the target bo, we need consistent swizzling on both. + */ + igt_require(known_swizzling(scratch_bo->handle)); staging_bo = drm_intel_bo_alloc(bufmgr, "staging bo", BO_SIZE, 4096); tiled_staging_bo = drm_intel_bo_alloc_tiled(bufmgr, "scratch bo", 1024, BO_SIZE/4096, 4,
As we want to compare a templated tiling pattern against the target_bo, we need to know that the swizzling is compatible. Or else the two tiling pattern may differ due to underlying page address that we cannot know, and so the test may sporadically fail. References: https://bugs.freedesktop.org/show_bug.cgi?id=102575 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- tests/gem_tiled_partial_pwrite_pread.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)