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

2015-05-06 Thread Daniel Vetter
On Tue, May 05, 2015 at 06:30:50PM -0300, Paulo Zanoni wrote:
 2015-04-07 10:44 GMT-03:00 Daniel Vetter dan...@ffwll.ch:
  On Tue, Apr 07, 2015 at 11:12:09AM +0100, Chris Wilson wrote:
  On Tue, Apr 07, 2015 at 11:07:07AM +0200, Daniel Vetter wrote:
   On Tue, Apr 07, 2015 at 09:36:37AM +0100, Chris Wilson wrote:
On Tue, Apr 07, 2015 at 10:10:25AM +0200, Daniel Vetter wrote:
 On Thu, Apr 02, 2015 at 12:15:13AM +0100, Chris Wilson wrote:
  On Wed, Apr 01, 2015 at 07:40:59PM -0300, Paulo Zanoni wrote:
   +static void draw_rect_mmap_wc(int fd, struct buf_data *buf, 
   struct rect *rect,
   + uint32_t color)
   +{
   +   uint32_t *ptr;
   +   uint32_t tiling, swizzle;
   +
   +   gem_get_tiling(fd, buf-handle, tiling, swizzle);
   +
   +   /* We didn't implement suport for the older tiling methods 
   yet. */
   +   if (tiling != I915_TILING_NONE)
   +   igt_require(intel_gen(intel_get_drm_devid(fd)) = 5);
 
  But you now do! You need something like:

 The problem is that the kernel hides bit17 swizzling. I chatted with 
 Paulo
 on irc about this and we decided just ignore them all is the simplest
 approach.
   
Urm, that was the whole point of GET_TILING v2. That small function is
all you need to determine when bit17 is in effect and then you get to
reuse all the direct CPU methods (as they are also used by userspace)
for earlier gen.
  
   Oh right completely forgot that we've added this. But imo can be added on
   top once we need it (it's not just gen2 but also some gen3 which need
   different tile dimensions).
 
  Tile size is 2048 for gen2 only right. Tile width is the same for all
  gen3 for tiling X (and gen2), but tiling Y width depends on subgen.
  Right?
 
  Just need to check because I have code that depends on this...
 
  Oh right thought about Y tiling which changes on gen3. X tiling matches
  your description afaik - I consider gem_tiled_pread the authoritative
  source for this stuff.
 
 Ok, so after all this discussion, what is the conclusion here? What is
 required before we can merge the patch?
 
 Notice that with this patch we still won't have anybody using the
 library (except for the test added by the patch), and all the users I
 plan to add are gen5+. Also, I created VIZ-5495 to make sure we create
 intel_bo_fill() at some point (and, when we do it, I suggest we just
 use the implementation from igt_draw.c).
 
 I really think the improvements discussed here can be done after we
 merge the patch because the it's not breaking anything, AFAICS. So,
 can I push this? Also, after the code is on igt, it will probably be
 easier to discuss the possible improvements since we'll be able to
 send patches.

I guess you could capture the discussion here in a TODO comment in
igt_draw? But yeah I thought we've discussed this on irc and agreed that
moving ahead is ok, and that extensions can be done later on.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-05-05 Thread Paulo Zanoni
2015-04-07 10:44 GMT-03:00 Daniel Vetter dan...@ffwll.ch:
 On Tue, Apr 07, 2015 at 11:12:09AM +0100, Chris Wilson wrote:
 On Tue, Apr 07, 2015 at 11:07:07AM +0200, Daniel Vetter wrote:
  On Tue, Apr 07, 2015 at 09:36:37AM +0100, Chris Wilson wrote:
   On Tue, Apr 07, 2015 at 10:10:25AM +0200, Daniel Vetter wrote:
On Thu, Apr 02, 2015 at 12:15:13AM +0100, Chris Wilson wrote:
 On Wed, Apr 01, 2015 at 07:40:59PM -0300, Paulo Zanoni wrote:
  +static void draw_rect_mmap_wc(int fd, struct buf_data *buf, 
  struct rect *rect,
  + uint32_t color)
  +{
  +   uint32_t *ptr;
  +   uint32_t tiling, swizzle;
  +
  +   gem_get_tiling(fd, buf-handle, tiling, swizzle);
  +
  +   /* We didn't implement suport for the older tiling methods 
  yet. */
  +   if (tiling != I915_TILING_NONE)
  +   igt_require(intel_gen(intel_get_drm_devid(fd)) = 5);

 But you now do! You need something like:
   
The problem is that the kernel hides bit17 swizzling. I chatted with 
Paulo
on irc about this and we decided just ignore them all is the simplest
approach.
  
   Urm, that was the whole point of GET_TILING v2. That small function is
   all you need to determine when bit17 is in effect and then you get to
   reuse all the direct CPU methods (as they are also used by userspace)
   for earlier gen.
 
  Oh right completely forgot that we've added this. But imo can be added on
  top once we need it (it's not just gen2 but also some gen3 which need
  different tile dimensions).

 Tile size is 2048 for gen2 only right. Tile width is the same for all
 gen3 for tiling X (and gen2), but tiling Y width depends on subgen.
 Right?

 Just need to check because I have code that depends on this...

 Oh right thought about Y tiling which changes on gen3. X tiling matches
 your description afaik - I consider gem_tiled_pread the authoritative
 source for this stuff.

Ok, so after all this discussion, what is the conclusion here? What is
required before we can merge the patch?

Notice that with this patch we still won't have anybody using the
library (except for the test added by the patch), and all the users I
plan to add are gen5+. Also, I created VIZ-5495 to make sure we create
intel_bo_fill() at some point (and, when we do it, I suggest we just
use the implementation from igt_draw.c).

I really think the improvements discussed here can be done after we
merge the patch because the it's not breaking anything, AFAICS. So,
can I push this? Also, after the code is on igt, it will probably be
easier to discuss the possible improvements since we'll be able to
send patches.

Thanks for the reviews,
Paulo

 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 http://blog.ffwll.ch



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-07 Thread Daniel Vetter
On Thu, Apr 02, 2015 at 12:15:13AM +0100, Chris Wilson wrote:
 On Wed, Apr 01, 2015 at 07:40:59PM -0300, Paulo Zanoni wrote:
  +static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect 
  *rect,
  + uint32_t color)
  +{
  +   uint32_t *ptr;
  +   uint32_t tiling, swizzle;
  +
  +   gem_get_tiling(fd, buf-handle, tiling, swizzle);
  +
  +   /* We didn't implement suport for the older tiling methods yet. */
  +   if (tiling != I915_TILING_NONE)
  +   igt_require(intel_gen(intel_get_drm_devid(fd)) = 5);
 
 But you now do! You need something like:

The problem is that the kernel hides bit17 swizzling. I chatted with Paulo
on irc about this and we decided just ignore them all is the simplest
approach.
-Daniel

 
 static void get_tiling_and_swizzle(int fd,
  struct buf_data *buf,
  int *tiling, int *swizzle)
 {
   struct local_i915_gem_get_tiling_v2 {
   uint32_t handle;
   uint32_t tiling_mode;
   uint32_t swizzle_mode;
   uint32_t phys_swizzle_mode;
   } tiling;
 #define LOCAL_IOCTL_I915_GEM_GET_TILING DRM_IOWR (DRM_COMMAND_BASE + 
 DRM_I915_GEM_GET_TILING, struct local_i915_gem_get_tiling_v2)
 
   memset(tiling, 0, sizeof(tiling));
   tiling.handle = buf-handle;
   do_ioctl(fd, LOCAL_IOCTL_I915_GEM_GET_TILING, tiling);
   igt_require(tiling.phys_swizzle == tiling.swizzle_mode ||
   intel_gen(intel_get_drm_devid(fd)) = 5); /* old kernel? */
 
   *tiling = tiling.tilling_mode;
   *swizzle = tiling.swizzle_mode;
 }
 
 -- 
 Chris Wilson, Intel Open Source Technology Centre
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-07 Thread Chris Wilson
On Tue, Apr 07, 2015 at 10:10:25AM +0200, Daniel Vetter wrote:
 On Thu, Apr 02, 2015 at 12:15:13AM +0100, Chris Wilson wrote:
  On Wed, Apr 01, 2015 at 07:40:59PM -0300, Paulo Zanoni wrote:
   +static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect 
   *rect,
   +   uint32_t color)
   +{
   + uint32_t *ptr;
   + uint32_t tiling, swizzle;
   +
   + gem_get_tiling(fd, buf-handle, tiling, swizzle);
   +
   + /* We didn't implement suport for the older tiling methods yet. */
   + if (tiling != I915_TILING_NONE)
   + igt_require(intel_gen(intel_get_drm_devid(fd)) = 5);
  
  But you now do! You need something like:
 
 The problem is that the kernel hides bit17 swizzling. I chatted with Paulo
 on irc about this and we decided just ignore them all is the simplest
 approach.

Urm, that was the whole point of GET_TILING v2. That small function is
all you need to determine when bit17 is in effect and then you get to
reuse all the direct CPU methods (as they are also used by userspace)
for earlier gen.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-07 Thread Daniel Vetter
On Tue, Apr 07, 2015 at 09:36:37AM +0100, Chris Wilson wrote:
 On Tue, Apr 07, 2015 at 10:10:25AM +0200, Daniel Vetter wrote:
  On Thu, Apr 02, 2015 at 12:15:13AM +0100, Chris Wilson wrote:
   On Wed, Apr 01, 2015 at 07:40:59PM -0300, Paulo Zanoni wrote:
+static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct 
rect *rect,
+ uint32_t color)
+{
+   uint32_t *ptr;
+   uint32_t tiling, swizzle;
+
+   gem_get_tiling(fd, buf-handle, tiling, swizzle);
+
+   /* We didn't implement suport for the older tiling methods yet. 
*/
+   if (tiling != I915_TILING_NONE)
+   igt_require(intel_gen(intel_get_drm_devid(fd)) = 5);
   
   But you now do! You need something like:
  
  The problem is that the kernel hides bit17 swizzling. I chatted with Paulo
  on irc about this and we decided just ignore them all is the simplest
  approach.
 
 Urm, that was the whole point of GET_TILING v2. That small function is
 all you need to determine when bit17 is in effect and then you get to
 reuse all the direct CPU methods (as they are also used by userspace)
 for earlier gen.

Oh right completely forgot that we've added this. But imo can be added on
top once we need it (it's not just gen2 but also some gen3 which need
different tile dimensions).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-07 Thread Chris Wilson
On Tue, Apr 07, 2015 at 11:07:07AM +0200, Daniel Vetter wrote:
 On Tue, Apr 07, 2015 at 09:36:37AM +0100, Chris Wilson wrote:
  On Tue, Apr 07, 2015 at 10:10:25AM +0200, Daniel Vetter wrote:
   On Thu, Apr 02, 2015 at 12:15:13AM +0100, Chris Wilson wrote:
On Wed, Apr 01, 2015 at 07:40:59PM -0300, Paulo Zanoni wrote:
 +static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct 
 rect *rect,
 +   uint32_t color)
 +{
 + uint32_t *ptr;
 + uint32_t tiling, swizzle;
 +
 + gem_get_tiling(fd, buf-handle, tiling, swizzle);
 +
 + /* We didn't implement suport for the older tiling methods yet. 
 */
 + if (tiling != I915_TILING_NONE)
 + igt_require(intel_gen(intel_get_drm_devid(fd)) = 5);

But you now do! You need something like:
   
   The problem is that the kernel hides bit17 swizzling. I chatted with Paulo
   on irc about this and we decided just ignore them all is the simplest
   approach.
  
  Urm, that was the whole point of GET_TILING v2. That small function is
  all you need to determine when bit17 is in effect and then you get to
  reuse all the direct CPU methods (as they are also used by userspace)
  for earlier gen.
 
 Oh right completely forgot that we've added this. But imo can be added on
 top once we need it (it's not just gen2 but also some gen3 which need
 different tile dimensions).

Tile size is 2048 for gen2 only right. Tile width is the same for all
gen3 for tiling X (and gen2), but tiling Y width depends on subgen.
Right?

Just need to check because I have code that depends on this...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-07 Thread Daniel Vetter
On Tue, Apr 07, 2015 at 11:12:09AM +0100, Chris Wilson wrote:
 On Tue, Apr 07, 2015 at 11:07:07AM +0200, Daniel Vetter wrote:
  On Tue, Apr 07, 2015 at 09:36:37AM +0100, Chris Wilson wrote:
   On Tue, Apr 07, 2015 at 10:10:25AM +0200, Daniel Vetter wrote:
On Thu, Apr 02, 2015 at 12:15:13AM +0100, Chris Wilson wrote:
 On Wed, Apr 01, 2015 at 07:40:59PM -0300, Paulo Zanoni wrote:
  +static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct 
  rect *rect,
  + uint32_t color)
  +{
  +   uint32_t *ptr;
  +   uint32_t tiling, swizzle;
  +
  +   gem_get_tiling(fd, buf-handle, tiling, swizzle);
  +
  +   /* We didn't implement suport for the older tiling methods yet. 
  */
  +   if (tiling != I915_TILING_NONE)
  +   igt_require(intel_gen(intel_get_drm_devid(fd)) = 5);
 
 But you now do! You need something like:

The problem is that the kernel hides bit17 swizzling. I chatted with 
Paulo
on irc about this and we decided just ignore them all is the simplest
approach.
   
   Urm, that was the whole point of GET_TILING v2. That small function is
   all you need to determine when bit17 is in effect and then you get to
   reuse all the direct CPU methods (as they are also used by userspace)
   for earlier gen.
  
  Oh right completely forgot that we've added this. But imo can be added on
  top once we need it (it's not just gen2 but also some gen3 which need
  different tile dimensions).
 
 Tile size is 2048 for gen2 only right. Tile width is the same for all
 gen3 for tiling X (and gen2), but tiling Y width depends on subgen.
 Right?
 
 Just need to check because I have code that depends on this...

Oh right thought about Y tiling which changes on gen3. X tiling matches
your description afaik - I consider gem_tiled_pread the authoritative
source for this stuff.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-01 Thread Chris Wilson
On Thu, Apr 02, 2015 at 12:15:13AM +0100, Chris Wilson wrote:
 On Wed, Apr 01, 2015 at 07:40:59PM -0300, Paulo Zanoni wrote:
  +static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect 
  *rect,
  + uint32_t color)
  +{
  +   uint32_t *ptr;
  +   uint32_t tiling, swizzle;
  +
  +   gem_get_tiling(fd, buf-handle, tiling, swizzle);
  +
  +   /* We didn't implement suport for the older tiling methods yet. */
  +   if (tiling != I915_TILING_NONE)
  +   igt_require(intel_gen(intel_get_drm_devid(fd)) = 5);
 
 But you now do!

Oh, with the exception of gen2. You need to adjust the tilesize
constants there.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-01 Thread Chris Wilson
On Wed, Apr 01, 2015 at 07:40:59PM -0300, Paulo Zanoni wrote:
 +static void draw_rect_mmap_wc(int fd, struct buf_data *buf, struct rect 
 *rect,
 +   uint32_t color)
 +{
 + uint32_t *ptr;
 + uint32_t tiling, swizzle;
 +
 + gem_get_tiling(fd, buf-handle, tiling, swizzle);
 +
 + /* We didn't implement suport for the older tiling methods yet. */
 + if (tiling != I915_TILING_NONE)
 + igt_require(intel_gen(intel_get_drm_devid(fd)) = 5);

But you now do! You need something like:

static void get_tiling_and_swizzle(int fd,
   struct buf_data *buf,
   int *tiling, int *swizzle)
{
struct local_i915_gem_get_tiling_v2 {
uint32_t handle;
uint32_t tiling_mode;
uint32_t swizzle_mode;
uint32_t phys_swizzle_mode;
} tiling;
#define LOCAL_IOCTL_I915_GEM_GET_TILING DRM_IOWR (DRM_COMMAND_BASE + 
DRM_I915_GEM_GET_TILING, struct local_i915_gem_get_tiling_v2)

memset(tiling, 0, sizeof(tiling));
tiling.handle = buf-handle;
do_ioctl(fd, LOCAL_IOCTL_I915_GEM_GET_TILING, tiling);
igt_require(tiling.phys_swizzle == tiling.swizzle_mode ||
intel_gen(intel_get_drm_devid(fd)) = 5); /* old kernel? */

*tiling = tiling.tilling_mode;
*swizzle = tiling.swizzle_mode;
}

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-01 Thread Paulo Zanoni
2015-03-31 19:05 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk:
 On Tue, Mar 31, 2015 at 06:52:08PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@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 tests/kms_draw_crc program to check if the library is
 sane.

 v2: - Move the test from lib/tests to tests/ (Daniel).
 - Add igt_require() to filter out the swizzling/tiling methods we
   don't support (Daniel).
 - Simplify reloc handling on the BLT case (Daniel).
 - Document enum igt_draw_method (Daniel).
 - Document igt_draw_get_method_name() (Paulo).

 You are already missing one draw path (mmap wc),

Oh, new stuff! Done.

 adding the extra swizzle
 modes for anything but bit17 is trivial,

Done.

 the BLT code is an opencoded
 intel_copy_bo

No, we do BLT fills, not copies. On the render side I used rendercopy
just because I still don't know how to do a render fill.

 and what is with all the sync? Moving everything into the
 GTT write domain (i.e. manually doing cache flushes) would seem to
 nullify the point of using the GPU in the first place.

The idea was to just be as simple as possible for the callers, but I
can remove the gem_sync()s and leave it to the callers. There's also a
little explanation on patch 2: I used this lib for some FBC tests, so
the syncs would allow us to not wait so much for the retire work
handler. But Daniel/Ville had also suggested a test without the sync,
so I guess I would have removed the sync from the library anyway when
implementing the test.

Thanks for the review!

 -Chris

 --
 Chris Wilson, Intel Open Source Technology Centre



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-01 Thread Chris Wilson
On Wed, Apr 01, 2015 at 07:08:18PM -0300, Paulo Zanoni wrote:
 2015-03-31 19:05 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk:
  the BLT code is an opencoded
  intel_copy_bo
 
 No, we do BLT fills, not copies. On the render side I used rendercopy
 just because I still don't know how to do a render fill.

Oh, my mistake then. We certainly could do with an intel_bo_fill()
routine as we use XY_COLOR_BLT in quite a few places now.

  and what is with all the sync? Moving everything into the
  GTT write domain (i.e. manually doing cache flushes) would seem to
  nullify the point of using the GPU in the first place.
 
 The idea was to just be as simple as possible for the callers, but I
 can remove the gem_sync()s and leave it to the callers. There's also a
 little explanation on patch 2: I used this lib for some FBC tests, so
 the syncs would allow us to not wait so much for the retire work
 handler.

Which retire work handler? Not the requests one surely? 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-01 Thread Paulo Zanoni
2015-04-01 19:22 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk:
 On Wed, Apr 01, 2015 at 07:08:18PM -0300, Paulo Zanoni wrote:
 2015-03-31 19:05 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk:
  the BLT code is an opencoded
  intel_copy_bo

 No, we do BLT fills, not copies. On the render side I used rendercopy
 just because I still don't know how to do a render fill.

 Oh, my mistake then. We certainly could do with an intel_bo_fill()
 routine as we use XY_COLOR_BLT in quite a few places now.

I just added this to my TODO list. Some of the current XY_COLOR_BLT
users will become users of igt_draw, so the duplication will be
reduced a little bit. Later we can create intel_bo_fill and make
igt_draw call it.


  and what is with all the sync? Moving everything into the
  GTT write domain (i.e. manually doing cache flushes) would seem to
  nullify the point of using the GPU in the first place.

 The idea was to just be as simple as possible for the callers, but I
 can remove the gem_sync()s and leave it to the callers. There's also a
 little explanation on patch 2: I used this lib for some FBC tests, so
 the syncs would allow us to not wait so much for the retire work
 handler.

 Which retire work handler? Not the requests one surely?

i915_gem_retire_work_handler()

Now that we use the frontbuffer tracking mechanism, when we use the
BLT, FBC gets disabled (by intel_fb_obj_invalidate(), called by
i915_gem_execbuffer2()). But if we don't gem_sync(), FBC only gets
reenabled after i915_gem_retire_work_handler() happens and calls
intel_frontbuffer_flush(). While having FBC disabled for a long time
is not a bug, the gem_sync() helps reducing the
many-undreds-of-milliseconds waits.


 --
 Chris Wilson, Intel Open Source Technology Centre



-- 
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-04-01 Thread Paulo Zanoni
From: Paulo Zanoni paulo.r.zan...@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 tests/kms_draw_crc program to check if the library is
sane.

v2: - Move the test from lib/tests to tests/ (Daniel).
- Add igt_require() to filter out the swizzling/tiling methods we
  don't support (Daniel).
- Simplify reloc handling on the BLT case (Daniel).
- Document enum igt_draw_method (Daniel).
- Document igt_draw_get_method_name() (Paulo).
v3: - Add IGT_DRAW_MMAP_WC (Chris).
- Implement the other trivial swizzling methods (Chris).
- Remove the gem_sync() calls (Chris).

Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 lib/Makefile.sources   |   2 +
 lib/igt_draw.c | 562 +
 lib/igt_draw.h |  65 ++
 tests/.gitignore   |   1 +
 tests/Makefile.sources |   1 +
 tests/kms_draw_crc.c   | 247 ++
 6 files changed, 878 insertions(+)
 create mode 100644 lib/igt_draw.c
 create mode 100644 lib/igt_draw.h
 create mode 100644 tests/kms_draw_crc.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 000..14e470f
--- /dev/null
+++ b/lib/igt_draw.c
@@ -0,0 +1,562 @@
+/*
+ * 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;
+};
+
+/**
+ * igt_draw_get_method_name:
+ *
+ * Simple function to transform the enum into a string. Useful when naming
+ * subtests and printing debug messages.
+ */
+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_MMAP_WC:
+   return mmap-wc;
+   case IGT_DRAW_PWRITE:
+   return pwrite;
+   case IGT_DRAW_BLT:
+   return blt;
+   case IGT_DRAW_RENDER:
+   return render;
+   default:
+   igt_assert(false);
+   }
+}
+
+#define BIT(num, bit) ((num  bit)  1)
+
+static int swizzle_addr(int addr, int swizzle)
+{
+   int bit6;
+
+   switch (swizzle) {
+   case I915_BIT_6_SWIZZLE_NONE:
+   bit6 = BIT(addr, 6);
+   break;
+   case I915_BIT_6_SWIZZLE_9:
+   bit6 = BIT(addr, 6) ^ BIT(addr, 9);
+   break;
+   case I915_BIT_6_SWIZZLE_9_10:
+   bit6 = 

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

2015-04-01 Thread Chris Wilson
On Wed, Apr 01, 2015 at 07:33:15PM -0300, Paulo Zanoni wrote:
 2015-04-01 19:22 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk:
  On Wed, Apr 01, 2015 at 07:08:18PM -0300, Paulo Zanoni wrote:
  2015-03-31 19:05 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk:
   the BLT code is an opencoded
   intel_copy_bo
 
  No, we do BLT fills, not copies. On the render side I used rendercopy
  just because I still don't know how to do a render fill.
 
  Oh, my mistake then. We certainly could do with an intel_bo_fill()
  routine as we use XY_COLOR_BLT in quite a few places now.
 
 I just added this to my TODO list. Some of the current XY_COLOR_BLT
 users will become users of igt_draw, so the duplication will be
 reduced a little bit. Later we can create intel_bo_fill and make
 igt_draw call it.
 
 
   and what is with all the sync? Moving everything into the
   GTT write domain (i.e. manually doing cache flushes) would seem to
   nullify the point of using the GPU in the first place.
 
  The idea was to just be as simple as possible for the callers, but I
  can remove the gem_sync()s and leave it to the callers. There's also a
  little explanation on patch 2: I used this lib for some FBC tests, so
  the syncs would allow us to not wait so much for the retire work
  handler.
 
  Which retire work handler? Not the requests one surely?
 
 i915_gem_retire_work_handler()
 
 Now that we use the frontbuffer tracking mechanism, when we use the
 BLT, FBC gets disabled (by intel_fb_obj_invalidate(), called by
 i915_gem_execbuffer2()). But if we don't gem_sync(), FBC only gets
 reenabled after i915_gem_retire_work_handler() happens and calls
 intel_frontbuffer_flush(). While having FBC disabled for a long time
 is not a bug, the gem_sync() helps reducing the
 many-undreds-of-milliseconds waits.

Hmm. Bad news, that side-effect of gem_sync() won't happen in future.
You would need to call gem_sync() on the framebuffer to trigger the
flush, but that itself would then set the framebuffer to the GTT write
domain which should then invalidate the FBC again. To flush without
invalidate you want set_domain(scanout, read=GTT, write=0).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

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

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

2015-03-31 Thread Paulo Zanoni
2015-03-31 10:07 GMT-03:00 Daniel Vetter dan...@ffwll.ch:
 On Mon, Mar 30, 2015 at 04:45:49PM -0300, Paulo Zanoni wrote:
 2015-03-26 7:19 GMT-03:00 Daniel Vetter dan...@ffwll.ch:
  On Wed, Mar 25, 2015 at 06:50:39PM -0300, Paulo Zanoni wrote:
  From: Paulo Zanoni paulo.r.zan...@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'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.zan...@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 000..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 

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

2015-03-31 Thread Chris Wilson
On Tue, Mar 31, 2015 at 06:52:08PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@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 tests/kms_draw_crc program to check if the library is
 sane.
 
 v2: - Move the test from lib/tests to tests/ (Daniel).
 - Add igt_require() to filter out the swizzling/tiling methods we
   don't support (Daniel).
 - Simplify reloc handling on the BLT case (Daniel).
 - Document enum igt_draw_method (Daniel).
 - Document igt_draw_get_method_name() (Paulo).

You are already missing one draw path (mmap wc), adding the extra swizzle
modes for anything but bit17 is trivial, the BLT code is an opencoded
intel_copy_bo and what is with all the sync? Moving everything into the
GTT write domain (i.e. manually doing cache flushes) would seem to
nullify the point of using the GPU in the first place.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2015-03-31 Thread Paulo Zanoni
From: Paulo Zanoni paulo.r.zan...@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 tests/kms_draw_crc program to check if the library is
sane.

v2: - Move the test from lib/tests to tests/ (Daniel).
- Add igt_require() to filter out the swizzling/tiling methods we
  don't support (Daniel).
- Simplify reloc handling on the BLT case (Daniel).
- Document enum igt_draw_method (Daniel).
- Document igt_draw_get_method_name() (Paulo).

Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 lib/Makefile.sources   |   2 +
 lib/igt_draw.c | 500 +
 lib/igt_draw.h |  63 +++
 tests/.gitignore   |   1 +
 tests/Makefile.sources |   1 +
 tests/kms_draw_crc.c   | 247 
 6 files changed, 814 insertions(+)
 create mode 100644 lib/igt_draw.c
 create mode 100644 lib/igt_draw.h
 create mode 100644 tests/kms_draw_crc.c

The only things suggested by Daniel and not addressed are:
 - Moving the igt_debug_wait_for_keypress() calls into the CRC code. This should
   be a separate patch.
 - Support for a fallback function for when tiling/swizzling is not supported.
   Function igt_draw_fill_fb() was already there and should be enough for these
   cases, since it uses the simple GTT mmap implementation.

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 000..2dbbe6a
--- /dev/null
+++ b/lib/igt_draw.c
@@ -0,0 +1,500 @@
+/*
+ * 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;
+};
+
+/**
+ * igt_draw_get_method_name:
+ *
+ * Simple function to transform the enum into a string. Useful when naming
+ * subtests and printing debug messages.
+ */
+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;
+
+   switch (swizzle) {
+   case I915_BIT_6_SWIZZLE_NONE:
+   break;
+   case I915_BIT_6_SWIZZLE_9_10:
+   bit6 = 

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

2015-03-30 Thread Paulo Zanoni
2015-03-26 7:19 GMT-03:00 Daniel Vetter dan...@ffwll.ch:
 On Wed, Mar 25, 2015 at 06:50:39PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@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?


 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.


 More comments below.

More below too :)

 -Daniel


 Signed-off-by: Paulo Zanoni paulo.r.zan...@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 000..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;
 + 

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

2015-03-26 Thread Daniel Vetter
On Wed, Mar 25, 2015 at 06:50:39PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@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. 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.

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.

More comments below.
-Daniel

 
 Signed-off-by: Paulo Zanoni paulo.r.zan...@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 000..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)
 +{
 + 

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

2015-03-25 Thread Paulo Zanoni
From: Paulo Zanoni paulo.r.zan...@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.

Signed-off-by: Paulo Zanoni paulo.r.zan...@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 000..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;
+