Re: [Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-05-11 Thread Daniel Vetter
On Mon, May 11, 2015 at 12:17:09AM +0530, Ankitprasad Sharma wrote:
 On Fri, 2015-05-08 at 09:16 +0200, Daniel Vetter wrote:
  On Fri, May 08, 2015 at 10:54:26AM +0530, Ankitprasad Sharma wrote:
   On Thu, 2015-05-07 at 08:52 +0200, Daniel Vetter wrote:
On Wed, May 06, 2015 at 03:51:52PM +0530, 
ankitprasad.r.sha...@intel.com wrote:
 From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
 
 This patch adds the testcases for verifying the new extended
 gem_create ioctl. By means of this extended ioctl, memory
 placement of the GEM object can be specified, i.e. either
 shmem or stolen memory.
 These testcases include functional tests and interface tests for
 testing the gem_create ioctl call for stolen memory placement
 
 v2: Testing pread/pwrite functionality for stolen backed objects,
 added local struct for extended gem_create and gem_get_aperture,
 until headers catch up (Chris)
 
 v3: Removed get_aperture related functions, extended gem_pread
 to compare speeds for user pages with and without page faults,
 unexposed local_gem_create struct, changed gem_create_stolen
 usage (Chris)
 
 Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com

An igt to check for invalid arguments of the gem create ioctl 
(especially
the newly added flags parameters) seems to be missing.
-Daniel
   
   \Wwe already have a test to check invalid arguments for the newly added
   flags parameter in the current set of tests.
   
   static void invalid_flag_test(int fd)
  
  Oh right I totally missed that. Especially for future extension I think
  Chris' idea to split up tests sounds really good, i.e.
  
  gem_create/invalid-flags (the one testcase I didn't spot)
  gem_stolen/all the other tests
  
  Otherwise the next person extending gem_create will miss your
  invalid-flags test for it.
  Thanks, Daniel
 
 As I see, to validate the parameters for gem_create ioctl there needs to
 be verification for size too, other than the newly added parameter
 flags. 
 I will create new igt for validating the parameters of gem_create ioctl.
 (i.e. invalid size and flags)
 Can you please suggest any other tests that can be added to the
 gem_create igt?

Besides testing a few invalid sizes (0, not page-aligned) and ensuring
invalid flags get rejected I don't think we need more gem_create ioctl
input validation tests.
-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] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-05-10 Thread Ankitprasad Sharma
On Fri, 2015-05-08 at 09:16 +0200, Daniel Vetter wrote:
 On Fri, May 08, 2015 at 10:54:26AM +0530, Ankitprasad Sharma wrote:
  On Thu, 2015-05-07 at 08:52 +0200, Daniel Vetter wrote:
   On Wed, May 06, 2015 at 03:51:52PM +0530, ankitprasad.r.sha...@intel.com 
   wrote:
From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com

This patch adds the testcases for verifying the new extended
gem_create ioctl. By means of this extended ioctl, memory
placement of the GEM object can be specified, i.e. either
shmem or stolen memory.
These testcases include functional tests and interface tests for
testing the gem_create ioctl call for stolen memory placement

v2: Testing pread/pwrite functionality for stolen backed objects,
added local struct for extended gem_create and gem_get_aperture,
until headers catch up (Chris)

v3: Removed get_aperture related functions, extended gem_pread
to compare speeds for user pages with and without page faults,
unexposed local_gem_create struct, changed gem_create_stolen
usage (Chris)

Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
   
   An igt to check for invalid arguments of the gem create ioctl (especially
   the newly added flags parameters) seems to be missing.
   -Daniel
  
  \Wwe already have a test to check invalid arguments for the newly added
  flags parameter in the current set of tests.
  
  static void invalid_flag_test(int fd)
 
 Oh right I totally missed that. Especially for future extension I think
 Chris' idea to split up tests sounds really good, i.e.
 
 gem_create/invalid-flags (the one testcase I didn't spot)
 gem_stolen/all the other tests
 
 Otherwise the next person extending gem_create will miss your
 invalid-flags test for it.
 Thanks, Daniel

As I see, to validate the parameters for gem_create ioctl there needs to
be verification for size too, other than the newly added parameter
flags. 
I will create new igt for validating the parameters of gem_create ioctl.
(i.e. invalid size and flags)
Can you please suggest any other tests that can be added to the
gem_create igt?

Thanks,
Ankit

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


Re: [Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-05-08 Thread Daniel Vetter
On Fri, May 08, 2015 at 10:54:26AM +0530, Ankitprasad Sharma wrote:
 On Thu, 2015-05-07 at 08:52 +0200, Daniel Vetter wrote:
  On Wed, May 06, 2015 at 03:51:52PM +0530, ankitprasad.r.sha...@intel.com 
  wrote:
   From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
   
   This patch adds the testcases for verifying the new extended
   gem_create ioctl. By means of this extended ioctl, memory
   placement of the GEM object can be specified, i.e. either
   shmem or stolen memory.
   These testcases include functional tests and interface tests for
   testing the gem_create ioctl call for stolen memory placement
   
   v2: Testing pread/pwrite functionality for stolen backed objects,
   added local struct for extended gem_create and gem_get_aperture,
   until headers catch up (Chris)
   
   v3: Removed get_aperture related functions, extended gem_pread
   to compare speeds for user pages with and without page faults,
   unexposed local_gem_create struct, changed gem_create_stolen
   usage (Chris)
   
   Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
  
  An igt to check for invalid arguments of the gem create ioctl (especially
  the newly added flags parameters) seems to be missing.
  -Daniel
 
 \Wwe already have a test to check invalid arguments for the newly added
 flags parameter in the current set of tests.
 
 static void invalid_flag_test(int fd)

Oh right I totally missed that. Especially for future extension I think
Chris' idea to split up tests sounds really good, i.e.

gem_create/invalid-flags (the one testcase I didn't spot)
gem_stolen/all the other tests

Otherwise the next person extending gem_create will miss your
invalid-flags test for it.
Thanks, 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] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-05-07 Thread Daniel Vetter
On Wed, May 06, 2015 at 03:51:52PM +0530, ankitprasad.r.sha...@intel.com wrote:
 From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
 
 This patch adds the testcases for verifying the new extended
 gem_create ioctl. By means of this extended ioctl, memory
 placement of the GEM object can be specified, i.e. either
 shmem or stolen memory.
 These testcases include functional tests and interface tests for
 testing the gem_create ioctl call for stolen memory placement
 
 v2: Testing pread/pwrite functionality for stolen backed objects,
 added local struct for extended gem_create and gem_get_aperture,
 until headers catch up (Chris)
 
 v3: Removed get_aperture related functions, extended gem_pread
 to compare speeds for user pages with and without page faults,
 unexposed local_gem_create struct, changed gem_create_stolen
 usage (Chris)
 
 Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com

An igt to check for invalid arguments of the gem create ioctl (especially
the newly added flags parameters) seems to be missing.
-Daniel

 ---
  lib/ioctl_wrappers.c  |  60 +++
  lib/ioctl_wrappers.h  |  12 ++
  tests/Makefile.sources|   1 +
  tests/gem_create_stolen.c | 387 
 ++
  tests/gem_pread.c | 100 
  tests/gem_pwrite.c|  45 ++
  6 files changed, 605 insertions(+)
  create mode 100644 tests/gem_create_stolen.c
 
 diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
 index ff78ef1..2bd341a 100644
 --- a/lib/ioctl_wrappers.c
 +++ b/lib/ioctl_wrappers.c
 @@ -379,6 +379,66 @@ void gem_sync(int fd, uint32_t handle)
   gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
  }
  
 +bool gem_create__has_stolen_support(int fd)
 +{
 + static int has_stolen_support = -1;
 + struct drm_i915_getparam gp;
 + int val = -1;
 +
 + return 1;
 + if (has_stolen_support  0) {
 + memset(gp, 0, sizeof(gp));
 + gp.param = 35; /* CREATE_VERSION */
 + gp.value = val;
 +
 + /* Do we have the extended gem_create_ioctl? */
 + ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp);
 + has_stolen_support = val = 1;
 + }
 +
 + return has_stolen_support;
 +}
 +
 +struct local_i915_gem_create_v2 {
 + uint64_t size;
 + uint32_t handle;
 + uint32_t pad;
 +#define I915_CREATE_PLACEMENT_STOLEN (10)
 + uint32_t flags;
 +};
 +
 +#define LOCAL_IOCTL_I915_GEM_CREATE   DRM_IOWR(DRM_COMMAND_BASE + 
 DRM_I915_GEM_CREATE, struct local_i915_gem_create_v2)
 +/**
 + * gem_create_stolen:
 + * @fd: open i915 drm file descriptor
 + * @size: desired size of the buffer
 + * @flags: desired placement i.e. stolen or shmem
 + *
 + * This wraps the new GEM_CREATE ioctl, which allocates a
 + * new gem buffer object of @size and placement based on @flags.
 + *
 + * Returns: The file-private handle of the created buffer object
 + */
 +
 +uint32_t gem_create_stolen(int fd, int size)
 +{
 + struct local_i915_gem_create_v2 create;
 + int ret;
 +
 + memset(create, 0, sizeof(create));
 + create.handle = 0;
 + create.size = size;
 + create.flags = I915_CREATE_PLACEMENT_STOLEN;
 + ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CREATE, create);
 +
 + if (ret  0)
 + return 0;
 +
 + errno = 0;
 + return create.handle;
 +}
 +
 +
  uint32_t __gem_create(int fd, int size)
  {
   struct drm_i915_gem_create create;
 diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
 index ced7ef3..cac1c11 100644
 --- a/lib/ioctl_wrappers.h
 +++ b/lib/ioctl_wrappers.h
 @@ -56,6 +56,9 @@ void gem_read(int fd, uint32_t handle, uint32_t offset, 
 void *buf, uint32_t leng
  void gem_set_domain(int fd, uint32_t handle,
   uint32_t read_domains, uint32_t write_domain);
  void gem_sync(int fd, uint32_t handle);
 +
 +bool gem_create__has_stolen_support(int fd);
 +uint32_t gem_create_stolen(int fd, int size);
  uint32_t __gem_create(int fd, int size);
  uint32_t gem_create(int fd, int size);
  void gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 *execbuf);
 @@ -67,6 +70,15 @@ bool gem_mmap__has_wc(int fd);
  void *gem_mmap__wc(int fd, uint32_t handle, int offset, int size, int prot);
  
  /**
 + * gem_require_stolen_support:
 + * @fd: open i915 drm file descriptor
 + *
 + * Test macro to query whether support for allocating objects from stolen
 + * memory is available. Automatically skips through igt_require() if not.
 + */
 +#define gem_require_stolen_support(fd) \
 + igt_require(gem_create__has_stolen_support(fd))
 +/**
   * gem_require_mmap_wc:
   * @fd: open i915 drm file descriptor
   *
 diff --git a/tests/Makefile.sources b/tests/Makefile.sources
 index a165978..9b2ee90 100644
 --- a/tests/Makefile.sources
 +++ b/tests/Makefile.sources
 @@ -23,6 +23,7 @@ TESTS_progs_M = \
   gem_caching \
   gem_close_race \
   gem_concurrent_blit \
 + gem_create_stolen \
   

Re: [Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-05-07 Thread Chris Wilson
On Thu, May 07, 2015 at 08:52:54AM +0200, Daniel Vetter wrote:
 On Wed, May 06, 2015 at 03:51:52PM +0530, ankitprasad.r.sha...@intel.com 
 wrote:
  From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
  
  This patch adds the testcases for verifying the new extended
  gem_create ioctl. By means of this extended ioctl, memory
  placement of the GEM object can be specified, i.e. either
  shmem or stolen memory.
  These testcases include functional tests and interface tests for
  testing the gem_create ioctl call for stolen memory placement
  
  v2: Testing pread/pwrite functionality for stolen backed objects,
  added local struct for extended gem_create and gem_get_aperture,
  until headers catch up (Chris)
  
  v3: Removed get_aperture related functions, extended gem_pread
  to compare speeds for user pages with and without page faults,
  unexposed local_gem_create struct, changed gem_create_stolen
  usage (Chris)
  
  Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
 
 An igt to check for invalid arguments of the gem create ioctl (especially
 the newly added flags parameters) seems to be missing.

If we do that,  I would actually create gem_create.c to do the parameter
testing of CREATE, and rename this to gem_stolen.c as this covers the
functional side of using stolen (i.e. not limited to testing the CREATE
API). And I want a pink pony.
-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] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-05-07 Thread Ankitprasad Sharma
On Thu, 2015-05-07 at 08:52 +0200, Daniel Vetter wrote:
 On Wed, May 06, 2015 at 03:51:52PM +0530, ankitprasad.r.sha...@intel.com 
 wrote:
  From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
  
  This patch adds the testcases for verifying the new extended
  gem_create ioctl. By means of this extended ioctl, memory
  placement of the GEM object can be specified, i.e. either
  shmem or stolen memory.
  These testcases include functional tests and interface tests for
  testing the gem_create ioctl call for stolen memory placement
  
  v2: Testing pread/pwrite functionality for stolen backed objects,
  added local struct for extended gem_create and gem_get_aperture,
  until headers catch up (Chris)
  
  v3: Removed get_aperture related functions, extended gem_pread
  to compare speeds for user pages with and without page faults,
  unexposed local_gem_create struct, changed gem_create_stolen
  usage (Chris)
  
  Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
 
 An igt to check for invalid arguments of the gem create ioctl (especially
 the newly added flags parameters) seems to be missing.
 -Daniel

\Wwe already have a test to check invalid arguments for the newly added
flags parameter in the current set of tests.

static void invalid_flag_test(int fd)

-Ankit

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


Re: [Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-05-07 Thread Daniel Vetter
On Thu, May 07, 2015 at 10:12:08AM +0100, Chris Wilson wrote:
 On Thu, May 07, 2015 at 08:52:54AM +0200, Daniel Vetter wrote:
  On Wed, May 06, 2015 at 03:51:52PM +0530, ankitprasad.r.sha...@intel.com 
  wrote:
   From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
   
   This patch adds the testcases for verifying the new extended
   gem_create ioctl. By means of this extended ioctl, memory
   placement of the GEM object can be specified, i.e. either
   shmem or stolen memory.
   These testcases include functional tests and interface tests for
   testing the gem_create ioctl call for stolen memory placement
   
   v2: Testing pread/pwrite functionality for stolen backed objects,
   added local struct for extended gem_create and gem_get_aperture,
   until headers catch up (Chris)
   
   v3: Removed get_aperture related functions, extended gem_pread
   to compare speeds for user pages with and without page faults,
   unexposed local_gem_create struct, changed gem_create_stolen
   usage (Chris)
   
   Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
  
  An igt to check for invalid arguments of the gem create ioctl (especially
  the newly added flags parameters) seems to be missing.
 
 If we do that,  I would actually create gem_create.c to do the parameter
 testing of CREATE, and rename this to gem_stolen.c as this covers the
 functional side of using stolen (i.e. not limited to testing the CREATE
 API). And I want a pink pony.

Yes, that would be even nicer.
-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


[Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-05-06 Thread ankitprasad . r . sharma
From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com

This patch adds the testcases for verifying the new extended
gem_create ioctl. By means of this extended ioctl, memory
placement of the GEM object can be specified, i.e. either
shmem or stolen memory.
These testcases include functional tests and interface tests for
testing the gem_create ioctl call for stolen memory placement

v2: Testing pread/pwrite functionality for stolen backed objects,
added local struct for extended gem_create and gem_get_aperture,
until headers catch up (Chris)

v3: Removed get_aperture related functions, extended gem_pread
to compare speeds for user pages with and without page faults,
unexposed local_gem_create struct, changed gem_create_stolen
usage (Chris)

Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
---
 lib/ioctl_wrappers.c  |  60 +++
 lib/ioctl_wrappers.h  |  12 ++
 tests/Makefile.sources|   1 +
 tests/gem_create_stolen.c | 387 ++
 tests/gem_pread.c | 100 
 tests/gem_pwrite.c|  45 ++
 6 files changed, 605 insertions(+)
 create mode 100644 tests/gem_create_stolen.c

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index ff78ef1..2bd341a 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -379,6 +379,66 @@ void gem_sync(int fd, uint32_t handle)
gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 }
 
+bool gem_create__has_stolen_support(int fd)
+{
+   static int has_stolen_support = -1;
+   struct drm_i915_getparam gp;
+   int val = -1;
+
+   return 1;
+   if (has_stolen_support  0) {
+   memset(gp, 0, sizeof(gp));
+   gp.param = 35; /* CREATE_VERSION */
+   gp.value = val;
+
+   /* Do we have the extended gem_create_ioctl? */
+   ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp);
+   has_stolen_support = val = 1;
+   }
+
+   return has_stolen_support;
+}
+
+struct local_i915_gem_create_v2 {
+   uint64_t size;
+   uint32_t handle;
+   uint32_t pad;
+#define I915_CREATE_PLACEMENT_STOLEN (10)
+   uint32_t flags;
+};
+
+#define LOCAL_IOCTL_I915_GEM_CREATE   DRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_CREATE, struct local_i915_gem_create_v2)
+/**
+ * gem_create_stolen:
+ * @fd: open i915 drm file descriptor
+ * @size: desired size of the buffer
+ * @flags: desired placement i.e. stolen or shmem
+ *
+ * This wraps the new GEM_CREATE ioctl, which allocates a
+ * new gem buffer object of @size and placement based on @flags.
+ *
+ * Returns: The file-private handle of the created buffer object
+ */
+
+uint32_t gem_create_stolen(int fd, int size)
+{
+   struct local_i915_gem_create_v2 create;
+   int ret;
+
+   memset(create, 0, sizeof(create));
+   create.handle = 0;
+   create.size = size;
+   create.flags = I915_CREATE_PLACEMENT_STOLEN;
+   ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CREATE, create);
+
+   if (ret  0)
+   return 0;
+
+   errno = 0;
+   return create.handle;
+}
+
+
 uint32_t __gem_create(int fd, int size)
 {
struct drm_i915_gem_create create;
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index ced7ef3..cac1c11 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -56,6 +56,9 @@ void gem_read(int fd, uint32_t handle, uint32_t offset, void 
*buf, uint32_t leng
 void gem_set_domain(int fd, uint32_t handle,
uint32_t read_domains, uint32_t write_domain);
 void gem_sync(int fd, uint32_t handle);
+
+bool gem_create__has_stolen_support(int fd);
+uint32_t gem_create_stolen(int fd, int size);
 uint32_t __gem_create(int fd, int size);
 uint32_t gem_create(int fd, int size);
 void gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 *execbuf);
@@ -67,6 +70,15 @@ bool gem_mmap__has_wc(int fd);
 void *gem_mmap__wc(int fd, uint32_t handle, int offset, int size, int prot);
 
 /**
+ * gem_require_stolen_support:
+ * @fd: open i915 drm file descriptor
+ *
+ * Test macro to query whether support for allocating objects from stolen
+ * memory is available. Automatically skips through igt_require() if not.
+ */
+#define gem_require_stolen_support(fd) \
+   igt_require(gem_create__has_stolen_support(fd))
+/**
  * gem_require_mmap_wc:
  * @fd: open i915 drm file descriptor
  *
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index a165978..9b2ee90 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -23,6 +23,7 @@ TESTS_progs_M = \
gem_caching \
gem_close_race \
gem_concurrent_blit \
+   gem_create_stolen \
gem_cs_tlb \
gem_ctx_param_basic \
gem_ctx_bad_exec \
diff --git a/tests/gem_create_stolen.c b/tests/gem_create_stolen.c
new file mode 100644
index 000..402d42b
--- /dev/null
+++ b/tests/gem_create_stolen.c
@@ -0,0 +1,387 @@
+/*
+ * Copyright © 2011 Intel Corporation
+ *
+ * 

Re: [Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-04-29 Thread Chris Wilson
On Wed, Apr 29, 2015 at 03:02:20PM +0530, ankitprasad.r.sha...@intel.com wrote:
 From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
 
 This patch adds the testcases for verifying the new extended
 gem_create ioctl. By means of this extended ioctl, memory
 placement of the GEM object can be specified, i.e. either
 shmem or stolen memory.
 These testcases include functional tests and interface tests for
 testing the gem_create ioctl call for stolen memory placement
 
 v2: Testing pread/pwrite functionality for stolen backed objects,
 added local struct for extended gem_create and gem_get_aperture,
 until headers catch up (Chris)
 
 Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
 ---
  lib/ioctl_wrappers.c  |  88 +++
  lib/ioctl_wrappers.h  |  28 
  tests/Makefile.sources|   1 +
  tests/gem_create_stolen.c | 392 
 ++
  tests/gem_pread.c |  43 +
  tests/gem_pwrite.c|  42 +
  6 files changed, 594 insertions(+)
  create mode 100644 tests/gem_create_stolen.c
 
 diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
 index ff78ef1..5980067 100644
 --- a/lib/ioctl_wrappers.c
 +++ b/lib/ioctl_wrappers.c
 @@ -379,6 +379,57 @@ void gem_sync(int fd, uint32_t handle)
   gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
  }
  
 +bool gem_create__has_stolen_support(int fd)
 +{
 + static int has_stolen_support = -1;
 + struct drm_i915_getparam gp;
 + int val = -1;
 +
 + if (has_stolen_support  0) {
 + memset(gp, 0, sizeof(gp));
 + gp.param = 35; /* CREATE_VERSION */
 + gp.value = val;
 +
 + /* Do we have the extended gem_create_ioctl? */
 + ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp);
 + has_stolen_support = val = 1;
 + }
 +
 + return has_stolen_support;
 +}
 +
 +#define LOCAL_IOCTL_I915_GEM_CREATE   DRM_IOWR(DRM_COMMAND_BASE + 
 DRM_I915_GEM_CREATE, struct local_i915_gem_create_v2)
 +/**
 + * gem_create_stolen:
 + * @fd: open i915 drm file descriptor
 + * @size: desired size of the buffer
 + * @flags: desired placement i.e. stolen or shmem
 + *
 + * This wraps the new GEM_CREATE ioctl, which allocates a
 + * new gem buffer object of @size and placement based on @flags.
 + *
 + * Returns: The file-private handle of the created buffer object
 + */
 +
 +uint32_t gem_create_stolen(int fd, int size, uint32_t flags)
 +{
 + struct local_i915_gem_create_v2 create;
 + int ret;

With a name like gem_create_stolen() I expect it to only create a stolen
obj i.e. always set create.flags = I915_GEM_CREATE_STOLEN.

 + memset(create, 0, sizeof(create));
 + create.handle = 0;
 + create.size = size;
 + create.flags = flags;
 + ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CREATE, create);

The default behaviour is for gem_create() to die if it fails (and so
cause a FAIL, we do explicit checks to skip the test if we don't have
kernel support beforehand). That saves all the various igt_assert()
noise you have in your test cases.

 +
 + if (ret  0)
 + return 0;
 +
 + errno = 0;
 + return create.handle;
 +}

  uint32_t __gem_create(int fd, int size)
  {
   struct drm_i915_gem_create create;
 @@ -1016,6 +1067,43 @@ uint64_t gem_mappable_aperture_size(void)
   return pci_dev-regions[bar].size;
  }
  
 +#define LOCAL_IOCTL_I915_GEM_GET_APERTURE DRM_IOR  (DRM_COMMAND_BASE + 
 DRM_I915_GEM_GET_APERTURE, struct local_i915_gem_get_aperture_v2)
 +/**
 + * gem_stolen_size:
 + *
 + * Feature test macro to query the total size of the stolen region
 + *
 + * Returns: The total size of the stolen region
 + */
 +uint64_t gem_stolen_size(fd)
 +{
 + struct local_i915_gem_get_aperture_v2 aperture;
 +
 + memset(aperture, 0, sizeof(aperture));
 + aperture.aper_size = 256*1024*1024;

Ah, there's no point initializing aperture.aper_size since we don't
return it in case of ioctl failure anyway.

 + do_ioctl(fd, LOCAL_IOCTL_I915_GEM_GET_APERTURE, aperture);
 +
 + return aperture.stolen_size;
 +}
 +
 +/**
 + * gem_available_stolen_size:
 + *
 + * Feature test macro to query the available size in the stolen region
 + *
 + * Returns: The available size in the stolen region
 + */
 +uint64_t gem_available_stolen_size(fd)
 +{
 + struct local_i915_gem_get_aperture_v2 aperture;
 +
 + memset(aperture, 0, sizeof(aperture));
 + aperture.aper_size = 256*1024*1024;

Ditto.

 + do_ioctl(fd, LOCAL_IOCTL_I915_GEM_GET_APERTURE, aperture);
 +
 + return aperture.stolen_available_size;
 +}
 +
  /**
   * gem_require_caching:
   * @fd: open i915 drm file descriptor
 diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
 index ced7ef3..af76af0 100644
 --- a/lib/ioctl_wrappers.h
 +++ b/lib/ioctl_wrappers.h
 @@ -56,6 +56,16 @@ void gem_read(int fd, uint32_t handle, uint32_t offset, 
 void *buf, uint32_t leng
  void gem_set_domain(int fd, uint32_t handle,
  

[Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-04-29 Thread ankitprasad . r . sharma
From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com

This patch adds the testcases for verifying the new extended
gem_create ioctl. By means of this extended ioctl, memory
placement of the GEM object can be specified, i.e. either
shmem or stolen memory.
These testcases include functional tests and interface tests for
testing the gem_create ioctl call for stolen memory placement

v2: Testing pread/pwrite functionality for stolen backed objects,
added local struct for extended gem_create and gem_get_aperture,
until headers catch up (Chris)

Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
---
 lib/ioctl_wrappers.c  |  88 +++
 lib/ioctl_wrappers.h  |  28 
 tests/Makefile.sources|   1 +
 tests/gem_create_stolen.c | 392 ++
 tests/gem_pread.c |  43 +
 tests/gem_pwrite.c|  42 +
 6 files changed, 594 insertions(+)
 create mode 100644 tests/gem_create_stolen.c

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index ff78ef1..5980067 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -379,6 +379,57 @@ void gem_sync(int fd, uint32_t handle)
gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 }
 
+bool gem_create__has_stolen_support(int fd)
+{
+   static int has_stolen_support = -1;
+   struct drm_i915_getparam gp;
+   int val = -1;
+
+   if (has_stolen_support  0) {
+   memset(gp, 0, sizeof(gp));
+   gp.param = 35; /* CREATE_VERSION */
+   gp.value = val;
+
+   /* Do we have the extended gem_create_ioctl? */
+   ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp);
+   has_stolen_support = val = 1;
+   }
+
+   return has_stolen_support;
+}
+
+#define LOCAL_IOCTL_I915_GEM_CREATE   DRM_IOWR(DRM_COMMAND_BASE + 
DRM_I915_GEM_CREATE, struct local_i915_gem_create_v2)
+/**
+ * gem_create_stolen:
+ * @fd: open i915 drm file descriptor
+ * @size: desired size of the buffer
+ * @flags: desired placement i.e. stolen or shmem
+ *
+ * This wraps the new GEM_CREATE ioctl, which allocates a
+ * new gem buffer object of @size and placement based on @flags.
+ *
+ * Returns: The file-private handle of the created buffer object
+ */
+
+uint32_t gem_create_stolen(int fd, int size, uint32_t flags)
+{
+   struct local_i915_gem_create_v2 create;
+   int ret;
+
+   memset(create, 0, sizeof(create));
+   create.handle = 0;
+   create.size = size;
+   create.flags = flags;
+   ret = drmIoctl(fd, LOCAL_IOCTL_I915_GEM_CREATE, create);
+
+   if (ret  0)
+   return 0;
+
+   errno = 0;
+   return create.handle;
+}
+
+
 uint32_t __gem_create(int fd, int size)
 {
struct drm_i915_gem_create create;
@@ -1016,6 +1067,43 @@ uint64_t gem_mappable_aperture_size(void)
return pci_dev-regions[bar].size;
 }
 
+#define LOCAL_IOCTL_I915_GEM_GET_APERTURE DRM_IOR  (DRM_COMMAND_BASE + 
DRM_I915_GEM_GET_APERTURE, struct local_i915_gem_get_aperture_v2)
+/**
+ * gem_stolen_size:
+ *
+ * Feature test macro to query the total size of the stolen region
+ *
+ * Returns: The total size of the stolen region
+ */
+uint64_t gem_stolen_size(fd)
+{
+   struct local_i915_gem_get_aperture_v2 aperture;
+
+   memset(aperture, 0, sizeof(aperture));
+   aperture.aper_size = 256*1024*1024;
+   do_ioctl(fd, LOCAL_IOCTL_I915_GEM_GET_APERTURE, aperture);
+
+   return aperture.stolen_size;
+}
+
+/**
+ * gem_available_stolen_size:
+ *
+ * Feature test macro to query the available size in the stolen region
+ *
+ * Returns: The available size in the stolen region
+ */
+uint64_t gem_available_stolen_size(fd)
+{
+   struct local_i915_gem_get_aperture_v2 aperture;
+
+   memset(aperture, 0, sizeof(aperture));
+   aperture.aper_size = 256*1024*1024;
+   do_ioctl(fd, LOCAL_IOCTL_I915_GEM_GET_APERTURE, aperture);
+
+   return aperture.stolen_available_size;
+}
+
 /**
  * gem_require_caching:
  * @fd: open i915 drm file descriptor
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index ced7ef3..af76af0 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -56,6 +56,16 @@ void gem_read(int fd, uint32_t handle, uint32_t offset, void 
*buf, uint32_t leng
 void gem_set_domain(int fd, uint32_t handle,
uint32_t read_domains, uint32_t write_domain);
 void gem_sync(int fd, uint32_t handle);
+
+struct local_i915_gem_create_v2 {
+   uint64_t size;
+   uint32_t handle;
+   uint32_t pad;
+#define I915_CREATE_PLACEMENT_STOLEN (10)
+   uint32_t flags;
+};
+bool gem_create__has_stolen_support(int fd);
+uint32_t gem_create_stolen(int fd, int size, uint32_t flags);
 uint32_t __gem_create(int fd, int size);
 uint32_t gem_create(int fd, int size);
 void gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 *execbuf);
@@ -67,6 +77,15 @@ bool gem_mmap__has_wc(int fd);
 void *gem_mmap__wc(int fd, uint32_t handle, int 

Re: [Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-04-11 Thread Chris Wilson
On Sat, Apr 11, 2015 at 05:58:12PM +0530, ankitprasad.r.sha...@intel.com wrote:
 From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
 
 This patch adds the testcases for verifying the new extended
 gem_create ioctl. By means of this extended ioctl, memory
 placement of the GEM object can be specified, i.e. either
 shmem or stolen memory.
 These testcases include functional tests and interface tests for
 testing the gem_create ioctl call for stolen memory placement
 
 Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
 ---
  lib/ioctl_wrappers.c  |  47 +++
  lib/ioctl_wrappers.h  |  11 ++
  tests/Makefile.sources|   1 +
  tests/gem_create_stolen.c | 333 
 ++
  4 files changed, 392 insertions(+)
  create mode 100644 tests/gem_create_stolen.c
 
 diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
 index ff78ef1..9e4b89d 100644
 --- a/lib/ioctl_wrappers.c
 +++ b/lib/ioctl_wrappers.c
 @@ -379,6 +379,53 @@ void gem_sync(int fd, uint32_t handle)
   gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
  }
  
 +bool gem_create__has_stolen_support(int fd)
 +{
 + static has_stolen_support = -1;
 + struct drm_i915_getparam gp;
 + int val = -1;

I am on a mission at the moment to cache the static queries like this,
i.e.

static int has_stolen_suuport = -1;
if (has_stolen_support  0) {
int version = -1;
gem_get_param(fd, 35 /* CREATE_VERSION */, version));
has_stolen_support = version = 1;
}
return has_stolen_support;

 +
 + memset(gp, 0, sizeof(gp));
 + gp.param = 35; /* CREATE_VERSION */
 + gp.value = val;
 +
 + /* Do we have the extended gem_create_ioctl? */
 + ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp);

We must have gem_get_param() already?

 + return val = 1;
 +}

 +
 +/**
 + * gem_create stolen:
 + * @fd: open i915 drm file descriptor
 + * @size: desired size of the buffer
 + * @flags: desired placement i.e. stolen or shmem
 + *
 + * This wraps the new GEM_CREATE ioctl, which allocates a
 + * new gem buffer object of @size and placement based on @flags.
 + *
 + * Returns: The file-private handle of the created buffer object
 + */
 +

 +uint32_t gem_create_stolen(int fd, int size, uint32_t flags)
 +{
 + struct drm_i915_gem_create create;

'struct local_i915_gem_create_v2' until headers catch up.

 +igt_main
 +{
 + int i, fd, gtt_size_total, gtt_size_mappable;
 + uint32_t devid;
 +
 + igt_skip_on_simulation();
 +
 + igt_fixture {
 + fd = drm_open_any();
 + devid = intel_get_drm_devid(fd);
 +
 + drm_intel_get_aperture_sizes(fd, (size_t*)gtt_size_total,
 + (size_t*)gtt_size_mappable);

Oh, we should extend get_aperture_size to include stolen as as well as GTT
aperture and full TT size.

 + bufmgr = drm_intel_bufmgr_gem_init(fd, 4096);
 + batch = intel_batchbuffer_alloc(bufmgr, devid);
 + }
 +
 + igt_subtest(stolen-invalid-flag)
 + invalid_flag_test(fd);
 +
 + igt_subtest(stolen-clear)
 + verify_object_clear(fd);
 +
 + /*
 +  * stolen mem special cases - checking for non cpu mappable
 +  */
 + igt_subtest(stolen-mmap)
 + stolen_mmap(fd);

I think changing this and the other negative tests to  stolen-no-mmap
to make it clear when running that is a negative test.

 + igt_subtest(stolen-pwrite)
 + stolen_pwrite(fd);

pwrite/pread to stolen is just a few lines of kernel to fix, if we want
the api. Considering how difficult it is to access stolen on gen6+, we
probably do.

 +
 + /* Functional Test - blt copy */
 + igt_subtest(stolen-copy)
 + copy_test(fd);

Also test GTT access. You should also hook up gem_concurrent_all to
exercise coherency tests through stolen. That will be not a fun task
though to teach it the restricted API. Similarly, we do want to exercise
set-caching on stolen as well. Our presumption is that it is normal
memory as far as the GPU goes (only the MCH has the filter) and should
verify that all stolen objects actually behave identially.

Similarly, check that we bind stolen to framebuffers (yes, I know the
kernel already does that, but it has been buggy in the past, and it is a
definite use-case for stolen).
-Chris

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


[Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl

2015-04-11 Thread ankitprasad . r . sharma
From: Ankitprasad Sharma ankitprasad.r.sha...@intel.com

This patch adds the testcases for verifying the new extended
gem_create ioctl. By means of this extended ioctl, memory
placement of the GEM object can be specified, i.e. either
shmem or stolen memory.
These testcases include functional tests and interface tests for
testing the gem_create ioctl call for stolen memory placement

Signed-off-by: Ankitprasad Sharma ankitprasad.r.sha...@intel.com
---
 lib/ioctl_wrappers.c  |  47 +++
 lib/ioctl_wrappers.h  |  11 ++
 tests/Makefile.sources|   1 +
 tests/gem_create_stolen.c | 333 ++
 4 files changed, 392 insertions(+)
 create mode 100644 tests/gem_create_stolen.c

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index ff78ef1..9e4b89d 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -379,6 +379,53 @@ void gem_sync(int fd, uint32_t handle)
gem_set_domain(fd, handle, I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
 }
 
+bool gem_create__has_stolen_support(int fd)
+{
+   static has_stolen_support = -1;
+   struct drm_i915_getparam gp;
+   int val = -1;
+
+   memset(gp, 0, sizeof(gp));
+   gp.param = 35; /* CREATE_VERSION */
+   gp.value = val;
+
+   /* Do we have the extended gem_create_ioctl? */
+   ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp);
+
+   return val = 1;
+}
+
+/**
+ * gem_create stolen:
+ * @fd: open i915 drm file descriptor
+ * @size: desired size of the buffer
+ * @flags: desired placement i.e. stolen or shmem
+ *
+ * This wraps the new GEM_CREATE ioctl, which allocates a
+ * new gem buffer object of @size and placement based on @flags.
+ *
+ * Returns: The file-private handle of the created buffer object
+ */
+
+uint32_t gem_create_stolen(int fd, int size, uint32_t flags)
+{
+   struct drm_i915_gem_create create;
+   int ret;
+
+   memset(create, 0, sizeof(create));
+   create.handle = 0;
+   create.size = size;
+   create.flags = flags;
+   ret = drmIoctl(fd, DRM_IOCTL_I915_GEM_CREATE, create);
+
+   if (ret  0)
+   return 0;
+
+   errno = 0;
+   return create.handle;
+}
+
+
 uint32_t __gem_create(int fd, int size)
 {
struct drm_i915_gem_create create;
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index ced7ef3..f9d43c6 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -56,6 +56,8 @@ void gem_read(int fd, uint32_t handle, uint32_t offset, void 
*buf, uint32_t leng
 void gem_set_domain(int fd, uint32_t handle,
uint32_t read_domains, uint32_t write_domain);
 void gem_sync(int fd, uint32_t handle);
+bool gem_create__has_stolen_support(int fd);
+uint32_t gem_create_stolen(int fd, int size, uint32_t flags);
 uint32_t __gem_create(int fd, int size);
 uint32_t gem_create(int fd, int size);
 void gem_execbuf(int fd, struct drm_i915_gem_execbuffer2 *execbuf);
@@ -67,6 +69,15 @@ bool gem_mmap__has_wc(int fd);
 void *gem_mmap__wc(int fd, uint32_t handle, int offset, int size, int prot);
 
 /**
+ * gem_require_stolen_support:
+ * @fd: open i915 drm file descriptor
+ *
+ * Test macro to query whether support for allocating objects from stolen
+ * memory is available. Automatically skips through igt_require() if not.
+ */
+#define gem_require_stolen_support(fd) \
+   igt_require(gem_create__has_stolen_support(fd))
+/**
  * gem_require_mmap_wc:
  * @fd: open i915 drm file descriptor
  *
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index a165978..9b2ee90 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -23,6 +23,7 @@ TESTS_progs_M = \
gem_caching \
gem_close_race \
gem_concurrent_blit \
+   gem_create_stolen \
gem_cs_tlb \
gem_ctx_param_basic \
gem_ctx_bad_exec \
diff --git a/tests/gem_create_stolen.c b/tests/gem_create_stolen.c
new file mode 100644
index 000..afab822
--- /dev/null
+++ b/tests/gem_create_stolen.c
@@ -0,0 +1,333 @@
+/*
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,