* [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons @ 2019-01-25 12:30 Juha-Pekka Heikkila 2019-01-25 13:01 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork ` (7 more replies) 0 siblings, 8 replies; 13+ messages in thread From: Juha-Pekka Heikkila @ 2019-01-25 12:30 UTC (permalink / raw) To: igt-dev; +Cc: Pandiyan, Dhinakaran Fixed handling of single plane modes so used bpp never get to be 0. Reduce verboseness if there's no errors. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106701 Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> --- tests/kms_available_modes_crc.c | 123 ++++++++++++++++++++-------------------- 1 file changed, 61 insertions(+), 62 deletions(-) diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c index 7ff385f..432c084 100644 --- a/tests/kms_available_modes_crc.c +++ b/tests/kms_available_modes_crc.c @@ -54,6 +54,12 @@ typedef struct { igt_crc_t cursor_crc; igt_crc_t fullscreen_crc; + + /* + * If there's unknown format setting up mode can fail + * without failing entire test. + */ + bool allow_fail; } data_t; @@ -120,7 +126,6 @@ static void generate_comparison_crc_list(data_t *data, igt_output_t *output) static const struct { uint32_t fourcc; - char zeropadding; enum { BYTES_PP_1=1, BYTES_PP_2=2, BYTES_PP_4=4, @@ -129,10 +134,10 @@ static const struct { SKIP4 } bpp; uint32_t value; } fillers[] = { - { DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff}, - { DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff}, - { DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff}, - { DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff}, + { DRM_FORMAT_C8, BYTES_PP_1, 0xff}, + { DRM_FORMAT_RGB565, BYTES_PP_2, 0xffff}, + { DRM_FORMAT_XRGB8888, BYTES_PP_4, 0xffffffff}, + { DRM_FORMAT_XBGR8888, BYTES_PP_4, 0xffffffff}, /* * following two are skipped because blending seems to work @@ -140,31 +145,31 @@ static const struct { * Test still creates the planes, just filling plane * and getting crc is skipped. */ - { DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff}, - { DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff}, + { DRM_FORMAT_ARGB8888, SKIP4, 0xffffffff}, + { DRM_FORMAT_ABGR8888, SKIP4, 0x00ffffff}, - { DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff}, - { DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff}, + { DRM_FORMAT_XRGB2101010, BYTES_PP_4, 0xffffffff}, + { DRM_FORMAT_XBGR2101010, BYTES_PP_4, 0xffffffff}, - { DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb}, - { DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb}, - { DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80}, - { DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80}, + { DRM_FORMAT_YUYV, BYTES_PP_4, 0x80eb80eb}, + { DRM_FORMAT_YVYU, BYTES_PP_4, 0x80eb80eb}, + { DRM_FORMAT_VYUY, BYTES_PP_4, 0xeb80eb80}, + { DRM_FORMAT_UYVY, BYTES_PP_4, 0xeb80eb80}, /* * (semi-)planar formats */ - { DRM_FORMAT_NV12, 0, NV12, 0x80eb}, + { DRM_FORMAT_NV12, NV12, 0x80eb}, #ifdef DRM_FORMAT_P010 - { DRM_FORMAT_P010, 0, P010, 0x8000eb00}, + { DRM_FORMAT_P010, P010, 0x8000eb00}, #endif #ifdef DRM_FORMAT_P012 - { DRM_FORMAT_P012, 0, P010, 0x8000eb00}, + { DRM_FORMAT_P012, P010, 0x8000eb00}, #endif #ifdef DRM_FORMAT_P016 - { DRM_FORMAT_P016, 0, P010, 0x8000eb00}, + { DRM_FORMAT_P016, P010, 0x8000eb00}, #endif - { 0, 0, 0, 0 } + { 0, SKIP4, 0 } }; /* @@ -233,13 +238,10 @@ static bool fill_in_fb(data_t *data, igt_output_t *output, igt_plane_t *plane, writesize = data->size; break; } - igt_info("Format %s CRC comparison skipped by design.\n", - (char*)&fillers[i].fourcc); - - return false; + /* Fall through */ default: - igt_info("Unsupported mode for test %s\n", - (char*)&fillers[i].fourcc); + igt_info("Unsupported mode for testing CRC %.4s\n", + (char *)&format); return false; } @@ -260,6 +262,8 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane, int bpp = 0; int i; + data->allow_fail = false; + mode = igt_output_get_mode(output); if (plane->type != DRM_PLANE_TYPE_CURSOR) { w = mode->hdisplay; @@ -288,6 +292,8 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane, break; case SKIP4: + data->allow_fail = true; + /* fall through */ case BYTES_PP_4: bpp = 32; break; @@ -324,7 +330,7 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane, if(ret < 0) { igt_info("Creating fb for format %s failed, return code %d\n", - (char*)&data->format.name, ret); + (char *)&data->format.name, ret); return false; } @@ -385,13 +391,14 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, static int -test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane, - int mode) +test_one_mode(data_t *data, igt_output_t *output, igt_plane_t *plane, + enum pipe pipe, int mode) { igt_crc_t current_crc; signed rVal = 0; bool do_crc; - char* crccompare[2]; + char *crccompare[2]; + igt_crc_t *p_crc; if (prepare_crtc(data, output, plane, mode)){ /* @@ -409,32 +416,32 @@ test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane, if (do_crc) { igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, ¤t_crc); - if (plane->type != DRM_PLANE_TYPE_CURSOR) { - if (!igt_check_crc_equal(¤t_crc, - &data->fullscreen_crc)) { - crccompare[0] = igt_crc_to_string(¤t_crc); - crccompare[1] = igt_crc_to_string(&data->fullscreen_crc); - igt_warn("crc mismatch. target %.8s, result %.8s.\n", crccompare[0], crccompare[1]); - free(crccompare[0]); - free(crccompare[1]); - rVal++; - } - } else { - if (!igt_check_crc_equal(¤t_crc, - &data->cursor_crc)) { - crccompare[0] = igt_crc_to_string(¤t_crc); - crccompare[1] = igt_crc_to_string(&data->cursor_crc); - igt_warn("crc mismatch. target %.8s, result %.8s.\n", crccompare[0], crccompare[1]); - free(crccompare[0]); - free(crccompare[1]); + if (plane->type != DRM_PLANE_TYPE_CURSOR) + p_crc = &data->fullscreen_crc; + else + p_crc = &data->cursor_crc; + + if (!igt_check_crc_equal(¤t_crc, p_crc)) { + crccompare[0] = igt_crc_to_string(¤t_crc); + crccompare[1] = igt_crc_to_string(p_crc); + igt_warn("crc mismatch on %s PIPE %s plane %d format %.4s. target %.8s, result %.8s.\n", + igt_output_name(output), + kmstest_pipe_name(pipe), + plane->index, + (char *)&data->format.name, + crccompare[0], + crccompare[1]); + + free(crccompare[0]); + free(crccompare[1]); + if (!data->allow_fail) rVal++; - } } } remove_fb(data, output, plane); return rVal; } - return 1; + return data->allow_fail ? 0 : 1; } @@ -445,11 +452,11 @@ test_available_modes(data_t* data) igt_plane_t *plane; int modeindex; enum pipe pipe; - int invalids = 0; + int failed_crcs = 0; drmModePlane *modePlane; - char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" }; for_each_pipe_with_valid_output(&data->display, pipe, output) { + igt_display_reset(&data->display); igt_output_set_pipe(output, pipe); igt_display_commit2(&data->display, data->commit); @@ -472,17 +479,9 @@ test_available_modes(data_t* data) modeindex++) { data->format.dword = modePlane->formats[modeindex]; - igt_info("Testing connector %s using pipe %s" \ - " plane index %d type %s mode %s\n", - igt_output_name(output), - kmstest_pipe_name(pipe), - plane->index, - planetype[plane->type], - (char*)&data->format.name); - - invalids += test_one_mode(data, output, - plane, - modePlane->formats[modeindex]); + failed_crcs += test_one_mode(data, output, + plane, pipe, + modePlane->formats[modeindex]); } drmModeFreePlane(modePlane); } @@ -491,7 +490,7 @@ test_available_modes(data_t* data) igt_display_commit2(&data->display, data->commit); igt_output_set_pipe(output, PIPE_NONE); } - igt_assert(invalids == 0); + igt_assert_f((failed_crcs == 0), "There was %d modes with invalid crc\n", failed_crcs); } -- 2.7.4 _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc: Fix handling of test comparisons 2019-01-25 12:30 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila @ 2019-01-25 13:01 ` Patchwork 2019-01-25 15:58 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork ` (6 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2019-01-25 13:01 UTC (permalink / raw) To: Juha-Pekka Heikkila; +Cc: igt-dev == Series Details == Series: tests/kms_available_modes_crc: Fix handling of test comparisons URL : https://patchwork.freedesktop.org/series/55727/ State : success == Summary == CI Bug Log - changes from CI_DRM_5481 -> IGTPW_2292 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/55727/revisions/1/mbox/ Known issues ------------ Here are the changes found in IGTPW_2292 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_exec_suspend@basic-s3: - fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718] * igt@gem_exec_suspend@basic-s4-devices: - fi-kbl-7500u: PASS -> DMESG-WARN [fdo#105128] / [fdo#107139] * igt@i915_module_load@reload-with-fault-injection: - fi-kbl-7567u: PASS -> DMESG-WARN [fdo#105602] / [fdo#108529] +1 * igt@kms_chamelium@common-hpd-after-suspend: - fi-kbl-7567u: PASS -> DMESG-WARN [fdo#103558] / [fdo#105079] / [fdo#105602] * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a: - fi-byt-clapper: PASS -> FAIL [fdo#107362] * igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence: - fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362] * igt@pm_rpm@module-reload: - fi-kbl-7567u: PASS -> DMESG-WARN [fdo#108529] #### Possible fixes #### * igt@gem_exec_suspend@basic-s3: - fi-cfl-8109u: DMESG-WARN [fdo#107345] -> PASS * igt@i915_selftest@live_hangcheck: - fi-bwr-2160: DMESG-FAIL [fdo#108735] -> PASS - fi-skl-gvtdvm: INCOMPLETE [fdo#105600] / [fdo#108744] -> PASS * igt@kms_busy@basic-flip-b: - fi-gdg-551: FAIL [fdo#103182] -> PASS * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a: - fi-byt-clapper: FAIL [fdo#103191] / [fdo#107362] -> PASS - fi-cfl-8109u: INCOMPLETE [fdo#106070] / [fdo#108126] -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558 [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079 [fdo#105128]: https://bugs.freedesktop.org/show_bug.cgi?id=105128 [fdo#105600]: https://bugs.freedesktop.org/show_bug.cgi?id=105600 [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602 [fdo#106070]: https://bugs.freedesktop.org/show_bug.cgi?id=106070 [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139 [fdo#107345]: https://bugs.freedesktop.org/show_bug.cgi?id=107345 [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362 [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#108126]: https://bugs.freedesktop.org/show_bug.cgi?id=108126 [fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529 [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735 [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 Participating hosts (44 -> 40) ------------------------------ Additional (1): fi-hsw-4770 Missing (5): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-pnv-d510 Build changes ------------- * IGT: IGT_4790 -> IGTPW_2292 CI_DRM_5481: 192c39147b7a320a73e6f0426ec4a5fe3f9b2a06 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2292: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2292/ IGT_4790: dcdf4b04e16312f8f52ad389388d834f9d74b8f0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2292/ _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* [igt-dev] ✗ Fi.CI.IGT: failure for tests/kms_available_modes_crc: Fix handling of test comparisons 2019-01-25 12:30 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila 2019-01-25 13:01 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork @ 2019-01-25 15:58 ` Patchwork 2019-01-25 16:50 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc: Fix handling of test comparisons (rev2) Patchwork ` (5 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2019-01-25 15:58 UTC (permalink / raw) To: Juha-Pekka Heikkila; +Cc: igt-dev == Series Details == Series: tests/kms_available_modes_crc: Fix handling of test comparisons URL : https://patchwork.freedesktop.org/series/55727/ State : failure == Summary == CI Bug Log - changes from CI_DRM_5481_full -> IGTPW_2292_full ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with IGTPW_2292_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in IGTPW_2292_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/55727/revisions/1/mbox/ Possible new issues ------------------- Here are the unknown changes that may have been introduced in IGTPW_2292_full: ### IGT changes ### #### Possible regressions #### * igt@kms_available_modes_crc@available_mode_test_crc: - shard-kbl: PASS -> FAIL Known issues ------------ Here are the changes found in IGTPW_2292_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_exec_schedule@pi-ringfull-render: - shard-kbl: NOTRUN -> FAIL [fdo#103158] * igt@gem_workarounds@suspend-resume-fd: - shard-kbl: PASS -> DMESG-WARN [fdo#103313] * igt@i915_suspend@shrink: - shard-kbl: NOTRUN -> DMESG-WARN [fdo#109244] * igt@kms_busy@extended-modeset-hang-newfb-render-a: - shard-kbl: NOTRUN -> DMESG-WARN [fdo#107956] +1 * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a: - shard-snb: NOTRUN -> DMESG-WARN [fdo#107956] +1 * igt@kms_color@pipe-b-ctm-max: - shard-apl: PASS -> FAIL [fdo#108147] - shard-kbl: PASS -> FAIL [fdo#108147] * igt@kms_cursor_crc@cursor-128x42-sliding: - shard-apl: PASS -> FAIL [fdo#103232] +3 * igt@kms_cursor_crc@cursor-256x256-dpms: - shard-glk: PASS -> FAIL [fdo#103232] +3 * igt@kms_cursor_crc@cursor-64x64-random: - shard-kbl: PASS -> FAIL [fdo#103232] +2 * igt@kms_cursor_crc@cursor-alpha-opaque: - shard-snb: NOTRUN -> FAIL [fdo#109350] * igt@kms_cursor_legacy@all-pipes-forked-bo: - shard-snb: PASS -> INCOMPLETE [fdo#105411] * igt@kms_flip@2x-modeset-vs-vblank-race-interruptible: - shard-glk: PASS -> FAIL [fdo#103060] * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb: - shard-glk: NOTRUN -> FAIL [fdo#108145] * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc: - shard-kbl: NOTRUN -> FAIL [fdo#108145] / [fdo#108590] * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb: - shard-apl: PASS -> FAIL [fdo#108145] * igt@kms_plane_multiple@atomic-pipe-a-tiling-x: - shard-apl: PASS -> FAIL [fdo#103166] - shard-kbl: PASS -> FAIL [fdo#103166] * igt@kms_plane_multiple@atomic-pipe-c-tiling-y: - shard-glk: PASS -> FAIL [fdo#103166] +1 * igt@kms_rotation_crc@multiplane-rotation-cropping-top: - shard-apl: PASS -> DMESG-FAIL [fdo#108950] * igt@kms_setmode@basic: - shard-apl: PASS -> FAIL [fdo#99912] - shard-kbl: PASS -> FAIL [fdo#99912] * igt@kms_vblank@pipe-a-ts-continuation-suspend: - shard-apl: PASS -> INCOMPLETE [fdo#103927] #### Possible fixes #### * igt@gem_ctx_isolation@bcs0-s3: - shard-kbl: INCOMPLETE [fdo#103665] -> PASS * igt@kms_atomic_transition@2x-modeset-transitions: - shard-hsw: DMESG-FAIL [fdo#102614] -> PASS * igt@kms_color@pipe-a-degamma: - shard-apl: FAIL [fdo#104782] / [fdo#108145] -> PASS * igt@kms_color@pipe-b-degamma: - shard-kbl: FAIL [fdo#104782] -> PASS - shard-apl: FAIL [fdo#104782] -> PASS * igt@kms_cursor_crc@cursor-128x128-suspend: - shard-apl: FAIL [fdo#103191] / [fdo#103232] -> PASS * igt@kms_cursor_crc@cursor-64x21-onscreen: - shard-glk: FAIL [fdo#103232] -> PASS +1 * igt@kms_cursor_crc@cursor-64x21-random: - shard-apl: FAIL [fdo#103232] -> PASS +5 - shard-kbl: FAIL [fdo#103232] -> PASS * igt@kms_plane@pixel-format-pipe-a-planes: - shard-kbl: FAIL [fdo#103166] -> PASS * igt@kms_plane@pixel-format-pipe-c-planes: - shard-apl: FAIL [fdo#103166] -> PASS +4 * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb: - shard-glk: FAIL [fdo#108145] -> PASS * igt@kms_plane_multiple@atomic-pipe-b-tiling-none: - shard-glk: FAIL [fdo#103166] -> PASS +2 * igt@kms_vblank@pipe-a-query-idle: - shard-hsw: DMESG-WARN [fdo#102614] -> PASS * igt@kms_vblank@pipe-c-ts-continuation-suspend: - shard-hsw: FAIL [fdo#104894] -> PASS * igt@perf_pmu@rc6-runtime-pm-long: - shard-kbl: FAIL [fdo#105010] -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614 [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060 [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158 [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232 [fdo#103313]: https://bugs.freedesktop.org/show_bug.cgi?id=103313 [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665 [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927 [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782 [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894 [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010 [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411 [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956 [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145 [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147 [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590 [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950 [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278 [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350 [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912 Participating hosts (7 -> 5) ------------------------------ Missing (2): shard-skl shard-iclb Build changes ------------- * IGT: IGT_4790 -> IGTPW_2292 * Piglit: piglit_4509 -> None CI_DRM_5481: 192c39147b7a320a73e6f0426ec4a5fe3f9b2a06 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2292: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2292/ IGT_4790: dcdf4b04e16312f8f52ad389388d834f9d74b8f0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2292/ _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc: Fix handling of test comparisons (rev2) 2019-01-25 12:30 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila 2019-01-25 13:01 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork 2019-01-25 15:58 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork @ 2019-01-25 16:50 ` Patchwork 2019-01-25 19:37 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork ` (4 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2019-01-25 16:50 UTC (permalink / raw) To: Juha-Pekka Heikkila; +Cc: igt-dev == Series Details == Series: tests/kms_available_modes_crc: Fix handling of test comparisons (rev2) URL : https://patchwork.freedesktop.org/series/55727/ State : success == Summary == CI Bug Log - changes from CI_DRM_5484 -> IGTPW_2295 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/55727/revisions/2/mbox/ Known issues ------------ Here are the changes found in IGTPW_2295 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@kms_busy@basic-flip-a: - fi-gdg-551: PASS -> FAIL [fdo#103182] * igt@kms_pipe_crc_basic@read-crc-pipe-a: - fi-byt-clapper: PASS -> FAIL [fdo#107362] #### Possible fixes #### * igt@kms_busy@basic-flip-b: - fi-gdg-551: FAIL [fdo#103182] -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182 [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278 Participating hosts (44 -> 40) ------------------------------ Additional (1): fi-bsw-n3050 Missing (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-y Build changes ------------- * IGT: IGT_4790 -> IGTPW_2295 CI_DRM_5484: 9f66ac94341eb12501097f9f8991c86aee70981c @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2295: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2295/ IGT_4790: dcdf4b04e16312f8f52ad389388d834f9d74b8f0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2295/ _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* [igt-dev] ✗ Fi.CI.IGT: failure for tests/kms_available_modes_crc: Fix handling of test comparisons (rev2) 2019-01-25 12:30 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila ` (2 preceding siblings ...) 2019-01-25 16:50 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc: Fix handling of test comparisons (rev2) Patchwork @ 2019-01-25 19:37 ` Patchwork 2019-01-28 11:43 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila ` (3 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2019-01-25 19:37 UTC (permalink / raw) To: Juha-Pekka Heikkila; +Cc: igt-dev == Series Details == Series: tests/kms_available_modes_crc: Fix handling of test comparisons (rev2) URL : https://patchwork.freedesktop.org/series/55727/ State : failure == Summary == CI Bug Log - changes from CI_DRM_5484_full -> IGTPW_2295_full ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with IGTPW_2295_full absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in IGTPW_2295_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://patchwork.freedesktop.org/api/1.0/series/55727/revisions/2/mbox/ Possible new issues ------------------- Here are the unknown changes that may have been introduced in IGTPW_2295_full: ### IGT changes ### #### Possible regressions #### * igt@gem_eio@in-flight-immediate: - shard-kbl: PASS -> DMESG-FAIL * igt@kms_available_modes_crc@available_mode_test_crc: - shard-kbl: PASS -> FAIL #### Suppressed #### The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * {igt@runner@aborted}: - shard-kbl: ( 4 FAIL ) -> ( 5 FAIL ) Known issues ------------ Here are the changes found in IGTPW_2295_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_exec_schedule@pi-ringfull-bsd2: - shard-kbl: NOTRUN -> FAIL [fdo#103158] * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b: - shard-kbl: PASS -> DMESG-WARN [fdo#107956] * igt@kms_cursor_crc@cursor-128x128-random: - shard-apl: PASS -> FAIL [fdo#103232] +5 * igt@kms_cursor_crc@cursor-256x256-sliding: - shard-glk: PASS -> FAIL [fdo#103232] +4 * igt@kms_cursor_crc@cursor-64x64-random: - shard-kbl: PASS -> FAIL [fdo#103232] * igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-ytiled: - shard-glk: PASS -> FAIL [fdo#107791] * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping: - shard-glk: PASS -> FAIL [fdo#108948] - shard-apl: PASS -> FAIL [fdo#108948] - shard-kbl: PASS -> FAIL [fdo#108948] * igt@kms_plane@plane-position-covered-pipe-a-planes: - shard-glk: PASS -> FAIL [fdo#103166] +1 * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb: - shard-glk: NOTRUN -> FAIL [fdo#108145] * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max: - shard-apl: PASS -> FAIL [fdo#108145] * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf: - shard-apl: PASS -> FAIL [fdo#103166] #### Possible fixes #### * igt@kms_color@pipe-a-degamma: - shard-apl: FAIL [fdo#104782] / [fdo#108145] -> PASS * igt@kms_color@pipe-c-degamma: - shard-apl: FAIL [fdo#104782] -> PASS * igt@kms_cursor_crc@cursor-64x21-random: - shard-apl: FAIL [fdo#103232] -> PASS +1 - shard-kbl: FAIL [fdo#103232] -> PASS * igt@kms_cursor_crc@cursor-size-change: - shard-glk: FAIL [fdo#103232] -> PASS +2 * igt@kms_plane@pixel-format-pipe-a-planes: - shard-kbl: FAIL [fdo#103166] -> PASS * igt@kms_plane@plane-position-covered-pipe-c-planes: - shard-apl: FAIL [fdo#103166] -> PASS +2 * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb: - shard-apl: FAIL [fdo#108145] -> PASS * igt@kms_plane_multiple@atomic-pipe-a-tiling-y: - shard-glk: FAIL [fdo#103166] -> PASS +2 * igt@perf_pmu@rc6-runtime-pm-long: - shard-kbl: FAIL [fdo#105010] -> PASS * igt@pm_rc6_residency@rc6-accuracy: - shard-snb: {SKIP} [fdo#109271] -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158 [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166 [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232 [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782 [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010 [fdo#107791]: https://bugs.freedesktop.org/show_bug.cgi?id=107791 [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956 [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145 [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278 [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373 [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321 Participating hosts (7 -> 5) ------------------------------ Missing (2): shard-skl shard-iclb Build changes ------------- * IGT: IGT_4790 -> IGTPW_2295 * Piglit: piglit_4509 -> None CI_DRM_5484: 9f66ac94341eb12501097f9f8991c86aee70981c @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2295: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2295/ IGT_4790: dcdf4b04e16312f8f52ad389388d834f9d74b8f0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2295/ _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons 2019-01-25 12:30 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila ` (3 preceding siblings ...) 2019-01-25 19:37 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork @ 2019-01-28 11:43 ` Juha-Pekka Heikkila 2019-01-28 12:13 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc: Fix handling of test comparisons (rev3) Patchwork ` (2 subsequent siblings) 7 siblings, 0 replies; 13+ messages in thread From: Juha-Pekka Heikkila @ 2019-01-28 11:43 UTC (permalink / raw) To: igt-dev Fixed handling of single plane modes so used bpp never get to be 0 and ease off crc comparison a bit with gamma tables as idea of this test is to see chosen modes are able to initialize. Reduce verboseness if there's no errors. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106701 Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> --- tests/kms_available_modes_crc.c | 122 ++++++++++++++++++++-------------------- 1 file changed, 60 insertions(+), 62 deletions(-) diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c index 7ff385f..2c8f347 100644 --- a/tests/kms_available_modes_crc.c +++ b/tests/kms_available_modes_crc.c @@ -54,6 +54,12 @@ typedef struct { igt_crc_t cursor_crc; igt_crc_t fullscreen_crc; + + /* + * If there's unknown format setting up mode can fail + * without failing entire test. + */ + bool allow_fail; } data_t; @@ -120,7 +126,6 @@ static void generate_comparison_crc_list(data_t *data, igt_output_t *output) static const struct { uint32_t fourcc; - char zeropadding; enum { BYTES_PP_1=1, BYTES_PP_2=2, BYTES_PP_4=4, @@ -129,10 +134,10 @@ static const struct { SKIP4 } bpp; uint32_t value; } fillers[] = { - { DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff}, - { DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff}, - { DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff}, - { DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff}, + { DRM_FORMAT_C8, BYTES_PP_1, 0xff}, + { DRM_FORMAT_RGB565, BYTES_PP_2, 0xffff}, + { DRM_FORMAT_XRGB8888, BYTES_PP_4, 0xffffffff}, + { DRM_FORMAT_XBGR8888, BYTES_PP_4, 0xffffffff}, /* * following two are skipped because blending seems to work @@ -140,31 +145,31 @@ static const struct { * Test still creates the planes, just filling plane * and getting crc is skipped. */ - { DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff}, - { DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff}, + { DRM_FORMAT_ARGB8888, SKIP4, 0xffffffff}, + { DRM_FORMAT_ABGR8888, SKIP4, 0x00ffffff}, - { DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff}, - { DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff}, + { DRM_FORMAT_XRGB2101010, BYTES_PP_4, 0xffffffff}, + { DRM_FORMAT_XBGR2101010, BYTES_PP_4, 0xffffffff}, - { DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb}, - { DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb}, - { DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80}, - { DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80}, + { DRM_FORMAT_YUYV, BYTES_PP_4, 0x80eb80eb}, + { DRM_FORMAT_YVYU, BYTES_PP_4, 0x80eb80eb}, + { DRM_FORMAT_VYUY, BYTES_PP_4, 0xeb80eb80}, + { DRM_FORMAT_UYVY, BYTES_PP_4, 0xeb80eb80}, /* * (semi-)planar formats */ - { DRM_FORMAT_NV12, 0, NV12, 0x80eb}, + { DRM_FORMAT_NV12, NV12, 0x80eb}, #ifdef DRM_FORMAT_P010 - { DRM_FORMAT_P010, 0, P010, 0x8000eb00}, + { DRM_FORMAT_P010, P010, 0x8000eb00}, #endif #ifdef DRM_FORMAT_P012 - { DRM_FORMAT_P012, 0, P010, 0x8000eb00}, + { DRM_FORMAT_P012, P010, 0x8000eb00}, #endif #ifdef DRM_FORMAT_P016 - { DRM_FORMAT_P016, 0, P010, 0x8000eb00}, + { DRM_FORMAT_P016, P010, 0x8000eb00}, #endif - { 0, 0, 0, 0 } + { 0, SKIP4, 0 } }; /* @@ -233,13 +238,10 @@ static bool fill_in_fb(data_t *data, igt_output_t *output, igt_plane_t *plane, writesize = data->size; break; } - igt_info("Format %s CRC comparison skipped by design.\n", - (char*)&fillers[i].fourcc); - - return false; + /* Fall through */ default: - igt_info("Unsupported mode for test %s\n", - (char*)&fillers[i].fourcc); + igt_info("Unsupported mode for testing CRC %.4s\n", + (char *)&format); return false; } @@ -260,6 +262,8 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane, int bpp = 0; int i; + data->allow_fail = false; + mode = igt_output_get_mode(output); if (plane->type != DRM_PLANE_TYPE_CURSOR) { w = mode->hdisplay; @@ -288,6 +292,8 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane, break; case SKIP4: + data->allow_fail = true; + /* fall through */ case BYTES_PP_4: bpp = 32; break; @@ -324,7 +330,7 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane, if(ret < 0) { igt_info("Creating fb for format %s failed, return code %d\n", - (char*)&data->format.name, ret); + (char *)&data->format.name, ret); return false; } @@ -385,13 +391,14 @@ static bool prepare_crtc(data_t *data, igt_output_t *output, static int -test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane, - int mode) +test_one_mode(data_t *data, igt_output_t *output, igt_plane_t *plane, + enum pipe pipe, int mode) { igt_crc_t current_crc; signed rVal = 0; bool do_crc; - char* crccompare[2]; + char *crccompare[2]; + igt_crc_t *p_crc; if (prepare_crtc(data, output, plane, mode)){ /* @@ -409,32 +416,32 @@ test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane, if (do_crc) { igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, ¤t_crc); - if (plane->type != DRM_PLANE_TYPE_CURSOR) { - if (!igt_check_crc_equal(¤t_crc, - &data->fullscreen_crc)) { - crccompare[0] = igt_crc_to_string(¤t_crc); - crccompare[1] = igt_crc_to_string(&data->fullscreen_crc); - igt_warn("crc mismatch. target %.8s, result %.8s.\n", crccompare[0], crccompare[1]); - free(crccompare[0]); - free(crccompare[1]); - rVal++; - } - } else { - if (!igt_check_crc_equal(¤t_crc, - &data->cursor_crc)) { - crccompare[0] = igt_crc_to_string(¤t_crc); - crccompare[1] = igt_crc_to_string(&data->cursor_crc); - igt_warn("crc mismatch. target %.8s, result %.8s.\n", crccompare[0], crccompare[1]); - free(crccompare[0]); - free(crccompare[1]); + if (plane->type != DRM_PLANE_TYPE_CURSOR) + p_crc = &data->fullscreen_crc; + else + p_crc = &data->cursor_crc; + + if (!igt_check_crc_equal(¤t_crc, p_crc)) { + crccompare[0] = igt_crc_to_string(¤t_crc); + crccompare[1] = igt_crc_to_string(p_crc); + igt_warn("crc mismatch on %s PIPE %s plane %d format %.4s. target %.8s, result %.8s.\n", + igt_output_name(output), + kmstest_pipe_name(pipe), + plane->index, + (char *)&data->format.name, + crccompare[0], + crccompare[1]); + + free(crccompare[0]); + free(crccompare[1]); + if (!data->allow_fail) rVal++; - } } } remove_fb(data, output, plane); return rVal; } - return 1; + return data->allow_fail ? 0 : 1; } @@ -445,9 +452,8 @@ test_available_modes(data_t* data) igt_plane_t *plane; int modeindex; enum pipe pipe; - int invalids = 0; + int failed_crcs = 0; drmModePlane *modePlane; - char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" }; for_each_pipe_with_valid_output(&data->display, pipe, output) { igt_output_set_pipe(output, pipe); @@ -472,17 +478,9 @@ test_available_modes(data_t* data) modeindex++) { data->format.dword = modePlane->formats[modeindex]; - igt_info("Testing connector %s using pipe %s" \ - " plane index %d type %s mode %s\n", - igt_output_name(output), - kmstest_pipe_name(pipe), - plane->index, - planetype[plane->type], - (char*)&data->format.name); - - invalids += test_one_mode(data, output, - plane, - modePlane->formats[modeindex]); + failed_crcs += test_one_mode(data, output, + plane, pipe, + modePlane->formats[modeindex]); } drmModeFreePlane(modePlane); } @@ -491,7 +489,7 @@ test_available_modes(data_t* data) igt_display_commit2(&data->display, data->commit); igt_output_set_pipe(output, PIPE_NONE); } - igt_assert(invalids == 0); + igt_assert_f((failed_crcs == 0), "There was %d modes with invalid crc\n", failed_crcs); } -- 2.7.4 _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc: Fix handling of test comparisons (rev3) 2019-01-25 12:30 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila ` (4 preceding siblings ...) 2019-01-28 11:43 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila @ 2019-01-28 12:13 ` Patchwork 2019-01-28 15:39 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork 2019-01-30 6:40 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Dhinakaran Pandiyan 7 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2019-01-28 12:13 UTC (permalink / raw) To: Juha-Pekka Heikkila; +Cc: igt-dev == Series Details == Series: tests/kms_available_modes_crc: Fix handling of test comparisons (rev3) URL : https://patchwork.freedesktop.org/series/55727/ State : success == Summary == CI Bug Log - changes from IGT_4792 -> IGTPW_2304 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/55727/revisions/3/mbox/ Known issues ------------ Here are the changes found in IGTPW_2304 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@amdgpu/amd_basic@cs-compute: - fi-kbl-8809g: NOTRUN -> FAIL [fdo#108094] * igt@amdgpu/amd_prime@amd-to-i915: - fi-kbl-8809g: NOTRUN -> FAIL [fdo#107341] * igt@i915_module_load@reload: - fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718] * igt@kms_busy@basic-flip-a: - fi-gdg-551: PASS -> FAIL [fdo#103182] * igt@kms_frontbuffer_tracking@basic: - fi-byt-clapper: PASS -> FAIL [fdo#103167] * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence: - fi-byt-clapper: PASS -> FAIL [fdo#103191] / [fdo#107362] #### Possible fixes #### * igt@amdgpu/amd_basic@userptr: - fi-kbl-8809g: DMESG-WARN [fdo#108965] -> PASS * igt@kms_busy@basic-flip-c: - fi-kbl-7500u: {SKIP} [fdo#109271] / [fdo#109278] -> PASS +2 * igt@kms_chamelium@dp-hpd-fast: - fi-kbl-7500u: DMESG-WARN [fdo#102505] / [fdo#103558] / [fdo#105602] -> PASS {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505 [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167 [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558 [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602 [fdo#107341]: https://bugs.freedesktop.org/show_bug.cgi?id=107341 [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362 [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718 [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094 [fdo#108915]: https://bugs.freedesktop.org/show_bug.cgi?id=108915 [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278 Participating hosts (43 -> 40) ------------------------------ Additional (1): fi-icl-y Missing (4): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan Build changes ------------- * IGT: IGT_4792 -> IGTPW_2304 CI_DRM_5492: bc2fcdf82a5b94f015126dfa9db03f88feb9ae17 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2304: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2304/ IGT_4792: e061af7bc37bfc77893dae8dab93d712d39d18df @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2304/ _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for tests/kms_available_modes_crc: Fix handling of test comparisons (rev3) 2019-01-25 12:30 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila ` (5 preceding siblings ...) 2019-01-28 12:13 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc: Fix handling of test comparisons (rev3) Patchwork @ 2019-01-28 15:39 ` Patchwork 2019-01-30 6:40 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Dhinakaran Pandiyan 7 siblings, 0 replies; 13+ messages in thread From: Patchwork @ 2019-01-28 15:39 UTC (permalink / raw) To: Juha-Pekka Heikkila; +Cc: igt-dev == Series Details == Series: tests/kms_available_modes_crc: Fix handling of test comparisons (rev3) URL : https://patchwork.freedesktop.org/series/55727/ State : success == Summary == CI Bug Log - changes from IGT_4792_full -> IGTPW_2304_full ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://patchwork.freedesktop.org/api/1.0/series/55727/revisions/3/mbox/ Known issues ------------ Here are the changes found in IGTPW_2304_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@gem_pwrite_pread@display-pwrite-blt-gtt_mmap-performance: - shard-apl: PASS -> INCOMPLETE [fdo#103927] * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b: - shard-hsw: NOTRUN -> DMESG-WARN [fdo#107956] - shard-glk: NOTRUN -> DMESG-WARN [fdo#107956] * igt@kms_cursor_crc@cursor-128x128-suspend: - shard-apl: PASS -> FAIL [fdo#103191] / [fdo#103232] * igt@kms_cursor_crc@cursor-128x42-onscreen: - shard-glk: PASS -> FAIL [fdo#103232] +1 - shard-kbl: NOTRUN -> FAIL [fdo#103232] * igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic: - shard-glk: NOTRUN -> FAIL [fdo#105454] * igt@kms_invalid_dotclock: - shard-glk: NOTRUN -> DMESG-WARN [fdo#109373] * igt@kms_plane@pixel-format-pipe-b-planes: - shard-kbl: PASS -> FAIL [fdo#103166] * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc: - shard-kbl: NOTRUN -> FAIL [fdo#108145] / [fdo#108590] +1 * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb: - shard-kbl: NOTRUN -> FAIL [fdo#108145] - shard-apl: NOTRUN -> FAIL [fdo#108145] * igt@kms_plane_multiple@atomic-pipe-a-tiling-x: - shard-apl: PASS -> FAIL [fdo#103166] +3 - shard-glk: PASS -> FAIL [fdo#103166] +1 * igt@kms_setmode@basic: - shard-kbl: PASS -> FAIL [fdo#99912] #### Possible fixes #### * igt@kms_available_modes_crc@available_mode_test_crc: - shard-apl: FAIL [fdo#106641] -> PASS - shard-glk: FAIL [fdo#106641] -> PASS * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c: - shard-kbl: DMESG-WARN [fdo#107956] -> PASS * igt@kms_ccs@pipe-b-crc-sprite-planes-basic: - shard-glk: FAIL [fdo#108145] -> PASS * igt@kms_color@pipe-c-ctm-max: - shard-apl: FAIL [fdo#108147] -> PASS * igt@kms_cursor_crc@cursor-128x128-random: - shard-apl: FAIL [fdo#103232] -> PASS +1 * igt@kms_cursor_crc@cursor-128x128-suspend: - shard-kbl: INCOMPLETE [fdo#103665] -> PASS * igt@kms_cursor_crc@cursor-256x256-sliding: - shard-glk: FAIL [fdo#103232] -> PASS +2 - shard-kbl: FAIL [fdo#103232] -> PASS * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping: - shard-glk: FAIL [fdo#108948] -> PASS * igt@kms_plane@plane-position-covered-pipe-c-planes: - shard-apl: FAIL [fdo#103166] -> PASS +4 * igt@kms_universal_plane@universal-plane-pipe-c-functional: - shard-glk: FAIL [fdo#103166] -> PASS +4 * igt@perf@oa-exponents: - shard-glk: FAIL [fdo#105483] -> PASS * igt@perf_pmu@busy-check-all-rcs0: - shard-snb: INCOMPLETE [fdo#105411] -> PASS * igt@pm_rc6_residency@rc6-accuracy: - shard-kbl: {SKIP} [fdo#109271] -> PASS * igt@prime_busy@hang-bsd: - shard-hsw: FAIL [fdo#108807] -> PASS #### Warnings #### * igt@gem_eio@in-flight-immediate: - shard-kbl: DMESG-WARN [fdo#109467] -> DMESG-FAIL [fdo#109467] {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166 [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191 [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232 [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665 [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927 [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411 [fdo#105454]: https://bugs.freedesktop.org/show_bug.cgi?id=105454 [fdo#105483]: https://bugs.freedesktop.org/show_bug.cgi?id=105483 [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641 [fdo#107886]: https://bugs.freedesktop.org/show_bug.cgi?id=107886 [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956 [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145 [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147 [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590 [fdo#108807]: https://bugs.freedesktop.org/show_bug.cgi?id=108807 [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948 [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278 [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373 [fdo#109467]: https://bugs.freedesktop.org/show_bug.cgi?id=109467 [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912 [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321 Participating hosts (7 -> 5) ------------------------------ Missing (2): shard-skl shard-iclb Build changes ------------- * IGT: IGT_4792 -> IGTPW_2304 CI_DRM_5492: bc2fcdf82a5b94f015126dfa9db03f88feb9ae17 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_2304: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2304/ IGT_4792: e061af7bc37bfc77893dae8dab93d712d39d18df @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2304/ _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons 2019-01-25 12:30 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila ` (6 preceding siblings ...) 2019-01-28 15:39 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork @ 2019-01-30 6:40 ` Dhinakaran Pandiyan 2019-01-30 11:25 ` Juha-Pekka Heikkila 7 siblings, 1 reply; 13+ messages in thread From: Dhinakaran Pandiyan @ 2019-01-30 6:40 UTC (permalink / raw) To: Juha-Pekka Heikkila, igt-dev On Fri, 2019-01-25 at 14:30 +0200, Juha-Pekka Heikkila wrote: > Fixed handling of single plane modes so used bpp never get to > be 0. > > Reduce verboseness if there's no errors. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106701 > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> > --- > tests/kms_available_modes_crc.c | 123 ++++++++++++++++++++-------- > ------------ > 1 file changed, 61 insertions(+), 62 deletions(-) > > diff --git a/tests/kms_available_modes_crc.c > b/tests/kms_available_modes_crc.c > index 7ff385f..432c084 100644 > --- a/tests/kms_available_modes_crc.c > +++ b/tests/kms_available_modes_crc.c > @@ -54,6 +54,12 @@ typedef struct { > > igt_crc_t cursor_crc; > igt_crc_t fullscreen_crc; > + > + /* > + * If there's unknown format setting up mode can fail > + * without failing entire test. > + */ > + bool allow_fail; > } data_t; > > > @@ -120,7 +126,6 @@ static void generate_comparison_crc_list(data_t > *data, igt_output_t *output) > > static const struct { > uint32_t fourcc; > - char zeropadding; > enum { BYTES_PP_1=1, > BYTES_PP_2=2, > BYTES_PP_4=4, > @@ -129,10 +134,10 @@ static const struct { > SKIP4 } bpp; > uint32_t value; > } fillers[] = { > - { DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff}, > - { DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff}, > - { DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff}, > - { DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff}, > + { DRM_FORMAT_C8, BYTES_PP_1, 0xff}, > + { DRM_FORMAT_RGB565, BYTES_PP_2, 0xffff}, > + { DRM_FORMAT_XRGB8888, BYTES_PP_4, 0xffffffff}, > + { DRM_FORMAT_XBGR8888, BYTES_PP_4, 0xffffffff}, > > /* > * following two are skipped because blending seems to work > @@ -140,31 +145,31 @@ static const struct { > * Test still creates the planes, just filling plane > * and getting crc is skipped. > */ > - { DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff}, > - { DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff}, > + { DRM_FORMAT_ARGB8888, SKIP4, 0xffffffff}, > + { DRM_FORMAT_ABGR8888, SKIP4, 0x00ffffff}, > > - { DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff}, > - { DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff}, > + { DRM_FORMAT_XRGB2101010, BYTES_PP_4, 0xffffffff}, > + { DRM_FORMAT_XBGR2101010, BYTES_PP_4, 0xffffffff}, > > - { DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb}, > - { DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb}, > - { DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80}, > - { DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80}, > + { DRM_FORMAT_YUYV, BYTES_PP_4, 0x80eb80eb}, > + { DRM_FORMAT_YVYU, BYTES_PP_4, 0x80eb80eb}, > + { DRM_FORMAT_VYUY, BYTES_PP_4, 0xeb80eb80}, > + { DRM_FORMAT_UYVY, BYTES_PP_4, 0xeb80eb80}, > > /* > * (semi-)planar formats > */ > - { DRM_FORMAT_NV12, 0, NV12, 0x80eb}, > + { DRM_FORMAT_NV12, NV12, 0x80eb}, > #ifdef DRM_FORMAT_P010 > - { DRM_FORMAT_P010, 0, P010, 0x8000eb00}, > + { DRM_FORMAT_P010, P010, 0x8000eb00}, > #endif > #ifdef DRM_FORMAT_P012 > - { DRM_FORMAT_P012, 0, P010, 0x8000eb00}, > + { DRM_FORMAT_P012, P010, 0x8000eb00}, > #endif > #ifdef DRM_FORMAT_P016 > - { DRM_FORMAT_P016, 0, P010, 0x8000eb00}, > + { DRM_FORMAT_P016, P010, 0x8000eb00}, Doesn't the library implement this stuff already? I see similar code in create_bo_for_fb(). Even if new formats are needed, I think it's best to put that in the lib/igt_fb. > #endif > - { 0, 0, 0, 0 } > + { 0, SKIP4, 0 } > }; > > /* > @@ -233,13 +238,10 @@ static bool fill_in_fb(data_t *data, > igt_output_t *output, igt_plane_t *plane, > writesize = data->size; > break; > } > - igt_info("Format %s CRC comparison skipped by > design.\n", > - (char*)&fillers[i].fourcc); > - > - return false; > + /* Fall through */ > default: > - igt_info("Unsupported mode for test %s\n", > - (char*)&fillers[i].fourcc); > + igt_info("Unsupported mode for testing CRC %.4s\n", > + (char *)&format); > return false; > } > > @@ -260,6 +262,8 @@ static bool setup_fb(data_t *data, igt_output_t > *output, igt_plane_t *plane, > int bpp = 0; > int i; > > + data->allow_fail = false; > + > mode = igt_output_get_mode(output); > if (plane->type != DRM_PLANE_TYPE_CURSOR) { > w = mode->hdisplay; > @@ -288,6 +292,8 @@ static bool setup_fb(data_t *data, igt_output_t > *output, igt_plane_t *plane, > break; > > case SKIP4: > + data->allow_fail = true; > + /* fall through */ Wouldn't this prevent CRC testing for (fillers[i].fourcc == DRM_FORMAT_ARGB8888 && plane->type == DRM_PLANE_TYPE_CURSOR) ? > case BYTES_PP_4: > bpp = 32; > break; > @@ -324,7 +330,7 @@ static bool setup_fb(data_t *data, igt_output_t > *output, igt_plane_t *plane, > > if(ret < 0) { > igt_info("Creating fb for format %s failed, return code > %d\n", > - (char*)&data->format.name, ret); > + (char *)&data->format.name, ret); > > return false; > } > @@ -385,13 +391,14 @@ static bool prepare_crtc(data_t *data, > igt_output_t *output, > > > static int > -test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* Side note not related to the changes you are making, "mode" is quite misleading. Shouldn't the test be called kms_pixel_formats_crc or something? I thought this was about testing different connector modes. > plane, > - int mode) > +test_one_mode(data_t *data, igt_output_t *output, igt_plane_t > *plane, > + enum pipe pipe, int mode) > { > igt_crc_t current_crc; > signed rVal = 0; > bool do_crc; > - char* crccompare[2]; > + char *crccompare[2]; > + igt_crc_t *p_crc; > > if (prepare_crtc(data, output, plane, mode)){ > /* > @@ -409,32 +416,32 @@ test_one_mode(data_t* data, igt_output_t > *output, igt_plane_t* plane, > if (do_crc) { > igt_pipe_crc_get_current(data->gfx_fd, data- > >pipe_crc, ¤t_crc); > > - if (plane->type != DRM_PLANE_TYPE_CURSOR) { > - if (!igt_check_crc_equal(¤t_crc, > - &data->fullscreen_crc)) { > - crccompare[0] = > igt_crc_to_string(¤t_crc); > - crccompare[1] = > igt_crc_to_string(&data->fullscreen_crc); > - igt_warn("crc mismatch. target > %.8s, result %.8s.\n", crccompare[0], crccompare[1]); > - free(crccompare[0]); > - free(crccompare[1]); > - rVal++; > - } > - } else { > - if (!igt_check_crc_equal(¤t_crc, > - &data->cursor_crc)) { > - crccompare[0] = > igt_crc_to_string(¤t_crc); > - crccompare[1] = > igt_crc_to_string(&data->cursor_crc); > - igt_warn("crc mismatch. target > %.8s, result %.8s.\n", crccompare[0], crccompare[1]); > - free(crccompare[0]); > - free(crccompare[1]); > + if (plane->type != DRM_PLANE_TYPE_CURSOR) > + p_crc = &data->fullscreen_crc; > + else > + p_crc = &data->cursor_crc; > + > + if (!igt_check_crc_equal(¤t_crc, p_crc)) > { > + crccompare[0] = > igt_crc_to_string(¤t_crc); > + crccompare[1] = > igt_crc_to_string(p_crc); > + igt_warn("crc mismatch on %s PIPE %s > plane %d format %.4s. target %.8s, result %.8s.\n", > + igt_output_name(output), > + kmstest_pipe_name(pipe), > + plane->index, > + (char *)&data->format.name, > + crccompare[0], > + crccompare[1]); This is the only hunk I was hoping you would be able to split into a separate patch. > + > + free(crccompare[0]); > + free(crccompare[1]); > + if (!data->allow_fail) Is there a point of reading CRCs if failures are allowed? > rVal++; > - } > } > } > remove_fb(data, output, plane); > return rVal; > } > - return 1; > + return data->allow_fail ? 0 : 1; > } > > > @@ -445,11 +452,11 @@ test_available_modes(data_t* data) > igt_plane_t *plane; > int modeindex; > enum pipe pipe; > - int invalids = 0; > + int failed_crcs = 0; > drmModePlane *modePlane; > - char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" }; > > for_each_pipe_with_valid_output(&data->display, pipe, output) { > + igt_display_reset(&data->display); > igt_output_set_pipe(output, pipe); > igt_display_commit2(&data->display, data->commit); > > @@ -472,17 +479,9 @@ test_available_modes(data_t* data) > modeindex++) { > data->format.dword = modePlane- > >formats[modeindex]; > > - igt_info("Testing connector %s using > pipe %s" \ > - " plane index %d type %s mode > %s\n", > - igt_output_name(output), > - kmstest_pipe_name(pipe), > - plane->index, > - planetype[plane->type], > - (char*)&data->format.name); > - > - invalids += test_one_mode(data, output, > - plane, > - modePlane- > >formats[modeindex]); > + failed_crcs += test_one_mode(data, > output, > + plane, > pipe, > + modePlane- > >formats[modeindex]); > } > drmModeFreePlane(modePlane); > } > @@ -491,7 +490,7 @@ test_available_modes(data_t* data) > igt_display_commit2(&data->display, data->commit); > igt_output_set_pipe(output, PIPE_NONE); > } > - igt_assert(invalids == 0); > + igt_assert_f((failed_crcs == 0), "There was %d modes with > invalid crc\n", failed_crcs); > } > > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons 2019-01-30 6:40 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Dhinakaran Pandiyan @ 2019-01-30 11:25 ` Juha-Pekka Heikkila 2019-01-30 18:23 ` Dhinakaran Pandiyan 0 siblings, 1 reply; 13+ messages in thread From: Juha-Pekka Heikkila @ 2019-01-30 11:25 UTC (permalink / raw) To: dhinakaran.pandiyan, igt-dev On 30.1.2019 8.40, Dhinakaran Pandiyan wrote: > On Fri, 2019-01-25 at 14:30 +0200, Juha-Pekka Heikkila wrote: >> Fixed handling of single plane modes so used bpp never get to >> be 0. >> >> Reduce verboseness if there's no errors. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106701 >> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> >> --- >> tests/kms_available_modes_crc.c | 123 ++++++++++++++++++++-------- >> ------------ >> 1 file changed, 61 insertions(+), 62 deletions(-) >> >> diff --git a/tests/kms_available_modes_crc.c >> b/tests/kms_available_modes_crc.c >> index 7ff385f..432c084 100644 >> --- a/tests/kms_available_modes_crc.c >> +++ b/tests/kms_available_modes_crc.c >> @@ -54,6 +54,12 @@ typedef struct { >> >> igt_crc_t cursor_crc; >> igt_crc_t fullscreen_crc; >> + >> + /* >> + * If there's unknown format setting up mode can fail >> + * without failing entire test. >> + */ >> + bool allow_fail; >> } data_t; >> >> >> @@ -120,7 +126,6 @@ static void generate_comparison_crc_list(data_t >> *data, igt_output_t *output) >> >> static const struct { >> uint32_t fourcc; >> - char zeropadding; >> enum { BYTES_PP_1=1, >> BYTES_PP_2=2, >> BYTES_PP_4=4, >> @@ -129,10 +134,10 @@ static const struct { >> SKIP4 } bpp; >> uint32_t value; >> } fillers[] = { >> - { DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff}, >> - { DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff}, >> - { DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff}, >> - { DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff}, >> + { DRM_FORMAT_C8, BYTES_PP_1, 0xff}, >> + { DRM_FORMAT_RGB565, BYTES_PP_2, 0xffff}, >> + { DRM_FORMAT_XRGB8888, BYTES_PP_4, 0xffffffff}, >> + { DRM_FORMAT_XBGR8888, BYTES_PP_4, 0xffffffff}, >> >> /* >> * following two are skipped because blending seems to work >> @@ -140,31 +145,31 @@ static const struct { >> * Test still creates the planes, just filling plane >> * and getting crc is skipped. >> */ >> - { DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff}, >> - { DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff}, >> + { DRM_FORMAT_ARGB8888, SKIP4, 0xffffffff}, >> + { DRM_FORMAT_ABGR8888, SKIP4, 0x00ffffff}, >> >> - { DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff}, >> - { DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff}, >> + { DRM_FORMAT_XRGB2101010, BYTES_PP_4, 0xffffffff}, >> + { DRM_FORMAT_XBGR2101010, BYTES_PP_4, 0xffffffff}, >> >> - { DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb}, >> - { DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb}, >> - { DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80}, >> - { DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80}, >> + { DRM_FORMAT_YUYV, BYTES_PP_4, 0x80eb80eb}, >> + { DRM_FORMAT_YVYU, BYTES_PP_4, 0x80eb80eb}, >> + { DRM_FORMAT_VYUY, BYTES_PP_4, 0xeb80eb80}, >> + { DRM_FORMAT_UYVY, BYTES_PP_4, 0xeb80eb80}, >> >> /* >> * (semi-)planar formats >> */ >> - { DRM_FORMAT_NV12, 0, NV12, 0x80eb}, >> + { DRM_FORMAT_NV12, NV12, 0x80eb}, >> #ifdef DRM_FORMAT_P010 >> - { DRM_FORMAT_P010, 0, P010, 0x8000eb00}, >> + { DRM_FORMAT_P010, P010, 0x8000eb00}, >> #endif >> #ifdef DRM_FORMAT_P012 >> - { DRM_FORMAT_P012, 0, P010, 0x8000eb00}, >> + { DRM_FORMAT_P012, P010, 0x8000eb00}, >> #endif >> #ifdef DRM_FORMAT_P016 >> - { DRM_FORMAT_P016, 0, P010, 0x8000eb00}, >> + { DRM_FORMAT_P016, P010, 0x8000eb00}, > > Doesn't the library implement this stuff already? I see similar code in > create_bo_for_fb(). Even if new formats are needed, I think it's best > to put that in the lib/igt_fb. These days igt_fb does implement most of these modes I agree. This test was written back in the day when igt support was limited to RGB888 and RGB565. Idea of this test was just to try all fb formats kernel advertised could actually initialize. If you feel this test is obsolete you could ask for votes to get it removed, I tried that before xmas but nobody was interested in removing this so I started to fix problems seen in this test. In any case these days one cannot include new fb formats into i915 without IGT support so kms_plane test does what this test does. > > >> #endif >> - { 0, 0, 0, 0 } >> + { 0, SKIP4, 0 } >> }; >> >> /* >> @@ -233,13 +238,10 @@ static bool fill_in_fb(data_t *data, >> igt_output_t *output, igt_plane_t *plane, >> writesize = data->size; >> break; >> } >> - igt_info("Format %s CRC comparison skipped by >> design.\n", >> - (char*)&fillers[i].fourcc); >> - >> - return false; >> + /* Fall through */ >> default: >> - igt_info("Unsupported mode for test %s\n", >> - (char*)&fillers[i].fourcc); >> + igt_info("Unsupported mode for testing CRC %.4s\n", >> + (char *)&format); >> return false; >> } >> >> @@ -260,6 +262,8 @@ static bool setup_fb(data_t *data, igt_output_t >> *output, igt_plane_t *plane, >> int bpp = 0; >> int i; >> >> + data->allow_fail = false; >> + >> mode = igt_output_get_mode(output); >> if (plane->type != DRM_PLANE_TYPE_CURSOR) { >> w = mode->hdisplay; >> @@ -288,6 +292,8 @@ static bool setup_fb(data_t *data, igt_output_t >> *output, igt_plane_t *plane, >> break; >> >> case SKIP4: >> + data->allow_fail = true; >> + /* fall through */ > Wouldn't this prevent CRC testing for > (fillers[i].fourcc == DRM_FORMAT_ARGB8888 && plane->type == > DRM_PLANE_TYPE_CURSOR) ? There was later patch which fixed this. This version didn't pass CI test because I had forgotten "igt_display_reset(&data->display)" call bit below, it killed test on KBL. > > >> case BYTES_PP_4: >> bpp = 32; >> break; >> @@ -324,7 +330,7 @@ static bool setup_fb(data_t *data, igt_output_t >> *output, igt_plane_t *plane, >> >> if(ret < 0) { >> igt_info("Creating fb for format %s failed, return code >> %d\n", >> - (char*)&data->format.name, ret); >> + (char *)&data->format.name, ret); >> >> return false; >> } >> @@ -385,13 +391,14 @@ static bool prepare_crtc(data_t *data, >> igt_output_t *output, >> >> >> static int >> -test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* > Side note not related to the changes you are making, "mode" is quite > misleading. Shouldn't the test be called kms_pixel_formats_crc or > something? I thought this was about testing different connector modes. > > >> plane, >> - int mode) >> +test_one_mode(data_t *data, igt_output_t *output, igt_plane_t >> *plane, >> + enum pipe pipe, int mode) >> { >> igt_crc_t current_crc; >> signed rVal = 0; >> bool do_crc; >> - char* crccompare[2]; >> + char *crccompare[2]; >> + igt_crc_t *p_crc; >> >> if (prepare_crtc(data, output, plane, mode)){ >> /* >> @@ -409,32 +416,32 @@ test_one_mode(data_t* data, igt_output_t >> *output, igt_plane_t* plane, >> if (do_crc) { >> igt_pipe_crc_get_current(data->gfx_fd, data- >>> pipe_crc, ¤t_crc); >> >> - if (plane->type != DRM_PLANE_TYPE_CURSOR) { >> - if (!igt_check_crc_equal(¤t_crc, >> - &data->fullscreen_crc)) { >> - crccompare[0] = >> igt_crc_to_string(¤t_crc); >> - crccompare[1] = >> igt_crc_to_string(&data->fullscreen_crc); >> - igt_warn("crc mismatch. target >> %.8s, result %.8s.\n", crccompare[0], crccompare[1]); >> - free(crccompare[0]); >> - free(crccompare[1]); >> - rVal++; >> - } >> - } else { >> - if (!igt_check_crc_equal(¤t_crc, >> - &data->cursor_crc)) { >> - crccompare[0] = >> igt_crc_to_string(¤t_crc); >> - crccompare[1] = >> igt_crc_to_string(&data->cursor_crc); >> - igt_warn("crc mismatch. target >> %.8s, result %.8s.\n", crccompare[0], crccompare[1]); >> - free(crccompare[0]); >> - free(crccompare[1]); >> + if (plane->type != DRM_PLANE_TYPE_CURSOR) >> + p_crc = &data->fullscreen_crc; >> + else >> + p_crc = &data->cursor_crc; >> + >> + if (!igt_check_crc_equal(¤t_crc, p_crc)) >> { >> + crccompare[0] = >> igt_crc_to_string(¤t_crc); >> + crccompare[1] = >> igt_crc_to_string(p_crc); >> + igt_warn("crc mismatch on %s PIPE %s >> plane %d format %.4s. target %.8s, result %.8s.\n", >> + igt_output_name(output), >> + kmstest_pipe_name(pipe), >> + plane->index, >> + (char *)&data->format.name, >> + crccompare[0], >> + crccompare[1]); > This is the only hunk I was hoping you would be able to split into a > separate patch. > >> + >> + free(crccompare[0]); >> + free(crccompare[1]); >> + if (!data->allow_fail) > Is there a point of reading CRCs if failures are allowed? It is marked as mode which is not listed in this test but initialization worked so default paths are followed. It will produce warning which I think is useful for indicating this test need updating but will not produce failure to mark newly added features caused problems. > >> rVal++; >> - } >> } >> } >> remove_fb(data, output, plane); >> return rVal; >> } >> - return 1; >> + return data->allow_fail ? 0 : 1; >> } >> >> >> @@ -445,11 +452,11 @@ test_available_modes(data_t* data) >> igt_plane_t *plane; >> int modeindex; >> enum pipe pipe; >> - int invalids = 0; >> + int failed_crcs = 0; >> drmModePlane *modePlane; >> - char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" }; >> >> for_each_pipe_with_valid_output(&data->display, pipe, output) { >> + igt_display_reset(&data->display); >> igt_output_set_pipe(output, pipe); >> igt_display_commit2(&data->display, data->commit); >> >> @@ -472,17 +479,9 @@ test_available_modes(data_t* data) >> modeindex++) { >> data->format.dword = modePlane- >>> formats[modeindex]; >> >> - igt_info("Testing connector %s using >> pipe %s" \ >> - " plane index %d type %s mode >> %s\n", >> - igt_output_name(output), >> - kmstest_pipe_name(pipe), >> - plane->index, >> - planetype[plane->type], >> - (char*)&data->format.name); >> - >> - invalids += test_one_mode(data, output, >> - plane, >> - modePlane- >>> formats[modeindex]); >> + failed_crcs += test_one_mode(data, >> output, >> + plane, >> pipe, >> + modePlane- >>> formats[modeindex]); >> } >> drmModeFreePlane(modePlane); >> } >> @@ -491,7 +490,7 @@ test_available_modes(data_t* data) >> igt_display_commit2(&data->display, data->commit); >> igt_output_set_pipe(output, PIPE_NONE); >> } >> - igt_assert(invalids == 0); >> + igt_assert_f((failed_crcs == 0), "There was %d modes with >> invalid crc\n", failed_crcs); >> } >> >> > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons 2019-01-30 11:25 ` Juha-Pekka Heikkila @ 2019-01-30 18:23 ` Dhinakaran Pandiyan 2019-02-05 15:18 ` Juha-Pekka Heikkila 0 siblings, 1 reply; 13+ messages in thread From: Dhinakaran Pandiyan @ 2019-01-30 18:23 UTC (permalink / raw) To: juhapekka.heikkila, igt-dev On Wed, 2019-01-30 at 13:25 +0200, Juha-Pekka Heikkila wrote: > On 30.1.2019 8.40, Dhinakaran Pandiyan wrote: > > On Fri, 2019-01-25 at 14:30 +0200, Juha-Pekka Heikkila wrote: > > > Fixed handling of single plane modes so used bpp never get to > > > be 0. > > > > > > Reduce verboseness if there's no errors. > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106701 > > > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> > > > --- > > > tests/kms_available_modes_crc.c | 123 ++++++++++++++++++++----- > > > --- > > > ------------ > > > 1 file changed, 61 insertions(+), 62 deletions(-) > > > > > > diff --git a/tests/kms_available_modes_crc.c > > > b/tests/kms_available_modes_crc.c > > > index 7ff385f..432c084 100644 > > > --- a/tests/kms_available_modes_crc.c > > > +++ b/tests/kms_available_modes_crc.c > > > @@ -54,6 +54,12 @@ typedef struct { > > > > > > igt_crc_t cursor_crc; > > > igt_crc_t fullscreen_crc; > > > + > > > + /* > > > + * If there's unknown format setting up mode can fail > > > + * without failing entire test. > > > + */ > > > + bool allow_fail; > > > } data_t; > > > > > > > > > @@ -120,7 +126,6 @@ static void > > > generate_comparison_crc_list(data_t > > > *data, igt_output_t *output) > > > > > > static const struct { > > > uint32_t fourcc; > > > - char zeropadding; > > > enum { BYTES_PP_1=1, > > > BYTES_PP_2=2, > > > BYTES_PP_4=4, > > > @@ -129,10 +134,10 @@ static const struct { > > > SKIP4 } bpp; > > > uint32_t value; > > > } fillers[] = { > > > - { DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff}, > > > - { DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff}, > > > - { DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff}, > > > - { DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff}, > > > + { DRM_FORMAT_C8, BYTES_PP_1, 0xff}, > > > + { DRM_FORMAT_RGB565, BYTES_PP_2, 0xffff}, > > > + { DRM_FORMAT_XRGB8888, BYTES_PP_4, 0xffffffff}, > > > + { DRM_FORMAT_XBGR8888, BYTES_PP_4, 0xffffffff}, > > > > > > /* > > > * following two are skipped because blending seems to > > > work > > > @@ -140,31 +145,31 @@ static const struct { > > > * Test still creates the planes, just filling plane > > > * and getting crc is skipped. > > > */ > > > - { DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff}, > > > - { DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff}, > > > + { DRM_FORMAT_ARGB8888, SKIP4, 0xffffffff}, > > > + { DRM_FORMAT_ABGR8888, SKIP4, 0x00ffffff}, > > > > > > - { DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff}, > > > - { DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff}, > > > + { DRM_FORMAT_XRGB2101010, BYTES_PP_4, 0xffffffff}, > > > + { DRM_FORMAT_XBGR2101010, BYTES_PP_4, 0xffffffff}, > > > > > > - { DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb}, > > > - { DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb}, > > > - { DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80}, > > > - { DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80}, > > > + { DRM_FORMAT_YUYV, BYTES_PP_4, 0x80eb80eb}, > > > + { DRM_FORMAT_YVYU, BYTES_PP_4, 0x80eb80eb}, > > > + { DRM_FORMAT_VYUY, BYTES_PP_4, 0xeb80eb80}, > > > + { DRM_FORMAT_UYVY, BYTES_PP_4, 0xeb80eb80}, > > > > > > /* > > > * (semi-)planar formats > > > */ > > > - { DRM_FORMAT_NV12, 0, NV12, 0x80eb}, > > > + { DRM_FORMAT_NV12, NV12, 0x80eb}, > > > #ifdef DRM_FORMAT_P010 > > > - { DRM_FORMAT_P010, 0, P010, 0x8000eb00}, > > > + { DRM_FORMAT_P010, P010, 0x8000eb00}, > > > #endif > > > #ifdef DRM_FORMAT_P012 > > > - { DRM_FORMAT_P012, 0, P010, 0x8000eb00}, > > > + { DRM_FORMAT_P012, P010, 0x8000eb00}, > > > #endif > > > #ifdef DRM_FORMAT_P016 > > > - { DRM_FORMAT_P016, 0, P010, 0x8000eb00}, > > > + { DRM_FORMAT_P016, P010, 0x8000eb00}, > > > > Doesn't the library implement this stuff already? I see similar > > code in > > create_bo_for_fb(). Even if new formats are needed, I think it's > > best > > to put that in the lib/igt_fb. > > These days igt_fb does implement most of these modes I agree. This > test > was written back in the day when igt support was limited to RGB888 > and > RGB565. Idea of this test was just to try all fb formats kernel > advertised could actually initialize. > > If you feel this test is obsolete you could ask for votes to get it > removed, I tried that before xmas but nobody was interested in > removing > this so I started to fix problems seen in this test. I had a quick glance at kms_plane, like you said it pretty much does the same things but better. My suggestion is to 1) add support in the library for the missing formats i.e., DRM_FORMAT_C8, DRM_FORMAT_XBGR2101010 2) get rid of this test. -DK > > In any case these days one cannot include new fb formats into i915 > without IGT support so kms_plane test does what this test does. > > > > > > > > #endif > > > - { 0, 0, 0, 0 } > > > + { 0, SKIP4, 0 } > > > }; > > > > > > /* > > > @@ -233,13 +238,10 @@ static bool fill_in_fb(data_t *data, > > > igt_output_t *output, igt_plane_t *plane, > > > writesize = data->size; > > > break; > > > } > > > - igt_info("Format %s CRC comparison skipped by > > > design.\n", > > > - (char*)&fillers[i].fourcc); > > > - > > > - return false; > > > + /* Fall through */ > > > default: > > > - igt_info("Unsupported mode for test %s\n", > > > - (char*)&fillers[i].fourcc); > > > + igt_info("Unsupported mode for testing CRC %.4s\n", > > > + (char *)&format); > > > return false; > > > } > > > > > > @@ -260,6 +262,8 @@ static bool setup_fb(data_t *data, > > > igt_output_t > > > *output, igt_plane_t *plane, > > > int bpp = 0; > > > int i; > > > > > > + data->allow_fail = false; > > > + > > > mode = igt_output_get_mode(output); > > > if (plane->type != DRM_PLANE_TYPE_CURSOR) { > > > w = mode->hdisplay; > > > @@ -288,6 +292,8 @@ static bool setup_fb(data_t *data, > > > igt_output_t > > > *output, igt_plane_t *plane, > > > break; > > > > > > case SKIP4: > > > + data->allow_fail = true; > > > + /* fall through */ > > > > Wouldn't this prevent CRC testing for > > (fillers[i].fourcc == DRM_FORMAT_ARGB8888 && plane->type == > > DRM_PLANE_TYPE_CURSOR) ? > > There was later patch which fixed this. This version didn't pass CI > test > because I had forgotten "igt_display_reset(&data->display)" call bit > below, it killed test on KBL. > > > > > > > > case BYTES_PP_4: > > > bpp = 32; > > > break; > > > @@ -324,7 +330,7 @@ static bool setup_fb(data_t *data, > > > igt_output_t > > > *output, igt_plane_t *plane, > > > > > > if(ret < 0) { > > > igt_info("Creating fb for format %s failed, > > > return code > > > %d\n", > > > - (char*)&data->format.name, ret); > > > + (char *)&data->format.name, ret); > > > > > > return false; > > > } > > > @@ -385,13 +391,14 @@ static bool prepare_crtc(data_t *data, > > > igt_output_t *output, > > > > > > > > > static int > > > -test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* > > > > Side note not related to the changes you are making, "mode" is > > quite > > misleading. Shouldn't the test be called kms_pixel_formats_crc or > > something? I thought this was about testing different connector > > modes. > > > > > > > plane, > > > - int mode) > > > +test_one_mode(data_t *data, igt_output_t *output, igt_plane_t > > > *plane, > > > + enum pipe pipe, int mode) > > > { > > > igt_crc_t current_crc; > > > signed rVal = 0; > > > bool do_crc; > > > - char* crccompare[2]; > > > + char *crccompare[2]; > > > + igt_crc_t *p_crc; > > > > > > if (prepare_crtc(data, output, plane, mode)){ > > > /* > > > @@ -409,32 +416,32 @@ test_one_mode(data_t* data, igt_output_t > > > *output, igt_plane_t* plane, > > > if (do_crc) { > > > igt_pipe_crc_get_current(data->gfx_fd, > > > data- > > > > pipe_crc, ¤t_crc); > > > > > > > > > - if (plane->type != DRM_PLANE_TYPE_CURSOR) { > > > - if (!igt_check_crc_equal(¤t_crc, > > > - &data->fullscreen_crc)) { > > > - crccompare[0] = > > > igt_crc_to_string(¤t_crc); > > > - crccompare[1] = > > > igt_crc_to_string(&data->fullscreen_crc); > > > - igt_warn("crc mismatch. target > > > %.8s, result %.8s.\n", crccompare[0], crccompare[1]); > > > - free(crccompare[0]); > > > - free(crccompare[1]); > > > - rVal++; > > > - } > > > - } else { > > > - if (!igt_check_crc_equal(¤t_crc, > > > - &data->cursor_crc)) { > > > - crccompare[0] = > > > igt_crc_to_string(¤t_crc); > > > - crccompare[1] = > > > igt_crc_to_string(&data->cursor_crc); > > > - igt_warn("crc mismatch. target > > > %.8s, result %.8s.\n", crccompare[0], crccompare[1]); > > > - free(crccompare[0]); > > > - free(crccompare[1]); > > > + if (plane->type != DRM_PLANE_TYPE_CURSOR) > > > + p_crc = &data->fullscreen_crc; > > > + else > > > + p_crc = &data->cursor_crc; > > > + > > > + if (!igt_check_crc_equal(¤t_crc, p_crc)) > > > { > > > + crccompare[0] = > > > igt_crc_to_string(¤t_crc); > > > + crccompare[1] = > > > igt_crc_to_string(p_crc); > > > + igt_warn("crc mismatch on %s PIPE %s > > > plane %d format %.4s. target %.8s, result %.8s.\n", > > > + igt_output_name(output), > > > + kmstest_pipe_name(pipe), > > > + plane->index, > > > + (char *)&data->format.name, > > > + crccompare[0], > > > + crccompare[1]); > > > > This is the only hunk I was hoping you would be able to split into > > a > > separate patch. > > > > > + > > > + free(crccompare[0]); > > > + free(crccompare[1]); > > > + if (!data->allow_fail) > > > > Is there a point of reading CRCs if failures are allowed? > > It is marked as mode which is not listed in this test but > initialization > worked so default paths are followed. It will produce warning which > I > think is useful for indicating this test need updating but will not > produce failure to mark newly added features caused problems. > > > > > > rVal++; > > > - } > > > } > > > } > > > remove_fb(data, output, plane); > > > return rVal; > > > } > > > - return 1; > > > + return data->allow_fail ? 0 : 1; > > > } > > > > > > > > > @@ -445,11 +452,11 @@ test_available_modes(data_t* data) > > > igt_plane_t *plane; > > > int modeindex; > > > enum pipe pipe; > > > - int invalids = 0; > > > + int failed_crcs = 0; > > > drmModePlane *modePlane; > > > - char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" }; > > > > > > for_each_pipe_with_valid_output(&data->display, pipe, > > > output) { > > > + igt_display_reset(&data->display); > > > igt_output_set_pipe(output, pipe); > > > igt_display_commit2(&data->display, data- > > > >commit); > > > > > > @@ -472,17 +479,9 @@ test_available_modes(data_t* data) > > > modeindex++) { > > > data->format.dword = modePlane- > > > > formats[modeindex]; > > > > > > > > > - igt_info("Testing connector %s using > > > pipe %s" \ > > > - " plane index %d type %s mode > > > %s\n", > > > - igt_output_name(output), > > > - kmstest_pipe_name(pipe), > > > - plane->index, > > > - planetype[plane->type], > > > - (char*)&data->format.name); > > > - > > > - invalids += test_one_mode(data, output, > > > - plane, > > > - modePlane- > > > > formats[modeindex]); > > > > > > + failed_crcs += test_one_mode(data, > > > output, > > > + plane, > > > pipe, > > > + modePlane- > > > > formats[modeindex]); > > > > > > } > > > drmModeFreePlane(modePlane); > > > } > > > @@ -491,7 +490,7 @@ test_available_modes(data_t* data) > > > igt_display_commit2(&data->display, data- > > > >commit); > > > igt_output_set_pipe(output, PIPE_NONE); > > > } > > > - igt_assert(invalids == 0); > > > + igt_assert_f((failed_crcs == 0), "There was %d modes with > > > invalid crc\n", failed_crcs); > > > } > > > > > > > > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons 2019-01-30 18:23 ` Dhinakaran Pandiyan @ 2019-02-05 15:18 ` Juha-Pekka Heikkila 2019-02-05 22:01 ` Dhinakaran Pandiyan 0 siblings, 1 reply; 13+ messages in thread From: Juha-Pekka Heikkila @ 2019-02-05 15:18 UTC (permalink / raw) To: dhinakaran.pandiyan, igt-dev On 30.1.2019 20.23, Dhinakaran Pandiyan wrote: > On Wed, 2019-01-30 at 13:25 +0200, Juha-Pekka Heikkila wrote: >> On 30.1.2019 8.40, Dhinakaran Pandiyan wrote: >>> On Fri, 2019-01-25 at 14:30 +0200, Juha-Pekka Heikkila wrote: >>>> Fixed handling of single plane modes so used bpp never get to >>>> be 0. >>>> >>>> Reduce verboseness if there's no errors. >>>> >>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106701 >>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> >>>> --- >>>> tests/kms_available_modes_crc.c | 123 ++++++++++++++++++++----- >>>> --- >>>> ------------ >>>> 1 file changed, 61 insertions(+), 62 deletions(-) >>>> >>>> diff --git a/tests/kms_available_modes_crc.c >>>> b/tests/kms_available_modes_crc.c >>>> index 7ff385f..432c084 100644 >>>> --- a/tests/kms_available_modes_crc.c >>>> +++ b/tests/kms_available_modes_crc.c >>>> @@ -54,6 +54,12 @@ typedef struct { >>>> >>>> igt_crc_t cursor_crc; >>>> igt_crc_t fullscreen_crc; >>>> + >>>> + /* >>>> + * If there's unknown format setting up mode can fail >>>> + * without failing entire test. >>>> + */ >>>> + bool allow_fail; >>>> } data_t; >>>> >>>> >>>> @@ -120,7 +126,6 @@ static void >>>> generate_comparison_crc_list(data_t >>>> *data, igt_output_t *output) >>>> >>>> static const struct { >>>> uint32_t fourcc; >>>> - char zeropadding; >>>> enum { BYTES_PP_1=1, >>>> BYTES_PP_2=2, >>>> BYTES_PP_4=4, >>>> @@ -129,10 +134,10 @@ static const struct { >>>> SKIP4 } bpp; >>>> uint32_t value; >>>> } fillers[] = { >>>> - { DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff}, >>>> - { DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff}, >>>> - { DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff}, >>>> - { DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff}, >>>> + { DRM_FORMAT_C8, BYTES_PP_1, 0xff}, >>>> + { DRM_FORMAT_RGB565, BYTES_PP_2, 0xffff}, >>>> + { DRM_FORMAT_XRGB8888, BYTES_PP_4, 0xffffffff}, >>>> + { DRM_FORMAT_XBGR8888, BYTES_PP_4, 0xffffffff}, >>>> >>>> /* >>>> * following two are skipped because blending seems to >>>> work >>>> @@ -140,31 +145,31 @@ static const struct { >>>> * Test still creates the planes, just filling plane >>>> * and getting crc is skipped. >>>> */ >>>> - { DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff}, >>>> - { DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff}, >>>> + { DRM_FORMAT_ARGB8888, SKIP4, 0xffffffff}, >>>> + { DRM_FORMAT_ABGR8888, SKIP4, 0x00ffffff}, >>>> >>>> - { DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff}, >>>> - { DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff}, >>>> + { DRM_FORMAT_XRGB2101010, BYTES_PP_4, 0xffffffff}, >>>> + { DRM_FORMAT_XBGR2101010, BYTES_PP_4, 0xffffffff}, >>>> >>>> - { DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb}, >>>> - { DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb}, >>>> - { DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80}, >>>> - { DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80}, >>>> + { DRM_FORMAT_YUYV, BYTES_PP_4, 0x80eb80eb}, >>>> + { DRM_FORMAT_YVYU, BYTES_PP_4, 0x80eb80eb}, >>>> + { DRM_FORMAT_VYUY, BYTES_PP_4, 0xeb80eb80}, >>>> + { DRM_FORMAT_UYVY, BYTES_PP_4, 0xeb80eb80}, >>>> >>>> /* >>>> * (semi-)planar formats >>>> */ >>>> - { DRM_FORMAT_NV12, 0, NV12, 0x80eb}, >>>> + { DRM_FORMAT_NV12, NV12, 0x80eb}, >>>> #ifdef DRM_FORMAT_P010 >>>> - { DRM_FORMAT_P010, 0, P010, 0x8000eb00}, >>>> + { DRM_FORMAT_P010, P010, 0x8000eb00}, >>>> #endif >>>> #ifdef DRM_FORMAT_P012 >>>> - { DRM_FORMAT_P012, 0, P010, 0x8000eb00}, >>>> + { DRM_FORMAT_P012, P010, 0x8000eb00}, >>>> #endif >>>> #ifdef DRM_FORMAT_P016 >>>> - { DRM_FORMAT_P016, 0, P010, 0x8000eb00}, >>>> + { DRM_FORMAT_P016, P010, 0x8000eb00}, >>> >>> Doesn't the library implement this stuff already? I see similar >>> code in >>> create_bo_for_fb(). Even if new formats are needed, I think it's >>> best >>> to put that in the lib/igt_fb. >> >> These days igt_fb does implement most of these modes I agree. This >> test >> was written back in the day when igt support was limited to RGB888 >> and >> RGB565. Idea of this test was just to try all fb formats kernel >> advertised could actually initialize. >> >> If you feel this test is obsolete you could ask for votes to get it >> removed, I tried that before xmas but nobody was interested in >> removing >> this so I started to fix problems seen in this test. > > I had a quick glance at kms_plane, like you said it pretty much does > the same things but better. My suggestion is to > 1) add support in the library for the missing formats i.e., > DRM_FORMAT_C8, DRM_FORMAT_XBGR2101010 > 2) get rid of this test. > Hi DK, I was looking at those formats missing from list of IGT supported formats. DRM_FORMAT_XBGR2101010 shouldn't be much of a problem to add but it's yet to be seen how it will show in different tests. DRM_FORMAT_C8 on the other hand is different story, I think it probably should not be added as supported format for IGT. Reason for not adding C8 would be the 8bit nature, I think it will never have enough colors to be passing most of CRC tests. I had idea for C8 there would be separate test which will only test C8 features. I'd begin by removing available modes test as it seems to cause only useless noise now and is doubled by kms_plane in places where it really matter. You have opinions or ideas on this? I'd add above on my 'to-do' list. /Juha-Pekka > >> >> In any case these days one cannot include new fb formats into i915 >> without IGT support so kms_plane test does what this test does. >> >>> >>> >>>> #endif >>>> - { 0, 0, 0, 0 } >>>> + { 0, SKIP4, 0 } >>>> }; >>>> >>>> /* >>>> @@ -233,13 +238,10 @@ static bool fill_in_fb(data_t *data, >>>> igt_output_t *output, igt_plane_t *plane, >>>> writesize = data->size; >>>> break; >>>> } >>>> - igt_info("Format %s CRC comparison skipped by >>>> design.\n", >>>> - (char*)&fillers[i].fourcc); >>>> - >>>> - return false; >>>> + /* Fall through */ >>>> default: >>>> - igt_info("Unsupported mode for test %s\n", >>>> - (char*)&fillers[i].fourcc); >>>> + igt_info("Unsupported mode for testing CRC %.4s\n", >>>> + (char *)&format); >>>> return false; >>>> } >>>> >>>> @@ -260,6 +262,8 @@ static bool setup_fb(data_t *data, >>>> igt_output_t >>>> *output, igt_plane_t *plane, >>>> int bpp = 0; >>>> int i; >>>> >>>> + data->allow_fail = false; >>>> + >>>> mode = igt_output_get_mode(output); >>>> if (plane->type != DRM_PLANE_TYPE_CURSOR) { >>>> w = mode->hdisplay; >>>> @@ -288,6 +292,8 @@ static bool setup_fb(data_t *data, >>>> igt_output_t >>>> *output, igt_plane_t *plane, >>>> break; >>>> >>>> case SKIP4: >>>> + data->allow_fail = true; >>>> + /* fall through */ >>> >>> Wouldn't this prevent CRC testing for >>> (fillers[i].fourcc == DRM_FORMAT_ARGB8888 && plane->type == >>> DRM_PLANE_TYPE_CURSOR) ? >> >> There was later patch which fixed this. This version didn't pass CI >> test >> because I had forgotten "igt_display_reset(&data->display)" call bit >> below, it killed test on KBL. >> >>> >>> >>>> case BYTES_PP_4: >>>> bpp = 32; >>>> break; >>>> @@ -324,7 +330,7 @@ static bool setup_fb(data_t *data, >>>> igt_output_t >>>> *output, igt_plane_t *plane, >>>> >>>> if(ret < 0) { >>>> igt_info("Creating fb for format %s failed, >>>> return code >>>> %d\n", >>>> - (char*)&data->format.name, ret); >>>> + (char *)&data->format.name, ret); >>>> >>>> return false; >>>> } >>>> @@ -385,13 +391,14 @@ static bool prepare_crtc(data_t *data, >>>> igt_output_t *output, >>>> >>>> >>>> static int >>>> -test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* >>> >>> Side note not related to the changes you are making, "mode" is >>> quite >>> misleading. Shouldn't the test be called kms_pixel_formats_crc or >>> something? I thought this was about testing different connector >>> modes. >>> >>> >>>> plane, >>>> - int mode) >>>> +test_one_mode(data_t *data, igt_output_t *output, igt_plane_t >>>> *plane, >>>> + enum pipe pipe, int mode) >>>> { >>>> igt_crc_t current_crc; >>>> signed rVal = 0; >>>> bool do_crc; >>>> - char* crccompare[2]; >>>> + char *crccompare[2]; >>>> + igt_crc_t *p_crc; >>>> >>>> if (prepare_crtc(data, output, plane, mode)){ >>>> /* >>>> @@ -409,32 +416,32 @@ test_one_mode(data_t* data, igt_output_t >>>> *output, igt_plane_t* plane, >>>> if (do_crc) { >>>> igt_pipe_crc_get_current(data->gfx_fd, >>>> data- >>>>> pipe_crc, ¤t_crc); >>>> >>>> >>>> - if (plane->type != DRM_PLANE_TYPE_CURSOR) { >>>> - if (!igt_check_crc_equal(¤t_crc, >>>> - &data->fullscreen_crc)) { >>>> - crccompare[0] = >>>> igt_crc_to_string(¤t_crc); >>>> - crccompare[1] = >>>> igt_crc_to_string(&data->fullscreen_crc); >>>> - igt_warn("crc mismatch. target >>>> %.8s, result %.8s.\n", crccompare[0], crccompare[1]); >>>> - free(crccompare[0]); >>>> - free(crccompare[1]); >>>> - rVal++; >>>> - } >>>> - } else { >>>> - if (!igt_check_crc_equal(¤t_crc, >>>> - &data->cursor_crc)) { >>>> - crccompare[0] = >>>> igt_crc_to_string(¤t_crc); >>>> - crccompare[1] = >>>> igt_crc_to_string(&data->cursor_crc); >>>> - igt_warn("crc mismatch. target >>>> %.8s, result %.8s.\n", crccompare[0], crccompare[1]); >>>> - free(crccompare[0]); >>>> - free(crccompare[1]); >>>> + if (plane->type != DRM_PLANE_TYPE_CURSOR) >>>> + p_crc = &data->fullscreen_crc; >>>> + else >>>> + p_crc = &data->cursor_crc; >>>> + >>>> + if (!igt_check_crc_equal(¤t_crc, p_crc)) >>>> { >>>> + crccompare[0] = >>>> igt_crc_to_string(¤t_crc); >>>> + crccompare[1] = >>>> igt_crc_to_string(p_crc); >>>> + igt_warn("crc mismatch on %s PIPE %s >>>> plane %d format %.4s. target %.8s, result %.8s.\n", >>>> + igt_output_name(output), >>>> + kmstest_pipe_name(pipe), >>>> + plane->index, >>>> + (char *)&data->format.name, >>>> + crccompare[0], >>>> + crccompare[1]); >>> >>> This is the only hunk I was hoping you would be able to split into >>> a >>> separate patch. >>> >>>> + >>>> + free(crccompare[0]); >>>> + free(crccompare[1]); >>>> + if (!data->allow_fail) >>> >>> Is there a point of reading CRCs if failures are allowed? >> >> It is marked as mode which is not listed in this test but >> initialization >> worked so default paths are followed. It will produce warning which >> I >> think is useful for indicating this test need updating but will not >> produce failure to mark newly added features caused problems. >> >>> >>>> rVal++; >>>> - } >>>> } >>>> } >>>> remove_fb(data, output, plane); >>>> return rVal; >>>> } >>>> - return 1; >>>> + return data->allow_fail ? 0 : 1; >>>> } >>>> >>>> >>>> @@ -445,11 +452,11 @@ test_available_modes(data_t* data) >>>> igt_plane_t *plane; >>>> int modeindex; >>>> enum pipe pipe; >>>> - int invalids = 0; >>>> + int failed_crcs = 0; >>>> drmModePlane *modePlane; >>>> - char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" }; >>>> >>>> for_each_pipe_with_valid_output(&data->display, pipe, >>>> output) { >>>> + igt_display_reset(&data->display); >>>> igt_output_set_pipe(output, pipe); >>>> igt_display_commit2(&data->display, data- >>>>> commit); >>>> >>>> @@ -472,17 +479,9 @@ test_available_modes(data_t* data) >>>> modeindex++) { >>>> data->format.dword = modePlane- >>>>> formats[modeindex]; >>>> >>>> >>>> - igt_info("Testing connector %s using >>>> pipe %s" \ >>>> - " plane index %d type %s mode >>>> %s\n", >>>> - igt_output_name(output), >>>> - kmstest_pipe_name(pipe), >>>> - plane->index, >>>> - planetype[plane->type], >>>> - (char*)&data->format.name); >>>> - >>>> - invalids += test_one_mode(data, output, >>>> - plane, >>>> - modePlane- >>>>> formats[modeindex]); >>>> >>>> + failed_crcs += test_one_mode(data, >>>> output, >>>> + plane, >>>> pipe, >>>> + modePlane- >>>>> formats[modeindex]); >>>> >>>> } >>>> drmModeFreePlane(modePlane); >>>> } >>>> @@ -491,7 +490,7 @@ test_available_modes(data_t* data) >>>> igt_display_commit2(&data->display, data- >>>>> commit); >>>> igt_output_set_pipe(output, PIPE_NONE); >>>> } >>>> - igt_assert(invalids == 0); >>>> + igt_assert_f((failed_crcs == 0), "There was %d modes with >>>> invalid crc\n", failed_crcs); >>>> } >>>> >>>> >> >> > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons 2019-02-05 15:18 ` Juha-Pekka Heikkila @ 2019-02-05 22:01 ` Dhinakaran Pandiyan 0 siblings, 0 replies; 13+ messages in thread From: Dhinakaran Pandiyan @ 2019-02-05 22:01 UTC (permalink / raw) To: juhapekka.heikkila, igt-dev On Tue, 2019-02-05 at 17:18 +0200, Juha-Pekka Heikkila wrote: > On 30.1.2019 20.23, Dhinakaran Pandiyan wrote: > > On Wed, 2019-01-30 at 13:25 +0200, Juha-Pekka Heikkila wrote: > > > On 30.1.2019 8.40, Dhinakaran Pandiyan wrote: > > > > On Fri, 2019-01-25 at 14:30 +0200, Juha-Pekka Heikkila wrote: > > > > > Fixed handling of single plane modes so used bpp never get to > > > > > be 0. > > > > > > > > > > Reduce verboseness if there's no errors. > > > > > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106701 > > > > > Signed-off-by: Juha-Pekka Heikkila < > > > > > juhapekka.heikkila@gmail.com> > > > > > --- > > > > > tests/kms_available_modes_crc.c | 123 > > > > > ++++++++++++++++++++----- > > > > > --- > > > > > ------------ > > > > > 1 file changed, 61 insertions(+), 62 deletions(-) > > > > > > > > > > diff --git a/tests/kms_available_modes_crc.c > > > > > b/tests/kms_available_modes_crc.c > > > > > index 7ff385f..432c084 100644 > > > > > --- a/tests/kms_available_modes_crc.c > > > > > +++ b/tests/kms_available_modes_crc.c > > > > > @@ -54,6 +54,12 @@ typedef struct { > > > > > > > > > > igt_crc_t cursor_crc; > > > > > igt_crc_t fullscreen_crc; > > > > > + > > > > > + /* > > > > > + * If there's unknown format setting up mode can fail > > > > > + * without failing entire test. > > > > > + */ > > > > > + bool allow_fail; > > > > > } data_t; > > > > > > > > > > > > > > > @@ -120,7 +126,6 @@ static void > > > > > generate_comparison_crc_list(data_t > > > > > *data, igt_output_t *output) > > > > > > > > > > static const struct { > > > > > uint32_t fourcc; > > > > > - char zeropadding; > > > > > enum { BYTES_PP_1=1, > > > > > BYTES_PP_2=2, > > > > > BYTES_PP_4=4, > > > > > @@ -129,10 +134,10 @@ static const struct { > > > > > SKIP4 } bpp; > > > > > uint32_t value; > > > > > } fillers[] = { > > > > > - { DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff}, > > > > > - { DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff}, > > > > > - { DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff}, > > > > > - { DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff}, > > > > > + { DRM_FORMAT_C8, BYTES_PP_1, 0xff}, > > > > > + { DRM_FORMAT_RGB565, BYTES_PP_2, 0xffff}, > > > > > + { DRM_FORMAT_XRGB8888, BYTES_PP_4, 0xffffffff}, > > > > > + { DRM_FORMAT_XBGR8888, BYTES_PP_4, 0xffffffff}, > > > > > > > > > > /* > > > > > * following two are skipped because blending seems to > > > > > work > > > > > @@ -140,31 +145,31 @@ static const struct { > > > > > * Test still creates the planes, just filling plane > > > > > * and getting crc is skipped. > > > > > */ > > > > > - { DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff}, > > > > > - { DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff}, > > > > > + { DRM_FORMAT_ARGB8888, SKIP4, 0xffffffff}, > > > > > + { DRM_FORMAT_ABGR8888, SKIP4, 0x00ffffff}, > > > > > > > > > > - { DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff}, > > > > > - { DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff}, > > > > > + { DRM_FORMAT_XRGB2101010, BYTES_PP_4, 0xffffffff}, > > > > > + { DRM_FORMAT_XBGR2101010, BYTES_PP_4, 0xffffffff}, > > > > > > > > > > - { DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb}, > > > > > - { DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb}, > > > > > - { DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80}, > > > > > - { DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80}, > > > > > + { DRM_FORMAT_YUYV, BYTES_PP_4, 0x80eb80eb}, > > > > > + { DRM_FORMAT_YVYU, BYTES_PP_4, 0x80eb80eb}, > > > > > + { DRM_FORMAT_VYUY, BYTES_PP_4, 0xeb80eb80}, > > > > > + { DRM_FORMAT_UYVY, BYTES_PP_4, 0xeb80eb80}, > > > > > > > > > > /* > > > > > * (semi-)planar formats > > > > > */ > > > > > - { DRM_FORMAT_NV12, 0, NV12, 0x80eb}, > > > > > + { DRM_FORMAT_NV12, NV12, 0x80eb}, > > > > > #ifdef DRM_FORMAT_P010 > > > > > - { DRM_FORMAT_P010, 0, P010, 0x8000eb00}, > > > > > + { DRM_FORMAT_P010, P010, 0x8000eb00}, > > > > > #endif > > > > > #ifdef DRM_FORMAT_P012 > > > > > - { DRM_FORMAT_P012, 0, P010, 0x8000eb00}, > > > > > + { DRM_FORMAT_P012, P010, 0x8000eb00}, > > > > > #endif > > > > > #ifdef DRM_FORMAT_P016 > > > > > - { DRM_FORMAT_P016, 0, P010, 0x8000eb00}, > > > > > + { DRM_FORMAT_P016, P010, 0x8000eb00}, > > > > > > > > Doesn't the library implement this stuff already? I see similar > > > > code in > > > > create_bo_for_fb(). Even if new formats are needed, I think > > > > it's > > > > best > > > > to put that in the lib/igt_fb. > > > > > > These days igt_fb does implement most of these modes I agree. > > > This > > > test > > > was written back in the day when igt support was limited to > > > RGB888 > > > and > > > RGB565. Idea of this test was just to try all fb formats kernel > > > advertised could actually initialize. > > > > > > If you feel this test is obsolete you could ask for votes to get > > > it > > > removed, I tried that before xmas but nobody was interested in > > > removing > > > this so I started to fix problems seen in this test. > > > > I had a quick glance at kms_plane, like you said it pretty much > > does > > the same things but better. My suggestion is to > > 1) add support in the library for the missing formats i.e., > > DRM_FORMAT_C8, DRM_FORMAT_XBGR2101010 > > 2) get rid of this test. > > > > Hi DK, > > I was looking at those formats missing from list of IGT supported > formats. DRM_FORMAT_XBGR2101010 shouldn't be much of a problem to > add > but it's yet to be seen how it will show in different tests. > DRM_FORMAT_C8 on the other hand is different story, I think it > probably > should not be added as supported format for IGT. Reason for not > adding > C8 would be the 8bit nature, I think it will never have enough colors > to > be passing most of CRC tests. I see. Would it not pass even if the color that we use is base color like R, G or B? Can we conditionally limit the colors that are tested for DRM_FORMAT_C8 in kms_plane? > I had idea for C8 there would be separate > test which will only test C8 features. > > I'd begin by removing available modes test as it seems to cause only > useless noise now and is doubled by kms_plane in places where it > really > matter. > > You have opinions or ideas on this? I'd add above on my 'to-do' list. How about first removing only the formats that are tested elsewhere - reduces noise and test duplication? That should leave DRM_FORMAT_XBGR2101010 and DRM_FORMAT_C8. When you've added support for DRM_FORMAT_XBGR2101010 in kms_plane, then that can be removed too. Finally, test a limited set of colors for DRM_FORMAT_C8 in kms_plane and kill kms_available_modes_crc. -DK > > /Juha-Pekka > > > > > > > > > In any case these days one cannot include new fb formats into > > > i915 > > > without IGT support so kms_plane test does what this test does. > > > > > > > > > > > > > > > > #endif > > > > > - { 0, 0, 0, 0 } > > > > > + { 0, SKIP4, 0 } > > > > > }; > > > > > > > > > > /* > > > > > @@ -233,13 +238,10 @@ static bool fill_in_fb(data_t *data, > > > > > igt_output_t *output, igt_plane_t *plane, > > > > > writesize = data->size; > > > > > break; > > > > > } > > > > > - igt_info("Format %s CRC comparison skipped by > > > > > design.\n", > > > > > - (char*)&fillers[i].fourcc); > > > > > - > > > > > - return false; > > > > > + /* Fall through */ > > > > > default: > > > > > - igt_info("Unsupported mode for test %s\n", > > > > > - (char*)&fillers[i].fourcc); > > > > > + igt_info("Unsupported mode for testing CRC > > > > > %.4s\n", > > > > > + (char *)&format); > > > > > return false; > > > > > } > > > > > > > > > > @@ -260,6 +262,8 @@ static bool setup_fb(data_t *data, > > > > > igt_output_t > > > > > *output, igt_plane_t *plane, > > > > > int bpp = 0; > > > > > int i; > > > > > > > > > > + data->allow_fail = false; > > > > > + > > > > > mode = igt_output_get_mode(output); > > > > > if (plane->type != DRM_PLANE_TYPE_CURSOR) { > > > > > w = mode->hdisplay; > > > > > @@ -288,6 +292,8 @@ static bool setup_fb(data_t *data, > > > > > igt_output_t > > > > > *output, igt_plane_t *plane, > > > > > break; > > > > > > > > > > case SKIP4: > > > > > + data->allow_fail = true; > > > > > + /* fall through */ > > > > > > > > Wouldn't this prevent CRC testing for > > > > (fillers[i].fourcc == DRM_FORMAT_ARGB8888 && plane->type == > > > > DRM_PLANE_TYPE_CURSOR) ? > > > > > > There was later patch which fixed this. This version didn't pass > > > CI > > > test > > > because I had forgotten "igt_display_reset(&data->display)" call > > > bit > > > below, it killed test on KBL. > > > > > > > > > > > > > > > > case BYTES_PP_4: > > > > > bpp = 32; > > > > > break; > > > > > @@ -324,7 +330,7 @@ static bool setup_fb(data_t *data, > > > > > igt_output_t > > > > > *output, igt_plane_t *plane, > > > > > > > > > > if(ret < 0) { > > > > > igt_info("Creating fb for format %s failed, > > > > > return code > > > > > %d\n", > > > > > - (char*)&data->format.name, ret); > > > > > + (char *)&data->format.name, ret); > > > > > > > > > > return false; > > > > > } > > > > > @@ -385,13 +391,14 @@ static bool prepare_crtc(data_t *data, > > > > > igt_output_t *output, > > > > > > > > > > > > > > > static int > > > > > -test_one_mode(data_t* data, igt_output_t *output, > > > > > igt_plane_t* > > > > > > > > Side note not related to the changes you are making, "mode" is > > > > quite > > > > misleading. Shouldn't the test be called kms_pixel_formats_crc > > > > or > > > > something? I thought this was about testing different connector > > > > modes. > > > > > > > > > > > > > plane, > > > > > - int mode) > > > > > +test_one_mode(data_t *data, igt_output_t *output, > > > > > igt_plane_t > > > > > *plane, > > > > > + enum pipe pipe, int mode) > > > > > { > > > > > igt_crc_t current_crc; > > > > > signed rVal = 0; > > > > > bool do_crc; > > > > > - char* crccompare[2]; > > > > > + char *crccompare[2]; > > > > > + igt_crc_t *p_crc; > > > > > > > > > > if (prepare_crtc(data, output, plane, mode)){ > > > > > /* > > > > > @@ -409,32 +416,32 @@ test_one_mode(data_t* data, > > > > > igt_output_t > > > > > *output, igt_plane_t* plane, > > > > > if (do_crc) { > > > > > igt_pipe_crc_get_current(data->gfx_fd, > > > > > data- > > > > > > pipe_crc, ¤t_crc); > > > > > > > > > > > > > > > - if (plane->type != > > > > > DRM_PLANE_TYPE_CURSOR) { > > > > > - if > > > > > (!igt_check_crc_equal(¤t_crc, > > > > > - &data->fullscreen_crc)) > > > > > { > > > > > - crccompare[0] = > > > > > igt_crc_to_string(¤t_crc); > > > > > - crccompare[1] = > > > > > igt_crc_to_string(&data->fullscreen_crc); > > > > > - igt_warn("crc mismatch. > > > > > target > > > > > %.8s, result %.8s.\n", crccompare[0], crccompare[1]); > > > > > - free(crccompare[0]); > > > > > - free(crccompare[1]); > > > > > - rVal++; > > > > > - } > > > > > - } else { > > > > > - if > > > > > (!igt_check_crc_equal(¤t_crc, > > > > > - &data->cursor_crc)) { > > > > > - crccompare[0] = > > > > > igt_crc_to_string(¤t_crc); > > > > > - crccompare[1] = > > > > > igt_crc_to_string(&data->cursor_crc); > > > > > - igt_warn("crc mismatch. > > > > > target > > > > > %.8s, result %.8s.\n", crccompare[0], crccompare[1]); > > > > > - free(crccompare[0]); > > > > > - free(crccompare[1]); > > > > > + if (plane->type != > > > > > DRM_PLANE_TYPE_CURSOR) > > > > > + p_crc = &data->fullscreen_crc; > > > > > + else > > > > > + p_crc = &data->cursor_crc; > > > > > + > > > > > + if (!igt_check_crc_equal(¤t_crc, > > > > > p_crc)) > > > > > { > > > > > + crccompare[0] = > > > > > igt_crc_to_string(¤t_crc); > > > > > + crccompare[1] = > > > > > igt_crc_to_string(p_crc); > > > > > + igt_warn("crc mismatch on %s > > > > > PIPE %s > > > > > plane %d format %.4s. target %.8s, result %.8s.\n", > > > > > + igt_output_name(output > > > > > ), > > > > > + kmstest_pipe_name(pipe > > > > > ), > > > > > + plane->index, > > > > > + (char *)&data- > > > > > >format.name, > > > > > + crccompare[0], > > > > > + crccompare[1]); > > > > > > > > This is the only hunk I was hoping you would be able to split > > > > into > > > > a > > > > separate patch. > > > > > > > > > + > > > > > + free(crccompare[0]); > > > > > + free(crccompare[1]); > > > > > + if (!data->allow_fail) > > > > > > > > Is there a point of reading CRCs if failures are allowed? > > > > > > It is marked as mode which is not listed in this test but > > > initialization > > > worked so default paths are followed. It will produce warning > > > which > > > I > > > think is useful for indicating this test need updating but will > > > not > > > produce failure to mark newly added features caused problems. > > > > > > > > > > > > rVal++; > > > > > - } > > > > > } > > > > > } > > > > > remove_fb(data, output, plane); > > > > > return rVal; > > > > > } > > > > > - return 1; > > > > > + return data->allow_fail ? 0 : 1; > > > > > } > > > > > > > > > > > > > > > @@ -445,11 +452,11 @@ test_available_modes(data_t* data) > > > > > igt_plane_t *plane; > > > > > int modeindex; > > > > > enum pipe pipe; > > > > > - int invalids = 0; > > > > > + int failed_crcs = 0; > > > > > drmModePlane *modePlane; > > > > > - char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", > > > > > "CURSOR\0" }; > > > > > > > > > > for_each_pipe_with_valid_output(&data->display, pipe, > > > > > output) { > > > > > + igt_display_reset(&data->display); > > > > > igt_output_set_pipe(output, pipe); > > > > > igt_display_commit2(&data->display, data- > > > > > > commit); > > > > > > > > > > > > > > > @@ -472,17 +479,9 @@ test_available_modes(data_t* data) > > > > > modeindex++) { > > > > > data->format.dword = modePlane- > > > > > > formats[modeindex]; > > > > > > > > > > > > > > > - igt_info("Testing connector %s > > > > > using > > > > > pipe %s" \ > > > > > - " plane index %d type > > > > > %s mode > > > > > %s\n", > > > > > - igt_output_name(output > > > > > ), > > > > > - kmstest_pipe_name(pipe > > > > > ), > > > > > - plane->index, > > > > > - planetype[plane- > > > > > >type], > > > > > - (char*)&data- > > > > > >format.name); > > > > > - > > > > > - invalids += test_one_mode(data, > > > > > output, > > > > > - plane > > > > > , > > > > > - modeP > > > > > lane- > > > > > > formats[modeindex]); > > > > > > > > > > + failed_crcs += > > > > > test_one_mode(data, > > > > > output, > > > > > + pl > > > > > ane, > > > > > pipe, > > > > > + mo > > > > > dePlane- > > > > > > formats[modeindex]); > > > > > > > > > > } > > > > > drmModeFreePlane(modePlane); > > > > > } > > > > > @@ -491,7 +490,7 @@ test_available_modes(data_t* data) > > > > > igt_display_commit2(&data->display, data- > > > > > > commit); > > > > > > > > > > igt_output_set_pipe(output, PIPE_NONE); > > > > > } > > > > > - igt_assert(invalids == 0); > > > > > + igt_assert_f((failed_crcs == 0), "There was %d modes > > > > > with > > > > > invalid crc\n", failed_crcs); > > > > > } > > > > > > > > > > > > > > > > > > _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-02-05 22:01 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-25 12:30 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila 2019-01-25 13:01 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork 2019-01-25 15:58 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork 2019-01-25 16:50 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc: Fix handling of test comparisons (rev2) Patchwork 2019-01-25 19:37 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork 2019-01-28 11:43 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Juha-Pekka Heikkila 2019-01-28 12:13 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc: Fix handling of test comparisons (rev3) Patchwork 2019-01-28 15:39 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork 2019-01-30 6:40 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons Dhinakaran Pandiyan 2019-01-30 11:25 ` Juha-Pekka Heikkila 2019-01-30 18:23 ` Dhinakaran Pandiyan 2019-02-05 15:18 ` Juha-Pekka Heikkila 2019-02-05 22:01 ` Dhinakaran Pandiyan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.