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

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

On 7/29/2024 9:47 PM, Riana Tauro wrote:
> Hi Vinay
>
> On 7/30/2024 5:47 AM, 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.
> >
> > v2: Fix bug in the tolerance function. Remove rpe usage from freq_range*
> > as well (Badal). Fix test documentation to reflect change (Riana).
> >
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2200
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2196
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2262
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2256
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2203
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2412
> >
> > Cc: Riana Tauro <riana.tauro@xxxxxxxxx>
> > Cc: Badal Nilawar <badal.nilawar@xxxxxxxxx>
> > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx>
> > ---
> >   tests/intel/xe_gt_freq.c | 53 ++++++++++++++++++++++++++++------------
> >   1 file changed, 37 insertions(+), 16 deletions(-)
> >
> > diff --git a/tests/intel/xe_gt_freq.c b/tests/intel/xe_gt_freq.c
> > index 93ebb5ed0..1ada37834 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 DIV_ROUND_CLOSEST(GT_FREQUENCY_MULTIPLIER, 
> > GT_FREQUENCY_SCALER)
> >     /*
> >    * Too many intermediate components and steps before freq is adjusted
> > @@ -70,9 +73,14 @@ static uint32_t get_freq(int fd, int gt_id, const 
> > char *freq_name)
> >       return freq;
> >   }
> >   -static uint32_t rpe(int fd, int gt_id)
> > +static bool within_expected_range(uint32_t freq, uint32_t val)
> >   {
> > -    return get_freq(fd, gt_id, "rpe");
> > +    /*
> > +     * 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 get_throttle(int fd, int gt_id, const char 
> > *throttle_file)
> > @@ -128,6 +136,8 @@ static void test_freq_basic_api(int fd, int gt_id)
> >   {
> >       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 +152,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");
> Seeing the first issue after this change. Do we need a delay?
>
> https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_11490/bat-adlp-7/igt@xe_gt_freq@freq_basic_api.html 

Hmm, rpmid here would have been 650. SLPC ended up setting min to RPe, 
which is probably 700 here. Will change this conditional check.

Thanks,

Vinay.

> Thanks,
> Riana
>
>
> > + 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, "max");
> > +    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,13 +180,16 @@ 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");
> >         /*
> >        * 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.
> > +     * And let's do this for all the 2 known Render Performance (RP) 
> > values
> > +     * RP0 and RPn and something in between.
> >        */
> >       igt_assert(set_freq(fd, gt_id, "min", rpn) > 0);
> >       igt_assert(set_freq(fd, gt_id, "max", rpn) > 0);
> > @@ -190,17 +205,19 @@ static void test_freq_fixed(int fd, int gt_id, 
> > bool gt_idle)
> >           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, cur_freq));
> >       }
> >         igt_assert(set_freq(fd, gt_id, "min", rp0) > 0);
> > @@ -232,15 +249,17 @@ static void test_freq_fixed(int fd, int gt_id, 
> > bool gt_idle)
> >   static void test_freq_range(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, act;
> >         igt_debug("Starting testing range request\n");
> >         igt_assert(set_freq(fd, gt_id, "min", rpn) > 0);
> > -    igt_assert(set_freq(fd, gt_id, "max", rpe(fd, gt_id)) > 0);
> > +    igt_assert(set_freq(fd, gt_id, "max", rpmid) > 0);
> >       usleep(ACT_FREQ_LATENCY_US);
> >       cur = get_freq(fd, gt_id, "cur");
> > -    igt_assert(rpn <= cur && cur <= rpe(fd, gt_id));
> > +    igt_assert(rpn <= cur && cur <= rpmid + FREQ_UNIT_MHZ);
> >         if (gt_idle) {
> >           igt_assert_f(igt_wait(xe_is_gt_in_c6(fd, gt_id), 1000, 10),
> > @@ -248,7 +267,7 @@ static void test_freq_range(int fd, int gt_id, 
> > bool gt_idle)
> >           igt_assert(get_freq(fd, gt_id, "act") == 0);
> >       } else {
> >           act = get_freq(fd, gt_id, "act");
> > -        igt_assert(rpn <= act && act <= rpe(fd, gt_id));
> > +        igt_assert(rpn <= act && within_expected_range(act, cur));
> >       }
> >         igt_debug("Finished testing range request\n");
> > @@ -262,17 +281,19 @@ static void test_freq_range(int fd, int gt_id, 
> > bool gt_idle)
> >   static void test_freq_low_max(int fd, int gt_id)
> >   {
> >       uint32_t rpn = get_freq(fd, gt_id, "rpn");
> > +    uint32_t rp0 = get_freq(fd, gt_id, "rp0");
> > +    uint32_t rpmid = (rp0 + rpn) / 2;
> >         /*
> >        *  When max request < min request, max is ignored and min 
> > works like
> >        * a fixed one. Let's assert this assumption
> >        */
> > -    igt_assert(set_freq(fd, gt_id, "min", rpe(fd, gt_id)) > 0);
> > +    igt_assert(set_freq(fd, gt_id, "min", rpmid) > 0);
> >       igt_assert(set_freq(fd, gt_id, "max", rpn) > 0);
> >       usleep(ACT_FREQ_LATENCY_US);
> >         /* Refresh value of rpe, pcode could have adjusted it */
> > -    igt_assert(get_freq(fd, gt_id, "cur") == rpe(fd, gt_id));
> > +    igt_assert(within_expected_range(get_freq(fd, gt_id, "cur"), 
> > rpmid));
> >   }
> >     /**