[Intel-gfx] [PATCH i-g-t] tests/xe_guc_pc: Restore max freq first

[Intel-gfx] [PATCH i-g-t] tests/xe_guc_pc: Restore max freq first

[Vinay Belgaumkar]

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(-)

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

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/xe_guc_pc: Restore max freq first

[Rodrigo Vivi]

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
> 

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/xe_guc_pc: Restore max freq first

[Belgaumkar, Vinay]

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
>>