| Message ID | 1450124156-12679-2-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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.
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
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
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
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
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);