[Intel-gfx] [PATCH i-g-t v2 08/12] tests/kms_atomic: stress possible fence settings

Brian Starkey brian.starkey at arm.com
Fri Dec 16 19:05:13 UTC 2016


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 at collabora.co.uk>
>>>
>>>Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>>>Signed-off-by: Robert Foss <robert.foss at 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
>>>


More information about the Intel-gfx mailing list