Re: [PATCH i-g-t] tests/pm_rps: Move some test logic out of boost function

[Sagar Arun Kamble <sagar.a.kamble@intel.com>]

On 10/18/2017 4:33 PM, Szwichtenberg, Radoslaw wrote:
> On Wed, 2017-10-18 at 11:10 +0530, Sagar Arun Kamble wrote:
>> On 10/17/2017 6:13 PM, Radoslaw Szwichtenberg wrote:
>>> Moving code out of the boost function will allow its usage
>>> in other/new test scenarios.
>>>
>>> Signed-off-by: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
>>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> ---
>>>    tests/pm_rps.c | 19 ++++++++++---------
>>>    1 file changed, 10 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/tests/pm_rps.c b/tests/pm_rps.c
>>> index 89f3e31c..dad87646 100644
>>> --- a/tests/pm_rps.c
>>> +++ b/tests/pm_rps.c
>>> @@ -583,11 +583,6 @@ static void boost_freq(int fd, int *boost_freqs)
>>>    	int64_t timeout = 1;
>>>    	igt_spin_t *load;
>>>    	unsigned int engine;
>>> -	int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
>>> -
>>> -	fmid = get_hw_rounded_freq(fmid);
>>> -	/* Set max freq to less then boost freq */
>>> -	writeval(sysfs_files[MAX].filp, fmid);
>>>    
>>>    	/* Put boost on the same engine as low load */
>>>    	engine = I915_EXEC_RENDER;
>>> @@ -604,9 +599,6 @@ static void boost_freq(int fd, int *boost_freqs)
>>>    	igt_spin_batch_end(load);
>>>    	gem_sync(fd, load->handle);
>>>    	igt_spin_batch_free(fd, load);
>>> -
>>> -	/* Set max freq to original softmax */
>>> -	writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
>>>    }
>>>    
>>>    static void waitboost(int fd, bool reset)
>>> @@ -615,6 +607,9 @@ static void waitboost(int fd, bool reset)
>>>    	int boost_freqs[NUMFREQ];
>>>    	int post_freqs[NUMFREQ];
>>>    
>>> +	int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
>>> +	fmid = get_hw_rounded_freq(fmid);
>>> +
>> How about function for getting mid? We can reuse in min_max_config too.
> You think about something like:
>
> static int get_fmid(void)
> {
> 	int fmid = (origfreqs[RPn] + origfreqs[RP0]) / 2;
> 	return get_hw_rounded_freq(fmid);
> }
>
> Even though it wasn't scope of this change I think I can add it - do you think
> it will be beneficial? We just use it two places so at this moment the benefit
> will be minimal.
Ok. Can skip for now.
>>>    	load_helper_run(LOW);
>>>    
>>>    	igt_debug("Apply low load...\n");
>>> @@ -627,11 +622,17 @@ static void waitboost(int fd, bool reset)
>>>    		sleep(1);
>>>    	}
>>>    
>>> +	/* Set max freq to less than boost freq */
>>> +	writeval(sysfs_files[MAX].filp, fmid);
>>> +
>>>    	/* When we wait upon the GPU, we want to temporarily boost it
>>>    	 * to maximum.
>>>    	 */
>>>    	boost_freq(fd, boost_freqs);
>>>    
>>> +	/* Set max freq to original softmax */
>>> +	writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
>>> +
>>>    	igt_debug("Apply low load again...\n");
>>>    	sleep(1);
>>>    	stabilize_check(post_freqs);
>>> @@ -643,7 +644,6 @@ static void waitboost(int fd, bool reset)
>>>    	igt_assert_lt(pre_freqs[CUR], pre_freqs[MAX]);
>>>    	igt_assert_eq(boost_freqs[CUR], boost_freqs[BOOST]);
>>>    	igt_assert_lt(post_freqs[CUR], post_freqs[MAX]);
>>> -
>>>    }
>>>    
>>>    static void pm_rps_exit_handler(int sig)
>>> @@ -656,6 +656,7 @@ static void pm_rps_exit_handler(int sig)
>>>    		writeval(sysfs_files[MAX].filp, origfreqs[MAX]);
>>>    	}
>>>    
>>> +	writeval(sysfs_files[BOOST].filp, origfreqs[BOOST]);
>> We are not changing boost_freq in current patch. Is this planned in new
>> testcase?
>> Please defer this change until then.
> Will remove!
>
> Thanks for review!
> Radek
>>>    	load_helper_deinit();
>>>    	close(drm_fd);
>>>    }

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx