[PATCH i-g-t] tests/intel: Don't abort the test on the unavailability of sysnode

[Gurram, Pravalika]

> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Sent: Thursday, December 5, 2024 12:00 AM
> To: Gurram, Pravalika <pravalika.gurram at intel.com>
> Cc: igt-dev at lists.freedesktop.org
> Subject: Re: [PATCH i-g-t] tests/intel: Don't abort the test on the unavailability of
> sysnode
> 
> Hi Pravalika,
> On 2024-12-03 at 19:36:26 +0530, Pravalika Gurram wrote:
> 
> I forgot about nits, first is subject, it is:
> 
> [PATCH i-g-t] tests/intel: Don't abort the test on the unavailability of sysnode
> 
> imho better:
> 
> [PATCH i-g-t] tests/intel/perf_pmu: Check availability of sysnode
> 
> > use calloc to assign NULL value in false case, because in restore case
> - ^
> Start with capital letter, s/use/Use/
> Also it is not NULL but zero, so
> 
> Use calloc to have zeros for sysnodes not present.
> 
> > it will restore the NULL value instead of garbage.
> 
> Here I do not get it, you do not want to write zero when an error happned and
> you did not read it. Do you have a case when only some nodes are present, and
> some others not?
> 
When sysnode not present it will have a garbage correct since we were using malloc before
and in the restore function, we are restoring with a garbage to avoid that am using aclloc here

> > test should not be abort/skip on the unavailability of sysnode
> 
> I also do not get it, you basically just skip when you cannot read a node, or am I
> missing something?
> On what platform did you test this? Which tests failed?
> Could you point to gitlab issues you are fixing?

In a few platforms "BLB ELK ILK PNV" the following node "rps_min_freq_mhz" is not present which is making 
igt to abort. Test should not be aborted

Please have a look of this JIRA
https://jira.devtools.intel.com/browse/VLK-65866?filter=-1

Regards,
Pravaika
> 
> Regards,
> Kamil
> 
> >
> > Signed-off-by: Pravalika Gurram <pravalika.gurram at intel.com>
> > ---
> >  tests/intel/perf_pmu.c | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/tests/intel/perf_pmu.c b/tests/intel/perf_pmu.c index
> > bfa2d501a..87ddfe1a9 100644
> > --- a/tests/intel/perf_pmu.c
> > +++ b/tests/intel/perf_pmu.c
> > @@ -1786,7 +1786,7 @@ test_frequency_idle(int gem_fd, unsigned int gt)
> >  	sysfs = igt_sysfs_gt_open(gem_fd, gt);
> >  	igt_require(sysfs >= 0);
> >
> > -	min_freq = igt_sysfs_get_u32(sysfs, "rps_RPn_freq_mhz");
> > +	__igt_sysfs_get_u32(sysfs, "rps_RPn_freq_mhz", &min_freq);
> >  	close(sysfs);
> >
> >  	/* While parked, our convention is to report the GPU at 0Hz */ @@
> > -2403,23 +2403,23 @@ static void save_sysfs_freq(int i915)
> >
> >  	num_gts = igt_sysfs_get_num_gt(i915);
> >
> > -	stash_min = (uint32_t *)malloc(sizeof(uint32_t) * num_gts);
> > -	stash_max = (uint32_t *)malloc(sizeof(uint32_t) * num_gts);
> > -	stash_boost = (uint32_t *)malloc(sizeof(uint32_t) * num_gts);
> > +	stash_min = (uint32_t *)calloc(num_gts, sizeof(uint32_t) * num_gts);
> > +	stash_max = (uint32_t *)calloc(num_gts, sizeof(uint32_t) * num_gts);
> > +	stash_boost = (uint32_t *)calloc(num_gts, sizeof(uint32_t) *
> > +num_gts);
> >
> >  	/* Save boost, min and max across GTs */
> >  	i915_for_each_gt(i915, tmp, gt) {
> >  		sysfs = igt_sysfs_gt_open(i915, gt);
> >  		igt_require(sysfs >= 0);
> >
> > -		stash_min[gt] = igt_sysfs_get_u32(sysfs, "rps_min_freq_mhz");
> > -		stash_max[gt] = igt_sysfs_get_u32(sysfs, "rps_max_freq_mhz");
> > -		stash_boost[gt] = igt_sysfs_get_u32(sysfs,
> "rps_boost_freq_mhz");
> > +		__igt_sysfs_get_u32(sysfs, "rps_min_freq_mhz",
> &stash_min[gt]);
> > +		__igt_sysfs_get_u32(sysfs, "rps_max_freq_mhz",
> &stash_max[gt]);
> > +		__igt_sysfs_get_u32(sysfs, "rps_boost_freq_mhz",
> &stash_boost[gt]);
> >  		igt_debug("GT: %d, min: %d, max: %d, boost:%d\n",
> >  			  gt, stash_min[gt], stash_max[gt], stash_boost[gt]);
> >
> > -		rpn_freq = igt_sysfs_get_u32(sysfs, "rps_RPn_freq_mhz");
> > -		rp0_freq = igt_sysfs_get_u32(sysfs, "rps_RP0_freq_mhz");
> > +		__igt_sysfs_get_u32(sysfs, "rps_RPn_freq_mhz", &rpn_freq);
> > +		__igt_sysfs_get_u32(sysfs, "rps_RP0_freq_mhz", &rp0_freq);
> >
> >  		/* Set pre-conditions, in case frequencies are in non-default
> state */
> >  		igt_require(__igt_sysfs_set_u32(sysfs, "rps_max_freq_mhz",
> > rp0_freq));
> > --
> > 2.34.1
> >