[igt-dev] [PATCH i-g-t v2 9/9] test: Add PSR2 selective update tests

Souza, Jose jose.souza at intel.com
Mon Jan 14 22:48:56 UTC 2019


On Fri, 2019-01-11 at 15:15 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-01-03 at 06:36 -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.
> Does this mean flipping once while polling SU status registers is not
> sufficient?

After a flip in DEEP_SLEEP it will do a PSR exit and PSR active
sequence, it will only do a selective update while in SLEEP state that
is why more than one flip is necessary and it will vary depending of
the values that is set to frames to activate PSR and frames to enter
deep sleep.

> 
> 
> > When loaded it is compared with the expected value, if matches
> > the polling is stopped and the test passed otherwise it will
> > wait until fail_timerfd to expire to fail the test.
> This patch warrants some explanation about the timeouts used and how
> they were selected.

The flip timeout is a value to achieve 15hz flips and the error timeout
will be removed as you suggested bellow, I will add a comment when
setting the interval to the flip timerfd.

> 
> > v2: Using new SU blocks debugfs output
> > 
> > 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          |  23 +++
> >  lib/igt_psr.h          |   1 +
> >  tests/Makefile.sources |   1 +
> >  tests/kms_psr2_su.c    | 362
> > +++++++++++++++++++++++++++++++++++++++++
> >  tests/meson.build      |   1 +
> >  5 files changed, 388 insertions(+)
> >  create mode 100644 tests/kms_psr2_su.c
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > index 0f4b2e23..a8b9e4ce 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -234,3 +234,26 @@ bool psr2_wait_update(int debugfs_fd)
> >  {
> >  	return igt_wait(psr2_status_update(debugfs_fd), 40, 10);
> >  }
> > +
> > +#define PSR2_SU_BLOCK_STR_LOOKUP "PSR2 SU blocks:\n0\t"
> > +
> > +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;
> > +}
> > diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> > index 2b8d1a90..349eb0c2 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -41,5 +41,6 @@ bool psr2_wait_update(int debugfs_fd);
> >  bool psr2_enable(int debugfs_fd);
> >  bool psr2_disable(int debugfs_fd);
> >  bool psr2_sink_support(int debugfs_fd);
> > +bool psr2_read_last_num_su_blocks_val(int debugfs_fd, uint16_t
> > *num_su_blocks);
> >  
> >  #endif
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index eedde1e8..3c79eec0 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -78,6 +78,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..67c02f25
> > --- /dev/null
> > +++ b/tests/kms_psr2_su.c
> > @@ -0,0 +1,362 @@
> > +/*
> > + * Copyright © 2018 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 <poll.h>
> > +#include <pthread.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
> > +
> > +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 {
> > +	struct igt_fb fb[2];
> > +	struct pollfd pollfds[2];
> > +	pthread_t thread_id;
> > +	int drm_fd;
> > +	int debugfs_fd;
> > +	int flip_timerfd;
> > +	int fail_timerfd;
> > +	uint32_t changes;
> > +	volatile bool run;
> > +	volatile bool sucess;
> > +
> > +	enum operations op;
> > +	uint32_t devid;
> > +	uint32_t crtc_id;
> > +	igt_display_t display;
> > +	drm_intel_bufmgr *bufmgr;
> > +	int mod_size;
> > +	int mod_stride;
> > +	drmModeModeInfo *mode;
> > +	igt_output_t *output;
> > +} 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->crtc_id = output->config.crtc->crtc_id;
> > +		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 *debugfs_thread(void *ptr)
> > +{
> > +	data_t *data = ptr;
> > +	uint16_t expected_num_su_blocks;
> > +
> > +	/* each selective update block is 4 lines tall */
> > +	expected_num_su_blocks = SQUARE_SIZE / 4;
> > +	expected_num_su_blocks += SQUARE_SIZE % 4 ? 1 : 0;
> > +
> > +	while (data->run) {
> > +		uint16_t num_su_blocks;
> > +		bool r;
> > +
> > +		r = psr2_read_last_num_su_blocks_val(data->debugfs_fd,
> > +						     &num_su_blocks);
> > +		if (r && num_su_blocks == expected_num_su_blocks) {
> > +			data->run = false;
> > +			data->sucess = true;
> > +			break;
> > +		}
> > +
> > +		usleep(1);
> > +	}
> > +
> 
> What happens if you start this thread and then flip just once? Do you
> see the expected block count in debugfs at some instance after the
> flip?

As said above no, as we wait for DEEP_SLEEP state and even in SLEEP
state sometimes HW decides to go to PSR exit and PSR active flow
instead of do a selective update.

> 
> > +	return NULL;
> > +}
> > +
> > +static void prepare(data_t *data)
> > +{
> > +	struct itimerspec interval;
> > +	igt_plane_t *primary;
> > +	int r;
> > +
> > +	/* 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]);
> > +		/* 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);
> > +	}
> > +
> > +	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(psr2_wait_deep_sleep(data->debugfs_fd));
> > +
> > +	data->run = true;
> > +	data->sucess = false;
> > +	data->changes = 0;
> > +
> > +	r = pthread_create(&data->thread_id, NULL, debugfs_thread,
> > data);
> > +	if (r)
> > +		igt_warn("Error starting thread: %i\n", r);
> > +
> > +	interval.it_value.tv_nsec = 0;
> > +	interval.it_value.tv_sec = 3;
> > +	interval.it_interval.tv_nsec = interval.it_value.tv_nsec;
> > +	interval.it_interval.tv_sec = interval.it_value.tv_sec;
> > +	r = timerfd_settime(data->fail_timerfd, 0, &interval, NULL);
> > +	igt_require_f(r != -1, "Error setting timerfd\n");
> > +}
> > +
> > +static void update_screen(data_t *data)
> > +{
> > +	data->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->changes &
> > 1]);
> > +		igt_display_commit(&data->display);
> > +		break;
> > +	}
> > +	case FRONTBUFFER: {
> > +		drmModeClip clip;
> > +		cairo_t *cr;
> > +		int r;
> > +
> > +		clip.x1 = clip.y1 = 0;
> > +		clip.x2 = clip.y2 = SQUARE_SIZE;
> > +
> > +		cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
> > +
> > +		if (data->changes & 1) {
> > +			/* go back to all green frame with with square
> > */
> > +			igt_paint_color_alpha(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(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");
> > +	}
> > +}
> > +
> > +static void run(data_t *data)
> > +{
> > +	while (data->run) {
> > +		uint64_t exp;
> > +		int r;
> > +
> > +		r = poll(data->pollfds,
> > +			 sizeof(data->pollfds) / sizeof(data-
> > > pollfds[0]), -1);
> > +		if (r < 0)
> > +			break;
> > +
> > +		/* flip_timerfd timeout */
> > +		if (data->pollfds[0].revents & POLLIN) {
> > +			r = read(data->pollfds[0].fd, &exp,
> > sizeof(exp));
> > +
> > +			if (r != sizeof(uint64_t)) {
> > +				igt_warn("read a not expected number of
> > bytes from flip_timerfd: %i\n", r);
> > +			} else if (exp)
> > +				update_screen(data);
> > +		}
> > +
> > +		/* fail_timerfd timeout */
> Wouldn't it be simpler to flip a fixed number of times?

Yeah it would be simpler, I will change to that.

> 
> 
> > +		if (data->pollfds[1].revents & POLLIN) {
> > +			r = read(data->pollfds[1].fd, &exp,
> > sizeof(exp));
> > +
> > +			if (r != sizeof(uint64_t)) {
> > +				igt_warn("read a not expected number of
> > bytes from fail_timerfd: %i\n", r);
> > +			} else if (exp)
> > +				break;
> > +		}
> > +	}
> > +
> > +	data->run = false;
> > +	pthread_join(data->thread_id, NULL);
> > +
> > +	igt_debug("Changes: %u\n", data->changes);
> > +	igt_assert(data->sucess);
> > +}
> > +
> > +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);
> > +
> > +	igt_remove_fb(data->drm_fd, &data->fb[0]);
> > +	if (data->op == PAGE_FLIP)
> > +		igt_remove_fb(data->drm_fd, &data->fb[1]);
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	data_t data = {};
> > +	enum operations op;
> > +
> > +	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();
> > +		data.devid = intel_get_drm_devid(data.drm_fd);
> > +
> > +		igt_require_f(psr2_sink_support(data.debugfs_fd),
> > +			      "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);
> > +
> > +		igt_require(psr2_enable(data.debugfs_fd));
> > +		igt_require(psr2_wait_deep_sleep(data.debugfs_fd));
> > +
> > +		data.flip_timerfd = timerfd_create(CLOCK_MONOTONIC,
> > +						   TFD_NONBLOCK);
> > +		igt_require(data.flip_timerfd != -1);
> > +		interval.it_value.tv_nsec = NSEC_PER_SEC / 15;
> What's the rationale for this number?


15hz flips, I will add a comment here.



> 
> > +		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.flip_timerfd, 0, &interval,
> > NULL);
> > +		igt_require_f(r != -1, "Error setting timerfd\n");
> > +
> > +		data.fail_timerfd = timerfd_create(CLOCK_MONOTONIC,
> > +						   TFD_NONBLOCK);
> > +		igt_require(data.fail_timerfd != -1);
> > +
> > +		data.pollfds[0].fd = data.flip_timerfd;
> > +		data.pollfds[0].events = POLLIN;
> > +		data.pollfds[0].revents = 0;
> > +
> > +		data.pollfds[1].fd = data.fail_timerfd;
> > +		data.pollfds[1].events = POLLIN;
> > +		data.pollfds[1].revents = 0;
> > +	}
> > +
> > +	for (op = PAGE_FLIP; op < LAST; op++) {
> > +		igt_subtest_f("%s", op_str(op)) {
> > +			data.op = 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 b8a6e61b..c557333c 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -49,6 +49,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/20190114/df1cc530/attachment.sig>


More information about the igt-dev mailing list