[igt-dev] [PATCH i-g-t 9/9] tests/kms_rotation_crc: Perform lazy cleanup and require atomic.
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Wed Feb 7 09:46:15 UTC 2018
Op 07-02-18 om 09:55 schreef Mika Kahola:
> On Thu, 2018-02-01 at 16:39 +0100, Maarten Lankhorst wrote:
>> This won't clean up resources between subtests, so if subtests fail
>> the next subtest will clean up everything. This allows all subtests
>> even if one fails.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> ---
>> tests/kms_rotation_crc.c | 213 +++++++++++++++--------------------
>> ------------
>> 1 file changed, 66 insertions(+), 147 deletions(-)
>>
>> diff --git a/tests/kms_rotation_crc.c b/tests/kms_rotation_crc.c
>> index 52a85fb62f9f..a0fb734d02f6 100644
>> --- a/tests/kms_rotation_crc.c
>> +++ b/tests/kms_rotation_crc.c
>> @@ -33,7 +33,6 @@ typedef struct {
>> struct igt_fb fb;
>> struct igt_fb fb_reference;
>> struct igt_fb fb_unrotated;
>> - struct igt_fb fb_modeset;
>> struct igt_fb fb_flip;
>> igt_crc_t ref_crc;
>> igt_crc_t flip_crc;
>> @@ -122,68 +121,50 @@ paint_squares(data_t *data, igt_rotation_t
>> rotation,
>> igt_put_cairo_ctx(data->gfx_fd, fb, cr);
>> }
>>
>> -static void prepare_crtc(data_t *data, igt_output_t *output, enum
>> pipe pipe,
>> - igt_plane_t *plane, enum igt_commit_style
>> commit)
>> +static void remove_fbs(data_t *data)
>> {
>> - drmModeModeInfo *mode;
>> - unsigned int w, h;
>> - uint64_t tiling = data->override_tiling ?:
>> LOCAL_DRM_FORMAT_MOD_NONE;
>> - uint32_t pixel_format = data->override_fmt ?:
>> DRM_FORMAT_XRGB8888;
>> - igt_display_t *display = &data->display;
>> - igt_plane_t *primary = igt_output_get_plane_type(output,
>> DRM_PLANE_TYPE_PRIMARY);
>> + if (data->fb.fb_id)
>> + igt_remove_fb(data->gfx_fd, &data->fb);
>>
>> - igt_output_set_pipe(output, pipe);
>> - igt_plane_set_rotation(plane, IGT_ROTATION_0);
>> + if (data->fb_reference.fb_id)
>> + igt_remove_fb(data->gfx_fd, &data->fb_reference);
>>
>> - /* create the pipe_crc object for this pipe */
>> - igt_pipe_crc_free(data->pipe_crc);
>> - data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe,
>> INTEL_PIPE_CRC_SOURCE_AUTO);
>> -
>> - mode = igt_output_get_mode(output);
>> + if (data->fb_unrotated.fb_id)
>> + igt_remove_fb(data->gfx_fd, &data->fb_unrotated);
>>
>> - w = mode->hdisplay;
>> - h = mode->vdisplay;
>> -
>> - igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling,
>> &data->fb_modeset);
>> + if (data->fb_flip.fb_id)
>> + igt_remove_fb(data->gfx_fd, &data->fb_flip);
>>
>> - /*
>> - * With igt_display_commit2 and COMMIT_UNIVERSAL, we call
>> just the
>> - * setplane without a modeset. So, to be able to call
>> - * igt_display_commit and ultimately setcrtc to do the first
>> modeset,
>> - * we create an fb covering the crtc and call commit
>> - *
>> - * It's also a good idea to set a primary fb on the primary
>> plane
>> - * regardless, to force a underrun when watermarks are
>> allocated
>> - * incorrectly for other planes.
>> - */
>> - igt_plane_set_fb(primary, &data->fb_modeset);
>> + data->fb_flip.fb_id = data->fb_reference.fb_id = data-
>>> fb_unrotated.fb_id = data->fb.fb_id = 0;
>> +}
>>
>> - if (commit < COMMIT_ATOMIC) {
>> - igt_plane_clear_prop_changed(primary,
>> IGT_PLANE_ROTATION);
>> - igt_display_commit(display);
>> +static void cleanup_crtc(data_t *data)
>> +{
>> + igt_display_t *display = &data->display;
>>
>> - if (plane->type == DRM_PLANE_TYPE_PRIMARY)
>> - igt_plane_set_prop_changed(primary,
>> IGT_PLANE_ROTATION);
>> - }
>> + igt_pipe_crc_free(data->pipe_crc);
>> + data->pipe_crc = NULL;
>>
>> - igt_plane_set_fb(plane, NULL);
>> + remove_fbs(data);
>>
>> - igt_display_commit2(display, commit);
>> + igt_display_reset(display);
>> }
>>
>> -static void remove_fbs(data_t *data)
>> +static void prepare_crtc(data_t *data, igt_output_t *output, enum
>> pipe pipe,
>> + igt_plane_t *plane)
>> {
>> - if (!data->fb.fb_id)
>> - return;
>> + igt_display_t *display = &data->display;
>>
>> - igt_remove_fb(data->gfx_fd, &data->fb);
>> - igt_remove_fb(data->gfx_fd, &data->fb_reference);
>> - igt_remove_fb(data->gfx_fd, &data->fb_unrotated);
>> + cleanup_crtc(data);
>>
>> - if (data->fb_flip.fb_id)
>> - igt_remove_fb(data->gfx_fd, &data->fb_flip);
>> + igt_output_set_pipe(output, pipe);
>> + igt_plane_set_rotation(plane, IGT_ROTATION_0);
>> +
>> + /* create the pipe_crc object for this pipe */
>> + igt_pipe_crc_free(data->pipe_crc);
>> + data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe,
>> INTEL_PIPE_CRC_SOURCE_AUTO);
>>
>> - data->fb_flip.fb_id = data->fb.fb_id = 0;
>> + igt_display_commit2(display, COMMIT_ATOMIC);
>> }
>>
>> enum rectangle_type {
>> @@ -206,7 +187,7 @@ static void prepare_fbs(data_t *data,
>> igt_output_t *output,
>>
>> if (data->fb.fb_id) {
>> igt_plane_set_fb(plane, NULL);
>> - igt_display_commit2(display, display->is_atomic ?
>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>> + igt_display_commit2(display, COMMIT_ATOMIC);
> Should we keep the legacy commit here or should we have a requirement
> for this test that it runs only atomic?
Hey,
Only the 180° tests won't run on gen4 without i915.nuclear_pageflip=1, at least until gen3 wm patch series is finally merged. But they are never run in CI (shards) anyway .
So in practice the requirement doesn't matter and if we do care then we should get those in finally...
~Maarten
>>
>> remove_fbs(data);
>> }
>> @@ -259,28 +240,24 @@ static void prepare_fbs(data_t *data,
>> igt_output_t *output,
>> /*
>> * Create a reference software rotated flip framebuffer.
>> */
>> - if (plane->type == DRM_PLANE_TYPE_PRIMARY || display-
>>> is_atomic) {
>> - igt_create_fb(data->gfx_fd, ref_w, ref_h,
>> pixel_format, tiling,
>> - &data->fb_flip);
>> - paint_squares(data, data->rotation, &data->fb_flip,
>> - flip_opacity);
>> - igt_plane_set_fb(plane, &data->fb_flip);
>> - if (plane->type != DRM_PLANE_TYPE_CURSOR)
>> - igt_plane_set_position(plane, data->pos_x,
>> data->pos_y);
>> - igt_display_commit2(display,
>> - display->is_atomic ?
>> - COMMIT_ATOMIC :
>> COMMIT_UNIVERSAL);
>> - igt_pipe_crc_collect_crc(data->pipe_crc, &data-
>>> flip_crc);
>> -
>> - /*
>> - * Prepare the non-rotated flip fb.
>> - */
>> - igt_remove_fb(data->gfx_fd, &data->fb_flip);
>> - igt_create_fb(data->gfx_fd, w, h, pixel_format,
>> tiling,
>> - &data->fb_flip);
>> - paint_squares(data, IGT_ROTATION_0, &data->fb_flip,
>> - flip_opacity);
>> - }
>> + igt_create_fb(data->gfx_fd, ref_w, ref_h, pixel_format,
>> tiling,
>> + &data->fb_flip);
>> + paint_squares(data, data->rotation, &data->fb_flip,
>> + flip_opacity);
>> + igt_plane_set_fb(plane, &data->fb_flip);
>> + if (plane->type != DRM_PLANE_TYPE_CURSOR)
>> + igt_plane_set_position(plane, data->pos_x, data-
>>> pos_y);
>> + igt_display_commit2(display, COMMIT_ATOMIC);
>> + igt_pipe_crc_collect_crc(data->pipe_crc, &data->flip_crc);
>> +
>> + /*
>> + * Prepare the non-rotated flip fb.
>> + */
>> + igt_remove_fb(data->gfx_fd, &data->fb_flip);
>> + igt_create_fb(data->gfx_fd, w, h, pixel_format, tiling,
>> + &data->fb_flip);
>> + paint_squares(data, IGT_ROTATION_0, &data->fb_flip,
>> + flip_opacity);
>>
>> /*
>> * Create a reference CRC for a software-rotated fb.
>> @@ -292,7 +269,7 @@ static void prepare_fbs(data_t *data,
>> igt_output_t *output,
>> igt_plane_set_fb(plane, &data->fb_reference);
>> if (plane->type != DRM_PLANE_TYPE_CURSOR)
>> igt_plane_set_position(plane, data->pos_x, data-
>>> pos_y);
>> - igt_display_commit2(display, display->is_atomic ?
>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>> + igt_display_commit2(display, COMMIT_ATOMIC);
>>
>> igt_pipe_crc_collect_crc(data->pipe_crc, &data->ref_crc);
>>
>> @@ -305,7 +282,7 @@ static void prepare_fbs(data_t *data,
>> igt_output_t *output,
>> igt_plane_set_rotation(plane, IGT_ROTATION_0);
>> if (plane->type != DRM_PLANE_TYPE_CURSOR)
>> igt_plane_set_position(plane, data->pos_x, data-
>>> pos_y);
>> - igt_display_commit2(display, display->is_atomic ?
>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>> + igt_display_commit2(display, COMMIT_ATOMIC);
>>
>> /*
>> * Prepare the plane with an non-rotated fb let the hw
>> rotate it.
>> @@ -318,35 +295,6 @@ static void prepare_fbs(data_t *data,
>> igt_output_t *output,
>> igt_plane_set_position(plane, data->pos_x, data-
>>> pos_y);
>> }
>>
>> -static void cleanup_crtc(data_t *data, igt_output_t *output,
>> igt_plane_t *plane)
>> -{
>> - igt_display_t *display = &data->display;
>> -
>> - igt_pipe_crc_free(data->pipe_crc);
>> - data->pipe_crc = NULL;
>> -
>> - remove_fbs(data);
>> -
>> - igt_remove_fb(data->gfx_fd, &data->fb_modeset);
>> -
>> - /* XXX: see the note in prepare_crtc() */
>> - if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
>> - igt_plane_t *primary;
>> -
>> - primary = igt_output_get_plane_type(output,
>> DRM_PLANE_TYPE_PRIMARY);
>> - igt_plane_set_fb(primary, NULL);
>> - }
>> -
>> - igt_plane_set_fb(plane, NULL);
>> - igt_plane_set_rotation(plane, IGT_ROTATION_0);
>> -
>> - igt_display_commit2(display, COMMIT_UNIVERSAL);
>> -
>> - igt_output_set_pipe(output, PIPE_ANY);
>> -
>> - igt_display_commit(display);
>> -}
>> -
>> static void wait_for_pageflip(int fd)
>> {
>> drmEventContext evctx = { .version = 2 };
>> @@ -371,18 +319,11 @@ static void test_plane_rotation(data_t *data,
>> int plane_type, bool test_bad_form
>> enum pipe pipe;
>> int valid_tests = 0;
>> igt_crc_t crc_output;
>> - enum igt_commit_style commit = COMMIT_LEGACY;
>> int ret;
>>
>> - if (plane_type == DRM_PLANE_TYPE_PRIMARY || plane_type ==
>> DRM_PLANE_TYPE_CURSOR)
>> - commit = COMMIT_UNIVERSAL;
>> -
>> if (plane_type == DRM_PLANE_TYPE_CURSOR)
>> igt_require(display->has_cursor_plane);
>>
>> - if (data->display.is_atomic)
>> - commit = COMMIT_ATOMIC;
>> -
>> for_each_pipe_with_valid_output(display, pipe, output) {
>> igt_plane_t *plane;
>> int i;
>> @@ -395,7 +336,7 @@ static void test_plane_rotation(data_t *data, int
>> plane_type, bool test_bad_form
>> plane = igt_output_get_plane_type(output,
>> plane_type);
>> igt_require(igt_plane_has_prop(plane,
>> IGT_PLANE_ROTATION));
>>
>> - prepare_crtc(data, output, pipe, plane, commit);
>> + prepare_crtc(data, output, pipe, plane);
>>
>> for (i = 0; i < num_rectangle_types; i++) {
>> /* Unsupported on i915 */
>> @@ -415,7 +356,7 @@ static void test_plane_rotation(data_t *data, int
>> plane_type, bool test_bad_form
>> if (data->rotation & (IGT_ROTATION_90 |
>> IGT_ROTATION_270))
>> igt_plane_set_size(plane, data-
>>> fb.height, data->fb.width);
>>
>> - ret = igt_display_try_commit2(display,
>> commit);
>> + ret = igt_display_try_commit2(display,
>> COMMIT_ATOMIC);
>> if (test_bad_format) {
>> igt_assert_eq(ret, -EINVAL);
>> continue;
>> @@ -456,41 +397,28 @@ static void test_plane_rotation(data_t *data,
>> int plane_type, bool test_bad_form
>> }
>>
>> valid_tests++;
>> - cleanup_crtc(data, output, plane);
>> }
>> igt_require_f(valid_tests, "no valid crtc/connector
>> combinations found\n");
>> }
>>
>> static void test_plane_rotation_exhaust_fences(data_t *data,
>> igt_output_t *output,
>> - int plane_type)
>> + igt_plane_t *plane)
>> {
>> igt_display_t *display = &data->display;
>> uint64_t tiling = LOCAL_I915_FORMAT_MOD_Y_TILED;
>> uint32_t format = DRM_FORMAT_XRGB8888;
>> int bpp = igt_drm_format_to_bpp(format);
>> - enum igt_commit_style commit = COMMIT_LEGACY;
>> int fd = data->gfx_fd;
>> - igt_plane_t *plane;
>> drmModeModeInfo *mode;
>> - data_t data2[MAX_FENCES+1] = {};
>> + struct igt_fb fb[MAX_FENCES+1] = {};
>> unsigned int stride, size, w, h;
>> uint32_t gem_handle;
>> uint64_t total_aperture_size, total_fbs_size;
>> int i, ret;
>>
>> - plane = igt_output_get_plane_type(output, plane_type);
>> igt_require(igt_plane_has_prop(plane, IGT_PLANE_ROTATION));
>>
>> - if (plane_type == DRM_PLANE_TYPE_PRIMARY || plane_type ==
>> DRM_PLANE_TYPE_CURSOR)
>> - commit = COMMIT_UNIVERSAL;
>> -
>> - if (plane_type == DRM_PLANE_TYPE_CURSOR)
>> - igt_require(display->has_cursor_plane);
>> -
>> - if (data->display.is_atomic)
>> - commit = COMMIT_ATOMIC;
>> -
>> mode = igt_output_get_mode(output);
>> w = mode->hdisplay;
>> h = mode->vdisplay;
>> @@ -509,7 +437,7 @@ static void
>> test_plane_rotation_exhaust_fences(data_t *data,
>> igt_require(total_fbs_size < total_aperture_size * 0.9);
>>
>> igt_plane_set_fb(plane, NULL);
>> - igt_display_commit(display);
>> + igt_display_commit2(display, COMMIT_ATOMIC);
>>
>> for (i = 0; i < MAX_FENCES + 1; i++) {
>> gem_handle = gem_create(fd, size);
>> @@ -522,20 +450,20 @@ static void
>> test_plane_rotation_exhaust_fences(data_t *data,
>> ret = (__kms_addfb(fd, gem_handle, w, h, stride,
>> format, tiling, NULL,
>> LOCAL_DRM_MODE_FB_MODIFIERS,
>> - &data2[i].fb.fb_id));
>> + &fb[i].fb_id));
>> if (ret) {
>> igt_warn("failed to create framebuffer\n");
>> goto err_alloc;
>> }
>>
>> - data2[i].fb.width = w;
>> - data2[i].fb.height = h;
>> - data2[i].fb.gem_handle = gem_handle;
>> + fb[i].width = w;
>> + fb[i].height = h;
>> + fb[i].gem_handle = gem_handle;
>>
>> - igt_plane_set_fb(plane, &data2[i].fb);
>> + igt_plane_set_fb(plane, &fb[i]);
>> igt_plane_set_rotation(plane, IGT_ROTATION_0);
>>
>> - ret = igt_display_try_commit2(display, commit);
>> + igt_display_commit2(display, COMMIT_ATOMIC);
>> if (ret) {
>> igt_warn("failed to commit unrotated fb\n");
>> goto err_commit;
>> @@ -544,7 +472,7 @@ 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);
>>
>> - igt_display_commit2(display, commit);
>> + igt_display_commit2(display, COMMIT_ATOMIC);
>> if (ret) {
>> igt_warn("failed to commit hardware rotated
>> fb: %i\n", ret);
>> goto err_commit;
>> @@ -557,19 +485,9 @@ err_alloc:
>>
>> i--;
>> err_commit:
>> - igt_plane_set_fb(plane, NULL);
>> - igt_plane_set_rotation(plane, IGT_ROTATION_0);
>> -
>> - if (commit < COMMIT_ATOMIC)
>> - igt_display_commit2(display, commit);
>> -
>> - igt_output_set_pipe(output, PIPE_NONE);
>> - igt_display_commit2(display, display->is_atomic ?
>> COMMIT_ATOMIC : COMMIT_LEGACY);
>> -
>> for (; i >= 0; i--)
>> - igt_remove_fb(fd, &data2[i].fb);
>> + igt_remove_fb(fd, &fb[i]);
>>
>> - kmstest_restore_vt_mode();
>> igt_assert_eq(ret, 0);
>> }
>>
>> @@ -726,9 +644,10 @@ igt_main
>> igt_require(gen >= 9);
>>
>> for_each_pipe_with_valid_output(&data.display, pipe,
>> output) {
>> - igt_output_set_pipe(output, pipe);
>> + igt_plane_t *primary =
>> &data.display.pipes[pipe].planes[0];
>> + prepare_crtc(&data, output, pipe, primary);
>>
>> - test_plane_rotation_exhaust_fences(&data,
>> output, DRM_PLANE_TYPE_PRIMARY);
>> + test_plane_rotation_exhaust_fences(&data,
>> output, primary);
>>
>> valid_tests++;
>> break;
More information about the igt-dev
mailing list