Re: [Intel-gfx] [igt-dev] [PATCH v2 i-g-t] i915_pm_freq_api: Add some debug to tests

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 18 Jul 2023 11:00:36 -0700, Belgaumkar, Vinay wrote:
>
>
> On 7/17/2023 9:26 PM, Dixit, Ashutosh wrote:
> > On Mon, 17 Jul 2023 21:19:13 -0700, Belgaumkar, Vinay wrote:
> >>
> >> On 7/17/2023 6:50 PM, Dixit, Ashutosh wrote:
> >>> On Mon, 17 Jul 2023 11:42:13 -0700, Vinay Belgaumkar wrote:
> >>>> Some subtests seem to be failing in CI, use igt_assert_(lt/eq) which
> >>>> print the values being compared and some additional debug as well.
> >>>>
> >>>> v2: Print GT as well (Ashutosh)
> >>>>
> >>>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> >>>> ---
> >>>>    tests/i915/i915_pm_freq_api.c | 18 ++++++++----------
> >>>>    1 file changed, 8 insertions(+), 10 deletions(-)
> >>>>
> >>>> diff --git a/tests/i915/i915_pm_freq_api.c b/tests/i915/i915_pm_freq_api.c
> >>>> index 522abee35..a7bbd4896 100644
> >>>> --- a/tests/i915/i915_pm_freq_api.c
> >>>> +++ b/tests/i915/i915_pm_freq_api.c
> >>>> @@ -55,6 +55,7 @@ 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);
> >>>>
> >>>>	/*
> >>>>	 * Negative bound tests
> >>>> @@ -90,21 +91,18 @@ static void test_reset(int i915, int dirfd, int gt, int count)
> >>>>	int fd;
> >>>>
> >>>>	for (int i = 0; i < count; i++) {
> >>>> -		igt_assert_f(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0,
> >>>> -			     "Failed after %d good cycles\n", i);
> >>>> -		igt_assert_f(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpn) > 0,
> >>>> -			     "Failed after %d good cycles\n", i);
> >>>> +		igt_debug("Running cycle: %d", i);
> >>>> +		igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
> >>>> +		igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpn) > 0);
> >>> I am R-b'ing this but stuff like this should be using igt_assert_lt()
> >>> according to the commit message?
> >>>
> >>> This _lt stuff has to be fixed all over the file, not just this patch, if
> >>> it brings any value (again according to the commit message).
> >>>
> >>> Let me know if you want to fix this now or in a later patch. I'll wait
> >>> before merging.
> >> Yup, I will send out another version with the corrected commit message.
> > Hmm, I thought the code needs to be fixed not the commit message :)
>
> Ok, I meant this specific patch will address just the area where we check
> for the requested frequency. I will change the remaining in a separate
> patch.

Ok, merged. Thanks.