[igt-dev] [PATCH 1/2] i915/gem_exec_await: Add subtests description

Kamil Konieczny kamil.konieczny at linux.intel.com
Fri Jun 10 12:49:02 UTC 2022


Hi Sinjan,

On 2022-06-10 at 11:18:33 +0530, sinjan.kumar at intel.com wrote:
> From: Sinjan Kumar <sinjan.kumar at intel.com>
> 
> Add subtest description for the following subtest
> 1. wide-all
> 2. wide-context

You do not need to add this here, it will be better placed in
cover letter.

> 
> Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Cc: Priyanka Dandamudi <priyanka.dandamudi at intel.com>
> Signed-off-by: Sinjan Kumar <sinjan.kumar at intel.com>
> ---
>  tests/i915/gem_exec_await.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/i915/gem_exec_await.c b/tests/i915/gem_exec_await.c
> index 4935cf39..dc8d0255 100644
> --- a/tests/i915/gem_exec_await.c
> +++ b/tests/i915/gem_exec_await.c
> @@ -276,9 +276,13 @@ igt_main
>  		igt_fork_hang_detector(device);
>  	}

Please add also global description of this test, please see for
other tests descrption, like gem_exec_basic.

>  
> +        igt_describe("Test to catch GPU hang by executing batch buffer "
> +                     "in a loop for the given timeout period.");

Please change this description, look at commit messages in git,
git log tests/i915/gem_exec_await.c

and when you see commit about moving tests into i195 folder,
use its hash to search further:

git log 741bf7064c46 -- tests/gem_exec_await.c

>  	igt_subtest("wide-all")
>  		wide(device, ctx, ring_size, TIMEOUT, 0);
>  
> +        igt_describe("Test to catch GPU hang by creating context and executing "
> +                     "batch buffer in a loop for the given timeout period.");

Same here, change it.

One last thing, use checkpatch script from linux kernel to catch
possible errors, here you used spaces at begin of line instead of
tabs, see log from checkpatch:

checkpatch 0002-i915-Add-subtests-description.patch

ERROR: code indent should use tabs where possible
#25: FILE: tests/i915/gem_exec_await.c:279:
+        igt_describe("Test to catch GPU hang by executing batch buffer "$

WARNING: please, no spaces at the start of a line
#25: FILE: tests/i915/gem_exec_await.c:279:
+        igt_describe("Test to catch GPU hang by executing batch buffer "$

Regards,
Kamil

>  	igt_subtest("wide-contexts") {
>  		gem_require_contexts(device);
>  		wide(device, ctx, ring_size, TIMEOUT, CONTEXTS);
> -- 
> 2.25.1
> 


More information about the igt-dev mailing list