[Intel-gfx] [PATCH i-g-t v4 1/7] tests: Stop looking at plane private members
Mika Kahola
mika.kahola at intel.com
Fri Sep 29 13:13:07 UTC 2017
On Fri, 2017-09-29 at 11:59 +0200, Maarten Lankhorst wrote:
> Most of these tests have no reason to look at those members,
> so try other ways of getting the information.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Reviewed-by: Mika Kahola <mika.kahola at intel.com>
> ---
> lib/igt_kms.h | 21 ++++++++
> tests/kms_atomic_transition.c | 117 +++++++++++++++++++++-----------
> ----------
> tests/kms_busy.c | 5 +-
> tests/kms_concurrent.c | 4 +-
> tests/kms_cursor_legacy.c | 6 +--
> tests/kms_fbc_crc.c | 3 +-
> tests/kms_panel_fitting.c | 14 +----
> tests/kms_plane_scaling.c | 14 +----
> tests/kms_rmfb.c | 2 +-
> tests/kms_rotation_crc.c | 10 ----
> 10 files changed, 93 insertions(+), 103 deletions(-)
>
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 3d1061fa08c8..62197dcfea7b 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -556,6 +556,27 @@ static inline bool
> igt_output_is_connected(igt_output_t *output)
> igt_assert_lt(0, drmModeAtomicAddProperty(req, output-
> >config.connector->connector_id,\
> output-
> >config.atomic_props_connector[prop], value))
>
> +/*
> + * igt_pipe_refresh:
> + * @display: a pointer to an #igt_display_t structure
> + * @pipe: Pipe to refresh
> + * @force: Should be set to true if mode_blob is no longer
> considered
> + * to be valid, for example after doing an atomic commit during fork
> or closing display fd.
> + *
> + * Requests the pipe to be part of the state on next update.
> + * This is useful when state may have been out of sync after
> + * a fork, or we just want to be sure the pipe is included
> + * in the next commit.
> + */
> +static inline void
> +igt_pipe_refresh(igt_display_t *display, enum pipe pipe, bool force)
> +{
> + if (force)
> + display->pipes[pipe].mode_blob = 0;
> +
> + display->pipes[pipe].mode_changed = true;
> +}
> +
> void igt_enable_connectors(void);
> void igt_reset_connectors(void);
>
> diff --git a/tests/kms_atomic_transition.c
> b/tests/kms_atomic_transition.c
> index e22763bdf94b..48823a09aed2 100644
> --- a/tests/kms_atomic_transition.c
> +++ b/tests/kms_atomic_transition.c
> @@ -46,6 +46,11 @@ struct plane_parms {
>
> #define hweight32 __builtin_popcount
>
> +/* globals for fence support */
> +int *timeline;
> +pthread_t *thread;
> +int *seqno;
> +
> static void
> run_primary_test(igt_display_t *display, enum pipe pipe,
> igt_output_t *output)
> {
> @@ -89,9 +94,34 @@ run_primary_test(igt_display_t *display, enum pipe
> pipe, igt_output_t *output)
> igt_remove_fb(display->drm_fd, &fb);
> }
>
> +static void *fence_inc_thread(void *arg)
> +{
> + int t = *((int *) arg);
> +
> + pthread_detach(pthread_self());
> +
> + usleep(5000);
> + sw_sync_timeline_inc(t, 1);
> + return NULL;
> +}
> +
> +static void configure_fencing(igt_plane_t *plane)
> +{
> + int i, fd, ret;
> +
> + i = plane->index;
> +
> + seqno[i]++;
> + fd = sw_sync_timeline_create_fence(timeline[i], seqno[i]);
> + igt_plane_set_fence_fd(plane, fd);
> + close(fd);
> + ret = pthread_create(&thread[i], NULL, fence_inc_thread,
> &timeline[i]);
> + igt_assert_eq(ret, 0);
> +}
> +
> static void
> wm_setup_plane(igt_display_t *display, enum pipe pipe,
> - uint32_t mask, struct plane_parms *parms)
> + uint32_t mask, struct plane_parms *parms, bool
> fencing)
> {
> igt_plane_t *plane;
>
> @@ -108,6 +138,9 @@ wm_setup_plane(igt_display_t *display, enum pipe
> pipe,
> continue;
> }
>
> + if (fencing)
> + configure_fencing(plane);
> +
> igt_plane_set_fb(plane, parms[i].fb);
> igt_fb_set_size(parms[i].fb, plane, parms[i].width,
> parms[i].height);
> igt_plane_set_size(plane, parms[i].width,
> parms[i].height);
> @@ -136,7 +169,7 @@ static bool
> skip_on_unsupported_nonblocking_modeset(igt_display_t *display)
>
> /* Force the next state to update all crtc's, to synchronize
> with the nonblocking modeset. */
> for_each_pipe(display, pipe)
> - display->pipes[pipe].mode_changed = true;
> + igt_pipe_refresh(display, pipe, false);
>
> return false;
> }
> @@ -183,6 +216,7 @@ static void set_sprite_wh(igt_display_t *display,
> enum pipe pipe,
>
> static void setup_parms(igt_display_t *display, enum pipe pipe,
> const drmModeModeInfo *mode,
> + struct igt_fb *primary_fb,
> struct igt_fb *argb_fb,
> struct igt_fb *sprite_fb,
> struct plane_parms *parms)
> @@ -205,7 +239,7 @@ static void setup_parms(igt_display_t *display,
> enum pipe pipe,
> int i = plane->index;
>
> if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> - parms[i].fb = plane->fb;
> + parms[i].fb = primary_fb;
> parms[i].width = mode->hdisplay;
> parms[i].height = mode->vdisplay;
> } else if (plane->type == DRM_PLANE_TYPE_CURSOR) {
> @@ -242,7 +276,7 @@ retry:
> set_sprite_wh(display, pipe, parms, sprite_fb,
> alpha, sprite_width, sprite_height);
>
> - wm_setup_plane(display, pipe, (1 << n_planes) - 1,
> parms);
> + wm_setup_plane(display, pipe, (1 << n_planes) - 1,
> parms, false);
> ret = igt_display_try_commit_atomic(display,
> DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>
> if (ret == -EINVAL) {
> @@ -300,10 +334,6 @@ retry:
> sprite_width, sprite_height, alpha);
> }
>
> -int *timeline;
> -pthread_t *thread;
> -int *seqno;
> -
> static void prepare_fencing(igt_display_t *display, enum pipe pipe)
> {
> igt_plane_t *plane;
> @@ -335,44 +365,10 @@ static void unprepare_fencing(igt_display_t
> *display, enum pipe pipe)
> free(seqno);
> }
>
> -static void *fence_inc_thread(void *arg)
> -{
> - int t = *((int *) arg);
> -
> - pthread_detach(pthread_self());
> -
> - usleep(5000);
> - sw_sync_timeline_inc(t, 1);
> - return NULL;
> -}
> -
> -static void configure_fencing(igt_display_t *display, enum pipe
> pipe)
> -{
> - igt_plane_t *plane;
> - int i, fd, ret;
> -
> - for_each_plane_on_pipe(display, pipe, plane) {
> -
> - if (!plane->fb)
> - continue;
> -
> - i = plane->index;
> -
> - seqno[i]++;
> - fd = sw_sync_timeline_create_fence(timeline[i],
> seqno[i]);
> - igt_plane_set_fence_fd(plane, fd);
> - close(fd);
> - ret = pthread_create(&thread[i], NULL,
> fence_inc_thread, &timeline[i]);
> - igt_assert_eq(ret, 0);
> - }
> -}
> -
> static void atomic_commit(igt_display_t *display, enum pipe pipe,
> unsigned int flags, void *data, bool fencing)
> {
> - if (fencing) {
> - configure_fencing(display, pipe);
> + if (fencing)
> igt_pipe_request_out_fence(&display->pipes[pipe]);
> - }
>
> igt_display_commit_atomic(display, flags, data);
> }
> @@ -445,7 +441,7 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
>
> igt_output_set_pipe(output, pipe);
>
> - wm_setup_plane(display, pipe, 0, NULL);
> + wm_setup_plane(display, pipe, 0, NULL, false);
>
> if (flags & DRM_MODE_ATOMIC_ALLOW_MODESET) {
> skip_test = nonblocking &&
> skip_on_unsupported_nonblocking_modeset(display);
> @@ -461,7 +457,7 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
>
> igt_display_commit2(display, COMMIT_ATOMIC);
>
> - setup_parms(display, pipe, mode, &argb_fb, &sprite_fb,
> parms);
> + setup_parms(display, pipe, mode, &fb, &argb_fb, &sprite_fb,
> parms);
>
> /*
> * In some configurations the tests may not run to
> completion with all
> @@ -469,7 +465,7 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
> * planes to fix this
> */
> while (1) {
> - wm_setup_plane(display, pipe, iter_max - 1, parms);
> + wm_setup_plane(display, pipe, iter_max - 1, parms,
> false);
>
> if (fencing)
> igt_pipe_request_out_fence(pipe_obj);
> @@ -504,7 +500,7 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
> if (type == TRANSITION_AFTER_FREE) {
> int fence_fd = -1;
>
> - wm_setup_plane(display, pipe, 0, parms);
> + wm_setup_plane(display, pipe, 0, parms, fencing);
>
> atomic_commit(display, pipe, flags, (void
> *)(unsigned long)0, fencing);
> if (fencing) {
> @@ -512,8 +508,9 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
> pipe_obj->out_fence_fd = -1;
> }
>
> + /* force planes to be part of commit */
> for_each_plane_on_pipe(display, pipe, plane)
> - plane->fb_changed = true;
> + igt_plane_set_position(plane, 0, 0);
>
> igt_display_commit2(display, COMMIT_ATOMIC);
>
> @@ -530,7 +527,7 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
> for (i = 0; i < iter_max; i++) {
> igt_output_set_pipe(output, pipe);
>
> - wm_setup_plane(display, pipe, i, parms);
> + wm_setup_plane(display, pipe, i, parms, fencing);
>
> atomic_commit(display, pipe, flags, (void
> *)(unsigned long)i, fencing);
> wait_for_transition(display, pipe, nonblocking,
> fencing);
> @@ -538,7 +535,7 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
> if (type == TRANSITION_MODESET_DISABLE) {
> igt_output_set_pipe(output, PIPE_NONE);
>
> - wm_setup_plane(display, pipe, 0, parms);
> + wm_setup_plane(display, pipe, 0, parms,
> fencing);
>
> atomic_commit(display, pipe, flags, (void *)
> 0UL, fencing);
> wait_for_transition(display, pipe,
> nonblocking, fencing);
> @@ -547,7 +544,7 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
>
> /* i -> i+1 will be done when i increases,
> can be skipped here */
> for (j = iter_max - 1; j > i + 1; j--) {
> - wm_setup_plane(display, pipe, j,
> parms);
> + wm_setup_plane(display, pipe, j,
> parms, fencing);
>
> if (type == TRANSITION_MODESET)
> igt_output_override_mode(out
> put, &override_mode);
> @@ -555,7 +552,7 @@ run_transition_test(igt_display_t *display, enum
> pipe pipe, igt_output_t *output
> atomic_commit(display, pipe, flags,
> (void *)(unsigned long) j, fencing);
> wait_for_transition(display, pipe,
> nonblocking, fencing);
>
> - wm_setup_plane(display, pipe, i,
> parms);
> + wm_setup_plane(display, pipe, i,
> parms, fencing);
> if (type == TRANSITION_MODESET)
> igt_output_override_mode(out
> put, NULL);
>
> @@ -666,15 +663,19 @@ static unsigned set_combinations(igt_display_t
> *display, unsigned mask, struct i
> return event_mask;
> }
>
> -static void refresh_primaries(igt_display_t *display)
> +static void refresh_primaries(igt_display_t *display, int mask)
> {
> enum pipe pipe;
> igt_plane_t *plane;
>
> - for_each_pipe(display, pipe)
> + for_each_pipe(display, pipe) {
> + if (!((1 << pipe) & mask))
> + continue;
> +
> for_each_plane_on_pipe(display, pipe, plane)
> - if (plane->type == DRM_PLANE_TYPE_PRIMARY &&
> plane->fb)
> - plane->fb_changed = true;
> + if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> + igt_plane_set_position(plane, 0, 0);
> + }
> }
>
> static void collect_crcs_mask(igt_pipe_crc_t **pipe_crcs, unsigned
> mask, igt_crc_t *crcs)
> @@ -783,7 +784,7 @@ static void run_modeset_tests(igt_display_t
> *display, int howmany, bool nonblock
>
> collect_crcs_mask(pipe_crcs, j, crcs[1]);
>
> - refresh_primaries(display);
> + refresh_primaries(display, j);
> commit_display(display, j, nonblocking);
> collect_crcs_mask(pipe_crcs, j, crcs[2]);
>
> @@ -794,7 +795,7 @@ static void run_modeset_tests(igt_display_t
> *display, int howmany, bool nonblock
> commit_display(display, event_mask,
> nonblocking);
> collect_crcs_mask(pipe_crcs, i, crcs[3]);
>
> - refresh_primaries(display);
> + refresh_primaries(display, i);
> commit_display(display, i, nonblocking);
> collect_crcs_mask(pipe_crcs, i, crcs[4]);
>
> diff --git a/tests/kms_busy.c b/tests/kms_busy.c
> index 7e31c2c82dc5..f430beaf7373 100644
> --- a/tests/kms_busy.c
> +++ b/tests/kms_busy.c
> @@ -146,7 +146,10 @@ static void flip_to_fb(igt_display_t *dpy, int
> pipe,
>
> if (modeset) {
> igt_set_module_param_int("enable_hangcheck", 1);
> - dpy->pipes[pipe].mode_blob = 0;
> +
> + /* Clear old mode blob. */
> + igt_pipe_refresh(dpy, pipe, true);
> +
> igt_output_set_pipe(output, pipe);
> igt_display_commit2(dpy, COMMIT_ATOMIC);
> }
> diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
> index 5d1e0bc5ad19..7f44c596b4c4 100644
> --- a/tests/kms_concurrent.c
> +++ b/tests/kms_concurrent.c
> @@ -91,7 +91,7 @@ static void test_fini(data_t *data, enum pipe pipe,
> int n_planes,
> {
> int i;
>
> - data->display.pipes[pipe].mode_blob = 0;
> + igt_pipe_refresh(&data->display, pipe, true);
>
> for (i = 0; i < n_planes; i++) {
> igt_plane_t *plane = data->plane[i];
> @@ -292,7 +292,7 @@ test_resolution_with_output(data_t *data, enum
> pipe pipe, igt_output_t *output)
>
> i = 0;
> while (i < iterations || loop_forever) {
> - data->display.pipes[pipe].mode_blob = 0;
> + igt_pipe_refresh(&data->display, pipe, true);
>
> mode_hi = igt_output_get_mode(output);
> mode_lo = get_lowres_mode(data, mode_hi, output);
> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> index ba7c3f51ba21..65a001a9dec0 100644
> --- a/tests/kms_cursor_legacy.c
> +++ b/tests/kms_cursor_legacy.c
> @@ -784,7 +784,7 @@ static bool
> skip_on_unsupported_nonblocking_modeset(igt_display_t *display)
>
> /* Force the next state to update all crtc's, to synchronize
> with the nonblocking modeset. */
> for_each_pipe(display, pipe)
> - display->pipes[pipe].mode_changed = true;
> + igt_pipe_refresh(display, pipe, false);
>
> return false;
> }
> @@ -971,8 +971,8 @@ static void
> two_screens_flip_vs_cursor(igt_display_t *display, int nloops, bool
>
> if (modeset) {
> /* wait for pending modeset and page flip to
> complete, to prevent -EBUSY */
> - display->pipes[pipe].mode_changed = true;
> - display->pipes[pipe2].mode_changed = true;
> + igt_pipe_refresh(display, pipe, false);
> + igt_pipe_refresh(display, pipe2, false);
> igt_display_commit2(display, COMMIT_ATOMIC);
> }
> }
> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> index 08abb8b4b95e..06234566c232 100644
> --- a/tests/kms_fbc_crc.c
> +++ b/tests/kms_fbc_crc.c
> @@ -461,8 +461,7 @@ static void reset_display(data_t *data)
> igt_pipe_t *pipe = &display->pipes[pipe_id];
> igt_plane_t *plane = igt_pipe_get_plane_type(pipe,
> DRM_PLANE_TYPE_PRIMARY);
>
> - if (plane->fb)
> - igt_plane_set_fb(plane, NULL);
> + igt_plane_set_fb(plane, NULL);
> }
>
> for_each_connected_output(display, data->output)
> diff --git a/tests/kms_panel_fitting.c b/tests/kms_panel_fitting.c
> index 5266862a70cf..85a231e60ea2 100644
> --- a/tests/kms_panel_fitting.c
> +++ b/tests/kms_panel_fitting.c
> @@ -84,19 +84,7 @@ static void prepare_crtc(data_t *data,
> igt_output_t *output, enum pipe pipe,
> }
>
> igt_plane_set_fb(plane, &data->fb1);
> - if (s == COMMIT_LEGACY) {
> - int ret;
> - ret = drmModeSetCrtc(data->drm_fd,
> - plane->pipe->crtc_id,
> - data->fb_id1,
> - plane->src_x, plane->src_y,
> - &output->id,
> - 1,
> - mode);
> - igt_assert_eq(ret, 0);
> - } else {
> - igt_display_commit2(display, s);
> - }
> + igt_display_commit2(display, s);
> }
>
> static void cleanup_crtc(data_t *data, igt_output_t *output,
> igt_plane_t *plane)
> diff --git a/tests/kms_plane_scaling.c b/tests/kms_plane_scaling.c
> index 1457894aaa10..5ed69f35f267 100644
> --- a/tests/kms_plane_scaling.c
> +++ b/tests/kms_plane_scaling.c
> @@ -93,19 +93,7 @@ static void prepare_crtc(data_t *data,
> igt_output_t *output, enum pipe pipe,
> }
>
> igt_plane_set_fb(plane, &data->fb1);
> - if (s == COMMIT_LEGACY) {
> - int ret;
> - ret = drmModeSetCrtc(data->drm_fd,
> - plane->pipe->crtc_id,
> - data->fb_id1,
> - plane->src_x, plane->src_y,
> - &output->id,
> - 1,
> - mode);
> - igt_assert_eq(ret, 0);
> - } else {
> - igt_display_commit2(display, s);
> - }
> + igt_display_commit2(display, s);
> }
>
> static void cleanup_crtc(data_t *data, igt_output_t *output,
> igt_plane_t *plane)
> diff --git a/tests/kms_rmfb.c b/tests/kms_rmfb.c
> index 9f1f564e54aa..c97adceea761 100644
> --- a/tests/kms_rmfb.c
> +++ b/tests/kms_rmfb.c
> @@ -102,7 +102,7 @@ test_rmfb(struct rmfb_data *data, igt_output_t
> *output, enum pipe pipe, bool reo
> drmSetClientCap(data->drm_fd,
> DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> drmSetClientCap(data->drm_fd, DRM_CLIENT_CAP_ATOMIC,
> 1);
>
> - data->display.pipes[pipe].mode_blob = 0;
> + igt_pipe_refresh(&data->display, pipe, true);
> } else {
> igt_remove_fb(data->drm_fd, &fb);
> igt_remove_fb(data->drm_fd, &argb_fb);
> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
> index 21e264addc09..4d2ef1c184f0 100644
> --- a/tests/kms_rotation_crc.c
> +++ b/tests/kms_rotation_crc.c
> @@ -465,12 +465,6 @@ static void
> test_plane_rotation_ytiled_obj(data_t *data,
> igt_plane_set_fb(plane, &data->fb);
> igt_plane_set_size(plane, h, w);
>
> - if (commit < COMMIT_ATOMIC)
> - drmModeObjectSetProperty(fd, plane->drm_plane-
> >plane_id,
> - DRM_MODE_OBJECT_PLANE,
> - plane->rotation_property,
> - plane->rotation);
> -
> ret = igt_display_try_commit2(display, commit);
>
> igt_output_set_pipe(output, PIPE_NONE);
> @@ -562,10 +556,6 @@ static void
> test_plane_rotation_exhaust_fences(data_t *data,
> igt_plane_set_rotation(plane, IGT_ROTATION_90);
> igt_plane_set_size(plane, h, w);
>
> - drmModeObjectSetProperty(fd, plane->drm_plane-
> >plane_id,
> - DRM_MODE_OBJECT_PLANE,
> - plane->rotation_property,
> - plane->rotation);
> igt_display_commit2(display, commit);
> if (ret) {
> igt_warn("failed to commit hardware rotated
> fb: %i\n", ret);
--
Mika Kahola - Intel OTC
More information about the Intel-gfx
mailing list