Re: [igt-dev] [PATCH i-g-t 1/2] lib: Add --skip-crc-compare option

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev