[PATCH] tests/kms_addfb: Add support for fb modifiers

[PATCH] tests/kms_addfb: Add support for fb modifiers

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Just a few basic tests to make sure fb modifiers can be used and
behave sanely when mixed with the old set_tiling API.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/ioctl_wrappers.c | 45 ++++++++++++++++++++++++++++++++++++++
 lib/ioctl_wrappers.h | 36 ++++++++++++++++++++++++++++++
 tests/kms_addfb.c    | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 19a457a..bca6d2a 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -1091,3 +1091,48 @@ int gem_context_has_param(int fd, uint64_t param)
 
 	return gem_context_get_param(fd, &p) == 0;
 }
+
+int _drmModeAddFB2(int fd, uint32_t width, uint32_t height,
+		   uint32_t pixel_format, uint32_t bo_handles[4],
+		   uint32_t pitches[4], uint32_t offsets[4],
+		   uint64_t modifier[0], uint32_t *buf_id, uint32_t flags)
+{
+	struct local_drm_mode_fb_cmd2 f;
+	int ret;
+
+	f.width  = width;
+	f.height = height;
+	f.pixel_format = pixel_format;
+	f.flags = flags;
+
+	memcpy(f.handles, bo_handles, 4 * sizeof(bo_handles[0]));
+	memcpy(f.pitches, pitches, 4 * sizeof(pitches[0]));
+	memcpy(f.offsets, offsets, 4 * sizeof(offsets[0]));
+	memcpy(f.modifier, modifier, 4 * sizeof(modifier[0]));
+
+	if ((ret = drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f)))
+		return ret < 0 ? -errno : ret;
+
+	*buf_id = f.fb_id;
+	return 0;
+}
+
+unsigned int has_drm_fb_modifiers(int fd)
+{
+	static unsigned int has_modifiers, cap_modifiers_tested;
+	uint64_t cap_modifiers;
+	int ret;
+
+	if (cap_modifiers_tested)
+		return has_modifiers;
+
+	ret = drmGetCap(fd, LOCAL_DRM_CAP_ADDFB2_MODIFIERS, &cap_modifiers);
+	igt_assert(ret == 0 || errno == EINVAL);
+	has_modifiers = ret == 0 && cap_modifiers == 1;
+	cap_modifiers_tested = 1;
+
+	if (has_modifiers)
+		igt_debug("DRM_CAP_ADDFB2_MODIFIERS\n");
+
+	return has_modifiers;
+}
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 30ab836..c277012 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -117,4 +117,40 @@ int gem_context_has_param(int fd, uint64_t param);
 int gem_context_get_param(int fd, struct local_i915_gem_context_param *p);
 int gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
 
+struct local_drm_mode_fb_cmd2 {
+	uint32_t fb_id;
+	uint32_t width, height;
+	uint32_t pixel_format;
+	uint32_t flags;
+	uint32_t handles[4];
+	uint32_t pitches[4];
+	uint32_t offsets[4];
+	uint64_t modifier[4];
+};
+
+#define LOCAL_DRM_MODE_FB_MODIFIERS	(1<<1)
+
+#define LOCAL_DRM_FORMAT_MOD_VENDOR_INTEL   0x01
+
+#define local_fourcc_mod_code(vendor, val) \
+	((((uint64_t)LOCAL_DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | \
+	  (val & 0x00ffffffffffffffL))
+
+#define LOCAL_I915_FORMAT_MOD_NONE	local_fourcc_mod_code(INTEL, \
+							      0x00000000000000L)
+#define LOCAL_I915_FORMAT_MOD_X_TILED	local_fourcc_mod_code(INTEL, \
+							      0x00000000000001L)
+
+#define LOCAL_DRM_IOCTL_MODE_ADDFB2	DRM_IOWR(0xB8, \
+						 struct local_drm_mode_fb_cmd2)
+
+int _drmModeAddFB2(int fd, uint32_t width, uint32_t height,
+		   uint32_t pixel_format, uint32_t bo_handles[4],
+		   uint32_t pitches[4], uint32_t offsets[4],
+		   uint64_t modifier[0], uint32_t *buf_id, uint32_t flags);
+
+#define LOCAL_DRM_CAP_ADDFB2_MODIFIERS	0x10
+
+unsigned int has_drm_fb_modifiers(int fd);
+
 #endif /* IOCTL_WRAPPERS_H */
diff --git a/tests/kms_addfb.c b/tests/kms_addfb.c
index 756589e..9b0f77c 100644
--- a/tests/kms_addfb.c
+++ b/tests/kms_addfb.c
@@ -213,6 +213,66 @@ static void size_tests(int fd)
 	}
 }
 
+static void addfb25_tests(int fd)
+{
+	struct local_drm_mode_fb_cmd2 f = {};
+
+
+	igt_require(has_drm_fb_modifiers(fd));
+
+	memset(&f, 0, sizeof(f));
+
+	f.width = 1024;
+	f.height = 1024;
+	f.pixel_format = DRM_FORMAT_XRGB8888;
+	f.pitches[0] = 1024*4;
+	f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
+	f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
+
+	igt_fixture {
+		gem_bo = gem_create(fd, 1024*1024*4);
+		igt_assert(gem_bo);
+	}
+
+	f.handles[0] = gem_bo;
+
+	igt_subtest("addfb25-X-tiled") {
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+		f.fb_id = 0;
+	}
+
+	igt_subtest("addfb25-framebuffer-vs-set-tiling") {
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 512*4) == -EBUSY);
+		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4) == -EBUSY);
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+		f.fb_id = 0;
+	}
+
+	igt_fixture
+		gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4);
+	f.pitches[0] = 1024*4;
+
+	igt_subtest("addfb25-X-tiled-both") {
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+		f.fb_id = 0;
+	}
+
+	igt_subtest("addfb25-X-tiled-mismatch") {
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
+		f.fb_id = 0;
+	}
+
+	igt_fixture {
+		gem_close(fd, gem_bo);
+	}
+}
+
 int fd;
 
 igt_main
@@ -224,6 +284,8 @@ igt_main
 
 	size_tests(fd);
 
+	addfb25_tests(fd);
+
 	igt_fixture
 		close(fd);
 }
-- 
2.2.2

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

Re: [PATCH] tests/kms_addfb: Add support for fb modifiers

[Daniel Vetter]

On Wed, Feb 04, 2015 at 04:42:14PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Just a few basic tests to make sure fb modifiers can be used and
> behave sanely when mixed with the old set_tiling API.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/ioctl_wrappers.c | 45 ++++++++++++++++++++++++++++++++++++++
>  lib/ioctl_wrappers.h | 36 ++++++++++++++++++++++++++++++
>  tests/kms_addfb.c    | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 143 insertions(+)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 19a457a..bca6d2a 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -1091,3 +1091,48 @@ int gem_context_has_param(int fd, uint64_t param)
>  
>  	return gem_context_get_param(fd, &p) == 0;
>  }
> +

gtkdoc is missing here. Easiest way to avoid it is to just move these two
wrappers into the kms_fb testcase. Whomever needs to reuse these gets to
write the docs.

> +int _drmModeAddFB2(int fd, uint32_t width, uint32_t height,
> +		   uint32_t pixel_format, uint32_t bo_handles[4],
> +		   uint32_t pitches[4], uint32_t offsets[4],
> +		   uint64_t modifier[0], uint32_t *buf_id, uint32_t flags)
> +{
> +	struct local_drm_mode_fb_cmd2 f;
> +	int ret;
> +
> +	f.width  = width;
> +	f.height = height;
> +	f.pixel_format = pixel_format;
> +	f.flags = flags;
> +
> +	memcpy(f.handles, bo_handles, 4 * sizeof(bo_handles[0]));
> +	memcpy(f.pitches, pitches, 4 * sizeof(pitches[0]));
> +	memcpy(f.offsets, offsets, 4 * sizeof(offsets[0]));
> +	memcpy(f.modifier, modifier, 4 * sizeof(modifier[0]));
> +
> +	if ((ret = drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f)))
> +		return ret < 0 ? -errno : ret;
> +
> +	*buf_id = f.fb_id;
> +	return 0;
> +}
> +
> +unsigned int has_drm_fb_modifiers(int fd)
> +{
> +	static unsigned int has_modifiers, cap_modifiers_tested;
> +	uint64_t cap_modifiers;
> +	int ret;
> +
> +	if (cap_modifiers_tested)
> +		return has_modifiers;
> +
> +	ret = drmGetCap(fd, LOCAL_DRM_CAP_ADDFB2_MODIFIERS, &cap_modifiers);
> +	igt_assert(ret == 0 || errno == EINVAL);
> +	has_modifiers = ret == 0 && cap_modifiers == 1;
> +	cap_modifiers_tested = 1;
> +
> +	if (has_modifiers)
> +		igt_debug("DRM_CAP_ADDFB2_MODIFIERS\n");
> +
> +	return has_modifiers;
> +}
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index 30ab836..c277012 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -117,4 +117,40 @@ int gem_context_has_param(int fd, uint64_t param);
>  int gem_context_get_param(int fd, struct local_i915_gem_context_param *p);
>  int gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
>  
> +struct local_drm_mode_fb_cmd2 {
> +	uint32_t fb_id;
> +	uint32_t width, height;
> +	uint32_t pixel_format;
> +	uint32_t flags;
> +	uint32_t handles[4];
> +	uint32_t pitches[4];
> +	uint32_t offsets[4];
> +	uint64_t modifier[4];
> +};
> +
> +#define LOCAL_DRM_MODE_FB_MODIFIERS	(1<<1)
> +
> +#define LOCAL_DRM_FORMAT_MOD_VENDOR_INTEL   0x01
> +
> +#define local_fourcc_mod_code(vendor, val) \
> +	((((uint64_t)LOCAL_DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | \
> +	  (val & 0x00ffffffffffffffL))
> +
> +#define LOCAL_I915_FORMAT_MOD_NONE	local_fourcc_mod_code(INTEL, \
> +							      0x00000000000000L)
> +#define LOCAL_I915_FORMAT_MOD_X_TILED	local_fourcc_mod_code(INTEL, \
> +							      0x00000000000001L)
> +
> +#define LOCAL_DRM_IOCTL_MODE_ADDFB2	DRM_IOWR(0xB8, \
> +						 struct local_drm_mode_fb_cmd2)
> +
> +int _drmModeAddFB2(int fd, uint32_t width, uint32_t height,
> +		   uint32_t pixel_format, uint32_t bo_handles[4],
> +		   uint32_t pitches[4], uint32_t offsets[4],
> +		   uint64_t modifier[0], uint32_t *buf_id, uint32_t flags);
> +
> +#define LOCAL_DRM_CAP_ADDFB2_MODIFIERS	0x10
> +
> +unsigned int has_drm_fb_modifiers(int fd);
> +
>  #endif /* IOCTL_WRAPPERS_H */
> diff --git a/tests/kms_addfb.c b/tests/kms_addfb.c
> index 756589e..9b0f77c 100644
> --- a/tests/kms_addfb.c
> +++ b/tests/kms_addfb.c
> @@ -213,6 +213,66 @@ static void size_tests(int fd)
>  	}
>  }
>  
> +static void addfb25_tests(int fd)
> +{
> +	struct local_drm_mode_fb_cmd2 f = {};
> +
> +
> +	igt_require(has_drm_fb_modifiers(fd));

Imo better to move the igt_require int igt_main so that ppl aren't
suprised that they're tests are skipping when they add more below the call
to addfb25_tests.

> +
> +	memset(&f, 0, sizeof(f));
> +
> +	f.width = 1024;
> +	f.height = 1024;
> +	f.pixel_format = DRM_FORMAT_XRGB8888;
> +	f.pitches[0] = 1024*4;
> +	f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
> +	f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;

c99 initializes nicely combine the memset with the explicit struct init.
But just a bikeshed.

> +
> +	igt_fixture {
> +		gem_bo = gem_create(fd, 1024*1024*4);
> +		igt_assert(gem_bo);
> +	}
> +
> +	f.handles[0] = gem_bo;
> +
> +	igt_subtest("addfb25-X-tiled") {
> +		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
> +		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
> +		f.fb_id = 0;
> +	}
> +
> +	igt_subtest("addfb25-framebuffer-vs-set-tiling") {
> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
> +		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 512*4) == -EBUSY);
> +		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4) == -EBUSY);
> +		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
> +		f.fb_id = 0;
> +	}
> +
> +	igt_fixture
> +		gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4);
> +	f.pitches[0] = 1024*4;
> +
> +	igt_subtest("addfb25-X-tiled-both") {
> +		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
> +		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
> +		f.fb_id = 0;
> +	}
> +
> +	igt_subtest("addfb25-X-tiled-mismatch") {
> +		f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
> +		f.fb_id = 0;
> +	}

Looks good, but two abuse cases are imo missing:

- f.flags = 0 but f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED
- f.flags = LOCAL_DRM_MODE_FB_MODIFIERS but f.modifier[0] = 0

Just to make sure we have that part of the addfb extension covered, too.

Cheers, Daniel

> +
> +	igt_fixture {
> +		gem_close(fd, gem_bo);
> +	}
> +}
> +
>  int fd;
>  
>  igt_main
> @@ -224,6 +284,8 @@ igt_main
>  
>  	size_tests(fd);
>  
> +	addfb25_tests(fd);
> +
>  	igt_fixture
>  		close(fd);
>  }
> -- 
> 2.2.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/kms_addfb: Add support for fb modifiers

[Tvrtko Ursulin]

On 02/05/2015 02:21 PM, Daniel Vetter wrote:
> On Wed, Feb 04, 2015 at 04:42:14PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Just a few basic tests to make sure fb modifiers can be used and
>> behave sanely when mixed with the old set_tiling API.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   lib/ioctl_wrappers.c | 45 ++++++++++++++++++++++++++++++++++++++
>>   lib/ioctl_wrappers.h | 36 ++++++++++++++++++++++++++++++
>>   tests/kms_addfb.c    | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 143 insertions(+)
>>
>> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
>> index 19a457a..bca6d2a 100644
>> --- a/lib/ioctl_wrappers.c
>> +++ b/lib/ioctl_wrappers.c
>> @@ -1091,3 +1091,48 @@ int gem_context_has_param(int fd, uint64_t param)
>>
>>   	return gem_context_get_param(fd, &p) == 0;
>>   }
>> +
>
> gtkdoc is missing here. Easiest way to avoid it is to just move these two
> wrappers into the kms_fb testcase. Whomever needs to reuse these gets to
> write the docs.

I'll need it so will fix myself.

>> +int _drmModeAddFB2(int fd, uint32_t width, uint32_t height,
>> +		   uint32_t pixel_format, uint32_t bo_handles[4],
>> +		   uint32_t pitches[4], uint32_t offsets[4],
>> +		   uint64_t modifier[0], uint32_t *buf_id, uint32_t flags)
>> +{
>> +	struct local_drm_mode_fb_cmd2 f;
>> +	int ret;
>> +
>> +	f.width  = width;
>> +	f.height = height;
>> +	f.pixel_format = pixel_format;
>> +	f.flags = flags;
>> +
>> +	memcpy(f.handles, bo_handles, 4 * sizeof(bo_handles[0]));
>> +	memcpy(f.pitches, pitches, 4 * sizeof(pitches[0]));
>> +	memcpy(f.offsets, offsets, 4 * sizeof(offsets[0]));
>> +	memcpy(f.modifier, modifier, 4 * sizeof(modifier[0]));
>> +
>> +	if ((ret = drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f)))
>> +		return ret < 0 ? -errno : ret;
>> +
>> +	*buf_id = f.fb_id;
>> +	return 0;
>> +}
>> +
>> +unsigned int has_drm_fb_modifiers(int fd)
>> +{
>> +	static unsigned int has_modifiers, cap_modifiers_tested;
>> +	uint64_t cap_modifiers;
>> +	int ret;
>> +
>> +	if (cap_modifiers_tested)
>> +		return has_modifiers;
>> +
>> +	ret = drmGetCap(fd, LOCAL_DRM_CAP_ADDFB2_MODIFIERS, &cap_modifiers);
>> +	igt_assert(ret == 0 || errno == EINVAL);
>> +	has_modifiers = ret == 0 && cap_modifiers == 1;
>> +	cap_modifiers_tested = 1;
>> +
>> +	if (has_modifiers)
>> +		igt_debug("DRM_CAP_ADDFB2_MODIFIERS\n");
>> +
>> +	return has_modifiers;
>> +}
>> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
>> index 30ab836..c277012 100644
>> --- a/lib/ioctl_wrappers.h
>> +++ b/lib/ioctl_wrappers.h
>> @@ -117,4 +117,40 @@ int gem_context_has_param(int fd, uint64_t param);
>>   int gem_context_get_param(int fd, struct local_i915_gem_context_param *p);
>>   int gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
>>
>> +struct local_drm_mode_fb_cmd2 {
>> +	uint32_t fb_id;
>> +	uint32_t width, height;
>> +	uint32_t pixel_format;
>> +	uint32_t flags;
>> +	uint32_t handles[4];
>> +	uint32_t pitches[4];
>> +	uint32_t offsets[4];
>> +	uint64_t modifier[4];
>> +};
>> +
>> +#define LOCAL_DRM_MODE_FB_MODIFIERS	(1<<1)
>> +
>> +#define LOCAL_DRM_FORMAT_MOD_VENDOR_INTEL   0x01
>> +
>> +#define local_fourcc_mod_code(vendor, val) \
>> +	((((uint64_t)LOCAL_DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | \
>> +	  (val & 0x00ffffffffffffffL))
>> +
>> +#define LOCAL_I915_FORMAT_MOD_NONE	local_fourcc_mod_code(INTEL, \
>> +							      0x00000000000000L)
>> +#define LOCAL_I915_FORMAT_MOD_X_TILED	local_fourcc_mod_code(INTEL, \
>> +							      0x00000000000001L)
>> +
>> +#define LOCAL_DRM_IOCTL_MODE_ADDFB2	DRM_IOWR(0xB8, \
>> +						 struct local_drm_mode_fb_cmd2)
>> +
>> +int _drmModeAddFB2(int fd, uint32_t width, uint32_t height,
>> +		   uint32_t pixel_format, uint32_t bo_handles[4],
>> +		   uint32_t pitches[4], uint32_t offsets[4],
>> +		   uint64_t modifier[0], uint32_t *buf_id, uint32_t flags);
>> +
>> +#define LOCAL_DRM_CAP_ADDFB2_MODIFIERS	0x10
>> +
>> +unsigned int has_drm_fb_modifiers(int fd);
>> +
>>   #endif /* IOCTL_WRAPPERS_H */
>> diff --git a/tests/kms_addfb.c b/tests/kms_addfb.c
>> index 756589e..9b0f77c 100644
>> --- a/tests/kms_addfb.c
>> +++ b/tests/kms_addfb.c
>> @@ -213,6 +213,66 @@ static void size_tests(int fd)
>>   	}
>>   }
>>
>> +static void addfb25_tests(int fd)
>> +{
>> +	struct local_drm_mode_fb_cmd2 f = {};
>> +
>> +
>> +	igt_require(has_drm_fb_modifiers(fd));
>
> Imo better to move the igt_require int igt_main so that ppl aren't
> suprised that they're tests are skipping when they add more below the call
> to addfb25_tests.

Skipping individual test cases was my intention, but I overlooked the 
fact this function is not a subtest... I suppose igt_subtest in 
igt_subtest is not possible. :)

>> +
>> +	memset(&f, 0, sizeof(f));
>> +
>> +	f.width = 1024;
>> +	f.height = 1024;
>> +	f.pixel_format = DRM_FORMAT_XRGB8888;
>> +	f.pitches[0] = 1024*4;
>> +	f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
>> +	f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
>
> c99 initializes nicely combine the memset with the explicit struct init.
> But just a bikeshed.

I just copy pasted from libdrm, presumably at some point similar code 
will appear there and then after some years this can be removed. :)

>> +
>> +	igt_fixture {
>> +		gem_bo = gem_create(fd, 1024*1024*4);
>> +		igt_assert(gem_bo);
>> +	}
>> +
>> +	f.handles[0] = gem_bo;
>> +
>> +	igt_subtest("addfb25-X-tiled") {
>> +		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
>> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
>> +		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
>> +		f.fb_id = 0;
>> +	}
>> +
>> +	igt_subtest("addfb25-framebuffer-vs-set-tiling") {
>> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
>> +		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 512*4) == -EBUSY);
>> +		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4) == -EBUSY);
>> +		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
>> +		f.fb_id = 0;
>> +	}
>> +
>> +	igt_fixture
>> +		gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4);
>> +	f.pitches[0] = 1024*4;
>> +
>> +	igt_subtest("addfb25-X-tiled-both") {
>> +		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
>> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
>> +		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
>> +		f.fb_id = 0;
>> +	}
>> +
>> +	igt_subtest("addfb25-X-tiled-mismatch") {
>> +		f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
>> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
>> +		f.fb_id = 0;
>> +	}
>
> Looks good, but two abuse cases are imo missing:
>
> - f.flags = 0 but f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED
> - f.flags = LOCAL_DRM_MODE_FB_MODIFIERS but f.modifier[0] = 0
>
> Just to make sure we have that part of the addfb extension covered, too.

Test cases did look suspiciously few to me yesterday but obviously 
testing mindset wasn't fully on - you are right for those two.

Thanks,

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

Re: [PATCH] tests/kms_addfb: Add support for fb modifiers

[Daniel Vetter]

On Thu, Feb 05, 2015 at 02:50:09PM +0000, Tvrtko Ursulin wrote:
> 
> On 02/05/2015 02:21 PM, Daniel Vetter wrote:
> >On Wed, Feb 04, 2015 at 04:42:14PM +0000, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>Just a few basic tests to make sure fb modifiers can be used and
> >>behave sanely when mixed with the old set_tiling API.
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>---
> >>  lib/ioctl_wrappers.c | 45 ++++++++++++++++++++++++++++++++++++++
> >>  lib/ioctl_wrappers.h | 36 ++++++++++++++++++++++++++++++
> >>  tests/kms_addfb.c    | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 143 insertions(+)
> >>
> >>diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> >>index 19a457a..bca6d2a 100644
> >>--- a/lib/ioctl_wrappers.c
> >>+++ b/lib/ioctl_wrappers.c
> >>@@ -1091,3 +1091,48 @@ int gem_context_has_param(int fd, uint64_t param)
> >>
> >>  	return gem_context_get_param(fd, &p) == 0;
> >>  }
> >>+
> >
> >gtkdoc is missing here. Easiest way to avoid it is to just move these two
> >wrappers into the kms_fb testcase. Whomever needs to reuse these gets to
> >write the docs.
> 
> I'll need it so will fix myself.

A then we'll need to do some proper abi polish I think.
> 
> >>+int _drmModeAddFB2(int fd, uint32_t width, uint32_t height,
> >>+		   uint32_t pixel_format, uint32_t bo_handles[4],
> >>+		   uint32_t pitches[4], uint32_t offsets[4],
> >>+		   uint64_t modifier[0], uint32_t *buf_id, uint32_t flags)

No camelcase for library functions. Also I think we want a kms_ prefix
here for a bit of namespacing. And since your test only creates
framebuffers with 1 plane I think simplifying the function interface would
be good to. Finally we use a __ prefix for raw functions (i.e. those
returning errno) in the ioctl wrapper library. And I'd put the bo handle
right after the fd to be more consistent with other functions. And usually
we put out-parameters last. So

int __kms_addfb(int fd, unint32_t handle,
		width, height, stride, pixel_format, modifier, flags,
		uint32_t *buf_id);
> >>+{
> >>+	struct local_drm_mode_fb_cmd2 f;
> >>+	int ret;
> >>+
> >>+	f.width  = width;
> >>+	f.height = height;
> >>+	f.pixel_format = pixel_format;
> >>+	f.flags = flags;
> >>+
> >>+	memcpy(f.handles, bo_handles, 4 * sizeof(bo_handles[0]));
> >>+	memcpy(f.pitches, pitches, 4 * sizeof(pitches[0]));
> >>+	memcpy(f.offsets, offsets, 4 * sizeof(offsets[0]));
> >>+	memcpy(f.modifier, modifier, 4 * sizeof(modifier[0]));
> >>+
> >>+	if ((ret = drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f)))
> >>+		return ret < 0 ? -errno : ret;
> >>+
> >>+	*buf_id = f.fb_id;
> >>+	return 0;
> >>+}
> >>+
> >>+unsigned int has_drm_fb_modifiers(int fd)

Common patterin is to pusht he require into this function and use a void
return value. Also drm prefix is used for core drm and libdrm stuff,
better to pick kms_. So

void drm_require_fb_modifiers(int fd);

Also please make sure the new kms_ funcs are grouped into a separate
section, so that we can easily split them out. I've just noticed that the
new context stuff didn't follow that, so will fix that up asap.
-Daniel


> >>+{
> >>+	static unsigned int has_modifiers, cap_modifiers_tested;
> >>+	uint64_t cap_modifiers;
> >>+	int ret;
> >>+
> >>+	if (cap_modifiers_tested)
> >>+		return has_modifiers;
> >>+
> >>+	ret = drmGetCap(fd, LOCAL_DRM_CAP_ADDFB2_MODIFIERS, &cap_modifiers);
> >>+	igt_assert(ret == 0 || errno == EINVAL);
> >>+	has_modifiers = ret == 0 && cap_modifiers == 1;
> >>+	cap_modifiers_tested = 1;
> >>+
> >>+	if (has_modifiers)
> >>+		igt_debug("DRM_CAP_ADDFB2_MODIFIERS\n");
> >>+
> >>+	return has_modifiers;
> >>+}
> >>diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> >>index 30ab836..c277012 100644
> >>--- a/lib/ioctl_wrappers.h
> >>+++ b/lib/ioctl_wrappers.h
> >>@@ -117,4 +117,40 @@ int gem_context_has_param(int fd, uint64_t param);
> >>  int gem_context_get_param(int fd, struct local_i915_gem_context_param *p);
> >>  int gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
> >>
> >>+struct local_drm_mode_fb_cmd2 {
> >>+	uint32_t fb_id;
> >>+	uint32_t width, height;
> >>+	uint32_t pixel_format;
> >>+	uint32_t flags;
> >>+	uint32_t handles[4];
> >>+	uint32_t pitches[4];
> >>+	uint32_t offsets[4];
> >>+	uint64_t modifier[4];
> >>+};
> >>+
> >>+#define LOCAL_DRM_MODE_FB_MODIFIERS	(1<<1)
> >>+
> >>+#define LOCAL_DRM_FORMAT_MOD_VENDOR_INTEL   0x01
> >>+
> >>+#define local_fourcc_mod_code(vendor, val) \
> >>+	((((uint64_t)LOCAL_DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | \
> >>+	  (val & 0x00ffffffffffffffL))
> >>+
> >>+#define LOCAL_I915_FORMAT_MOD_NONE	local_fourcc_mod_code(INTEL, \
> >>+							      0x00000000000000L)
> >>+#define LOCAL_I915_FORMAT_MOD_X_TILED	local_fourcc_mod_code(INTEL, \
> >>+							      0x00000000000001L)
> >>+
> >>+#define LOCAL_DRM_IOCTL_MODE_ADDFB2	DRM_IOWR(0xB8, \
> >>+						 struct local_drm_mode_fb_cmd2)
> >>+
> >>+int _drmModeAddFB2(int fd, uint32_t width, uint32_t height,
> >>+		   uint32_t pixel_format, uint32_t bo_handles[4],
> >>+		   uint32_t pitches[4], uint32_t offsets[4],
> >>+		   uint64_t modifier[0], uint32_t *buf_id, uint32_t flags);
> >>+
> >>+#define LOCAL_DRM_CAP_ADDFB2_MODIFIERS	0x10
> >>+
> >>+unsigned int has_drm_fb_modifiers(int fd);
> >>+
> >>  #endif /* IOCTL_WRAPPERS_H */
> >>diff --git a/tests/kms_addfb.c b/tests/kms_addfb.c
> >>index 756589e..9b0f77c 100644
> >>--- a/tests/kms_addfb.c
> >>+++ b/tests/kms_addfb.c
> >>@@ -213,6 +213,66 @@ static void size_tests(int fd)
> >>  	}
> >>  }
> >>
> >>+static void addfb25_tests(int fd)
> >>+{
> >>+	struct local_drm_mode_fb_cmd2 f = {};
> >>+
> >>+
> >>+	igt_require(has_drm_fb_modifiers(fd));
> >
> >Imo better to move the igt_require int igt_main so that ppl aren't
> >suprised that they're tests are skipping when they add more below the call
> >to addfb25_tests.
> 
> Skipping individual test cases was my intention, but I overlooked the fact
> this function is not a subtest... I suppose igt_subtest in igt_subtest is
> not possible. :)
> 
> >>+
> >>+	memset(&f, 0, sizeof(f));
> >>+
> >>+	f.width = 1024;
> >>+	f.height = 1024;
> >>+	f.pixel_format = DRM_FORMAT_XRGB8888;
> >>+	f.pitches[0] = 1024*4;
> >>+	f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
> >>+	f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
> >
> >c99 initializes nicely combine the memset with the explicit struct init.
> >But just a bikeshed.
> 
> I just copy pasted from libdrm, presumably at some point similar code will
> appear there and then after some years this can be removed. :)
> 
> >>+
> >>+	igt_fixture {
> >>+		gem_bo = gem_create(fd, 1024*1024*4);
> >>+		igt_assert(gem_bo);
> >>+	}
> >>+
> >>+	f.handles[0] = gem_bo;
> >>+
> >>+	igt_subtest("addfb25-X-tiled") {
> >>+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
> >>+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
> >>+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
> >>+		f.fb_id = 0;
> >>+	}
> >>+
> >>+	igt_subtest("addfb25-framebuffer-vs-set-tiling") {
> >>+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
> >>+		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 512*4) == -EBUSY);
> >>+		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4) == -EBUSY);
> >>+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
> >>+		f.fb_id = 0;
> >>+	}
> >>+
> >>+	igt_fixture
> >>+		gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4);
> >>+	f.pitches[0] = 1024*4;
> >>+
> >>+	igt_subtest("addfb25-X-tiled-both") {
> >>+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
> >>+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
> >>+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
> >>+		f.fb_id = 0;
> >>+	}
> >>+
> >>+	igt_subtest("addfb25-X-tiled-mismatch") {
> >>+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
> >>+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
> >>+		f.fb_id = 0;
> >>+	}
> >
> >Looks good, but two abuse cases are imo missing:
> >
> >- f.flags = 0 but f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED
> >- f.flags = LOCAL_DRM_MODE_FB_MODIFIERS but f.modifier[0] = 0
> >
> >Just to make sure we have that part of the addfb extension covered, too.
> 
> Test cases did look suspiciously few to me yesterday but obviously testing
> mindset wasn't fully on - you are right for those two.
> 
> Thanks,
> 
> Tvrtko

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/kms_addfb: Add support for fb modifiers

[Tvrtko Ursulin]

On 02/06/2015 09:33 AM, Daniel Vetter wrote:
> On Thu, Feb 05, 2015 at 02:50:09PM +0000, Tvrtko Ursulin wrote:
>>
>> On 02/05/2015 02:21 PM, Daniel Vetter wrote:
>>> On Wed, Feb 04, 2015 at 04:42:14PM +0000, Tvrtko Ursulin wrote:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> Just a few basic tests to make sure fb modifiers can be used and
>>>> behave sanely when mixed with the old set_tiling API.
>>>>
>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>> ---
>>>>   lib/ioctl_wrappers.c | 45 ++++++++++++++++++++++++++++++++++++++
>>>>   lib/ioctl_wrappers.h | 36 ++++++++++++++++++++++++++++++
>>>>   tests/kms_addfb.c    | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>   3 files changed, 143 insertions(+)
>>>>
>>>> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
>>>> index 19a457a..bca6d2a 100644
>>>> --- a/lib/ioctl_wrappers.c
>>>> +++ b/lib/ioctl_wrappers.c
>>>> @@ -1091,3 +1091,48 @@ int gem_context_has_param(int fd, uint64_t param)
>>>>
>>>>   	return gem_context_get_param(fd, &p) == 0;
>>>>   }
>>>> +
>>>
>>> gtkdoc is missing here. Easiest way to avoid it is to just move these two
>>> wrappers into the kms_fb testcase. Whomever needs to reuse these gets to
>>> write the docs.
>>
>> I'll need it so will fix myself.
>
> A then we'll need to do some proper abi polish I think.
>>
>>>> +int _drmModeAddFB2(int fd, uint32_t width, uint32_t height,
>>>> +		   uint32_t pixel_format, uint32_t bo_handles[4],
>>>> +		   uint32_t pitches[4], uint32_t offsets[4],
>>>> +		   uint64_t modifier[0], uint32_t *buf_id, uint32_t flags)
>
> No camelcase for library functions. Also I think we want a kms_ prefix
> here for a bit of namespacing. And since your test only creates
> framebuffers with 1 plane I think simplifying the function interface would
> be good to. Finally we use a __ prefix for raw functions (i.e. those
> returning errno) in the ioctl wrapper library. And I'd put the bo handle
> right after the fd to be more consistent with other functions. And usually
> we put out-parameters last. So
>
> int __kms_addfb(int fd, unint32_t handle,
> 		width, height, stride, pixel_format, modifier, flags,
> 		uint32_t *buf_id);

I wanted to stay close to the drmModeAddFB2 as in libdrm, expecting new 
flavour for fb modifiers there will stay close to that API. Benefit 
would be trivial edits to move over to the official API and ability to 
drop the internal copy&paste when that happens.

And looked more convenient for users like igt_create_fb_with_bo_size who 
are using drmModeAddFB2 today, and are expected to grow features in the 
future.

Seems trivial to me, but if you feel strongly about this sure I'll 
change it.

>>>> +{
>>>> +	struct local_drm_mode_fb_cmd2 f;
>>>> +	int ret;
>>>> +
>>>> +	f.width  = width;
>>>> +	f.height = height;
>>>> +	f.pixel_format = pixel_format;
>>>> +	f.flags = flags;
>>>> +
>>>> +	memcpy(f.handles, bo_handles, 4 * sizeof(bo_handles[0]));
>>>> +	memcpy(f.pitches, pitches, 4 * sizeof(pitches[0]));
>>>> +	memcpy(f.offsets, offsets, 4 * sizeof(offsets[0]));
>>>> +	memcpy(f.modifier, modifier, 4 * sizeof(modifier[0]));
>>>> +
>>>> +	if ((ret = drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f)))
>>>> +		return ret < 0 ? -errno : ret;
>>>> +
>>>> +	*buf_id = f.fb_id;
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +unsigned int has_drm_fb_modifiers(int fd)
>
> Common patterin is to pusht he require into this function and use a void
> return value. Also drm prefix is used for core drm and libdrm stuff,
> better to pick kms_. So

Yes if I wanted it to be called igt_require_fb_modifiers, but it wasn't 
exactly the case. Idea was slightly different usage in future work, 
outside the kms_addfb test.

Regards,

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

[PATCH v2] tests/kms_addfb: Add support for fb modifiers

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Just a few basic tests to make sure fb modifiers can be used and
behave sanely when mixed with the old set_tiling API.

v2:
   * Review feedback from Daniel Vetter:
      1. Move cap detection into the subtest so skipping works.
      2. Added some gtkdoc comments.
      3. Two more test cases.
      4. Removed unused parts for now.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/ioctl_wrappers.c | 17 ++++++++++
 lib/ioctl_wrappers.h | 37 ++++++++++++++++++++++
 tests/kms_addfb.c    | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 19a457a..0a7841a 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -1091,3 +1091,20 @@ int gem_context_has_param(int fd, uint64_t param)
 
 	return gem_context_get_param(fd, &p) == 0;
 }
+
+void igt_require_fb_modifiers(int fd)
+{
+	static unsigned int has_modifiers, cap_modifiers_tested;
+
+	if (!cap_modifiers_tested) {
+		uint64_t cap_modifiers;
+		int ret;
+
+		ret = drmGetCap(fd, LOCAL_DRM_CAP_ADDFB2_MODIFIERS, &cap_modifiers);
+		igt_assert(ret == 0 || errno == EINVAL);
+		has_modifiers = ret == 0 && cap_modifiers == 1;
+		cap_modifiers_tested = 1;
+	}
+
+	igt_require(has_modifiers);
+}
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 30ab836..9459130 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -117,4 +117,41 @@ int gem_context_has_param(int fd, uint64_t param);
 int gem_context_get_param(int fd, struct local_i915_gem_context_param *p);
 int gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
 
+struct local_drm_mode_fb_cmd2 {
+	uint32_t fb_id;
+	uint32_t width, height;
+	uint32_t pixel_format;
+	uint32_t flags;
+	uint32_t handles[4];
+	uint32_t pitches[4];
+	uint32_t offsets[4];
+	uint64_t modifier[4];
+};
+
+#define LOCAL_DRM_MODE_FB_MODIFIERS	(1<<1)
+
+#define LOCAL_DRM_FORMAT_MOD_VENDOR_INTEL   0x01
+
+#define local_fourcc_mod_code(vendor, val) \
+	((((uint64_t)LOCAL_DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | \
+	  (val & 0x00ffffffffffffffL))
+
+#define LOCAL_I915_FORMAT_MOD_NONE	local_fourcc_mod_code(INTEL, \
+							      0x00000000000000L)
+#define LOCAL_I915_FORMAT_MOD_X_TILED	local_fourcc_mod_code(INTEL, \
+							      0x00000000000001L)
+
+#define LOCAL_DRM_IOCTL_MODE_ADDFB2	DRM_IOWR(0xB8, \
+						 struct local_drm_mode_fb_cmd2)
+
+#define LOCAL_DRM_CAP_ADDFB2_MODIFIERS	0x10
+
+/**
+ * igt_require_fb_modifiers:
+ * @fd: Open DRM file descriptor.
+ *
+ * Requires presence of DRM_CAP_ADDFB2_MODIFIERS.
+ */
+void igt_require_fb_modifiers(int fd);
+
 #endif /* IOCTL_WRAPPERS_H */
diff --git a/tests/kms_addfb.c b/tests/kms_addfb.c
index 756589e..d47f8d0 100644
--- a/tests/kms_addfb.c
+++ b/tests/kms_addfb.c
@@ -213,6 +213,91 @@ static void size_tests(int fd)
 	}
 }
 
+static void addfb25_tests(int fd)
+{
+	struct local_drm_mode_fb_cmd2 f = {};
+
+
+	memset(&f, 0, sizeof(f));
+
+	f.width = 1024;
+	f.height = 1024;
+	f.pixel_format = DRM_FORMAT_XRGB8888;
+	f.pitches[0] = 1024*4;
+	f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
+	f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
+
+	igt_fixture {
+		gem_bo = gem_create(fd, 1024*1024*4);
+		igt_assert(gem_bo);
+	}
+
+	f.handles[0] = gem_bo;
+
+	igt_subtest("addfb25-modifier-no-flag") {
+		igt_require_fb_modifiers(fd);
+
+		f.flags = 0;
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
+		f.fb_id = 0;
+	}
+
+	f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
+
+	igt_subtest("addfb25-zero-modifier") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = 0;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
+		f.fb_id = 0;
+	}
+
+	igt_subtest("addfb25-X-tiled") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+		f.fb_id = 0;
+	}
+
+	igt_subtest("addfb25-framebuffer-vs-set-tiling") {
+		igt_require_fb_modifiers(fd);
+
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 512*4) == -EBUSY);
+		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4) == -EBUSY);
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+		f.fb_id = 0;
+	}
+
+	igt_fixture
+		gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4);
+	f.pitches[0] = 1024*4;
+
+	igt_subtest("addfb25-X-tiled-both") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+		f.fb_id = 0;
+	}
+
+	igt_subtest("addfb25-X-tiled-mismatch") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
+		f.fb_id = 0;
+	}
+
+	igt_fixture {
+		gem_close(fd, gem_bo);
+	}
+}
+
 int fd;
 
 igt_main
@@ -224,6 +309,8 @@ igt_main
 
 	size_tests(fd);
 
+	addfb25_tests(fd);
+
 	igt_fixture
 		close(fd);
 }
-- 
2.2.2

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

[PATCH] tests/kms_addfb: Add support for fb modifiers

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Just a few basic tests to make sure fb modifiers can be used and
behave sanely when mixed with the old set_tiling API.

v2:
   * Review feedback from Daniel Vetter:
      1. Move cap detection into the subtest so skipping works.
      2. Added some gtkdoc comments.
      3. Two more test cases.
      4. Removed unused parts for now.

v3:
   * Removed two tests which do not make sense any more after the
     fb modifier rewrite.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/ioctl_wrappers.c | 17 +++++++++++++
 lib/ioctl_wrappers.h | 37 ++++++++++++++++++++++++++++
 tests/kms_addfb.c    | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 122 insertions(+)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 19a457a..0a7841a 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -1091,3 +1091,20 @@ int gem_context_has_param(int fd, uint64_t param)
 
 	return gem_context_get_param(fd, &p) == 0;
 }
+
+void igt_require_fb_modifiers(int fd)
+{
+	static unsigned int has_modifiers, cap_modifiers_tested;
+
+	if (!cap_modifiers_tested) {
+		uint64_t cap_modifiers;
+		int ret;
+
+		ret = drmGetCap(fd, LOCAL_DRM_CAP_ADDFB2_MODIFIERS, &cap_modifiers);
+		igt_assert(ret == 0 || errno == EINVAL);
+		has_modifiers = ret == 0 && cap_modifiers == 1;
+		cap_modifiers_tested = 1;
+	}
+
+	igt_require(has_modifiers);
+}
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 30ab836..9459130 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -117,4 +117,41 @@ int gem_context_has_param(int fd, uint64_t param);
 int gem_context_get_param(int fd, struct local_i915_gem_context_param *p);
 int gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
 
+struct local_drm_mode_fb_cmd2 {
+	uint32_t fb_id;
+	uint32_t width, height;
+	uint32_t pixel_format;
+	uint32_t flags;
+	uint32_t handles[4];
+	uint32_t pitches[4];
+	uint32_t offsets[4];
+	uint64_t modifier[4];
+};
+
+#define LOCAL_DRM_MODE_FB_MODIFIERS	(1<<1)
+
+#define LOCAL_DRM_FORMAT_MOD_VENDOR_INTEL   0x01
+
+#define local_fourcc_mod_code(vendor, val) \
+	((((uint64_t)LOCAL_DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | \
+	  (val & 0x00ffffffffffffffL))
+
+#define LOCAL_I915_FORMAT_MOD_NONE	local_fourcc_mod_code(INTEL, \
+							      0x00000000000000L)
+#define LOCAL_I915_FORMAT_MOD_X_TILED	local_fourcc_mod_code(INTEL, \
+							      0x00000000000001L)
+
+#define LOCAL_DRM_IOCTL_MODE_ADDFB2	DRM_IOWR(0xB8, \
+						 struct local_drm_mode_fb_cmd2)
+
+#define LOCAL_DRM_CAP_ADDFB2_MODIFIERS	0x10
+
+/**
+ * igt_require_fb_modifiers:
+ * @fd: Open DRM file descriptor.
+ *
+ * Requires presence of DRM_CAP_ADDFB2_MODIFIERS.
+ */
+void igt_require_fb_modifiers(int fd);
+
 #endif /* IOCTL_WRAPPERS_H */
diff --git a/tests/kms_addfb.c b/tests/kms_addfb.c
index 756589e..625aa27 100644
--- a/tests/kms_addfb.c
+++ b/tests/kms_addfb.c
@@ -213,6 +213,72 @@ static void size_tests(int fd)
 	}
 }
 
+static void addfb25_tests(int fd)
+{
+	struct local_drm_mode_fb_cmd2 f = {};
+
+
+	memset(&f, 0, sizeof(f));
+
+	f.width = 1024;
+	f.height = 1024;
+	f.pixel_format = DRM_FORMAT_XRGB8888;
+	f.pitches[0] = 1024*4;
+	f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
+
+	igt_fixture {
+		gem_bo = gem_create(fd, 1024*1024*4);
+		igt_assert(gem_bo);
+	}
+
+	f.handles[0] = gem_bo;
+
+	igt_subtest("addfb25-modifier-no-flag") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
+		f.fb_id = 0;
+	}
+
+	f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
+
+	igt_fixture
+		gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4);
+	f.pitches[0] = 1024*4;
+
+	igt_subtest("addfb25-X-tiled-mismatch") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
+		f.fb_id = 0;
+	}
+
+	igt_subtest("addfb25-X-tiled") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+		f.fb_id = 0;
+	}
+
+	igt_subtest("addfb25-framebuffer-vs-set-tiling") {
+		igt_require_fb_modifiers(fd);
+
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 512*4) == -EBUSY);
+		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4) == -EBUSY);
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+		f.fb_id = 0;
+	}
+
+	igt_fixture {
+		gem_close(fd, gem_bo);
+	}
+}
+
 int fd;
 
 igt_main
@@ -224,6 +290,8 @@ igt_main
 
 	size_tests(fd);
 
+	addfb25_tests(fd);
+
 	igt_fixture
 		close(fd);
 }
-- 
2.2.2

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

Re: [PATCH] tests/kms_addfb: Add support for fb modifiers

[Daniel Vetter]

On Tue, Feb 10, 2015 at 05:17:34PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Just a few basic tests to make sure fb modifiers can be used and
> behave sanely when mixed with the old set_tiling API.
> 
> v2:
>    * Review feedback from Daniel Vetter:
>       1. Move cap detection into the subtest so skipping works.
>       2. Added some gtkdoc comments.
>       3. Two more test cases.
>       4. Removed unused parts for now.
> 
> v3:
>    * Removed two tests which do not make sense any more after the
>      fb modifier rewrite.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  lib/ioctl_wrappers.c | 17 +++++++++++++
>  lib/ioctl_wrappers.h | 37 ++++++++++++++++++++++++++++
>  tests/kms_addfb.c    | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 122 insertions(+)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 19a457a..0a7841a 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -1091,3 +1091,20 @@ int gem_context_has_param(int fd, uint64_t param)
>  
>  	return gem_context_get_param(fd, &p) == 0;
>  }
> +
> +void igt_require_fb_modifiers(int fd)

gtkdoc is missing here.
-Daniel

> +{
> +	static unsigned int has_modifiers, cap_modifiers_tested;
> +
> +	if (!cap_modifiers_tested) {
> +		uint64_t cap_modifiers;
> +		int ret;
> +
> +		ret = drmGetCap(fd, LOCAL_DRM_CAP_ADDFB2_MODIFIERS, &cap_modifiers);
> +		igt_assert(ret == 0 || errno == EINVAL);
> +		has_modifiers = ret == 0 && cap_modifiers == 1;
> +		cap_modifiers_tested = 1;
> +	}
> +
> +	igt_require(has_modifiers);
> +}
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index 30ab836..9459130 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -117,4 +117,41 @@ int gem_context_has_param(int fd, uint64_t param);
>  int gem_context_get_param(int fd, struct local_i915_gem_context_param *p);
>  int gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
>  
> +struct local_drm_mode_fb_cmd2 {
> +	uint32_t fb_id;
> +	uint32_t width, height;
> +	uint32_t pixel_format;
> +	uint32_t flags;
> +	uint32_t handles[4];
> +	uint32_t pitches[4];
> +	uint32_t offsets[4];
> +	uint64_t modifier[4];
> +};
> +
> +#define LOCAL_DRM_MODE_FB_MODIFIERS	(1<<1)
> +
> +#define LOCAL_DRM_FORMAT_MOD_VENDOR_INTEL   0x01
> +
> +#define local_fourcc_mod_code(vendor, val) \
> +	((((uint64_t)LOCAL_DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | \
> +	  (val & 0x00ffffffffffffffL))
> +
> +#define LOCAL_I915_FORMAT_MOD_NONE	local_fourcc_mod_code(INTEL, \
> +							      0x00000000000000L)
> +#define LOCAL_I915_FORMAT_MOD_X_TILED	local_fourcc_mod_code(INTEL, \
> +							      0x00000000000001L)
> +
> +#define LOCAL_DRM_IOCTL_MODE_ADDFB2	DRM_IOWR(0xB8, \
> +						 struct local_drm_mode_fb_cmd2)
> +
> +#define LOCAL_DRM_CAP_ADDFB2_MODIFIERS	0x10
> +
> +/**
> + * igt_require_fb_modifiers:
> + * @fd: Open DRM file descriptor.
> + *
> + * Requires presence of DRM_CAP_ADDFB2_MODIFIERS.
> + */
> +void igt_require_fb_modifiers(int fd);
> +
>  #endif /* IOCTL_WRAPPERS_H */
> diff --git a/tests/kms_addfb.c b/tests/kms_addfb.c
> index 756589e..625aa27 100644
> --- a/tests/kms_addfb.c
> +++ b/tests/kms_addfb.c
> @@ -213,6 +213,72 @@ static void size_tests(int fd)
>  	}
>  }
>  
> +static void addfb25_tests(int fd)
> +{
> +	struct local_drm_mode_fb_cmd2 f = {};
> +
> +
> +	memset(&f, 0, sizeof(f));
> +
> +	f.width = 1024;
> +	f.height = 1024;
> +	f.pixel_format = DRM_FORMAT_XRGB8888;
> +	f.pitches[0] = 1024*4;
> +	f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
> +
> +	igt_fixture {
> +		gem_bo = gem_create(fd, 1024*1024*4);
> +		igt_assert(gem_bo);
> +	}
> +
> +	f.handles[0] = gem_bo;
> +
> +	igt_subtest("addfb25-modifier-no-flag") {
> +		igt_require_fb_modifiers(fd);
> +
> +		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
> +		f.fb_id = 0;
> +	}
> +
> +	f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
> +
> +	igt_fixture
> +		gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4);
> +	f.pitches[0] = 1024*4;
> +
> +	igt_subtest("addfb25-X-tiled-mismatch") {
> +		igt_require_fb_modifiers(fd);
> +
> +		f.modifier[0] = LOCAL_I915_FORMAT_MOD_NONE;
> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
> +		f.fb_id = 0;
> +	}
> +
> +	igt_subtest("addfb25-X-tiled") {
> +		igt_require_fb_modifiers(fd);
> +
> +		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
> +		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
> +		f.fb_id = 0;
> +	}
> +
> +	igt_subtest("addfb25-framebuffer-vs-set-tiling") {
> +		igt_require_fb_modifiers(fd);
> +
> +		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
> +		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 512*4) == -EBUSY);
> +		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4) == -EBUSY);
> +		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
> +		f.fb_id = 0;
> +	}
> +
> +	igt_fixture {
> +		gem_close(fd, gem_bo);
> +	}
> +}
> +
>  int fd;
>  
>  igt_main
> @@ -224,6 +290,8 @@ igt_main
>  
>  	size_tests(fd);
>  
> +	addfb25_tests(fd);
> +
>  	igt_fixture
>  		close(fd);
>  }
> -- 
> 2.2.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/kms_addfb: Add support for fb modifiers

[Tvrtko Ursulin]

On 02/11/2015 08:15 AM, Daniel Vetter wrote:
> On Tue, Feb 10, 2015 at 05:17:34PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Just a few basic tests to make sure fb modifiers can be used and
>> behave sanely when mixed with the old set_tiling API.
>>
>> v2:
>>     * Review feedback from Daniel Vetter:
>>        1. Move cap detection into the subtest so skipping works.
>>        2. Added some gtkdoc comments.
>>        3. Two more test cases.
>>        4. Removed unused parts for now.
>>
>> v3:
>>     * Removed two tests which do not make sense any more after the
>>       fb modifier rewrite.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   lib/ioctl_wrappers.c | 17 +++++++++++++
>>   lib/ioctl_wrappers.h | 37 ++++++++++++++++++++++++++++
>>   tests/kms_addfb.c    | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 122 insertions(+)
>>
>> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
>> index 19a457a..0a7841a 100644
>> --- a/lib/ioctl_wrappers.c
>> +++ b/lib/ioctl_wrappers.c
>> @@ -1091,3 +1091,20 @@ int gem_context_has_param(int fd, uint64_t param)
>>
>>   	return gem_context_get_param(fd, &p) == 0;
>>   }
>> +
>> +void igt_require_fb_modifiers(int fd)
>
> gtkdoc is missing here.

It's in the header - has to be in .c ?

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

Re: [PATCH] tests/kms_addfb: Add support for fb modifiers

[Daniel Vetter]

On Wed, Feb 11, 2015 at 09:51:22AM +0000, Tvrtko Ursulin wrote:
> 
> On 02/11/2015 08:15 AM, Daniel Vetter wrote:
> >On Tue, Feb 10, 2015 at 05:17:34PM +0000, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>Just a few basic tests to make sure fb modifiers can be used and
> >>behave sanely when mixed with the old set_tiling API.
> >>
> >>v2:
> >>    * Review feedback from Daniel Vetter:
> >>       1. Move cap detection into the subtest so skipping works.
> >>       2. Added some gtkdoc comments.
> >>       3. Two more test cases.
> >>       4. Removed unused parts for now.
> >>
> >>v3:
> >>    * Removed two tests which do not make sense any more after the
> >>      fb modifier rewrite.
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>---
> >>  lib/ioctl_wrappers.c | 17 +++++++++++++
> >>  lib/ioctl_wrappers.h | 37 ++++++++++++++++++++++++++++
> >>  tests/kms_addfb.c    | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 122 insertions(+)
> >>
> >>diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> >>index 19a457a..0a7841a 100644
> >>--- a/lib/ioctl_wrappers.c
> >>+++ b/lib/ioctl_wrappers.c
> >>@@ -1091,3 +1091,20 @@ int gem_context_has_param(int fd, uint64_t param)
> >>
> >>  	return gem_context_get_param(fd, &p) == 0;
> >>  }
> >>+
> >>+void igt_require_fb_modifiers(int fd)
> >
> >gtkdoc is missing here.
> 
> It's in the header - has to be in .c ?

Yeah we generally put them all into the .c file. Increases the chance that
when someone changes the code they update docs. And having them all
together makes it easier to keep the wording consistent for same concepts.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] tests/kms_addfb: Add support for fb modifiers

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Just a few basic tests to make sure fb modifiers can be used and
behave sanely when mixed with the old set_tiling API.

v2:
   * Review feedback from Daniel Vetter:
      1. Move cap detection into the subtest so skipping works.
      2. Added some gtkdoc comments.
      3. Two more test cases.
      4. Removed unused parts for now.

v3:
   * Removed two tests which do not make sense any more after the
     fb modifier rewrite.

v4:
   * Moved gtkdoc comments into .c file.
   * Moved all initialization into fixtures.
   * Rebased for fb modifier changes.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 lib/ioctl_wrappers.c | 23 +++++++++++++++++++
 lib/ioctl_wrappers.h | 29 +++++++++++++++++++++++
 tests/kms_addfb.c    | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 117 insertions(+)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 19a457a..47e9cbe 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -1091,3 +1091,26 @@ int gem_context_has_param(int fd, uint64_t param)
 
 	return gem_context_get_param(fd, &p) == 0;
 }
+
+/**
+ * igt_require_fb_modifiers:
+ * @fd: Open DRM file descriptor.
+ *
+ * Requires presence of DRM_CAP_ADDFB2_MODIFIERS.
+ */
+void igt_require_fb_modifiers(int fd)
+{
+	static bool has_modifiers, cap_modifiers_tested;
+
+	if (!cap_modifiers_tested) {
+		uint64_t cap_modifiers;
+		int ret;
+
+		ret = drmGetCap(fd, LOCAL_DRM_CAP_ADDFB2_MODIFIERS, &cap_modifiers);
+		igt_assert(ret == 0 || errno == EINVAL);
+		has_modifiers = ret == 0 && cap_modifiers == 1;
+		cap_modifiers_tested = true;
+	}
+
+	igt_require(has_modifiers);
+}
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index 30ab836..eae19a3 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -117,4 +117,33 @@ int gem_context_has_param(int fd, uint64_t param);
 int gem_context_get_param(int fd, struct local_i915_gem_context_param *p);
 int gem_context_set_param(int fd, struct local_i915_gem_context_param *p);
 
+struct local_drm_mode_fb_cmd2 {
+	uint32_t fb_id;
+	uint32_t width, height;
+	uint32_t pixel_format;
+	uint32_t flags;
+	uint32_t handles[4];
+	uint32_t pitches[4];
+	uint32_t offsets[4];
+	uint64_t modifier[4];
+};
+
+#define LOCAL_DRM_MODE_FB_MODIFIERS	(1<<1)
+
+#define LOCAL_DRM_FORMAT_MOD_VENDOR_INTEL   0x01
+
+#define local_fourcc_mod_code(vendor, val) \
+	((((uint64_t)LOCAL_DRM_FORMAT_MOD_VENDOR_## vendor) << 56) | \
+	  (val & 0x00ffffffffffffffL))
+
+#define LOCAL_DRM_FORMAT_MOD_NONE	(0)
+#define LOCAL_I915_FORMAT_MOD_X_TILED	local_fourcc_mod_code(INTEL, 1)
+
+#define LOCAL_DRM_IOCTL_MODE_ADDFB2	DRM_IOWR(0xB8, \
+						 struct local_drm_mode_fb_cmd2)
+
+#define LOCAL_DRM_CAP_ADDFB2_MODIFIERS	0x10
+
+void igt_require_fb_modifiers(int fd);
+
 #endif /* IOCTL_WRAPPERS_H */
diff --git a/tests/kms_addfb.c b/tests/kms_addfb.c
index 756589e..0a82619 100644
--- a/tests/kms_addfb.c
+++ b/tests/kms_addfb.c
@@ -213,6 +213,69 @@ static void size_tests(int fd)
 	}
 }
 
+static void addfb25_tests(int fd)
+{
+	struct local_drm_mode_fb_cmd2 f = {};
+
+	igt_fixture {
+		gem_bo = gem_create(fd, 1024*1024*4);
+		igt_assert(gem_bo);
+
+		memset(&f, 0, sizeof(f));
+
+		f.width = 1024;
+		f.height = 1024;
+		f.pixel_format = DRM_FORMAT_XRGB8888;
+		f.pitches[0] = 1024*4;
+		f.modifier[0] = LOCAL_DRM_FORMAT_MOD_NONE;
+
+		f.handles[0] = gem_bo;
+	}
+
+	igt_subtest("addfb25-modifier-no-flag") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
+	}
+
+	igt_fixture {
+		gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4);
+		f.flags = LOCAL_DRM_MODE_FB_MODIFIERS;
+	}
+
+	igt_subtest("addfb25-X-tiled-mismatch") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = LOCAL_DRM_FORMAT_MOD_NONE;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) < 0 && errno == EINVAL);
+	}
+
+	igt_subtest("addfb25-X-tiled") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+		f.fb_id = 0;
+	}
+
+	igt_subtest("addfb25-framebuffer-vs-set-tiling") {
+		igt_require_fb_modifiers(fd);
+
+		f.modifier[0] = LOCAL_I915_FORMAT_MOD_X_TILED;
+		igt_assert(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 512*4) == -EBUSY);
+		igt_assert(__gem_set_tiling(fd, gem_bo, I915_TILING_X, 1024*4) == -EBUSY);
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+		f.fb_id = 0;
+	}
+
+	igt_fixture {
+		gem_close(fd, gem_bo);
+	}
+}
+
 int fd;
 
 igt_main
@@ -224,6 +287,8 @@ igt_main
 
 	size_tests(fd);
 
+	addfb25_tests(fd);
+
 	igt_fixture
 		close(fd);
 }
-- 
2.2.2

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