[Intel-gfx] [PATCH igt] lib/debugfs: wait_for_keypress("crc") when collecting CRC

[Paulo Zanoni]

2015-05-08 4:29 GMT-03:00 Daniel Vetter <daniel at ffwll.ch>:
> On Thu, May 07, 2015 at 03:23:17PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> 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.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>
> 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.

> 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.

> -Daniel
>
>> ---
>>  lib/igt_debugfs.c    | 3 +++
>>  tests/kms_draw_crc.c | 3 ---
>>  tests/kms_plane.c    | 2 --
>>  3 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
>> index e7b69de..d9761d8 100644
>> --- a/lib/igt_debugfs.c
>> +++ b/lib/igt_debugfs.c
>> @@ -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);
>> diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
>> index 1630cc2..9fcf997 100644
>> --- a/tests/kms_draw_crc.c
>> +++ b/tests/kms_draw_crc.c
>> @@ -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);
>> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
>> index dc83a89..741d0b4 100644
>> --- a/tests/kms_plane.c
>> +++ b/tests/kms_plane.c
>> @@ -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
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Paulo Zanoni