lib/debugfs: wait_for_keypress("crc") when collecting CRC
PATCH 1/1 lib/debugfs: wait_for_keypress("crc") when collecting CRC
From: Paulo Zanoni <[email protected]>
Let's just steal the "crc" namespace and add this by default to igt_pipe_crc_collect_crc() instead of adding more calls to other tests. If tests want special waits on just some of their collect_crc() calls, they can use another name instead of "crc".
This is very useful when developing, especially when the CRC we get is
wrong: we want to look at the screen to see what's going on before we can think about how to fix the problem. So let's add this to the lib instead of adding this to every single test I need to debug.
--- lib/igt_debugfs.c | 3 +++ tests/kms_draw_crc.c | 3 --- tests/kms_plane.c | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-)
lib/igt_debugfs.c +3 -0
@@ -35,6 +35,7 @@#include <i915_drm.h> #include "drmtest.h" +#include "igt_aux.h" #include "igt_kms.h" #include "igt_debugfs.h"
@@ -512,6 +513,8 @@ static void crc_sanity_checks(igt_crc_t *crc)*/ void igt_pipe_crc_collect_crc (igt_pipe_crc_t *pipe_crc,igt_crc_t *out_crc) { +igt_debug_wait_for_keypress ("crc" ); +igt_pipe_crc_start (pipe_crc);read_one_crc (pipe_crc, out_crc);igt_pipe_crc_stop (pipe_crc);
tests/kms_draw_crc.c +0 -3
@@ -102,7 +102,6 @@ static void get_method_crc(enum igt_draw_method method, uint64_t tiling, &ms.connector_id, 1, ms.mode);igt_assert (rc ==0 ); -igt_debug_wait_for_keypress ("crc" );igt_pipe_crc_collect_crc (pipe_crc, crc);kmstest_unset_all_crtcs (drm_fd, drm_res);
@@ -144,7 +143,6 @@ static void get_fill_crc(uint64_t tiling, igt_crc_t *crc) &ms.connector_id, 1, ms.mode);igt_assert (rc ==0 ); -igt_debug_wait_for_keypress ("crc" );igt_pipe_crc_collect_crc (pipe_crc, crc);kmstest_unset_all_crtcs (drm_fd, drm_res);
@@ -171,7 +169,6 @@ static void fill_fb_subtest(void) &ms.connector_id, 1, ms.mode);igt_assert (rc ==0 ); -igt_debug_wait_for_keypress ("crc" );igt_pipe_crc_collect_crc (pipe_crc, &base_crc);get_fill_crc (LOCAL_DRM_FORMAT_MOD_NONE , &crc);
tests/kms_plane.c +0 -2
@@ -330,8 +330,6 @@ test_plane_panning_with_output(data_t *data,igt_pipe_crc_collect_crc (data->pipe_crc, &crc); -igt_debug_wait_for_keypress ("crc" ); -if (flags &TEST_PANNING_TOP_LEFT )igt_assert_crc_equal (&test.red_crc, &crc);else
Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 7 lines · Paulo Zanoni
From: Paulo Zanoni <[email protected]>
Let's just steal the "crc" namespace and add this by default to
igt_pipe_crc_collect_crc()instead of adding more calls to other tests. If tests want special waits on just some of theircollect_crc()calls, they can use another name instead of "crc".
Quoted 6 lines
This is very useful when developing, especially when the CRC we get is
wrong: we want to look at the screen to see what's going on before we can think about how to fix the problem. So let's add this to the lib instead of adding this to every single test I need to debug.Signed-off-by Paulo Zanoni <[email protected]>
My idea was that we'd only capture crc for test pictures and not reference pictures. But references pictures can equally go wrong, so this makes a lot of sense. A-b: me. One request though: Can you please update the docs to link to the interactive debug section and say that "crc" is the magic keyword?
-Daniel
Quoted 75 lines
--- lib/igt_debugfs.c | 3 +++ tests/kms_draw_crc.c | 3 --- tests/kms_plane.c | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-)lib/igt_debugfs.c +3
-0@@ -35,6 +35,7 @@#include <i915_drm.h> #include "drmtest.h" +#include "igt_aux.h" #include "igt_kms.h" #include "igt_debugfs.h" @@ -512,6 +513,8 @@ static void crc_sanity_checks(igt_crc_t *crc)*/ void igt_pipe_crc_collect_crc (igt_pipe_crc_t *pipe_crc,igt_crc_t *out_crc) { +igt_debug_wait_for_keypress ("crc" ); +igt_pipe_crc_start (pipe_crc);read_one_crc (pipe_crc, out_crc);igt_pipe_crc_stop (pipe_crc);tests/kms_draw_crc.c +0
-3@@ -102,7 +102,6 @@ static void get_method_crc(enum igt_draw_method method, uint64_t tiling, &ms.connector_id, 1, ms.mode);igt_assert (rc ==0 ); -igt_debug_wait_for_keypress ("crc" );igt_pipe_crc_collect_crc (pipe_crc, crc);kmstest_unset_all_crtcs (drm_fd, drm_res);@@ -144,7 +143,6 @@ static void get_fill_crc(uint64_t tiling, igt_crc_t *crc) &ms.connector_id, 1, ms.mode);igt_assert (rc ==0 ); -igt_debug_wait_for_keypress ("crc" );igt_pipe_crc_collect_crc (pipe_crc, crc);kmstest_unset_all_crtcs (drm_fd, drm_res);@@ -171,7 +169,6 @@ static void fill_fb_subtest(void) &ms.connector_id, 1, ms.mode);igt_assert (rc ==0 ); -igt_debug_wait_for_keypress ("crc" );igt_pipe_crc_collect_crc (pipe_crc, &base_crc);get_fill_crc (LOCAL_DRM_FORMAT_MOD_NONE , &crc);tests/kms_plane.c +0
-2@@ -330,8 +330,6 @@ test_plane_panning_with_output(data_t *data,igt_pipe_crc_collect_crc (data->pipe_crc, &crc); -igt_debug_wait_for_keypress ("crc" ); -if (flags &TEST_PANNING_TOP_LEFT )igt_assert_crc_equal (&test.red_crc, &crc);else Signature · 6 lines
2.1.4 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
2015-05-08 4:29 GMT-03:00 Daniel Vetter <[email protected]>:
Quoted 13 lines · Paulo Zanoni
Quoted 12 lines
From: Paulo Zanoni <[email protected]>
Let's just steal the "crc" namespace and add this by default to
igt_pipe_crc_collect_crc()instead of adding more calls to other tests. If tests want special waits on just some of theircollect_crc()calls, they can use another name instead of "crc".This is very useful when developing, especially when the CRC we get is
wrong: we want to look at the screen to see what's going on before we can think about how to fix the problem. So let's add this to the lib instead of adding this to every single test I need to debug.
Quoted 4 lines
Quoted 1 line
Signed-off-by Paulo Zanoni <[email protected]>My idea was that we'd only capture crc for test pictures and not reference pictures.
And how do we differentiate this inside the lib? This patch was suggested by you while reviewing one of my previous patches. You explicitly said we should have the wait_for_keypress("crc") call inside igt_pipe_crc_collect_crc().
Since the library can't guess what's what, if someone wants to get just the test pictures, they should add their own wait_for_keypress("somethingelse") calls.
Quoted 4 lines
But references pictures can equally go wrong, so this makes a lot of sense. A-b: me. One request though: Can you please update the docs to link to the interactive debug section and say that "crc" is the magic keyword?
Good idea. I'll do this.
Quoted 82 lines
-Daniel
Quoted 71 lines
--- lib/igt_debugfs.c | 3 +++ tests/kms_draw_crc.c | 3 --- tests/kms_plane.c | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-)lib/igt_debugfs.c +3
-0@@ -35,6 +35,7 @@#include <i915_drm.h> #include "drmtest.h" +#include "igt_aux.h" #include "igt_kms.h" #include "igt_debugfs.h" @@ -512,6 +513,8 @@ static void crc_sanity_checks(igt_crc_t *crc)*/ void igt_pipe_crc_collect_crc (igt_pipe_crc_t *pipe_crc,igt_crc_t *out_crc) { +igt_debug_wait_for_keypress ("crc" ); +igt_pipe_crc_start (pipe_crc);read_one_crc (pipe_crc, out_crc);igt_pipe_crc_stop (pipe_crc);tests/kms_draw_crc.c +0
-3@@ -102,7 +102,6 @@ static void get_method_crc(enum igt_draw_method method, uint64_t tiling, &ms.connector_id, 1, ms.mode);igt_assert (rc ==0 ); -igt_debug_wait_for_keypress ("crc" );igt_pipe_crc_collect_crc (pipe_crc, crc);kmstest_unset_all_crtcs (drm_fd, drm_res);@@ -144,7 +143,6 @@ static void get_fill_crc(uint64_t tiling, igt_crc_t *crc) &ms.connector_id, 1, ms.mode);igt_assert (rc ==0 ); -igt_debug_wait_for_keypress ("crc" );igt_pipe_crc_collect_crc (pipe_crc, crc);kmstest_unset_all_crtcs (drm_fd, drm_res);@@ -171,7 +169,6 @@ static void fill_fb_subtest(void) &ms.connector_id, 1, ms.mode);igt_assert (rc ==0 ); -igt_debug_wait_for_keypress ("crc" );igt_pipe_crc_collect_crc (pipe_crc, &base_crc);get_fill_crc (LOCAL_DRM_FORMAT_MOD_NONE , &crc);tests/kms_plane.c +0
-3@@ -330,8 +330,6 @@ test_plane_panning_with_output(data_t *data,igt_pipe_crc_collect_crc (data->pipe_crc, &crc); -igt_debug_wait_for_keypress ("crc" ); -if (flags &TEST_PANNING_TOP_LEFT )igt_assert_crc_equal (&test.red_crc, &crc);else --2.1.4
Quoted 4 lines
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Signature · 5 lines
Paulo Zanoni _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoted 19 lines · Paulo Zanoni
2015-05-08 4:29 GMT-03:00 Daniel Vetter <[email protected]>:
Quoted 13 lines · Paulo Zanoni
Quoted 12 lines
From: Paulo Zanoni <[email protected]>
Let's just steal the "crc" namespace and add this by default to
igt_pipe_crc_collect_crc()instead of adding more calls to other tests. If tests want special waits on just some of theircollect_crc()calls, they can use another name instead of "crc".This is very useful when developing, especially when the CRC we get is
wrong: we want to look at the screen to see what's going on before we can think about how to fix the problem. So let's add this to the lib instead of adding this to every single test I need to debug.Quoted 4 lines
Quoted 1 line
Signed-off-by Paulo Zanoni <[email protected]>My idea was that we'd only capture crc for test pictures and not reference pictures.
Quoted 8 lines
And how do we differentiate this inside the lib? This patch was suggested by you while reviewing one of my previous patches. You explicitly said we should have the
wait_for_keypress("crc")call insideigt_pipe_crc_collect_crc().Since the library can't guess what's what, if someone wants to get just the test pictures, they should add their own
wait_for_keypress("somethingelse")calls.
Fully agreed, my comment wasn't meant as critique, just some information why I didn't do this right away.
-Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx