Re: [PATCH i-g-t] tests/intel/xe_gt_freq: Add a wait after gt reset

[Riana Tauro <riana.tauro@intel.com>]

Hi Karthik

On 8/1/2024 11:50 AM, Poosa, Karthik wrote:
> 
> On 31-07-2024 16:06, Riana Tauro wrote:
>> Hi Karthik
>>
>> On 7/30/2024 5:20 PM, Karthik Poosa wrote:
>>> After reset min and max takes sometime to get set to rpn.
>>> So add a wait of 100ms after gt reset before reading
>>> min and max frequencies.
>>>
>>> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
>>> ---
>>>   tests/intel/xe_gt_freq.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/tests/intel/xe_gt_freq.c b/tests/intel/xe_gt_freq.c
>>> index 93ebb5ed0..718f720c0 100644
>>> --- a/tests/intel/xe_gt_freq.c
>>> +++ b/tests/intel/xe_gt_freq.c
>>> @@ -33,6 +33,12 @@
>>>    */
>>>   #define ACT_FREQ_LATENCY_US 100000
>>>   +/*
>>> + * After gt reset, min and max freq may take sometime to set to rpn.
>>> + * So adding a wait of 100ms after reset.
>>> + */
>>> +#define GT_RESET_FREQ_LATENCY_US 100000
>>
>> Both ACT_FREQ_LATENCY_US  and  GT_RESET_FREQ_LATENCY_US  are same values.
>>
>> Why not rename ACT_FREQ_LATENCY_US to LATENCY_US and use that instead?
> 
> ACT_FREQ_LATENCY_US is used between set_freq and get freq without gt 
> reset in between.
> 
> GT_RESET_FREQ_LATENCY_US is delay after reset, so kept it as a separate 
> macro.

Value in both cases is same. Can have a generic name and use it
in both cases.
> 
>>
>> Thanks,
>> Riana
>>> +
>>>   static int set_freq(int fd, int gt_id, const char *freq_name, 
>>> uint32_t freq)
>>>   {
>>>       int ret = -EAGAIN;
>>> @@ -318,6 +324,7 @@ static void test_reset(int fd, int gt_id, int 
>>> cycles)
>>>                    "Failed after %d good cycles\n", i);
>>>             xe_force_gt_reset_async(fd, gt_id);
>>> +        usleep(GT_RESET_FREQ_LATENCY_US);
>>>             igt_assert_f(get_freq(fd, gt_id, "min") == rpn,
>>>                    "Failed after %d good cycles\n", i);