2015-03-31 10:07 GMT-03:00 Daniel Vetter <daniel@xxxxxxxx>:
> On Mon, Mar 30, 2015 at 04:45:49PM -0300, Paulo Zanoni wrote:
>> 2015-03-26 7:19 GMT-03:00 Daniel Vetter <daniel@xxxxxxxx>:
>> > On Wed, Mar 25, 2015 at 06:50:39PM -0300, Paulo Zanoni wrote:
>> >> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>> >>
>> >> For all those IGT tests that need an easy way to draw rectangles on
>> >> buffers using different methods. Current planned users: FBC and PSR
>> >> CRC tests.
>> >>
>> >> There is also a lib/tests/igt_draw program to check if the library is
>> >> sane.
>> >
>> > We need to move that to tests/igt_draw. The testcase in lib/tests/* get
>> > run with make check, which must be possible as non-root on a non-intel
>> > (build) machine. If you need an gpu to run your test it must be in tests/*
>> > as a normal igt kernel test.
>> >
>> > Wrt the library itself I'm unsure about the explicit tiling/swizzling.
>> > Your current code only works on gen5-8 and maintaining a full-blown
>> > swizzle/tiling library is real work, and means some of the tests can't be
>> > converted to this.
>>
>> This would just be a problem for the tests that use it, and no test is
>> using it yet. This is just for the cases where you want to use the CPU
>> to write into tiled buffers.
>>
>> If we start using this in a test, then we need to properly test all
>> the affected platforms. I have some local FBC tests that use it -
>> which I was going to submit after getting feedback on this lib -, and
>> I don't think we'll end needing to run these tests on the older
>> platforms.
>>
>>
>> > We do have all the tiling modes encoded in the tiling
>> > tests though, so if you have a lot of time it might be useful to extract
>> > tiling helpers into the igt library which work on all generations.
>>
>> Can you please be more precise here? Where exactly should I look?
>
> gem_tiled_pread has the full-blown tiling/swizzle logic for all platforms.
> This is the one test we have which should work everywhere.
>
> My concern is that by implementing a library which doesn't support
> everywhere we havea bit a split in the testbase which might surprise
> people. Imo a library function should work everywhere. If you look at the
> code in there compared to yours there's just 2 things missing:
> - Variable tile size/height.
> - Some of the more crazy swizzle modes.
>
> Also if we have a library which works everywhere we could extend it with
> the new fancy gen9+ tiling modes.
I'll take a look at gem_tiled_pread and try to implement what is
missing. I agree that supporting everything is the ideal, but I also
think that we can grow this support as needed instead of all at once.
>
>> > I think we should at least have a fallback mode which allows us to fill an
>> > entire buffer completely (i.e. not just the hxw area) and treat it as
>> > untiled.
>>
>> That is not the goal of the library. Still, if we ever need this
>> function, it would be easy to add.
>
> That was just meant as a fallback for buffers where you don't support the
> exact tiling/swizzling mode.
>>
>> >
>> > More comments below.
>>
>> More below too :)
>>
>> > -Daniel
>> >
>> >>
>> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>> >> ---
>> >> lib/Makefile.sources | 2 +
>> >> lib/igt_draw.c | 467 +++++++++++++++++++++++++++++++++++++++++++++
>> >> lib/igt_draw.h | 54 ++++++
>> >> lib/tests/.gitignore | 1 +
>> >> lib/tests/Makefile.sources | 1 +
>> >> lib/tests/igt_draw.c | 247 ++++++++++++++++++++++++
>> >> 6 files changed, 772 insertions(+)
>> >> create mode 100644 lib/igt_draw.c
>> >> create mode 100644 lib/igt_draw.h
>> >> create mode 100644 lib/tests/igt_draw.c
>> >>
>> >> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
>> >> index 3d93629..85dc321 100644
>> >> --- a/lib/Makefile.sources
>> >> +++ b/lib/Makefile.sources
>> >> @@ -52,6 +52,8 @@ libintel_tools_la_SOURCES = \
>> >> igt_fb.h \
>> >> igt_core.c \
>> >> igt_core.h \
>> >> + igt_draw.c \
>> >> + igt_draw.h \
>> >> $(NULL)
>> >>
>> >> .PHONY: version.h.tmp
>> >> diff --git a/lib/igt_draw.c b/lib/igt_draw.c
>> >> new file mode 100644
>> >> index 0000000..4eb7507
>> >> --- /dev/null
>> >> +++ b/lib/igt_draw.c
>> >> @@ -0,0 +1,467 @@
>> >> +/*
>> >> + * Copyright © 2015 Intel Corporation
>> >> + *
>> >> + * Permission is hereby granted, free of charge, to any person obtaining a
>> >> + * copy of this software and associated documentation files (the "Software"),
>> >> + * to deal in the Software without restriction, including without limitation
>> >> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> >> + * and/or sell copies of the Software, and to permit persons to whom the
>> >> + * Software is furnished to do so, subject to the following conditions:
>> >> + *
>> >> + * The above copyright notice and this permission notice (including the next
>> >> + * paragraph) shall be included in all copies or substantial portions of the
>> >> + * Software.
>> >> + *
>> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> >> + * IN THE SOFTWARE.
>> >> + *
>> >> + */
>> >> +
>> >> +#include <sys/mman.h>
>> >> +
>> >> +#include "igt_draw.h"
>> >> +
>> >> +#include "drmtest.h"
>> >> +#include "intel_chipset.h"
>> >> +#include "igt_core.h"
>> >> +#include "igt_fb.h"
>> >> +#include "ioctl_wrappers.h"
>> >> +
>> >> +/**
>> >> + * SECTION:igt_draw
>> >> + * @short_description: drawing helpers for tests
>> >> + * @title: i-g-t draw
>> >> + * @include: igt_draw.h
>> >> + *
>> >> + * This library contains some functions for drawing rectangles on buffers using
>> >> + * the many different drawing methods we have. It also contains some wrappers
>> >> + * that make the process easier if you have the abstract objects in hand.
>> >> + *
>> >> + * All functions assume the buffers are in the XRGB 8:8:8 format.
>> >> + *
>> >> + */
>> >> +
>> >> +/* Some internal data structures to avoid having to pass tons of parameters
>> >> + * around everything. */
>> >> +struct cmd_data {
>> >> + drm_intel_bufmgr *bufmgr;
>> >> + drm_intel_context *context;
>> >> +};
>> >> +
>> >> +struct buf_data {
>> >> + uint32_t handle;
>> >> + uint32_t size;
>> >> + uint32_t stride;
>> >> +};
>> >> +
>> >> +struct rect {
>> >> + int x;
>> >> + int y;
>> >> + int w;
>> >> + int h;
>> >> +};
>> >> +
>> >> +const char *igt_draw_get_method_name(enum igt_draw_method method)
>> >> +{
>> >> + switch (method) {
>> >> + case IGT_DRAW_MMAP_CPU:
>> >> + return "mmap-cpu";
>> >> + case IGT_DRAW_MMAP_GTT:
>> >> + return "mmap-gtt";
>> >> + case IGT_DRAW_PWRITE:
>> >> + return "pwrite";
>> >> + case IGT_DRAW_BLT:
>> >> + return "blt";
>> >> + case IGT_DRAW_RENDER:
>> >> + return "render";
>> >> + default:
>> >> + igt_assert(false);
>> >> + }
>> >> +}
>> >> +
>> >> +static int swizzle_addr(int addr, int swizzle)
>> >> +{
>> >> + int bit6;
>> >> +
>> >> + if (swizzle == I915_BIT_6_SWIZZLE_9_10) {
>> >> + bit6 = ((addr >> 6) & 1) ^ ((addr >> 9) & 1) ^
>> >> + ((addr >> 10) & 1);
>> >> + addr &= ~(1 << 6);
>> >> + addr |= (bit6 << 6);
>> >> + }
>> >> +
>> >> + return addr;
>> >> +}
>> >> +
>> >> +/* It's all in "pixel coordinates", so make sure you multiply/divide by the bpp
>> >> + * if you need to. */
>> >> +static int linear_x_y_to_tiled_pos(int x, int y, uint32_t stride, int swizzle)
>> >> +{
>> >> + int x_tile_size, y_tile_size;
>> >> + int x_tile_n, y_tile_n, x_tile_off, y_tile_off;
>> >> + int line_size, tile_size;
>> >> + int tile_n, tile_off;
>> >> + int tiled_pos, tiles_per_line;
>> >> + int bpp;
>> >> +
>> >> + line_size = stride;
>> >> + x_tile_size = 512;
>> >> + y_tile_size = 8;
>> >> + tile_size = x_tile_size * y_tile_size;
>> >> + tiles_per_line = line_size / x_tile_size;
>> >> + bpp = sizeof(uint32_t);
>> >> +
>> >> + y_tile_n = y / y_tile_size;
>> >> + y_tile_off = y % y_tile_size;
>> >> +
>> >> + x_tile_n = (x * bpp) / x_tile_size;
>> >> + x_tile_off = (x * bpp) % x_tile_size;
>> >> +
>> >> + tile_n = y_tile_n * tiles_per_line + x_tile_n;
>> >> + tile_off = y_tile_off * x_tile_size + x_tile_off;
>> >> + tiled_pos = tile_n * tile_size + tile_off;
>> >> +
>> >> + tiled_pos = swizzle_addr(tiled_pos, swizzle);
>> >> +
>> >> + return tiled_pos / bpp;
>> >> +}
>> >> +
>> >> +/* It's all in "pixel coordinates", so make sure you multiply/divide by the bpp
>> >> + * if you need to. */
>> >> +static void tiled_pos_to_x_y_linear(int tiled_pos, uint32_t stride,
>> >> + int swizzle, int *x, int *y)
>> >> +{
>> >> + int tile_n, tile_off, tiles_per_line, line_size;
>> >> + int x_tile_off, y_tile_off;
>> >> + int x_tile_n, y_tile_n;
>> >> + int x_tile_size, y_tile_size, tile_size;
>> >> + int bpp;
>> >> +
>> >> + tiled_pos = swizzle_addr(tiled_pos, swizzle);
>> >> +
>> >> + line_size = stride;
>> >> + x_tile_size = 512;
>> >> + y_tile_size = 8;
>> >> + tile_size = x_tile_size * y_tile_size;
>> >> + tiles_per_line = line_size / x_tile_size;
>> >> + bpp = sizeof(uint32_t);
>> >> +
>> >> + tile_n = tiled_pos / tile_size;
>> >> + tile_off = tiled_pos % tile_size;
>> >> +
>> >> + y_tile_off = tile_off / x_tile_size;
>> >> + x_tile_off = tile_off % x_tile_size;
>> >> +
>> >> + x_tile_n = tile_n % tiles_per_line;
>> >> + y_tile_n = tile_n / tiles_per_line;
>> >> +
>> >> + *x = (x_tile_n * x_tile_size + x_tile_off) / bpp;
>> >> + *y = y_tile_n * y_tile_size + y_tile_off;
>> >> +}
>> >> +
>> >> +static void draw_rect_mmap_cpu(int fd, struct buf_data *buf, struct rect *rect,
>> >> + uint32_t color)
>> >> +{
>> >> + uint32_t *ptr;
>> >> + int x, y, pos;
>> >> + uint32_t tiling, swizzle;
>> >> +
>> >> + gem_set_domain(fd, buf->handle, I915_GEM_DOMAIN_CPU,
>> >> + I915_GEM_DOMAIN_CPU);
>> >> + gem_get_tiling(fd, buf->handle, &tiling, &swizzle);
>> >> +
>> >> + ptr = gem_mmap__cpu(fd, buf->handle, 0, buf->size, 0);
>> >> + igt_assert(ptr);
>> >> +
>> >> + for (y = rect->y; y < rect->y + rect->h; y++) {
>> >> + for (x = rect->x; x < rect->x + rect->w; x++) {
>> >> + if (tiling)
>> >> + pos = linear_x_y_to_tiled_pos(x, y, buf->stride,
>> >> + swizzle);
>> >> + else
>> >> + pos = (y * buf->stride / sizeof(uint32_t)) + x;
>> >> + ptr[pos] = color;
>> >> + }
>> >> + }
>> >> +
>> >> + gem_sw_finish(fd, buf->handle);
>> >> +
>> >> + igt_assert(munmap(ptr, buf->size) == 0);
>> >> +}
>> >> +
>> >> +static void draw_rect_mmap_gtt(int fd, struct buf_data *buf, struct rect *rect,
>> >> + uint32_t color)
>> >> +{
>> >> + uint32_t *ptr;
>> >> + int x, y;
>> >> +
>> >> + ptr = gem_mmap__gtt(fd, buf->handle, buf->size,
>> >> + PROT_READ | PROT_WRITE);
>> >> + igt_assert(ptr);
>> >> +
>> >> + for (y = rect->y; y < rect->y + rect->h; y++) {
>> >> + int line_begin = y * buf->stride / sizeof(uint32_t);
>> >> + for (x = rect->x; x < rect->x + rect->w; x++)
>> >> + ptr[line_begin + x] = color;
>> >> + }
>> >> +
>> >> + igt_assert(munmap(ptr, buf->size) == 0);
>> >> +}
>> >> +
>> >> +static void draw_rect_pwrite_untiled(int fd, struct buf_data *buf,
>> >> + struct rect *rect, uint32_t color)
>> >> +{
>> >> + uint32_t tmp[rect->w];
>> >> + int i, y, offset, bpp;
>> >> +
>> >> + bpp = sizeof(uint32_t);
>> >> +
>> >> + for (i = 0; i < rect->w; i++)
>> >> + tmp[i] = color;
>> >> +
>> >> + for (y = rect->y; y < rect->y + rect->h; y++) {
>> >> + offset = (y * buf->stride) + (rect->x * bpp);
>> >> + gem_write(fd, buf->handle, offset, tmp, rect->w * bpp);
>> >> + }
>> >> +}
>> >> +
>> >> +static void draw_rect_pwrite_tiled(int fd, struct buf_data *buf,
>> >> + struct rect *rect, uint32_t color,
>> >> + uint32_t swizzle)
>> >> +{
>> >> + int i;
>> >> + int tiled_pos, bpp, x, y;
>> >> + uint32_t tmp[1024];
>> >> + int tmp_used = 0, tmp_size = ARRAY_SIZE(tmp);
>> >> + bool flush_tmp = false;
>> >> + int tmp_start_pos = 0;
>> >> +
>> >> + bpp = sizeof(uint32_t);
>> >> +
>> >> + /* Instead of doing one pwrite per pixel, we try to group the maximum
>> >> + * amount of consecutive pixels we can in a single pwrite: that's why we
>> >> + * use the "tmp" variables. */
>> >> + for (i = 0; i < tmp_size; i++)
>> >> + tmp[i] = color;
>> >> +
>> >> + for (tiled_pos = 0; tiled_pos < buf->size; tiled_pos += bpp) {
>> >> + tiled_pos_to_x_y_linear(tiled_pos, buf->stride, swizzle, &x, &y);
>> >> +
>> >> + if (x >= rect->x && x < rect->x + rect->w &&
>> >> + y >= rect->y && y < rect->y + rect->h) {
>> >> + if (tmp_used == 0)
>> >> + tmp_start_pos = tiled_pos;
>> >> + tmp_used++;
>> >> + } else {
>> >> + flush_tmp = true;
>> >> + }
>> >> +
>> >> + if (tmp_used == tmp_size || (flush_tmp && tmp_used > 0)) {
>> >> + gem_write(fd, buf->handle, tmp_start_pos, tmp,
>> >> + tmp_used * bpp);
>> >> + flush_tmp = false;
>> >> + tmp_used = 0;
>> >> + }
>> >> + }
>> >> +}
>> >> +
>> >> +static void draw_rect_pwrite(int fd, struct buf_data *buf,
>> >> + struct rect *rect, uint32_t color)
>> >> +{
>> >> + uint32_t tiling, swizzle;
>> >> +
>> >> + gem_get_tiling(fd, buf->handle, &tiling, &swizzle);
>> >> +
>> >> + if (tiling)
>> >> + draw_rect_pwrite_tiled(fd, buf, rect, color, swizzle);
>> >> + else
>> >> + draw_rect_pwrite_untiled(fd, buf, rect, color);
>> >> +}
>> >> +
>> >> +static void draw_rect_blt(int fd, struct cmd_data *cmd_data,
>> >> + struct buf_data *buf, struct rect *rect,
>> >> + uint32_t color)
>> >> +{
>> >> + drm_intel_bo *dst;
>> >> + struct intel_batchbuffer *batch;
>> >> + int blt_cmd_len, blt_cmd_tiling;
>> >> + uint32_t devid = intel_get_drm_devid(fd);
>> >> + int gen = intel_gen(devid);
>> >> + uint32_t tiling, swizzle;
>> >> + int pitch;
>> >> +
>> >> + gem_get_tiling(fd, buf->handle, &tiling, &swizzle);
>> >> +
>> >> + dst = gem_handle_to_libdrm_bo(cmd_data->bufmgr, fd, "", buf->handle);
>> >> + igt_assert(dst);
>> >> +
>> >> + batch = intel_batchbuffer_alloc(cmd_data->bufmgr, devid);
>> >> + igt_assert(batch);
>> >> +
>> >> + blt_cmd_len = (gen >= 8) ? 0x5 : 0x4;
>> >> + blt_cmd_tiling = (tiling) ? XY_COLOR_BLT_TILED : 0;
>> >> + pitch = (tiling) ? buf->stride / 4 : buf->stride;
>> >> +
>> >> + BEGIN_BATCH(6, 1);
>> >
>> > This breaks on gen8 because there the relocations are 2 dwords. You need
>> > to use the magic macros for that.
>> >
>>
>> Please read the code again. I actually developed and tested everything
>> on gen8 :)
>
> Oh right I missed that OUT_RELOC uses qwords on gen8+ automatically. I
> still think using the same logic as with all other blitter tests would be
> better since blitter cmd copypaste fail has been the bane of igt.
Well, this is one of the main goals of the library: use it instead of
adding yet another implementation of the drawing commands...
>
>>
>> >> + OUT_BATCH(XY_COLOR_BLT_CMD_NOLEN | XY_COLOR_BLT_WRITE_ALPHA |
>> >> + XY_COLOR_BLT_WRITE_RGB | blt_cmd_tiling | blt_cmd_len);
>> >> + OUT_BATCH((3 << 24) | (0xF0 << 16) | pitch);
>> >> + OUT_BATCH((rect->y << 16) | rect->x);
>> >> + OUT_BATCH(((rect->y + rect->h) << 16) | (rect->x + rect->w));
>> >> + if (tiling)
>
> btw this isn't needed OUT_RELOC_FENCED works correctly for untiled
> buffers. It's actually required on gen2/3 for untiled buffers if you reuse
> buffers (which might be the case for a library function) since OUT_RELOC
> does _not_ remove a stale fence if there is one.
>
>> >> + OUT_RELOC_FENCED(dst, 0, I915_GEM_DOMAIN_RENDER, 0);
>> >> + else
>> >> + OUT_RELOC(dst, 0, I915_GEM_DOMAIN_RENDER, 0);
>> >> + OUT_BATCH(color);
>> >> + ADVANCE_BATCH();
>> >> +
>> >> + intel_batchbuffer_flush(batch);
>> >> + gem_sync(fd, buf->handle);
>> >> + intel_batchbuffer_free(batch);
>> >> +}
>> >> +
>> >> +static void draw_rect_render(int fd, struct cmd_data *cmd_data,
>> >> + struct buf_data *buf, struct rect *rect,
>> >> + uint32_t color)
>> >> +{
>> >> + drm_intel_bo *src, *dst;
>> >> + uint32_t devid = intel_get_drm_devid(fd);
>> >> + igt_render_copyfunc_t rendercopy = igt_get_render_copyfunc(devid);
>> >> + struct igt_buf src_buf, dst_buf;
>> >> + struct intel_batchbuffer *batch;
>> >> + uint32_t tiling, swizzle;
>> >> + struct buf_data tmp;
>> >> +
>> >> + igt_skip_on(!rendercopy);
>> >> +
>> >> + gem_get_tiling(fd, buf->handle, &tiling, &swizzle);
>> >> +
>> >> + /* We create a temporary buffer and copy from it using rendercopy. */
>> >> + tmp.size = rect->w * rect->h * sizeof(uint32_t);
>> >> + tmp.handle = gem_create(fd, tmp.size);
>> >> + tmp.stride = rect->w * sizeof(uint32_t);
>> >> + draw_rect_mmap_cpu(fd, &tmp, &(struct rect){0, 0, rect->w, rect->h},
>> >> + color);
>> >> +
>> >> + src = gem_handle_to_libdrm_bo(cmd_data->bufmgr, fd, "", tmp.handle);
>> >> + igt_assert(src);
>> >> + dst = gem_handle_to_libdrm_bo(cmd_data->bufmgr, fd, "", buf->handle);
>> >> + igt_assert(dst);
>> >> +
>> >> + src_buf.bo = src;
>> >> + src_buf.stride = tmp.stride;
>> >> + src_buf.tiling = I915_TILING_NONE;
>> >> + src_buf.size = tmp.size;
>> >> + dst_buf.bo = dst;
>> >> + dst_buf.stride = buf->stride;
>> >> + dst_buf.tiling = tiling;
>> >> + dst_buf.size = buf->size;
>> >> +
>> >> + batch = intel_batchbuffer_alloc(cmd_data->bufmgr, devid);
>> >> + igt_assert(batch);
>> >> +
>> >> + rendercopy(batch, cmd_data->context, &src_buf, 0, 0, rect->w, rect->h,
>> >> + &dst_buf, rect->x, rect->y);
>> >> +
>> >> + gem_sync(fd, buf->handle);
>> >> + intel_batchbuffer_free(batch);
>> >> + gem_close(fd, tmp.handle);
>> >> +}
>> >> +
>> >> +/**
>> >> + * igt_draw_rect:
>> >> + * @fd: the DRM file descriptor
>> >> + * @bufmgr: the libdrm bufmgr, only required for IGT_DRAW_BLT and
>> >> + * IGT_DRAW_RENDER
>> >> + * @context: the context, can be NULL if you don't want to think about it
>> >> + * @buf_handle: the handle of the buffer where you're going to draw to
>> >> + * @buf_size: the size of the buffer
>> >> + * @buf_stride: the stride of the buffer
>> >> + * @method: method you're going to use to write to the buffer
>> >> + * @rect_x: horizontal position on the buffer where your rectangle starts
>> >> + * @rect_y: vertical position on the buffer where your rectangle starts
>> >> + * @rect_w: width of the rectangle
>> >> + * @rect_h: height of the rectangle
>> >> + * @color: color of the rectangle
>> >> + *
>> >> + * This function draws a colored rectangle on the destination buffer, allowing
>> >> + * you to specify the method used to draw the rectangle. We assume 32 bit pixels
>> >> + * with 8 bits per color.
>> >> + */
>> >> +void igt_draw_rect(int fd, drm_intel_bufmgr *bufmgr, drm_intel_context *context,
>> >> + uint32_t buf_handle, uint32_t buf_size, uint32_t buf_stride,
>> >> + enum igt_draw_method method, int rect_x, int rect_y,
>> >> + int rect_w, int rect_h, uint32_t color)
>> >> +{
>> >> + struct cmd_data cmd_data = {
>> >> + .bufmgr = bufmgr,
>> >> + .context = context,
>> >> + };
>> >> + struct buf_data buf = {
>> >> + .handle = buf_handle,
>> >> + .size = buf_size,
>> >> + .stride = buf_stride,
>> >> + };
>> >> + struct rect rect = {
>> >> + .x = rect_x,
>> >> + .y = rect_y,
>> >> + .w = rect_w,
>> >> + .h = rect_h,
>> >> + };
>> >> +
>> >> + switch (method) {
>> >> + case IGT_DRAW_MMAP_CPU:
>> >> + draw_rect_mmap_cpu(fd, &buf, &rect, color);
>> >> + break;
>> >> + case IGT_DRAW_MMAP_GTT:
>> >> + draw_rect_mmap_gtt(fd, &buf, &rect, color);
>> >> + break;
>> >> + case IGT_DRAW_PWRITE:
>> >> + draw_rect_pwrite(fd, &buf, &rect, color);
>> >> + break;
>> >> + case IGT_DRAW_BLT:
>> >> + draw_rect_blt(fd, &cmd_data, &buf, &rect, color);
>> >> + break;
>> >> + case IGT_DRAW_RENDER:
>> >> + draw_rect_render(fd, &cmd_data, &buf, &rect, color);
>> >> + break;
>> >> + default:
>> >> + igt_assert(false);
>> >> + break;
>> >> + }
>> >> +}
>> >> +
>> >> +/**
>> >> + * igt_draw_rect_fb:
>> >> + *
>> >> + * This is exactly the same as igt_draw_rect, but you can pass an igt_fb instead
>> >> + * of manually providing its details. See igt_draw_rect.
>> >> + */
>> >> +void igt_draw_rect_fb(int fd, drm_intel_bufmgr *bufmgr,
>> >> + drm_intel_context *context, struct igt_fb *fb,
>> >> + enum igt_draw_method method, int rect_x, int rect_y,
>> >> + int rect_w, int rect_h, uint32_t color)
>> >> +{
>> >> + igt_draw_rect(fd, bufmgr, context, fb->gem_handle, fb->size, fb->stride,
>> >> + method, rect_x, rect_y, rect_w, rect_h, color);
>> >> +}
>> >> +
>> >> +/**
>> >> + * igt_draw_fill_fb:
>> >> + * @fd: the DRM file descriptor
>> >> + * @fb: the FB that is going to be filled
>> >> + * @color: the color you're going to paint it
>> >> + *
>> >> + * This function just paints an igt_fb using the provided color. It assumes 32
>> >> + * bit pixels with 8 bit colors.
>> >> + */
>> >> +void igt_draw_fill_fb(int fd, struct igt_fb *fb, uint32_t color)
>> >> +{
>> >> + igt_draw_rect_fb(fd, NULL, NULL, fb, IGT_DRAW_MMAP_GTT,
>> >> + 0, 0, fb->width, fb->height, color);
>> >> +}
>> >> diff --git a/lib/igt_draw.h b/lib/igt_draw.h
>> >> new file mode 100644
>> >> index 0000000..399e17c
>> >> --- /dev/null
>> >> +++ b/lib/igt_draw.h
>> >> @@ -0,0 +1,54 @@
>> >> +/*
>> >> + * Copyright © 2015 Intel Corporation
>> >> + *
>> >> + * Permission is hereby granted, free of charge, to any person obtaining a
>> >> + * copy of this software and associated documentation files (the "Software"),
>> >> + * to deal in the Software without restriction, including without limitation
>> >> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> >> + * and/or sell copies of the Software, and to permit persons to whom the
>> >> + * Software is furnished to do so, subject to the following conditions:
>> >> + *
>> >> + * The above copyright notice and this permission notice (including the next
>> >> + * paragraph) shall be included in all copies or substantial portions of the
>> >> + * Software.
>> >> + *
>> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> >> + * IN THE SOFTWARE.
>> >> + *
>> >> + */
>> >> +
>> >> +#ifndef __IGT_DRAW_H__
>> >> +#define __IGT_DRAW_H__
>> >> +
>> >> +#include <intel_bufmgr.h>
>> >> +#include "igt_fb.h"
>> >> +
>> >
>> > gtkdoc for this enum would be nice too I think.
>>
>> Ok.
>>
>>
>> >
>> >> +enum igt_draw_method {
>> >> + IGT_DRAW_MMAP_CPU,
>> >> + IGT_DRAW_MMAP_GTT,
>> >> + IGT_DRAW_PWRITE,
>> >> + IGT_DRAW_BLT,
>> >> + IGT_DRAW_RENDER,
>> >> + IGT_DRAW_METHOD_COUNT,
>> >> +};
>> >> +
>> >> +const char *igt_draw_get_method_name(enum igt_draw_method method);
>> >> +
>> >> +void igt_draw_rect(int fd, drm_intel_bufmgr *bufmgr, drm_intel_context *context,
>> >> + uint32_t buf_handle, uint32_t buf_size, uint32_t buf_stride,
>> >> + enum igt_draw_method method, int rect_x, int rect_y,
>> >> + int rect_w, int rect_h, uint32_t color);
>> >> +
>> >> +void igt_draw_rect_fb(int fd, drm_intel_bufmgr *bufmgr,
>> >> + drm_intel_context *context, struct igt_fb *fb,
>> >> + enum igt_draw_method method, int rect_x, int rect_y,
>> >> + int rect_w, int rect_h, uint32_t color);
>> >> +
>> >> +void igt_draw_fill_fb(int fd, struct igt_fb *fb, uint32_t color);
>> >> +
>> >> +#endif /* __IGT_DRAW_H__ */
>> >> diff --git a/lib/tests/.gitignore b/lib/tests/.gitignore
>> >> index a745a23..88f668a 100644
>> >> --- a/lib/tests/.gitignore
>> >> +++ b/lib/tests/.gitignore
>> >> @@ -1,4 +1,5 @@
>> >> # Please keep sorted alphabetically
>> >> +igt_draw
>> >> igt_fork_helper
>> >> igt_invalid_subtest_name
>> >> igt_list_only
>> >> diff --git a/lib/tests/Makefile.sources b/lib/tests/Makefile.sources
>> >> index ecd73ae..ff66d9d 100644
>> >> --- a/lib/tests/Makefile.sources
>> >> +++ b/lib/tests/Makefile.sources
>> >> @@ -1,4 +1,5 @@
>> >> check_PROGRAMS = \
>> >> + igt_draw \
>> >> igt_no_exit \
>> >> igt_no_exit_list_only \
>> >> igt_fork_helper \
>> >> diff --git a/lib/tests/igt_draw.c b/lib/tests/igt_draw.c
>> >> new file mode 100644
>> >> index 0000000..1630cc2
>> >> --- /dev/null
>> >> +++ b/lib/tests/igt_draw.c
>> >> @@ -0,0 +1,247 @@
>> >> +/*
>> >> + * Copyright © 2015 Intel Corporation
>> >> + *
>> >> + * Permission is hereby granted, free of charge, to any person obtaining a
>> >> + * copy of this software and associated documentation files (the "Software"),
>> >> + * to deal in the Software without restriction, including without limitation
>> >> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> >> + * and/or sell copies of the Software, and to permit persons to whom the
>> >> + * Software is furnished to do so, subject to the following conditions:
>> >> + *
>> >> + * The above copyright notice and this permission notice (including the next
>> >> + * paragraph) shall be included in all copies or substantial portions of the
>> >> + * Software.
>> >> + *
>> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> >> + * IN THE SOFTWARE.
>> >> + *
>> >> + */
>> >> +
>> >> +/* This program tests whether the igt_draw library actually works. */
>> >> +
>> >> +#include "drmtest.h"
>> >> +#include "igt_aux.h"
>> >> +#include "igt_draw.h"
>> >> +#include "igt_debugfs.h"
>> >> +#include "igt_fb.h"
>> >> +#include "igt_kms.h"
>> >> +
>> >> +#define MAX_CONNECTORS 32
>> >> +
>> >> +struct modeset_params {
>> >> + uint32_t crtc_id;
>> >> + uint32_t connector_id;
>> >> + drmModeModeInfoPtr mode;
>> >> +};
>> >> +
>> >> +int drm_fd;
>> >> +drmModeResPtr drm_res;
>> >> +drmModeConnectorPtr drm_connectors[MAX_CONNECTORS];
>> >> +drm_intel_bufmgr *bufmgr;
>> >> +igt_pipe_crc_t *pipe_crc;
>> >> +
>> >> +bool has_method_base_crc = false;
>> >> +igt_crc_t method_base_crc;
>> >> +
>> >> +struct modeset_params ms;
>> >> +
>> >> +static void find_modeset_params(void)
>> >> +{
>> >> + int i;
>> >> + uint32_t connector_id = 0, crtc_id;
>> >> + drmModeModeInfoPtr mode = NULL;
>> >> +
>> >> + for (i = 0; i < drm_res->count_connectors; i++) {
>> >> + drmModeConnectorPtr c = drm_connectors[i];
>> >> +
>> >> + if (c->count_modes) {
>> >> + connector_id = c->connector_id;
>> >> + mode = &c->modes[0];
>> >> + break;
>> >> + }
>> >> + }
>> >> + igt_require(connector_id);
>> >> +
>> >> + crtc_id = drm_res->crtcs[0];
>> >> + igt_assert(crtc_id);
>> >> + igt_assert(mode);
>> >> +
>> >> + ms.connector_id = connector_id;
>> >> + ms.crtc_id = crtc_id;
>> >> + ms.mode = mode;
>> >> +
>> >> +}
>> >> +
>> >> +static void get_method_crc(enum igt_draw_method method, uint64_t tiling,
>> >> + igt_crc_t *crc)
>> >> +{
>> >> + struct igt_fb fb;
>> >> + int rc;
>> >> +
>> >> + igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode->vdisplay,
>> >> + DRM_FORMAT_XRGB8888, tiling, &fb);
>> >> + igt_draw_rect_fb(drm_fd, bufmgr, NULL, &fb, method,
>> >> + 0, 0, fb.width, fb.height, 0xFF);
>> >> +
>> >> + igt_draw_rect_fb(drm_fd, bufmgr, NULL, &fb, method,
>> >> + fb.width / 4, fb.height / 4,
>> >> + fb.width / 2, fb.height / 2, 0xFF00);
>> >> + igt_draw_rect_fb(drm_fd, bufmgr, NULL, &fb, method,
>> >> + fb.width / 8, fb.height / 8,
>> >> + fb.width / 4, fb.height / 4, 0xFF0000);
>> >> + igt_draw_rect_fb(drm_fd, bufmgr, NULL, &fb, method,
>> >> + fb.width / 2, fb.height / 2,
>> >> + fb.width / 3, fb.height / 3, 0xFF00FF);
>> >> +
>> >> + rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
>> >> + &ms.connector_id, 1, ms.mode);
>> >> + igt_assert(rc == 0);
>> >> +
>> >> + igt_debug_wait_for_keypress("crc");
>> >> + igt_pipe_crc_collect_crc(pipe_crc, crc);
>> >
>> > Should we just move this igt_debug_wait_for_keypress into
>> > igt_pipe_crc_collect_crc?
>>
>> I think I prefer to leave these on the programs because you can then
>> opt-out of some cases such as when you're getting the reference CRCs.
>> It doesn't hurt to just add these to the problems, but it can be
>> annoying if the wait is on the lib and you don't want it.
>
> wait_for_keypress takes a key argument, so we could just use a "crc" key.
> Test could then use a "check" or similar (we don't have a standard yet) -
> generally you don't really want to run with --interactive-debug=all anyway
> since that tends to stop awfully often already ;-) Also you can list
> multiple keywords.
>
> So annoying waits from libraries are already taken care of, no harm in
> adding them.
>
Ok, I agree with you here now.
>> >> +
>> >> + kmstest_unset_all_crtcs(drm_fd, drm_res);
>> >> + igt_remove_fb(drm_fd, &fb);
>> >> +}
>> >> +
>> >> +static void draw_method_subtest(enum igt_draw_method method, uint64_t tiling)
>> >> +{
>> >> + igt_crc_t crc;
>> >> +
>> >> + kmstest_unset_all_crtcs(drm_fd, drm_res);
>> >> +
>> >> + find_modeset_params();
>> >> +
>> >> + /* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the parameter for
>> >> + * comparison. Cache the value so we don't recompute it for every single
>> >> + * subtest. */
>> >> + if (!has_method_base_crc) {
>> >> + get_method_crc(IGT_DRAW_MMAP_GTT, LOCAL_DRM_FORMAT_MOD_NONE,
>> >> + &method_base_crc);
>> >> + has_method_base_crc = true;
>> >> + }
>> >> +
>> >> + get_method_crc(method, tiling, &crc);
>> >> + igt_assert_crc_equal(&crc, &method_base_crc);
>> >> +}
>> >> +
>> >> +static void get_fill_crc(uint64_t tiling, igt_crc_t *crc)
>> >> +{
>> >> + struct igt_fb fb;
>> >> + int rc;
>> >> +
>> >> + igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode->vdisplay,
>> >> + DRM_FORMAT_XRGB8888, tiling, &fb);
>> >> +
>> >> + igt_draw_fill_fb(drm_fd, &fb, 0xFF);
>> >> +
>> >> + rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
>> >> + &ms.connector_id, 1, ms.mode);
>> >> + igt_assert(rc == 0);
>> >> +
>> >> + igt_debug_wait_for_keypress("crc");
>> >> + igt_pipe_crc_collect_crc(pipe_crc, crc);
>> >> +
>> >> + kmstest_unset_all_crtcs(drm_fd, drm_res);
>> >> + igt_remove_fb(drm_fd, &fb);
>> >> +}
>> >> +
>> >> +static void fill_fb_subtest(void)
>> >> +{
>> >> + int rc;
>> >> + struct igt_fb fb;
>> >> + igt_crc_t base_crc, crc;
>> >> +
>> >> + kmstest_unset_all_crtcs(drm_fd, drm_res);
>> >> +
>> >> + find_modeset_params();
>> >> +
>> >> + igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode->vdisplay,
>> >> + DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb);
>> >> +
>> >> + igt_draw_rect_fb(drm_fd, bufmgr, NULL, &fb, IGT_DRAW_MMAP_GTT,
>> >> + 0, 0, fb.width, fb.height, 0xFF);
>> >> +
>> >> + rc = drmModeSetCrtc(drm_fd, ms.crtc_id, fb.fb_id, 0, 0,
>> >> + &ms.connector_id, 1, ms.mode);
>> >> + igt_assert(rc == 0);
>> >> +
>> >> + igt_debug_wait_for_keypress("crc");
>> >> + igt_pipe_crc_collect_crc(pipe_crc, &base_crc);
>> >> +
>> >> + get_fill_crc(LOCAL_DRM_FORMAT_MOD_NONE, &crc);
>> >> + igt_assert_crc_equal(&crc, &base_crc);
>> >> +
>> >> + get_fill_crc(LOCAL_I915_FORMAT_MOD_X_TILED, &crc);
>> >> + igt_assert_crc_equal(&crc, &base_crc);
>> >> +
>> >> + kmstest_unset_all_crtcs(drm_fd, drm_res);
>> >> + igt_remove_fb(drm_fd, &fb);
>> >> +}
>> >> +
>> >> +static void setup_environment(void)
>> >> +{
>> >> + int i;
>> >> +
>> >> + drm_fd = drm_open_any_master();
>> >> + igt_require(drm_fd >= 0);
>> >> +
>> >> + drm_res = drmModeGetResources(drm_fd);
>> >> + igt_assert(drm_res->count_connectors <= MAX_CONNECTORS);
>> >> +
>> >> + for (i = 0; i < drm_res->count_connectors; i++)
>> >> + drm_connectors[i] = drmModeGetConnector(drm_fd,
>> >> + drm_res->connectors[i]);
>> >> +
>> >> + kmstest_set_vt_graphics_mode();
>> >> +
>> >> + bufmgr = drm_intel_bufmgr_gem_init(drm_fd, 4096);
>> >> + igt_assert(bufmgr);
>> >> + drm_intel_bufmgr_gem_enable_reuse(bufmgr);
>> >> +
>> >> + pipe_crc = igt_pipe_crc_new(0, INTEL_PIPE_CRC_SOURCE_AUTO);
>> >> +}
>> >> +
>> >> +static void teardown_environment(void)
>> >> +{
>> >> + int i;
>> >> +
>> >> + igt_pipe_crc_free(pipe_crc);
>> >> +
>> >> + drm_intel_bufmgr_destroy(bufmgr);
>> >> +
>> >> + for (i = 0; i < drm_res->count_connectors; i++)
>> >> + drmModeFreeConnector(drm_connectors[i]);
>> >> +
>> >> + drmModeFreeResources(drm_res);
>> >> + close(drm_fd);
>> >> +}
>> >> +
>> >> +igt_main
>> >> +{
>> >> + enum igt_draw_method method;
>> >> +
>> >> + igt_fixture
>> >> + setup_environment();
>> >> +
>> >> + for (method = 0; method < IGT_DRAW_METHOD_COUNT; method++) {
>> >> + igt_subtest_f("draw-method-%s-untiled",
>> >> + igt_draw_get_method_name(method))
>> >> + draw_method_subtest(method, LOCAL_DRM_FORMAT_MOD_NONE);
>> >> + igt_subtest_f("draw-method-%s-tiled",
>> >> + igt_draw_get_method_name(method))
>> >> + draw_method_subtest(method,
>> >> + LOCAL_I915_FORMAT_MOD_X_TILED);
>> >> + }
>> >> +
>> >> + igt_subtest("fill-fb")
>> >> + fill_fb_subtest();
>> >> +
>> >> + igt_fixture
>> >> + teardown_environment();
>> >> +}
>> >> --
>> >> 2.1.4
>> >>
>> >> _______________________________________________
>> >> Intel-gfx mailing list
>> >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> > --
>> > Daniel Vetter
>> > Software Engineer, Intel Corporation
>> > http://blog.ffwll.ch
>>
>>
>>
>> --
>> Paulo Zanoni
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
