[Intel-gfx] [PATCH] tests/gem_gtt_hog: Fix for BDW

[Rodrigo Vivi]

On Fri, Feb 7, 2014 at 7:51 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Thu, Feb 06, 2014 at 07:46:38PM -0200, Rodrigo Vivi wrote:
>> Update XY_COLOR_BLT command for Broadwell.
>>
>> v2: stash devid and remove ugly double allocation. (by Chris).
>> v3: fix inverted blt command size and stash fd, devid and intel_gen.
>>
>> Cc: Chris Wilson chris at chris-wilson.co.uk>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
>> ---
>>  tests/gem_gtt_hog.c | 59 +++++++++++++++++++++++++++++++++++------------------
>>  1 file changed, 39 insertions(+), 20 deletions(-)
>>
>> diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
>> index 53cd7eb..3149ff4 100644
>> --- a/tests/gem_gtt_hog.c
>> +++ b/tests/gem_gtt_hog.c
>> @@ -44,30 +44,43 @@
>>
>>  static const uint32_t canary = 0xdeadbeef;
>>
>> +typedef struct data {
>> +     int fd;
>> +     int devid;
>> +     int intel_gen;
>> +} data_t;
>> +
>>  static double elapsed(const struct timeval *start,
>>                     const struct timeval *end)
>>  {
>>       return 1e6*(end->tv_sec - start->tv_sec) + (end->tv_usec - start->tv_usec);
>>  }
>>
>> -static void busy(int fd, uint32_t handle, int size, int loops)
>> +static void busy(data_t *data, uint32_t handle, int size, int loops)
>>  {
>>       struct drm_i915_gem_relocation_entry reloc[20];
>>       struct drm_i915_gem_exec_object2 gem_exec[2];
>>       struct drm_i915_gem_execbuffer2 execbuf;
>>       struct drm_i915_gem_pwrite gem_pwrite;
>>       struct drm_i915_gem_create create;
>> -     uint32_t buf[122], *b;
>> -     int i;
>> +     uint32_t buf[170], *b;
>> +     int i, b_size;
>>
>>       memset(reloc, 0, sizeof(reloc));
>>       memset(gem_exec, 0, sizeof(gem_exec));
>>       memset(&execbuf, 0, sizeof(execbuf));
>>
>>       b = buf;
>> +     b_size = sizeof(uint32_t) * (data->intel_gen >= 8 ? 170 : 122);
>>       for (i = 0; i < 20; i++) {
>> -             *b++ = XY_COLOR_BLT_CMD_NOLEN | 4 |
>> -                     COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
>> +             if (data->intel_gen >= 8) {
>> +                     *b++ = MI_NOOP;
>
> Not required, instead you just round up the buf to the next qword after
> ending the batch.

I'm afraid I didn't get here, but if possible I'd like to let it just
like other bdw patches are.

>
>> +                     *b++ = XY_COLOR_BLT_CMD_NOLEN | 5 |
>> +                             COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
>> +             } else {
>> +                     *b++ = XY_COLOR_BLT_CMD_NOLEN | 4 |
>> +                             COLOR_BLT_WRITE_ALPHA | XY_COLOR_BLT_WRITE_RGB;
>> +             }
>>               *b++ = 0xf0 << 16 | 1 << 25 | 1 << 24 | 4096;
>>               *b++ = 0;
>>               *b++ = size >> 12 << 16 | 1024;
>> @@ -76,6 +89,8 @@ static void busy(int fd, uint32_t handle, int size, int loops)
>>               reloc[i].read_domains = I915_GEM_DOMAIN_RENDER;
>>               reloc[i].write_domain = I915_GEM_DOMAIN_RENDER;
>>               *b++ = 0;
>> +             if (data->intel_gen >= 8)
>> +                     *b++ = 0;
>>               *b++ = canary;
>>       }
>>       *b++ = MI_BATCH_BUFFER_END;
> if ((b - buf) & 1))
>  *b++ = 0;

is this related to your above's suggestion?

>
>> @@ -86,56 +101,55 @@ static void busy(int fd, uint32_t handle, int size, int loops)
>>
>>       create.handle = 0;
>>       create.size = 4096;
>> -     drmIoctl(fd, DRM_IOCTL_I915_GEM_CREATE, &create);
>> +     drmIoctl(data->fd, DRM_IOCTL_I915_GEM_CREATE, &create);
>>       gem_exec[1].handle = create.handle;
>>       gem_exec[1].relocation_count = 20;
>>       gem_exec[1].relocs_ptr = (uintptr_t)reloc;
>>
>>       execbuf.buffers_ptr = (uintptr_t)gem_exec;
>>       execbuf.buffer_count = 2;
>> -     execbuf.batch_len = sizeof(buf);
>> +     execbuf.batch_len = b_size;
>
> I would have personally used (b - buf) * sizeof(buf[0]);

much better for sure. Already changed here and will send this on next version.

>
>>       execbuf.flags = 1 << 11;
>> -     if (HAS_BLT_RING(intel_get_drm_devid(fd)))
>> +     if (HAS_BLT_RING(data->devid))
>>               execbuf.flags |= I915_EXEC_BLT;
>>
>>       gem_pwrite.handle = gem_exec[1].handle;
>>       gem_pwrite.offset = 0;
>> -     gem_pwrite.size = sizeof(buf);
>> +     gem_pwrite.size = b_size;
> and gem_pwrite.size = execbuf.batch_len;
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre

Thanks

-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br