Re: [PATCH i-g-t] tests/kms_frontbuffer_tracking: convert macros to functions

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Seg, 2017-09-04 às 10:00 +0200, Daniel Vetter escreveu:
> On Fri, Sep 01, 2017 at 04:44:38PM -0300, Paulo Zanoni wrote:
> > Em Seg, 2017-08-14 às 11:25 +0200, Daniel Vetter escreveu:
> > > Macros that should be C functions but aren't are really hard to
> > > read and confusing. Convert them over.
> > > 
> > > v2: Clean up commit message and keep printing the line numbers
> > > (Paulo).
> > > 
> > > v3: Actually git add (silly me).
> > > 
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > 
> > Thanks for doing that. Works for me this way:
> > Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Thanks, pushed.
> 
> > But I'm still waiting for the patch that removes those bogus line
> > numbers in every error message we print :).
> 
> Hm, which bogus line numbers where?

Every igt_log message is starts with:
(kms_frontbuffer_tracking:XXXX) where XXXX is a number that means
nothing but looks like a line number, except that that line usually has
nothing to do with the message.

> -Daniel
> > 
> > > ---
> > >  tests/kms_frontbuffer_tracking.c | 171 ++++++++++++++++++++-----
> > > ----
> > > ----------
> > >  1 file changed, 89 insertions(+), 82 deletions(-)
> > > 
> > > diff --git a/tests/kms_frontbuffer_tracking.c
> > > b/tests/kms_frontbuffer_tracking.c
> > > index e03524f1c45b..a068c8afb6d8 100644
> > > --- a/tests/kms_frontbuffer_tracking.c
> > > +++ b/tests/kms_frontbuffer_tracking.c
> > > @@ -1677,88 +1677,95 @@ static int adjust_assertion_flags(const
> > > struct test_mode *t, int flags)
> > >  	return flags;
> > >  }
> > >  
> > > -#define do_crc_assertions(flags, mandatory_sink_crc) do {	
> > > 	
> > > \
> > > -	int flags__ = (flags);					
> > > 	\
> > > -	struct both_crcs crc_;					
> > > 	\
> > > -									
> > > \
> > > -	if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))	
> > > 	
> > > \
> > > -		break;						
> > > 	\
> > > -									
> > > \
> > > -	collect_crcs(&crc_, mandatory_sink_crc);			
> > > \
> > > -	print_crc("Calculated CRC:", &crc_);			
> > > 	
> > > \
> > > -									
> > > \
> > > -	igt_assert(wanted_crc);					
> > > 	\
> > > -	igt_assert_crc_equal(&crc_.pipe, &wanted_crc->pipe);	
> > > 	
> > > \
> > > -	if (mandatory_sink_crc)					
> > > 	\
> > > -		assert_sink_crc_equal(&crc_.sink, &wanted_crc-
> > > > sink);	\
> > > 
> > > -	else if (sink_crc.reliable &&				
> > > 	\
> > > -		 !sink_crc_equal(&crc_.sink, &wanted_crc->sink))
> > > 	
> > > \
> > > -		igt_info("Sink CRC differ, but not
> > > required\n"); 	
> > > \
> > > -} while (0)
> > > -
> > > -#define do_status_assertions(flags_) do {			
> > > 	
> > > \
> > > -	if (!opt.check_status) {					
> > > \
> > > -		/* Make sure we settle before continuing. */	
> > > 	
> > > \
> > > -		sleep(1);					
> > > 	
> > > \
> > > -		break;						
> > > 	\
> > > -	}							
> > > 	
> > > \
> > > -									
> > > \
> > > -	if (flags_ & ASSERT_FBC_ENABLED) {			
> > > 	
> > > \
> > > -		igt_require(!fbc_not_enough_stolen());		
> > > 	\
> > > -		igt_require(!fbc_stride_not_supported());	
> > > 	
> > > \
> > > -		if (!fbc_wait_until_enabled()) {			
> > > \
> > > -			fbc_print_status();			
> > > 	
> > > \
> > > -			igt_assert_f(false, "FBC disabled\n");	
> > > 	\
> > > -		}						
> > > 	
> > > \
> > > -									
> > > \
> > > -		if (opt.fbc_check_compression)			
> > > 	\
> > > -			igt_assert(fbc_wait_for_compression());	
> > > 	\
> > > -	} else if (flags_ & ASSERT_FBC_DISABLED) {		
> > > 	
> > > \
> > > -		igt_assert(!fbc_wait_until_enabled());		
> > > 	\
> > > -	}							
> > > 	
> > > \
> > > -									
> > > \
> > > -	if (flags_ & ASSERT_PSR_ENABLED) {			
> > > 	
> > > \
> > > -		if (!psr_wait_until_enabled()) {			
> > > \
> > > -			psr_print_status();			
> > > 	
> > > \
> > > -			igt_assert_f(false, "PSR disabled\n");	
> > > 	\
> > > -		}						
> > > 	
> > > \
> > > -	} else if (flags_ & ASSERT_PSR_DISABLED) {		
> > > 	
> > > \
> > > -		igt_assert(!psr_wait_until_enabled());		
> > > 	\
> > > -	}							
> > > 	
> > > \
> > > -} while (0)
> > > -
> > > -#define do_assertions(flags) do {				
> > > 	
> > > \
> > > -	int flags_ = adjust_assertion_flags(t, (flags));		
> > > \
> > > -	bool mandatory_sink_crc = t->feature & FEATURE_PSR;	
> > > 	
> > > \
> > > -									
> > > \
> > > -	wait_user(2, "Paused before assertions.");		
> > > 	
> > > \
> > > -									
> > > \
> > > -	/* Check the CRC to make sure the drawing operations
> > > work	
> > > \
> > > -	 * immediately, independently of the features being
> > > enabled.
> > > */	\
> > > -	do_crc_assertions(flags_, mandatory_sink_crc);		
> > > 	\
> > > -									
> > > \
> > > -	/* Now we can flush things to make the test faster. */	
> > > 	\
> > > -	do_flush(t);						
> > > 	
> > > \
> > > -									
> > > \
> > > -	do_status_assertions(flags_);				
> > > 	\
> > > -									
> > > \
> > > -	/* Check CRC again to make sure the compressed screen is
> > > ok,
> > > 	\
> > > -	 * except if we're not drawing on the primary screen. On
> > > this	\
> > > -	 * case, the first check should be enough and a new CRC
> > > check	\
> > > -	 * would only delay the test suite while adding no value
> > > to
> > > the	\
> > > -	 * test suite. */					
> > > 	
> > > \
> > > -	if (t->screen == SCREEN_PRIM)				
> > > 	\
> > > -		do_crc_assertions(flags_, mandatory_sink_crc);	
> > > 	\
> > > -									
> > > \
> > > -	if (fbc.supports_last_action &&
> > > opt.fbc_check_last_action) {
> > > 	\
> > > -		if (flags_ & ASSERT_LAST_ACTION_CHANGED)		
> > > \
> > > -			igt_assert(fbc_last_action_changed());	
> > > 	\
> > > -		else if (flags_ & ASSERT_NO_ACTION_CHANGE)	
> > > 	
> > > \
> > > -			igt_assert(!fbc_last_action_changed());	
> > > 	\
> > > -	}							
> > > 	
> > > \
> > > -									
> > > \
> > > -	wait_user(1, "Paused after assertions.");		
> > > 	
> > > \
> > > -} while (0)
> > > +static void do_crc_assertions(int flags, bool
> > > mandatory_sink_crc)
> > > +{
> > > +	struct both_crcs crc;
> > > +
> > > +	if (!opt.check_crc || (flags & DONT_ASSERT_CRC))
> > > +		return;
> > > +
> > > +	collect_crcs(&crc, mandatory_sink_crc);
> > > +	print_crc("Calculated CRC:", &crc);
> > > +
> > > +	igt_assert(wanted_crc);
> > > +	igt_assert_crc_equal(&crc.pipe, &wanted_crc->pipe);
> > > +	if (mandatory_sink_crc)
> > > +		assert_sink_crc_equal(&crc.sink, &wanted_crc-
> > > >sink);
> > > +	else if (sink_crc.reliable &&
> > > +		 !sink_crc_equal(&crc.sink, &wanted_crc->sink))
> > > +		igt_info("Sink CRC differ, but not required\n");
> > > +}
> > > +
> > > +static void do_status_assertions(int flags)
> > > +{
> > > +	if (!opt.check_status) {
> > > +		/* Make sure we settle before continuing. */
> > > +		sleep(1);
> > > +		return;
> > > +	}
> > > +
> > > +	if (flags & ASSERT_FBC_ENABLED) {
> > > +		igt_require(!fbc_not_enough_stolen());
> > > +		igt_require(!fbc_stride_not_supported());
> > > +		if (!fbc_wait_until_enabled()) {
> > > +			fbc_print_status();
> > > +			igt_assert_f(false, "FBC disabled\n");
> > > +		}
> > > +
> > > +		if (opt.fbc_check_compression)
> > > +			igt_assert(fbc_wait_for_compression());
> > > +	} else if (flags & ASSERT_FBC_DISABLED) {
> > > +		igt_assert(!fbc_wait_until_enabled());
> > > +	}
> > > +
> > > +	if (flags & ASSERT_PSR_ENABLED) {
> > > +		if (!psr_wait_until_enabled()) {
> > > +			psr_print_status();
> > > +			igt_assert_f(false, "PSR disabled\n");
> > > +		}
> > > +	} else if (flags & ASSERT_PSR_DISABLED) {
> > > +		igt_assert(!psr_wait_until_enabled());
> > > +	}
> > > +}
> > > +
> > > +static void __do_assertions(const struct test_mode *t, int
> > > flags,
> > > +			    int line)
> > > +{
> > > +	flags = adjust_assertion_flags(t, flags);
> > > +	bool mandatory_sink_crc = t->feature & FEATURE_PSR;
> > > +
> > > +	igt_debug("checking asserts in line %i\n", line);
> > > +
> > > +	wait_user(2, "Paused before assertions.");
> > > +
> > > +	/* Check the CRC to make sure the drawing operations
> > > work
> > > +	 * immediately, independently of the features being
> > > enabled.
> > > */
> > > +	do_crc_assertions(flags, mandatory_sink_crc);
> > > +
> > > +	/* Now we can flush things to make the test faster. */
> > > +	do_flush(t);
> > > +
> > > +	do_status_assertions(flags);
> > > +
> > > +	/* Check CRC again to make sure the compressed screen is
> > > ok,
> > > +	 * except if we're not drawing on the primary screen. On
> > > this
> > > +	 * case, the first check should be enough and a new CRC
> > > check
> > > +	 * would only delay the test suite while adding no value
> > > to
> > > the
> > > +	 * test suite. */
> > > +	if (t->screen == SCREEN_PRIM)
> > > +		do_crc_assertions(flags, mandatory_sink_crc);
> > > +
> > > +	if (fbc.supports_last_action &&
> > > opt.fbc_check_last_action) {
> > > +		if (flags & ASSERT_LAST_ACTION_CHANGED)
> > > +			igt_assert(fbc_last_action_changed());
> > > +		else if (flags & ASSERT_NO_ACTION_CHANGE)
> > > +			igt_assert(!fbc_last_action_changed());
> > > +	}
> > > +
> > > +	wait_user(1, "Paused after assertions.");
> > > +}
> > > +
> > > +#define do_assertions(__flags) __do_assertions(t, (__flags),
> > > __LINE__)
> > >  
> > >  static void enable_prim_screen_and_wait(const struct test_mode
> > > *t)
> > >  {
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx