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 <[email protected]>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 <[email protected]> --- 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 [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
