Re: [Intel-gfx] [PATCH i-g-t] tests/gem_mmap_gtt: Use PAGE_SIZE instead of hard coded value

2015-05-21 Thread Jesse Barnes 
On 05/04/2015 07:16 AM, Daniel Vetter wrote:
 On Thu, Apr 30, 2015 at 12:22:56PM +0100, Chris Wilson wrote:
 On Thu, Apr 30, 2015 at 01:28:46PM +0300, Joonas Lahtinen wrote:
 On ma, 2015-04-27 at 20:43 +0100, Chris Wilson wrote:
 On Mon, Apr 27, 2015 at 06:35:54PM +0100, Thomas Wood wrote:
 On 24 April 2015 at 08:38, Joonas Lahtinen
 joonas.lahti...@linux.intel.com wrote:
 Now that there is PAGE_SIZE define, use it.

 Thanks, I've pushed this patch. I also noticed PAGE_SIZE gets defined
 in several tests, so at some point it might be worth moving it into
 the library.

 Be wary of these though. PAGE_SIZE should only ever be used wrt to struct
 page and not GPU pages. If you must, please use GTT_PAGE_SIZE instead.

 Do we have a platform/case where these are different? Just asking out of
 curiosity :)

 Yes. We just haven't enabled big pages yet. The thought of getting globs
 of 64k contiguous physical memory isn't too appealing, but like with
 hugepages there are likely enough tasks that benefit.
 
 I thought the verdict thus far was that hw engineers overspecced tlbs and
 64k pages aren't really worth it except in some corner-case video code
 workloads. Might have changed with the gen8+ pagetables, but I haven't
 seen any new noises about this.

I hadn't heard that; Damien looked at this awhile back but I'm not sure
if he got to the point of getting perf numbers.  Those would be nice...
 there's a lot of added complexity, but if our media processing overhead
goes down by 20% it would probably be worth it!

Jesse

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

Re: [Intel-gfx] [PATCH i-g-t] tests/gem_mmap_gtt: Use PAGE_SIZE instead of hard coded value

2015-05-04 Thread Daniel Vetter 
On Thu, Apr 30, 2015 at 12:22:56PM +0100, Chris Wilson wrote:
 On Thu, Apr 30, 2015 at 01:28:46PM +0300, Joonas Lahtinen wrote:
  On ma, 2015-04-27 at 20:43 +0100, Chris Wilson wrote:
   On Mon, Apr 27, 2015 at 06:35:54PM +0100, Thomas Wood wrote:
On 24 April 2015 at 08:38, Joonas Lahtinen
joonas.lahti...@linux.intel.com wrote:
 Now that there is PAGE_SIZE define, use it.

Thanks, I've pushed this patch. I also noticed PAGE_SIZE gets defined
in several tests, so at some point it might be worth moving it into
the library.
   
   Be wary of these though. PAGE_SIZE should only ever be used wrt to struct
   page and not GPU pages. If you must, please use GTT_PAGE_SIZE instead.
  
  Do we have a platform/case where these are different? Just asking out of
  curiosity :)
 
 Yes. We just haven't enabled big pages yet. The thought of getting globs
 of 64k contiguous physical memory isn't too appealing, but like with
 hugepages there are likely enough tasks that benefit.

I thought the verdict thus far was that hw engineers overspecced tlbs and
64k pages aren't really worth it except in some corner-case video code
workloads. Might have changed with the gen8+ pagetables, but I haven't
seen any new noises about this.
-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 i-g-t] tests/gem_mmap_gtt: Use PAGE_SIZE instead of hard coded value

2015-04-30 Thread Joonas Lahtinen 
On ma, 2015-04-27 at 20:43 +0100, Chris Wilson wrote:
 On Mon, Apr 27, 2015 at 06:35:54PM +0100, Thomas Wood wrote:
  On 24 April 2015 at 08:38, Joonas Lahtinen
  joonas.lahti...@linux.intel.com wrote:
   Now that there is PAGE_SIZE define, use it.
  
  Thanks, I've pushed this patch. I also noticed PAGE_SIZE gets defined
  in several tests, so at some point it might be worth moving it into
  the library.
 
 Be wary of these though. PAGE_SIZE should only ever be used wrt to struct
 page and not GPU pages. If you must, please use GTT_PAGE_SIZE instead.

Do we have a platform/case where these are different? Just asking out of
curiosity :)

And the use above was to a mmaped area, which I think should be
considered to behave like CPU paged memory? Otherwise kind of defeats
mmap purpose.

Regards, Joonas

 -Chris
 


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

Re: [Intel-gfx] [PATCH i-g-t] tests/gem_mmap_gtt: Use PAGE_SIZE instead of hard coded value

2015-04-30 Thread Chris Wilson 
On Thu, Apr 30, 2015 at 01:28:46PM +0300, Joonas Lahtinen wrote:
 On ma, 2015-04-27 at 20:43 +0100, Chris Wilson wrote:
  On Mon, Apr 27, 2015 at 06:35:54PM +0100, Thomas Wood wrote:
   On 24 April 2015 at 08:38, Joonas Lahtinen
   joonas.lahti...@linux.intel.com wrote:
Now that there is PAGE_SIZE define, use it.
   
   Thanks, I've pushed this patch. I also noticed PAGE_SIZE gets defined
   in several tests, so at some point it might be worth moving it into
   the library.
  
  Be wary of these though. PAGE_SIZE should only ever be used wrt to struct
  page and not GPU pages. If you must, please use GTT_PAGE_SIZE instead.
 
 Do we have a platform/case where these are different? Just asking out of
 curiosity :)

Yes. We just haven't enabled big pages yet. The thought of getting globs
of 64k contiguous physical memory isn't too appealing, but like with
hugepages there are likely enough tasks that benefit.
 
 And the use above was to a mmaped area, which I think should be
 considered to behave like CPU paged memory? Otherwise kind of defeats
 mmap purpose.

I was making the observation just in case someone wanted to go through
the whole code base fixing up the hardcoded 4096 constants.
-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 i-g-t] tests/gem_mmap_gtt: Use PAGE_SIZE instead of hard coded value

2015-04-27 Thread Chris Wilson 
On Mon, Apr 27, 2015 at 06:35:54PM +0100, Thomas Wood wrote:
 On 24 April 2015 at 08:38, Joonas Lahtinen
 joonas.lahti...@linux.intel.com wrote:
  Now that there is PAGE_SIZE define, use it.
 
 Thanks, I've pushed this patch. I also noticed PAGE_SIZE gets defined
 in several tests, so at some point it might be worth moving it into
 the library.

Be wary of these though. PAGE_SIZE should only ever be used wrt to struct
page and not GPU pages. If you must, please use GTT_PAGE_SIZE instead.
-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 i-g-t] tests/gem_mmap_gtt: Use PAGE_SIZE instead of hard coded value

2015-04-27 Thread Thomas Wood 
On 24 April 2015 at 08:38, Joonas Lahtinen
joonas.lahti...@linux.intel.com wrote:
 Now that there is PAGE_SIZE define, use it.

Thanks, I've pushed this patch. I also noticed PAGE_SIZE gets defined
in several tests, so at some point it might be worth moving it into
the library.



 Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com
 ---
  tests/gem_mmap_gtt.c | 15 ---
  1 file changed, 8 insertions(+), 7 deletions(-)

 diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
 index e80f52e..92fa644 100644
 --- a/tests/gem_mmap_gtt.c
 +++ b/tests/gem_mmap_gtt.c
 @@ -148,24 +148,25 @@ test_short(int fd)
 igt_assert(drmIoctl(fd,
 DRM_IOCTL_I915_GEM_MMAP_GTT,
 mmap_arg) == 0);
 -   for (pages = 1; pages = OBJECT_SIZE / 4096; pages = 1) {
 +   for (pages = 1; pages = OBJECT_SIZE / PAGE_SIZE; pages = 1) {
 uint8_t *r, *w;

 -   w = mmap64(0, pages * 4096, PROT_READ | PROT_WRITE,
 +   w = mmap64(0, pages * PAGE_SIZE, PROT_READ | PROT_WRITE,
MAP_SHARED, fd, mmap_arg.offset);
 igt_assert(w != MAP_FAILED);

 -   r = mmap64(0, pages * 4096, PROT_READ,
 +   r = mmap64(0, pages * PAGE_SIZE, PROT_READ,
MAP_SHARED, fd, mmap_arg.offset);
 igt_assert(r != MAP_FAILED);

 for (p = 0; p  pages; p++) {
 -   w[4096*p] = r[4096*p];
 -   w[4096*p+4095] = r[4096*p+4095];
 +   w[p*PAGE_SIZE] = r[p*PAGE_SIZE];
 +   w[p*PAGE_SIZE+(PAGE_SIZE-1)] =
 +   r[p*PAGE_SIZE+(PAGE_SIZE-1)];
 }

 -   munmap(r, pages * 4096);
 -   munmap(w, pages * 4096);
 +   munmap(r, pages * PAGE_SIZE);
 +   munmap(w, pages * PAGE_SIZE);
 }
 gem_close(fd, mmap_arg.handle);
  }
 --
 1.9.3



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

[Intel-gfx] [PATCH i-g-t] tests/gem_mmap_gtt: Use PAGE_SIZE instead of hard coded value

2015-04-24 Thread Joonas Lahtinen 
Now that there is PAGE_SIZE define, use it.

Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com
---
 tests/gem_mmap_gtt.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
index e80f52e..92fa644 100644
--- a/tests/gem_mmap_gtt.c
+++ b/tests/gem_mmap_gtt.c
@@ -148,24 +148,25 @@ test_short(int fd)
igt_assert(drmIoctl(fd,
DRM_IOCTL_I915_GEM_MMAP_GTT,
mmap_arg) == 0);
-   for (pages = 1; pages = OBJECT_SIZE / 4096; pages = 1) {
+   for (pages = 1; pages = OBJECT_SIZE / PAGE_SIZE; pages = 1) {
uint8_t *r, *w;
 
-   w = mmap64(0, pages * 4096, PROT_READ | PROT_WRITE,
+   w = mmap64(0, pages * PAGE_SIZE, PROT_READ | PROT_WRITE,
   MAP_SHARED, fd, mmap_arg.offset);
igt_assert(w != MAP_FAILED);
 
-   r = mmap64(0, pages * 4096, PROT_READ,
+   r = mmap64(0, pages * PAGE_SIZE, PROT_READ,
   MAP_SHARED, fd, mmap_arg.offset);
igt_assert(r != MAP_FAILED);
 
for (p = 0; p  pages; p++) {
-   w[4096*p] = r[4096*p];
-   w[4096*p+4095] = r[4096*p+4095];
+   w[p*PAGE_SIZE] = r[p*PAGE_SIZE];
+   w[p*PAGE_SIZE+(PAGE_SIZE-1)] =
+   r[p*PAGE_SIZE+(PAGE_SIZE-1)];
}
 
-   munmap(r, pages * 4096);
-   munmap(w, pages * 4096);
+   munmap(r, pages * PAGE_SIZE);
+   munmap(w, pages * PAGE_SIZE);
}
gem_close(fd, mmap_arg.handle);
 }
-- 
1.9.3



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