[PATCH i-g-t 1/7] Fix a bunch of printf types

[PATCH i-g-t 1/7] Fix a bunch of printf types

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

igt_kms.c: In function ‘igt_crtc_set_background’:
igt_kms.c:1940:2: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘uint64_t’ [-Wformat=]
  LOG(display, "%s.%d: crtc_set_background(%lu)\n",
  ^
intel_firmware_decode.c: In function ‘csr_open’:
intel_firmware_decode.c:169:2: warning: format ‘%zd’ expects argument of type ‘signed size_t’, but argument 3 has type ‘__off_t’ [-Wformat=]
  printf("Firmware: %s (%zd bytes)\n", filename, st.st_size);
  ^
intel_gpu_top.c: In function ‘main’:
intel_gpu_top.c:683:10: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘uint64_t’ [-Wformat=]
          stats[i] - last_stats[i]);
          ^
hsw_compute_wrpll.c: In function ‘main’:
hsw_compute_wrpll.c:644:3: warning: format ‘%li’ expects argument of type ‘long int’, but argument 7 has type ‘long long int’ [-Wformat=]
   igt_fail_on_f(ref->r2 != r2 || ref->n2 != n2 || ref->p != p,
   ^
gem_gtt_hog.c: In function ‘__real_main155’:
gem_gtt_hog.c:177:2: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘unsigned int’ [-Wformat=]
  igt_info("Time to execute %lu children:  %7.3fms\n",
  ^
kms_flip.c: In function ‘run_test_step’:
kms_flip.c:985:3: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 10 has type ‘__time_t’ [-Wformat=]
   igt_assert_f(end - start > 0.9 * frame_time(o) &&
   ^
kms_flip.c:985:3: warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 11 has type ‘__suseconds_t’ [-Wformat=]
kms_frontbuffer_tracking.c: In function ‘setup_sink_crc’:
kms_frontbuffer_tracking.c:1364:3: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 4 has type ‘ssize_t’ [-Wformat=]
   igt_info("Unexpected sink CRC error, rc=:%ld errno:%d %s\n",
   ^
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_kms.c                    |  2 +-
 tests/gem_gtt_hog.c              |  2 +-
 tests/kms_flip.c                 | 20 ++++++++++----------
 tests/kms_frontbuffer_tracking.c |  2 +-
 tools/hsw_compute_wrpll.c        |  2 +-
 tools/intel_firmware_decode.c    |  3 ++-
 tools/intel_gpu_top.c            |  2 +-
 7 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 5d5a95c20106..497118afe52a 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1937,7 +1937,7 @@ void igt_crtc_set_background(igt_pipe_t *pipe, uint64_t background)
 {
 	igt_display_t *display = pipe->display;
 
-	LOG(display, "%s.%d: crtc_set_background(%lu)\n",
+	LOG(display, "%s.%d: crtc_set_background(%"PRIx64")\n",
 	    kmstest_pipe_name(pipe->pipe),
 	    pipe->pipe, background);
 
diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
index 7ca7d12d96f1..365b43ee44ab 100644
--- a/tests/gem_gtt_hog.c
+++ b/tests/gem_gtt_hog.c
@@ -174,6 +174,6 @@ igt_simple_main
 	igt_waitchildren();
 	gettimeofday(&end, NULL);
 
-	igt_info("Time to execute %lu children:		%7.3fms\n",
+	igt_info("Time to execute %zu children:		%7.3fms\n",
 		 ARRAY_SIZE(children), elapsed(&start, &end) / 1000);
 }
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index a3acc3dd28e8..750a1882dc3c 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -132,11 +132,11 @@ struct event_state {
 static void dump_event_state(const struct event_state *es)
 {
 	igt_debug("name = %s\n"
-		  "last_ts = %lu.%lu usec\n"
-		  "last_received_ts = %lu.%lu usec\n"
+		  "last_ts = %ld.%ld usec\n"
+		  "last_received_ts = %ld.%ld usec\n"
 		  "last_seq = %u\n"
-		  "current_ts = %lu.%lu usec\n"
-		  "current_received_ts = %lu.%lu usec\n"
+		  "current_ts = %ld.%ld usec\n"
+		  "current_received_ts = %ld.%ld usec\n"
 		  "current_seq = %u\n"
 		  "count = %u\n"
 		  "seq_step = %d\n",
@@ -644,8 +644,8 @@ static void check_state(struct test_output *o, struct event_state *es)
 	timersub(&es->current_ts, &es->current_received_ts, &diff);
 	if (!analog_tv_connector(o)) {
 		igt_assert_f(diff.tv_sec < 0 || (diff.tv_sec == 0 && diff.tv_usec <= 2000),
-			     "%s ts delayed for too long: %is, %iusec\n",
-			     es->name, (int)diff.tv_sec, (int)diff.tv_usec);
+			     "%s ts delayed for too long: %lds, %ldusec\n",
+			     es->name, diff.tv_sec, diff.tv_usec);
 
 	}
 
@@ -655,9 +655,9 @@ static void check_state(struct test_output *o, struct event_state *es)
 	timersub(&es->current_ts, &es->last_received_ts, &diff);
 	igt_assert_f(timercmp(&es->last_received_ts, &es->current_ts, <),
 		     "%s ts before the %s was issued!\n"
-		     "timerdiff %is, %ius\n",
+		     "timerdiff %lds, %ldusec\n",
 		     es->name, es->name,
-		     (int) diff.tv_sec, (int) diff.tv_usec);
+		     diff.tv_sec, diff.tv_usec);
 
 	/* check only valid if no modeset happens in between, that increments by
 	 * (1 << 23) on each step. This bounding matches the one in
@@ -688,8 +688,8 @@ static void check_state(struct test_output *o, struct event_state *es)
 
 		igt_assert_f(fabs((((double) diff.tv_usec) - usec_interflip) /
 				  usec_interflip) <= 0.005,
-			     "inter-%s ts jitter: %is, %ius\n",
-			     es->name, (int) diff.tv_sec, (int) diff.tv_usec);
+			     "inter-%s ts jitter: %lds, %ldusec\n",
+			     es->name, diff.tv_sec, diff.tv_usec);
 
 		igt_assert_f(es->current_seq == es->last_seq + o->seq_step,
 			     "unexpected %s seq %u, expected %u\n",
diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 23c2b0658474..e7acc7cabbed 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1361,7 +1361,7 @@ static void setup_sink_crc(void)
 	else if (rc == SINK_CRC_SIZE)
 		sink_crc.supported = true;
 	else
-		igt_info("Unexpected sink CRC error, rc=:%ld errno:%d %s\n",
+		igt_info("Unexpected sink CRC error, rc=:%zd errno:%d %s\n",
 			 rc, errno_, strerror(errno_));
 }
 
diff --git a/tools/hsw_compute_wrpll.c b/tools/hsw_compute_wrpll.c
index e27cc349e630..6c85c07c77c4 100644
--- a/tools/hsw_compute_wrpll.c
+++ b/tools/hsw_compute_wrpll.c
@@ -642,7 +642,7 @@ int main(void)
 
 		wrpll_compute_rnp(ref->clock, &r2, &n2, &p);
 		igt_fail_on_f(ref->r2 != r2 || ref->n2 != n2 || ref->p != p,
-			      "Computed value differs for %li Hz:\n""  Reference: (%u,%u,%u)\n""  Computed:  (%u,%u,%u)\n", (int64_t)ref->clock * 1000, ref->r2, ref->n2, ref->p, r2, n2, p);
+			      "Computed value differs for %"PRId64" Hz:\n""  Reference: (%u,%u,%u)\n""  Computed:  (%u,%u,%u)\n", (int64_t)ref->clock * 1000, ref->r2, ref->n2, ref->p, r2, n2, p);
 	}
 
 	return 0;
diff --git a/tools/intel_firmware_decode.c b/tools/intel_firmware_decode.c
index ae6e02c4566c..61ce3a531165 100644
--- a/tools/intel_firmware_decode.c
+++ b/tools/intel_firmware_decode.c
@@ -25,6 +25,7 @@
  */
 
 #include <fcntl.h>
+#include <inttypes.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -166,7 +167,7 @@ static void csr_open(csr_t *ctx, const char *filename)
 			 ctx->fd, 0);
 	igt_fail_on_f(ctx->base == MAP_FAILED, "Couldn't mmap %s\n", filename);
 
-	printf("Firmware: %s (%zd bytes)\n", filename, st.st_size);
+	printf("Firmware: %s (%"PRId64" bytes)\n", filename, (int64_t)st.st_size);
 
 	ctx->css_header = (struct intel_css_header *)ctx->base;
 	ctx->package_header = (struct intel_package_header *)
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index b5cfda043dd9..4f327c6daf6b 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -679,7 +679,7 @@ int main(int argc, char **argv)
 
 			for (i = 0; i < MAX_NUM_TOP_BITS; i++) {
 				if (i < STATS_COUNT && HAS_STATS_REGS(devid)) {
-					fprintf(output, "%lu\t",
+					fprintf(output, "%"PRIu64"\t",
 						   stats[i] - last_stats[i]);
 					last_stats[i] = stats[i];
 				}
-- 
2.4.10

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

[PATCH i-g-t 2/7] tests/gem_mmap_gtt: Deal with tile sizes on gen2/3

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Gen2/3 platforms have some unusual tile dimensions. Account
for them to make the test work correctly.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/gem_mmap_gtt.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
index 4d146426535b..b9c413b6160b 100644
--- a/tests/gem_mmap_gtt.c
+++ b/tests/gem_mmap_gtt.c
@@ -253,6 +253,18 @@ test_write_gtt(int fd)
 	munmap(src, OBJECT_SIZE);
 }
 
+static int tile_width(uint32_t devid, int tiling)
+{
+	if (intel_gen(devid) == 2)
+		return 128;
+	else if (tiling == I915_TILING_X)
+		return 512;
+	else if (IS_915(devid))
+		return 512;
+	else
+		return 128;
+}
+
 static void
 test_huge_bo(int fd, int huge, int tiling)
 {
@@ -261,7 +273,8 @@ test_huge_bo(int fd, int huge, int tiling)
 	char *tiled_pattern;
 	char *linear_pattern;
 	uint64_t size, last_offset;
-	int pitch = tiling == I915_TILING_Y ? 128 : 512;
+	uint32_t devid = intel_get_drm_devid(fd);
+	int pitch = tile_width(devid, tiling);
 	int i;
 
 	switch (huge) {
@@ -327,6 +340,7 @@ test_huge_bo(int fd, int huge, int tiling)
 static void
 test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
 {
+	uint32_t devid = intel_get_drm_devid(fd);
 	uint64_t huge_object_size, i;
 	uint32_t bo, *pattern_a, *pattern_b;
 	char *a, *b;
@@ -357,7 +371,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
 
 	bo = gem_create(fd, huge_object_size);
 	if (tiling_a)
-		igt_require(__gem_set_tiling(fd, bo, tiling_a, tiling_a == I915_TILING_Y ? 128 : 512) == 0);
+		igt_require(__gem_set_tiling(fd, bo, tiling_a, tile_width(devid, tiling_a)) == 0);
 	a = __gem_mmap__gtt(fd, bo, huge_object_size, PROT_READ | PROT_WRITE);
 	igt_require(a);
 	gem_close(fd, bo);
@@ -367,7 +381,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
 
 	bo = gem_create(fd, huge_object_size);
 	if (tiling_b)
-		igt_require(__gem_set_tiling(fd, bo, tiling_b, tiling_b == I915_TILING_Y ? 128 : 512) == 0);
+		igt_require(__gem_set_tiling(fd, bo, tiling_b, tile_width(devid, tiling_b)) == 0);
 	b = __gem_mmap__gtt(fd, bo, huge_object_size, PROT_READ | PROT_WRITE);
 	igt_require(b);
 	gem_close(fd, bo);
-- 
2.4.10

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

Re: [PATCH i-g-t 2/7] tests/gem_mmap_gtt: Deal with tile sizes on gen2/3

[Chris Wilson]

On Mon, Dec 14, 2015 at 10:15:51PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Gen2/3 platforms have some unusual tile dimensions. Account
> for them to make the test work correctly.

iirc, for the purposes of the test, we could just set the stride to 512
and be done with it. The idea behind the test is to simply require a
fence register and then since we compare the same tiling pattern in
both, the actual tiling is irrevelant. (I wonder if using different
strides is of any value, not sure that it is - it doesn't change any
code paths).
-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: [PATCH i-g-t 2/7] tests/gem_mmap_gtt: Deal with tile sizes on gen2/3

[Ville Syrjälä]

On Mon, Dec 14, 2015 at 09:52:10PM +0000, Chris Wilson wrote:
> On Mon, Dec 14, 2015 at 10:15:51PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Gen2/3 platforms have some unusual tile dimensions. Account
> > for them to make the test work correctly.
> 
> iirc, for the purposes of the test, we could just set the stride to 512
> and be done with it. The idea behind the test is to simply require a
> fence register and then since we compare the same tiling pattern in
> both, the actual tiling is irrevelant. (I wonder if using different
> strides is of any value, not sure that it is - it doesn't change any
> code paths).

Well, it definitely didn't work with 512 for the X tiled.

Let's see. 512 bytes is four tiles horizontally on gen2, but we only
allocate two tiles for the object (4k). Then we walk over the mapping
meaning we first hit 128 bytes of the first tile, then 128 bytes of the
second tile. Then we walk into oblivion, which I suppose means the
scratch page. So we do that until we've written 4k bytes, meaning we
only manage to write something to the top halves of the two tiles we
actually allocated.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 2/7] tests/gem_mmap_gtt: Deal with tile sizes on gen2/3

[Paulo Zanoni]

2015-12-14 18:15 GMT-02:00  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Gen2/3 platforms have some unusual tile dimensions. Account
> for them to make the test work correctly.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/gem_mmap_gtt.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
> index 4d146426535b..b9c413b6160b 100644
> --- a/tests/gem_mmap_gtt.c
> +++ b/tests/gem_mmap_gtt.c
> @@ -253,6 +253,18 @@ test_write_gtt(int fd)
>         munmap(src, OBJECT_SIZE);
>  }
>
> +static int tile_width(uint32_t devid, int tiling)
> +{
> +       if (intel_gen(devid) == 2)
> +               return 128;
> +       else if (tiling == I915_TILING_X)
> +               return 512;
> +       else if (IS_915(devid))
> +               return 512;
> +       else
> +               return 128;
> +}
> +

Would this be useful: http://patchwork.freedesktop.org/patch/66846/ ?
I mean, we could export some subfunction out of the original patch.

>  static void
>  test_huge_bo(int fd, int huge, int tiling)
>  {
> @@ -261,7 +273,8 @@ test_huge_bo(int fd, int huge, int tiling)
>         char *tiled_pattern;
>         char *linear_pattern;
>         uint64_t size, last_offset;
> -       int pitch = tiling == I915_TILING_Y ? 128 : 512;
> +       uint32_t devid = intel_get_drm_devid(fd);
> +       int pitch = tile_width(devid, tiling);
>         int i;
>
>         switch (huge) {
> @@ -327,6 +340,7 @@ test_huge_bo(int fd, int huge, int tiling)
>  static void
>  test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
>  {
> +       uint32_t devid = intel_get_drm_devid(fd);
>         uint64_t huge_object_size, i;
>         uint32_t bo, *pattern_a, *pattern_b;
>         char *a, *b;
> @@ -357,7 +371,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
>
>         bo = gem_create(fd, huge_object_size);
>         if (tiling_a)
> -               igt_require(__gem_set_tiling(fd, bo, tiling_a, tiling_a == I915_TILING_Y ? 128 : 512) == 0);
> +               igt_require(__gem_set_tiling(fd, bo, tiling_a, tile_width(devid, tiling_a)) == 0);
>         a = __gem_mmap__gtt(fd, bo, huge_object_size, PROT_READ | PROT_WRITE);
>         igt_require(a);
>         gem_close(fd, bo);
> @@ -367,7 +381,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
>
>         bo = gem_create(fd, huge_object_size);
>         if (tiling_b)
> -               igt_require(__gem_set_tiling(fd, bo, tiling_b, tiling_b == I915_TILING_Y ? 128 : 512) == 0);
> +               igt_require(__gem_set_tiling(fd, bo, tiling_b, tile_width(devid, tiling_b)) == 0);
>         b = __gem_mmap__gtt(fd, bo, huge_object_size, PROT_READ | PROT_WRITE);
>         igt_require(b);
>         gem_close(fd, bo);
> --
> 2.4.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH i-g-t 2/7] tests/gem_mmap_gtt: Deal with tile sizes on gen2/3

[Ville Syrjälä]

On Tue, Dec 15, 2015 at 06:51:56PM -0200, Paulo Zanoni wrote:
> 2015-12-14 18:15 GMT-02:00  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Gen2/3 platforms have some unusual tile dimensions. Account
> > for them to make the test work correctly.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  tests/gem_mmap_gtt.c | 20 +++++++++++++++++---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
> > index 4d146426535b..b9c413b6160b 100644
> > --- a/tests/gem_mmap_gtt.c
> > +++ b/tests/gem_mmap_gtt.c
> > @@ -253,6 +253,18 @@ test_write_gtt(int fd)
> >         munmap(src, OBJECT_SIZE);
> >  }
> >
> > +static int tile_width(uint32_t devid, int tiling)
> > +{
> > +       if (intel_gen(devid) == 2)
> > +               return 128;
> > +       else if (tiling == I915_TILING_X)
> > +               return 512;
> > +       else if (IS_915(devid))
> > +               return 512;
> > +       else
> > +               return 128;
> > +}
> > +
> 
> Would this be useful: http://patchwork.freedesktop.org/patch/66846/ ?
> I mean, we could export some subfunction out of the original patch.

I suppose we might try to consolidate. One snag is that for display
stuff we want to use the modifiers, but I'm not sure we want to pollute
any fence stuff with those.

BTW your Yf width is not correct for 8/64 bpp (width*height != 4k).

> 
> >  static void
> >  test_huge_bo(int fd, int huge, int tiling)
> >  {
> > @@ -261,7 +273,8 @@ test_huge_bo(int fd, int huge, int tiling)
> >         char *tiled_pattern;
> >         char *linear_pattern;
> >         uint64_t size, last_offset;
> > -       int pitch = tiling == I915_TILING_Y ? 128 : 512;
> > +       uint32_t devid = intel_get_drm_devid(fd);
> > +       int pitch = tile_width(devid, tiling);
> >         int i;
> >
> >         switch (huge) {
> > @@ -327,6 +340,7 @@ test_huge_bo(int fd, int huge, int tiling)
> >  static void
> >  test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
> >  {
> > +       uint32_t devid = intel_get_drm_devid(fd);
> >         uint64_t huge_object_size, i;
> >         uint32_t bo, *pattern_a, *pattern_b;
> >         char *a, *b;
> > @@ -357,7 +371,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
> >
> >         bo = gem_create(fd, huge_object_size);
> >         if (tiling_a)
> > -               igt_require(__gem_set_tiling(fd, bo, tiling_a, tiling_a == I915_TILING_Y ? 128 : 512) == 0);
> > +               igt_require(__gem_set_tiling(fd, bo, tiling_a, tile_width(devid, tiling_a)) == 0);
> >         a = __gem_mmap__gtt(fd, bo, huge_object_size, PROT_READ | PROT_WRITE);
> >         igt_require(a);
> >         gem_close(fd, bo);
> > @@ -367,7 +381,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
> >
> >         bo = gem_create(fd, huge_object_size);
> >         if (tiling_b)
> > -               igt_require(__gem_set_tiling(fd, bo, tiling_b, tiling_b == I915_TILING_Y ? 128 : 512) == 0);
> > +               igt_require(__gem_set_tiling(fd, bo, tiling_b, tile_width(devid, tiling_b)) == 0);
> >         b = __gem_mmap__gtt(fd, bo, huge_object_size, PROT_READ | PROT_WRITE);
> >         igt_require(b);
> >         gem_close(fd, bo);
> > --
> > 2.4.10
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 2/7] tests/gem_mmap_gtt: Deal with tile sizes on gen2/3

[Paulo Zanoni]

2015-12-15 19:08 GMT-02:00 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> On Tue, Dec 15, 2015 at 06:51:56PM -0200, Paulo Zanoni wrote:
>> 2015-12-14 18:15 GMT-02:00  <ville.syrjala@linux.intel.com>:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Gen2/3 platforms have some unusual tile dimensions. Account
>> > for them to make the test work correctly.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  tests/gem_mmap_gtt.c | 20 +++++++++++++++++---
>> >  1 file changed, 17 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
>> > index 4d146426535b..b9c413b6160b 100644
>> > --- a/tests/gem_mmap_gtt.c
>> > +++ b/tests/gem_mmap_gtt.c
>> > @@ -253,6 +253,18 @@ test_write_gtt(int fd)
>> >         munmap(src, OBJECT_SIZE);
>> >  }
>> >
>> > +static int tile_width(uint32_t devid, int tiling)
>> > +{
>> > +       if (intel_gen(devid) == 2)
>> > +               return 128;
>> > +       else if (tiling == I915_TILING_X)
>> > +               return 512;
>> > +       else if (IS_915(devid))
>> > +               return 512;
>> > +       else
>> > +               return 128;
>> > +}
>> > +
>>
>> Would this be useful: http://patchwork.freedesktop.org/patch/66846/ ?
>> I mean, we could export some subfunction out of the original patch.
>
> I suppose we might try to consolidate. One snag is that for display
> stuff we want to use the modifiers, but I'm not sure we want to pollute
> any fence stuff with those.
>
> BTW your Yf width is not correct for 8/64 bpp (width*height != 4k).

Hmmm, so I guess we need to discover if the Kernel's i915_tiling_ok()
needs to be fixed.

>
>>
>> >  static void
>> >  test_huge_bo(int fd, int huge, int tiling)
>> >  {
>> > @@ -261,7 +273,8 @@ test_huge_bo(int fd, int huge, int tiling)
>> >         char *tiled_pattern;
>> >         char *linear_pattern;
>> >         uint64_t size, last_offset;
>> > -       int pitch = tiling == I915_TILING_Y ? 128 : 512;
>> > +       uint32_t devid = intel_get_drm_devid(fd);
>> > +       int pitch = tile_width(devid, tiling);
>> >         int i;
>> >
>> >         switch (huge) {
>> > @@ -327,6 +340,7 @@ test_huge_bo(int fd, int huge, int tiling)
>> >  static void
>> >  test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
>> >  {
>> > +       uint32_t devid = intel_get_drm_devid(fd);
>> >         uint64_t huge_object_size, i;
>> >         uint32_t bo, *pattern_a, *pattern_b;
>> >         char *a, *b;
>> > @@ -357,7 +371,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
>> >
>> >         bo = gem_create(fd, huge_object_size);
>> >         if (tiling_a)
>> > -               igt_require(__gem_set_tiling(fd, bo, tiling_a, tiling_a == I915_TILING_Y ? 128 : 512) == 0);
>> > +               igt_require(__gem_set_tiling(fd, bo, tiling_a, tile_width(devid, tiling_a)) == 0);
>> >         a = __gem_mmap__gtt(fd, bo, huge_object_size, PROT_READ | PROT_WRITE);
>> >         igt_require(a);
>> >         gem_close(fd, bo);
>> > @@ -367,7 +381,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
>> >
>> >         bo = gem_create(fd, huge_object_size);
>> >         if (tiling_b)
>> > -               igt_require(__gem_set_tiling(fd, bo, tiling_b, tiling_b == I915_TILING_Y ? 128 : 512) == 0);
>> > +               igt_require(__gem_set_tiling(fd, bo, tiling_b, tile_width(devid, tiling_b)) == 0);
>> >         b = __gem_mmap__gtt(fd, bo, huge_object_size, PROT_READ | PROT_WRITE);
>> >         igt_require(b);
>> >         gem_close(fd, bo);
>> > --
>> > 2.4.10
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>
>> --
>> Paulo Zanoni
>
> --
> Ville Syrjälä
> Intel OTC



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

Re: [PATCH i-g-t 2/7] tests/gem_mmap_gtt: Deal with tile sizes on gen2/3

[Ville Syrjälä]

On Tue, Dec 15, 2015 at 07:19:24PM -0200, Paulo Zanoni wrote:
> 2015-12-15 19:08 GMT-02:00 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> > On Tue, Dec 15, 2015 at 06:51:56PM -0200, Paulo Zanoni wrote:
> >> 2015-12-14 18:15 GMT-02:00  <ville.syrjala@linux.intel.com>:
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >
> >> > Gen2/3 platforms have some unusual tile dimensions. Account
> >> > for them to make the test work correctly.
> >> >
> >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > ---
> >> >  tests/gem_mmap_gtt.c | 20 +++++++++++++++++---
> >> >  1 file changed, 17 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
> >> > index 4d146426535b..b9c413b6160b 100644
> >> > --- a/tests/gem_mmap_gtt.c
> >> > +++ b/tests/gem_mmap_gtt.c
> >> > @@ -253,6 +253,18 @@ test_write_gtt(int fd)
> >> >         munmap(src, OBJECT_SIZE);
> >> >  }
> >> >
> >> > +static int tile_width(uint32_t devid, int tiling)
> >> > +{
> >> > +       if (intel_gen(devid) == 2)
> >> > +               return 128;
> >> > +       else if (tiling == I915_TILING_X)
> >> > +               return 512;
> >> > +       else if (IS_915(devid))
> >> > +               return 512;
> >> > +       else
> >> > +               return 128;
> >> > +}
> >> > +
> >>
> >> Would this be useful: http://patchwork.freedesktop.org/patch/66846/ ?
> >> I mean, we could export some subfunction out of the original patch.
> >
> > I suppose we might try to consolidate. One snag is that for display
> > stuff we want to use the modifiers, but I'm not sure we want to pollute
> > any fence stuff with those.
> >
> > BTW your Yf width is not correct for 8/64 bpp (width*height != 4k).
> 
> Hmmm, so I guess we need to discover if the Kernel's i915_tiling_ok()
> needs to be fixed.

No. That functions is all about fences. Fences and Yf don't mix.

> 
> >
> >>
> >> >  static void
> >> >  test_huge_bo(int fd, int huge, int tiling)
> >> >  {
> >> > @@ -261,7 +273,8 @@ test_huge_bo(int fd, int huge, int tiling)
> >> >         char *tiled_pattern;
> >> >         char *linear_pattern;
> >> >         uint64_t size, last_offset;
> >> > -       int pitch = tiling == I915_TILING_Y ? 128 : 512;
> >> > +       uint32_t devid = intel_get_drm_devid(fd);
> >> > +       int pitch = tile_width(devid, tiling);
> >> >         int i;
> >> >
> >> >         switch (huge) {
> >> > @@ -327,6 +340,7 @@ test_huge_bo(int fd, int huge, int tiling)
> >> >  static void
> >> >  test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
> >> >  {
> >> > +       uint32_t devid = intel_get_drm_devid(fd);
> >> >         uint64_t huge_object_size, i;
> >> >         uint32_t bo, *pattern_a, *pattern_b;
> >> >         char *a, *b;
> >> > @@ -357,7 +371,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
> >> >
> >> >         bo = gem_create(fd, huge_object_size);
> >> >         if (tiling_a)
> >> > -               igt_require(__gem_set_tiling(fd, bo, tiling_a, tiling_a == I915_TILING_Y ? 128 : 512) == 0);
> >> > +               igt_require(__gem_set_tiling(fd, bo, tiling_a, tile_width(devid, tiling_a)) == 0);
> >> >         a = __gem_mmap__gtt(fd, bo, huge_object_size, PROT_READ | PROT_WRITE);
> >> >         igt_require(a);
> >> >         gem_close(fd, bo);
> >> > @@ -367,7 +381,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
> >> >
> >> >         bo = gem_create(fd, huge_object_size);
> >> >         if (tiling_b)
> >> > -               igt_require(__gem_set_tiling(fd, bo, tiling_b, tiling_b == I915_TILING_Y ? 128 : 512) == 0);
> >> > +               igt_require(__gem_set_tiling(fd, bo, tiling_b, tile_width(devid, tiling_b)) == 0);
> >> >         b = __gem_mmap__gtt(fd, bo, huge_object_size, PROT_READ | PROT_WRITE);
> >> >         igt_require(b);
> >> >         gem_close(fd, bo);
> >> > --
> >> > 2.4.10
> >> >
> >> > _______________________________________________
> >> > Intel-gfx mailing list
> >> > Intel-gfx@lists.freedesktop.org
> >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>
> >>
> >>
> >> --
> >> Paulo Zanoni
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> 
> 
> 
> -- 
> Paulo Zanoni

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 3/7] tests/gem_mmap_gtt: Add progress indicators

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Some of the copy tests take a while, so let the user know how
far along we are via a progress indicator.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/gem_mmap_gtt.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
index b9c413b6160b..3a64a6fdc342 100644
--- a/tests/gem_mmap_gtt.c
+++ b/tests/gem_mmap_gtt.c
@@ -376,8 +376,10 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
 	igt_require(a);
 	gem_close(fd, bo);
 
-	for (i = 0; i < huge_object_size / PAGE_SIZE; i++)
+	for (i = 0; i < huge_object_size / PAGE_SIZE; i++) {
 		memcpy(a + PAGE_SIZE*i, pattern_a, PAGE_SIZE);
+		igt_progress("Writing a ", i, huge_object_size / PAGE_SIZE);
+	}
 
 	bo = gem_create(fd, huge_object_size);
 	if (tiling_b)
@@ -386,14 +388,17 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
 	igt_require(b);
 	gem_close(fd, bo);
 
-	for (i = 0; i < huge_object_size / PAGE_SIZE; i++)
+	for (i = 0; i < huge_object_size / PAGE_SIZE; i++) {
 		memcpy(b + PAGE_SIZE*i, pattern_b, PAGE_SIZE);
+		igt_progress("Writing b ", i, huge_object_size / PAGE_SIZE);
+	}
 
 	for (i = 0; i < huge_object_size / PAGE_SIZE; i++) {
 		if (i & 1)
 			memcpy(a + i *PAGE_SIZE, b + i*PAGE_SIZE, PAGE_SIZE);
 		else
 			memcpy(b + i *PAGE_SIZE, a + i*PAGE_SIZE, PAGE_SIZE);
+		igt_progress("Copying a<->b ", i, huge_object_size / PAGE_SIZE);
 	}
 
 	for (i = 0; i < huge_object_size / PAGE_SIZE; i++) {
@@ -401,6 +406,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
 			igt_assert(memcmp(pattern_b, a + PAGE_SIZE*i, PAGE_SIZE) == 0);
 		else
 			igt_assert(memcmp(pattern_a, a + PAGE_SIZE*i, PAGE_SIZE) == 0);
+		igt_progress("Checking a ", i, huge_object_size / PAGE_SIZE);
 	}
 	munmap(a, huge_object_size);
 
@@ -409,6 +415,7 @@ test_huge_copy(int fd, int huge, int tiling_a, int tiling_b)
 			igt_assert(memcmp(pattern_b, b + PAGE_SIZE*i, PAGE_SIZE) == 0);
 		else
 			igt_assert(memcmp(pattern_a, b + PAGE_SIZE*i, PAGE_SIZE) == 0);
+		igt_progress("Checking b ", i, huge_object_size / PAGE_SIZE);
 	}
 	munmap(b, huge_object_size);
 
-- 
2.4.10

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

[PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Several factors conspire against us when trying to execute
the tiled small-bo tests:
- pre-gen4 require power of two fences, with natural alignment
- the entire gtt may be mappable
- we put a guard page at the end of gtt

What all that means is that when we try to use a tiled object half
the size of the mappable area, we can only fit it in the first half
of the gtt. That leads to a SIGBUS when we try to fault in the
object when there's already something (eg. fbdev) occupying the
first half of gtt.

So in order to make the tests run on old machines, let's further
halve the object size when things look too tight.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/gem_mmap_gtt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
index 3a64a6fdc342..5c3bafcc38ba 100644
--- a/tests/gem_mmap_gtt.c
+++ b/tests/gem_mmap_gtt.c
@@ -280,6 +280,18 @@ test_huge_bo(int fd, int huge, int tiling)
 	switch (huge) {
 	case -1:
 		size = gem_mappable_aperture_size() / 2;
+
+		/* Power of two fence size, natural fence
+		 * alignment, and the guard page at the end
+		 * gtt means that if the entire gtt is
+		 * mappable, we can't usually fit in a tiled
+		 * object half the size of the gtt. Let's use
+		 * a quarter size one instead.
+		 */
+		if (tiling &&
+		    intel_gen(intel_get_drm_devid(fd)) < 4 &&
+		    size >= gem_aperture_size(fd) / 2)
+			size /= 2;
 		break;
 	case 0:
 		size = gem_mappable_aperture_size() + PAGE_SIZE;
-- 
2.4.10

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

Re: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Chris Wilson]

On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Several factors conspire against us when trying to execute
> the tiled small-bo tests:
> - pre-gen4 require power of two fences, with natural alignment
> - the entire gtt may be mappable
> - we put a guard page at the end of gtt
> 
> What all that means is that when we try to use a tiled object half
> the size of the mappable area, we can only fit it in the first half
> of the gtt. That leads to a SIGBUS when we try to fault in the
> object when there's already something (eg. fbdev) occupying the
> first half of gtt.
> 
> So in order to make the tests run on old machines, let's further
> halve the object size when things look too tight.

That defeats the point of the test. The idea is to have the two objects
that just don't fit, but only just.
-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: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Chris Wilson]

On Mon, Dec 14, 2015 at 08:49:38PM +0000, Chris Wilson wrote:
> On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Several factors conspire against us when trying to execute
> > the tiled small-bo tests:
> > - pre-gen4 require power of two fences, with natural alignment
> > - the entire gtt may be mappable
> > - we put a guard page at the end of gtt
> > 
> > What all that means is that when we try to use a tiled object half
> > the size of the mappable area, we can only fit it in the first half
> > of the gtt. That leads to a SIGBUS when we try to fault in the
> > object when there's already something (eg. fbdev) occupying the
> > first half of gtt.
> > 
> > So in order to make the tests run on old machines, let's further
> > halve the object size when things look too tight.
> 
> That defeats the point of the test. The idea is to have the two objects
> that just don't fit, but only just.

i.e. the test is meant to show that the kernel heuristics for using
partial vma do not prevent the page-fault-of-doom.
-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: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Ville Syrjälä]

On Mon, Dec 14, 2015 at 08:54:35PM +0000, Chris Wilson wrote:
> On Mon, Dec 14, 2015 at 08:49:38PM +0000, Chris Wilson wrote:
> > On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Several factors conspire against us when trying to execute
> > > the tiled small-bo tests:
> > > - pre-gen4 require power of two fences, with natural alignment
> > > - the entire gtt may be mappable
> > > - we put a guard page at the end of gtt
> > > 
> > > What all that means is that when we try to use a tiled object half
> > > the size of the mappable area, we can only fit it in the first half
> > > of the gtt. That leads to a SIGBUS when we try to fault in the
> > > object when there's already something (eg. fbdev) occupying the
> > > first half of gtt.
> > > 
> > > So in order to make the tests run on old machines, let's further
> > > halve the object size when things look too tight.
> > 
> > That defeats the point of the test. The idea is to have the two objects
> > that just don't fit, but only just.
> 
> i.e. the test is meant to show that the kernel heuristics for using
> partial vma do not prevent the page-fault-of-doom.

So just skip then?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Chris Wilson]

On Tue, Dec 15, 2015 at 11:41:44AM +0200, Ville Syrjälä wrote:
> On Mon, Dec 14, 2015 at 08:54:35PM +0000, Chris Wilson wrote:
> > On Mon, Dec 14, 2015 at 08:49:38PM +0000, Chris Wilson wrote:
> > > On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Several factors conspire against us when trying to execute
> > > > the tiled small-bo tests:
> > > > - pre-gen4 require power of two fences, with natural alignment
> > > > - the entire gtt may be mappable
> > > > - we put a guard page at the end of gtt
> > > > 
> > > > What all that means is that when we try to use a tiled object half
> > > > the size of the mappable area, we can only fit it in the first half
> > > > of the gtt. That leads to a SIGBUS when we try to fault in the
> > > > object when there's already something (eg. fbdev) occupying the
> > > > first half of gtt.
> > > > 
> > > > So in order to make the tests run on old machines, let's further
> > > > halve the object size when things look too tight.
> > > 
> > > That defeats the point of the test. The idea is to have the two objects
> > > that just don't fit, but only just.
> > 
> > i.e. the test is meant to show that the kernel heuristics for using
> > partial vma do not prevent the page-fault-of-doom.
> 
> So just skip then?

The kernel has a bug that partial vma was supposed to address. In theory
it can strike at any time (although userspace has a similar approach to
try and not tempt fate). And other than a getparm "bug fixed", there
won't be an ABI change for userspace to take note of.

I have ~237 patches ahead of my "make partial-vma actually work" fix. :|
-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: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Ville Syrjälä]

On Tue, Dec 15, 2015 at 09:57:22AM +0000, Chris Wilson wrote:
> On Tue, Dec 15, 2015 at 11:41:44AM +0200, Ville Syrjälä wrote:
> > On Mon, Dec 14, 2015 at 08:54:35PM +0000, Chris Wilson wrote:
> > > On Mon, Dec 14, 2015 at 08:49:38PM +0000, Chris Wilson wrote:
> > > > On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > Several factors conspire against us when trying to execute
> > > > > the tiled small-bo tests:
> > > > > - pre-gen4 require power of two fences, with natural alignment
> > > > > - the entire gtt may be mappable
> > > > > - we put a guard page at the end of gtt
> > > > > 
> > > > > What all that means is that when we try to use a tiled object half
> > > > > the size of the mappable area, we can only fit it in the first half
> > > > > of the gtt. That leads to a SIGBUS when we try to fault in the
> > > > > object when there's already something (eg. fbdev) occupying the
> > > > > first half of gtt.
> > > > > 
> > > > > So in order to make the tests run on old machines, let's further
> > > > > halve the object size when things look too tight.
> > > > 
> > > > That defeats the point of the test. The idea is to have the two objects
> > > > that just don't fit, but only just.
> > > 
> > > i.e. the test is meant to show that the kernel heuristics for using
> > > partial vma do not prevent the page-fault-of-doom.
> > 
> > So just skip then?
> 
> The kernel has a bug that partial vma was supposed to address.

I'm not sure partial vma has any benefit in this case. Sure, you could
them mmap the thing but unless we teach the GPU to also use partial
mappings it won't do any good since we just can't fit the thing into
the gtt.

> In theory
> it can strike at any time (although userspace has a similar approach to
> try and not tempt fate). And other than a getparm "bug fixed", there
> won't be an ABI change for userspace to take note of.
> 
> I have ~237 patches ahead of my "make partial-vma actually work" fix. :|
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Chris Wilson]

On Tue, Dec 15, 2015 at 12:57:15PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 15, 2015 at 09:57:22AM +0000, Chris Wilson wrote:
> > On Tue, Dec 15, 2015 at 11:41:44AM +0200, Ville Syrjälä wrote:
> > > On Mon, Dec 14, 2015 at 08:54:35PM +0000, Chris Wilson wrote:
> > > > On Mon, Dec 14, 2015 at 08:49:38PM +0000, Chris Wilson wrote:
> > > > > On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > 
> > > > > > Several factors conspire against us when trying to execute
> > > > > > the tiled small-bo tests:
> > > > > > - pre-gen4 require power of two fences, with natural alignment
> > > > > > - the entire gtt may be mappable
> > > > > > - we put a guard page at the end of gtt
> > > > > > 
> > > > > > What all that means is that when we try to use a tiled object half
> > > > > > the size of the mappable area, we can only fit it in the first half
> > > > > > of the gtt. That leads to a SIGBUS when we try to fault in the
> > > > > > object when there's already something (eg. fbdev) occupying the
> > > > > > first half of gtt.
> > > > > > 
> > > > > > So in order to make the tests run on old machines, let's further
> > > > > > halve the object size when things look too tight.
> > > > > 
> > > > > That defeats the point of the test. The idea is to have the two objects
> > > > > that just don't fit, but only just.
> > > > 
> > > > i.e. the test is meant to show that the kernel heuristics for using
> > > > partial vma do not prevent the page-fault-of-doom.
> > > 
> > > So just skip then?
> > 
> > The kernel has a bug that partial vma was supposed to address.
> 
> I'm not sure partial vma has any benefit in this case. Sure, you could
> them mmap the thing but unless we teach the GPU to also use partial
> mappings it won't do any good since we just can't fit the thing into
> the gtt.

For gen2, not being able to fit into mappable is an issue for the GPU as
well, sure. But for userspace being oblivious and *always* being able to
use mmaping of a bo, it is a big deal. (Being oblivious helps with
robustness in the stack, X/display-server-de-jour should not die just
because of a resource conflict - now X should catch the fault and handle
it, give or take bugs, avoiding that error path entirely is even better.)

Using "sparse" resources so that we can arbitrary sized objects is
something I'd like. But we have so much "whole-object-at-once" baked
into the code, it will be a challenge.
-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: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Ville Syrjälä]

On Tue, Dec 15, 2015 at 11:16:52AM +0000, Chris Wilson wrote:
> On Tue, Dec 15, 2015 at 12:57:15PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 15, 2015 at 09:57:22AM +0000, Chris Wilson wrote:
> > > On Tue, Dec 15, 2015 at 11:41:44AM +0200, Ville Syrjälä wrote:
> > > > On Mon, Dec 14, 2015 at 08:54:35PM +0000, Chris Wilson wrote:
> > > > > On Mon, Dec 14, 2015 at 08:49:38PM +0000, Chris Wilson wrote:
> > > > > > On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > 
> > > > > > > Several factors conspire against us when trying to execute
> > > > > > > the tiled small-bo tests:
> > > > > > > - pre-gen4 require power of two fences, with natural alignment
> > > > > > > - the entire gtt may be mappable
> > > > > > > - we put a guard page at the end of gtt
> > > > > > > 
> > > > > > > What all that means is that when we try to use a tiled object half
> > > > > > > the size of the mappable area, we can only fit it in the first half
> > > > > > > of the gtt. That leads to a SIGBUS when we try to fault in the
> > > > > > > object when there's already something (eg. fbdev) occupying the
> > > > > > > first half of gtt.
> > > > > > > 
> > > > > > > So in order to make the tests run on old machines, let's further
> > > > > > > halve the object size when things look too tight.
> > > > > > 
> > > > > > That defeats the point of the test. The idea is to have the two objects
> > > > > > that just don't fit, but only just.
> > > > > 
> > > > > i.e. the test is meant to show that the kernel heuristics for using
> > > > > partial vma do not prevent the page-fault-of-doom.
> > > > 
> > > > So just skip then?
> > > 
> > > The kernel has a bug that partial vma was supposed to address.
> > 
> > I'm not sure partial vma has any benefit in this case. Sure, you could
> > them mmap the thing but unless we teach the GPU to also use partial
> > mappings it won't do any good since we just can't fit the thing into
> > the gtt.
> 
> For gen2, not being able to fit into mappable is an issue for the GPU as
> well, sure. But for userspace being oblivious and *always* being able to
> use mmaping of a bo, it is a big deal. (Being oblivious helps with
> robustness in the stack, X/display-server-de-jour should not die just
> because of a resource conflict - now X should catch the fault and handle
> it, give or take bugs, avoiding that error path entirely is even better.)

Yeah, I suppose it would be nice not to SIGBUS even if userspace is
trying something a bit crazy. It would just get an error later from
execbuf when trying to use the bo with the GPU. So I guess the right
fix would be to fall back to partial vma if we fail to bind the full
vma.

> 
> Using "sparse" resources so that we can arbitrary sized objects is
> something I'd like. But we have so much "whole-object-at-once" baked
> into the code, it will be a challenge.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Ville Syrjälä]

On Tue, Dec 15, 2015 at 01:29:59PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 15, 2015 at 11:16:52AM +0000, Chris Wilson wrote:
> > On Tue, Dec 15, 2015 at 12:57:15PM +0200, Ville Syrjälä wrote:
> > > On Tue, Dec 15, 2015 at 09:57:22AM +0000, Chris Wilson wrote:
> > > > On Tue, Dec 15, 2015 at 11:41:44AM +0200, Ville Syrjälä wrote:
> > > > > On Mon, Dec 14, 2015 at 08:54:35PM +0000, Chris Wilson wrote:
> > > > > > On Mon, Dec 14, 2015 at 08:49:38PM +0000, Chris Wilson wrote:
> > > > > > > On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > 
> > > > > > > > Several factors conspire against us when trying to execute
> > > > > > > > the tiled small-bo tests:
> > > > > > > > - pre-gen4 require power of two fences, with natural alignment
> > > > > > > > - the entire gtt may be mappable
> > > > > > > > - we put a guard page at the end of gtt
> > > > > > > > 
> > > > > > > > What all that means is that when we try to use a tiled object half
> > > > > > > > the size of the mappable area, we can only fit it in the first half
> > > > > > > > of the gtt. That leads to a SIGBUS when we try to fault in the
> > > > > > > > object when there's already something (eg. fbdev) occupying the
> > > > > > > > first half of gtt.
> > > > > > > > 
> > > > > > > > So in order to make the tests run on old machines, let's further
> > > > > > > > halve the object size when things look too tight.
> > > > > > > 
> > > > > > > That defeats the point of the test. The idea is to have the two objects
> > > > > > > that just don't fit, but only just.
> > > > > > 
> > > > > > i.e. the test is meant to show that the kernel heuristics for using
> > > > > > partial vma do not prevent the page-fault-of-doom.
> > > > > 
> > > > > So just skip then?
> > > > 
> > > > The kernel has a bug that partial vma was supposed to address.
> > > 
> > > I'm not sure partial vma has any benefit in this case. Sure, you could
> > > them mmap the thing but unless we teach the GPU to also use partial
> > > mappings it won't do any good since we just can't fit the thing into
> > > the gtt.
> > 
> > For gen2, not being able to fit into mappable is an issue for the GPU as
> > well, sure. But for userspace being oblivious and *always* being able to
> > use mmaping of a bo, it is a big deal. (Being oblivious helps with
> > robustness in the stack, X/display-server-de-jour should not die just
> > because of a resource conflict - now X should catch the fault and handle
> > it, give or take bugs, avoiding that error path entirely is even better.)
> 
> Yeah, I suppose it would be nice not to SIGBUS even if userspace is
> trying something a bit crazy. It would just get an error later from
> execbuf when trying to use the bo with the GPU. So I guess the right
> fix would be to fall back to partial vma if we fail to bind the full
> vma.

Hmm, actaully no. This was about tiled objects, and we don't do
tiled+partial at all currently. Would need to teach the code to do that
first.

> 
> > 
> > Using "sparse" resources so that we can arbitrary sized objects is
> > something I'd like. But we have so much "whole-object-at-once" baked
> > into the code, it will be a challenge.
> > -Chris
> > 
> > -- 
> > Chris Wilson, Intel Open Source Technology Centre
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Chris Wilson]

On Tue, Dec 15, 2015 at 02:01:24PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 15, 2015 at 01:29:59PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 15, 2015 at 11:16:52AM +0000, Chris Wilson wrote:
> > > On Tue, Dec 15, 2015 at 12:57:15PM +0200, Ville Syrjälä wrote:
> > > > On Tue, Dec 15, 2015 at 09:57:22AM +0000, Chris Wilson wrote:
> > > > > On Tue, Dec 15, 2015 at 11:41:44AM +0200, Ville Syrjälä wrote:
> > > > > > On Mon, Dec 14, 2015 at 08:54:35PM +0000, Chris Wilson wrote:
> > > > > > > On Mon, Dec 14, 2015 at 08:49:38PM +0000, Chris Wilson wrote:
> > > > > > > > On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > > 
> > > > > > > > > Several factors conspire against us when trying to execute
> > > > > > > > > the tiled small-bo tests:
> > > > > > > > > - pre-gen4 require power of two fences, with natural alignment
> > > > > > > > > - the entire gtt may be mappable
> > > > > > > > > - we put a guard page at the end of gtt
> > > > > > > > > 
> > > > > > > > > What all that means is that when we try to use a tiled object half
> > > > > > > > > the size of the mappable area, we can only fit it in the first half
> > > > > > > > > of the gtt. That leads to a SIGBUS when we try to fault in the
> > > > > > > > > object when there's already something (eg. fbdev) occupying the
> > > > > > > > > first half of gtt.
> > > > > > > > > 
> > > > > > > > > So in order to make the tests run on old machines, let's further
> > > > > > > > > halve the object size when things look too tight.
> > > > > > > > 
> > > > > > > > That defeats the point of the test. The idea is to have the two objects
> > > > > > > > that just don't fit, but only just.
> > > > > > > 
> > > > > > > i.e. the test is meant to show that the kernel heuristics for using
> > > > > > > partial vma do not prevent the page-fault-of-doom.
> > > > > > 
> > > > > > So just skip then?
> > > > > 
> > > > > The kernel has a bug that partial vma was supposed to address.
> > > > 
> > > > I'm not sure partial vma has any benefit in this case. Sure, you could
> > > > them mmap the thing but unless we teach the GPU to also use partial
> > > > mappings it won't do any good since we just can't fit the thing into
> > > > the gtt.
> > > 
> > > For gen2, not being able to fit into mappable is an issue for the GPU as
> > > well, sure. But for userspace being oblivious and *always* being able to
> > > use mmaping of a bo, it is a big deal. (Being oblivious helps with
> > > robustness in the stack, X/display-server-de-jour should not die just
> > > because of a resource conflict - now X should catch the fault and handle
> > > it, give or take bugs, avoiding that error path entirely is even better.)
> > 
> > Yeah, I suppose it would be nice not to SIGBUS even if userspace is
> > trying something a bit crazy. It would just get an error later from
> > execbuf when trying to use the bo with the GPU. So I guess the right
> > fix would be to fall back to partial vma if we fail to bind the full
> > vma.
> 
> Hmm, actaully no. This was about tiled objects, and we don't do
> tiled+partial at all currently. Would need to teach the code to do that
> first.

It's is one and the same fix ;) (Ok, same patchset!)
-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: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Daniel Vetter]

On Tue, Dec 15, 2015 at 12:30:54PM +0000, Chris Wilson wrote:
> On Tue, Dec 15, 2015 at 02:01:24PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 15, 2015 at 01:29:59PM +0200, Ville Syrjälä wrote:
> > > On Tue, Dec 15, 2015 at 11:16:52AM +0000, Chris Wilson wrote:
> > > > On Tue, Dec 15, 2015 at 12:57:15PM +0200, Ville Syrjälä wrote:
> > > > > On Tue, Dec 15, 2015 at 09:57:22AM +0000, Chris Wilson wrote:
> > > > > > On Tue, Dec 15, 2015 at 11:41:44AM +0200, Ville Syrjälä wrote:
> > > > > > > On Mon, Dec 14, 2015 at 08:54:35PM +0000, Chris Wilson wrote:
> > > > > > > > On Mon, Dec 14, 2015 at 08:49:38PM +0000, Chris Wilson wrote:
> > > > > > > > > On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > > > 
> > > > > > > > > > Several factors conspire against us when trying to execute
> > > > > > > > > > the tiled small-bo tests:
> > > > > > > > > > - pre-gen4 require power of two fences, with natural alignment
> > > > > > > > > > - the entire gtt may be mappable
> > > > > > > > > > - we put a guard page at the end of gtt
> > > > > > > > > > 
> > > > > > > > > > What all that means is that when we try to use a tiled object half
> > > > > > > > > > the size of the mappable area, we can only fit it in the first half
> > > > > > > > > > of the gtt. That leads to a SIGBUS when we try to fault in the
> > > > > > > > > > object when there's already something (eg. fbdev) occupying the
> > > > > > > > > > first half of gtt.
> > > > > > > > > > 
> > > > > > > > > > So in order to make the tests run on old machines, let's further
> > > > > > > > > > halve the object size when things look too tight.
> > > > > > > > > 
> > > > > > > > > That defeats the point of the test. The idea is to have the two objects
> > > > > > > > > that just don't fit, but only just.
> > > > > > > > 
> > > > > > > > i.e. the test is meant to show that the kernel heuristics for using
> > > > > > > > partial vma do not prevent the page-fault-of-doom.
> > > > > > > 
> > > > > > > So just skip then?
> > > > > > 
> > > > > > The kernel has a bug that partial vma was supposed to address.
> > > > > 
> > > > > I'm not sure partial vma has any benefit in this case. Sure, you could
> > > > > them mmap the thing but unless we teach the GPU to also use partial
> > > > > mappings it won't do any good since we just can't fit the thing into
> > > > > the gtt.
> > > > 
> > > > For gen2, not being able to fit into mappable is an issue for the GPU as
> > > > well, sure. But for userspace being oblivious and *always* being able to
> > > > use mmaping of a bo, it is a big deal. (Being oblivious helps with
> > > > robustness in the stack, X/display-server-de-jour should not die just
> > > > because of a resource conflict - now X should catch the fault and handle
> > > > it, give or take bugs, avoiding that error path entirely is even better.)
> > > 
> > > Yeah, I suppose it would be nice not to SIGBUS even if userspace is
> > > trying something a bit crazy. It would just get an error later from
> > > execbuf when trying to use the bo with the GPU. So I guess the right
> > > fix would be to fall back to partial vma if we fail to bind the full
> > > vma.
> > 
> > Hmm, actaully no. This was about tiled objects, and we don't do
> > tiled+partial at all currently. Would need to teach the code to do that
> > first.
> 
> It's is one and the same fix ;) (Ok, same patchset!)

Yeah the problem is that partial views are stuck half-implemented, and
seem to not move forward.

I'd just skip on gen2/3 tbh.
-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: [PATCH i-g-t 4/7] tests/gem_mmap_gtt: Make the small-bo tiling tests work on old platforms

[Chris Wilson]

On Wed, Dec 16, 2015 at 11:46:50AM +0100, Daniel Vetter wrote:
> On Tue, Dec 15, 2015 at 12:30:54PM +0000, Chris Wilson wrote:
> > On Tue, Dec 15, 2015 at 02:01:24PM +0200, Ville Syrjälä wrote:
> > > On Tue, Dec 15, 2015 at 01:29:59PM +0200, Ville Syrjälä wrote:
> > > > On Tue, Dec 15, 2015 at 11:16:52AM +0000, Chris Wilson wrote:
> > > > > On Tue, Dec 15, 2015 at 12:57:15PM +0200, Ville Syrjälä wrote:
> > > > > > On Tue, Dec 15, 2015 at 09:57:22AM +0000, Chris Wilson wrote:
> > > > > > > On Tue, Dec 15, 2015 at 11:41:44AM +0200, Ville Syrjälä wrote:
> > > > > > > > On Mon, Dec 14, 2015 at 08:54:35PM +0000, Chris Wilson wrote:
> > > > > > > > > On Mon, Dec 14, 2015 at 08:49:38PM +0000, Chris Wilson wrote:
> > > > > > > > > > On Mon, Dec 14, 2015 at 10:15:53PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > > > > > 
> > > > > > > > > > > Several factors conspire against us when trying to execute
> > > > > > > > > > > the tiled small-bo tests:
> > > > > > > > > > > - pre-gen4 require power of two fences, with natural alignment
> > > > > > > > > > > - the entire gtt may be mappable
> > > > > > > > > > > - we put a guard page at the end of gtt
> > > > > > > > > > > 
> > > > > > > > > > > What all that means is that when we try to use a tiled object half
> > > > > > > > > > > the size of the mappable area, we can only fit it in the first half
> > > > > > > > > > > of the gtt. That leads to a SIGBUS when we try to fault in the
> > > > > > > > > > > object when there's already something (eg. fbdev) occupying the
> > > > > > > > > > > first half of gtt.
> > > > > > > > > > > 
> > > > > > > > > > > So in order to make the tests run on old machines, let's further
> > > > > > > > > > > halve the object size when things look too tight.
> > > > > > > > > > 
> > > > > > > > > > That defeats the point of the test. The idea is to have the two objects
> > > > > > > > > > that just don't fit, but only just.
> > > > > > > > > 
> > > > > > > > > i.e. the test is meant to show that the kernel heuristics for using
> > > > > > > > > partial vma do not prevent the page-fault-of-doom.
> > > > > > > > 
> > > > > > > > So just skip then?
> > > > > > > 
> > > > > > > The kernel has a bug that partial vma was supposed to address.
> > > > > > 
> > > > > > I'm not sure partial vma has any benefit in this case. Sure, you could
> > > > > > them mmap the thing but unless we teach the GPU to also use partial
> > > > > > mappings it won't do any good since we just can't fit the thing into
> > > > > > the gtt.
> > > > > 
> > > > > For gen2, not being able to fit into mappable is an issue for the GPU as
> > > > > well, sure. But for userspace being oblivious and *always* being able to
> > > > > use mmaping of a bo, it is a big deal. (Being oblivious helps with
> > > > > robustness in the stack, X/display-server-de-jour should not die just
> > > > > because of a resource conflict - now X should catch the fault and handle
> > > > > it, give or take bugs, avoiding that error path entirely is even better.)
> > > > 
> > > > Yeah, I suppose it would be nice not to SIGBUS even if userspace is
> > > > trying something a bit crazy. It would just get an error later from
> > > > execbuf when trying to use the bo with the GPU. So I guess the right
> > > > fix would be to fall back to partial vma if we fail to bind the full
> > > > vma.
> > > 
> > > Hmm, actaully no. This was about tiled objects, and we don't do
> > > tiled+partial at all currently. Would need to teach the code to do that
> > > first.
> > 
> > It's is one and the same fix ;) (Ok, same patchset!)
> 
> Yeah the problem is that partial views are stuck half-implemented, and
> seem to not move forward.
> 
> I'd just skip on gen2/3 tbh.

Oh, this is small-bo. Sorry for the digression. medium-bo is to exploit
the page-fault-of-doom.  small-bo is the one that is meant to always
work.

In that context, reducing the size of the small bo (just not to a single
page!) would be fine. And tiny-bo can be one page!
-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

[PATCH i-g-t 5/7] lib/debugfs: Read the crc frame counter as hex

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

With the kernel fixed to dump out the crc frame counts in hex,
we must follow suit.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 lib/igt_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 2c3b1cfe2370..5e71f50d7326 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -484,7 +484,7 @@ static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
 	int n;
 
 	crc->n_words = 5;
-	n = sscanf(line, "%8u %8x %8x %8x %8x %8x", &crc->frame, &crc->crc[0],
+	n = sscanf(line, "%8x %8x %8x %8x %8x %8x", &crc->frame, &crc->crc[0],
 		   &crc->crc[1], &crc->crc[2], &crc->crc[3], &crc->crc[4]);
 	return n == 6;
 }
-- 
2.4.10

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

Re: [PATCH i-g-t 5/7] lib/debugfs: Read the crc frame counter as hex

[Morton, Derek J]

>
>
>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of ville.syrjala@linux.intel.com
>Sent: Monday, December 14, 2015 8:16 PM
>To: intel-gfx@lists.freedesktop.org
>Subject: [Intel-gfx] [PATCH i-g-t 5/7] lib/debugfs: Read the crc frame counter as hex
>
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>With the kernel fixed to dump out the crc frame counts in hex, we must follow suit.
>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> lib/igt_debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index 2c3b1cfe2370..5e71f50d7326 100644
>--- a/lib/igt_debugfs.c
>+++ b/lib/igt_debugfs.c
>@@ -484,7 +484,7 @@ static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
> 	int n;
> 
> 	crc->n_words = 5;
>-	n = sscanf(line, "%8u %8x %8x %8x %8x %8x", &crc->frame, &crc->crc[0],
>+	n = sscanf(line, "%8x %8x %8x %8x %8x %8x", &crc->frame, &crc->crc[0],

What will happen for kernels that have not been 'fixed'? Android is always behind drm_nightly. Is there any way of knowing whether this value is in hex or decimal and read it accordingly? What tests will be affected if this is wrong?

//Derek 

> 		   &crc->crc[1], &crc->crc[2], &crc->crc[3], &crc->crc[4]);
> 	return n == 6;
> }
>--
>2.4.10
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 5/7] lib/debugfs: Read the crc frame counter as hex

[Ville Syrjälä]

On Tue, Dec 15, 2015 at 12:03:11PM +0000, Morton, Derek J wrote:
> >
> >
> >-----Original Message-----
> >From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of ville.syrjala@linux.intel.com
> >Sent: Monday, December 14, 2015 8:16 PM
> >To: intel-gfx@lists.freedesktop.org
> >Subject: [Intel-gfx] [PATCH i-g-t 5/7] lib/debugfs: Read the crc frame counter as hex
> >
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >With the kernel fixed to dump out the crc frame counts in hex, we must follow suit.
> >
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> > lib/igt_debugfs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index 2c3b1cfe2370..5e71f50d7326 100644
> >--- a/lib/igt_debugfs.c
> >+++ b/lib/igt_debugfs.c
> >@@ -484,7 +484,7 @@ static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
> > 	int n;
> > 
> > 	crc->n_words = 5;
> >-	n = sscanf(line, "%8u %8x %8x %8x %8x %8x", &crc->frame, &crc->crc[0],
> >+	n = sscanf(line, "%8x %8x %8x %8x %8x %8x", &crc->frame, &crc->crc[0],
> 
> What will happen for kernels that have not been 'fixed'? Android is always behind drm_nightly. Is there any way of knowing whether this value is in hex or decimal and read it accordingly? What tests will be affected if this is wrong?

You might know if it's hex if the number happes to have a-f in it.
Othwerwise you can't tell since we don't use a 0x prefix.

Another option is to fix the size assumptions about the string on
both kernel and igt side.

And a third option would be to have the kernel return 'count & 0xffffff'
which would fit in the 8 characters allotted. igt wouldn't need to be
changed in this case. That might actually make some sense since on
gen3/4 the hw frame counter is only 24 bits. If userspace would have
to deal with wraparound it could just assume that it occurs at 2^24.
Actually now that I think about the current tests are probably broken
if the counter wraps sooner than 2^32. But since the interface can't
even carry such large frame counter numbers the test will simply fail
in other ways until the kernel gets fixed.

> 
> //Derek 
> 
> > 		   &crc->crc[1], &crc->crc[2], &crc->crc[3], &crc->crc[4]);
> > 	return n == 6;
> > }
> >--
> >2.4.10
> >
> >_______________________________________________
> >Intel-gfx mailing list
> >Intel-gfx@lists.freedesktop.org
> >http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 6/7] tests/kms_flip: Increase TEST_TS_CONT max seq difference to 150

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

During suspend tests we can exceed the current 100 frame difference
in sequence numbers. Bump the limit to 150 frames.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_flip.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 750a1882dc3c..0dae83046b20 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -674,9 +674,9 @@ static void check_state(struct test_output *o, struct event_state *es)
 		igt_assert_f(es->current_seq - es->last_seq >= 0,
 			     "unexpected %s seq %u, should be >= %u\n",
 			     es->name, es->current_seq, es->last_seq);
-		igt_assert_f(es->current_seq - es->last_seq <= 100,
+		igt_assert_f(es->current_seq - es->last_seq <= 150,
 			     "unexpected %s seq %u, should be < %u\n",
-			     es->name, es->current_seq, es->last_seq + 100);
+			     es->name, es->current_seq, es->last_seq + 150);
 
 		igt_debug("testing ts continuity: Current frame %u, old frame %u\n",
 			  es->current_seq, es->last_seq);
-- 
2.4.10

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

Re: [PATCH i-g-t 6/7] tests/kms_flip: Increase TEST_TS_CONT max seq difference to 150

[Daniel Vetter]

On Mon, Dec 14, 2015 at 10:15:55PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> During suspend tests we can exceed the current 100 frame difference
> in sequence numbers. Bump the limit to 150 frames.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I'd bump a lot more tbh, or maybe check for system suspend/resume and then
bump to 1000, otherwise leave at 100.
-Daniel

> ---
>  tests/kms_flip.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_flip.c b/tests/kms_flip.c
> index 750a1882dc3c..0dae83046b20 100644
> --- a/tests/kms_flip.c
> +++ b/tests/kms_flip.c
> @@ -674,9 +674,9 @@ static void check_state(struct test_output *o, struct event_state *es)
>  		igt_assert_f(es->current_seq - es->last_seq >= 0,
>  			     "unexpected %s seq %u, should be >= %u\n",
>  			     es->name, es->current_seq, es->last_seq);
> -		igt_assert_f(es->current_seq - es->last_seq <= 100,
> +		igt_assert_f(es->current_seq - es->last_seq <= 150,
>  			     "unexpected %s seq %u, should be < %u\n",
> -			     es->name, es->current_seq, es->last_seq + 100);
> +			     es->name, es->current_seq, es->last_seq + 150);
>  
>  		igt_debug("testing ts continuity: Current frame %u, old frame %u\n",
>  			  es->current_seq, es->last_seq);
> -- 
> 2.4.10
> 
> _______________________________________________
> 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

[PATCH i-g-t 7/7] tests/kms_pipe_crc_basic: Use igt_assert_eq() to see the failing frame counts

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use igt_assert_eq() to compare the frame numbers during the frame
sequence tests so that we'll see exactly what the bad frame counts
are when the test fails.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_pipe_crc_basic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index a3292c225203..bfb14780fa72 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -176,7 +176,7 @@ test_read_crc_for_output(data_t *data, int pipe, igt_output_t *output,
 
 		if (flags & TEST_SEQUENCE)
 			for (j = 0; j < (N_CRCS - 1); j++)
-				igt_assert(crcs[j].frame + 1 == crcs[j + 1].frame);
+				igt_assert_eq(crcs[j].frame + 1, crcs[j + 1].frame);
 
 		free(crcs);
 		igt_pipe_crc_free(pipe_crc);
-- 
2.4.10

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