From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <14d722ff-f577-297d-db95-4d6f95d39336@intel.com> Date: Mon, 27 Mar 2023 16:04:50 -0700 Content-Language: en-US To: Rodrigo Vivi References: <20230325003442.1767568-1-vinay.belgaumkar@intel.com> From: "Belgaumkar, Vinay" In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Subject: Re: [igt-dev] [PATCH i-g-t] tests/xe_guc_pc: Restore max freq first 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 3/26/2023 3:51 AM, Rodrigo Vivi wrote: > On Fri, Mar 24, 2023 at 05:34:42PM -0700, Vinay Belgaumkar wrote: >> When min/max are both at RPn, restoring min back to 300 >> will not work. Max needs to be increased first. > why max needs to come first in this case? we should probably at > least document so we don't forget it again... I was assuming we use soft limits like in i915, but looks like we don't. So, this is not an issue. > >> Also, add >> igt_assert() here, which would have caught the issue. > I was going to ask if we should really add asserts inside the fixture > or maybe using igt_require instead, but then I noticed more cases > doing the assert... Do we still need to add the assert in this case? Thanks, Vinay. > >> Cc: Rodrigo Vivi >> Signed-off-by: Vinay Belgaumkar >> --- >> tests/xe/xe_guc_pc.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tests/xe/xe_guc_pc.c b/tests/xe/xe_guc_pc.c >> index 60c93288..43bf6f48 100644 >> --- a/tests/xe/xe_guc_pc.c >> +++ b/tests/xe/xe_guc_pc.c >> @@ -489,8 +489,8 @@ igt_main >> >> igt_fixture { >> xe_for_each_gt(fd, gt) { >> - set_freq(sysfs, gt, "min", stash_min); >> - set_freq(sysfs, gt, "max", stash_max); >> + igt_assert(set_freq(sysfs, gt, "max", stash_max) > 0); >> + igt_assert(set_freq(sysfs, gt, "min", stash_min) > 0); >> } >> close(sysfs); >> xe_device_put(fd); >> -- >> 2.38.1 >>