[igt-dev] [PATCH i-g-t 2/2] Add descriptions to pm rps tests

[Kamil Konieczny]

Hi Karthikeya,

few more nits, see below.

On 2022-09-15 at 13:47:04 +0530, Karthikeya Sunkesula wrote:
> Signed-off-by: Karthikeya Sunkesula <karthikeya.sunkesula at intel.com>
> ---
>  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
>