[igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[José Roberto de Souza]

Lets run the same PSR1 basic tests for PSR2 to caught PSR2
regressions faster.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/intel-ci/fast-feedback.testlist | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index da3c4c8e..e48cb8a5 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
 igt@kms_psr@cursor_plane_move
 igt@kms_psr@sprite_plane_onoff
 igt@kms_psr@primary_mmap_gtt
+igt@kms_psr@psr2_primary_page_flip
+igt@kms_psr@psr2_cursor_plane_move
+igt@kms_psr@psr2_sprite_plane_onoff
+igt@kms_psr@psr2_primary_mmap_gtt
 igt@kms_setmode@basic-clone-single-crtc
 igt@pm_backlight@basic-brightness
 igt@pm_rpm@basic-pci-d3-state
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[José Roberto de Souza]

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

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 lib/igt_psr.c          |  29 ++++
 lib/igt_psr.h          |   1 +
 tests/Makefile.sources |   1 +
 tests/kms_psr2_su.c    | 302 +++++++++++++++++++++++++++++++++++++++++
 tests/meson.build      |   1 +
 5 files changed, 334 insertions(+)
 create mode 100644 tests/kms_psr2_su.c

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index d726fad5..8c0f05e8 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -178,3 +178,32 @@ bool psr_sink_support(int debugfs_fd, enum psr_mode mode)
 		 */
 		return strstr(buf, "Sink support: yes [0x03]");
 }
+
+#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 7e7017bf..49599cf8 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -40,5 +40,6 @@ bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
 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 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..f6e85a2f
--- /dev/null
+++ b/tests/kms_psr2_su.c
@@ -0,0 +1,302 @@
+/*
+ * 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 <poll.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];
+	struct pollfd pollfds[1];
+	enum operations op;
+	int change_screen_timerfd;
+	uint32_t screen_changes;
+	bool success;
+
+} 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);
+	}
+
+	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->success = false;
+	data->screen_changes = 0;
+}
+
+static void update_screen_and_test(data_t *data)
+{
+	uint16_t su_blocks;
+
+	data->screen_changes++;
+
+	switch (data->op) {
+	case PAGE_FLIP: {
+		igt_plane_t *primary;
+
+		primary = igt_output_get_plane_type(data->output,
+						    DRM_PLANE_TYPE_PRIMARY);
+
+		igt_plane_set_fb(primary, &data->fb[data->screen_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->screen_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");
+	}
+
+	if (psr2_wait_su(data->debugfs_fd, &su_blocks))
+		data->success = su_blocks == EXPECTED_NUM_SU_BLOCKS;
+}
+
+static void run(data_t *data)
+{
+	while (data->screen_changes < MAX_SCREEN_CHANGES && !data->success) {
+		uint64_t exp;
+		int r;
+
+		r = poll(data->pollfds,
+			 sizeof(data->pollfds) / sizeof(data->pollfds[0]), -1);
+		if (r < 0)
+			break;
+
+		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 change_screen_timerfd: %i\n", r);
+			} else if (exp)
+				update_screen_and_test(data);
+		}
+	}
+
+	igt_debug("Screen changes: %u\n", data->screen_changes);
+	igt_assert(data->success);
+}
+
+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 = {};
+
+	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);
+
+		igt_require(psr_enable(data.debugfs_fd, PSR_MODE_2));
+		igt_require(psr_wait_entry(data.debugfs_fd, PSR_MODE_2));
+
+		data.change_screen_timerfd = timerfd_create(CLOCK_MONOTONIC,
+							    TFD_NONBLOCK);
+		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");
+
+		data.pollfds[0].fd = data.change_screen_timerfd;
+		data.pollfds[0].events = POLLIN;
+		data.pollfds[0].revents = 0;
+	}
+
+	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',
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 2/2] test: Add PSR2 selective update tests

[Dhinakaran Pandiyan]

On Tue, 2019-01-22 at 17:09 -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.

How do you ensure the hardware hasn't gone to deep sleep? Can we make
the test fail if the test configuration allowed DEEP_SLEEP? 
> 
> 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
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_psr.c          |  29 ++++
>  lib/igt_psr.h          |   1 +
>  tests/Makefile.sources |   1 +
>  tests/kms_psr2_su.c    | 302
> +++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build      |   1 +
>  5 files changed, 334 insertions(+)
>  create mode 100644 tests/kms_psr2_su.c
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index d726fad5..8c0f05e8 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -178,3 +178,32 @@ bool psr_sink_support(int debugfs_fd, enum
> psr_mode mode)
>  		 */
>  		return strstr(buf, "Sink support: yes [0x03]");
>  }
> +
> +#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 7e7017bf..49599cf8 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -40,5 +40,6 @@ bool psr_wait_update(int debugfs_fd, enum psr_mode
> mode);
>  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 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..f6e85a2f
> --- /dev/null
> +++ b/tests/kms_psr2_su.c
> @@ -0,0 +1,302 @@
> +/*
> + * 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 <poll.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];
> +	struct pollfd pollfds[1];

> +	enum operations op;
> +	int change_screen_timerfd;
> +	uint32_t screen_changes;
> +	bool success;
> +
> +} 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);
> +	}
> +
> +	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->success = false;
> +	data->screen_changes = 0;
> +}
> +
> +static void update_screen_and_test(data_t *data)
> +{
> +	uint16_t su_blocks;
> +
> +	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;
> +		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]);
I'm not familiar with cairo usage, looks like igt_put_cairo_ctx() is
missing. And do you need a get and put for each update? I assume you
could do a get at the beginning of the test.

> +
> +		if (data->screen_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");
> +	}
> +
> +	if (psr2_wait_su(data->debugfs_fd, &su_blocks))
> +		data->success = su_blocks == EXPECTED_NUM_SU_BLOCKS;

Looks good overall, I haven't reviewed the details yet. Now that
there's only one thread, return bool and kill data->success?

> +}
> +
> +static void run(data_t *data)
> +{
> +	while (data->screen_changes < MAX_SCREEN_CHANGES && !data-
> >success) {
> +		uint64_t exp;
> +		int r;
> +
> +		r = poll(data->pollfds,
> +			 sizeof(data->pollfds) / sizeof(data-
> >pollfds[0]), -1);
> +		if (r < 0)
> +			break;
> +
> +		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 change_screen_timerfd: %i\n", r);
> +			} else if (exp)
> +				update_screen_and_test(data);
> +		}
> +	}
> +
> +	igt_debug("Screen changes: %u\n", data->screen_changes);
> +	igt_assert(data->success);
Consider using assert_f() to add some debug information when the test
fails.
> +}
> +
> +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 = {};
> +
> +	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);
> +
> +		igt_require(psr_enable(data.debugfs_fd, PSR_MODE_2));
> +		igt_require(psr_wait_entry(data.debugfs_fd,
> PSR_MODE_2));
> +
> +		data.change_screen_timerfd =
> timerfd_create(CLOCK_MONOTONIC,
> +							    TFD_NONBLOC
> K);
Does this need to be non-blocking, can't we allow read() to block until
the timer expires and then do a screen update?

> +		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");
> +
> +		data.pollfds[0].fd = data.change_screen_timerfd;
> +		data.pollfds[0].events = POLLIN;
> +		data.pollfds[0].revents = 0;
> +	}
> +
> +	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',
Please send a hack patch to include this in BAT so that we can see the
results.

>  	'kms_pwrite_crc',
>  	'kms_rmfb',
>  	'kms_rotation_crc',

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 2/2] test: Add PSR2 selective update tests

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 15411 bytes --]

On Tue, 2019-01-22 at 21:30 -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2019-01-22 at 17:09 -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.
> 
> How do you ensure the hardware hasn't gone to deep sleep? Can we make
> the test fail if the test configuration allowed DEEP_SLEEP? 

The test will fail if it goes to DEEP_SLEEP after the
MAX_SCREEN_CHANGES but this is unlikely to happen as the minimum number
of frames to enter deep sleep is 6.


> > 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
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  lib/igt_psr.c          |  29 ++++
> >  lib/igt_psr.h          |   1 +
> >  tests/Makefile.sources |   1 +
> >  tests/kms_psr2_su.c    | 302
> > +++++++++++++++++++++++++++++++++++++++++
> >  tests/meson.build      |   1 +
> >  5 files changed, 334 insertions(+)
> >  create mode 100644 tests/kms_psr2_su.c
> > 
> > diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> > index d726fad5..8c0f05e8 100644
> > --- a/lib/igt_psr.c
> > +++ b/lib/igt_psr.c
> > @@ -178,3 +178,32 @@ bool psr_sink_support(int debugfs_fd, enum
> > psr_mode mode)
> >  		 */
> >  		return strstr(buf, "Sink support: yes [0x03]");
> >  }
> > +
> > +#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 7e7017bf..49599cf8 100644
> > --- a/lib/igt_psr.h
> > +++ b/lib/igt_psr.h
> > @@ -40,5 +40,6 @@ bool psr_wait_update(int debugfs_fd, enum
> > psr_mode
> > mode);
> >  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 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..f6e85a2f
> > --- /dev/null
> > +++ b/tests/kms_psr2_su.c
> > @@ -0,0 +1,302 @@
> > +/*
> > + * 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 <poll.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];
> > +	struct pollfd pollfds[1];
> > +	enum operations op;
> > +	int change_screen_timerfd;
> > +	uint32_t screen_changes;
> > +	bool success;
> > +
> > +} 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);
> > +	}
> > +
> > +	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->success = false;
> > +	data->screen_changes = 0;
> > +}
> > +
> > +static void update_screen_and_test(data_t *data)
> > +{
> > +	uint16_t su_blocks;
> > +
> > +	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;
> > +		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]);
> I'm not familiar with cairo usage, looks like igt_put_cairo_ctx() is
> missing. And do you need a get and put for each update? I assume you
> could do a get at the beginning of the test.

Thanks for catching it, I forgot to call igt_put_cairo_ctx() for
frontbuffer and yes the context could be acquired when preparing for
test.

> 
> > +
> > +		if (data->screen_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");
> > +	}
> > +
> > +	if (psr2_wait_su(data->debugfs_fd, &su_blocks))
> > +		data->success = su_blocks == EXPECTED_NUM_SU_BLOCKS;
> 
> Looks good overall, I haven't reviewed the details yet. Now that
> there's only one thread, return bool and kill data->success?

done

> 
> > +}
> > +
> > +static void run(data_t *data)
> > +{
> > +	while (data->screen_changes < MAX_SCREEN_CHANGES && !data-
> > > success) {
> > +		uint64_t exp;
> > +		int r;
> > +
> > +		r = poll(data->pollfds,
> > +			 sizeof(data->pollfds) / sizeof(data-
> > > pollfds[0]), -1);
> > +		if (r < 0)
> > +			break;
> > +
> > +		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 change_screen_timerfd: %i\n", r);
> > +			} else if (exp)
> > +				update_screen_and_test(data);
> > +		}
> > +	}
> > +
> > +	igt_debug("Screen changes: %u\n", data->screen_changes);
> > +	igt_assert(data->success);
> Consider using assert_f() to add some debug information when the test
> fails.
> > +}
> > +
> > +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 = {};
> > +
> > +	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);
> > +
> > +		igt_require(psr_enable(data.debugfs_fd, PSR_MODE_2));
> > +		igt_require(psr_wait_entry(data.debugfs_fd,
> > PSR_MODE_2));
> > +
> > +		data.change_screen_timerfd =
> > timerfd_create(CLOCK_MONOTONIC,
> > +							    TFD_NONBLOC
> > K);
> Does this need to be non-blocking, can't we allow read() to block
> until
> the timer expires and then do a screen update?

Huum, this way we can remove the poll().

> 
> > +		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");
> > +
> > +		data.pollfds[0].fd = data.change_screen_timerfd;
> > +		data.pollfds[0].events = POLLIN;
> > +		data.pollfds[0].revents = 0;
> > +	}
> > +
> > +	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',
> Please send a hack patch to include this in BAT so that we can see
> the
> results.

Okay

> 
> >  	'kms_pwrite_crc',
> >  	'kms_rmfb',
> >  	'kms_rotation_crc',

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,v3,1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v3,1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list
URL   : https://patchwork.freedesktop.org/series/55594/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5468 -> IGTPW_2276
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55594/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_2276:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@kms_psr@psr2_primary_page_flip}:
    - fi-whl-u:           NOTRUN -> FAIL +3

  
Known issues
------------

  Here are the changes found in IGTPW_2276 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_hangcheck:
    - fi-bwr-2160:        PASS -> DMESG-FAIL [fdo#108735]

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362] +1

  
#### Possible fixes ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       FAIL [fdo#108767] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS +1

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (46 -> 39)
------------------------------

  Missing    (7): fi-kbl-soraka fi-kbl-7567u fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ivb-3770 fi-pnv-d510 


Build changes
-------------

    * IGT: IGT_4784 -> IGTPW_2276

  CI_DRM_5468: fc4e30d30d90ed5d5bd467de0439e9522d34cdf0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2276: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2276/
  IGT_4784: 1c5a4432293369f85859c748c08155e79d92c4ce @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_psr2_su@frontbuffer
+igt@kms_psr2_su@page_flip

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2276/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,v3,1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v3,1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list
URL   : https://patchwork.freedesktop.org/series/55594/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5468_full -> IGTPW_2276_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55594/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2276_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-apl:          PASS -> FAIL [fdo#106641]
    - shard-glk:          PASS -> FAIL [fdo#106641]

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-glk:          PASS -> FAIL [fdo#103232] +9

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +5
    - shard-kbl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_legacy@pipe-c-torture-move:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#107122]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          PASS -> FAIL [fdo#102887] / [fdo#105363]

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-kbl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166] +7

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          PASS -> FAIL [fdo#103166] +5

  
#### Possible fixes ####

  * igt@i915_suspend@forcewake:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-glk:          FAIL [fdo#103232] -> PASS +2
    - shard-apl:          FAIL [fdo#103232] -> PASS
    - shard-kbl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          FAIL [fdo#105363] -> PASS +1

  * igt@kms_flip@dpms-vs-vblank-race:
    - shard-kbl:          FAIL [fdo#103060] -> PASS
    - shard-glk:          FAIL [fdo#103060] -> PASS

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          FAIL [fdo#103166] -> PASS +4
    - shard-glk:          FAIL [fdo#103166] -> PASS +1
    - shard-kbl:          FAIL [fdo#103166] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-apl:          DMESG-FAIL [fdo#108950] -> PASS

  * igt@template@b:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

  
#### Warnings ####

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-indfb-msflip-blt:
    - shard-snb:          {SKIP} [fdo#109271] -> INCOMPLETE [fdo#105411] / [fdo#107469]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107122]: https://bugs.freedesktop.org/show_bug.cgi?id=107122
  [fdo#107469]: https://bugs.freedesktop.org/show_bug.cgi?id=107469
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4784 -> IGTPW_2276
    * Piglit: piglit_4509 -> None

  CI_DRM_5468: fc4e30d30d90ed5d5bd467de0439e9522d34cdf0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2276: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2276/
  IGT_4784: 1c5a4432293369f85859c748c08155e79d92c4ce @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2276/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Petri Latvala]

On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza wrote:
> Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> regressions faster.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  tests/intel-ci/fast-feedback.testlist | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> index da3c4c8e..e48cb8a5 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
>  igt@kms_psr@cursor_plane_move
>  igt@kms_psr@sprite_plane_onoff
>  igt@kms_psr@primary_mmap_gtt
> +igt@kms_psr@psr2_primary_page_flip
> +igt@kms_psr@psr2_cursor_plane_move
> +igt@kms_psr@psr2_sprite_plane_onoff
> +igt@kms_psr@psr2_primary_mmap_gtt


The BAT results mail said success because these are new tests, but do
note that they failed. They must pass to get onto the BAT list.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Daniel Vetter]

On Wed, Jan 23, 2019 at 01:37:19PM +0200, Petri Latvala wrote:
> On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza wrote:
> > Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> > regressions faster.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> > index da3c4c8e..e48cb8a5 100644
> > --- a/tests/intel-ci/fast-feedback.testlist
> > +++ b/tests/intel-ci/fast-feedback.testlist
> > @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
> >  igt@kms_psr@cursor_plane_move
> >  igt@kms_psr@sprite_plane_onoff
> >  igt@kms_psr@primary_mmap_gtt
> > +igt@kms_psr@psr2_primary_page_flip
> > +igt@kms_psr@psr2_cursor_plane_move
> > +igt@kms_psr@psr2_sprite_plane_onoff
> > +igt@kms_psr@psr2_primary_mmap_gtt
> 
> 
> The BAT results mail said success because these are new tests, but do
> note that they failed. They must pass to get onto the BAT list.

Also, adding all kinds of tests to BAT to validate features doesn't scale.
We need some way to run these tests on specific machines as part of the
follow-up shard runs ... Otherwise we're stuck with a huge pressure to add
all kinds of super-important-feature-right-now things to BAT.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Rodrigo Vivi]

On Wed, Jan 23, 2019 at 01:07:32PM +0100, Daniel Vetter wrote:
> On Wed, Jan 23, 2019 at 01:37:19PM +0200, Petri Latvala wrote:
> > On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza wrote:
> > > Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> > > regressions faster.
> > > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> > > index da3c4c8e..e48cb8a5 100644
> > > --- a/tests/intel-ci/fast-feedback.testlist
> > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
> > >  igt@kms_psr@cursor_plane_move
> > >  igt@kms_psr@sprite_plane_onoff
> > >  igt@kms_psr@primary_mmap_gtt
> > > +igt@kms_psr@psr2_primary_page_flip
> > > +igt@kms_psr@psr2_cursor_plane_move
> > > +igt@kms_psr@psr2_sprite_plane_onoff
> > > +igt@kms_psr@psr2_primary_mmap_gtt
> > 
> > 
> > The BAT results mail said success because these are new tests, but do
> > note that they failed. They must pass to get onto the BAT list.
> 
> Also, adding all kinds of tests to BAT to validate features doesn't scale.
> We need some way to run these tests on specific machines as part of the
> follow-up shard runs ... Otherwise we're stuck with a huge pressure to add
> all kinds of super-important-feature-right-now things to BAT.

I understand and I agree with your point. But on this very specific case
no shard have PSR1 or PSR2 panels.

Also this shouldn't increase the test time much, because machines with PSR1 are
already running the PSR1 tests only, machines without PSR are not running
anything and machines. Only machines with PSR2 panels that are now coming from
no PSR tests to running this few PSR2 tests.

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Daniel Vetter]

On Wed, Jan 23, 2019 at 5:45 PM Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>
> On Wed, Jan 23, 2019 at 01:07:32PM +0100, Daniel Vetter wrote:
> > On Wed, Jan 23, 2019 at 01:37:19PM +0200, Petri Latvala wrote:
> > > On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza wrote:
> > > > Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> > > > regressions faster.
> > > >
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> > > > index da3c4c8e..e48cb8a5 100644
> > > > --- a/tests/intel-ci/fast-feedback.testlist
> > > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > > @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
> > > >  igt@kms_psr@cursor_plane_move
> > > >  igt@kms_psr@sprite_plane_onoff
> > > >  igt@kms_psr@primary_mmap_gtt
> > > > +igt@kms_psr@psr2_primary_page_flip
> > > > +igt@kms_psr@psr2_cursor_plane_move
> > > > +igt@kms_psr@psr2_sprite_plane_onoff
> > > > +igt@kms_psr@psr2_primary_mmap_gtt
> > >
> > >
> > > The BAT results mail said success because these are new tests, but do
> > > note that they failed. They must pass to get onto the BAT list.
> >
> > Also, adding all kinds of tests to BAT to validate features doesn't scale.
> > We need some way to run these tests on specific machines as part of the
> > follow-up shard runs ... Otherwise we're stuck with a huge pressure to add
> > all kinds of super-important-feature-right-now things to BAT.
>
> I understand and I agree with your point. But on this very specific case
> no shard have PSR1 or PSR2 panels.

Yeah. Same way that no shard has:
-mst
-hdcp
-dsi
-4k
- ...

The list is very long. Everyone wants their feature to be an
exception. Everyone's feature only increase test time by "not much".

> Also this shouldn't increase the test time much, because machines with PSR1 are
> already running the PSR1 tests only, machines without PSR are not running
> anything and machines. Only machines with PSR2 panels that are now coming from
> no PSR tests to running this few PSR2 tests.

Ok, I guess that ship sailed with the psr1 tests already then.
-Daniel

>
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Rodrigo Vivi]

On Wed, Jan 23, 2019 at 05:51:11PM +0100, Daniel Vetter wrote:
> On Wed, Jan 23, 2019 at 5:45 PM Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> >
> > On Wed, Jan 23, 2019 at 01:07:32PM +0100, Daniel Vetter wrote:
> > > On Wed, Jan 23, 2019 at 01:37:19PM +0200, Petri Latvala wrote:
> > > > On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza wrote:
> > > > > Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> > > > > regressions faster.
> > > > >
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > ---
> > > > >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> > > > > index da3c4c8e..e48cb8a5 100644
> > > > > --- a/tests/intel-ci/fast-feedback.testlist
> > > > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > > > @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
> > > > >  igt@kms_psr@cursor_plane_move
> > > > >  igt@kms_psr@sprite_plane_onoff
> > > > >  igt@kms_psr@primary_mmap_gtt
> > > > > +igt@kms_psr@psr2_primary_page_flip
> > > > > +igt@kms_psr@psr2_cursor_plane_move
> > > > > +igt@kms_psr@psr2_sprite_plane_onoff
> > > > > +igt@kms_psr@psr2_primary_mmap_gtt
> > > >
> > > >
> > > > The BAT results mail said success because these are new tests, but do
> > > > note that they failed. They must pass to get onto the BAT list.
> > >
> > > Also, adding all kinds of tests to BAT to validate features doesn't scale.
> > > We need some way to run these tests on specific machines as part of the
> > > follow-up shard runs ... Otherwise we're stuck with a huge pressure to add
> > > all kinds of super-important-feature-right-now things to BAT.
> >
> > I understand and I agree with your point. But on this very specific case
> > no shard have PSR1 or PSR2 panels.
> 
> Yeah. Same way that no shard has:
> -mst
> -hdcp
> -dsi
> -4k
> - ...

"coincidentally" all display related :-)

> 
> The list is very long. Everyone wants their feature to be an
> exception. Everyone's feature only increase test time by "not much".

Yeap, I understand that everybody will put their feature as important,
but for me another factor that justify that increase is the "fragile"
part.

For me the important + fragile deserves a space even if we have to wait
minutes more for the result :/

> 
> > Also this shouldn't increase the test time much, because machines with PSR1 are
> > already running the PSR1 tests only, machines without PSR are not running
> > anything and machines. Only machines with PSR2 panels that are now coming from
> > no PSR tests to running this few PSR2 tests.
> 
> Ok, I guess that ship sailed with the psr1 tests already then.

besides, I think MST also deserves this "privilege" :)

> -Daniel
> 
> >
> > > -Daniel
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > http://blog.ffwll.ch
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Dhinakaran Pandiyan]

On Wed, 2019-01-23 at 09:17 -0800, Rodrigo Vivi wrote:
> On Wed, Jan 23, 2019 at 05:51:11PM +0100, Daniel Vetter wrote:
> > On Wed, Jan 23, 2019 at 5:45 PM Rodrigo Vivi <
> > rodrigo.vivi@intel.com> wrote:
> > > 
> > > On Wed, Jan 23, 2019 at 01:07:32PM +0100, Daniel Vetter wrote:
> > > > On Wed, Jan 23, 2019 at 01:37:19PM +0200, Petri Latvala wrote:
> > > > > On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de
> > > > > Souza wrote:
> > > > > > Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> > > > > > regressions faster.
> > > > > > 
> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > > ---
> > > > > >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> > > > > >  1 file changed, 4 insertions(+)
> > > > > > 
> > > > > > diff --git a/tests/intel-ci/fast-feedback.testlist
> > > > > > b/tests/intel-ci/fast-feedback.testlist
> > > > > > index da3c4c8e..e48cb8a5 100644
> > > > > > --- a/tests/intel-ci/fast-feedback.testlist
> > > > > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > > > > @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
> > > > > >  igt@kms_psr@cursor_plane_move
> > > > > >  igt@kms_psr@sprite_plane_onoff
> > > > > >  igt@kms_psr@primary_mmap_gtt
> > > > > > +igt@kms_psr@psr2_primary_page_flip
> > > > > > +igt@kms_psr@psr2_cursor_plane_move
> > > > > > +igt@kms_psr@psr2_sprite_plane_onoff
> > > > > > +igt@kms_psr@psr2_primary_mmap_gtt
> > > > > 
> > > > > 
> > > > > The BAT results mail said success because these are new
> > > > > tests, but do
> > > > > note that they failed. They must pass to get onto the BAT
> > > > > list.
> > > > 
> > > > Also, adding all kinds of tests to BAT to validate features
> > > > doesn't scale.
> > > > We need some way to run these tests on specific machines as
> > > > part of the
> > > > follow-up shard runs ... Otherwise we're stuck with a huge
> > > > pressure to add
> > > > all kinds of super-important-feature-right-now things to BAT.
> > > 
> > > I understand and I agree with your point. But on this very
> > > specific case
> > > no shard have PSR1 or PSR2 panels.
> > 
> > Yeah. Same way that no shard has:
> > -mst
> > -hdcp
> > -dsi
> > -4k
> > - ...

This is a chicken and egg problem, without testing, we can't enable new
features.

I believe there was some problem in acquiring the same panel for all
shard instances. Another issue that came up was shard machines had to
be stable and production NUC's were preferred. Which means, no eDP.

 
> 
> "coincidentally" all display related :-)
> 
> > 
> > The list is very long. Everyone wants their feature to be an
> > exception. Everyone's feature only increase test time by "not
> > much".
> 
> Yeap, I understand that everybody will put their feature as
> important,
> but for me another factor that justify that increase is the "fragile"
> part.
> 
> For me the important + fragile deserves a space even if we have to
> wait
> minutes more for the result :/
> 
> > 
> > > Also this shouldn't increase the test time much, because machines
> > > with PSR1 are
> > > already running the PSR1 tests only, machines without PSR are not
> > > running
> > > anything and machines. Only machines with PSR2 panels that are
> > > now coming from
> > > no PSR tests to running this few PSR2 tests.
> > 
> > Ok, I guess that ship sailed with the psr1 tests already then.
> 
> besides, I think MST also deserves this "privilege" :)
> 
> > -Daniel
> > 
> > > 
> > > > -Daniel
> > > > --
> > > > Daniel Vetter
> > > > Software Engineer, Intel Corporation
> > > > http://blog.ffwll.ch
> > 
> > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Daniel Vetter]

On Wed, Jan 23, 2019 at 09:17:17AM -0800, Rodrigo Vivi wrote:
> On Wed, Jan 23, 2019 at 05:51:11PM +0100, Daniel Vetter wrote:
> > On Wed, Jan 23, 2019 at 5:45 PM Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > >
> > > On Wed, Jan 23, 2019 at 01:07:32PM +0100, Daniel Vetter wrote:
> > > > On Wed, Jan 23, 2019 at 01:37:19PM +0200, Petri Latvala wrote:
> > > > > On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza wrote:
> > > > > > Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> > > > > > regressions faster.
> > > > > >
> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > > ---
> > > > > >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> > > > > >  1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> > > > > > index da3c4c8e..e48cb8a5 100644
> > > > > > --- a/tests/intel-ci/fast-feedback.testlist
> > > > > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > > > > @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
> > > > > >  igt@kms_psr@cursor_plane_move
> > > > > >  igt@kms_psr@sprite_plane_onoff
> > > > > >  igt@kms_psr@primary_mmap_gtt
> > > > > > +igt@kms_psr@psr2_primary_page_flip
> > > > > > +igt@kms_psr@psr2_cursor_plane_move
> > > > > > +igt@kms_psr@psr2_sprite_plane_onoff
> > > > > > +igt@kms_psr@psr2_primary_mmap_gtt
> > > > >
> > > > >
> > > > > The BAT results mail said success because these are new tests, but do
> > > > > note that they failed. They must pass to get onto the BAT list.
> > > >
> > > > Also, adding all kinds of tests to BAT to validate features doesn't scale.
> > > > We need some way to run these tests on specific machines as part of the
> > > > follow-up shard runs ... Otherwise we're stuck with a huge pressure to add
> > > > all kinds of super-important-feature-right-now things to BAT.
> > >
> > > I understand and I agree with your point. But on this very specific case
> > > no shard have PSR1 or PSR2 panels.
> > 
> > Yeah. Same way that no shard has:
> > -mst
> > -hdcp
> > -dsi
> > -4k
> > - ...
> 
> "coincidentally" all display related :-)
> 
> > 
> > The list is very long. Everyone wants their feature to be an
> > exception. Everyone's feature only increase test time by "not much".
> 
> Yeap, I understand that everybody will put their feature as important,
> but for me another factor that justify that increase is the "fragile"
> part.
> 
> For me the important + fragile deserves a space even if we have to wait
> minutes more for the result :/
> 
> > 
> > > Also this shouldn't increase the test time much, because machines with PSR1 are
> > > already running the PSR1 tests only, machines without PSR are not running
> > > anything and machines. Only machines with PSR2 panels that are now coming from
> > > no PSR tests to running this few PSR2 tests.
> > 
> > Ok, I guess that ship sailed with the psr1 tests already then.
> 
> besides, I think MST also deserves this "privilege" :)

You misunderstood I think, I'm not saying we shouldn't test this. I'm
saying we shouldn't test this in BAT, but solve this problem for real,
through some dedicated machines that run specific tests as part of shards.
That's the real fix, and the fix that scales, and the fix that will allow
us to test a lot more than just a few BAT tests on a few very select
machines.

And imo as feature owners for this, _you_ folks should be fighting for
this, instead of being ok with squeezing a few tests into BAT. That's not
good enough (aside from that it's inefficient).

I want more testing, not less. So should you :-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Rodrigo Vivi]

On Thu, Jan 24, 2019 at 01:55:41PM +0100, Daniel Vetter wrote:
> On Wed, Jan 23, 2019 at 09:17:17AM -0800, Rodrigo Vivi wrote:
> > On Wed, Jan 23, 2019 at 05:51:11PM +0100, Daniel Vetter wrote:
> > > On Wed, Jan 23, 2019 at 5:45 PM Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > >
> > > > On Wed, Jan 23, 2019 at 01:07:32PM +0100, Daniel Vetter wrote:
> > > > > On Wed, Jan 23, 2019 at 01:37:19PM +0200, Petri Latvala wrote:
> > > > > > On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza wrote:
> > > > > > > Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> > > > > > > regressions faster.
> > > > > > >
> > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > > > ---
> > > > > > >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> > > > > > >  1 file changed, 4 insertions(+)
> > > > > > >
> > > > > > > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> > > > > > > index da3c4c8e..e48cb8a5 100644
> > > > > > > --- a/tests/intel-ci/fast-feedback.testlist
> > > > > > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > > > > > @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
> > > > > > >  igt@kms_psr@cursor_plane_move
> > > > > > >  igt@kms_psr@sprite_plane_onoff
> > > > > > >  igt@kms_psr@primary_mmap_gtt
> > > > > > > +igt@kms_psr@psr2_primary_page_flip
> > > > > > > +igt@kms_psr@psr2_cursor_plane_move
> > > > > > > +igt@kms_psr@psr2_sprite_plane_onoff
> > > > > > > +igt@kms_psr@psr2_primary_mmap_gtt
> > > > > >
> > > > > >
> > > > > > The BAT results mail said success because these are new tests, but do
> > > > > > note that they failed. They must pass to get onto the BAT list.
> > > > >
> > > > > Also, adding all kinds of tests to BAT to validate features doesn't scale.
> > > > > We need some way to run these tests on specific machines as part of the
> > > > > follow-up shard runs ... Otherwise we're stuck with a huge pressure to add
> > > > > all kinds of super-important-feature-right-now things to BAT.
> > > >
> > > > I understand and I agree with your point. But on this very specific case
> > > > no shard have PSR1 or PSR2 panels.
> > > 
> > > Yeah. Same way that no shard has:
> > > -mst
> > > -hdcp
> > > -dsi
> > > -4k
> > > - ...
> > 
> > "coincidentally" all display related :-)
> > 
> > > 
> > > The list is very long. Everyone wants their feature to be an
> > > exception. Everyone's feature only increase test time by "not much".
> > 
> > Yeap, I understand that everybody will put their feature as important,
> > but for me another factor that justify that increase is the "fragile"
> > part.
> > 
> > For me the important + fragile deserves a space even if we have to wait
> > minutes more for the result :/
> > 
> > > 
> > > > Also this shouldn't increase the test time much, because machines with PSR1 are
> > > > already running the PSR1 tests only, machines without PSR are not running
> > > > anything and machines. Only machines with PSR2 panels that are now coming from
> > > > no PSR tests to running this few PSR2 tests.
> > > 
> > > Ok, I guess that ship sailed with the psr1 tests already then.
> > 
> > besides, I think MST also deserves this "privilege" :)
> 
> You misunderstood I think, I'm not saying we shouldn't test this. I'm
> saying we shouldn't test this in BAT, but solve this problem for real,
> through some dedicated machines that run specific tests as part of shards.
> That's the real fix, and the fix that scales, and the fix that will allow
> us to test a lot more than just a few BAT tests on a few very select
> machines.

Oh! I see now... That's indeed a very smarter way of scaling this.

And maybe not necessarily "shard" machines and not necessarily running all IGT.
And maybe some specific feature-machine.testlist that is part of the
second round of CI-IGT...

Martin? :$

> 
> And imo as feature owners for this, _you_ folks should be fighting for
> this, instead of being ok with squeezing a few tests into BAT. That's not
> good enough (aside from that it's inefficient).
> 
> I want more testing, not less. So should you :-)
> 
> Cheers, Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Daniel Vetter]

On Thu, Jan 24, 2019 at 02:11:30PM -0800, Rodrigo Vivi wrote:
> On Thu, Jan 24, 2019 at 01:55:41PM +0100, Daniel Vetter wrote:
> > On Wed, Jan 23, 2019 at 09:17:17AM -0800, Rodrigo Vivi wrote:
> > > On Wed, Jan 23, 2019 at 05:51:11PM +0100, Daniel Vetter wrote:
> > > > On Wed, Jan 23, 2019 at 5:45 PM Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > > >
> > > > > On Wed, Jan 23, 2019 at 01:07:32PM +0100, Daniel Vetter wrote:
> > > > > > On Wed, Jan 23, 2019 at 01:37:19PM +0200, Petri Latvala wrote:
> > > > > > > On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza wrote:
> > > > > > > > Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> > > > > > > > regressions faster.
> > > > > > > >
> > > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > > > > ---
> > > > > > > >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> > > > > > > >  1 file changed, 4 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> > > > > > > > index da3c4c8e..e48cb8a5 100644
> > > > > > > > --- a/tests/intel-ci/fast-feedback.testlist
> > > > > > > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > > > > > > @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
> > > > > > > >  igt@kms_psr@cursor_plane_move
> > > > > > > >  igt@kms_psr@sprite_plane_onoff
> > > > > > > >  igt@kms_psr@primary_mmap_gtt
> > > > > > > > +igt@kms_psr@psr2_primary_page_flip
> > > > > > > > +igt@kms_psr@psr2_cursor_plane_move
> > > > > > > > +igt@kms_psr@psr2_sprite_plane_onoff
> > > > > > > > +igt@kms_psr@psr2_primary_mmap_gtt
> > > > > > >
> > > > > > >
> > > > > > > The BAT results mail said success because these are new tests, but do
> > > > > > > note that they failed. They must pass to get onto the BAT list.
> > > > > >
> > > > > > Also, adding all kinds of tests to BAT to validate features doesn't scale.
> > > > > > We need some way to run these tests on specific machines as part of the
> > > > > > follow-up shard runs ... Otherwise we're stuck with a huge pressure to add
> > > > > > all kinds of super-important-feature-right-now things to BAT.
> > > > >
> > > > > I understand and I agree with your point. But on this very specific case
> > > > > no shard have PSR1 or PSR2 panels.
> > > > 
> > > > Yeah. Same way that no shard has:
> > > > -mst
> > > > -hdcp
> > > > -dsi
> > > > -4k
> > > > - ...
> > > 
> > > "coincidentally" all display related :-)
> > > 
> > > > 
> > > > The list is very long. Everyone wants their feature to be an
> > > > exception. Everyone's feature only increase test time by "not much".
> > > 
> > > Yeap, I understand that everybody will put their feature as important,
> > > but for me another factor that justify that increase is the "fragile"
> > > part.
> > > 
> > > For me the important + fragile deserves a space even if we have to wait
> > > minutes more for the result :/
> > > 
> > > > 
> > > > > Also this shouldn't increase the test time much, because machines with PSR1 are
> > > > > already running the PSR1 tests only, machines without PSR are not running
> > > > > anything and machines. Only machines with PSR2 panels that are now coming from
> > > > > no PSR tests to running this few PSR2 tests.
> > > > 
> > > > Ok, I guess that ship sailed with the psr1 tests already then.
> > > 
> > > besides, I think MST also deserves this "privilege" :)
> > 
> > You misunderstood I think, I'm not saying we shouldn't test this. I'm
> > saying we shouldn't test this in BAT, but solve this problem for real,
> > through some dedicated machines that run specific tests as part of shards.
> > That's the real fix, and the fix that scales, and the fix that will allow
> > us to test a lot more than just a few BAT tests on a few very select
> > machines.
> 
> Oh! I see now... That's indeed a very smarter way of scaling this.
> 
> And maybe not necessarily "shard" machines and not necessarily running all IGT.
> And maybe some specific feature-machine.testlist that is part of the
> second round of CI-IGT...

Yes, not a full "shard", just as part of the shard runs. We don't have
enough lab space to have a full shard for every interesting combination,
that's the underlying problem. Those special machines would only run psr
tests, or mst tests, or whatever else is special with them. Of course if
there's idle time we could maybe add more interesting tests to their
testlist.

Also not sure where to maintain the testslist for these, maybe in igt
even. Issue is we want to make sure that any new psr test is added
automatically to the psr machines (as an example).

Cheers, Daniel
> 
> Martin? :$
> 
> > 
> > And imo as feature owners for this, _you_ folks should be fighting for
> > this, instead of being ok with squeezing a few tests into BAT. That's not
> > good enough (aside from that it's inefficient).
> > 
> > I want more testing, not less. So should you :-)
> > 
> > Cheers, Daniel
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Martin Peres]

On 25/01/2019 11:45, Daniel Vetter wrote:
> On Thu, Jan 24, 2019 at 02:11:30PM -0800, Rodrigo Vivi wrote:
>> On Thu, Jan 24, 2019 at 01:55:41PM +0100, Daniel Vetter wrote:
>>> On Wed, Jan 23, 2019 at 09:17:17AM -0800, Rodrigo Vivi wrote:
>>>> On Wed, Jan 23, 2019 at 05:51:11PM +0100, Daniel Vetter wrote:
>>>>> On Wed, Jan 23, 2019 at 5:45 PM Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>>>>>>
>>>>>> On Wed, Jan 23, 2019 at 01:07:32PM +0100, Daniel Vetter wrote:
>>>>>>> On Wed, Jan 23, 2019 at 01:37:19PM +0200, Petri Latvala wrote:
>>>>>>>> On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza wrote:
>>>>>>>>> Lets run the same PSR1 basic tests for PSR2 to caught PSR2
>>>>>>>>> regressions faster.
>>>>>>>>>
>>>>>>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>>>>>>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>>>>>>>>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>>>>>>>>> ---
>>>>>>>>>  tests/intel-ci/fast-feedback.testlist | 4 ++++
>>>>>>>>>  1 file changed, 4 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
>>>>>>>>> index da3c4c8e..e48cb8a5 100644
>>>>>>>>> --- a/tests/intel-ci/fast-feedback.testlist
>>>>>>>>> +++ b/tests/intel-ci/fast-feedback.testlist
>>>>>>>>> @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
>>>>>>>>>  igt@kms_psr@cursor_plane_move
>>>>>>>>>  igt@kms_psr@sprite_plane_onoff
>>>>>>>>>  igt@kms_psr@primary_mmap_gtt
>>>>>>>>> +igt@kms_psr@psr2_primary_page_flip
>>>>>>>>> +igt@kms_psr@psr2_cursor_plane_move
>>>>>>>>> +igt@kms_psr@psr2_sprite_plane_onoff
>>>>>>>>> +igt@kms_psr@psr2_primary_mmap_gtt
>>>>>>>>
>>>>>>>>
>>>>>>>> The BAT results mail said success because these are new tests, but do
>>>>>>>> note that they failed. They must pass to get onto the BAT list.
>>>>>>>
>>>>>>> Also, adding all kinds of tests to BAT to validate features doesn't scale.
>>>>>>> We need some way to run these tests on specific machines as part of the
>>>>>>> follow-up shard runs ... Otherwise we're stuck with a huge pressure to add
>>>>>>> all kinds of super-important-feature-right-now things to BAT.
>>>>>>
>>>>>> I understand and I agree with your point. But on this very specific case
>>>>>> no shard have PSR1 or PSR2 panels.
>>>>>
>>>>> Yeah. Same way that no shard has:
>>>>> -mst
>>>>> -hdcp
>>>>> -dsi
>>>>> -4k
>>>>> - ...
>>>>
>>>> "coincidentally" all display related :-)
>>>>
>>>>>
>>>>> The list is very long. Everyone wants their feature to be an
>>>>> exception. Everyone's feature only increase test time by "not much".
>>>>
>>>> Yeap, I understand that everybody will put their feature as important,
>>>> but for me another factor that justify that increase is the "fragile"
>>>> part.
>>>>
>>>> For me the important + fragile deserves a space even if we have to wait
>>>> minutes more for the result :/
>>>>
>>>>>
>>>>>> Also this shouldn't increase the test time much, because machines with PSR1 are
>>>>>> already running the PSR1 tests only, machines without PSR are not running
>>>>>> anything and machines. Only machines with PSR2 panels that are now coming from
>>>>>> no PSR tests to running this few PSR2 tests.
>>>>>
>>>>> Ok, I guess that ship sailed with the psr1 tests already then.
>>>>
>>>> besides, I think MST also deserves this "privilege" :)
>>>
>>> You misunderstood I think, I'm not saying we shouldn't test this. I'm
>>> saying we shouldn't test this in BAT, but solve this problem for real,
>>> through some dedicated machines that run specific tests as part of shards.
>>> That's the real fix, and the fix that scales, and the fix that will allow
>>> us to test a lot more than just a few BAT tests on a few very select
>>> machines.
>>
>> Oh! I see now... That's indeed a very smarter way of scaling this.
>>
>> And maybe not necessarily "shard" machines and not necessarily running all IGT.
>> And maybe some specific feature-machine.testlist that is part of the
>> second round of CI-IGT...
> 
> Yes, not a full "shard", just as part of the shard runs. We don't have
> enough lab space to have a full shard for every interesting combination,
> that's the underlying problem. Those special machines would only run psr
> tests, or mst tests, or whatever else is special with them. Of course if
> there's idle time we could maybe add more interesting tests to their
> testlist.
> 
> Also not sure where to maintain the testslist for these, maybe in igt
> even. Issue is we want to make sure that any new psr test is added
> automatically to the psr machines (as an example).
> 
> Cheers, Daniel
>>
>> Martin? :$
>>
>>>
>>> And imo as feature owners for this, _you_ folks should be fighting for
>>> this, instead of being ok with squeezing a few tests into BAT. That's not
>>> good enough (aside from that it's inefficient).
>>>
>>> I want more testing, not less. So should you :-)

I agree with the idea. I actually don't like to call the second round
the "Shard Run", but rather want to call it CI Full.

Indeed, the piglit machines are only running piglit during the CI full
run, and they are not sharded. Having more machines as part of the CI
Full run, dedicated to execute a set of tests makes sense, even if it is
part of IGT, is IMO the way to go for features requiring specific HW.

As to how to implement this, I think the testlist should be a whitelist
hosted in the IGT repo. As for the CI system, I will let Tomi comment on
this!

Martin

>>>
>>> Cheers, Daniel
>>> -- 
>>> Daniel Vetter
>>> Software Engineer, Intel Corporation
>>> http://blog.ffwll.ch
>>> _______________________________________________
>>> igt-dev mailing list
>>> igt-dev@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Tomi Sarvela]

On 1/25/19 1:03 PM, Martin Peres wrote:
> On 25/01/2019 11:45, Daniel Vetter wrote:
>> On Thu, Jan 24, 2019 at 02:11:30PM -0800, Rodrigo Vivi wrote:
>>> On Thu, Jan 24, 2019 at 01:55:41PM +0100, Daniel Vetter wrote:
>>>> On Wed, Jan 23, 2019 at 09:17:17AM -0800, Rodrigo Vivi wrote:
>>>>> On Wed, Jan 23, 2019 at 05:51:11PM +0100, Daniel Vetter wrote:
>>>>>> On Wed, Jan 23, 2019 at 5:45 PM Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>>>>>>>
>>>>>>> On Wed, Jan 23, 2019 at 01:07:32PM +0100, Daniel Vetter wrote:
>>>>>>>> On Wed, Jan 23, 2019 at 01:37:19PM +0200, Petri Latvala wrote:
>>>>>>>>> On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza wrote:
>>>>>>>>>> Lets run the same PSR1 basic tests for PSR2 to caught PSR2
>>>>>>>>>> regressions faster.
>>>>>>>>>>
>>>>>>>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>>>>>>>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>>>>>>>>>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>>>>>>>>>> ---
>>>>>>>>>>   tests/intel-ci/fast-feedback.testlist | 4 ++++
>>>>>>>>>>   1 file changed, 4 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
>>>>>>>>>> index da3c4c8e..e48cb8a5 100644
>>>>>>>>>> --- a/tests/intel-ci/fast-feedback.testlist
>>>>>>>>>> +++ b/tests/intel-ci/fast-feedback.testlist
>>>>>>>>>> @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
>>>>>>>>>>   igt@kms_psr@cursor_plane_move
>>>>>>>>>>   igt@kms_psr@sprite_plane_onoff
>>>>>>>>>>   igt@kms_psr@primary_mmap_gtt
>>>>>>>>>> +igt@kms_psr@psr2_primary_page_flip
>>>>>>>>>> +igt@kms_psr@psr2_cursor_plane_move
>>>>>>>>>> +igt@kms_psr@psr2_sprite_plane_onoff
>>>>>>>>>> +igt@kms_psr@psr2_primary_mmap_gtt
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The BAT results mail said success because these are new tests, but do
>>>>>>>>> note that they failed. They must pass to get onto the BAT list.
>>>>>>>>
>>>>>>>> Also, adding all kinds of tests to BAT to validate features doesn't scale.
>>>>>>>> We need some way to run these tests on specific machines as part of the
>>>>>>>> follow-up shard runs ... Otherwise we're stuck with a huge pressure to add
>>>>>>>> all kinds of super-important-feature-right-now things to BAT.
>>>>>>>
>>>>>>> I understand and I agree with your point. But on this very specific case
>>>>>>> no shard have PSR1 or PSR2 panels.
>>>>>>
>>>>>> Yeah. Same way that no shard has:
>>>>>> -mst
>>>>>> -hdcp
>>>>>> -dsi
>>>>>> -4k
>>>>>> - ...
>>>>>
>>>>> "coincidentally" all display related :-)
>>>>>
>>>>>>
>>>>>> The list is very long. Everyone wants their feature to be an
>>>>>> exception. Everyone's feature only increase test time by "not much".
>>>>>
>>>>> Yeap, I understand that everybody will put their feature as important,
>>>>> but for me another factor that justify that increase is the "fragile"
>>>>> part.
>>>>>
>>>>> For me the important + fragile deserves a space even if we have to wait
>>>>> minutes more for the result :/
>>>>>
>>>>>>
>>>>>>> Also this shouldn't increase the test time much, because machines with PSR1 are
>>>>>>> already running the PSR1 tests only, machines without PSR are not running
>>>>>>> anything and machines. Only machines with PSR2 panels that are now coming from
>>>>>>> no PSR tests to running this few PSR2 tests.
>>>>>>
>>>>>> Ok, I guess that ship sailed with the psr1 tests already then.
>>>>>
>>>>> besides, I think MST also deserves this "privilege" :)
>>>>
>>>> You misunderstood I think, I'm not saying we shouldn't test this. I'm
>>>> saying we shouldn't test this in BAT, but solve this problem for real,
>>>> through some dedicated machines that run specific tests as part of shards.
>>>> That's the real fix, and the fix that scales, and the fix that will allow
>>>> us to test a lot more than just a few BAT tests on a few very select
>>>> machines.
>>>
>>> Oh! I see now... That's indeed a very smarter way of scaling this.
>>>
>>> And maybe not necessarily "shard" machines and not necessarily running all IGT.
>>> And maybe some specific feature-machine.testlist that is part of the
>>> second round of CI-IGT...
>>
>> Yes, not a full "shard", just as part of the shard runs. We don't have
>> enough lab space to have a full shard for every interesting combination,
>> that's the underlying problem. Those special machines would only run psr
>> tests, or mst tests, or whatever else is special with them. Of course if
>> there's idle time we could maybe add more interesting tests to their
>> testlist.
>>
>> Also not sure where to maintain the testslist for these, maybe in igt
>> even. Issue is we want to make sure that any new psr test is added
>> automatically to the psr machines (as an example).
>>
>> Cheers, Daniel
>>>
>>> Martin? :$
>>>
>>>>
>>>> And imo as feature owners for this, _you_ folks should be fighting for
>>>> this, instead of being ok with squeezing a few tests into BAT. That's not
>>>> good enough (aside from that it's inefficient).
>>>>
>>>> I want more testing, not less. So should you :-)
> 
> I agree with the idea. I actually don't like to call the second round
> the "Shard Run", but rather want to call it CI Full.
> 
> Indeed, the piglit machines are only running piglit during the CI full
> run, and they are not sharded. Having more machines as part of the CI
> Full run, dedicated to execute a set of tests makes sense, even if it is
> part of IGT, is IMO the way to go for features requiring specific HW.
> 
> As to how to implement this, I think the testlist should be a whitelist
> hosted in the IGT repo. As for the CI system, I will let Tomi comment on
> this!

I think this is doable much like the piglit hosts are done. They each 
have gen-specific testlist (created with blacklists from Mesa CI repo) 
and around 40 minutes time to complete one run. Fastest pig-host uses 
about 20 minutes and idles 70% of the time.

Now, the interesting part comes when we create hardware setup with 
DP-MST (or PSR2 or 4 monitors) and want to run it through specific 
testlist with different software features. Lets say that the 40 minutes 
would be split between non-guc, guc, and iommu-enabled runs ... still 
possible to do, but need more hang recovery. It helps a lot timewise if 
there is as few hangs/incompletes as possible.

Tomi
-- 
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 1857 bytes --]

On Wed, 2019-01-23 at 13:37 +0200, Petri Latvala wrote:
> On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza
> wrote:
> > Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> > regressions faster.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-
> > ci/fast-feedback.testlist
> > index da3c4c8e..e48cb8a5 100644
> > --- a/tests/intel-ci/fast-feedback.testlist
> > +++ b/tests/intel-ci/fast-feedback.testlist
> > @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
> >  igt@kms_psr@cursor_plane_move
> >  igt@kms_psr@sprite_plane_onoff
> >  igt@kms_psr@primary_mmap_gtt
> > +igt@kms_psr@psr2_primary_page_flip
> > +igt@kms_psr@psr2_cursor_plane_move
> > +igt@kms_psr@psr2_sprite_plane_onoff
> > +igt@kms_psr@psr2_primary_mmap_gtt
> 
> The BAT results mail said success because these are new tests, but do
> note that they failed. They must pass to get onto the BAT list.
> 
> 

<7>[   91.805388] [drm:intel_psr_compute_config [i915]] PSR2 not
enabled, resolution 3840x2160 > max supported 3640x2304

It failed because the GEN9 PSR2 HW only supports resolutions up to
3640x2304, so it switched back go PSR1 and the test failed because it
was expecting a PSR2 state not a PSR1.

I talked with DK, he thinks that the panel should be replaced in that
machine so we can have some PSR2 test coverage in GEN9.

If not possible we should add more code in igt_fixture() to check if
PSR2 is supported but that could potentially hide PSR2 issues behind
tests skipped or whitelist PSR2 tests in fi-whl-u.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 1/2] tests/intel-ci: Add basic PSR2 tests to fast feedback test list

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 2288 bytes --]

On Wed, 2019-01-23 at 21:00 +0000, Souza, Jose wrote:
> On Wed, 2019-01-23 at 13:37 +0200, Petri Latvala wrote:
> > On Tue, Jan 22, 2019 at 05:09:49PM -0800, José Roberto de Souza
> > wrote:
> > > Lets run the same PSR1 basic tests for PSR2 to caught PSR2
> > > regressions faster.
> > > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  tests/intel-ci/fast-feedback.testlist | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-
> > > ci/fast-feedback.testlist
> > > index da3c4c8e..e48cb8a5 100644
> > > --- a/tests/intel-ci/fast-feedback.testlist
> > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > @@ -227,6 +227,10 @@ igt@kms_psr@primary_page_flip
> > >  igt@kms_psr@cursor_plane_move
> > >  igt@kms_psr@sprite_plane_onoff
> > >  igt@kms_psr@primary_mmap_gtt
> > > +igt@kms_psr@psr2_primary_page_flip
> > > +igt@kms_psr@psr2_cursor_plane_move
> > > +igt@kms_psr@psr2_sprite_plane_onoff
> > > +igt@kms_psr@psr2_primary_mmap_gtt
> > 
> > The BAT results mail said success because these are new tests, but
> > do
> > note that they failed. They must pass to get onto the BAT list.
> > 
> > 
> 
> <7>[   91.805388] [drm:intel_psr_compute_config [i915]] PSR2 not
> enabled, resolution 3840x2160 > max supported 3640x2304
> 
> It failed because the GEN9 PSR2 HW only supports resolutions up to
> 3640x2304, so it switched back go PSR1 and the test failed because it
> was expecting a PSR2 state not a PSR1.
> 
> I talked with DK, he thinks that the panel should be replaced in that
> machine so we can have some PSR2 test coverage in GEN9.
> 
> If not possible we should add more code in igt_fixture() to check if
> PSR2 is supported but that could potentially hide PSR2 issues behind
> tests skipped or whitelist PSR2 tests in fi-whl-u.

Bypassed with: https://patchwork.freedesktop.org/patch/279874/

Let me know what you guys think about this.

> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev