| Message ID | 20230325003442.1767568-1-vinay.belgaumkar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t] tests/xe_guc_pc: Restore max freq first | expand |
On Fri, Mar 24, 2023 at 05:34:42PM -0700, Vinay Belgaumkar wrote: > When min/max are both at RPn, restoring min back to 300 > will not work. Max needs to be increased first. why max needs to come first in this case? we should probably at least document so we don't forget it again... > Also, add > igt_assert() here, which would have caught the issue. I was going to ask if we should really add asserts inside the fixture or maybe using igt_require instead, but then I noticed more cases doing the assert... > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> > --- > tests/xe/xe_guc_pc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/xe/xe_guc_pc.c b/tests/xe/xe_guc_pc.c > index 60c93288..43bf6f48 100644 > --- a/tests/xe/xe_guc_pc.c > +++ b/tests/xe/xe_guc_pc.c > @@ -489,8 +489,8 @@ igt_main > > igt_fixture { > xe_for_each_gt(fd, gt) { > - set_freq(sysfs, gt, "min", stash_min); > - set_freq(sysfs, gt, "max", stash_max); > + igt_assert(set_freq(sysfs, gt, "max", stash_max) > 0); > + igt_assert(set_freq(sysfs, gt, "min", stash_min) > 0); > } > close(sysfs); > xe_device_put(fd); > -- > 2.38.1 >
On 3/26/2023 3:51 AM, Rodrigo Vivi wrote: > On Fri, Mar 24, 2023 at 05:34:42PM -0700, Vinay Belgaumkar wrote: >> When min/max are both at RPn, restoring min back to 300 >> will not work. Max needs to be increased first. > why max needs to come first in this case? we should probably at > least document so we don't forget it again... I was assuming we use soft limits like in i915, but looks like we don't. So, this is not an issue. > >> Also, add >> igt_assert() here, which would have caught the issue. > I was going to ask if we should really add asserts inside the fixture > or maybe using igt_require instead, but then I noticed more cases > doing the assert... Do we still need to add the assert in this case? Thanks, Vinay. > >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> >> --- >> tests/xe/xe_guc_pc.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tests/xe/xe_guc_pc.c b/tests/xe/xe_guc_pc.c >> index 60c93288..43bf6f48 100644 >> --- a/tests/xe/xe_guc_pc.c >> +++ b/tests/xe/xe_guc_pc.c >> @@ -489,8 +489,8 @@ igt_main >> >> igt_fixture { >> xe_for_each_gt(fd, gt) { >> - set_freq(sysfs, gt, "min", stash_min); >> - set_freq(sysfs, gt, "max", stash_max); >> + igt_assert(set_freq(sysfs, gt, "max", stash_max) > 0); >> + igt_assert(set_freq(sysfs, gt, "min", stash_min) > 0); >> } >> close(sysfs); >> xe_device_put(fd); >> -- >> 2.38.1 >>
diff --git a/tests/xe/xe_guc_pc.c b/tests/xe/xe_guc_pc.c index 60c93288..43bf6f48 100644 --- a/tests/xe/xe_guc_pc.c +++ b/tests/xe/xe_guc_pc.c @@ -489,8 +489,8 @@ igt_main igt_fixture { xe_for_each_gt(fd, gt) { - set_freq(sysfs, gt, "min", stash_min); - set_freq(sysfs, gt, "max", stash_max); + igt_assert(set_freq(sysfs, gt, "max", stash_max) > 0); + igt_assert(set_freq(sysfs, gt, "min", stash_min) > 0); } close(sysfs); xe_device_put(fd);
When min/max are both at RPn, restoring min back to 300 will not work. Max needs to be increased first. Also, add igt_assert() here, which would have caught the issue. Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com> --- tests/xe/xe_guc_pc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)