[Intel-gfx] [igt-dev] [PATCH i-g-t 02/17] igt/gem_tiled_partial_pwrite_pread: Check for known swizzling

[Tvrtko Ursulin]

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 at 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