| Message ID | 20190221003422.5413-1-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t,1/2] lib: Add --skip-crc-compare option | expand |
On Wed, Feb 20, 2019 at 04:34:21PM -0800, Matt Roper wrote: > When using --interactive-debug, it's sometimes desirable to ignore CRC > mismatches and let the test proceed as if they passed so that the > on-screen outcome can be inspected. Let's add a debug option to allow > this. > > Cc: igt-dev@lists.freedesktop.org > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> I'm wondering whether we shouldn't tie this into an --interactive-debug mode, since you'll need both for the manual run to be actually useful. E.g. anytime any --interactive-debug is set we skip all crc checks. -Daniel > --- > lib/igt_core.c | 7 +++++++ > lib/igt_core.h | 1 + > lib/igt_debugfs.c | 8 +++++++- > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index 71b05d3b..5523950f 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -256,6 +256,7 @@ > > static unsigned int exit_handler_count; > const char *igt_interactive_debug; > +bool igt_skip_crc_compare; > > /* subtests helpers */ > static bool list_subtests = false; > @@ -285,6 +286,7 @@ enum { > OPT_DESCRIPTION, > OPT_DEBUG, > OPT_INTERACTIVE_DEBUG, > + OPT_SKIP_CRC, > OPT_HELP = 'h' > }; > > @@ -552,6 +554,7 @@ static void print_usage(const char *help_str, bool output_on_stderr) > " --run-subtest <pattern>\n" > " --debug[=log-domain]\n" > " --interactive-debug[=domain]\n" > + " --skip-crc-compare\n" > " --help-description\n" > " --help\n"); > if (help_str) > @@ -668,6 +671,7 @@ static int common_init(int *argc, char **argv, > {"help-description", 0, 0, OPT_DESCRIPTION}, > {"debug", optional_argument, 0, OPT_DEBUG}, > {"interactive-debug", optional_argument, 0, OPT_INTERACTIVE_DEBUG}, > + {"skip-crc-compare", 0, 0, OPT_SKIP_CRC}, > {"help", 0, 0, OPT_HELP}, > {0, 0, 0, 0} > }; > @@ -768,6 +772,9 @@ static int common_init(int *argc, char **argv, > print_test_description(); > ret = -1; > goto out; > + case OPT_SKIP_CRC: > + igt_skip_crc_compare = true; > + goto out; > case OPT_HELP: > print_usage(help_str, false); > ret = -1; > diff --git a/lib/igt_core.h b/lib/igt_core.h > index 47ffd9e7..f12fc5cb 100644 > --- a/lib/igt_core.h > +++ b/lib/igt_core.h > @@ -843,6 +843,7 @@ bool igt_run_in_simulation(void); > void igt_skip_on_simulation(void); > > extern const char *igt_interactive_debug; > +extern bool igt_skip_crc_compare; > > /** > * igt_log_level: > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c > index 640c78e9..04d1648b 100644 > --- a/lib/igt_debugfs.c > +++ b/lib/igt_debugfs.c > @@ -405,6 +405,12 @@ static bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b, > * assert that CRCs match, never that they are different. Otherwise there might > * be random testcase failures when different screen contents end up with the > * same CRC by chance. > + * > + * Passing --skip-crc-compare on the command line will force this function > + * to always pass, which can be useful in interactive debugging where you > + * might know the test will fail, but still want the test to keep going as if > + * it had succeeded so that you can see the on-screen behavior. > + * > */ > void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > { > @@ -416,7 +422,7 @@ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > igt_debug("CRC mismatch at index %d: 0x%x != 0x%x\n", index, > a->crc[index], b->crc[index]); > > - igt_assert(!mismatch); > + igt_assert(!mismatch || igt_skip_crc_compare); > } > > /** > -- > 2.14.5 > > _______________________________________________ > igt-dev mailing list > igt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/igt-dev
On Thu, Feb 21, 2019 at 10:33:25AM +0100, Daniel Vetter wrote: > On Wed, Feb 20, 2019 at 04:34:21PM -0800, Matt Roper wrote: > > When using --interactive-debug, it's sometimes desirable to ignore CRC > > mismatches and let the test proceed as if they passed so that the > > on-screen outcome can be inspected. Let's add a debug option to allow > > this. > > > > Cc: igt-dev@lists.freedesktop.org > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > I'm wondering whether we shouldn't tie this into an --interactive-debug > mode, since you'll need both for the manual run to be actually useful. > E.g. anytime any --interactive-debug is set we skip all crc checks. That might make sense, but I'd like to still have the option to disable the crc checks without interactive debug as well. Good for observing tearing and whatnot. When looking at failures I seem to end up patching the crc checks out most of the time and relying on the tracepoints to get me the data I need. This knob could save me a step. So Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > -Daniel > > > --- > > lib/igt_core.c | 7 +++++++ > > lib/igt_core.h | 1 + > > lib/igt_debugfs.c | 8 +++++++- > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/lib/igt_core.c b/lib/igt_core.c > > index 71b05d3b..5523950f 100644 > > --- a/lib/igt_core.c > > +++ b/lib/igt_core.c > > @@ -256,6 +256,7 @@ > > > > static unsigned int exit_handler_count; > > const char *igt_interactive_debug; > > +bool igt_skip_crc_compare; > > > > /* subtests helpers */ > > static bool list_subtests = false; > > @@ -285,6 +286,7 @@ enum { > > OPT_DESCRIPTION, > > OPT_DEBUG, > > OPT_INTERACTIVE_DEBUG, > > + OPT_SKIP_CRC, > > OPT_HELP = 'h' > > }; > > > > @@ -552,6 +554,7 @@ static void print_usage(const char *help_str, bool output_on_stderr) > > " --run-subtest <pattern>\n" > > " --debug[=log-domain]\n" > > " --interactive-debug[=domain]\n" > > + " --skip-crc-compare\n" > > " --help-description\n" > > " --help\n"); > > if (help_str) > > @@ -668,6 +671,7 @@ static int common_init(int *argc, char **argv, > > {"help-description", 0, 0, OPT_DESCRIPTION}, > > {"debug", optional_argument, 0, OPT_DEBUG}, > > {"interactive-debug", optional_argument, 0, OPT_INTERACTIVE_DEBUG}, > > + {"skip-crc-compare", 0, 0, OPT_SKIP_CRC}, > > {"help", 0, 0, OPT_HELP}, > > {0, 0, 0, 0} > > }; > > @@ -768,6 +772,9 @@ static int common_init(int *argc, char **argv, > > print_test_description(); > > ret = -1; > > goto out; > > + case OPT_SKIP_CRC: > > + igt_skip_crc_compare = true; > > + goto out; > > case OPT_HELP: > > print_usage(help_str, false); > > ret = -1; > > diff --git a/lib/igt_core.h b/lib/igt_core.h > > index 47ffd9e7..f12fc5cb 100644 > > --- a/lib/igt_core.h > > +++ b/lib/igt_core.h > > @@ -843,6 +843,7 @@ bool igt_run_in_simulation(void); > > void igt_skip_on_simulation(void); > > > > extern const char *igt_interactive_debug; > > +extern bool igt_skip_crc_compare; > > > > /** > > * igt_log_level: > > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c > > index 640c78e9..04d1648b 100644 > > --- a/lib/igt_debugfs.c > > +++ b/lib/igt_debugfs.c > > @@ -405,6 +405,12 @@ static bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b, > > * assert that CRCs match, never that they are different. Otherwise there might > > * be random testcase failures when different screen contents end up with the > > * same CRC by chance. > > + * > > + * Passing --skip-crc-compare on the command line will force this function > > + * to always pass, which can be useful in interactive debugging where you > > + * might know the test will fail, but still want the test to keep going as if > > + * it had succeeded so that you can see the on-screen behavior. > > + * > > */ > > void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > > { > > @@ -416,7 +422,7 @@ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > > igt_debug("CRC mismatch at index %d: 0x%x != 0x%x\n", index, > > a->crc[index], b->crc[index]); > > > > - igt_assert(!mismatch); > > + igt_assert(!mismatch || igt_skip_crc_compare); > > } > > > > /** > > -- > > 2.14.5 > > > > _______________________________________________ > > igt-dev mailing list > > igt-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/igt-dev > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > igt-dev mailing list > igt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/igt-dev
On Wed, Feb 20, 2019 at 04:34:21PM -0800, Matt Roper wrote: > When using --interactive-debug, it's sometimes desirable to ignore CRC > mismatches and let the test proceed as if they passed so that the > on-screen outcome can be inspected. Let's add a debug option to allow > this. > > Cc: igt-dev@lists.freedesktop.org > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > lib/igt_core.c | 7 +++++++ > lib/igt_core.h | 1 + > lib/igt_debugfs.c | 8 +++++++- > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index 71b05d3b..5523950f 100644 > --- a/lib/igt_core.c > +++ b/lib/igt_core.c > @@ -256,6 +256,7 @@ > > static unsigned int exit_handler_count; > const char *igt_interactive_debug; > +bool igt_skip_crc_compare; > > /* subtests helpers */ > static bool list_subtests = false; > @@ -285,6 +286,7 @@ enum { > OPT_DESCRIPTION, > OPT_DEBUG, > OPT_INTERACTIVE_DEBUG, > + OPT_SKIP_CRC, > OPT_HELP = 'h' > }; > > @@ -552,6 +554,7 @@ static void print_usage(const char *help_str, bool output_on_stderr) > " --run-subtest <pattern>\n" > " --debug[=log-domain]\n" > " --interactive-debug[=domain]\n" > + " --skip-crc-compare\n" > " --help-description\n" > " --help\n"); > if (help_str) > @@ -668,6 +671,7 @@ static int common_init(int *argc, char **argv, > {"help-description", 0, 0, OPT_DESCRIPTION}, > {"debug", optional_argument, 0, OPT_DEBUG}, > {"interactive-debug", optional_argument, 0, OPT_INTERACTIVE_DEBUG}, > + {"skip-crc-compare", 0, 0, OPT_SKIP_CRC}, > {"help", 0, 0, OPT_HELP}, > {0, 0, 0, 0} > }; > @@ -768,6 +772,9 @@ static int common_init(int *argc, char **argv, > print_test_description(); > ret = -1; > goto out; > + case OPT_SKIP_CRC: > + igt_skip_crc_compare = true; > + goto out; > case OPT_HELP: > print_usage(help_str, false); > ret = -1; > diff --git a/lib/igt_core.h b/lib/igt_core.h > index 47ffd9e7..f12fc5cb 100644 > --- a/lib/igt_core.h > +++ b/lib/igt_core.h > @@ -843,6 +843,7 @@ bool igt_run_in_simulation(void); > void igt_skip_on_simulation(void); > > extern const char *igt_interactive_debug; > +extern bool igt_skip_crc_compare; > > /** > * igt_log_level: > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c > index 640c78e9..04d1648b 100644 > --- a/lib/igt_debugfs.c > +++ b/lib/igt_debugfs.c > @@ -405,6 +405,12 @@ static bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b, > * assert that CRCs match, never that they are different. Otherwise there might > * be random testcase failures when different screen contents end up with the > * same CRC by chance. > + * > + * Passing --skip-crc-compare on the command line will force this function > + * to always pass, which can be useful in interactive debugging where you > + * might know the test will fail, but still want the test to keep going as if > + * it had succeeded so that you can see the on-screen behavior. > + * > */ > void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > { > @@ -416,7 +422,7 @@ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > igt_debug("CRC mismatch at index %d: 0x%x != 0x%x\n", index, > a->crc[index], b->crc[index]); > > - igt_assert(!mismatch); > + igt_assert(!mismatch || igt_skip_crc_compare); I think an igt_debug line here could be useful when we skip the assert. With that Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> aka Ville convinced me. -Daniel > } > > /** > -- > 2.14.5 > > _______________________________________________ > igt-dev mailing list > igt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/igt-dev
On Fri, Feb 22, 2019 at 03:32:50PM +0100, Daniel Vetter wrote: > On Wed, Feb 20, 2019 at 04:34:21PM -0800, Matt Roper wrote: > > When using --interactive-debug, it's sometimes desirable to ignore CRC > > mismatches and let the test proceed as if they passed so that the > > on-screen outcome can be inspected. Let's add a debug option to allow > > this. > > > > Cc: igt-dev@lists.freedesktop.org > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > --- > > lib/igt_core.c | 7 +++++++ > > lib/igt_core.h | 1 + > > lib/igt_debugfs.c | 8 +++++++- > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/lib/igt_core.c b/lib/igt_core.c > > index 71b05d3b..5523950f 100644 > > --- a/lib/igt_core.c > > +++ b/lib/igt_core.c > > @@ -256,6 +256,7 @@ > > > > static unsigned int exit_handler_count; > > const char *igt_interactive_debug; > > +bool igt_skip_crc_compare; > > > > /* subtests helpers */ > > static bool list_subtests = false; > > @@ -285,6 +286,7 @@ enum { > > OPT_DESCRIPTION, > > OPT_DEBUG, > > OPT_INTERACTIVE_DEBUG, > > + OPT_SKIP_CRC, > > OPT_HELP = 'h' > > }; > > > > @@ -552,6 +554,7 @@ static void print_usage(const char *help_str, bool output_on_stderr) > > " --run-subtest <pattern>\n" > > " --debug[=log-domain]\n" > > " --interactive-debug[=domain]\n" > > + " --skip-crc-compare\n" > > " --help-description\n" > > " --help\n"); > > if (help_str) > > @@ -668,6 +671,7 @@ static int common_init(int *argc, char **argv, > > {"help-description", 0, 0, OPT_DESCRIPTION}, > > {"debug", optional_argument, 0, OPT_DEBUG}, > > {"interactive-debug", optional_argument, 0, OPT_INTERACTIVE_DEBUG}, > > + {"skip-crc-compare", 0, 0, OPT_SKIP_CRC}, > > {"help", 0, 0, OPT_HELP}, > > {0, 0, 0, 0} > > }; > > @@ -768,6 +772,9 @@ static int common_init(int *argc, char **argv, > > print_test_description(); > > ret = -1; > > goto out; > > + case OPT_SKIP_CRC: > > + igt_skip_crc_compare = true; > > + goto out; > > case OPT_HELP: > > print_usage(help_str, false); > > ret = -1; > > diff --git a/lib/igt_core.h b/lib/igt_core.h > > index 47ffd9e7..f12fc5cb 100644 > > --- a/lib/igt_core.h > > +++ b/lib/igt_core.h > > @@ -843,6 +843,7 @@ bool igt_run_in_simulation(void); > > void igt_skip_on_simulation(void); > > > > extern const char *igt_interactive_debug; > > +extern bool igt_skip_crc_compare; > > > > /** > > * igt_log_level: > > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c > > index 640c78e9..04d1648b 100644 > > --- a/lib/igt_debugfs.c > > +++ b/lib/igt_debugfs.c > > @@ -405,6 +405,12 @@ static bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b, > > * assert that CRCs match, never that they are different. Otherwise there might > > * be random testcase failures when different screen contents end up with the > > * same CRC by chance. > > + * > > + * Passing --skip-crc-compare on the command line will force this function > > + * to always pass, which can be useful in interactive debugging where you > > + * might know the test will fail, but still want the test to keep going as if > > + * it had succeeded so that you can see the on-screen behavior. > > + * > > */ > > void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > > { > > @@ -416,7 +422,7 @@ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > > igt_debug("CRC mismatch at index %d: 0x%x != 0x%x\n", index, > > a->crc[index], b->crc[index]); > > > > - igt_assert(!mismatch); > > + igt_assert(!mismatch || igt_skip_crc_compare); > > I think an igt_debug line here could be useful when we skip the assert. > With that Daniel, is the debug print just above not sufficient? Or would you like a more explicit indication that we skipped the assert even though there was mismatch? I guess we could do something like: if (mismatch) igt_debug("CRC mismatch%s at index %d: 0x%x != 0x%x\n", skip_crc_compare ? " (ignored)" : "", index, a->crc[index], b->crc[index]); to avoid spamming the log with two lines per mismatch? I was just looking for this knob until I realized we never applied this patch :/ > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > aka Ville convinced me. > -Daniel > > > } > > > > /** > > -- > > 2.14.5 > > > > _______________________________________________ > > igt-dev mailing list > > igt-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/igt-dev > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jun 27, 2019 at 07:05:17PM +0300, Ville Syrjälä wrote: > On Fri, Feb 22, 2019 at 03:32:50PM +0100, Daniel Vetter wrote: > > On Wed, Feb 20, 2019 at 04:34:21PM -0800, Matt Roper wrote: > > > When using --interactive-debug, it's sometimes desirable to ignore CRC > > > mismatches and let the test proceed as if they passed so that the > > > on-screen outcome can be inspected. Let's add a debug option to allow > > > this. > > > > > > Cc: igt-dev@lists.freedesktop.org > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > --- > > > lib/igt_core.c | 7 +++++++ > > > lib/igt_core.h | 1 + > > > lib/igt_debugfs.c | 8 +++++++- > > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > > > diff --git a/lib/igt_core.c b/lib/igt_core.c > > > index 71b05d3b..5523950f 100644 > > > --- a/lib/igt_core.c > > > +++ b/lib/igt_core.c > > > @@ -256,6 +256,7 @@ > > > > > > static unsigned int exit_handler_count; > > > const char *igt_interactive_debug; > > > +bool igt_skip_crc_compare; > > > > > > /* subtests helpers */ > > > static bool list_subtests = false; > > > @@ -285,6 +286,7 @@ enum { > > > OPT_DESCRIPTION, > > > OPT_DEBUG, > > > OPT_INTERACTIVE_DEBUG, > > > + OPT_SKIP_CRC, > > > OPT_HELP = 'h' > > > }; > > > > > > @@ -552,6 +554,7 @@ static void print_usage(const char *help_str, bool output_on_stderr) > > > " --run-subtest <pattern>\n" > > > " --debug[=log-domain]\n" > > > " --interactive-debug[=domain]\n" > > > + " --skip-crc-compare\n" > > > " --help-description\n" > > > " --help\n"); > > > if (help_str) > > > @@ -668,6 +671,7 @@ static int common_init(int *argc, char **argv, > > > {"help-description", 0, 0, OPT_DESCRIPTION}, > > > {"debug", optional_argument, 0, OPT_DEBUG}, > > > {"interactive-debug", optional_argument, 0, OPT_INTERACTIVE_DEBUG}, > > > + {"skip-crc-compare", 0, 0, OPT_SKIP_CRC}, > > > {"help", 0, 0, OPT_HELP}, > > > {0, 0, 0, 0} > > > }; > > > @@ -768,6 +772,9 @@ static int common_init(int *argc, char **argv, > > > print_test_description(); > > > ret = -1; > > > goto out; > > > + case OPT_SKIP_CRC: > > > + igt_skip_crc_compare = true; > > > + goto out; > > > case OPT_HELP: > > > print_usage(help_str, false); > > > ret = -1; > > > diff --git a/lib/igt_core.h b/lib/igt_core.h > > > index 47ffd9e7..f12fc5cb 100644 > > > --- a/lib/igt_core.h > > > +++ b/lib/igt_core.h > > > @@ -843,6 +843,7 @@ bool igt_run_in_simulation(void); > > > void igt_skip_on_simulation(void); > > > > > > extern const char *igt_interactive_debug; > > > +extern bool igt_skip_crc_compare; > > > > > > /** > > > * igt_log_level: > > > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c > > > index 640c78e9..04d1648b 100644 > > > --- a/lib/igt_debugfs.c > > > +++ b/lib/igt_debugfs.c > > > @@ -405,6 +405,12 @@ static bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b, > > > * assert that CRCs match, never that they are different. Otherwise there might > > > * be random testcase failures when different screen contents end up with the > > > * same CRC by chance. > > > + * > > > + * Passing --skip-crc-compare on the command line will force this function > > > + * to always pass, which can be useful in interactive debugging where you > > > + * might know the test will fail, but still want the test to keep going as if > > > + * it had succeeded so that you can see the on-screen behavior. > > > + * > > > */ > > > void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > > > { > > > @@ -416,7 +422,7 @@ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > > > igt_debug("CRC mismatch at index %d: 0x%x != 0x%x\n", index, > > > a->crc[index], b->crc[index]); > > > > > > - igt_assert(!mismatch); > > > + igt_assert(!mismatch || igt_skip_crc_compare); > > > > I think an igt_debug line here could be useful when we skip the assert. > > With that > > Daniel, is the debug print just above not sufficient? Or would you like > a more explicit indication that we skipped the assert even though there > was mismatch? > > I guess we could do something like: > > if (mismatch) > igt_debug("CRC mismatch%s at index %d: 0x%x != 0x%x\n", > skip_crc_compare ? " (ignored)" : "", > index, a->crc[index], b->crc[index]); > > to avoid spamming the log with two lines per mismatch? > > I was just looking for this knob until I realized we never applied this > patch :/ Hm yeah I missed that debug line, I think that's good enough. -Daniel > > > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > aka Ville convinced me. > > -Daniel > > > > > } > > > > > > /** > > > -- > > > 2.14.5 > > > > > > _______________________________________________ > > > igt-dev mailing list > > > igt-dev@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
On Wed, Jul 03, 2019 at 12:36:27PM +0200, Daniel Vetter wrote: > On Thu, Jun 27, 2019 at 07:05:17PM +0300, Ville Syrjälä wrote: > > On Fri, Feb 22, 2019 at 03:32:50PM +0100, Daniel Vetter wrote: > > > On Wed, Feb 20, 2019 at 04:34:21PM -0800, Matt Roper wrote: > > > > When using --interactive-debug, it's sometimes desirable to ignore CRC > > > > mismatches and let the test proceed as if they passed so that the > > > > on-screen outcome can be inspected. Let's add a debug option to allow > > > > this. > > > > > > > > Cc: igt-dev@lists.freedesktop.org > > > > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > > > > --- > > > > lib/igt_core.c | 7 +++++++ > > > > lib/igt_core.h | 1 + > > > > lib/igt_debugfs.c | 8 +++++++- > > > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/lib/igt_core.c b/lib/igt_core.c > > > > index 71b05d3b..5523950f 100644 > > > > --- a/lib/igt_core.c > > > > +++ b/lib/igt_core.c > > > > @@ -256,6 +256,7 @@ > > > > > > > > static unsigned int exit_handler_count; > > > > const char *igt_interactive_debug; > > > > +bool igt_skip_crc_compare; > > > > > > > > /* subtests helpers */ > > > > static bool list_subtests = false; > > > > @@ -285,6 +286,7 @@ enum { > > > > OPT_DESCRIPTION, > > > > OPT_DEBUG, > > > > OPT_INTERACTIVE_DEBUG, > > > > + OPT_SKIP_CRC, > > > > OPT_HELP = 'h' > > > > }; > > > > > > > > @@ -552,6 +554,7 @@ static void print_usage(const char *help_str, bool output_on_stderr) > > > > " --run-subtest <pattern>\n" > > > > " --debug[=log-domain]\n" > > > > " --interactive-debug[=domain]\n" > > > > + " --skip-crc-compare\n" > > > > " --help-description\n" > > > > " --help\n"); > > > > if (help_str) > > > > @@ -668,6 +671,7 @@ static int common_init(int *argc, char **argv, > > > > {"help-description", 0, 0, OPT_DESCRIPTION}, > > > > {"debug", optional_argument, 0, OPT_DEBUG}, > > > > {"interactive-debug", optional_argument, 0, OPT_INTERACTIVE_DEBUG}, > > > > + {"skip-crc-compare", 0, 0, OPT_SKIP_CRC}, > > > > {"help", 0, 0, OPT_HELP}, > > > > {0, 0, 0, 0} > > > > }; > > > > @@ -768,6 +772,9 @@ static int common_init(int *argc, char **argv, > > > > print_test_description(); > > > > ret = -1; > > > > goto out; > > > > + case OPT_SKIP_CRC: > > > > + igt_skip_crc_compare = true; > > > > + goto out; > > > > case OPT_HELP: > > > > print_usage(help_str, false); > > > > ret = -1; > > > > diff --git a/lib/igt_core.h b/lib/igt_core.h > > > > index 47ffd9e7..f12fc5cb 100644 > > > > --- a/lib/igt_core.h > > > > +++ b/lib/igt_core.h > > > > @@ -843,6 +843,7 @@ bool igt_run_in_simulation(void); > > > > void igt_skip_on_simulation(void); > > > > > > > > extern const char *igt_interactive_debug; > > > > +extern bool igt_skip_crc_compare; > > > > > > > > /** > > > > * igt_log_level: > > > > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c > > > > index 640c78e9..04d1648b 100644 > > > > --- a/lib/igt_debugfs.c > > > > +++ b/lib/igt_debugfs.c > > > > @@ -405,6 +405,12 @@ static bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b, > > > > * assert that CRCs match, never that they are different. Otherwise there might > > > > * be random testcase failures when different screen contents end up with the > > > > * same CRC by chance. > > > > + * > > > > + * Passing --skip-crc-compare on the command line will force this function > > > > + * to always pass, which can be useful in interactive debugging where you > > > > + * might know the test will fail, but still want the test to keep going as if > > > > + * it had succeeded so that you can see the on-screen behavior. > > > > + * > > > > */ > > > > void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > > > > { > > > > @@ -416,7 +422,7 @@ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) > > > > igt_debug("CRC mismatch at index %d: 0x%x != 0x%x\n", index, > > > > a->crc[index], b->crc[index]); > > > > > > > > - igt_assert(!mismatch); > > > > + igt_assert(!mismatch || igt_skip_crc_compare); > > > > > > I think an igt_debug line here could be useful when we skip the assert. > > > With that > > > > Daniel, is the debug print just above not sufficient? Or would you like > > a more explicit indication that we skipped the assert even though there > > was mismatch? > > > > I guess we could do something like: > > > > if (mismatch) > > igt_debug("CRC mismatch%s at index %d: 0x%x != 0x%x\n", > > skip_crc_compare ? " (ignored)" : "", > > index, a->crc[index], b->crc[index]); > > > > to avoid spamming the log with two lines per mismatch? > > > > I was just looking for this knob until I realized we never applied this > > patch :/ > > Hm yeah I missed that debug line, I think that's good enough. Cool. Pimped the debug message as above and pushed.
diff --git a/lib/igt_core.c b/lib/igt_core.c index 71b05d3b..5523950f 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -256,6 +256,7 @@ static unsigned int exit_handler_count; const char *igt_interactive_debug; +bool igt_skip_crc_compare; /* subtests helpers */ static bool list_subtests = false; @@ -285,6 +286,7 @@ enum { OPT_DESCRIPTION, OPT_DEBUG, OPT_INTERACTIVE_DEBUG, + OPT_SKIP_CRC, OPT_HELP = 'h' }; @@ -552,6 +554,7 @@ static void print_usage(const char *help_str, bool output_on_stderr) " --run-subtest <pattern>\n" " --debug[=log-domain]\n" " --interactive-debug[=domain]\n" + " --skip-crc-compare\n" " --help-description\n" " --help\n"); if (help_str) @@ -668,6 +671,7 @@ static int common_init(int *argc, char **argv, {"help-description", 0, 0, OPT_DESCRIPTION}, {"debug", optional_argument, 0, OPT_DEBUG}, {"interactive-debug", optional_argument, 0, OPT_INTERACTIVE_DEBUG}, + {"skip-crc-compare", 0, 0, OPT_SKIP_CRC}, {"help", 0, 0, OPT_HELP}, {0, 0, 0, 0} }; @@ -768,6 +772,9 @@ static int common_init(int *argc, char **argv, print_test_description(); ret = -1; goto out; + case OPT_SKIP_CRC: + igt_skip_crc_compare = true; + goto out; case OPT_HELP: print_usage(help_str, false); ret = -1; diff --git a/lib/igt_core.h b/lib/igt_core.h index 47ffd9e7..f12fc5cb 100644 --- a/lib/igt_core.h +++ b/lib/igt_core.h @@ -843,6 +843,7 @@ bool igt_run_in_simulation(void); void igt_skip_on_simulation(void); extern const char *igt_interactive_debug; +extern bool igt_skip_crc_compare; /** * igt_log_level: diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index 640c78e9..04d1648b 100644 --- a/lib/igt_debugfs.c +++ b/lib/igt_debugfs.c @@ -405,6 +405,12 @@ static bool igt_find_crc_mismatch(const igt_crc_t *a, const igt_crc_t *b, * assert that CRCs match, never that they are different. Otherwise there might * be random testcase failures when different screen contents end up with the * same CRC by chance. + * + * Passing --skip-crc-compare on the command line will force this function + * to always pass, which can be useful in interactive debugging where you + * might know the test will fail, but still want the test to keep going as if + * it had succeeded so that you can see the on-screen behavior. + * */ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) { @@ -416,7 +422,7 @@ void igt_assert_crc_equal(const igt_crc_t *a, const igt_crc_t *b) igt_debug("CRC mismatch at index %d: 0x%x != 0x%x\n", index, a->crc[index], b->crc[index]); - igt_assert(!mismatch); + igt_assert(!mismatch || igt_skip_crc_compare); } /**
When using --interactive-debug, it's sometimes desirable to ignore CRC mismatches and let the test proceed as if they passed so that the on-screen outcome can be inspected. Let's add a debug option to allow this. Cc: igt-dev@lists.freedesktop.org Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- lib/igt_core.c | 7 +++++++ lib/igt_core.h | 1 + lib/igt_debugfs.c | 8 +++++++- 3 files changed, 15 insertions(+), 1 deletion(-)