Re: [PATCH i-g-t] tests/xe_gt_freq: Avoid RPe usage in subtests

["Belgaumkar, Vinay" <vinay.belgaumkar@intel.com>]

On 7/28/2024 10:19 PM, Riana Tauro wrote:
> Hi Vinay
>
> On 7/26/2024 10:38 AM, Nilawar, Badal wrote:
>> Hi Vinay,
>>
>> On 24-07-2024 22:29, Vinay Belgaumkar wrote:
>>> We are seeing several instances where the RPe, which can be altered by
>>> pcode dynamically, is causing subtests to fail randomly. Instead of 
>>> relying
>>> on it, we can use a mid frequency value for these subtests and avoid 
>>> these
>>> failures.
>>>
>>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2262
>>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2256
>>>
>>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>> ---
>>>   tests/intel/xe_gt_freq.c | 37 +++++++++++++++++++++++++++++--------
>>>   1 file changed, 29 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/tests/intel/xe_gt_freq.c b/tests/intel/xe_gt_freq.c
>>> index 93ebb5ed0..442f5658f 100644
>>> --- a/tests/intel/xe_gt_freq.c
>>> +++ b/tests/intel/xe_gt_freq.c
>>> @@ -26,6 +26,9 @@
>>>   #include <sys/time.h>
>>>   #define MAX_N_EXEC_QUEUES 16
>>> +#define GT_FREQUENCY_MULTIPLIER    50
>>> +#define GT_FREQUENCY_SCALER    3
>>> +#define FREQ_UNIT_MHZ (GT_FREQUENCY_MULTIPLIER / GT_FREQUENCY_SCALER)
>>>   /*
>>>    * Too many intermediate components and steps before freq is adjusted
>>> @@ -70,6 +73,16 @@ static uint32_t get_freq(int fd, int gt_id, const 
>>> char *freq_name)
>>>       return freq;
>>>   }
>>> +static bool within_expected_range(uint32_t freq, uint32_t val)
>>> +{
>>> +    /*
>>> +     * GT Frequencies are requested at units of 16.66 Mhz, so allow
>>> +     * that tolerance.
>>> +     */
>>> +    return (freq <= val + FREQ_UNIT_MHZ) ||
>>> +        (freq >= val - FREQ_UNIT_MHZ);
>>> +}
>>> +
>>>   static uint32_t rpe(int fd, int gt_id)
>>>   {
>>>       return get_freq(fd, gt_id, "rpe");
>>> @@ -128,6 +141,8 @@ static void test_freq_basic_api(int fd, int gt_id)
> This test doesn't need the current rpe since it only sets min and max 
> to rpe and reads it back.
>
> We could use the previous saved value here
We saw failures here before which were related to the same RPe issue. 
SLPC min freq follows RPe. So it did cause failures previously - 
http://gfx-ci-internal.igk.intel.com/cibuglog/testresult/507280198?query_key=fc962f34cc955ef24ea629f91af5c7284fcf890c
>>>   {
>>>       uint32_t rpn = get_freq(fd, gt_id, "rpn");
>>>       uint32_t rp0 = get_freq(fd, gt_id, "rp0");
>>> +    uint32_t rpmid = (rp0 + rpn) / 2;
>>> +    uint32_t min_freq, max_freq;
>>>       /*
>>>        * Negative bound tests
>>> @@ -142,16 +157,18 @@ static void test_freq_basic_api(int fd, int 
>>> gt_id)
>>>       /* Assert min requests are respected from rp0 to rpn */
>>>       igt_assert(set_freq(fd, gt_id, "min", rp0) > 0);
>>>       igt_assert(get_freq(fd, gt_id, "min") == rp0);
>>> -    igt_assert(set_freq(fd, gt_id, "min", rpe(fd, gt_id)) > 0);
>>> -    igt_assert(get_freq(fd, gt_id, "min") == rpe(fd, gt_id));
>>> +    igt_assert(set_freq(fd, gt_id, "min", rpmid) > 0);
>>> +    min_freq = get_freq(fd, gt_id, "min");
>>> +    igt_assert(within_expected_range(min_freq, rpmid));
>>>       igt_assert(set_freq(fd, gt_id, "min", rpn) > 0);
>>>       igt_assert(get_freq(fd, gt_id, "min") == rpn);
>>>       /* Assert max requests are respected from rpn to rp0 */
>>>       igt_assert(set_freq(fd, gt_id, "max", rpn) > 0);
>>>       igt_assert(get_freq(fd, gt_id, "max") == rpn);
>>> -    igt_assert(set_freq(fd, gt_id, "max", rpe(fd, gt_id)) > 0);
>>> -    igt_assert(get_freq(fd, gt_id, "max") == rpe(fd, gt_id));
>>> +    igt_assert(set_freq(fd, gt_id, "max", rpmid) > 0);
>>> +    max_freq = get_freq(fd, gt_id, "min");
> get_freq(fd, gt_id, "max")
yup, typo!
>>> + igt_assert(within_expected_range(max_freq, rpmid));
>>>       igt_assert(set_freq(fd, gt_id, "max", rp0) > 0);
>>>       igt_assert(get_freq(fd, gt_id, "max") == rp0);
>>>   }
>>> @@ -168,6 +185,8 @@ static void test_freq_fixed(int fd, int gt_id, 
>>> bool gt_idle)
>>>   {
>>>       uint32_t rpn = get_freq(fd, gt_id, "rpn");
>>>       uint32_t rp0 = get_freq(fd, gt_id, "rp0");
>>> +    uint32_t rpmid = (rp0 + rpn) / 2;
>>> +    uint32_t cur_freq, act_freq;
>>>       igt_debug("Starting testing fixed request\n");
>>> @@ -190,17 +209,19 @@ static void test_freq_fixed(int fd, int gt_id, 
>>> bool gt_idle)
>
>
> The comment in this test needs to be modified since we are not using 
> Rpe here anymore

yes, will update.

Thanks,

Vinay.

>
>
> /*
>  * For Fixed freq we need to set both min and max to the desired value
>  * Then we check if hardware is actually operating at the desired freq
>  * And let's do this for all the 3 known Render Performance (RP) values.
>  */
>
> Thanks,
> Riana
>
>>>           igt_assert(get_freq(fd, gt_id, "act") == rpn);
>>>       }
>>> -    igt_assert(set_freq(fd, gt_id, "min", rpe(fd, gt_id)) > 0);
>>> -    igt_assert(set_freq(fd, gt_id, "max", rpe(fd, gt_id)) > 0);
>>> +    igt_assert(set_freq(fd, gt_id, "min", rpmid) > 0);
>>> +    igt_assert(set_freq(fd, gt_id, "max", rpmid) > 0);
>>>       usleep(ACT_FREQ_LATENCY_US);
>>> -    igt_assert(get_freq(fd, gt_id, "cur") == rpe(fd, gt_id));
>>> +    cur_freq = get_freq(fd, gt_id, "cur");
>>> +    igt_assert(within_expected_range(cur_freq, rpmid));
>>>       if (gt_idle) {
>>>           igt_assert_f(igt_wait(xe_is_gt_in_c6(fd, gt_id), 1000, 10),
>>>                    "GT %d should be in C6\n", gt_id);
>>>           igt_assert(get_freq(fd, gt_id, "act") == 0);
>>>       } else {
>>> -        igt_assert(get_freq(fd, gt_id, "act") == rpe(fd, gt_id));
>>> +        act_freq = get_freq(fd, gt_id, "act");
>>> +        igt_assert(within_expected_range(act_freq, rpmid));
>>>       }
>>>       igt_assert(set_freq(fd, gt_id, "min", rp0) > 0);
>>
>> We should cover freq range subtests as well.
>>
>> https://intel-gfx-ci.01.org/tree/intel-xe/xe-1667-9f8e597a1c39d7e316f9479e6f627c15dbc58e1d/shard-lnl-7/igt@xe_gt_freq@freq_range_exec.html 
>>
>>
>> Regards,
>> Badal