[igt-dev] [PATCH i-g-t 3/3] tests: Add new PSR2 selective fetch test

Souza, Jose jose.souza at intel.com
Wed Nov 18 20:20:14 UTC 2020


On Wed, 2020-11-18 at 13:44 +0530, Pankaj Bharadiya wrote:
> Selective fetch reduces display engine use of memory bandwidth
> by only fetching (reading from memory) the updated regions of the frame
> buffer and sending those updated regions to a panel with a PSR2 capability.
> 
> The FB_DAMAGE_CLIPS plane property provides user-space a way inform
> kernel about the updated regions.
> 
> Add new test to verify selective fetch by using FB_DAMAGE_CLIPS property
> to send  updated regions.
> 
> Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya at intel.com>
> ---
>  tests/Makefile.sources |   1 +
>  tests/kms_psr2_sf.c    | 717 +++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build      |   1 +
>  3 files changed, 719 insertions(+)
>  create mode 100644 tests/kms_psr2_sf.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 96d835820..4e653c8b2 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -79,6 +79,7 @@ TESTS_progs = \
>  	kms_properties \
>  	kms_psr \
>  	kms_psr2_su \
> +	kms_psr2_sf \
>  	kms_pwrite_crc \
>  	kms_rmfb \
>  	kms_rotation_crc \
> diff --git a/tests/kms_psr2_sf.c b/tests/kms_psr2_sf.c
> new file mode 100644
> index 000000000..baa640851
> --- /dev/null
> +++ b/tests/kms_psr2_sf.c
> @@ -0,0 +1,717 @@
> +/*
> + * Copyright © 2020 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 (including the next
> + * paragraph) 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.
> + *
> + */
> +
> +#include "igt.h"
> +#include "igt_sysfs.h"
> +#include "igt_psr.h"
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include "intel_bufmgr.h"
> +
> +IGT_TEST_DESCRIPTION("Test PSR2 selective fetch");
> +
> +#define SQUARE_SIZE 100
> +
> +#define CUR_SIZE 64
> +
> +enum operations {
> +	PLANE_UPDATE,
> +	PLANE_MOVE,
> +	OVERLAY_PRIM_UPDATE
> +};
> +
> +enum plane_move_postion {
> +	POS_TOP_LEFT,
> +	POS_TOP_RIGHT,
> +	POS_BOTTOM_LEFT,
> +	POS_BOTTOM_RIGHT
> +};
> +
> +typedef struct {
> +	int drm_fd;
> +	int debugfs_fd;
> +	igt_display_t display;
> +	drm_intel_bufmgr *bufmgr;
> +	drmModeModeInfo *mode;
> +	igt_output_t *output;
> +	struct igt_fb fb[3];
> +	int damage_area_count;
> +	enum operations op;
> +	enum plane_move_postion pos;
> +	int test_plane_id;
> +	igt_plane_t *test_plane;
> +	cairo_t *cr;
> +} data_t;
> +
> +static const char *op_str(enum operations op)
> +{
> +	static const char * const name[] = {
> +		[PLANE_UPDATE] = "plane-update",
> +		[PLANE_MOVE] = "plane-move",
> +		[OVERLAY_PRIM_UPDATE] = "overlay-primary-update",
> +	};
> +
> +	return name[op];
> +}
> +
> +static void setup_output(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +
> +	for_each_pipe_with_valid_output(display, pipe, output) {
> +		drmModeConnectorPtr c = output->config.connector;
> +
> +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> +			continue;
> +
> +		igt_output_set_pipe(output, pipe);
> +		data->output = output;
> +		data->mode = igt_output_get_mode(output);
> +
> +		return;
> +	}
> +}
> +
> +static void display_init(data_t *data)
> +{
> +	igt_display_require(&data->display, data->drm_fd);
> +	setup_output(data);
> +}
> +
> +static void display_fini(data_t *data)
> +{
> +	igt_display_fini(&data->display);
> +}
> +
> +static void draw_rect(data_t *data, igt_fb_t *fb, int x, int y, int w, int h,
> +			double r, double g, double b, double a)
> +{
> +	cairo_t *cr;
> +
> +	cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +	igt_paint_color_alpha(cr, x, y, w, h, r, g, b, a);
> +	igt_put_cairo_ctx(cr);
> +}
> +
> +static void plane_update_paint_squares(data_t *data, igt_fb_t *fb, uint32_t h,
> +				       uint32_t v)
> +{
> +
> +	switch (data->damage_area_count) {
> +	case 5:
> +		/*Bottom right corner*/
> +		draw_rect(data, fb, h - SQUARE_SIZE, v - SQUARE_SIZE,
> +			  SQUARE_SIZE, SQUARE_SIZE, 1.0, 1.0, 1.0, 1.0);
> +	case 4:
> +		/*Bottom left corner*/
> +		draw_rect(data, fb, h - SQUARE_SIZE, 0,
> +			  SQUARE_SIZE, SQUARE_SIZE,
> +			  1.0, 1.0, 1.0, 1.0);
> +	case 3:
> +		/*Top right corner*/
> +		draw_rect(data, fb, 0, v - SQUARE_SIZE,
> +			  SQUARE_SIZE, SQUARE_SIZE, 1.0, 1.0, 1.0, 1.0);
> +	case 2:
> +		/*Top left corner*/
> +		draw_rect(data, fb, 0, 0, SQUARE_SIZE, SQUARE_SIZE,
> +			  1.0, 1.0, 1.0, 1.0);
> +
> +	case 1:
> +		/*Center*/
> +		draw_rect(data, fb, h/2 - SQUARE_SIZE/2, v/2 - SQUARE_SIZE/2,
> +			  SQUARE_SIZE, SQUARE_SIZE, 1.0, 1.0, 1.0, 1.0);
> +	}

Want to see a damage clip being set every time you draw in the fb, together calls. Even better if you set int x, y, w and h variables and reuse to it
to call draw_rect() and set clips(clip[x].x2 = clip[x].x1 + w);

Also you are only setting the damage area of the places you are drawing to, you need also to mark the regions of the previous fb that was draw and in
the new fb have the background color, example:

FB0
|------|
|O     |
|      |
--------

FB1
|------|
|      |
|      |
--------

When flipping to FB1 you need to mark as damaged the region that had a 0 but now is only background.

> +}
> +
> +static void plane_move_paint_square(data_t *data, igt_fb_t *fb, uint32_t h,
> +				       uint32_t v)
> +{
> +
> +	switch (data->pos) {
> +	case POS_TOP_LEFT:
> +		/*Bottom right corner*/
> +		draw_rect(data, fb, h - SQUARE_SIZE, v - SQUARE_SIZE,
> +			  SQUARE_SIZE, SQUARE_SIZE, 1.0, 1.0, 1.0, 1.0);
> +		break;
> +	case POS_TOP_RIGHT:
> +		/*Bottom left corner*/
> +		draw_rect(data, fb, 0, v - SQUARE_SIZE,
> +			  SQUARE_SIZE, SQUARE_SIZE, 1.0, 1.0, 1.0, 1.0);
> +		break;
> +	case POS_BOTTOM_LEFT:
> +		/*Top right corner*/
> +		draw_rect(data, fb, h - SQUARE_SIZE, 0,
> +			  SQUARE_SIZE, SQUARE_SIZE, 1.0, 1.0, 1.0, 1.0);
> +		break;
> +	case POS_BOTTOM_RIGHT:
> +		/*Top left corner*/
> +		draw_rect(data, fb, 0, 0, SQUARE_SIZE, SQUARE_SIZE,
> +			  1.0, 1.0, 1.0, 1.0);
> +		break;
> +	}
> +}
> +
> +static struct drm_mode_rect *plane_update_setup_damage_areas(data_t *data,
> +							     uint32_t h,
> +							     uint32_t v)
> +{
> +	struct drm_mode_rect *clip;
> +
> +	if (data->test_plane_id == DRM_PLANE_TYPE_CURSOR) {
> +		clip = (struct drm_mode_rect *) malloc(sizeof(struct
> +							      drm_mode_rect));
> +		igt_assert(clip);
> +
> +		clip->x1 = clip->y1 = 0;
> +		clip->x2 = clip->y2 = CUR_SIZE;
> +
> +	} else {
> +		clip = (struct drm_mode_rect *) malloc(sizeof(struct
> +							      drm_mode_rect) *
> +						       data->damage_area_count);

alloc enough clips for all tests in stack and reuse in every update.


> +		igt_assert(clip);
> +
> +		switch (data->damage_area_count) {
> +		case 5:
> +			/*Bottom right corner*/
> +			clip[4].x1 = h - SQUARE_SIZE;
> +			clip[4].y1 = v - SQUARE_SIZE;
> +			clip[4].x2 = clip[4].x1 + SQUARE_SIZE;
> +			clip[4].y2 = clip[4].y1 + SQUARE_SIZE;
> +		case 4:
> +			/*Bottom left corner*/
> +			clip[3].x1 = h - SQUARE_SIZE;
> +			clip[3].y1 = 0;
> +			clip[3].x2 = clip[3].x1 + SQUARE_SIZE;
> +			clip[3].y2 = clip[3].y1 + SQUARE_SIZE;
> +		case 3:
> +			/*Top right corner*/
> +			clip[2].x1 = 0;
> +			clip[2].y1 = v - SQUARE_SIZE;
> +			clip[2].x2 = clip[2].x1 + SQUARE_SIZE;
> +			clip[2].y2 = clip[2].y1 + SQUARE_SIZE;
> +		case 2:
> +			/*Top left corner*/
> +			clip[1].x1 = 0;
> +			clip[1].y1 = 0;
> +			clip[1].x2 = clip[1].x1 + SQUARE_SIZE;
> +			clip[1].y2 = clip[1].y1 + SQUARE_SIZE;
> +
> +		case 1:
> +			/*Center*/
> +			clip[0].x1 = h/2 - SQUARE_SIZE/2;
> +			clip[0].y1 = v/2 - SQUARE_SIZE/2;
> +			clip[0].x2 = clip[0].x1 + SQUARE_SIZE;
> +			clip[0].y2 = clip[0].y1 + SQUARE_SIZE;
> +		}
> +	}
> +
> +	return clip;
> +}
> +
> +static struct drm_mode_rect *plane_move_setup_damage_area(data_t *data,
> +							  uint32_t h,
> +							  uint32_t v)
> +{
> +	struct drm_mode_rect *clip;
> +
> +	clip = (struct drm_mode_rect *) malloc(sizeof(struct drm_mode_rect));
> +	igt_assert(clip);
> +
> +	switch (data->pos) {
> +	case POS_TOP_LEFT:
> +		/*Bottom right corner*/
> +		clip->x1 = h - SQUARE_SIZE;
> +		clip->y1 = v - SQUARE_SIZE;
> +		clip->x2 = clip->x1 + SQUARE_SIZE;
> +		clip->y2 = clip->y1 + SQUARE_SIZE;
> +
> +		break;
> +	case POS_TOP_RIGHT:
> +		/*Bottom left corner*/
> +		clip->x1 = h - SQUARE_SIZE;
> +		clip->y1 = 0;
> +		clip->x2 = clip->x1 + SQUARE_SIZE;
> +		clip->y2 = clip->y1 + SQUARE_SIZE;
> +
> +		break;
> +	case POS_BOTTOM_LEFT:
> +		/*Top right corner*/
> +		clip->x1 = 0;
> +		clip->y1 = v - SQUARE_SIZE;
> +		clip->x2 = clip->x1 + SQUARE_SIZE;
> +		clip->y2 = clip->y1 + SQUARE_SIZE;
> +
> +		break;
> +	case POS_BOTTOM_RIGHT:
> +		/*Top left corner*/
> +		clip->x1 = 0;
> +		clip->y1 = 0;
> +		clip->x2 = clip->x1 + SQUARE_SIZE;
> +		clip->y2 = clip->y1 + SQUARE_SIZE;
> +	}
> +
> +	return clip;
> +}
> +
> +static void free_damage_areas(struct drm_mode_rect *clip)
> +{
> +	if (!clip)
> +		return;
> +
> +	free(clip);
> +}
> +
> +static void prepare(data_t *data)
> +{
> +	igt_plane_t *primary, *sprite = NULL, *cursor = NULL;
> +
> +	/* all green frame */
> +	igt_create_color_fb(data->drm_fd,
> +			    data->mode->hdisplay, data->mode->vdisplay,
> +			    DRM_FORMAT_XRGB8888,
> +			    LOCAL_DRM_FORMAT_MOD_NONE,
> +			    0.0, 1.0, 0.0,
> +			    &data->fb[0]);
> +
> +	primary = igt_output_get_plane_type(data->output,
> +			DRM_PLANE_TYPE_PRIMARY);
> +
> +	switch (data->test_plane_id) {
> +	case DRM_PLANE_TYPE_OVERLAY:
> +		sprite = igt_output_get_plane_type(data->output,
> +						   DRM_PLANE_TYPE_OVERLAY);
> +		/*All blue plane*/
> +		igt_create_color_fb(data->drm_fd,
> +				    data->mode->hdisplay/2,
> +				    data->mode->vdisplay/2,
> +				    DRM_FORMAT_XRGB8888,
> +				    LOCAL_DRM_FORMAT_MOD_NONE,
> +				    0.0, 0.0, 1.0,
> +				    &data->fb[1]);

Kinda of confusing data->fb, add data->fb_primary[], data->fb_overlay[] and data->fb_cursor[].

> +
> +		igt_create_color_fb(data->drm_fd,
> +				    data->mode->hdisplay/2,
> +				    data->mode->vdisplay/2,
> +				    DRM_FORMAT_XRGB8888,
> +				    LOCAL_DRM_FORMAT_MOD_NONE,
> +				    0.0, 0.0, 1.0,
> +				    &data->fb[2]);
> +
> +		if (data->op == PLANE_MOVE) {
> +			plane_move_paint_square(data, &data->fb[2],
> +					   data->mode->hdisplay/2,
> +					   data->mode->vdisplay/2);
> +
> +		} else {
> +			plane_update_paint_squares(data, &data->fb[2],
> +					   data->mode->hdisplay/2,
> +					   data->mode->vdisplay/2);
> +		}
> +
> +		igt_plane_set_fb(sprite, &data->fb[1]);
> +		data->test_plane = sprite;
> +		break;
> +
> +	case DRM_PLANE_TYPE_PRIMARY:
> +		igt_create_color_fb(data->drm_fd,
> +			    data->mode->hdisplay, data->mode->vdisplay,
> +			    DRM_FORMAT_XRGB8888,
> +			    LOCAL_DRM_FORMAT_MOD_NONE,
> +			    0.0, 1.0, 0.0,
> +			    &data->fb[2]);
> +
> +		plane_update_paint_squares(data, &data->fb[2],
> +					   data->mode->hdisplay,
> +					   data->mode->vdisplay);
> +		data->test_plane = primary;
> +
> +		if (data->op == OVERLAY_PRIM_UPDATE) {
> +			sprite = igt_output_get_plane_type(data->output,
> +						   DRM_PLANE_TYPE_OVERLAY);
> +
> +			igt_create_color_fb(data->drm_fd,
> +					    data->mode->hdisplay,
> +					    data->mode->vdisplay,
> +					    DRM_FORMAT_XRGB8888,
> +					    LOCAL_DRM_FORMAT_MOD_NONE,
> +					    0.0, 0.0, 1.0,
> +					    &data->fb[1]);
> +
> +			igt_plane_set_fb(sprite, &data->fb[1]);
> +			igt_plane_set_prop_value(sprite, IGT_PLANE_ALPHA,
> +						 0x6060);
> +		}
> +		break;
> +
> +	case DRM_PLANE_TYPE_CURSOR:
> +		cursor = igt_output_get_plane_type(data->output,
> +						   DRM_PLANE_TYPE_CURSOR);
> +		igt_plane_set_position(cursor, 0, 0);
> +
> +		igt_create_fb(data->drm_fd, CUR_SIZE, CUR_SIZE,
> +			      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> +			      &data->fb[1]);
> +
> +		draw_rect(data, &data->fb[1], 0, 0, CUR_SIZE, CUR_SIZE,
> +			    0.0, 0.0, 1.0, 1.0);
> +
> +		igt_create_fb(data->drm_fd, CUR_SIZE, CUR_SIZE,
> +			      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> +			      &data->fb[2]);
> +
> +		draw_rect(data, &data->fb[2], 0, 0, CUR_SIZE, CUR_SIZE,
> +			    1.0, 1.0, 1.0, 1.0);
> +
> +		igt_plane_set_fb(cursor, &data->fb[1]);
> +		data->test_plane = cursor;
> +		break;
> +	}
> +
> +	igt_plane_set_fb(primary, &data->fb[0]);
> +
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +}
> +
> +static inline void manual(const char *expected)
> +{
> +	igt_debug_manual_check("all", expected);
> +}
> +
> +static void plane_update_expected_output(int plane_type, int box_count)
> +{
> +	char expected[64] = {};
> +
> +	switch (plane_type) {
> +	case DRM_PLANE_TYPE_PRIMARY:
> +		sprintf(expected, "screen Green with %d White box(es)",
> +			box_count);
> +		break;
> +	case DRM_PLANE_TYPE_OVERLAY:
> +		sprintf(expected,
> +			"screen Green with Blue box and %d White box(es)",
> +			box_count);
> +		break;
> +	case DRM_PLANE_TYPE_CURSOR:
> +		sprintf(expected, "screen Green with %d White box(es)",
> +			box_count);
> +		break;
> +	}
> +
> +	manual(expected);
> +}
> +
> +static void plane_move_expected_output(enum plane_move_postion pos)
> +{
> +	char expected[64] = {};
> +
> +	switch (pos) {
> +	case POS_TOP_LEFT:
> +		sprintf(expected,
> +			"screen Green with Blue box on top left corner and White box");
> +		break;
> +	case POS_TOP_RIGHT:
> +		sprintf(expected,
> +			"screen Green with Blue box on top right corner and White box");
> +		break;
> +	case POS_BOTTOM_LEFT:
> +		sprintf(expected,
> +			"screen Green with Blue box on bottom left corner and White box");
> +		break;
> +	case POS_BOTTOM_RIGHT:
> +		sprintf(expected,
> +			"screen Green with Blue box on bottom right corner and White box");
> +		break;
> +	}
> +
> +	manual(expected);
> +}
> +
> +static void overlay_prim_update_expected_output(int box_count)
> +{
> +	char expected[64] = {};
> +
> +	sprintf(expected,
> +		"screen Green with Blue overlay, %d light Blue box(es)",
> +		box_count);
> +
> +	manual(expected);
> +
> +}
> +
> +static void expected_output(data_t *data)
> +{
> +	switch (data->op) {
> +	case PLANE_MOVE:
> +		plane_move_expected_output(data->pos);
> +		break;
> +	case PLANE_UPDATE:
> +		plane_update_expected_output(data->test_plane_id,
> +					     data->damage_area_count);
> +		break;
> +	case OVERLAY_PRIM_UPDATE:
> +		overlay_prim_update_expected_output(data->damage_area_count);
> +		break;
> +	}
> +}
> +
> +static void damaged_plane_move(data_t *data)
> +{
> +	igt_plane_t *test_plane = data->test_plane;
> +	struct drm_mode_rect *clip;
> +	uint32_t h = data->mode->hdisplay;
> +	uint32_t v = data->mode->vdisplay;
> +
> +	igt_plane_set_fb(test_plane, &data->fb[2]);
> +
> +	if (data->test_plane_id == DRM_PLANE_TYPE_OVERLAY) {
> +		h = h/2;
> +		v = v/2;
> +	}
> +
> +	clip = plane_move_setup_damage_area(data, h, v);

This property is in relation to the plane, this damage areas you are setting in the function makes no sense.
When moving a plane or set the whole visible plane area as damaged or nothing(it should be one of the cases handled by driver).

> +	igt_plane_replace_prop_blob(test_plane, IGT_PLANE_FB_DAMAGE_CLIPS, clip,
> +				    sizeof(struct drm_mode_rect));
> +
> +	switch (data->pos) {
> +	case POS_TOP_LEFT:
> +		igt_plane_set_position(data->test_plane, 0, 0);
> +		break;
> +	case POS_TOP_RIGHT:
> +		igt_plane_set_position(data->test_plane,
> +				       data->mode->hdisplay/2, 0);
> +		break;
> +	case POS_BOTTOM_LEFT:
> +		igt_plane_set_position(data->test_plane, 0,
> +				       data->mode->vdisplay/2);
> +		break;
> +	case POS_BOTTOM_RIGHT:
> +		igt_plane_set_position(data->test_plane,
> +				       data->mode->hdisplay/2,
> +				       data->mode->vdisplay/2);
> +		break;
> +	}
> +
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> +	igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> +
> +	expected_output(data);
> +
> +	free_damage_areas(clip);
> +}
> +
> +static void damaged_plane_update(data_t *data)
> +{
> +	igt_plane_t *test_plane = data->test_plane;
> +	struct drm_mode_rect *clip;
> +	uint32_t h = data->mode->hdisplay;
> +	uint32_t v = data->mode->vdisplay;
> +
> +	igt_plane_set_fb(test_plane, &data->fb[2]);
> +
> +	if (data->test_plane_id == DRM_PLANE_TYPE_OVERLAY) {
> +		h = h/2;
> +		v = v/2;
> +	}
> +
> +	clip = plane_update_setup_damage_areas(data, h, v);
> +	igt_plane_replace_prop_blob(test_plane, IGT_PLANE_FB_DAMAGE_CLIPS, clip,
> +				    sizeof(struct drm_mode_rect)*
> +				    data->damage_area_count);
> +	igt_plane_set_position(data->test_plane, 0, 0);
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> +	igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> +
> +	expected_output(data);
> +
> +	free_damage_areas(clip);
> +}
> +
> +static void update_screen_and_test(data_t *data)
> +{
> +	switch (data->op) {
> +	case PLANE_UPDATE:
> +	case OVERLAY_PRIM_UPDATE:
> +		damaged_plane_update(data);
> +		break;
> +	case PLANE_MOVE:
> +		damaged_plane_move(data);
> +		break;
> +	}
> +}
> +
> +static void screen_reset(data_t  *data)
> +{
> +	igt_plane_t *test_plane = data->test_plane;
> +
> +	if (data->test_plane_id == DRM_PLANE_TYPE_PRIMARY)
> +		igt_plane_set_fb(test_plane, &data->fb[0]);
> +	else
> +		igt_plane_set_fb(test_plane, &data->fb[1]);
> +
> +	igt_plane_set_position(data->test_plane, 0, 0);
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +}
> +
> +static void run(data_t *data)
> +{
> +	igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> +
> +	screen_reset(data);

why you need this if you have committed already in the prepare()?

> +	update_screen_and_test(data);

Content of this function could be moved to here.


> +}
> +
> +static void cleanup(data_t *data)
> +{
> +	igt_plane_t *primary;
> +	igt_plane_t *sprite;
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +
> +	igt_plane_set_fb(primary, NULL);
> +
> +	if (data->test_plane_id != DRM_PLANE_TYPE_PRIMARY) {
> +		igt_plane_set_position(data->test_plane, 0, 0);
> +		igt_plane_set_fb(data->test_plane, NULL);
> +	}
> +
> +	if (data->op == OVERLAY_PRIM_UPDATE) {
> +		sprite = igt_output_get_plane_type(data->output,
> +				DRM_PLANE_TYPE_OVERLAY);
> +		igt_plane_set_position(sprite, 0, 0);
> +		igt_plane_set_fb(sprite, NULL);
> +	}
> +
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> +	igt_remove_fb(data->drm_fd, &data->fb[0]);
> +	igt_remove_fb(data->drm_fd, &data->fb[1]);
> +	igt_remove_fb(data->drm_fd, &data->fb[2]);
> +}
> +
> +igt_main
> +{
> +	data_t data = {};
> +	int i;
> +
> +	igt_fixture {
> +		int r;
> +
> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> +		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> +		kmstest_set_vt_graphics_mode();
> +
> +		igt_require_f(psr_sink_support(data.drm_fd,
> +					       data.debugfs_fd, PSR_MODE_2),
> +			      "Sink does not support PSR2\n");
> +
> +		igt_require_f(psr2_selective_fetch_check(data.debugfs_fd),
> +			      "PSR2 selective fetch not enabled\n");
> +
> +		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
> +		igt_assert(data.bufmgr);
> +		drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
> +
> +		display_init(&data);
> +
> +		/* Test if PSR2 can be enabled */
> +		igt_require_f(psr_enable(data.drm_fd,
> +					 data.debugfs_fd, PSR_MODE_2),
> +			      "Error enabling PSR2\n");
> +
> +		data.damage_area_count = 5;
> +		data.op = PLANE_UPDATE;
> +		data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> +		prepare(&data);
> +		r = psr_wait_entry(data.debugfs_fd, PSR_MODE_2);
> +		cleanup(&data);
> +		if (!r)
> +			psr_print_debugfs(data.debugfs_fd);
> +		igt_require_f(r, "PSR2 can not be enabled\n");
> +	}
> +

Missing some description of what each subtest will, also a better name.
What we will see in CI would be: primary-plane-update-sf-5, 5 what?

> +	for (i = 1; i <= 5; i++) {
> +		igt_subtest_f("primary-%s-sf-%d", op_str(data.op), i) {
> +			data.damage_area_count = i;
> +			data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> +			prepare(&data);
> +			run(&data);
> +			cleanup(&data);
> +		}
> +	}
> +
> +	for (i = 1; i <= 5; i++) {
> +		igt_subtest_f("overlay-%s-sf-%d", op_str(data.op), i) {
> +			data.damage_area_count = i;
> +			data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
> +			prepare(&data);
> +			run(&data);
> +			cleanup(&data);
> +		}
> +	}
> +
> +	igt_subtest_f("cursor-%s-sf", op_str(data.op)) {
> +		data.damage_area_count = 1;
> +		data.test_plane_id = DRM_PLANE_TYPE_CURSOR;
> +		prepare(&data);
> +		run(&data);
> +		cleanup(&data);
> +	}
> +
> +	/* Only for overlay plane */
> +	data.op = PLANE_MOVE;
> +	for (i = POS_TOP_LEFT; i <= POS_BOTTOM_RIGHT ; i++) {
> +		igt_subtest_f("%s-sf-%d", op_str(data.op), i) {
> +			data.pos = i;
> +			data.test_plane_id = DRM_PLANE_TYPE_OVERLAY;
> +			prepare(&data);
> +			run(&data);
> +			cleanup(&data);
> +		}
> +	}
> +
> +	data.op = OVERLAY_PRIM_UPDATE;
> +	for (i = 1; i <= 5; i++) {
> +		igt_subtest_f("%s-sf-%d", op_str(data.op), i) {
> +			data.damage_area_count = i;
> +			data.test_plane_id = DRM_PLANE_TYPE_PRIMARY;
> +			prepare(&data);
> +			run(&data);
> +			cleanup(&data);
> +		}
> +	}
> +
> +	igt_fixture {
> +		close(data.debugfs_fd);
> +		drm_intel_bufmgr_destroy(data.bufmgr);
> +		display_fini(&data);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index cc79ac228..b33cfb63f 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -63,6 +63,7 @@ test_progs = [
>  	'kms_properties',
>  	'kms_psr',
>  	'kms_psr2_su',
> +	'kms_psr2_sf',
>  	'kms_pwrite_crc',
>  	'kms_rmfb',
>  	'kms_rotation_crc',



More information about the igt-dev mailing list