From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4947010EB4A for ; Fri, 14 Oct 2022 17:16:34 +0000 (UTC) Date: Fri, 14 Oct 2022 19:16:28 +0200 From: Kamil Konieczny To: igt-dev@lists.freedesktop.org Message-ID: References: <20220915081704.3039599-1-karthikeya.sunkesula@intel.com> <20220915081704.3039599-3-karthikeya.sunkesula@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20220915081704.3039599-3-karthikeya.sunkesula@intel.com> Subject: Re: [igt-dev] [PATCH i-g-t 2/2] Add descriptions to pm rps tests List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: badal.nilawar@intel.com, Karthikeya Sunkesula Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: Hi Karthikeya, few more nits, see below. On 2022-09-15 at 13:47:04 +0530, Karthikeya Sunkesula wrote: > Signed-off-by: Karthikeya Sunkesula > --- > tests/i915/i915_pm_rps.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/tests/i915/i915_pm_rps.c b/tests/i915/i915_pm_rps.c > index db39ec69..44228b44 100644 > --- a/tests/i915/i915_pm_rps.c > +++ b/tests/i915/i915_pm_rps.c > @@ -914,12 +914,16 @@ igt_main > igt_install_exit_handler(pm_rps_exit_handler); > } > > + igt_describe("Verify the rps sysfs constraints, by varying min, max gt frequencies " --------------------- ^ > + "with RPn, RP0 and mid values with valid and invalid cases."); ------------------------^ Align to previous line, it should look like: igt_describe("Verify the rps sysfs constraints, by varying min, max gt frequencies " "with RPn, RP0 and mid values with valid and invalid cases."); > igt_subtest("basic-api") { > igt_skip_on_f(i915_is_slpc_enabled(drm_fd), > "This subtest is not supported when SLPC is enabled\n"); > min_max_config(basic_check, false); > } > > + igt_describe("Verify the constraints and verify current frequency settles down to " > + "RPn or min within the allotted time after a low workload is run."); Same here, align. > /* Verify the constraints, check if we can reach idle */ > igt_subtest("min-max-config-idle") { > igt_skip_on_f(i915_is_slpc_enabled(drm_fd), > @@ -927,6 +931,8 @@ igt_main > min_max_config(idle_check, true); > } > > + igt_describe("Verify the constraints and with a high workload, verify current " > + "frequency scales upto RP0 or max within the allotted time."); Same here. > /* Verify the constraints with high load, check if we can reach max */ > igt_subtest("min-max-config-loaded") { > igt_skip_on_f(i915_is_slpc_enabled(drm_fd), > @@ -937,6 +943,8 @@ igt_main > } > > /* Checks if we achieve boost using gem_wait */ > + igt_describe("Verify actual frequency bumps to boost frequency with workload " > + "with waitboost scenario."); Please make it shorter (look at previous replay). > igt_subtest("waitboost") { > igt_skip_on_f(i915_is_slpc_enabled(drm_fd), > "This subtest is not supported when SLPC is enabled\n"); > @@ -958,6 +966,7 @@ igt_main > } > > /* Test boost frequency after GPU reset */ > + igt_describe("Verify waitboost after GPU reset"); ----------------------------------------------------- ^ Keep it consistent, put dot at end of description. Regards, Kamil > igt_subtest("reset") { > igt_hang_t hang; > igt_skip_on_f(i915_is_slpc_enabled(drm_fd), > -- > 2.25.1 >