From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 18 Jul 2023 11:16:11 -0700 Message-ID: <87fs5lhyqc.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: "Belgaumkar, Vinay" In-Reply-To: References: <20230717184213.624518-1-vinay.belgaumkar@intel.com> <87pm4qhtss.wl-ashutosh.dixit@intel.com> <8544177f-994d-88a4-6c7f-aa64c84e8846@intel.com> <87msztj14r.wl-ashutosh.dixit@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [igt-dev] [PATCH v2 i-g-t] i915_pm_freq_api: Add some debug to tests List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: 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 > >>>> --- > >>>> 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.