| Message ID | 20161214090509.15716-9-robert.foss@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Wed, Dec 14, 2016 at 04:05:05AM -0500, Robert Foss wrote: >From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> > >Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> >Signed-off-by: Robert Foss <robert.foss@collabora.com> >--- > tests/kms_atomic.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 176 insertions(+), 10 deletions(-) > >diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c >index 8b648eba..a557ac60 100644 >--- a/tests/kms_atomic.c >+++ b/tests/kms_atomic.c >@@ -44,6 +44,7 @@ > #include "drmtest.h" > #include "igt.h" > #include "igt_aux.h" >+#include "sw_sync.h" > > #ifndef DRM_CLIENT_CAP_ATOMIC > #define DRM_CLIENT_CAP_ATOMIC 3 >@@ -126,6 +127,7 @@ struct kms_atomic_plane_state { > uint32_t fb_id; /* 0 to disable */ > uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-point */ > uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal integers */ >+ int32_t fence_fd; > }; > > struct kms_atomic_crtc_state { >@@ -133,6 +135,7 @@ struct kms_atomic_crtc_state { > uint32_t obj; > int idx; > bool active; >+ uint64_t out_fence_ptr; > struct kms_atomic_blob mode; > }; > >@@ -190,11 +193,13 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig) > crtc_populate_req(crtc, req); \ > plane_populate_req(plane, req); \ > do_atomic_commit((crtc)->state->desc->fd, req, flags); \ >- crtc_check_current_state(crtc, plane, relax); \ >- plane_check_current_state(plane, relax); \ >+ if (!(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { \ >+ crtc_check_current_state(crtc, plane, relax); \ >+ plane_check_current_state(plane, relax); \ >+ } \ > } > >-#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, relax, e) { \ >+#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, flags, relax, e) { \ > drmModeAtomicSetCursor(req, 0); \ > crtc_populate_req(crtc, req); \ > plane_populate_req(plane, req); \ >@@ -299,6 +304,9 @@ find_connector(struct kms_atomic_state *state, > static void plane_populate_req(struct kms_atomic_plane_state *plane, > drmModeAtomicReq *req) > { >+ if (plane->fence_fd) >+ plane_set_prop(req, plane, IGT_PLANE_IN_FENCE_FD, plane->fence_fd); >+ > plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane->crtc_id); > plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane->fb_id); > plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane->src_x); >@@ -424,6 +432,10 @@ find_plane(struct kms_atomic_state *state, enum plane_type type, > static void crtc_populate_req(struct kms_atomic_crtc_state *crtc, > drmModeAtomicReq *req) > { >+ if (crtc->out_fence_ptr) >+ crtc_set_prop(req, crtc, IGT_CRTC_OUT_FENCE_PTR, >+ crtc->out_fence_ptr); >+ > crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc->mode.id); > crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active); > } >@@ -1052,6 +1064,37 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc, > drmModeAtomicFree(req); > } > >+static void plane_invalid_params_fence(struct kms_atomic_crtc_state *crtc, >+ struct kms_atomic_plane_state *plane_old, >+ struct kms_atomic_connector_state *conn) >+{ >+ struct kms_atomic_plane_state plane = *plane_old; >+ drmModeAtomicReq *req = drmModeAtomicAlloc(); >+ int timeline, fence_fd; >+ >+ igt_require_sw_sync(); >+ >+ /* invalid fence fd */ >+ plane.fence_fd = plane.state->desc->fd; >+ plane.crtc_id = plane_old->crtc_id; >+ plane_commit_atomic_err(&plane, plane_old, req, >+ ATOMIC_RELAX_NONE, EINVAL); >+ >+ /* Valid fence_fd but invalid CRTC */ >+ timeline = sw_sync_timeline_create(); >+ fence_fd = sw_sync_fence_create(timeline, 1); >+ plane.fence_fd = fence_fd; >+ plane.crtc_id = ~0U; >+ plane_commit_atomic_err(&plane, plane_old, req, >+ ATOMIC_RELAX_NONE, EINVAL); >+ >+ plane.fence_fd = -1; >+ close(fence_fd); >+ close(timeline); >+ >+ drmModeAtomicFree(req); >+} >+ > static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old, > struct kms_atomic_plane_state *plane, > struct kms_atomic_connector_state *conn) >@@ -1063,30 +1106,32 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old, > > /* Pass a series of invalid object IDs for the mode ID. */ > crtc.mode.id = plane->obj; >- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, > ATOMIC_RELAX_NONE, EINVAL); > > crtc.mode.id = crtc.obj; >- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, > ATOMIC_RELAX_NONE, EINVAL); > > crtc.mode.id = conn->obj; >- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, > ATOMIC_RELAX_NONE, EINVAL); > > crtc.mode.id = plane->fb_id; >- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, > ATOMIC_RELAX_NONE, EINVAL); > >+ /* successful TEST_ONLY with fences set */ > crtc.mode.id = crtc_old->mode.id; >- crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0); >+ crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, >+ DRM_MODE_ATOMIC_TEST_ONLY); > > /* Create a blob which is the wrong size to be a valid mode. */ > do_or_die(drmModeCreatePropertyBlob(crtc.state->desc->fd, > crtc.mode.data, > sizeof(struct drm_mode_modeinfo) - 1, > &crtc.mode.id)); >- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, > ATOMIC_RELAX_NONE, EINVAL); > > >@@ -1094,15 +1139,107 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old, > crtc.mode.data, > sizeof(struct drm_mode_modeinfo) + 1, > &crtc.mode.id)); >- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, > ATOMIC_RELAX_NONE, EINVAL); > >+ > /* Restore the CRTC and check the state matches the old. */ > crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0); > > drmModeAtomicFree(req); > } > >+static void crtc_invalid_params_fence(struct kms_atomic_crtc_state *crtc_old, >+ struct kms_atomic_plane_state *plane, >+ struct kms_atomic_connector_state *conn) >+{ >+ struct kms_atomic_crtc_state crtc = *crtc_old; >+ drmModeAtomicReq *req = drmModeAtomicAlloc(); >+ int timeline, fence_fd, *out_fence; >+ >+ igt_require_sw_sync(); >+ >+ /* invalid out_fence_ptr */ >+ crtc.mode.id = crtc_old->mode.id; >+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) crtc_invalid_params; >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >+ ATOMIC_RELAX_NONE, EFAULT); >+ >+ /* invalid out_fence_ptr */ >+ crtc.mode.id = crtc_old->mode.id; >+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0x8; >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >+ ATOMIC_RELAX_NONE, EFAULT); >+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >+ >+ /* valid in fence but not allowed prop on crtc */ >+ timeline = sw_sync_timeline_create(); >+ fence_fd = sw_sync_fence_create(timeline, 1); >+ plane->fence_fd = fence_fd; >+ crtc.active = false; >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >+ ATOMIC_RELAX_NONE, EINVAL); >+ >+ out_fence = malloc(sizeof(uint64_t)); >+ igt_assert(out_fence); >+ >+ >+ /* valid out fence ptr and flip event but not allowed prop on crtc */ >+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >+ ATOMIC_RELAX_NONE, EINVAL); >+ >+ /* valid out fence ptr and flip event but not allowed prop on crtc */ >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >+ DRM_MODE_PAGE_FLIP_EVENT, >+ ATOMIC_RELAX_NONE, EINVAL); >+ >+ /* valid page flip event but not allowed prop on crtc */ >+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >+ DRM_MODE_PAGE_FLIP_EVENT, >+ ATOMIC_RELAX_NONE, EINVAL); >+ crtc.active = true; >+ >+ /* valid out fence ptr and flip event but invalid prop on crtc */ >+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; >+ crtc.mode.id = plane->fb_id; >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >+ ATOMIC_RELAX_NONE, EINVAL); >+ >+ /* valid out fence ptr and flip event but invalid prop on crtc */ >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >+ DRM_MODE_PAGE_FLIP_EVENT, >+ ATOMIC_RELAX_NONE, EINVAL); >+ >+ /* valid page flip event but invalid prop on crtc */ >+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >+ DRM_MODE_PAGE_FLIP_EVENT, >+ ATOMIC_RELAX_NONE, EINVAL); >+ >+ /* successful TEST_ONLY with fences set */ >+ plane->fence_fd = fence_fd; >+ crtc.mode.id = crtc_old->mode.id; >+ crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, >+ DRM_MODE_ATOMIC_TEST_ONLY); >+ igt_assert(*out_fence == -1); >+ close(fence_fd); >+ close(timeline); >+ >+ /* reset fences */ >+ plane->fence_fd = -1; >+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >+ crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0); >+ >+ /* out fence ptr but not page flip event */ >+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; >+ crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0); What are the previous two commits for? They don't seem to be invalid. >+ >+ free(out_fence); Need to close(*out_fence) before freeing it? -Brian >+ drmModeAtomicFree(req); >+} >+ > /* Abuse the atomic ioctl directly in order to test various invalid conditions, > * which the libdrm wrapper won't allow us to create. */ > static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc, >@@ -1306,6 +1443,20 @@ igt_main > atomic_state_free(scratch); > } > >+ igt_subtest("plane_invalid_params_fence") { >+ struct kms_atomic_state *scratch = atomic_state_dup(current); >+ struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >+ struct kms_atomic_plane_state *plane = >+ find_plane(current, PLANE_TYPE_PRIMARY, crtc); >+ struct kms_atomic_connector_state *conn = >+ find_connector(scratch, crtc); >+ >+ igt_require(crtc); >+ igt_require(plane); >+ plane_invalid_params_fence(crtc, plane, conn); >+ atomic_state_free(scratch); >+ } >+ > igt_subtest("crtc_invalid_params") { > struct kms_atomic_state *scratch = atomic_state_dup(current); > struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >@@ -1321,6 +1472,21 @@ igt_main > atomic_state_free(scratch); > } > >+ igt_subtest("crtc_invalid_params_fence") { >+ struct kms_atomic_state *scratch = atomic_state_dup(current); >+ struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >+ struct kms_atomic_plane_state *plane = >+ find_plane(scratch, NUM_PLANE_TYPE_PROPS, crtc); >+ struct kms_atomic_connector_state *conn = >+ find_connector(scratch, crtc); >+ >+ igt_require(crtc); >+ igt_require(plane); >+ igt_require(conn); >+ crtc_invalid_params_fence(crtc, plane, conn); >+ atomic_state_free(scratch); >+ } >+ > igt_subtest("atomic_invalid_params") { > struct kms_atomic_state *scratch = atomic_state_dup(current); > struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >-- >2.11.0 >
On 2016-12-14 11:39 AM, Brian Starkey wrote: > Hi, > > On Wed, Dec 14, 2016 at 04:05:05AM -0500, Robert Foss wrote: >> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> >> >> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> >> Signed-off-by: Robert Foss <robert.foss@collabora.com> >> --- >> tests/kms_atomic.c | 186 >> ++++++++++++++++++++++++++++++++++++++++++++++++++--- >> 1 file changed, 176 insertions(+), 10 deletions(-) >> >> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c >> index 8b648eba..a557ac60 100644 >> --- a/tests/kms_atomic.c >> +++ b/tests/kms_atomic.c >> @@ -44,6 +44,7 @@ >> #include "drmtest.h" >> #include "igt.h" >> #include "igt_aux.h" >> +#include "sw_sync.h" >> >> #ifndef DRM_CLIENT_CAP_ATOMIC >> #define DRM_CLIENT_CAP_ATOMIC 3 >> @@ -126,6 +127,7 @@ struct kms_atomic_plane_state { >> uint32_t fb_id; /* 0 to disable */ >> uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-point */ >> uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal integers */ >> + int32_t fence_fd; >> }; >> >> struct kms_atomic_crtc_state { >> @@ -133,6 +135,7 @@ struct kms_atomic_crtc_state { >> uint32_t obj; >> int idx; >> bool active; >> + uint64_t out_fence_ptr; >> struct kms_atomic_blob mode; >> }; >> >> @@ -190,11 +193,13 @@ static uint32_t blob_duplicate(int fd, uint32_t >> id_orig) >> crtc_populate_req(crtc, req); \ >> plane_populate_req(plane, req); \ >> do_atomic_commit((crtc)->state->desc->fd, req, flags); \ >> - crtc_check_current_state(crtc, plane, relax); \ >> - plane_check_current_state(plane, relax); \ >> + if (!(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { \ >> + crtc_check_current_state(crtc, plane, relax); \ >> + plane_check_current_state(plane, relax); \ >> + } \ >> } >> >> -#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, >> relax, e) { \ >> +#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, >> flags, relax, e) { \ >> drmModeAtomicSetCursor(req, 0); \ >> crtc_populate_req(crtc, req); \ >> plane_populate_req(plane, req); \ >> @@ -299,6 +304,9 @@ find_connector(struct kms_atomic_state *state, >> static void plane_populate_req(struct kms_atomic_plane_state *plane, >> drmModeAtomicReq *req) >> { >> + if (plane->fence_fd) >> + plane_set_prop(req, plane, IGT_PLANE_IN_FENCE_FD, >> plane->fence_fd); >> + >> plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane->crtc_id); >> plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane->fb_id); >> plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane->src_x); >> @@ -424,6 +432,10 @@ find_plane(struct kms_atomic_state *state, enum >> plane_type type, >> static void crtc_populate_req(struct kms_atomic_crtc_state *crtc, >> drmModeAtomicReq *req) >> { >> + if (crtc->out_fence_ptr) >> + crtc_set_prop(req, crtc, IGT_CRTC_OUT_FENCE_PTR, >> + crtc->out_fence_ptr); >> + >> crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc->mode.id); >> crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active); >> } >> @@ -1052,6 +1064,37 @@ static void plane_invalid_params(struct >> kms_atomic_crtc_state *crtc, >> drmModeAtomicFree(req); >> } >> >> +static void plane_invalid_params_fence(struct kms_atomic_crtc_state >> *crtc, >> + struct kms_atomic_plane_state *plane_old, >> + struct kms_atomic_connector_state *conn) >> +{ >> + struct kms_atomic_plane_state plane = *plane_old; >> + drmModeAtomicReq *req = drmModeAtomicAlloc(); >> + int timeline, fence_fd; >> + >> + igt_require_sw_sync(); >> + >> + /* invalid fence fd */ >> + plane.fence_fd = plane.state->desc->fd; >> + plane.crtc_id = plane_old->crtc_id; >> + plane_commit_atomic_err(&plane, plane_old, req, >> + ATOMIC_RELAX_NONE, EINVAL); >> + >> + /* Valid fence_fd but invalid CRTC */ >> + timeline = sw_sync_timeline_create(); >> + fence_fd = sw_sync_fence_create(timeline, 1); >> + plane.fence_fd = fence_fd; >> + plane.crtc_id = ~0U; >> + plane_commit_atomic_err(&plane, plane_old, req, >> + ATOMIC_RELAX_NONE, EINVAL); >> + >> + plane.fence_fd = -1; >> + close(fence_fd); >> + close(timeline); >> + >> + drmModeAtomicFree(req); >> +} >> + >> static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old, >> struct kms_atomic_plane_state *plane, >> struct kms_atomic_connector_state *conn) >> @@ -1063,30 +1106,32 @@ static void crtc_invalid_params(struct >> kms_atomic_crtc_state *crtc_old, >> >> /* Pass a series of invalid object IDs for the mode ID. */ >> crtc.mode.id = plane->obj; >> - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> ATOMIC_RELAX_NONE, EINVAL); >> >> crtc.mode.id = crtc.obj; >> - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> ATOMIC_RELAX_NONE, EINVAL); >> >> crtc.mode.id = conn->obj; >> - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> ATOMIC_RELAX_NONE, EINVAL); >> >> crtc.mode.id = plane->fb_id; >> - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> ATOMIC_RELAX_NONE, EINVAL); >> >> + /* successful TEST_ONLY with fences set */ >> crtc.mode.id = crtc_old->mode.id; >> - crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0); >> + crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, >> + DRM_MODE_ATOMIC_TEST_ONLY); >> >> /* Create a blob which is the wrong size to be a valid mode. */ >> do_or_die(drmModeCreatePropertyBlob(crtc.state->desc->fd, >> crtc.mode.data, >> sizeof(struct drm_mode_modeinfo) - 1, >> &crtc.mode.id)); >> - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> ATOMIC_RELAX_NONE, EINVAL); >> >> >> @@ -1094,15 +1139,107 @@ static void crtc_invalid_params(struct >> kms_atomic_crtc_state *crtc_old, >> crtc.mode.data, >> sizeof(struct drm_mode_modeinfo) + 1, >> &crtc.mode.id)); >> - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> ATOMIC_RELAX_NONE, EINVAL); >> >> + >> /* Restore the CRTC and check the state matches the old. */ >> crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0); >> >> drmModeAtomicFree(req); >> } >> >> +static void crtc_invalid_params_fence(struct kms_atomic_crtc_state >> *crtc_old, >> + struct kms_atomic_plane_state *plane, >> + struct kms_atomic_connector_state *conn) >> +{ >> + struct kms_atomic_crtc_state crtc = *crtc_old; >> + drmModeAtomicReq *req = drmModeAtomicAlloc(); >> + int timeline, fence_fd, *out_fence; >> + >> + igt_require_sw_sync(); >> + >> + /* invalid out_fence_ptr */ >> + crtc.mode.id = crtc_old->mode.id; >> + crtc.out_fence_ptr = (uint64_t)(uintptr_t) crtc_invalid_params; >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> + ATOMIC_RELAX_NONE, EFAULT); >> + >> + /* invalid out_fence_ptr */ >> + crtc.mode.id = crtc_old->mode.id; >> + crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0x8; >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> + ATOMIC_RELAX_NONE, EFAULT); >> + crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >> + >> + /* valid in fence but not allowed prop on crtc */ >> + timeline = sw_sync_timeline_create(); >> + fence_fd = sw_sync_fence_create(timeline, 1); >> + plane->fence_fd = fence_fd; >> + crtc.active = false; >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> + ATOMIC_RELAX_NONE, EINVAL); >> + >> + out_fence = malloc(sizeof(uint64_t)); >> + igt_assert(out_fence); >> + >> + >> + /* valid out fence ptr and flip event but not allowed prop on >> crtc */ >> + crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> + ATOMIC_RELAX_NONE, EINVAL); >> + >> + /* valid out fence ptr and flip event but not allowed prop on >> crtc */ >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >> + DRM_MODE_PAGE_FLIP_EVENT, >> + ATOMIC_RELAX_NONE, EINVAL); >> + >> + /* valid page flip event but not allowed prop on crtc */ >> + crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >> + DRM_MODE_PAGE_FLIP_EVENT, >> + ATOMIC_RELAX_NONE, EINVAL); >> + crtc.active = true; >> + >> + /* valid out fence ptr and flip event but invalid prop on crtc */ >> + crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; >> + crtc.mode.id = plane->fb_id; >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >> + ATOMIC_RELAX_NONE, EINVAL); >> + >> + /* valid out fence ptr and flip event but invalid prop on crtc */ >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >> + DRM_MODE_PAGE_FLIP_EVENT, >> + ATOMIC_RELAX_NONE, EINVAL); >> + >> + /* valid page flip event but invalid prop on crtc */ >> + crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >> + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >> + DRM_MODE_PAGE_FLIP_EVENT, >> + ATOMIC_RELAX_NONE, EINVAL); >> + >> + /* successful TEST_ONLY with fences set */ >> + plane->fence_fd = fence_fd; >> + crtc.mode.id = crtc_old->mode.id; >> + crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, >> + DRM_MODE_ATOMIC_TEST_ONLY); >> + igt_assert(*out_fence == -1); >> + close(fence_fd); >> + close(timeline); >> + >> + /* reset fences */ >> + plane->fence_fd = -1; >> + crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >> + crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0); >> + >> + /* out fence ptr but not page flip event */ >> + crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; >> + crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0); > > What are the previous two commits for? They don't seem to be invalid. They don't seem to be invalid to me either, would you prefer to have them separated out into a subtest without invalid in its name? > >> + >> + free(out_fence); > > Need to close(*out_fence) before freeing it? > > -Brian Yep, fixing in v3. > >> + drmModeAtomicFree(req); >> +} >> + >> /* Abuse the atomic ioctl directly in order to test various invalid >> conditions, >> * which the libdrm wrapper won't allow us to create. */ >> static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc, >> @@ -1306,6 +1443,20 @@ igt_main >> atomic_state_free(scratch); >> } >> >> + igt_subtest("plane_invalid_params_fence") { >> + struct kms_atomic_state *scratch = atomic_state_dup(current); >> + struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >> + struct kms_atomic_plane_state *plane = >> + find_plane(current, PLANE_TYPE_PRIMARY, crtc); >> + struct kms_atomic_connector_state *conn = >> + find_connector(scratch, crtc); >> + >> + igt_require(crtc); >> + igt_require(plane); >> + plane_invalid_params_fence(crtc, plane, conn); >> + atomic_state_free(scratch); >> + } >> + >> igt_subtest("crtc_invalid_params") { >> struct kms_atomic_state *scratch = atomic_state_dup(current); >> struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >> @@ -1321,6 +1472,21 @@ igt_main >> atomic_state_free(scratch); >> } >> >> + igt_subtest("crtc_invalid_params_fence") { >> + struct kms_atomic_state *scratch = atomic_state_dup(current); >> + struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >> + struct kms_atomic_plane_state *plane = >> + find_plane(scratch, NUM_PLANE_TYPE_PROPS, crtc); >> + struct kms_atomic_connector_state *conn = >> + find_connector(scratch, crtc); >> + >> + igt_require(crtc); >> + igt_require(plane); >> + igt_require(conn); >> + crtc_invalid_params_fence(crtc, plane, conn); >> + atomic_state_free(scratch); >> + } >> + >> igt_subtest("atomic_invalid_params") { >> struct kms_atomic_state *scratch = atomic_state_dup(current); >> struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >> -- >> 2.11.0 >>
On Fri, Dec 16, 2016 at 03:35:36AM -0500, Robert Foss wrote: > > >On 2016-12-14 11:39 AM, Brian Starkey wrote: >>Hi, >> >>On Wed, Dec 14, 2016 at 04:05:05AM -0500, Robert Foss wrote: >>>From: Gustavo Padovan <gustavo.padovan@collabora.co.uk> >>> >>>Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk> >>>Signed-off-by: Robert Foss <robert.foss@collabora.com> >>>--- >>>tests/kms_atomic.c | 186 >>>++++++++++++++++++++++++++++++++++++++++++++++++++--- >>>1 file changed, 176 insertions(+), 10 deletions(-) >>> >>>diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c >>>index 8b648eba..a557ac60 100644 >>>--- a/tests/kms_atomic.c >>>+++ b/tests/kms_atomic.c >>>@@ -44,6 +44,7 @@ >>>#include "drmtest.h" >>>#include "igt.h" >>>#include "igt_aux.h" >>>+#include "sw_sync.h" >>> >>>#ifndef DRM_CLIENT_CAP_ATOMIC >>>#define DRM_CLIENT_CAP_ATOMIC 3 >>>@@ -126,6 +127,7 @@ struct kms_atomic_plane_state { >>> uint32_t fb_id; /* 0 to disable */ >>> uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-point */ >>> uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal integers */ >>>+ int32_t fence_fd; >>>}; >>> >>>struct kms_atomic_crtc_state { >>>@@ -133,6 +135,7 @@ struct kms_atomic_crtc_state { >>> uint32_t obj; >>> int idx; >>> bool active; >>>+ uint64_t out_fence_ptr; >>> struct kms_atomic_blob mode; >>>}; >>> >>>@@ -190,11 +193,13 @@ static uint32_t blob_duplicate(int fd, uint32_t >>>id_orig) >>> crtc_populate_req(crtc, req); \ >>> plane_populate_req(plane, req); \ >>> do_atomic_commit((crtc)->state->desc->fd, req, flags); \ >>>- crtc_check_current_state(crtc, plane, relax); \ >>>- plane_check_current_state(plane, relax); \ >>>+ if (!(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { \ >>>+ crtc_check_current_state(crtc, plane, relax); \ >>>+ plane_check_current_state(plane, relax); \ >>>+ } \ >>>} >>> >>>-#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, >>>relax, e) { \ >>>+#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, >>>flags, relax, e) { \ >>> drmModeAtomicSetCursor(req, 0); \ >>> crtc_populate_req(crtc, req); \ >>> plane_populate_req(plane, req); \ >>>@@ -299,6 +304,9 @@ find_connector(struct kms_atomic_state *state, >>>static void plane_populate_req(struct kms_atomic_plane_state *plane, >>> drmModeAtomicReq *req) >>>{ >>>+ if (plane->fence_fd) >>>+ plane_set_prop(req, plane, IGT_PLANE_IN_FENCE_FD, >>>plane->fence_fd); >>>+ >>> plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane->crtc_id); >>> plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane->fb_id); >>> plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane->src_x); >>>@@ -424,6 +432,10 @@ find_plane(struct kms_atomic_state *state, enum >>>plane_type type, >>>static void crtc_populate_req(struct kms_atomic_crtc_state *crtc, >>> drmModeAtomicReq *req) >>>{ >>>+ if (crtc->out_fence_ptr) >>>+ crtc_set_prop(req, crtc, IGT_CRTC_OUT_FENCE_PTR, >>>+ crtc->out_fence_ptr); >>>+ >>> crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc->mode.id); >>> crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active); >>>} >>>@@ -1052,6 +1064,37 @@ static void plane_invalid_params(struct >>>kms_atomic_crtc_state *crtc, >>> drmModeAtomicFree(req); >>>} >>> >>>+static void plane_invalid_params_fence(struct kms_atomic_crtc_state >>>*crtc, >>>+ struct kms_atomic_plane_state *plane_old, >>>+ struct kms_atomic_connector_state *conn) >>>+{ >>>+ struct kms_atomic_plane_state plane = *plane_old; >>>+ drmModeAtomicReq *req = drmModeAtomicAlloc(); >>>+ int timeline, fence_fd; >>>+ >>>+ igt_require_sw_sync(); >>>+ >>>+ /* invalid fence fd */ >>>+ plane.fence_fd = plane.state->desc->fd; >>>+ plane.crtc_id = plane_old->crtc_id; >>>+ plane_commit_atomic_err(&plane, plane_old, req, >>>+ ATOMIC_RELAX_NONE, EINVAL); >>>+ >>>+ /* Valid fence_fd but invalid CRTC */ >>>+ timeline = sw_sync_timeline_create(); >>>+ fence_fd = sw_sync_fence_create(timeline, 1); >>>+ plane.fence_fd = fence_fd; >>>+ plane.crtc_id = ~0U; >>>+ plane_commit_atomic_err(&plane, plane_old, req, >>>+ ATOMIC_RELAX_NONE, EINVAL); >>>+ >>>+ plane.fence_fd = -1; >>>+ close(fence_fd); >>>+ close(timeline); >>>+ >>>+ drmModeAtomicFree(req); >>>+} >>>+ >>>static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old, >>> struct kms_atomic_plane_state *plane, >>> struct kms_atomic_connector_state *conn) >>>@@ -1063,30 +1106,32 @@ static void crtc_invalid_params(struct >>>kms_atomic_crtc_state *crtc_old, >>> >>> /* Pass a series of invalid object IDs for the mode ID. */ >>> crtc.mode.id = plane->obj; >>>- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>> ATOMIC_RELAX_NONE, EINVAL); >>> >>> crtc.mode.id = crtc.obj; >>>- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>> ATOMIC_RELAX_NONE, EINVAL); >>> >>> crtc.mode.id = conn->obj; >>>- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>> ATOMIC_RELAX_NONE, EINVAL); >>> >>> crtc.mode.id = plane->fb_id; >>>- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>> ATOMIC_RELAX_NONE, EINVAL); >>> >>>+ /* successful TEST_ONLY with fences set */ >>> crtc.mode.id = crtc_old->mode.id; >>>- crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0); >>>+ crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, >>>+ DRM_MODE_ATOMIC_TEST_ONLY); >>> >>> /* Create a blob which is the wrong size to be a valid mode. */ >>> do_or_die(drmModeCreatePropertyBlob(crtc.state->desc->fd, >>> crtc.mode.data, >>> sizeof(struct drm_mode_modeinfo) - 1, >>> &crtc.mode.id)); >>>- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>> ATOMIC_RELAX_NONE, EINVAL); >>> >>> >>>@@ -1094,15 +1139,107 @@ static void crtc_invalid_params(struct >>>kms_atomic_crtc_state *crtc_old, >>> crtc.mode.data, >>> sizeof(struct drm_mode_modeinfo) + 1, >>> &crtc.mode.id)); >>>- crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>> ATOMIC_RELAX_NONE, EINVAL); >>> >>>+ >>> /* Restore the CRTC and check the state matches the old. */ >>> crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0); >>> >>> drmModeAtomicFree(req); >>>} >>> >>>+static void crtc_invalid_params_fence(struct kms_atomic_crtc_state >>>*crtc_old, >>>+ struct kms_atomic_plane_state *plane, >>>+ struct kms_atomic_connector_state *conn) >>>+{ >>>+ struct kms_atomic_crtc_state crtc = *crtc_old; >>>+ drmModeAtomicReq *req = drmModeAtomicAlloc(); >>>+ int timeline, fence_fd, *out_fence; >>>+ >>>+ igt_require_sw_sync(); >>>+ >>>+ /* invalid out_fence_ptr */ >>>+ crtc.mode.id = crtc_old->mode.id; >>>+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) crtc_invalid_params; >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>>+ ATOMIC_RELAX_NONE, EFAULT); >>>+ >>>+ /* invalid out_fence_ptr */ >>>+ crtc.mode.id = crtc_old->mode.id; >>>+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0x8; >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>>+ ATOMIC_RELAX_NONE, EFAULT); >>>+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >>>+ >>>+ /* valid in fence but not allowed prop on crtc */ >>>+ timeline = sw_sync_timeline_create(); >>>+ fence_fd = sw_sync_fence_create(timeline, 1); >>>+ plane->fence_fd = fence_fd; >>>+ crtc.active = false; >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>>+ ATOMIC_RELAX_NONE, EINVAL); >>>+ >>>+ out_fence = malloc(sizeof(uint64_t)); >>>+ igt_assert(out_fence); >>>+ >>>+ >>>+ /* valid out fence ptr and flip event but not allowed prop on >>>crtc */ >>>+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>>+ ATOMIC_RELAX_NONE, EINVAL); >>>+ >>>+ /* valid out fence ptr and flip event but not allowed prop on >>>crtc */ >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >>>+ DRM_MODE_PAGE_FLIP_EVENT, >>>+ ATOMIC_RELAX_NONE, EINVAL); >>>+ >>>+ /* valid page flip event but not allowed prop on crtc */ >>>+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >>>+ DRM_MODE_PAGE_FLIP_EVENT, >>>+ ATOMIC_RELAX_NONE, EINVAL); >>>+ crtc.active = true; >>>+ >>>+ /* valid out fence ptr and flip event but invalid prop on crtc */ >>>+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; >>>+ crtc.mode.id = plane->fb_id; >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, >>>+ ATOMIC_RELAX_NONE, EINVAL); >>>+ >>>+ /* valid out fence ptr and flip event but invalid prop on crtc */ >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >>>+ DRM_MODE_PAGE_FLIP_EVENT, >>>+ ATOMIC_RELAX_NONE, EINVAL); >>>+ >>>+ /* valid page flip event but invalid prop on crtc */ >>>+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >>>+ crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, >>>+ DRM_MODE_PAGE_FLIP_EVENT, >>>+ ATOMIC_RELAX_NONE, EINVAL); >>>+ >>>+ /* successful TEST_ONLY with fences set */ >>>+ plane->fence_fd = fence_fd; >>>+ crtc.mode.id = crtc_old->mode.id; >>>+ crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, >>>+ DRM_MODE_ATOMIC_TEST_ONLY); >>>+ igt_assert(*out_fence == -1); >>>+ close(fence_fd); >>>+ close(timeline); >>>+ >>>+ /* reset fences */ >>>+ plane->fence_fd = -1; >>>+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; >>>+ crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0); >>>+ >>>+ /* out fence ptr but not page flip event */ >>>+ crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; >>>+ crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0); >> >>What are the previous two commits for? They don't seem to be invalid. > >They don't seem to be invalid to me either, would you prefer to have >them separated out into a subtest without invalid in its name? > If you don't think splitting them out is too excessive, then yeah. I think the last one isn't really needed at all - isn't it exercising the same code-paths as the normal, valid "*-fencing" tests? Anyway, at least a comment saying they aren't invalid could save some head-scratching. Cheers, Brian >> >>>+ >>>+ free(out_fence); >> >>Need to close(*out_fence) before freeing it? >> >>-Brian > >Yep, fixing in v3. > >> >>>+ drmModeAtomicFree(req); >>>+} >>>+ >>>/* Abuse the atomic ioctl directly in order to test various invalid >>>conditions, >>> * which the libdrm wrapper won't allow us to create. */ >>>static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc, >>>@@ -1306,6 +1443,20 @@ igt_main >>> atomic_state_free(scratch); >>> } >>> >>>+ igt_subtest("plane_invalid_params_fence") { >>>+ struct kms_atomic_state *scratch = atomic_state_dup(current); >>>+ struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >>>+ struct kms_atomic_plane_state *plane = >>>+ find_plane(current, PLANE_TYPE_PRIMARY, crtc); >>>+ struct kms_atomic_connector_state *conn = >>>+ find_connector(scratch, crtc); >>>+ >>>+ igt_require(crtc); >>>+ igt_require(plane); >>>+ plane_invalid_params_fence(crtc, plane, conn); >>>+ atomic_state_free(scratch); >>>+ } >>>+ >>> igt_subtest("crtc_invalid_params") { >>> struct kms_atomic_state *scratch = atomic_state_dup(current); >>> struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >>>@@ -1321,6 +1472,21 @@ igt_main >>> atomic_state_free(scratch); >>> } >>> >>>+ igt_subtest("crtc_invalid_params_fence") { >>>+ struct kms_atomic_state *scratch = atomic_state_dup(current); >>>+ struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >>>+ struct kms_atomic_plane_state *plane = >>>+ find_plane(scratch, NUM_PLANE_TYPE_PROPS, crtc); >>>+ struct kms_atomic_connector_state *conn = >>>+ find_connector(scratch, crtc); >>>+ >>>+ igt_require(crtc); >>>+ igt_require(plane); >>>+ igt_require(conn); >>>+ crtc_invalid_params_fence(crtc, plane, conn); >>>+ atomic_state_free(scratch); >>>+ } >>>+ >>> igt_subtest("atomic_invalid_params") { >>> struct kms_atomic_state *scratch = atomic_state_dup(current); >>> struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); >>>-- >>>2.11.0 >>>
diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c index 8b648eba..a557ac60 100644 --- a/tests/kms_atomic.c +++ b/tests/kms_atomic.c @@ -44,6 +44,7 @@ #include "drmtest.h" #include "igt.h" #include "igt_aux.h" +#include "sw_sync.h" #ifndef DRM_CLIENT_CAP_ATOMIC #define DRM_CLIENT_CAP_ATOMIC 3 @@ -126,6 +127,7 @@ struct kms_atomic_plane_state { uint32_t fb_id; /* 0 to disable */ uint32_t src_x, src_y, src_w, src_h; /* 16.16 fixed-point */ uint32_t crtc_x, crtc_y, crtc_w, crtc_h; /* normal integers */ + int32_t fence_fd; }; struct kms_atomic_crtc_state { @@ -133,6 +135,7 @@ struct kms_atomic_crtc_state { uint32_t obj; int idx; bool active; + uint64_t out_fence_ptr; struct kms_atomic_blob mode; }; @@ -190,11 +193,13 @@ static uint32_t blob_duplicate(int fd, uint32_t id_orig) crtc_populate_req(crtc, req); \ plane_populate_req(plane, req); \ do_atomic_commit((crtc)->state->desc->fd, req, flags); \ - crtc_check_current_state(crtc, plane, relax); \ - plane_check_current_state(plane, relax); \ + if (!(flags & DRM_MODE_ATOMIC_TEST_ONLY)) { \ + crtc_check_current_state(crtc, plane, relax); \ + plane_check_current_state(plane, relax); \ + } \ } -#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, relax, e) { \ +#define crtc_commit_atomic_err(crtc, plane, crtc_old, plane_old, req, flags, relax, e) { \ drmModeAtomicSetCursor(req, 0); \ crtc_populate_req(crtc, req); \ plane_populate_req(plane, req); \ @@ -299,6 +304,9 @@ find_connector(struct kms_atomic_state *state, static void plane_populate_req(struct kms_atomic_plane_state *plane, drmModeAtomicReq *req) { + if (plane->fence_fd) + plane_set_prop(req, plane, IGT_PLANE_IN_FENCE_FD, plane->fence_fd); + plane_set_prop(req, plane, IGT_PLANE_CRTC_ID, plane->crtc_id); plane_set_prop(req, plane, IGT_PLANE_FB_ID, plane->fb_id); plane_set_prop(req, plane, IGT_PLANE_SRC_X, plane->src_x); @@ -424,6 +432,10 @@ find_plane(struct kms_atomic_state *state, enum plane_type type, static void crtc_populate_req(struct kms_atomic_crtc_state *crtc, drmModeAtomicReq *req) { + if (crtc->out_fence_ptr) + crtc_set_prop(req, crtc, IGT_CRTC_OUT_FENCE_PTR, + crtc->out_fence_ptr); + crtc_set_prop(req, crtc, IGT_CRTC_MODE_ID, crtc->mode.id); crtc_set_prop(req, crtc, IGT_CRTC_ACTIVE, crtc->active); } @@ -1052,6 +1064,37 @@ static void plane_invalid_params(struct kms_atomic_crtc_state *crtc, drmModeAtomicFree(req); } +static void plane_invalid_params_fence(struct kms_atomic_crtc_state *crtc, + struct kms_atomic_plane_state *plane_old, + struct kms_atomic_connector_state *conn) +{ + struct kms_atomic_plane_state plane = *plane_old; + drmModeAtomicReq *req = drmModeAtomicAlloc(); + int timeline, fence_fd; + + igt_require_sw_sync(); + + /* invalid fence fd */ + plane.fence_fd = plane.state->desc->fd; + plane.crtc_id = plane_old->crtc_id; + plane_commit_atomic_err(&plane, plane_old, req, + ATOMIC_RELAX_NONE, EINVAL); + + /* Valid fence_fd but invalid CRTC */ + timeline = sw_sync_timeline_create(); + fence_fd = sw_sync_fence_create(timeline, 1); + plane.fence_fd = fence_fd; + plane.crtc_id = ~0U; + plane_commit_atomic_err(&plane, plane_old, req, + ATOMIC_RELAX_NONE, EINVAL); + + plane.fence_fd = -1; + close(fence_fd); + close(timeline); + + drmModeAtomicFree(req); +} + static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old, struct kms_atomic_plane_state *plane, struct kms_atomic_connector_state *conn) @@ -1063,30 +1106,32 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old, /* Pass a series of invalid object IDs for the mode ID. */ crtc.mode.id = plane->obj; - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, ATOMIC_RELAX_NONE, EINVAL); crtc.mode.id = crtc.obj; - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, ATOMIC_RELAX_NONE, EINVAL); crtc.mode.id = conn->obj; - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, ATOMIC_RELAX_NONE, EINVAL); crtc.mode.id = plane->fb_id; - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, ATOMIC_RELAX_NONE, EINVAL); + /* successful TEST_ONLY with fences set */ crtc.mode.id = crtc_old->mode.id; - crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0); + crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, + DRM_MODE_ATOMIC_TEST_ONLY); /* Create a blob which is the wrong size to be a valid mode. */ do_or_die(drmModeCreatePropertyBlob(crtc.state->desc->fd, crtc.mode.data, sizeof(struct drm_mode_modeinfo) - 1, &crtc.mode.id)); - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, ATOMIC_RELAX_NONE, EINVAL); @@ -1094,15 +1139,107 @@ static void crtc_invalid_params(struct kms_atomic_crtc_state *crtc_old, crtc.mode.data, sizeof(struct drm_mode_modeinfo) + 1, &crtc.mode.id)); - crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, ATOMIC_RELAX_NONE, EINVAL); + /* Restore the CRTC and check the state matches the old. */ crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0); drmModeAtomicFree(req); } +static void crtc_invalid_params_fence(struct kms_atomic_crtc_state *crtc_old, + struct kms_atomic_plane_state *plane, + struct kms_atomic_connector_state *conn) +{ + struct kms_atomic_crtc_state crtc = *crtc_old; + drmModeAtomicReq *req = drmModeAtomicAlloc(); + int timeline, fence_fd, *out_fence; + + igt_require_sw_sync(); + + /* invalid out_fence_ptr */ + crtc.mode.id = crtc_old->mode.id; + crtc.out_fence_ptr = (uint64_t)(uintptr_t) crtc_invalid_params; + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, + ATOMIC_RELAX_NONE, EFAULT); + + /* invalid out_fence_ptr */ + crtc.mode.id = crtc_old->mode.id; + crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0x8; + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, + ATOMIC_RELAX_NONE, EFAULT); + crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; + + /* valid in fence but not allowed prop on crtc */ + timeline = sw_sync_timeline_create(); + fence_fd = sw_sync_fence_create(timeline, 1); + plane->fence_fd = fence_fd; + crtc.active = false; + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, + ATOMIC_RELAX_NONE, EINVAL); + + out_fence = malloc(sizeof(uint64_t)); + igt_assert(out_fence); + + + /* valid out fence ptr and flip event but not allowed prop on crtc */ + crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, + ATOMIC_RELAX_NONE, EINVAL); + + /* valid out fence ptr and flip event but not allowed prop on crtc */ + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, + DRM_MODE_PAGE_FLIP_EVENT, + ATOMIC_RELAX_NONE, EINVAL); + + /* valid page flip event but not allowed prop on crtc */ + crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, + DRM_MODE_PAGE_FLIP_EVENT, + ATOMIC_RELAX_NONE, EINVAL); + crtc.active = true; + + /* valid out fence ptr and flip event but invalid prop on crtc */ + crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; + crtc.mode.id = plane->fb_id; + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, 0, + ATOMIC_RELAX_NONE, EINVAL); + + /* valid out fence ptr and flip event but invalid prop on crtc */ + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, + DRM_MODE_PAGE_FLIP_EVENT, + ATOMIC_RELAX_NONE, EINVAL); + + /* valid page flip event but invalid prop on crtc */ + crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; + crtc_commit_atomic_err(&crtc, plane, crtc_old, plane, req, + DRM_MODE_PAGE_FLIP_EVENT, + ATOMIC_RELAX_NONE, EINVAL); + + /* successful TEST_ONLY with fences set */ + plane->fence_fd = fence_fd; + crtc.mode.id = crtc_old->mode.id; + crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, + DRM_MODE_ATOMIC_TEST_ONLY); + igt_assert(*out_fence == -1); + close(fence_fd); + close(timeline); + + /* reset fences */ + plane->fence_fd = -1; + crtc.out_fence_ptr = (uint64_t)(uintptr_t) 0; + crtc_commit_atomic(&crtc, plane, req, ATOMIC_RELAX_NONE, 0); + + /* out fence ptr but not page flip event */ + crtc.out_fence_ptr = (uint64_t)(uintptr_t) out_fence; + crtc_commit_atomic(crtc_old, plane, req, ATOMIC_RELAX_NONE, 0); + + free(out_fence); + drmModeAtomicFree(req); +} + /* Abuse the atomic ioctl directly in order to test various invalid conditions, * which the libdrm wrapper won't allow us to create. */ static void atomic_invalid_params(struct kms_atomic_crtc_state *crtc, @@ -1306,6 +1443,20 @@ igt_main atomic_state_free(scratch); } + igt_subtest("plane_invalid_params_fence") { + struct kms_atomic_state *scratch = atomic_state_dup(current); + struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); + struct kms_atomic_plane_state *plane = + find_plane(current, PLANE_TYPE_PRIMARY, crtc); + struct kms_atomic_connector_state *conn = + find_connector(scratch, crtc); + + igt_require(crtc); + igt_require(plane); + plane_invalid_params_fence(crtc, plane, conn); + atomic_state_free(scratch); + } + igt_subtest("crtc_invalid_params") { struct kms_atomic_state *scratch = atomic_state_dup(current); struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); @@ -1321,6 +1472,21 @@ igt_main atomic_state_free(scratch); } + igt_subtest("crtc_invalid_params_fence") { + struct kms_atomic_state *scratch = atomic_state_dup(current); + struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true); + struct kms_atomic_plane_state *plane = + find_plane(scratch, NUM_PLANE_TYPE_PROPS, crtc); + struct kms_atomic_connector_state *conn = + find_connector(scratch, crtc); + + igt_require(crtc); + igt_require(plane); + igt_require(conn); + crtc_invalid_params_fence(crtc, plane, conn); + atomic_state_free(scratch); + } + igt_subtest("atomic_invalid_params") { struct kms_atomic_state *scratch = atomic_state_dup(current); struct kms_atomic_crtc_state *crtc = find_crtc(scratch, true);