[PATCH i-g-t] tests/intel/kms_frontbuffer_tracking: Reduce test complexity for simulation

[Cavitt, Jonathan]

-----Original Message-----
From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Reddy Guddati, Santhosh
Sent: Tuesday, July 15, 2025 7:05 AM
To: igt-dev at lists.freedesktop.org
Subject: Re: [PATCH i-g-t] tests/intel/kms_frontbuffer_tracking: Reduce test complexity for simulation
> 
> Hi Pranay,
> 
> On 11-07-2025 13:34, Pranay Samala wrote:
> > Reduce the number of iterations, draw_method combinations, and limit
> > execution to a single pipe to enable this test to run on simulation.
> > 
> > Signed-off-by: Pranay Samala <pranay.samala at intel.com>
> > ---
> >   tests/intel/kms_frontbuffer_tracking.c | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> > 
> > diff --git a/tests/intel/kms_frontbuffer_tracking.c b/tests/intel/kms_frontbuffer_tracking.c
> > index 66dcdc281..631b13aa1 100644
> > --- a/tests/intel/kms_frontbuffer_tracking.c
> > +++ b/tests/intel/kms_frontbuffer_tracking.c
> > @@ -2908,6 +2908,8 @@ static void multidraw_subtest(const struct test_mode *t)
> >   					assertions |= ASSERT_PSR_DISABLED;
> >   
> >   				do_assertions(assertions);
> > +				if (igt_run_in_simulation())
> > +					break;
> IMHO, Since the igt_run_in_simulation() is being called at multiple 
> places and if there is further optimisation needed for further tests, I 
> would recommend to add a static global variable and initialise it in 
> main, replace all instances with this variable.  This way we can avoid 
> function call overhead as the value remains constant through out the test.
> 
> Regards,
> Santhosh


The overhead for calling igt_run_in_simulation is rather small.  And it looks
like each instance will only be called once per run on simulation, wherein the
overhead would make the most difference.  So, the current usage should be
fine as-is.

Though I will partially agree that we can find a better way to use the function
to achieve this desired result.  Specifically, it looks like most of the added function
calls are used to short-circuit some for-loops and break out of them early.  Instead
of trying to break out of each for-loop early, perhaps we should be reducing
the duration of the for-loops to match the simulation requirements?

So here, for example, we'd do something like:

"""
static void multidraw_subtest(const struct test_mode *t)
{
	int r;
	int m1_max = IGT_DRAW_METHOD_COUNT;
	int m2_max = IGT_DRAW_METHOD_COUNT;
	int r_max = pattern->n_rects;
	int assertion = 0;
...
	target = pick_target(t, params);

	if (igt_run_in_simulation())
		m1_max = m2_max = r_max = 1;

	for (m1 = 0; m1 < m1_max; m1++) {
		for (m2 = m1 + 1; m2 < m2_max; m2++) {

			igt_debug("Methods %s and %s\n",
...
			for (r = 0; r < r_max; r++) {
				used_method = (r % 2 == 0) ? m1 : m2;
...
				do_assertions(assertions);
			}
...
		}
	}
}
"""

This kind of change should work everywhere except for move_subtest and
igt at kms_frontbuffer_tracking@pipe-fbc-rte at pipe-%s-%s (the last change
in this change log), though I'm confident that some related workaround
principles could be applied there as well.
-Jonathan Cavitt

>   			}
> >   
> >   			fill_fb_region(target, m2, COLOR_PRIM_BG);
> > @@ -2915,7 +2917,11 @@ static void multidraw_subtest(const struct test_mode *t)
> >   
> >   			update_wanted_crc(t, &blue_crcs[t->format].crc);
> >   			do_assertions(ASSERT_NO_ACTION_CHANGE);
> > +			if (igt_run_in_simulation())
> > +				break;
> >   		}
> > +		if (igt_run_in_simulation())
> > +			break;
> >   	}
> >   }
> >   
> > @@ -3275,6 +3281,8 @@ static void move_subtest(const struct test_mode *t)
> >   		update_wanted_crc(t, &pattern->crcs[t->format][r]);
> >   
> >   		do_assertions(assertions);
> > +		if (igt_run_in_simulation())
> > +			break;
> >   
> >   		/* "Move" the last rect to the same position just to make sure
> >   		 * this works too. */
> > @@ -3332,6 +3340,8 @@ static void onoff_subtest(const struct test_mode *t)
> >   		}
> >   
> >   		do_assertions(assertions);
> > +		if (igt_run_in_simulation())
> > +			break;
> >   	}
> >   }
> >   
> > @@ -4184,6 +4194,8 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
> >   
> >   					break; /* One output is enough. */
> >   				}
> > +				if (igt_run_in_simulation())
> > +					break;
> >   			}
> >   		}
> >   
> 
>