Re: [Intel-gfx] [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl
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
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
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
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
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
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
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
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
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
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
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
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,