[igt-dev] [PATCH i-g-t v5 3/4] test: Add PSR2 selective update tests

Souza, Jose jose.souza at intel.com
Wed Jan 30 01:34:06 UTC 2019


On Tue, 2019-01-29 at 15:52 -0800, Pandiyan, Dhinakaran wrote:
> On Fri, 2019-01-25 at 17:32 -0800, José Roberto de Souza wrote:
> > This tests checks if hardware is able to do selective update when
> > screen changes.
> > PSR2 don't trigger interruptions and the 'PSR2 SU status' register
> > is not kept loaded all the times, so it is necessary keep polling
> > PSR status debugfs until those values are loaded.
> > 
> > Also from DEEP_SLEEP state HW will not do a seletive update, as
> > most of the memory/context is lost in deep sleep state hardware
> > will
> > need to exit PSR mode then wait a configured number of frames to
> > activate PSR again to then start doing seletive updates, that is
> > why
> > just one screen change is not enough to pass this tests.
> > 
> > When a selective update happens and the values are loaded and read
> > from debugfs it is compared with the expected value of seletive
> > update blocks, if matches the polling is stopped and the test
> > passed
> > otherwise it will wait until it reachs a maximum number o screen
> > changes to fail the test.
> > 
> > v2: Using new SU blocks debugfs output
> > 
> > v3:
> > - removed the timerfd to fail the test, now failing based in a
> > maximum number of screen changes
> > - removing thread to read debugfs, read from main thread is enough
> > - improved commit message
> > 
> > v4:
> > - getting cairo context for frontbuffer test in prepare()
> > - droppoing poll(), using blocking timerfd instead
> > 
> > v5:
> > - Doing a modeset before trying to enable PSR2
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > ---
> >  lib/igt_psr.c          |  29 ++++
> >  lib/igt_psr.h          |   1 +
> >  tests/Makefile.sources |   1 +
> >  tests/kms_psr2_su.c    | 298
> > +++++++++++++++++++++++++++++++++++++++++
> >  tests/meson.build      |   1 +
> >  5 files changed, 330 insertions(+)
> >  create mode 100644 tests/kms_psr2_su.c
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > index 591b2013..015cb4a4 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -208,3 +208,32 @@ bool psr_disabled_reason_get(int debugfs_fd,
> > char *reason, int len)
> >  
> >  	return true;
> >  }
> > +
> > +#define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
> > +
> > +static bool
> > +psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t
> > *num_su_blocks)
> > +{
> > +	char buf[PSR_STATUS_MAX_LEN];
> > +	char *str;
> > +	int ret;
> > +
> > +	ret = igt_debugfs_simple_read(debugfs_fd,
> > "i915_edp_psr_status", buf,
> > +				      sizeof(buf));
> > +	if (ret < 0)
> > +		return false;
> > +
> > +	str = strstr(buf, PSR2_SU_BLOCK_STR_LOOKUP);
> > +	if (!str)
> > +		return false;
> > +
> > +	str = &str[strlen(PSR2_SU_BLOCK_STR_LOOKUP)];
> > +	*num_su_blocks = (uint16_t)strtol(str, NULL, 10);
> > +
> > +	return true;
> > +}
> > +
> > +bool psr2_wait_su(int debugfs_fd, uint16_t *num_su_blocks)
> > +{
> > +	return igt_wait(psr2_read_last_num_su_blocks_val(debugfs_fd,
> > num_su_blocks), 40, 1);
> > +}
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> > index 465839d2..b1e84e39 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -41,5 +41,6 @@ bool psr_enable(int debugfs_fd, enum psr_mode);
> >  bool psr_disable(int debugfs_fd);
> >  bool psr_sink_support(int debugfs_fd, enum psr_mode);
> >  bool psr_disabled_reason_get(int debugfs_fd, char *reason, int
> > len);
> > +bool psr2_wait_su(int debugfs_fd, uint16_t *num_su_blocks);
> >  
> >  #endif
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index 519eac79..9174aecc 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -80,6 +80,7 @@ TESTS_progs = \
> >  	kms_plane_scaling \
> >  	kms_properties \
> >  	kms_psr \
> > +	kms_psr2_su \
> >  	kms_pwrite_crc \
> >  	kms_rmfb \
> >  	kms_rotation_crc \
> > diff --git a/tests/kms_psr2_su.c b/tests/kms_psr2_su.c
> > new file mode 100644
> > index 00000000..b0abb048
> > --- /dev/null
> > +++ b/tests/kms_psr2_su.c
> > @@ -0,0 +1,298 @@
> > +/*
> > + * 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
> > (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 <sys/timerfd.h>
> > +#include "intel_bufmgr.h"
> > +
> > +IGT_TEST_DESCRIPTION("Test PSR2 selective update");
> > +
> > +#define SQUARE_SIZE 100
> > +/* each selective update block is 4 lines tall */
> > +#define EXPECTED_NUM_SU_BLOCKS ((SQUARE_SIZE / 4) + (SQUARE_SIZE %
> > 4
> > ? 1 : 0))
> > +
> > +/*
> > + * Minimum is 15 as the number of frames to active PSR2 could be
> > configured
> > + * to 15 frames plus a few more in case we miss a selective update
> > between
> > + * debugfs reads.
> > + */
> > +#define MAX_SCREEN_CHANGES 20
> > +
> > +enum operations {
> > +	PAGE_FLIP,
> > +	FRONTBUFFER,
> > +	LAST
> > +};
> > +
> > +static const char *op_str(enum operations op)
> > +{
> > +	static const char * const name[] = {
> > +		[PAGE_FLIP] = "page_flip",
> > +		[FRONTBUFFER] = "frontbuffer"
> > +	};
> > +
> > +	return name[op];
> > +}
> > +
> > +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[2];
> > +	enum operations op;
> > +	cairo_t *cr;
> > +	int change_screen_timerfd;
> > +	uint32_t screen_changes;
> > +} data_t;
> > +
> > +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 prepare(data_t *data)
> > +{
> > +	igt_plane_t *primary;
> > +
> > +	/* 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]);
> > +
> > +	if (data->op == PAGE_FLIP) {
> > +		cairo_t *cr;
> > +
> > +		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[1]);
> > +
> > +		cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[1]);
> > +		/* paint a white square */
> > +		igt_paint_color_alpha(cr, 0, 0, SQUARE_SIZE,
> > SQUARE_SIZE,
> > +				      1.0, 1.0, 1.0, 1.0);
> > +		igt_put_cairo_ctx(data->drm_fd,  &data->fb[1], cr);
> > +	} else if (data->op == FRONTBUFFER) {
> > +		data->cr = igt_get_cairo_ctx(data->drm_fd, &data-
> > > fb[0]);
> > +	}
> > +
> > +	primary = igt_output_get_plane_type(data->output,
> > +					    DRM_PLANE_TYPE_PRIMARY);
> > +	igt_plane_set_fb(primary, NULL);
> > +
> > +	igt_display_commit(&data->display);
> > +	igt_plane_set_fb(primary, &data->fb[0]);
> > +	igt_display_commit(&data->display);
> > +
> > +	igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
> > +
> > +	data->screen_changes = 0;
> > +}
> > +
> > +static bool update_screen_and_test(data_t *data)
> > +{
> > +	uint16_t su_blocks;
> > +	bool ret = false;
> > +
> > +	data->screen_changes++;
> > +
> > +	switch (data->op) {
> > +	case PAGE_FLIP: {
> > +		igt_plane_t *primary;
> > +
> > +		primary = igt_output_get_plane_type(data->output,
> > +						    DRM_PLANE_TYPE_PRIM
> > ARY);
> > +
> > +		igt_plane_set_fb(primary, &data->fb[data-
> > > screen_changes & 1]);
> > +		igt_display_commit(&data->display);
> > +		break;
> > +	}
> > +	case FRONTBUFFER: {
> > +		drmModeClip clip;
> > +		int r;
> > +
> > +		clip.x1 = clip.y1 = 0;
> > +		clip.x2 = clip.y2 = SQUARE_SIZE;
> > +
> > +		if (data->screen_changes & 1) {
> > +			/* go back to all green frame with with square
> > */
> > +			igt_paint_color_alpha(data->cr, 0, 0,
> > SQUARE_SIZE,
> > +					      SQUARE_SIZE, 1.0, 1.0,
> > 1.0, 1.0);
> > +		} else {
> > +			/* go back to all green frame */
> > +			igt_paint_color_alpha(data->cr, 0, 0,
> > SQUARE_SIZE,
> > +					      SQUARE_SIZE, 0, 1.0, 0,
> > 1.0);
> > +		}
> > +
> > +		r = drmModeDirtyFB(data->drm_fd, data->fb[0].fb_id,
> > &clip, 1);
> > +		igt_assert(r == 0 || r == -ENOSYS);
> > +		break;
> > +	}
> > +	default:
> > +		igt_assert_f(data->op, "Operation not handled\n");
> > +	}
> > +
> > +	if (psr2_wait_su(data->debugfs_fd, &su_blocks))
> > +		ret = su_blocks == EXPECTED_NUM_SU_BLOCKS;
> > +
> > +	return ret;
> > +}
> > +
> > +static void run(data_t *data)
> > +{
> > +	bool result = false;
> > +
> > +	while (data->screen_changes < MAX_SCREEN_CHANGES && !result) {
> > +		uint64_t exp;
> > +		int r;
> > +
> > +		r = read(data->change_screen_timerfd, &exp,
> > sizeof(exp));
> > +		if (r == sizeof(uint64_t) && exp)
> > +			result = update_screen_and_test(data);
> > +	}
> > +
> > +	igt_debug("Screen changes: %u\n", data->screen_changes);
> > +	igt_assert_f(result,
> > +		     "No matching selective update blocks read from
> > debugfs\n");
> > +}
> > +
> > +static void cleanup(data_t *data)
> > +{
> > +	igt_plane_t *primary;
> > +
> > +	primary = igt_output_get_plane_type(data->output,
> > +					    DRM_PLANE_TYPE_PRIMARY);
> > +	igt_plane_set_fb(primary, NULL);
> > +	igt_display_commit(&data->display);
> > +
> > +	if (data->op == PAGE_FLIP)
> > +		igt_remove_fb(data->drm_fd, &data->fb[1]);
> > +	else if (data->op == FRONTBUFFER)
> > +		igt_put_cairo_ctx(data->drm_fd, &data->fb[0], data-
> > > cr);
> > +
> > +	igt_remove_fb(data->drm_fd, &data->fb[0]);
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	data_t data = {};
> > +
> > +	igt_subtest_init_parse_opts(&argc, argv, "", NULL,
> > +				    NULL, NULL, NULL);
> > +	igt_skip_on_simulation();
> > +
> > +	igt_fixture {
> > +		struct itimerspec interval;
> > +		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.debugfs_fd,
> > PSR_MODE_2),
> > +			      "Sink does not support PSR2\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.debugfs_fd, PSR_MODE_2),
> > +			      "Sink do not support PSR2\n");
> The comment and error message look wrong, a false from psr_enable()
> indicates debugfs does not support PSR 2 mode change.

You are right, changing to: Error enabling PSR2
What do you think?


> 
> > +		data.op = FRONTBUFFER;
> > +		prepare(&data);
> > +		r = psr_wait_entry(data.debugfs_fd, PSR_MODE_2);
> > +		cleanup(&data);
> > +		igt_require_f(r, "PSR2 can not be enabled\n");
> CI results shows the assertion in prepare() failed. The assertion
> should have caused a skip on whl, no?

Huum it failed in the prepare() inside of igt_fixture(), one the last
things that prepare() does is igt_assert(psr_wait_entry(data-
>debugfs_fd, PSR_MODE_2)).
So I'm moving it to the begining of run(), it should fix the issue. 


> 
> > +
> > +		/* blocking timerfd */
> > +		data.change_screen_timerfd =
> > timerfd_create(CLOCK_MONOTONIC, 0);
> > +		igt_require(data.change_screen_timerfd != -1);
> > +		/* Changing screen at 30hz to support 30hz panels */
> > +		interval.it_value.tv_nsec = NSEC_PER_SEC / 30;
> > +		interval.it_value.tv_sec = 0;
> > +		interval.it_interval.tv_nsec =
> > interval.it_value.tv_nsec;
> > +		interval.it_interval.tv_sec = interval.it_value.tv_sec;
> > +		r = timerfd_settime(data.change_screen_timerfd, 0,
> > &interval, NULL);
> > +		igt_require_f(r != -1, "Error setting timerfd\n");
> > +	}
> > +
> > +	for (data.op = PAGE_FLIP; data.op < LAST; data.op++) {
> > +		igt_subtest_f("%s", op_str(data.op)) {
> > +			prepare(&data);
> > +			run(&data);
> > +			cleanup(&data);
> > +		}
> > +	}
> > +
> > +	igt_fixture {
> > +		close(data.debugfs_fd);
> > +		drm_intel_bufmgr_destroy(data.bufmgr);
> > +		display_fini(&data);
> > +	}
> > +
> > +	igt_exit();
> > +}
> > diff --git a/tests/meson.build b/tests/meson.build
> > index e14ab2b4..682ca939 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -50,6 +50,7 @@ test_progs = [
> >  	'kms_plane_scaling',
> >  	'kms_properties',
> >  	'kms_psr',
> > +	'kms_psr2_su',
> >  	'kms_pwrite_crc',
> >  	'kms_rmfb',
> >  	'kms_rotation_crc',
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20190130/698771e9/attachment.sig>


More information about the igt-dev mailing list