[PATCH i-g-t] tests/intel/xe-oa: Use subslice_mask in pec_sanity_check

[Cavitt, Jonathan]

-----Original Message-----
> From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of Ashutosh Dixit
> Sent: Monday, July 14, 2025 2:24 PM
> To: igt-dev at lists.freedesktop.org
> Cc: Nerlige Ramappa, Umesh <umesh.nerlige.ramappa at intel.com>
> Subject: [PATCH i-g-t] tests/intel/xe-oa: Use subslice_mask in pec_sanity_check
> > 
> > pec_sanity_check previously relied on pec counter values being 0 to ignore
> > missing xecores's. However we are now running into situations where pec
> > counters are non-zero even when xecores are missing. To get past this, use
> > subslice_mask (i.e. mask of present xecore's) to ignore missing xecore's.
> > 
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit at intel.com>
> > ---
> >  tests/intel/xe_oa.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> > index 724fea73f0..2de5802876 100644
> > --- a/tests/intel/xe_oa.c
> > +++ b/tests/intel/xe_oa.c
> > @@ -1023,12 +1023,12 @@ static void pec_sanity_check(const u32 *report0, const u32 *report1,
> >  
> >  		igt_debug("n %d: pec1[n] - pec0[n] %#" PRIx64 ", tick delta %#" PRIx64 "\n",
> >  			  n, pec1[n] - pec0[n], tick_delta);
> > -		/* 0 value for pec[xecore_idx[i]] indicates missing xecore */
> > -		if (pec1[n] && pec0[n])
> > +
> > +		/* Skip missing xecore's */
> > +		if (intel_xe_perf->devinfo.subslice_mask & BIT(i)) {
> > +			igt_assert(pec1[n] && pec0[n]);

On one hand, I don't think the check written here lines up with what we were checking for
previously, as I don't see where we were previously asserting that both pec1[n] and pec0[n]
had to exist.  On the other hand, the closest equivalent check we had, as seen below
(asserting pec0[n] exists if pec1[n] exists), didn't make much sense in isolation either, so it's
probably for the best we corrected that here if that was wrong and made the assertion far
stricter than before.

I wouldn't mind a short explanation of what's going on here, but I won't block on it.  Either
way, I agree with the change:
Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-Jonathan Cavitt

> >  			assert_within_epsilon(pec1[n] - pec0[n], tick_delta, 0.1);
> > -		/* Same test_event1_cycles_xecore* should be present in all reports */
> > -		if (pec1[n])
> > -			igt_assert(pec0[n]);
> > +		}
> >  	}
> >  
> >  	igt_debug("pec1[2] - pec0[2] %#" PRIx64 ", tick_delta * num_xecores: %#" PRIx64 "\n",
> > @@ -1201,6 +1201,7 @@ init_sys_info(void)
> >  	igt_debug("n_eu_slices: %"PRIu64"\n", intel_xe_perf->devinfo.n_eu_slices);
> >  	igt_debug("n_eu_sub_slices: %"PRIu64"\n", intel_xe_perf->devinfo.n_eu_sub_slices);
> >  	igt_debug("n_eus: %"PRIu64"\n", intel_xe_perf->devinfo.n_eus);
> > +	igt_debug("subslice_mask: %#"PRIx64"\n", intel_xe_perf->devinfo.subslice_mask);
> >  	igt_debug("timestamp_frequency = %"PRIu64"\n",
> >  		  intel_xe_perf->devinfo.timestamp_frequency);
> >  	igt_assert_neq(intel_xe_perf->devinfo.timestamp_frequency, 0);
> > -- 
> > 2.48.1
> > 
> > 
>