2015-05-08 4:29 GMT-03:00 Daniel Vetter <daniel@xxxxxxxx>:
> On Thu, May 07, 2015 at 03:23:17PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>>
>> 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@xxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
