[igt-dev] [PATCH i-g-t v5 4/4] lib: Adjust refactored gpu_fill library to our coding style
Katarzyna Dec
katarzyna.dec at intel.com
Tue Apr 10 13:42:48 UTC 2018
On Tue, Apr 10, 2018 at 01:34:45PM +0200, Katarzyna Dec wrote:
> While I am making changes in gpgpu and media fill area let's
> adjust code to our coding style.
>
> v2: rebased on series new version (patch is now last from
> series so change seems larger)
> v3: rebased
>
> Signed-off-by: Katarzyna Dec <katarzyna.dec at intel.com>
> Cc: Lukasz Kalamarz <lukasz.kalamarz at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> ---
> lib/gpgpu_fill.c | 24 ++++-----
> lib/gpgpu_fill.h | 12 ++---
> lib/gpu_fill.c | 142 +++++++++++++++++++++++---------------------------
> lib/media_fill.h | 20 +++----
> lib/media_fill_gen7.c | 7 ++-
> lib/media_fill_gen8.c | 7 +--
> lib/media_fill_gen9.c | 7 +--
> 7 files changed, 105 insertions(+), 114 deletions(-)
>
> diff --git a/lib/gpgpu_fill.c b/lib/gpgpu_fill.c
> index 5a77ebd4..72a1445a 100644
> --- a/lib/gpgpu_fill.c
> +++ b/lib/gpgpu_fill.c
> @@ -99,8 +99,8 @@ static const uint32_t gen9_gpgpu_kernel[][4] = {
> void
> gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> uint8_t color)
> {
> uint32_t curbe_buffer, interface_descriptor;
> @@ -120,8 +120,8 @@ gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
>
> interface_descriptor = gen7_fill_interface_descriptor(batch, dst,
> - gen7_gpgpu_kernel,
> - sizeof(gen7_gpgpu_kernel));
> + gen7_gpgpu_kernel, sizeof(gen7_gpgpu_kernel));
> +
> igt_assert(batch->ptr < &batch->buffer[4095]);
>
> batch->ptr = batch->buffer;
> @@ -147,8 +147,8 @@ gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> void
> gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> uint8_t color)
> {
> uint32_t curbe_buffer, interface_descriptor;
> @@ -168,8 +168,8 @@ gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
>
> interface_descriptor = gen8_fill_interface_descriptor(batch, dst,
> - gen8_gpgpu_kernel,
> - sizeof(gen8_gpgpu_kernel));
> + gen8_gpgpu_kernel, sizeof(gen8_gpgpu_kernel));
> +
> igt_assert(batch->ptr < &batch->buffer[4095]);
>
> batch->ptr = batch->buffer;
> @@ -195,8 +195,8 @@ gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> void
> gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> uint8_t color)
> {
> uint32_t curbe_buffer, interface_descriptor;
> @@ -216,8 +216,8 @@ gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
>
> interface_descriptor = gen8_fill_interface_descriptor(batch, dst,
> - gen9_gpgpu_kernel,
> - sizeof(gen9_gpgpu_kernel));
> + gen9_gpgpu_kernel, sizeof(gen9_gpgpu_kernel));
> +
> igt_assert(batch->ptr < &batch->buffer[4095]);
>
> batch->ptr = batch->buffer;
> diff --git a/lib/gpgpu_fill.h b/lib/gpgpu_fill.h
> index 7b5c8322..f0d188ae 100644
> --- a/lib/gpgpu_fill.h
> +++ b/lib/gpgpu_fill.h
> @@ -30,22 +30,22 @@
> void
> gen7_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> uint8_t color);
>
> void
> gen8_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> uint8_t color);
>
> void
> gen9_gpgpu_fillfunc(struct intel_batchbuffer *batch,
> struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> uint8_t color);
>
> #endif /* GPGPU_FILL_H */
> diff --git a/lib/gpu_fill.c b/lib/gpu_fill.c
> index 102f141b..fc28a945 100644
> --- a/lib/gpu_fill.c
> +++ b/lib/gpu_fill.c
> @@ -10,6 +10,7 @@ uint32_t
> batch_align(struct intel_batchbuffer *batch, uint32_t align)
> {
> uint32_t offset = batch_used(batch);
> +
> offset = ALIGN(offset, align);
> batch->ptr = batch->buffer + offset;
> return offset;
> @@ -19,6 +20,7 @@ void *
> batch_alloc(struct intel_batchbuffer *batch, uint32_t size, uint32_t align)
> {
> uint32_t offset = batch_align(batch, align);
> +
> batch->ptr += size;
> return memset(batch->buffer + offset, 0, size);
> }
> @@ -30,9 +32,11 @@ batch_offset(struct intel_batchbuffer *batch, void *ptr)
> }
>
> uint32_t
> -batch_copy(struct intel_batchbuffer *batch, const void *ptr, uint32_t size, uint32_t align)
> +batch_copy(struct intel_batchbuffer *batch, const void *ptr, uint32_t size,
> + uint32_t align)
> {
> - return batch_offset(batch, memcpy(batch_alloc(batch, size, align), ptr, size));
> + return batch_offset(batch, memcpy(batch_alloc(batch, size, align),
> + ptr, size));
> }
>
> void
> @@ -43,13 +47,13 @@ gen7_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end)
> ret = drm_intel_bo_subdata(batch->bo, 0, 4096, batch->buffer);
> if (ret == 0)
> ret = drm_intel_bo_mrb_exec(batch->bo, batch_end,
> - NULL, 0, 0, 0);
> + NULL, 0, 0, 0);
> igt_assert(ret == 0);
> }
>
> uint32_t
> gen7_fill_curbe_buffer_data(struct intel_batchbuffer *batch,
> - uint8_t color)
> + uint8_t color)
> {
> uint8_t *curbe_buffer;
> uint32_t offset;
> @@ -62,10 +66,8 @@ gen7_fill_curbe_buffer_data(struct intel_batchbuffer *batch,
> }
>
> uint32_t
> -gen7_fill_surface_state(struct intel_batchbuffer *batch,
> - struct igt_buf *buf,
> - uint32_t format,
> - int is_dst)
> +gen7_fill_surface_state(struct intel_batchbuffer *batch, struct igt_buf *buf,
> + uint32_t format, int is_dst)
> {
> struct gen7_surface_state *ss;
> uint32_t write_domain, read_domain, offset;
> @@ -111,8 +113,7 @@ gen7_fill_surface_state(struct intel_batchbuffer *batch,
> }
>
> uint32_t
> -gen7_fill_binding_table(struct intel_batchbuffer *batch,
> - struct igt_buf *dst)
> +gen7_fill_binding_table(struct intel_batchbuffer *batch, struct igt_buf *dst)
> {
> uint32_t *binding_table, offset;
>
> @@ -129,9 +130,8 @@ gen7_fill_binding_table(struct intel_batchbuffer *batch,
> }
>
> uint32_t
> -gen7_fill_kernel(struct intel_batchbuffer *batch,
> - const uint32_t kernel[][4],
> - size_t size)
> +gen7_fill_kernel(struct intel_batchbuffer *batch, const uint32_t kernel[][4],
> + size_t size)
> {
> uint32_t offset;
>
> @@ -141,8 +141,9 @@ gen7_fill_kernel(struct intel_batchbuffer *batch,
> }
>
> uint32_t
> -gen7_fill_interface_descriptor(struct intel_batchbuffer *batch, struct igt_buf *dst,
> - const uint32_t kernel[][4], size_t size)
> +gen7_fill_interface_descriptor(struct intel_batchbuffer *batch,
> + struct igt_buf *dst, const uint32_t kernel[][4],
> + size_t size)
> {
> struct gen7_interface_descriptor_data *idd;
> uint32_t offset;
> @@ -180,16 +181,19 @@ gen7_emit_state_base_address(struct intel_batchbuffer *batch)
> OUT_BATCH(0);
>
> /* surface */
> - OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> + OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> + BASE_ADDRESS_MODIFY);
>
> /* dynamic */
> - OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> + OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> + BASE_ADDRESS_MODIFY);
>
> /* indirect */
> OUT_BATCH(0);
>
> /* instruction */
> - OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
> + OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> + BASE_ADDRESS_MODIFY);
>
> /* general/dynamic/indirect/instruction access Bound */
> OUT_BATCH(0);
> @@ -214,7 +218,7 @@ gen7_emit_vfe_state(struct intel_batchbuffer *batch)
>
> /* urb entry size & curbe size */
> OUT_BATCH(2 << 16 | /* in 256 bits unit */
> - 2); /* in 256 bits unit */
> + 2); /* in 256 bits unit */
>
> /* scoreboard */
> OUT_BATCH(0);
> @@ -268,14 +272,16 @@ gen7_emit_interface_descriptor_load(struct intel_batchbuffer *batch, uint32_t in
> OUT_BATCH(sizeof(struct gen7_interface_descriptor_data));
> else
> OUT_BATCH(sizeof(struct gen8_interface_descriptor_data));
> - /* interface descriptor address, is relative to the dynamics base address */
> + /* interface descriptor address, is relative to the dynamics base
> + * address
> + */
> OUT_BATCH(interface_descriptor);
> }
>
> void
> gen7_emit_media_objects(struct intel_batchbuffer *batch,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height)
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height)
> {
> int i, j;
>
> @@ -297,7 +303,8 @@ gen7_emit_media_objects(struct intel_batchbuffer *batch,
> /* inline data (xoffset, yoffset) */
> OUT_BATCH(x + i * 16);
> OUT_BATCH(y + j * 16);
> - if (AT_LEAST_GEN(batch->devid, 8) && !IS_CHERRYVIEW(batch->devid))
> + if (AT_LEAST_GEN(batch->devid, 8) &&
> + !IS_CHERRYVIEW(batch->devid))
> gen8_emit_media_state_flush(batch);
> }
> }
> @@ -305,8 +312,8 @@ gen7_emit_media_objects(struct intel_batchbuffer *batch,
>
> void
> gen7_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height)
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height)
> {
> uint32_t x_dim, y_dim, tmp, right_mask;
>
> @@ -459,8 +466,8 @@ gen8_emit_state_base_address(struct intel_batchbuffer *batch)
> OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
>
> /* dynamic */
> - OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION,
> - 0, BASE_ADDRESS_MODIFY);
> + OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER |
> + I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
>
> /* indirect */
> OUT_BATCH(0);
> @@ -475,7 +482,9 @@ gen8_emit_state_base_address(struct intel_batchbuffer *batch)
> OUT_BATCH(1 << 12 | 1);
> /* indirect object buffer size */
> OUT_BATCH(0xfffff000 | 1);
> - /* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
> + /* instruction buffer size, must set modify enable bit, otherwise it
> + * may result in GPU hang
> + */
> OUT_BATCH(1 << 12 | 1);
> }
>
> @@ -536,62 +545,41 @@ gen8_emit_vfe_state_gpgpu(struct intel_batchbuffer *batch)
>
> void
> gen8_emit_gpgpu_walk(struct intel_batchbuffer *batch,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height)
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height)
> {
> - uint32_t x_dim, y_dim, tmp, right_mask;
> -
It looks like something went wrong with rebase in this function (after changes in
patch1), nothing should be deleted from this function. For now gpgpu_fill is not
working because of this... Fix in progress.
> - /*
> - * Simply do SIMD16 based dispatch, so every thread uses
> - * SIMD16 channels.
> - *
> - * Define our own thread group size, e.g 16x1 for every group, then
> - * will have 1 thread each group in SIMD16 dispatch. So thread
> - * width/height/depth are all 1.
> - *
> - * Then thread group X = width / 16 (aligned to 16)
> - * thread group Y = height;
> - */
> - x_dim = (width + 15) / 16;
> - y_dim = height;
> -
> - tmp = width & 15;
> - if (tmp == 0)
> - right_mask = (1 << 16) - 1;
> - else
> - right_mask = (1 << tmp) - 1;
> -
> - OUT_BATCH(GEN7_GPGPU_WALKER | 13);
> + /* general */
> + OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
> + OUT_BATCH(0);
>
> - OUT_BATCH(0); /* kernel offset */
> - OUT_BATCH(0); /* indirect data length */
> - OUT_BATCH(0); /* indirect data offset */
> + /* stateless data port */
> + OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
>
> - /* SIMD size, thread w/h/d */
> - OUT_BATCH(1 << 30 | /* SIMD16 */
> - 0 << 16 | /* depth:1 */
> - 0 << 8 | /* height:1 */
> - 0); /* width:1 */
> + /* surface */
> + OUT_RELOC(batch->bo, I915_GEM_DOMAIN_SAMPLER, 0, BASE_ADDRESS_MODIFY);
>
> - /* thread group X */
> - OUT_BATCH(0);
> - OUT_BATCH(0);
> - OUT_BATCH(x_dim);
> + /* dynamic */
> + OUT_RELOC(batch->bo, I915_GEM_DOMAIN_RENDER |
> + I915_GEM_DOMAIN_INSTRUCTION, 0, BASE_ADDRESS_MODIFY);
>
> - /* thread group Y */
> - OUT_BATCH(0);
> + /* indirect */
> OUT_BATCH(0);
> - OUT_BATCH(y_dim);
> -
> - /* thread group Z */
> OUT_BATCH(0);
> - OUT_BATCH(1);
>
> - /* right mask */
> - OUT_BATCH(right_mask);
> + /* instruction */
> + OUT_RELOC(batch->bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> + BASE_ADDRESS_MODIFY);
>
> - /* bottom mask, height 1, always 0xffffffff */
> - OUT_BATCH(0xffffffff);
> + /* general state buffer size */
> + OUT_BATCH(0xfffff000 | 1);
> + /* dynamic state buffer size */
> + OUT_BATCH(1 << 12 | 1);
> + /* indirect object buffer size */
> + OUT_BATCH(0xfffff000 | 1);
> + /* instruction buffer size, must set modify enable bit, otherwise it
> + * may result in GPU hang
> + */
> + OUT_BATCH(1 << 12 | 1);
> }
>
> void
> @@ -626,7 +614,9 @@ gen9_emit_state_base_address(struct intel_batchbuffer *batch)
> OUT_BATCH(1 << 12 | 1);
> /* indirect object buffer size */
> OUT_BATCH(0xfffff000 | 1);
> - /* intruction buffer size, must set modify enable bit, otherwise it may result in GPU hang */
> + /* intruction buffer size, must set modify enable bit, otherwise it may
> + * result in GPU hang
> + */
> OUT_BATCH(1 << 12 | 1);
>
> /* Bindless surface state base address */
> diff --git a/lib/media_fill.h b/lib/media_fill.h
> index 161af8cf..f6db734e 100644
> --- a/lib/media_fill.h
> +++ b/lib/media_fill.h
> @@ -7,22 +7,22 @@
> void
> gen8_media_fillfunc(struct intel_batchbuffer *batch,
> struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> uint8_t color);
>
> void
> gen7_media_fillfunc(struct intel_batchbuffer *batch,
> - struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> - uint8_t color);
> + struct igt_buf *dst,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> + uint8_t color);
>
> void
> gen9_media_fillfunc(struct intel_batchbuffer *batch,
> - struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> - uint8_t color);
> + struct igt_buf *dst,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> + uint8_t color);
>
> #endif /* RENDE_MEDIA_FILL_H */
> diff --git a/lib/media_fill_gen7.c b/lib/media_fill_gen7.c
> index c97555a6..5a8c32fb 100644
> --- a/lib/media_fill_gen7.c
> +++ b/lib/media_fill_gen7.c
> @@ -47,8 +47,8 @@ static const uint32_t media_kernel[][4] = {
> void
> gen7_media_fillfunc(struct intel_batchbuffer *batch,
> struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> uint8_t color)
> {
> uint32_t curbe_buffer, interface_descriptor;
> @@ -61,8 +61,7 @@ gen7_media_fillfunc(struct intel_batchbuffer *batch,
>
> curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
> interface_descriptor = gen7_fill_interface_descriptor(batch, dst,
> - media_kernel,
> - sizeof(media_kernel));
> + media_kernel, sizeof(media_kernel));
> igt_assert(batch->ptr < &batch->buffer[4095]);
>
> /* media pipeline */
> diff --git a/lib/media_fill_gen8.c b/lib/media_fill_gen8.c
> index 362abd61..d6dd7410 100644
> --- a/lib/media_fill_gen8.c
> +++ b/lib/media_fill_gen8.c
> @@ -50,8 +50,8 @@ static const uint32_t media_kernel[][4] = {
> void
> gen8_media_fillfunc(struct intel_batchbuffer *batch,
> struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> uint8_t color)
> {
> uint32_t curbe_buffer, interface_descriptor;
> @@ -63,7 +63,8 @@ gen8_media_fillfunc(struct intel_batchbuffer *batch,
> batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>
> curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
> - interface_descriptor = gen8_fill_interface_descriptor(batch, dst, media_kernel, sizeof(media_kernel));
> + interface_descriptor = gen8_fill_interface_descriptor(batch, dst,
> + media_kernel, sizeof(media_kernel));
> igt_assert(batch->ptr < &batch->buffer[4095]);
>
> /* media pipeline */
> diff --git a/lib/media_fill_gen9.c b/lib/media_fill_gen9.c
> index d1335fe6..a9a829f2 100644
> --- a/lib/media_fill_gen9.c
> +++ b/lib/media_fill_gen9.c
> @@ -47,8 +47,8 @@ static const uint32_t media_kernel[][4] = {
> void
> gen9_media_fillfunc(struct intel_batchbuffer *batch,
> struct igt_buf *dst,
> - unsigned x, unsigned y,
> - unsigned width, unsigned height,
> + unsigned int x, unsigned int y,
> + unsigned int width, unsigned int height,
> uint8_t color)
> {
> uint32_t curbe_buffer, interface_descriptor;
> @@ -60,7 +60,8 @@ gen9_media_fillfunc(struct intel_batchbuffer *batch,
> batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
>
> curbe_buffer = gen7_fill_curbe_buffer_data(batch, color);
> - interface_descriptor = gen8_fill_interface_descriptor(batch, dst, media_kernel, sizeof(media_kernel));
> + interface_descriptor = gen8_fill_interface_descriptor(batch, dst,
> + media_kernel, sizeof(media_kernel));
> assert(batch->ptr < &batch->buffer[4095]);
>
> /* media pipeline */
> --
> 2.14.3
>
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
More information about the igt-dev
mailing list