Re: [igt-dev] [PATCH i-g-t 3/5] tests/kms_plane_multiple: Describe the test

["Kahola, Mika" <mika.kahola@intel.com>]

Improving documentation is always a positive thing.

Reviewed-by: Mika Kahola <mika.kahola@intel.com>

On Mon, 2019-06-17 at 13:54 +0300, Arkadiusz Hiler wrote:
> In this test all the subtest are doing the same exact thing but with
> different tiling / on a different pipe. Tiling/pipe is in the test
> name
> so no need to elaborate on them.
> 
> We can have just one igt_describe() on top of the group to describe
> all
> the subtests in one go, no copy-and-paste necessary.
> 
> The description is short and explains the spirit of the test
> (verifying
> atomicity of plane updates) without delving into the implementation
> details (holes, colors, etc.)
> 
> Other changes:
>  * The function used for grabbing reference CRC is now under a more
>    descriptive name.
>  * Comment explaining test implementation in more detail is moved to
> the
>    top of the function actually implementing those steps.
>  * Minor formatting touch ups.
> 
> Cc: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  tests/kms_plane_multiple.c | 39 ++++++++++++++++++++--------------
> ----
>  1 file changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
> index 0d3ba4ff..81ed45dd 100644
> --- a/tests/kms_plane_multiple.c
> +++ b/tests/kms_plane_multiple.c
> @@ -30,7 +30,7 @@
>  #include <string.h>
>  #include <time.h>
>  
> -IGT_TEST_DESCRIPTION("Test atomic mode setting with multiple planes
> ");
> +IGT_TEST_DESCRIPTION("Test atomic mode setting with multiple
> planes.");
>  
>  #define SIZE_PLANE      256
>  #define SIZE_CURSOR     128
> @@ -96,7 +96,7 @@ static void test_fini(data_t *data, igt_output_t
> *output, int n_planes)
>  }
>  
>  static void
> -test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe,
> +get_reference_crc(data_t *data, igt_output_t *output, enum pipe
> pipe,
>  	      color_t *color, uint64_t tiling)
>  {
>  	drmModeModeInfo *mode;
> @@ -125,17 +125,6 @@ test_grab_crc(data_t *data, igt_output_t
> *output, enum pipe pipe,
>  	igt_pipe_crc_get_single(data->pipe_crc, &data->ref_crc);
>  }
>  
> -/*
> - * Multiple plane position test.
> - *   - We start by grabbing a reference CRC of a full blue fb being
> scanned
> - *     out on the primary plane
> - *   - Then we scannout number of planes:
> - *      * the primary plane uses a blue fb with a black rectangle
> hole
> - *      * planes, on top of the primary plane, with a blue fb that
> is set-up
> - *        to cover the black rectangles of the primary plane fb
> - *     The resulting CRC should be identical to the reference CRC
> - */
> -
>  static void
>  create_fb_for_mode_position(data_t *data, igt_output_t *output,
> drmModeModeInfo *mode,
>  			    color_t *color, int *rect_x, int *rect_y,
> @@ -281,6 +270,17 @@ prepare_planes(data_t *data, enum pipe pipe_id,
> color_t *color,
>  	free((void*)suffle);
>  }
>  
> +/*
> + * Multiple plane position test.
> + *   - We start by grabbing a reference CRC of a full blue fb being
> scanned
> + *     out on the primary plane
> + *   - Then we scannout number of planes:
> + *      * the primary plane uses a blue fb with a black rectangle
> holes
> + *      * planes, on top of the primary plane, with a blue fb that
> is set-up
> + *        to cover the black rectangles of the primary plane
> + *     The resulting CRC should be identical to the reference CRC
> + */
> +
>  static void
>  test_plane_position_with_output(data_t *data, enum pipe pipe,
>  				igt_output_t *output, int n_planes,
> @@ -306,11 +306,9 @@ test_plane_position_with_output(data_t *data,
> enum pipe pipe,
>  
>  	test_init(data, pipe, n_planes);
>  
> -	test_grab_crc(data, output, pipe, &blue, tiling);
> +	get_reference_crc(data, output, pipe, &blue, tiling);
>  
> -	/*
> -	 * Find out how many planes are allowed simultaneously
> -	 */
> +	/* Find out how many planes are allowed simultaneously */
>  	do {
>  		c++;
>  		prepare_planes(data, pipe, &blue, tiling, c, output);
> @@ -323,7 +321,7 @@ test_plane_position_with_output(data_t *data,
> enum pipe pipe,
>  			igt_remove_fb(data->drm_fd, &data->fb[x]);
>  	} while (!err && c < n_planes);
>  
> -	if(err)
> +	if (err)
>  		c--;
>  
>  	igt_info("Testing connector %s using pipe %s with %d planes %s
> with seed %d\n",
> @@ -332,6 +330,7 @@ test_plane_position_with_output(data_t *data,
> enum pipe pipe,
>  
>  	i = 0;
>  	while (i < iterations || loop_forever) {
> +		/* randomize planes and set up the holes */
>  		prepare_planes(data, pipe, &blue, tiling, c, output);
>  
>  		igt_display_commit2(&data->display, COMMIT_ATOMIC);
> @@ -441,6 +440,10 @@ igt_main_args("", long_options, help_str,
> opt_handler, NULL)
>  	}
>  
>  	for_each_pipe_static(pipe) {
> +		igt_describe("Check that the kernel handles atomic
> updates of "
> +			     "multiple planes correctly by changing
> their "
> +			     "geometry and making sure the changes are
> "
> +			     "reflected immediately after each
> commit.");
>  		igt_subtest_group
>  			run_tests_for_pipe(&data, pipe);
>  	}
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev