[igt-dev] [PATCH i-g-t 3/3] tests/kms_yuv: Add yuv specific tests, v3.

[Ville Syrjälä]

On Wed, Mar 27, 2019 at 10:29:30AM +0100, Maarten Lankhorst wrote:
> Add tests excercising switching between various scaled/unscaled
> transitions to and from various nv12 formats, since some of the
> workarounds mention this may fail.
> 
> We also add NV12 specific clipping/scaling tests, to make sure
> that YUV src coordinates are always programmed as a multiple of 2
> correctly, and verify scaling/clipping works with CRC tests.
> 
> crop-scale-bug shows a corruption when scaling and cropping at the
> same time.
> 
> The subpixel clip test rotates and clips a rectangle on each side,
> which should produce the same CRC as when that part of the rectangle
> is hidden by a cursor and a whole 2x2 pixel is hidden.
> 
> Changes since v1:
> - Reset plane position to 0,0 at the start of yuv_valid_width_plane().
> - Handle Swati's feedback.
> Changes since v2:
> - Skip 90°/270° rotation in tests when formats don't support it.
> - Add all new HDR formats to test.
> - Use igt_plane_has_format_mod.
> - Support tests on !i915 by checking if X/Y tiling modifiers are supported.
> - Do not enable untested planes to prevent exhausing memory bandwidth
>   and available NV12 Y planes in the rgb-switch-scaled test.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
>  tests/Makefile.sources |   1 +
>  tests/kms_yuv.c        | 843 +++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build      |   1 +
>  3 files changed, 845 insertions(+)
>  create mode 100644 tests/kms_yuv.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 71ccf00af256..5eb46087e362 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -74,6 +74,7 @@ TESTS_progs = \
>  	kms_universal_plane \
>  	kms_vblank \
>  	kms_vrr \
> +	kms_yuv \
>  	meta_test \
>  	perf \
>  	perf_pmu \
> diff --git a/tests/kms_yuv.c b/tests/kms_yuv.c
> new file mode 100644
> index 000000000000..ce9e4ade334f
> --- /dev/null
> +++ b/tests/kms_yuv.c
> @@ -0,0 +1,843 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> + */
> +#include "config.h"
> +
> +#include "igt.h"
> +#include <cairo.h>
> +#include <errno.h>
> +#include <stdint.h>
> +#include <unistd.h>
> +#include <sys/time.h>
> +
> +typedef struct {
> +	igt_display_t display;
> +
> +	igt_pipe_crc_t *pipe_crc;
> +	struct igt_fb fb[6];
> +} data_t;
> +
> +static uint64_t x_modifier = LOCAL_DRM_FORMAT_MOD_NONE;
> +static uint64_t y_modifier = LOCAL_DRM_FORMAT_MOD_NONE;
> +
> +static bool pipe_supports_format(igt_display_t *display, enum pipe pipe, uint32_t format, uint64_t tiling)
> +{
> +	igt_plane_t *plane;
> +
> +	for_each_plane_on_pipe(display, pipe, plane)
> +		if (igt_plane_has_format_mod(plane, format, tiling))
> +			return true;
> +
> +	return false;
> +}
> +
> +static void remove_fbs(data_t *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(data->fb); i++)
> +		igt_remove_fb(data->display.drm_fd, &data->fb[i]);
> +}
> +
> +static void prepare_crtc(data_t *data, enum pipe pipe, igt_output_t *output)
> +{
> +	igt_display_t *display = &data->display;
> +
> +	remove_fbs(data);
> +	igt_display_reset(display);
> +	igt_output_set_pipe(output, pipe);
> +	igt_display_commit2(display, COMMIT_ATOMIC);
> +
> +	igt_pipe_crc_free(data->pipe_crc);
> +	data->pipe_crc = igt_pipe_crc_new(display->drm_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> +}
> +
> +static void set_fb(igt_plane_t *plane, struct igt_fb *fb, bool scaled)
> +{
> +	igt_plane_set_fb(plane, fb);
> +
> +	if (scaled && fb)
> +		igt_fb_set_size(fb, plane, fb->width, 16);
> +
> +	if (fb && fb->modifier == y_modifier) { // TODO

Overloading the modifier to also indicate rotation is a bit unexpected.

> +		igt_plane_set_rotation(plane, IGT_ROTATION_90);
> +		igt_plane_set_size(plane, fb->height, fb->width);
> +	} else
> +		igt_plane_set_rotation(plane, IGT_ROTATION_0);
> +}
> +
> +static void yuv_rgb_switch(data_t *data, enum pipe pipe, igt_output_t *output, bool scaled, unsigned format)
> +{
> +	drmModeModeInfo *mode = igt_output_get_mode(output);
> +	igt_display_t *display = &data->display;
> +	igt_plane_t *plane;
> +	int i;
> +	igt_crc_t ref_crc[4], crc;
> +	bool valid[4] = {};
> +
> +	prepare_crtc(data, pipe, output);
> +
> +	igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
> +			      format, x_modifier, &data->fb[0]);
> +
> +	igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay,
> +			      format, y_modifier, &data->fb[1]);
> +
> +	igt_create_pattern_fb(display->drm_fd, mode->hdisplay, mode->vdisplay,
> +			      DRM_FORMAT_XRGB8888, x_modifier, &data->fb[2]);
> +
> +	igt_create_pattern_fb(display->drm_fd, mode->vdisplay, mode->hdisplay,
> +			      DRM_FORMAT_XRGB8888, y_modifier, &data->fb[3]);
> +
> +	for_each_plane_on_pipe(display, pipe, plane) {
> +		const int seq[] = {
> +			2, 0, 2, 1, 3, 1, 3
> +		};
> +
> +		if (!igt_plane_has_format_mod(plane, format, data->fb[0].modifier))
> +			continue;
> +
> +		/* Collect reference crc with toggle in between. */
> +		for (i = 0; i < ARRAY_SIZE(ref_crc); i++) {
> +			set_fb(plane, &data->fb[i], scaled);
> +
> +			if (i != 1)
> +				igt_display_commit2(display, COMMIT_ATOMIC);
> +			else if (igt_display_try_commit2(display, COMMIT_ATOMIC) < 0)
> +				continue;

What's so magical about index 1?

As far as the rest goes it's a bit hard to decode what is being tested.
I'd appreciate some higher level comments about what each subtest is
actually trying to test.

-- 
Ville Syrjälä
Intel