[Intel-gfx] [PATCH 7/7] lib: add igt_draw

Daniel Vetter daniel at ffwll.ch
Tue Mar 31 06:07:27 PDT 2015


On Mon, Mar 30, 2015 at 04:45:49PM -0300, Paulo Zanoni wrote:
> 2015-03-26 7:19 GMT-03:00 Daniel Vetter <daniel at ffwll.ch>:
> > On Wed, Mar 25, 2015 at 06:50:39PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >>
> >> 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 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 at intel.com>
> >> ---
> >>  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.

> 
> >> +     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.

> >> +
> >> +     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 at lists.freedesktop.org
> >> 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


More information about the Intel-gfx mailing list