Intel-GFX Archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH i-g-t] tests/i915_pm_freq_api: Set min/max to expected values
@ 2023-09-18 19:02 Vinay Belgaumkar
  2023-09-20 14:04 ` [Intel-gfx] [igt-dev] " Riana Tauro
  2023-09-20 14:07 ` [Intel-gfx] " Rodrigo Vivi
  0 siblings, 2 replies; 5+ messages in thread
From: Vinay Belgaumkar @ 2023-09-18 19:02 UTC (permalink / raw
  To: intel-gfx, igt-dev

A prior(rps) test leaves the system in a bad state causing failures
in the basic test. Set min/max to expected values before running it.
Test will restore values at the end.

Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8670

Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 tests/intel/i915_pm_freq_api.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/intel/i915_pm_freq_api.c b/tests/intel/i915_pm_freq_api.c
index 03bd0d05b..6018692a2 100644
--- a/tests/intel/i915_pm_freq_api.c
+++ b/tests/intel/i915_pm_freq_api.c
@@ -55,7 +55,11 @@ static void test_freq_basic_api(int dirfd, int gt)
 	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
 	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
 	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
-	igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d", gt, rpn, rpe, rp0);
+	igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d\n", gt, rpn, rpe, rp0);
+
+	/* Set min/max to RPn, RP0 for baseline behavior */
+	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
+	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0) > 0);
 
 	/*
 	 * Negative bound tests
@@ -170,7 +174,7 @@ igt_main
 		for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
 			stash_min[gt] = get_freq(dirfd, RPS_MIN_FREQ_MHZ);
 			stash_max[gt] = get_freq(dirfd, RPS_MAX_FREQ_MHZ);
-			igt_debug("GT: %d, min: %d, max: %d", gt, stash_min[gt], stash_max[gt]);
+			igt_debug("GT: %d, min: %d, max: %d\n", gt, stash_min[gt], stash_max[gt]);
 			igt_pm_ignore_slpc_efficient_freq(i915, dirfd, true);
 		}
 		igt_install_exit_handler(restore_sysfs_freq);
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] tests/i915_pm_freq_api: Set min/max to expected values
  2023-09-18 19:02 [Intel-gfx] [PATCH i-g-t] tests/i915_pm_freq_api: Set min/max to expected values Vinay Belgaumkar
@ 2023-09-20 14:04 ` Riana Tauro
  2023-09-20 14:07 ` [Intel-gfx] " Rodrigo Vivi
  1 sibling, 0 replies; 5+ messages in thread
From: Riana Tauro @ 2023-09-20 14:04 UTC (permalink / raw
  To: Vinay Belgaumkar, intel-gfx, igt-dev



On 9/19/2023 12:32 AM, Vinay Belgaumkar wrote:
> A prior(rps) test leaves the system in a bad state causing failures
> in the basic test. Set min/max to expected values before running it.
> Test will restore values at the end.
> 
> Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8670
> 
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
LGTM
Reviewed-by: Riana Tauro <riana.tauro@intel.com>
> ---
>   tests/intel/i915_pm_freq_api.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/intel/i915_pm_freq_api.c b/tests/intel/i915_pm_freq_api.c
> index 03bd0d05b..6018692a2 100644
> --- a/tests/intel/i915_pm_freq_api.c
> +++ b/tests/intel/i915_pm_freq_api.c
> @@ -55,7 +55,11 @@ static void test_freq_basic_api(int dirfd, int gt)
>   	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
>   	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
>   	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
> -	igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d", gt, rpn, rpe, rp0);
> +	igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d\n", gt, rpn, rpe, rp0);
> +
> +	/* Set min/max to RPn, RP0 for baseline behavior */
> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
> +	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0) > 0);
>   
>   	/*
>   	 * Negative bound tests
> @@ -170,7 +174,7 @@ igt_main
>   		for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
>   			stash_min[gt] = get_freq(dirfd, RPS_MIN_FREQ_MHZ);
>   			stash_max[gt] = get_freq(dirfd, RPS_MAX_FREQ_MHZ);
> -			igt_debug("GT: %d, min: %d, max: %d", gt, stash_min[gt], stash_max[gt]);
> +			igt_debug("GT: %d, min: %d, max: %d\n", gt, stash_min[gt], stash_max[gt]);
>   			igt_pm_ignore_slpc_efficient_freq(i915, dirfd, true);
>   		}
>   		igt_install_exit_handler(restore_sysfs_freq);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH i-g-t] tests/i915_pm_freq_api: Set min/max to expected values
  2023-09-18 19:02 [Intel-gfx] [PATCH i-g-t] tests/i915_pm_freq_api: Set min/max to expected values Vinay Belgaumkar
  2023-09-20 14:04 ` [Intel-gfx] [igt-dev] " Riana Tauro
@ 2023-09-20 14:07 ` Rodrigo Vivi
  2023-09-20 16:18   ` Belgaumkar, Vinay
  1 sibling, 1 reply; 5+ messages in thread
From: Rodrigo Vivi @ 2023-09-20 14:07 UTC (permalink / raw
  To: Vinay Belgaumkar; +Cc: igt-dev, intel-gfx

On Mon, Sep 18, 2023 at 12:02:59PM -0700, Vinay Belgaumkar wrote:
> A prior(rps) test leaves the system in a bad state causing failures
> in the basic test.

Why?

What was the freq immediately before the failure that made the
machine to be busted and not accept the new freq request?

Maybe we should use this information to limit the freq requests
that we accept instead of workaround the test case. Otherwise
we are at risk of users selecting the bad freq that let " the
system in a bad state"...

> Set min/max to expected values before running it.
> Test will restore values at the end.
> 
> Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8670
> 
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  tests/intel/i915_pm_freq_api.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/intel/i915_pm_freq_api.c b/tests/intel/i915_pm_freq_api.c
> index 03bd0d05b..6018692a2 100644
> --- a/tests/intel/i915_pm_freq_api.c
> +++ b/tests/intel/i915_pm_freq_api.c
> @@ -55,7 +55,11 @@ static void test_freq_basic_api(int dirfd, int gt)
>  	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
>  	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
>  	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
> -	igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d", gt, rpn, rpe, rp0);
> +	igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d\n", gt, rpn, rpe, rp0);
> +
> +	/* Set min/max to RPn, RP0 for baseline behavior */
> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
> +	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0) > 0);
>  
>  	/*
>  	 * Negative bound tests
> @@ -170,7 +174,7 @@ igt_main
>  		for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
>  			stash_min[gt] = get_freq(dirfd, RPS_MIN_FREQ_MHZ);
>  			stash_max[gt] = get_freq(dirfd, RPS_MAX_FREQ_MHZ);
> -			igt_debug("GT: %d, min: %d, max: %d", gt, stash_min[gt], stash_max[gt]);
> +			igt_debug("GT: %d, min: %d, max: %d\n", gt, stash_min[gt], stash_max[gt]);
>  			igt_pm_ignore_slpc_efficient_freq(i915, dirfd, true);
>  		}
>  		igt_install_exit_handler(restore_sysfs_freq);
> -- 
> 2.38.1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH i-g-t] tests/i915_pm_freq_api: Set min/max to expected values
  2023-09-20 14:07 ` [Intel-gfx] " Rodrigo Vivi
@ 2023-09-20 16:18   ` Belgaumkar, Vinay
  2023-09-20 20:33     ` Rodrigo Vivi
  0 siblings, 1 reply; 5+ messages in thread
From: Belgaumkar, Vinay @ 2023-09-20 16:18 UTC (permalink / raw
  To: Rodrigo Vivi; +Cc: igt-dev, intel-gfx


On 9/20/2023 7:07 AM, Rodrigo Vivi wrote:
> On Mon, Sep 18, 2023 at 12:02:59PM -0700, Vinay Belgaumkar wrote:
>> A prior(rps) test leaves the system in a bad state causing failures
>> in the basic test.
> Why?
>
> What was the freq immediately before the failure that made the
> machine to be busted and not accept the new freq request?
>
> Maybe we should use this information to limit the freq requests
> that we accept instead of workaround the test case. Otherwise
> we are at risk of users selecting the bad freq that let " the
> system in a bad state"...

i915_pm_rps (waitboost) test sets soft max_freq to some value less than 
RP0 and then fails. The restore on failure does not work properly as the 
test is not multitile capable(it sets the root sysfs entry instead of 
using the per tile entry). Then, the current test (i915_pm_freq_api --r 
basic-api) tries to set min_freq to RP0 as part of normal testing. This 
fails as soft_max is < RP0.

There is some non-trivial effort needed to convert i915_pm_rps to 
multitile, and this is a BAT failure, hence adding the quick fix to 
ensure the test runs with a good pre-environment.

Thanks,

Vinay.

>
>> Set min/max to expected values before running it.
>> Test will restore values at the end.
>>
>> Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8670
>>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> ---
>>   tests/intel/i915_pm_freq_api.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/intel/i915_pm_freq_api.c b/tests/intel/i915_pm_freq_api.c
>> index 03bd0d05b..6018692a2 100644
>> --- a/tests/intel/i915_pm_freq_api.c
>> +++ b/tests/intel/i915_pm_freq_api.c
>> @@ -55,7 +55,11 @@ static void test_freq_basic_api(int dirfd, int gt)
>>   	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
>>   	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
>>   	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
>> -	igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d", gt, rpn, rpe, rp0);
>> +	igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d\n", gt, rpn, rpe, rp0);
>> +
>> +	/* Set min/max to RPn, RP0 for baseline behavior */
>> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
>> +	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0) > 0);
>>   
>>   	/*
>>   	 * Negative bound tests
>> @@ -170,7 +174,7 @@ igt_main
>>   		for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
>>   			stash_min[gt] = get_freq(dirfd, RPS_MIN_FREQ_MHZ);
>>   			stash_max[gt] = get_freq(dirfd, RPS_MAX_FREQ_MHZ);
>> -			igt_debug("GT: %d, min: %d, max: %d", gt, stash_min[gt], stash_max[gt]);
>> +			igt_debug("GT: %d, min: %d, max: %d\n", gt, stash_min[gt], stash_max[gt]);
>>   			igt_pm_ignore_slpc_efficient_freq(i915, dirfd, true);
>>   		}
>>   		igt_install_exit_handler(restore_sysfs_freq);
>> -- 
>> 2.38.1
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Intel-gfx] [PATCH i-g-t] tests/i915_pm_freq_api: Set min/max to expected values
  2023-09-20 16:18   ` Belgaumkar, Vinay
@ 2023-09-20 20:33     ` Rodrigo Vivi
  0 siblings, 0 replies; 5+ messages in thread
From: Rodrigo Vivi @ 2023-09-20 20:33 UTC (permalink / raw
  To: Belgaumkar, Vinay; +Cc: igt-dev, intel-gfx

On Wed, Sep 20, 2023 at 09:18:07AM -0700, Belgaumkar, Vinay wrote:
> 
> On 9/20/2023 7:07 AM, Rodrigo Vivi wrote:
> > On Mon, Sep 18, 2023 at 12:02:59PM -0700, Vinay Belgaumkar wrote:
> > > A prior(rps) test leaves the system in a bad state causing failures
> > > in the basic test.
> > Why?
> > 
> > What was the freq immediately before the failure that made the
> > machine to be busted and not accept the new freq request?
> > 
> > Maybe we should use this information to limit the freq requests
> > that we accept instead of workaround the test case. Otherwise
> > we are at risk of users selecting the bad freq that let " the
> > system in a bad state"...
> 
> i915_pm_rps (waitboost) test sets soft max_freq to some value less than RP0
> and then fails. The restore on failure does not work properly as the test is
> not multitile capable(it sets the root sysfs entry instead of using the per
> tile entry). Then, the current test (i915_pm_freq_api --r basic-api) tries
> to set min_freq to RP0 as part of normal testing. This fails as soft_max is
> < RP0.
> 
> There is some non-trivial effort needed to convert i915_pm_rps to multitile,
> and this is a BAT failure, hence adding the quick fix to ensure the test
> runs with a good pre-environment.

okay, right, regardless the issue on the other test, this one is working
with some assumptions that needs to be corrected.
We either correct the assumption and set the max while setting the min,
or we do like this patch and make the assumption true.

Let's go with your patch

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> 
> Thanks,
> 
> Vinay.
> 
> > 
> > > Set min/max to expected values before running it.
> > > Test will restore values at the end.
> > > 
> > > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8670
> > > 
> > > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> > > ---
> > >   tests/intel/i915_pm_freq_api.c | 8 ++++++--
> > >   1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tests/intel/i915_pm_freq_api.c b/tests/intel/i915_pm_freq_api.c
> > > index 03bd0d05b..6018692a2 100644
> > > --- a/tests/intel/i915_pm_freq_api.c
> > > +++ b/tests/intel/i915_pm_freq_api.c
> > > @@ -55,7 +55,11 @@ static void test_freq_basic_api(int dirfd, int gt)
> > >   	rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
> > >   	rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
> > >   	rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
> > > -	igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d", gt, rpn, rpe, rp0);
> > > +	igt_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d\n", gt, rpn, rpe, rp0);
> > > +
> > > +	/* Set min/max to RPn, RP0 for baseline behavior */
> > > +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
> > > +	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0) > 0);
> > >   	/*
> > >   	 * Negative bound tests
> > > @@ -170,7 +174,7 @@ igt_main
> > >   		for_each_sysfs_gt_dirfd(i915, dirfd, gt) {
> > >   			stash_min[gt] = get_freq(dirfd, RPS_MIN_FREQ_MHZ);
> > >   			stash_max[gt] = get_freq(dirfd, RPS_MAX_FREQ_MHZ);
> > > -			igt_debug("GT: %d, min: %d, max: %d", gt, stash_min[gt], stash_max[gt]);
> > > +			igt_debug("GT: %d, min: %d, max: %d\n", gt, stash_min[gt], stash_max[gt]);
> > >   			igt_pm_ignore_slpc_efficient_freq(i915, dirfd, true);
> > >   		}
> > >   		igt_install_exit_handler(restore_sysfs_freq);
> > > -- 
> > > 2.38.1
> > > 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-09-20 20:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-18 19:02 [Intel-gfx] [PATCH i-g-t] tests/i915_pm_freq_api: Set min/max to expected values Vinay Belgaumkar
2023-09-20 14:04 ` [Intel-gfx] [igt-dev] " Riana Tauro
2023-09-20 14:07 ` [Intel-gfx] " Rodrigo Vivi
2023-09-20 16:18   ` Belgaumkar, Vinay
2023-09-20 20:33     ` Rodrigo Vivi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).