[igt-dev] [PATCH i-g-t v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[igt-dev] [PATCH i-g-t v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[José Roberto de Souza]

If resolution that will be used in PSR tests is bigger than the
resolution that PSR2 HW supports, PSR2 will not be enabled so those
tests should be skiped. Most of eDP panels only supports one
resolution that is why we don't even try to use other resolution.

BSpec: 7713

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/kms_psr.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/tests/kms_psr.c b/tests/kms_psr.c
index 3e16a6bf..4792e158 100644
--- a/tests/kms_psr.c
+++ b/tests/kms_psr.c
@@ -445,13 +445,30 @@ int main(int argc, char *argv[])
 		igt_require_f(sink_support(&data, PSR_MODE_1),
 			      "Sink does not support PSR\n");
 
-		data.supports_psr2 = sink_support(&data, PSR_MODE_2);
-
 		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);
+
+		data.supports_psr2 = sink_support(&data, PSR_MODE_2);
+		if (data.supports_psr2) {
+			uint32_t devid = intel_get_drm_devid(data.drm_fd);
+			uint16_t max_h = 0, max_v = 0;
+			int gen = intel_gen(devid);
+
+			if (gen >= 10 || IS_GEMINILAKE(devid)) {
+				max_h = 4096;
+				max_v = 2304;
+			} else if (gen == 9) {
+				max_h = 3640;
+				max_v = 2304;
+			}
+
+			if (data.mode->hdisplay > max_h ||
+			    data.mode->vdisplay > max_v)
+				data.supports_psr2 = false;
+		}
 	}
 
 	for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <= PSR_MODE_2;
-- 
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 v4 2/4] 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 v4 3/4] 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

v4:
- getting cairo context for frontbuffer test in prepare()
- droppoing poll(), using blocking timerfd instead

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    | 292 +++++++++++++++++++++++++++++++++++++++++
 tests/meson.build      |   1 +
 5 files changed, 324 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..344ddffe
--- /dev/null
+++ b/tests/kms_psr2_su.c
@@ -0,0 +1,292 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "igt.h"
+#include "igt_sysfs.h"
+#include "igt_psr.h"
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/timerfd.h>
+#include "intel_bufmgr.h"
+
+IGT_TEST_DESCRIPTION("Test PSR2 selective update");
+
+#define SQUARE_SIZE 100
+/* each selective update block is 4 lines tall */
+#define EXPECTED_NUM_SU_BLOCKS ((SQUARE_SIZE / 4) + (SQUARE_SIZE % 4 ? 1 : 0))
+
+/*
+ * Minimum is 15 as the number of frames to active PSR2 could be configured
+ * to 15 frames plus a few more in case we miss a selective update between
+ * debugfs reads.
+ */
+#define MAX_SCREEN_CHANGES 20
+
+enum operations {
+	PAGE_FLIP,
+	FRONTBUFFER,
+	LAST
+};
+
+static const char *op_str(enum operations op)
+{
+	static const char * const name[] = {
+		[PAGE_FLIP] = "page_flip",
+		[FRONTBUFFER] = "frontbuffer"
+	};
+
+	return name[op];
+}
+
+typedef struct {
+	int drm_fd;
+	int debugfs_fd;
+	igt_display_t display;
+	drm_intel_bufmgr *bufmgr;
+	drmModeModeInfo *mode;
+	igt_output_t *output;
+	struct igt_fb fb[2];
+	enum operations op;
+	cairo_t *cr;
+	int change_screen_timerfd;
+	uint32_t screen_changes;
+} data_t;
+
+static void setup_output(data_t *data)
+{
+	igt_display_t *display = &data->display;
+	igt_output_t *output;
+	enum pipe pipe;
+
+	for_each_pipe_with_valid_output(display, pipe, output) {
+		drmModeConnectorPtr c = output->config.connector;
+
+		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
+			continue;
+
+		igt_output_set_pipe(output, pipe);
+		data->output = output;
+		data->mode = igt_output_get_mode(output);
+
+		return;
+	}
+}
+
+static void display_init(data_t *data)
+{
+	igt_display_require(&data->display, data->drm_fd);
+	setup_output(data);
+}
+
+static void display_fini(data_t *data)
+{
+	igt_display_fini(&data->display);
+}
+
+static void prepare(data_t *data)
+{
+	igt_plane_t *primary;
+
+	/* all green frame */
+	igt_create_color_fb(data->drm_fd,
+			    data->mode->hdisplay, data->mode->vdisplay,
+			    DRM_FORMAT_XRGB8888,
+			    LOCAL_DRM_FORMAT_MOD_NONE,
+			    0.0, 1.0, 0.0,
+			    &data->fb[0]);
+
+	if (data->op == PAGE_FLIP) {
+		cairo_t *cr;
+
+		igt_create_color_fb(data->drm_fd,
+				    data->mode->hdisplay, data->mode->vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    LOCAL_DRM_FORMAT_MOD_NONE,
+				    0.0, 1.0, 0.0,
+				    &data->fb[1]);
+
+		cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[1]);
+		/* paint a white square */
+		igt_paint_color_alpha(cr, 0, 0, SQUARE_SIZE, SQUARE_SIZE,
+				      1.0, 1.0, 1.0, 1.0);
+		igt_put_cairo_ctx(data->drm_fd,  &data->fb[1], cr);
+	} else if (data->op == FRONTBUFFER) {
+		data->cr = igt_get_cairo_ctx(data->drm_fd, &data->fb[0]);
+	}
+
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(primary, NULL);
+
+	igt_display_commit(&data->display);
+	igt_plane_set_fb(primary, &data->fb[0]);
+	igt_display_commit(&data->display);
+
+	igt_assert(psr_wait_entry(data->debugfs_fd, PSR_MODE_2));
+
+	data->screen_changes = 0;
+}
+
+static bool update_screen_and_test(data_t *data)
+{
+	uint16_t su_blocks;
+	bool ret = false;
+
+	data->screen_changes++;
+
+	switch (data->op) {
+	case PAGE_FLIP: {
+		igt_plane_t *primary;
+
+		primary = igt_output_get_plane_type(data->output,
+						    DRM_PLANE_TYPE_PRIMARY);
+
+		igt_plane_set_fb(primary, &data->fb[data->screen_changes & 1]);
+		igt_display_commit(&data->display);
+		break;
+	}
+	case FRONTBUFFER: {
+		drmModeClip clip;
+		int r;
+
+		clip.x1 = clip.y1 = 0;
+		clip.x2 = clip.y2 = SQUARE_SIZE;
+
+		if (data->screen_changes & 1) {
+			/* go back to all green frame with with square */
+			igt_paint_color_alpha(data->cr, 0, 0, SQUARE_SIZE,
+					      SQUARE_SIZE, 1.0, 1.0, 1.0, 1.0);
+		} else {
+			/* go back to all green frame */
+			igt_paint_color_alpha(data->cr, 0, 0, SQUARE_SIZE,
+					      SQUARE_SIZE, 0, 1.0, 0, 1.0);
+		}
+
+		r = drmModeDirtyFB(data->drm_fd, data->fb[0].fb_id, &clip, 1);
+		igt_assert(r == 0 || r == -ENOSYS);
+		break;
+	}
+	default:
+		igt_assert_f(data->op, "Operation not handled\n");
+	}
+
+	if (psr2_wait_su(data->debugfs_fd, &su_blocks))
+		ret = su_blocks == EXPECTED_NUM_SU_BLOCKS;
+
+	return ret;
+}
+
+static void run(data_t *data)
+{
+	bool result = false;
+
+	while (data->screen_changes < MAX_SCREEN_CHANGES && !result) {
+		uint64_t exp;
+		int r;
+
+		r = read(data->change_screen_timerfd, &exp, sizeof(exp));
+		if (r == sizeof(uint64_t) && exp)
+			result = update_screen_and_test(data);
+	}
+
+	igt_debug("Screen changes: %u\n", data->screen_changes);
+	igt_assert_f(result,
+		     "No matching selective update blocks read from debugfs\n");
+}
+
+static void cleanup(data_t *data)
+{
+	igt_plane_t *primary;
+
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(primary, NULL);
+	igt_display_commit(&data->display);
+
+	if (data->op == PAGE_FLIP)
+		igt_remove_fb(data->drm_fd, &data->fb[1]);
+	else if (data->op == FRONTBUFFER)
+		igt_put_cairo_ctx(data->drm_fd, &data->fb[0], data->cr);
+
+	igt_remove_fb(data->drm_fd, &data->fb[0]);
+}
+
+int main(int argc, char *argv[])
+{
+	data_t data = {};
+
+	igt_subtest_init_parse_opts(&argc, argv, "", NULL,
+				    NULL, NULL, NULL);
+	igt_skip_on_simulation();
+
+	igt_fixture {
+		struct itimerspec interval;
+		int r;
+
+		data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+		data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
+		kmstest_set_vt_graphics_mode();
+
+		igt_require_f(psr_sink_support(data.debugfs_fd, PSR_MODE_2),
+			      "Sink does not support PSR2\n");
+
+		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
+		igt_assert(data.bufmgr);
+		drm_intel_bufmgr_gem_enable_reuse(data.bufmgr);
+
+		display_init(&data);
+
+		igt_require(psr_enable(data.debugfs_fd, PSR_MODE_2));
+		igt_require(psr_wait_entry(data.debugfs_fd, PSR_MODE_2));
+
+		/* blocking timerfd */
+		data.change_screen_timerfd = timerfd_create(CLOCK_MONOTONIC, 0);
+		igt_require(data.change_screen_timerfd != -1);
+		/* Changing screen at 30hz to support 30hz panels */
+		interval.it_value.tv_nsec = NSEC_PER_SEC / 30;
+		interval.it_value.tv_sec = 0;
+		interval.it_interval.tv_nsec = interval.it_value.tv_nsec;
+		interval.it_interval.tv_sec = interval.it_value.tv_sec;
+		r = timerfd_settime(data.change_screen_timerfd, 0, &interval, NULL);
+		igt_require_f(r != -1, "Error setting timerfd\n");
+	}
+
+	for (data.op = PAGE_FLIP; data.op < LAST; data.op++) {
+		igt_subtest_f("%s", op_str(data.op)) {
+			prepare(&data);
+			run(&data);
+			cleanup(&data);
+		}
+	}
+
+	igt_fixture {
+		close(data.debugfs_fd);
+		drm_intel_bufmgr_destroy(data.bufmgr);
+		display_fini(&data);
+	}
+
+	igt_exit();
+}
diff --git a/tests/meson.build b/tests/meson.build
index e14ab2b4..682ca939 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -50,6 +50,7 @@ test_progs = [
 	'kms_plane_scaling',
 	'kms_properties',
 	'kms_psr',
+	'kms_psr2_su',
 	'kms_pwrite_crc',
 	'kms_rmfb',
 	'kms_rotation_crc',
-- 
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 v4 4/4] DO NOT MERGE: Check result of kms_psr2_su tests

[José Roberto de Souza]

This is a hack patch just to check the result of kms_psr2_su tests in
CI.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 tests/intel-ci/fast-feedback.testlist | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index e48cb8a5..eb96a67f 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -231,6 +231,8 @@ 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_psr2_su@page_flip
+igt@kms_psr2_su@frontbuffer
 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] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,v4,1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v4,1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports
URL   : https://patchwork.freedesktop.org/series/55659/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5471 -> IGTPW_2286
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2286 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2286, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@pm_rpm@module-reload:
    - fi-skl-6770hq:      PASS -> FAIL

  
#### Suppressed ####

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

  * {igt@kms_psr2_su@frontbuffer}:
    - {fi-icl-u2}:        NOTRUN -> {SKIP} +1

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

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

### IGT changes ###

#### Issues hit ####

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

  
#### Possible fixes ####

  * igt@kms_busy@basic-flip-c:
    - fi-kbl-7500u:       {SKIP} [fdo#109271] / [fdo#109278] -> PASS +2

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-kbl-7500u:       DMESG-WARN [fdo#102505] / [fdo#103558] / [fdo#105602] -> PASS

  * 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

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

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

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (46 -> 40)
------------------------------

  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-icl-y fi-bdw-samus 


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

    * IGT: IGT_4786 -> IGTPW_2286

  CI_DRM_5471: 198addb18e12d2469bc41d57f9ed63e1072a7f82 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2286: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2286/
  IGT_4786: 85cf76182087c09604bcae2bbee9e58b33bcb4f2 @ 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_2286/
_______________________________________________
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,v4,1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v4,1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports (rev2)
URL   : https://patchwork.freedesktop.org/series/55659/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5472 -> IGTPW_2287
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55659/revisions/2/mbox/

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

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

### IGT changes ###

#### Suppressed ####

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

  * {igt@kms_psr2_su@frontbuffer}:
    - {fi-icl-u2}:        NOTRUN -> {SKIP} +1

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-byt-clapper:     PASS -> INCOMPLETE [fdo#102657]

  * igt@i915_selftest@live_coherency:
    - fi-gdg-551:         PASS -> DMESG-FAIL [fdo#107164]

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       PASS -> WARN [fdo#109380]

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

  
#### Possible fixes ####

  * igt@i915_selftest@live_hangcheck:
    - {fi-icl-u3}:        INCOMPLETE [fdo#108569] / [fdo#109226] -> PASS

  * igt@pm_rpm@basic-pci-d3-state:
    - fi-byt-j1900:       {SKIP} [fdo#109271] -> PASS

  * igt@pm_rpm@basic-rte:
    - fi-byt-j1900:       FAIL [fdo#108800] -> PASS

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

  [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
  [fdo#107164]: https://bugs.freedesktop.org/show_bug.cgi?id=107164
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109226]: https://bugs.freedesktop.org/show_bug.cgi?id=109226
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380


Participating hosts (45 -> 41)
------------------------------

  Missing    (4): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


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

    * IGT: IGT_4786 -> IGTPW_2287

  CI_DRM_5472: c7fd80b9e4d5e789377f0f4a52a889c589226064 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2287: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2287/
  IGT_4786: 85cf76182087c09604bcae2bbee9e58b33bcb4f2 @ 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_2287/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[Dhinakaran Pandiyan]

On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza wrote:
> If resolution that will be used in PSR tests is bigger than the
> resolution that PSR2 HW supports, PSR2 will not be enabled so those
> tests should be skiped. Most of eDP panels only supports one
> resolution that is why we don't even try to use other resolution.
> 
> BSpec: 7713
I don't like the fact that we are re-implementing driver code in the
test to avoid false-positives. We should really replace the panel on
the WHL machine with a panel that is expected to work. 


But, as a temporary measure to make CI happy and enable PSR2.
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Rodrigo, what's your opinion on this?

-DK
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  tests/kms_psr.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> index 3e16a6bf..4792e158 100644
> --- a/tests/kms_psr.c
> +++ b/tests/kms_psr.c
> @@ -445,13 +445,30 @@ int main(int argc, char *argv[])
>  		igt_require_f(sink_support(&data, PSR_MODE_1),
>  			      "Sink does not support PSR\n");
>  
> -		data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> -
>  		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);
> +
> +		data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> +		if (data.supports_psr2) {
> +			uint32_t devid =
> intel_get_drm_devid(data.drm_fd);
> +			uint16_t max_h = 0, max_v = 0;
> +			int gen = intel_gen(devid);
> +
> +			if (gen >= 10 || IS_GEMINILAKE(devid)) {
> +				max_h = 4096;
> +				max_v = 2304;
> +			} else if (gen == 9) {
> +				max_h = 3640;
> +				max_v = 2304;
> +			}
> +
> +			if (data.mode->hdisplay > max_h ||
> +			    data.mode->vdisplay > max_v)
> +				data.supports_psr2 = false;
> +		}
>  	}
>  
>  	for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <=
> PSR_MODE_2;

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

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

[Dhinakaran Pandiyan]

On Wed, 2019-01-23 at 15:56 -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>


If these tests pass on all CI machines,
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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

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

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

[Dhinakaran Pandiyan]

On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza wrote:
> This tests checks if hardware is able to do selective update when
> screen changes.
> PSR2 don't trigger interruptions and the 'PSR2 SU status' register
> is not kept loaded all the times, so it is necessary keep polling
> PSR status debugfs until those values are loaded.
> 
> Also from DEEP_SLEEP state HW will not do a seletive update, as
> most of the memory/context is lost in deep sleep state hardware will
> need to exit PSR mode then wait a configured number of frames to
> activate PSR again to then start doing seletive updates, that is why
> just one screen change is not enough to pass this tests.
> 
> When a selective update happens and the values are loaded and read
> from debugfs it is compared with the expected value of seletive
> update blocks, if matches the polling is stopped and the test passed
> otherwise it will wait until it reachs a maximum number o screen
> changes to fail the test.
> 
> v2: Using new SU blocks debugfs output
> 
> v3:
> - removed the timerfd to fail the test, now failing based in a
> maximum number of screen changes
> - removing thread to read debugfs, read from main thread is enough
> - improved commit message
> 
> v4:
> - getting cairo context for frontbuffer test in prepare()
> - droppoing poll(), using blocking timerfd instead
> 
> 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    | 292
> +++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build      |   1 +
>  5 files changed, 324 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..344ddffe
> --- /dev/null
> +++ b/tests/kms_psr2_su.c
> @@ -0,0 +1,292 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person
> obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> the
> + * Software is furnished to do so, subject to the following
> conditions:
> + *
> + * The above copyright notice and this permission notice (including
> the next
> + * paragraph) shall be included in all copies or substantial
> portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "igt.h"
> +#include "igt_sysfs.h"
> +#include "igt_psr.h"
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/timerfd.h>
> +#include "intel_bufmgr.h"
> +
> +IGT_TEST_DESCRIPTION("Test PSR2 selective update");
> +
> +#define SQUARE_SIZE 100
> +/* each selective update block is 4 lines tall */
> +#define EXPECTED_NUM_SU_BLOCKS ((SQUARE_SIZE / 4) + (SQUARE_SIZE % 4
> ? 1 : 0))
> +
> +/*
> + * Minimum is 15 as the number of frames to active PSR2 could be
> configured
> + * to 15 frames plus a few more in case we miss a selective update
> between
> + * debugfs reads.
> + */
> +#define MAX_SCREEN_CHANGES 20
> +
> +enum operations {
> +	PAGE_FLIP,
> +	FRONTBUFFER,
> +	LAST
> +};
> +
> +static const char *op_str(enum operations op)
> +{
> +	static const char * const name[] = {
> +		[PAGE_FLIP] = "page_flip",
> +		[FRONTBUFFER] = "frontbuffer"
> +	};
> +
> +	return name[op];
> +}
> +
> +typedef struct {
> +	int drm_fd;
> +	int debugfs_fd;
> +	igt_display_t display;
> +	drm_intel_bufmgr *bufmgr;
> +	drmModeModeInfo *mode;
> +	igt_output_t *output;
> +	struct igt_fb fb[2];
> +	enum operations op;
> +	cairo_t *cr;
> +	int change_screen_timerfd;
> +	uint32_t screen_changes;
> +} data_t;
> +
> +static void setup_output(data_t *data)
> +{
> +	igt_display_t *display = &data->display;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +
> +	for_each_pipe_with_valid_output(display, pipe, output) {
> +		drmModeConnectorPtr c = output->config.connector;
> +
> +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> +			continue;
> +
> +		igt_output_set_pipe(output, pipe);
> +		data->output = output;
> +		data->mode = igt_output_get_mode(output);

Won't this fail the same way as other PSR2 tests on WHL because of the
default mode exceeding PSR2 capability?

_______________________________________________
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,v4,1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,v4,1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports (rev2)
URL   : https://patchwork.freedesktop.org/series/55659/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5472_full -> IGTPW_2287_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55659/revisions/2/mbox/

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

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

### IGT changes ###

#### Issues hit ####

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

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

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#107956]

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

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#108566]

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-glk:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-dpms:
    - shard-kbl:          PASS -> FAIL [fdo#103232] +2

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

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

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-glk:          NOTRUN -> FAIL [fdo#103166]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145]

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

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

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-kbl:          PASS -> FAIL [fdo#103166] +1

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

  
#### Possible fixes ####

  * igt@gem_pwrite@big-cpu-backwards:
    - shard-snb:          INCOMPLETE [fdo#105411] / [fdo#107469] -> PASS

  * igt@kms_color@pipe-b-degamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +8

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-kbl:          FAIL [fdo#103232] -> PASS +1

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

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

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_plane_lowres@pipe-a-tiling-none:
    - shard-snb:          {SKIP} [fdo#109271] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-none:
    - shard-kbl:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-apl:          FAIL [fdo#103166] -> PASS +4

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

  
#### Warnings ####

  * igt@i915_suspend@shrink:
    - shard-glk:          DMESG-WARN [fdo#109244] -> INCOMPLETE [fdo#103359] / [fdo#106886] / [k.org#198133]

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

  
  {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#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [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#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [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#108337]: https://bugs.freedesktop.org/show_bug.cgi?id=108337
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  Missing    (2): shard-skl shard-iclb 


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

    * IGT: IGT_4786 -> IGTPW_2287
    * Piglit: piglit_4509 -> None

  CI_DRM_5472: c7fd80b9e4d5e789377f0f4a52a889c589226064 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2287: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2287/
  IGT_4786: 85cf76182087c09604bcae2bbee9e58b33bcb4f2 @ 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_2287/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[Rodrigo Vivi]

On Wed, Jan 23, 2019 at 06:25:07PM -0800, Dhinakaran Pandiyan wrote:
> On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza wrote:
> > If resolution that will be used in PSR tests is bigger than the
> > resolution that PSR2 HW supports, PSR2 will not be enabled so those
> > tests should be skiped. Most of eDP panels only supports one
> > resolution that is why we don't even try to use other resolution.
> > 
> > BSpec: 7713
> I don't like the fact that we are re-implementing driver code in the
> test to avoid false-positives. We should really replace the panel on
> the WHL machine with a panel that is expected to work. 
>

I fully agree. The debugfs should be auto-suficient.

In the past it was the idea of Source OK. if sink support yes and source ok
and we tried to enabled but it is not getting active,
than we have a bug somewhere.

I don't know the current status of the psr debugfs anymore, but
I believe the test case should infer this information from there
somehow instead of re-implement the driver code here.

> 
> But, as a temporary measure to make CI happy and enable PSR2.

why is this only affecting PSR2 case?

> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> Rodrigo, what's your opinion on this?
> 
> -DK
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  tests/kms_psr.c | 21 +++++++++++++++++++--
> >  1 file changed, 19 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > index 3e16a6bf..4792e158 100644
> > --- a/tests/kms_psr.c
> > +++ b/tests/kms_psr.c
> > @@ -445,13 +445,30 @@ int main(int argc, char *argv[])
> >  		igt_require_f(sink_support(&data, PSR_MODE_1),
> >  			      "Sink does not support PSR\n");
> >  
> > -		data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> > -
> >  		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);
> > +
> > +		data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> > +		if (data.supports_psr2) {
> > +			uint32_t devid =
> > intel_get_drm_devid(data.drm_fd);
> > +			uint16_t max_h = 0, max_v = 0;
> > +			int gen = intel_gen(devid);
> > +
> > +			if (gen >= 10 || IS_GEMINILAKE(devid)) {
> > +				max_h = 4096;
> > +				max_v = 2304;
> > +			} else if (gen == 9) {
> > +				max_h = 3640;
> > +				max_v = 2304;
> > +			}
> > +
> > +			if (data.mode->hdisplay > max_h ||
> > +			    data.mode->vdisplay > max_v)
> > +				data.supports_psr2 = false;
> > +		}
> >  	}
> >  
> >  	for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <=
> > PSR_MODE_2;
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[Souza, Jose]

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

On Wed, 2019-01-23 at 22:39 -0800, Rodrigo Vivi wrote:
> On Wed, Jan 23, 2019 at 06:25:07PM -0800, Dhinakaran Pandiyan wrote:
> > On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza wrote:
> > > If resolution that will be used in PSR tests is bigger than the
> > > resolution that PSR2 HW supports, PSR2 will not be enabled so
> > > those
> > > tests should be skiped. Most of eDP panels only supports one
> > > resolution that is why we don't even try to use other resolution.
> > > 
> > > BSpec: 7713
> > I don't like the fact that we are re-implementing driver code in
> > the
> > test to avoid false-positives. We should really replace the panel
> > on
> > the WHL machine with a panel that is expected to work. 
> > 
> 
> I fully agree. The debugfs should be auto-suficient.
> 
> In the past it was the idea of Source OK. if sink support yes and
> source ok
> and we tried to enabled but it is not getting active,
> than we have a bug somewhere.
> 
> I don't know the current status of the psr debugfs anymore, but
> I believe the test case should infer this information from there
> somehow instead of re-implement the driver code here.
> 
> > But, as a temporary measure to make CI happy and enable PSR2.
> 
> why is this only affecting PSR2 case?

PSR2 has a maximum resolution suported and the only resolution that CI
eDP panel supports is bigger than that.

> 
> > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > 
> > Rodrigo, what's your opinion on this?
> > 
> > -DK
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  tests/kms_psr.c | 21 +++++++++++++++++++--
> > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > index 3e16a6bf..4792e158 100644
> > > --- a/tests/kms_psr.c
> > > +++ b/tests/kms_psr.c
> > > @@ -445,13 +445,30 @@ int main(int argc, char *argv[])
> > >  		igt_require_f(sink_support(&data, PSR_MODE_1),
> > >  			      "Sink does not support PSR\n");
> > >  
> > > -		data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> > > -
> > >  		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);
> > > +
> > > +		data.supports_psr2 = sink_support(&data, PSR_MODE_2);
> > > +		if (data.supports_psr2) {
> > > +			uint32_t devid =
> > > intel_get_drm_devid(data.drm_fd);
> > > +			uint16_t max_h = 0, max_v = 0;
> > > +			int gen = intel_gen(devid);
> > > +
> > > +			if (gen >= 10 || IS_GEMINILAKE(devid)) {
> > > +				max_h = 4096;
> > > +				max_v = 2304;
> > > +			} else if (gen == 9) {
> > > +				max_h = 3640;
> > > +				max_v = 2304;
> > > +			}
> > > +
> > > +			if (data.mode->hdisplay > max_h ||
> > > +			    data.mode->vdisplay > max_v)
> > > +				data.supports_psr2 = false;
> > > +		}
> > >  	}
> > >  
> > >  	for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <=
> > > PSR_MODE_2;

[-- 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 v4 3/4] test: Add PSR2 selective update tests

[Souza, Jose]

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

On Wed, 2019-01-23 at 18:33 -0800, Dhinakaran Pandiyan wrote:
> On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza wrote:
> > This tests checks if hardware is able to do selective update when
> > screen changes.
> > PSR2 don't trigger interruptions and the 'PSR2 SU status' register
> > is not kept loaded all the times, so it is necessary keep polling
> > PSR status debugfs until those values are loaded.
> > 
> > Also from DEEP_SLEEP state HW will not do a seletive update, as
> > most of the memory/context is lost in deep sleep state hardware
> > will
> > need to exit PSR mode then wait a configured number of frames to
> > activate PSR again to then start doing seletive updates, that is
> > why
> > just one screen change is not enough to pass this tests.
> > 
> > When a selective update happens and the values are loaded and read
> > from debugfs it is compared with the expected value of seletive
> > update blocks, if matches the polling is stopped and the test
> > passed
> > otherwise it will wait until it reachs a maximum number o screen
> > changes to fail the test.
> > 
> > v2: Using new SU blocks debugfs output
> > 
> > v3:
> > - removed the timerfd to fail the test, now failing based in a
> > maximum number of screen changes
> > - removing thread to read debugfs, read from main thread is enough
> > - improved commit message
> > 
> > v4:
> > - getting cairo context for frontbuffer test in prepare()
> > - droppoing poll(), using blocking timerfd instead
> > 
> > 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    | 292
> > +++++++++++++++++++++++++++++++++++++++++
> >  tests/meson.build      |   1 +
> >  5 files changed, 324 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..344ddffe
> > --- /dev/null
> > +++ b/tests/kms_psr2_su.c
> > @@ -0,0 +1,292 @@
> > +/*
> > + * Copyright © 2019 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> > sublicense,
> > + * and/or sell copies of the Software, and to permit persons to
> > whom
> > the
> > + * Software is furnished to do so, subject to the following
> > conditions:
> > + *
> > + * The above copyright notice and this permission notice
> > (including
> > the next
> > + * paragraph) shall be included in all copies or substantial
> > portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> > EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > DAMAGES
> > OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + */
> > +
> > +#include "igt.h"
> > +#include "igt_sysfs.h"
> > +#include "igt_psr.h"
> > +#include <errno.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <sys/timerfd.h>
> > +#include "intel_bufmgr.h"
> > +
> > +IGT_TEST_DESCRIPTION("Test PSR2 selective update");
> > +
> > +#define SQUARE_SIZE 100
> > +/* each selective update block is 4 lines tall */
> > +#define EXPECTED_NUM_SU_BLOCKS ((SQUARE_SIZE / 4) + (SQUARE_SIZE %
> > 4
> > ? 1 : 0))
> > +
> > +/*
> > + * Minimum is 15 as the number of frames to active PSR2 could be
> > configured
> > + * to 15 frames plus a few more in case we miss a selective update
> > between
> > + * debugfs reads.
> > + */
> > +#define MAX_SCREEN_CHANGES 20
> > +
> > +enum operations {
> > +	PAGE_FLIP,
> > +	FRONTBUFFER,
> > +	LAST
> > +};
> > +
> > +static const char *op_str(enum operations op)
> > +{
> > +	static const char * const name[] = {
> > +		[PAGE_FLIP] = "page_flip",
> > +		[FRONTBUFFER] = "frontbuffer"
> > +	};
> > +
> > +	return name[op];
> > +}
> > +
> > +typedef struct {
> > +	int drm_fd;
> > +	int debugfs_fd;
> > +	igt_display_t display;
> > +	drm_intel_bufmgr *bufmgr;
> > +	drmModeModeInfo *mode;
> > +	igt_output_t *output;
> > +	struct igt_fb fb[2];
> > +	enum operations op;
> > +	cairo_t *cr;
> > +	int change_screen_timerfd;
> > +	uint32_t screen_changes;
> > +} data_t;
> > +
> > +static void setup_output(data_t *data)
> > +{
> > +	igt_display_t *display = &data->display;
> > +	igt_output_t *output;
> > +	enum pipe pipe;
> > +
> > +	for_each_pipe_with_valid_output(display, pipe, output) {
> > +		drmModeConnectorPtr c = output->config.connector;
> > +
> > +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> > +			continue;
> > +
> > +		igt_output_set_pipe(output, pipe);
> > +		data->output = output;
> > +		data->mode = igt_output_get_mode(output);
> 
> Won't this fail the same way as other PSR2 tests on WHL because of
> the
> default mode exceeding PSR2 capability?

As the goal of this test is not to test if PSR2 can be enabled, on
igt_fixture() we have:

igt_require(psr_enable(data.debugfs_fd, PSR_MODE_2));
igt_require(psr_wait_entry(data.debugfs_fd, PSR_MODE_2));

So this test will be skiped and the error will be reported by
kms_psr@psr2_basic and other tests in kms_psr.

> 

[-- 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 v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[Dhinakaran Pandiyan]

On Thu, 2019-01-24 at 10:20 -0800, Souza, Jose wrote:
> On Wed, 2019-01-23 at 22:39 -0800, Rodrigo Vivi wrote:
> > On Wed, Jan 23, 2019 at 06:25:07PM -0800, Dhinakaran Pandiyan
> > wrote:
> > > On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza wrote:
> > > > If resolution that will be used in PSR tests is bigger than the
> > > > resolution that PSR2 HW supports, PSR2 will not be enabled so
> > > > those
> > > > tests should be skiped. Most of eDP panels only supports one
> > > > resolution that is why we don't even try to use other
> > > > resolution.
> > > > 
> > > > BSpec: 7713
> > > 
> > > I don't like the fact that we are re-implementing driver code in
> > > the
> > > test to avoid false-positives. We should really replace the panel
> > > on
> > > the WHL machine with a panel that is expected to work. 
> > > 
> > 
> > I fully agree. The debugfs should be auto-suficient.
> > 
> > In the past it was the idea of Source OK. if sink support yes and
> > source ok
That had a problem too, "Source OK" was set after driver completed all
the checks and just before enabling PSR2. In effect, it started
becoming a duplicate of the "Enabled" field and we weren't able to
catch if the driver checks before psr_enable() were wrong.


> > and we tried to enabled but it is not getting active,
> > than we have a bug somewhere.
> > 
> > I don't know the current status of the psr debugfs anymore, but
> > I believe the test case should infer this information from there
> > somehow instead of re-implement the driver code here.
> > 
> > > But, as a temporary measure to make CI happy and enable PSR2.
> > 
> > why is this only affecting PSR2 case?
> 
> PSR2 has a maximum resolution suported and the only resolution that
> CI
> eDP panel supports is bigger than that.
> 
> > 
> > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > 
> > > Rodrigo, what's your opinion on this?
> > > 
> > > -DK
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  tests/kms_psr.c | 21 +++++++++++++++++++--
> > > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > > index 3e16a6bf..4792e158 100644
> > > > --- a/tests/kms_psr.c
> > > > +++ b/tests/kms_psr.c
> > > > @@ -445,13 +445,30 @@ int main(int argc, char *argv[])
> > > >  		igt_require_f(sink_support(&data, PSR_MODE_1),
> > > >  			      "Sink does not support PSR\n");
> > > >  
> > > > -		data.supports_psr2 = sink_support(&data,
> > > > PSR_MODE_2);
> > > > -
> > > >  		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);
> > > > +
> > > > +		data.supports_psr2 = sink_support(&data,
> > > > PSR_MODE_2);
> > > > +		if (data.supports_psr2) {
> > > > +			uint32_t devid =
> > > > intel_get_drm_devid(data.drm_fd);
> > > > +			uint16_t max_h = 0, max_v = 0;
> > > > +			int gen = intel_gen(devid);
> > > > +
> > > > +			if (gen >= 10 || IS_GEMINILAKE(devid))
> > > > {
> > > > +				max_h = 4096;
> > > > +				max_v = 2304;
> > > > +			} else if (gen == 9) {
> > > > +				max_h = 3640;
> > > > +				max_v = 2304;
> > > > +			}
> > > > +
> > > > +			if (data.mode->hdisplay > max_h ||
> > > > +			    data.mode->vdisplay > max_v)
> > > > +				data.supports_psr2 = false;
> > > > +		}
> > > >  	}
> > > >  
> > > >  	for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <=
> > > > PSR_MODE_2;

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

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

[Dhinakaran Pandiyan]

On Thu, 2019-01-24 at 10:27 -0800, Souza, Jose wrote:
> On Wed, 2019-01-23 at 18:33 -0800, Dhinakaran Pandiyan wrote:
> > On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza wrote:
> > > This tests checks if hardware is able to do selective update when
> > > screen changes.
> > > PSR2 don't trigger interruptions and the 'PSR2 SU status'
> > > register
> > > is not kept loaded all the times, so it is necessary keep polling
> > > PSR status debugfs until those values are loaded.
> > > 
> > > Also from DEEP_SLEEP state HW will not do a seletive update, as
> > > most of the memory/context is lost in deep sleep state hardware
> > > will
> > > need to exit PSR mode then wait a configured number of frames to
> > > activate PSR again to then start doing seletive updates, that is
> > > why
> > > just one screen change is not enough to pass this tests.
> > > 
> > > When a selective update happens and the values are loaded and
> > > read
> > > from debugfs it is compared with the expected value of seletive
> > > update blocks, if matches the polling is stopped and the test
> > > passed
> > > otherwise it will wait until it reachs a maximum number o screen
> > > changes to fail the test.
> > > 
> > > v2: Using new SU blocks debugfs output
> > > 
> > > v3:
> > > - removed the timerfd to fail the test, now failing based in a
> > > maximum number of screen changes
> > > - removing thread to read debugfs, read from main thread is
> > > enough
> > > - improved commit message
> > > 
> > > v4:
> > > - getting cairo context for frontbuffer test in prepare()
> > > - droppoing poll(), using blocking timerfd instead
> > > 
> > > 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    | 292
> > > +++++++++++++++++++++++++++++++++++++++++
> > >  tests/meson.build      |   1 +
> > >  5 files changed, 324 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..344ddffe
> > > --- /dev/null
> > > +++ b/tests/kms_psr2_su.c
> > > @@ -0,0 +1,292 @@
> > > +/*
> > > + * Copyright © 2019 Intel Corporation
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person
> > > obtaining a
> > > + * copy of this software and associated documentation files (the
> > > "Software"),
> > > + * to deal in the Software without restriction, including
> > > without
> > > limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to
> > > whom
> > > the
> > > + * Software is furnished to do so, subject to the following
> > > conditions:
> > > + *
> > > + * The above copyright notice and this permission notice
> > > (including
> > > the next
> > > + * paragraph) shall be included in all copies or substantial
> > > portions of the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> > > KIND,
> > > EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> > > EVENT SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > > DAMAGES
> > > OR OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> > > OTHERWISE,
> > > ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > > OTHER DEALINGS
> > > + * IN THE SOFTWARE.
> > > + *
> > > + */
> > > +
> > > +#include "igt.h"
> > > +#include "igt_sysfs.h"
> > > +#include "igt_psr.h"
> > > +#include <errno.h>
> > > +#include <stdbool.h>
> > > +#include <stdio.h>
> > > +#include <string.h>
> > > +#include <sys/timerfd.h>
> > > +#include "intel_bufmgr.h"
> > > +
> > > +IGT_TEST_DESCRIPTION("Test PSR2 selective update");
> > > +
> > > +#define SQUARE_SIZE 100
> > > +/* each selective update block is 4 lines tall */
> > > +#define EXPECTED_NUM_SU_BLOCKS ((SQUARE_SIZE / 4) + (SQUARE_SIZE
> > > %
> > > 4
> > > ? 1 : 0))
> > > +
> > > +/*
> > > + * Minimum is 15 as the number of frames to active PSR2 could be
> > > configured
> > > + * to 15 frames plus a few more in case we miss a selective
> > > update
> > > between
> > > + * debugfs reads.
> > > + */
> > > +#define MAX_SCREEN_CHANGES 20
> > > +
> > > +enum operations {
> > > +	PAGE_FLIP,
> > > +	FRONTBUFFER,
> > > +	LAST
> > > +};
> > > +
> > > +static const char *op_str(enum operations op)
> > > +{
> > > +	static const char * const name[] = {
> > > +		[PAGE_FLIP] = "page_flip",
> > > +		[FRONTBUFFER] = "frontbuffer"
> > > +	};
> > > +
> > > +	return name[op];
> > > +}
> > > +
> > > +typedef struct {
> > > +	int drm_fd;
> > > +	int debugfs_fd;
> > > +	igt_display_t display;
> > > +	drm_intel_bufmgr *bufmgr;
> > > +	drmModeModeInfo *mode;
> > > +	igt_output_t *output;
> > > +	struct igt_fb fb[2];
> > > +	enum operations op;
> > > +	cairo_t *cr;
> > > +	int change_screen_timerfd;
> > > +	uint32_t screen_changes;
> > > +} data_t;
> > > +
> > > +static void setup_output(data_t *data)
> > > +{
> > > +	igt_display_t *display = &data->display;
> > > +	igt_output_t *output;
> > > +	enum pipe pipe;
> > > +
> > > +	for_each_pipe_with_valid_output(display, pipe, output) {
> > > +		drmModeConnectorPtr c = output->config.connector;
> > > +
> > > +		if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> > > +			continue;
> > > +
> > > +		igt_output_set_pipe(output, pipe);
> > > +		data->output = output;
> > > +		data->mode = igt_output_get_mode(output);
> > 
> > Won't this fail the same way as other PSR2 tests on WHL because of
> > the
> > default mode exceeding PSR2 capability?
> 
> As the goal of this test is not to test if PSR2 can be enabled, on
> igt_fixture() we have:
> 
> igt_require(psr_enable(data.debugfs_fd, PSR_MODE_2));
> igt_require(psr_wait_entry(data.debugfs_fd, PSR_MODE_2));
psr_wait_entry() is the one that would return false, isn't it?

> 
> So this test will be skiped and the error will be reported by
> kms_psr@psr2_basic and other tests in kms_psr.
> 
Fair enough.

I was about add a Reviewed-by but then noticed that the test skipped on
one of the ICL. Do you know what's happening?

> > 

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

Re: [igt-dev] [PATCH i-g-t v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[Rodrigo Vivi]

On Thu, Jan 24, 2019 at 11:02:51AM -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-01-24 at 10:20 -0800, Souza, Jose wrote:
> > On Wed, 2019-01-23 at 22:39 -0800, Rodrigo Vivi wrote:
> > > On Wed, Jan 23, 2019 at 06:25:07PM -0800, Dhinakaran Pandiyan
> > > wrote:
> > > > On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza wrote:
> > > > > If resolution that will be used in PSR tests is bigger than the
> > > > > resolution that PSR2 HW supports, PSR2 will not be enabled so
> > > > > those
> > > > > tests should be skiped. Most of eDP panels only supports one
> > > > > resolution that is why we don't even try to use other
> > > > > resolution.
> > > > > 
> > > > > BSpec: 7713
> > > > 
> > > > I don't like the fact that we are re-implementing driver code in
> > > > the
> > > > test to avoid false-positives. We should really replace the panel
> > > > on
> > > > the WHL machine with a panel that is expected to work. 
> > > > 
> > > 
> > > I fully agree. The debugfs should be auto-suficient.
> > > 
> > > In the past it was the idea of Source OK. if sink support yes and
> > > source ok
> That had a problem too, "Source OK" was set after driver completed all
> the checks and just before enabling PSR2. In effect, it started
> becoming a duplicate of the "Enabled" field and we weren't able to
> catch if the driver checks before psr_enable() were wrong.

yeap, this is true indeed...

But we need to find a way where the debugfs tells us if
conditions are not match and enable is not expected to
avoid having to reimplement all the conditions twice.

> 
> 
> > > and we tried to enabled but it is not getting active,
> > > than we have a bug somewhere.
> > > 
> > > I don't know the current status of the psr debugfs anymore, but
> > > I believe the test case should infer this information from there
> > > somehow instead of re-implement the driver code here.
> > > 
> > > > But, as a temporary measure to make CI happy and enable PSR2.
> > > 
> > > why is this only affecting PSR2 case?
> > 
> > PSR2 has a maximum resolution suported and the only resolution that
> > CI
> > eDP panel supports is bigger than that.
> > 
> > > 
> > > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > 
> > > > Rodrigo, what's your opinion on this?
> > > > 
> > > > -DK
> > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > ---
> > > > >  tests/kms_psr.c | 21 +++++++++++++++++++--
> > > > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > > > index 3e16a6bf..4792e158 100644
> > > > > --- a/tests/kms_psr.c
> > > > > +++ b/tests/kms_psr.c
> > > > > @@ -445,13 +445,30 @@ int main(int argc, char *argv[])
> > > > >  		igt_require_f(sink_support(&data, PSR_MODE_1),
> > > > >  			      "Sink does not support PSR\n");
> > > > >  
> > > > > -		data.supports_psr2 = sink_support(&data,
> > > > > PSR_MODE_2);
> > > > > -
> > > > >  		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);
> > > > > +
> > > > > +		data.supports_psr2 = sink_support(&data,
> > > > > PSR_MODE_2);
> > > > > +		if (data.supports_psr2) {
> > > > > +			uint32_t devid =
> > > > > intel_get_drm_devid(data.drm_fd);
> > > > > +			uint16_t max_h = 0, max_v = 0;
> > > > > +			int gen = intel_gen(devid);
> > > > > +
> > > > > +			if (gen >= 10 || IS_GEMINILAKE(devid))
> > > > > {
> > > > > +				max_h = 4096;
> > > > > +				max_v = 2304;
> > > > > +			} else if (gen == 9) {
> > > > > +				max_h = 3640;
> > > > > +				max_v = 2304;
> > > > > +			}
> > > > > +
> > > > > +			if (data.mode->hdisplay > max_h ||
> > > > > +			    data.mode->vdisplay > max_v)
> > > > > +				data.supports_psr2 = false;
> > > > > +		}
> > > > >  	}
> > > > >  
> > > > >  	for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <=
> > > > > PSR_MODE_2;
> 
> _______________________________________________
> 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 v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[Souza, Jose]

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

On Thu, 2019-01-24 at 14:06 -0800, Rodrigo Vivi wrote:
> On Thu, Jan 24, 2019 at 11:02:51AM -0800, Dhinakaran Pandiyan wrote:
> > On Thu, 2019-01-24 at 10:20 -0800, Souza, Jose wrote:
> > > On Wed, 2019-01-23 at 22:39 -0800, Rodrigo Vivi wrote:
> > > > On Wed, Jan 23, 2019 at 06:25:07PM -0800, Dhinakaran Pandiyan
> > > > wrote:
> > > > > On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza
> > > > > wrote:
> > > > > > If resolution that will be used in PSR tests is bigger than
> > > > > > the
> > > > > > resolution that PSR2 HW supports, PSR2 will not be enabled
> > > > > > so
> > > > > > those
> > > > > > tests should be skiped. Most of eDP panels only supports
> > > > > > one
> > > > > > resolution that is why we don't even try to use other
> > > > > > resolution.
> > > > > > 
> > > > > > BSpec: 7713
> > > > > 
> > > > > I don't like the fact that we are re-implementing driver code
> > > > > in
> > > > > the
> > > > > test to avoid false-positives. We should really replace the
> > > > > panel
> > > > > on
> > > > > the WHL machine with a panel that is expected to work. 
> > > > > 
> > > > 
> > > > I fully agree. The debugfs should be auto-suficient.
> > > > 
> > > > In the past it was the idea of Source OK. if sink support yes
> > > > and
> > > > source ok
> > That had a problem too, "Source OK" was set after driver completed
> > all
> > the checks and just before enabling PSR2. In effect, it started
> > becoming a duplicate of the "Enabled" field and we weren't able to
> > catch if the driver checks before psr_enable() were wrong.
> 
> yeap, this is true indeed...
> 
> But we need to find a way where the debugfs tells us if
> conditions are not match and enable is not expected to
> avoid having to reimplement all the conditions twice.

I could add a reason field to debugfs and like we have for FBC(fbc-
>no_fbc_reason)

What do you think?

> 
> > 
> > > > and we tried to enabled but it is not getting active,
> > > > than we have a bug somewhere.
> > > > 
> > > > I don't know the current status of the psr debugfs anymore, but
> > > > I believe the test case should infer this information from
> > > > there
> > > > somehow instead of re-implement the driver code here.
> > > > 
> > > > > But, as a temporary measure to make CI happy and enable PSR2.
> > > > 
> > > > why is this only affecting PSR2 case?
> > > 
> > > PSR2 has a maximum resolution suported and the only resolution
> > > that
> > > CI
> > > eDP panel supports is bigger than that.
> > > 
> > > > > Reviewed-by: Dhinakaran Pandiyan <
> > > > > dhinakaran.pandiyan@intel.com>
> > > > > 
> > > > > Rodrigo, what's your opinion on this?
> > > > > 
> > > > > -DK
> > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > > ---
> > > > > >  tests/kms_psr.c | 21 +++++++++++++++++++--
> > > > > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > > > > index 3e16a6bf..4792e158 100644
> > > > > > --- a/tests/kms_psr.c
> > > > > > +++ b/tests/kms_psr.c
> > > > > > @@ -445,13 +445,30 @@ int main(int argc, char *argv[])
> > > > > >  		igt_require_f(sink_support(&data, PSR_MODE_1),
> > > > > >  			      "Sink does not support PSR\n");
> > > > > >  
> > > > > > -		data.supports_psr2 = sink_support(&data,
> > > > > > PSR_MODE_2);
> > > > > > -
> > > > > >  		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);
> > > > > > +
> > > > > > +		data.supports_psr2 = sink_support(&data,
> > > > > > PSR_MODE_2);
> > > > > > +		if (data.supports_psr2) {
> > > > > > +			uint32_t devid =
> > > > > > intel_get_drm_devid(data.drm_fd);
> > > > > > +			uint16_t max_h = 0, max_v = 0;
> > > > > > +			int gen = intel_gen(devid);
> > > > > > +
> > > > > > +			if (gen >= 10 || IS_GEMINILAKE(devid))
> > > > > > {
> > > > > > +				max_h = 4096;
> > > > > > +				max_v = 2304;
> > > > > > +			} else if (gen == 9) {
> > > > > > +				max_h = 3640;
> > > > > > +				max_v = 2304;
> > > > > > +			}
> > > > > > +
> > > > > > +			if (data.mode->hdisplay > max_h ||
> > > > > > +			    data.mode->vdisplay > max_v)
> > > > > > +				data.supports_psr2 = false;
> > > > > > +		}
> > > > > >  	}
> > > > > >  
> > > > > >  	for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <=
> > > > > > PSR_MODE_2;
> > 
> > _______________________________________________
> > 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

Re: [igt-dev] [PATCH i-g-t v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[Rodrigo Vivi]

On Thu, Jan 24, 2019 at 04:09:35PM -0800, Souza, Jose wrote:
> On Thu, 2019-01-24 at 14:06 -0800, Rodrigo Vivi wrote:
> > On Thu, Jan 24, 2019 at 11:02:51AM -0800, Dhinakaran Pandiyan wrote:
> > > On Thu, 2019-01-24 at 10:20 -0800, Souza, Jose wrote:
> > > > On Wed, 2019-01-23 at 22:39 -0800, Rodrigo Vivi wrote:
> > > > > On Wed, Jan 23, 2019 at 06:25:07PM -0800, Dhinakaran Pandiyan
> > > > > wrote:
> > > > > > On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza
> > > > > > wrote:
> > > > > > > If resolution that will be used in PSR tests is bigger than
> > > > > > > the
> > > > > > > resolution that PSR2 HW supports, PSR2 will not be enabled
> > > > > > > so
> > > > > > > those
> > > > > > > tests should be skiped. Most of eDP panels only supports
> > > > > > > one
> > > > > > > resolution that is why we don't even try to use other
> > > > > > > resolution.
> > > > > > > 
> > > > > > > BSpec: 7713
> > > > > > 
> > > > > > I don't like the fact that we are re-implementing driver code
> > > > > > in
> > > > > > the
> > > > > > test to avoid false-positives. We should really replace the
> > > > > > panel
> > > > > > on
> > > > > > the WHL machine with a panel that is expected to work. 
> > > > > > 
> > > > > 
> > > > > I fully agree. The debugfs should be auto-suficient.
> > > > > 
> > > > > In the past it was the idea of Source OK. if sink support yes
> > > > > and
> > > > > source ok
> > > That had a problem too, "Source OK" was set after driver completed
> > > all
> > > the checks and just before enabling PSR2. In effect, it started
> > > becoming a duplicate of the "Enabled" field and we weren't able to
> > > catch if the driver checks before psr_enable() were wrong.
> > 
> > yeap, this is true indeed...
> > 
> > But we need to find a way where the debugfs tells us if
> > conditions are not match and enable is not expected to
> > avoid having to reimplement all the conditions twice.
> 
> I could add a reason field to debugfs and like we have for FBC(fbc-
> >no_fbc_reason)
> 
> What do you think?

well, I always disliked the no_fbc_reason, but it is better than duplicating
the code and I don't have better idea ;)

> 
> > 
> > > 
> > > > > and we tried to enabled but it is not getting active,
> > > > > than we have a bug somewhere.
> > > > > 
> > > > > I don't know the current status of the psr debugfs anymore, but
> > > > > I believe the test case should infer this information from
> > > > > there
> > > > > somehow instead of re-implement the driver code here.
> > > > > 
> > > > > > But, as a temporary measure to make CI happy and enable PSR2.
> > > > > 
> > > > > why is this only affecting PSR2 case?
> > > > 
> > > > PSR2 has a maximum resolution suported and the only resolution
> > > > that
> > > > CI
> > > > eDP panel supports is bigger than that.
> > > > 
> > > > > > Reviewed-by: Dhinakaran Pandiyan <
> > > > > > dhinakaran.pandiyan@intel.com>
> > > > > > 
> > > > > > Rodrigo, what's your opinion on this?
> > > > > > 
> > > > > > -DK
> > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > > > ---
> > > > > > >  tests/kms_psr.c | 21 +++++++++++++++++++--
> > > > > > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > > > > > index 3e16a6bf..4792e158 100644
> > > > > > > --- a/tests/kms_psr.c
> > > > > > > +++ b/tests/kms_psr.c
> > > > > > > @@ -445,13 +445,30 @@ int main(int argc, char *argv[])
> > > > > > >  		igt_require_f(sink_support(&data, PSR_MODE_1),
> > > > > > >  			      "Sink does not support PSR\n");
> > > > > > >  
> > > > > > > -		data.supports_psr2 = sink_support(&data,
> > > > > > > PSR_MODE_2);
> > > > > > > -
> > > > > > >  		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);
> > > > > > > +
> > > > > > > +		data.supports_psr2 = sink_support(&data,
> > > > > > > PSR_MODE_2);
> > > > > > > +		if (data.supports_psr2) {
> > > > > > > +			uint32_t devid =
> > > > > > > intel_get_drm_devid(data.drm_fd);
> > > > > > > +			uint16_t max_h = 0, max_v = 0;
> > > > > > > +			int gen = intel_gen(devid);
> > > > > > > +
> > > > > > > +			if (gen >= 10 || IS_GEMINILAKE(devid))
> > > > > > > {
> > > > > > > +				max_h = 4096;
> > > > > > > +				max_v = 2304;
> > > > > > > +			} else if (gen == 9) {
> > > > > > > +				max_h = 3640;
> > > > > > > +				max_v = 2304;
> > > > > > > +			}
> > > > > > > +
> > > > > > > +			if (data.mode->hdisplay > max_h ||
> > > > > > > +			    data.mode->vdisplay > max_v)
> > > > > > > +				data.supports_psr2 = false;
> > > > > > > +		}
> > > > > > >  	}
> > > > > > >  
> > > > > > >  	for (data.op_psr_mode = PSR_MODE_1; data.op_psr_mode <=
> > > > > > > PSR_MODE_2;
> > > 
> > > _______________________________________________
> > > 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 v4 1/4] test/psr: Skip PSR2 tests when panel resolution is bigger than PSR2 HW supports

[Dhinakaran Pandiyan]

On Thu, 2019-01-24 at 16:15 -0800, Rodrigo Vivi wrote:
> On Thu, Jan 24, 2019 at 04:09:35PM -0800, Souza, Jose wrote:
> > On Thu, 2019-01-24 at 14:06 -0800, Rodrigo Vivi wrote:
> > > On Thu, Jan 24, 2019 at 11:02:51AM -0800, Dhinakaran Pandiyan
> > > wrote:
> > > > On Thu, 2019-01-24 at 10:20 -0800, Souza, Jose wrote:
> > > > > On Wed, 2019-01-23 at 22:39 -0800, Rodrigo Vivi wrote:
> > > > > > On Wed, Jan 23, 2019 at 06:25:07PM -0800, Dhinakaran
> > > > > > Pandiyan
> > > > > > wrote:
> > > > > > > On Wed, 2019-01-23 at 15:56 -0800, José Roberto de Souza
> > > > > > > wrote:
> > > > > > > > If resolution that will be used in PSR tests is bigger
> > > > > > > > than
> > > > > > > > the
> > > > > > > > resolution that PSR2 HW supports, PSR2 will not be
> > > > > > > > enabled
> > > > > > > > so
> > > > > > > > those
> > > > > > > > tests should be skiped. Most of eDP panels only
> > > > > > > > supports
> > > > > > > > one
> > > > > > > > resolution that is why we don't even try to use other
> > > > > > > > resolution.
> > > > > > > > 
> > > > > > > > BSpec: 7713
> > > > > > > 
> > > > > > > I don't like the fact that we are re-implementing driver
> > > > > > > code
> > > > > > > in
> > > > > > > the
> > > > > > > test to avoid false-positives. We should really replace
> > > > > > > the
> > > > > > > panel
> > > > > > > on
> > > > > > > the WHL machine with a panel that is expected to work. 
> > > > > > > 
> > > > > > 
> > > > > > I fully agree. The debugfs should be auto-suficient.
> > > > > > 
> > > > > > In the past it was the idea of Source OK. if sink support
> > > > > > yes
> > > > > > and
> > > > > > source ok
> > > > 
> > > > That had a problem too, "Source OK" was set after driver
> > > > completed
> > > > all
> > > > the checks and just before enabling PSR2. In effect, it started
> > > > becoming a duplicate of the "Enabled" field and we weren't able
> > > > to
> > > > catch if the driver checks before psr_enable() were wrong.
> > > 
> > > yeap, this is true indeed...
> > > 
> > > But we need to find a way where the debugfs tells us if
> > > conditions are not match and enable is not expected to
> > > avoid having to reimplement all the conditions twice.
> > 
> > I could add a reason field to debugfs and like we have for FBC(fbc-
> > > no_fbc_reason)
> > 
> > What do you think?
> 
> well, I always disliked the no_fbc_reason, but it is better than
> duplicating
> the code and I don't have better idea ;)
> 
Replace the panel :) We need PSR2 testing on gen-9 anyway, we cannot
work around it.

-DK

> > 
> > > 
> > > > 
> > > > > > and we tried to enabled but it is not getting active,
> > > > > > than we have a bug somewhere.
> > > > > > 
> > > > > > I don't know the current status of the psr debugfs anymore,
> > > > > > but
> > > > > > I believe the test case should infer this information from
> > > > > > there
> > > > > > somehow instead of re-implement the driver code here.
> > > > > > 
> > > > > > > But, as a temporary measure to make CI happy and enable
> > > > > > > PSR2.
> > > > > > 
> > > > > > why is this only affecting PSR2 case?
> > > > > 
> > > > > PSR2 has a maximum resolution suported and the only
> > > > > resolution
> > > > > that
> > > > > CI
> > > > > eDP panel supports is bigger than that.
> > > > > 
> > > > > > > Reviewed-by: Dhinakaran Pandiyan <
> > > > > > > dhinakaran.pandiyan@intel.com>
> > > > > > > 
> > > > > > > Rodrigo, what's your opinion on this?
> > > > > > > 
> > > > > > > -DK
> > > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > > > Signed-off-by: José Roberto de Souza <
> > > > > > > > jose.souza@intel.com>
> > > > > > > > ---
> > > > > > > >  tests/kms_psr.c | 21 +++++++++++++++++++--
> > > > > > > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/tests/kms_psr.c b/tests/kms_psr.c
> > > > > > > > index 3e16a6bf..4792e158 100644
> > > > > > > > --- a/tests/kms_psr.c
> > > > > > > > +++ b/tests/kms_psr.c
> > > > > > > > @@ -445,13 +445,30 @@ int main(int argc, char *argv[])
> > > > > > > >  		igt_require_f(sink_support(&data,
> > > > > > > > PSR_MODE_1),
> > > > > > > >  			      "Sink does not support
> > > > > > > > PSR\n");
> > > > > > > >  
> > > > > > > > -		data.supports_psr2 =
> > > > > > > > sink_support(&data,
> > > > > > > > PSR_MODE_2);
> > > > > > > > -
> > > > > > > >  		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);
> > > > > > > > +
> > > > > > > > +		data.supports_psr2 =
> > > > > > > > sink_support(&data,
> > > > > > > > PSR_MODE_2);
> > > > > > > > +		if (data.supports_psr2) {
> > > > > > > > +			uint32_t devid =
> > > > > > > > intel_get_drm_devid(data.drm_fd);
> > > > > > > > +			uint16_t max_h = 0, max_v = 0;
> > > > > > > > +			int gen = intel_gen(devid);
> > > > > > > > +
> > > > > > > > +			if (gen >= 10 ||
> > > > > > > > IS_GEMINILAKE(devid))
> > > > > > > > {
> > > > > > > > +				max_h = 4096;
> > > > > > > > +				max_v = 2304;
> > > > > > > > +			} else if (gen == 9) {
> > > > > > > > +				max_h = 3640;
> > > > > > > > +				max_v = 2304;
> > > > > > > > +			}
> > > > > > > > +
> > > > > > > > +			if (data.mode->hdisplay > max_h
> > > > > > > > ||
> > > > > > > > +			    data.mode->vdisplay >
> > > > > > > > max_v)
> > > > > > > > +				data.supports_psr2 =
> > > > > > > > false;
> > > > > > > > +		}
> > > > > > > >  	}
> > > > > > > >  
> > > > > > > >  	for (data.op_psr_mode = PSR_MODE_1;
> > > > > > > > data.op_psr_mode <=
> > > > > > > > PSR_MODE_2;
> > > > 
> > > > _______________________________________________
> > > > 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