2015-11-05 16:53 GMT-02:00 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>:
> Sink CRC should be enough by itself to validate PSR. Also sometimes
> the error might be on the CRC calculations themselves. So let's give
> the flexibility to use either individually.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> tests/kms_frontbuffer_tracking.c | 29 +++++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index 606d0a9..d879493 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -232,7 +232,8 @@ struct draw_pattern_info pattern4;
> /* Command line parameters. */
> struct {
> bool check_status;
> - bool check_crc;
> + bool check_pipe_crc;
> + bool check_sink_crc;
> bool fbc_check_compression;
> bool fbc_check_last_action;
> bool no_edp;
> @@ -244,7 +245,8 @@ struct {
> int shared_fb_y_offset;
> } opt = {
> .check_status = true,
> - .check_crc = true,
> + .check_pipe_crc = true,
> + .check_sink_crc = true,
> .fbc_check_compression = true,
> .fbc_check_last_action = true,
> .no_edp = false,
> @@ -1578,15 +1580,17 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
> int flags__ = (flags); \
> struct both_crcs crc_; \
> \
> - if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC)) \
> + if (flags__ & DONT_ASSERT_CRC) \
> break; \
> \
> collect_crcs(&crc_); \
> print_crc("Calculated CRC:", &crc_); \
> \
> igt_assert(wanted_crc); \
> - igt_assert_crc_equal(&crc_.pipe, &wanted_crc->pipe); \
> - assert_sink_crc_equal(&crc_.sink, &wanted_crc->sink); \
> + if (opt.check_pipe_crc) \
> + igt_assert_crc_equal(&crc_.pipe, &wanted_crc->pipe); \
> + if (opt.check_sink_crc) \
> + assert_sink_crc_equal(&crc_.sink, &wanted_crc->sink); \
This, combined with patch 2/8, means that we may still get SKIPs in
case reading the sink CRC fails, even if we specify --pipe-crc-only.
We should avoid even collecting the CRCs in case the flags are passed
so we don't risk that SKIP you introduced.
> } while (0)
>
> #define do_status_assertions(flags_) do { \
> @@ -2926,7 +2930,16 @@ static int opt_handler(int option, int option_index, void *data)
> opt.check_status = false;
> break;
> case 'c':
> - opt.check_crc = false;
> + opt.check_pipe_crc = false;
> + opt.check_sink_crc = false;
> + break;
> + case 'S':
> + opt.check_pipe_crc = false;
> + opt.check_sink_crc = true;
> + break;
> + case 'P':
> + opt.check_pipe_crc = true;
> + opt.check_sink_crc = false;
> break;
> case 'o':
> opt.fbc_check_compression = false;
> @@ -2974,6 +2987,8 @@ static int opt_handler(int option, int option_index, void *data)
> const char *help_str =
> " --no-status-check Don't check for enable/disable status\n"
> " --no-crc-check Don't check for CRC values\n"
> +" --sink-crc-only Check for Sink CRC only. Don't check for Pipe CRC value\n"
> +" --pipe-crc-only Check for Pipe CRC only. Don't check for Sink CRC value\n"
I was trying really hard to keep all descriptions under 80 columns
because OCD :)
By the way, the logic behind these flags seems inverted. Since both
CRCs are enabled by default, the flags should be to prevent them. Why
not just make them "--no-sink-crc" and "--no-pipe-crc"? IMHO it's more
intuitive and clear since each flag only touches one of the knobs
instead of mandating the behavior on two different knobs (i.e.,
passing --sink-crc-only --pipe-crc-only doesn't make sense and only
the last flag will be respected, but passing --no-pipe-crc
--no-sink-crc makes sense since each flag controls only one of the
knobs).
Otherwise, I like the idea: many times I wrote small hacks to ignore
just sink CRCs but not pipe CRC but never ended up writing the option.
> " --no-fbc-compression-check Don't check for the FBC compression status\n"
> " --no-fbc-action-check Don't check for the FBC last action\n"
> " --no-edp Don't use eDP monitors\n"
> @@ -3097,6 +3112,8 @@ int main(int argc, char *argv[])
> struct option long_options[] = {
> { "no-status-check", 0, 0, 's'},
> { "no-crc-check", 0, 0, 'c'},
> + { "sink-crc-only", 0, 0, 'S'},
> + { "pipe-crc-only", 0, 0, 'P'},
> { "no-fbc-compression-check", 0, 0, 'o'},
> { "no-fbc-action-check", 0, 0, 'a'},
> { "no-edp", 0, 0, 'e'},
> --
> 2.4.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
