[PATCH i-g-t v2] tests/intel/xe_pm_residency: Skip on igt_power_open failure

[Cavitt, Jonathan]

-----Original Message-----
From: Tauro, Riana <riana.tauro at intel.com> 
Sent: Friday, October 4, 2024 7:22 AM
To: Cavitt, Jonathan <jonathan.cavitt at intel.com>; igt-dev at lists.freedesktop.org
Cc: Gupta, saurabhg <saurabhg.gupta at intel.com>; Zuo, Alex <alex.zuo at intel.com>; kamil.konieczny at linux.intel.com
Subject: Re: [PATCH i-g-t v2] tests/intel/xe_pm_residency: Skip on igt_power_open failure
> 
> Hi Jonathan
> 
> On 10/4/2024 7:28 PM, Jonathan-Cavitt wrote:
> > The xe_pm_residency at toggle-gt-c6 test fails when
> > gt_c6_power >= gt_c0_power.  These values are reported by
> > igt_power_get_mW, which will always report 0 in the case that
> > igt_power_open fails to open either the hwmon_fd or rapl.fd.  Since the
> > expected result will always be a failure in this case, skip the
> > assertion that gt_c6_power < gt_c0_power.
> > 
> > v2: Do not skip the full test, and instead only ignore the part that is
> > guaranteed to fail.
> > 
> > Signed-off-by: Jonathan-Cavitt <jonathan.cavitt at intel.com>
> > Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > CC: Riana Tauro <riana.tauro at intel.com>
> > ---
> >   tests/intel/xe_pm_residency.c | 10 +++++++---
> >   1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/intel/xe_pm_residency.c b/tests/intel/xe_pm_residency.c
> > index 772fe9b574..0f146bad1c 100644
> > --- a/tests/intel/xe_pm_residency.c
> > +++ b/tests/intel/xe_pm_residency.c
> > @@ -282,10 +282,14 @@ static void measure_power(struct igt_power *gpu, double *power)
> >   static void toggle_gt_c6(int fd, int n)
> >   {
> >   	double gt_c0_power, gt_c6_power;
> > -	int gt;
> > +	int gt, ret;
> >   	struct igt_power gpu;
> >   
> > -	igt_power_open(fd, &gpu, "gpu");
> > +	/*
> > +	 * igt_power_get_mW will always return 0 if igt_power_open fails,
> > +	 * so skip the power check in this case.
> > +	 */
> > +	ret = igt_power_open(fd, &gpu, "gpu");
> >   
> >   	do {
> >   		fw_handle = igt_debugfs_open(fd, "forcewake_all", O_RDONLY);
> > @@ -312,7 +316,7 @@ static void toggle_gt_c6(int fd, int n)
> >   	igt_info("GPU consumed %fmW in GT C6 and %fmW in GT C0\n", gt_c6_power, gt_c0_power);
> >   
> >   	/* FIXME: Remove dgfx check after hwmon is added */
> > -	if (!xe_has_vram(fd))
> > +	if (!xe_has_vram(fd) && !ret)
> Should be ||

|| would make it a valid use case to run the below assert when the system doesn't have vram, irrespective of if igt_power_open succeeds or not.
It would also make it a valid use case to run the below assert when igt_power_open succeeds, regardless of if the system has vram or not.
Are these valid use cases for checking the below assert?
-Jonathan Cavitt

> 
> Thanks
> Riana Tauro
> >   		igt_assert_f(gt_c6_power < gt_c0_power,
> >   			     "Power consumed in GT C6 should be lower than GT C0\n");
> >   }
>