[igt-dev] [PATCH i-g-t 0/2] Add a new IGT test to validate DC3CO state.

[igt-dev] [PATCH i-g-t 0/2] Add a new IGT test to validate DC3CO state.

[Jeevan B]

This test is creating a vpb scenario for
selective frame update and validating
that DC state stays in DC3CO during execution.

Anshuman Gupta (1):
  igt/i915/i915_pm_dc: DC3CO PSR2 helpers

Jeevan B (1):
  Add a new IGT test to validate DC3CO state

 lib/igt_psr.c           |  10 +++
 lib/igt_psr.h           |   1 +
 tests/i915/i915_pm_dc.c | 172 +++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 175 insertions(+), 8 deletions(-)

-- 
2.7.4

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

[igt-dev] [PATCH i-g-t 1/2] igt/i915/i915_pm_dc: DC3CO PSR2 helpers

[Jeevan B]

From: Anshuman Gupta <anshuman.gupta@intel.com>

Add DC3CO IGT validation prerequisites stuff
so we can enable DC3CO IGT test.

v2: Removed psr2_idle_wait_entry and get_psr2_status function.

v3: Changed macro definition and removed extra line.

Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Jeevan B <jeevan.b@intel.com>
---
 tests/i915/i915_pm_dc.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
index ce3319b..0ddd6b3 100644
--- a/tests/i915/i915_pm_dc.c
+++ b/tests/i915/i915_pm_dc.c
@@ -34,8 +34,9 @@
 #include "limits.h"
 
 /* DC State Flags */
-#define CHECK_DC5	1
-#define CHECK_DC6	2
+#define CHECK_DC5	(1 << 0)
+#define CHECK_DC6	(1 << 1)
+#define CHECK_DC3CO	(1 << 2)
 
 typedef struct {
 	int drm_fd;
@@ -88,6 +89,19 @@ static bool edp_psr_sink_support(data_t *data)
 	return strstr(buf, "Sink support: yes");
 }
 
+static bool edp_psr2_enabled(data_t *data)
+{
+	char buf[512];
+
+	igt_debugfs_simple_read(data->debugfs_fd, "i915_edp_psr_status",
+				buf, sizeof(buf));
+
+	if (data->op_psr_mode == PSR_MODE_2)
+		return strstr(buf, "PSR mode: PSR2 enabled");
+
+	return false;
+}
+
 static void cleanup_dc_psr(data_t *data)
 {
 	igt_plane_t *primary;
@@ -141,12 +155,18 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
 		str = strstr(buf, "DC3 -> DC5 count");
 	else if (dc_flag & CHECK_DC6)
 		str = strstr(buf, "DC5 -> DC6 count");
+	else if (dc_flag & CHECK_DC3CO)
+		str = strstr(buf, "DC3CO count");
 
-	/* Check DC5/DC6 counter is available for the platform.
+	/* Check DC counter is available for the platform.
 	 * Skip the test if counter is not available.
 	 */
-	igt_skip_on_f(!str, "DC%d counter is not available\n",
-		      dc_flag & CHECK_DC5 ? 5 : 6);
+	if (dc_flag & CHECK_DC3CO)
+		igt_skip_on_f(!str, "DC3CO counter is not available\n");
+	else
+		igt_skip_on_f(!str, "DC%d counter is not available\n",
+			      dc_flag & CHECK_DC5 ? 5 : 6);
+
 	return get_dc_counter(str);
 }
 
@@ -158,9 +178,12 @@ static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count)
 
 static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
 {
+	char tmp[64];
+
+	snprintf(tmp, sizeof(tmp), "%s", dc_flag & CHECK_DC3CO ? "DC3CO" :
+		 (dc_flag & CHECK_DC5 ? "DC5" : "DC6"));
 	igt_assert_f(dc_state_wait_entry(drm_fd, dc_flag, prev_dc_count),
-		     "DC%d state is not achieved\n",
-		     dc_flag & CHECK_DC5 ? 5 : 6);
+		     "%s state is not achieved\n", tmp);
 }
 
 static void test_dc_state_psr(data_t *data, int dc_flag)
-- 
2.7.4

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

[igt-dev] [PATCH i-g-t 2/2] Add a new IGT test to validate DC3CO state

[Jeevan B]

Add a subtest for DC3CO video playback case
to generate selective frame update and validate
that system stays in DC3CO state during execution.

v2: Changed PSR2 idle check to sleep check and addressed cosmetic changes.
v3: Renamed a function and restructured code according to Anshuman’s comments.
v4: Cosmetic changes.
v5: Removed DC5 check, Platform check and a function parameter. 
    Renamed a function name as per Arek and Imre's Comments.

Signed-off-by: Jeevan B <jeevan.b@intel.com>
---
 lib/igt_psr.c           |  10 ++++
 lib/igt_psr.h           |   1 +
 tests/i915/i915_pm_dc.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 145 insertions(+), 1 deletion(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index b92ea73..9127a0b 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -36,6 +36,16 @@ static bool psr_active_check(int debugfs_fd, enum psr_mode mode)
 	return strstr(buf, state);
 }
 
+bool psr2_active_sleep_check(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+	const char *state = "SLEEP";
+
+	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				sizeof(buf));
+	return strstr(buf, state);
+}
+
 static inline const char *psr_active_state_get(enum psr_mode mode)
 {
 	return mode == PSR_MODE_1 ? "SRDENT" : "DEEP_SLEEP";
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index ca38573..a0627dc 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -35,6 +35,7 @@ enum psr_mode {
 	PSR_MODE_2
 };
 
+bool psr2_active_sleep_check(int debugfs_fd);
 bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
 bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
 bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
index 0ddd6b3..ac6270f 100644
--- a/tests/i915/i915_pm_dc.c
+++ b/tests/i915/i915_pm_dc.c
@@ -38,13 +38,20 @@
 #define CHECK_DC6	(1 << 1)
 #define CHECK_DC3CO	(1 << 2)
 
+/*Number of Frames Video Playback*/
+#define VIDEO_FRAMES 130
+
+typedef struct {
+	double r, g, b;
+} color_t;
+
 typedef struct {
 	int drm_fd;
 	int msr_fd;
 	int debugfs_fd;
 	uint32_t devid;
 	igt_display_t display;
-	struct igt_fb fb_white;
+	struct igt_fb fb_white, fb_rgb, fb_rgr;
 	enum psr_mode op_psr_mode;
 	drmModeModeInfo *mode;
 	igt_output_t *output;
@@ -113,6 +120,42 @@ static void cleanup_dc_psr(data_t *data)
 	igt_remove_fb(data->drm_fd, &data->fb_white);
 }
 
+static void cleanup_dc3co(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);
+	/* Clear Frame Buffers */
+	igt_display_commit(&data->display);
+	igt_remove_fb(data->drm_fd, &data->fb_rgb);
+	igt_remove_fb(data->drm_fd, &data->fb_rgr);
+}
+
+static void paint_rectangles(data_t *data,
+			     drmModeModeInfo *mode,
+			     color_t *colors,
+			     igt_fb_t *fb)
+{
+	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
+	int i, l = mode->hdisplay / 3;
+	int rows_remaining = mode->hdisplay % 3;
+
+	/* Paint 3 solid rectangles. */
+	for (i = 0 ; i < 3; i++) {
+		igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
+				colors[i].r, colors[i].g, colors[i].b);
+	}
+
+	if (rows_remaining > 0)
+		igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay,
+				colors[i - 1].r, colors[i - 1].g,
+				colors[i - 1].b);
+
+	igt_put_cairo_ctx(data->drm_fd, fb, cr);
+}
+
 static void setup_primary(data_t *data)
 {
 	igt_plane_t *primary;
@@ -130,6 +173,20 @@ static void setup_primary(data_t *data)
 	igt_display_commit(&data->display);
 }
 
+static void create_color_fb(data_t *data, igt_fb_t *fb, color_t *fb_color)
+{
+	int fb_id;
+
+	fb_id = igt_create_fb(data->drm_fd,
+			      data->mode->hdisplay,
+			      data->mode->vdisplay,
+			      DRM_FORMAT_XRGB8888,
+			      LOCAL_DRM_FORMAT_MOD_NONE,
+			      fb);
+	igt_assert(fb_id);
+	paint_rectangles(data, data->mode, fb_color, fb);
+}
+
 static uint32_t get_dc_counter(char *dc_data)
 {
 	char *e;
@@ -170,6 +227,11 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
 	return get_dc_counter(str);
 }
 
+static bool psr2_wait_sleep_entry(int debugfs_fd)
+{
+	return igt_wait(psr2_active_sleep_check(debugfs_fd), 50, 10);
+}
+
 static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count)
 {
 	return igt_wait(read_dc_counter(drm_fd, dc_flag) >
@@ -186,6 +248,70 @@ static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
 		     "%s state is not achieved\n", tmp);
 }
 
+static void setup_videoplayback(data_t *data)
+{
+	color_t red_green_blue[] = {
+			{ 1.0, 0.0, 0.0 },
+			{ 0.0, 1.0, 0.0 },
+			{ 0.0, 0.0, 1.0 },
+	};
+	color_t red_green_red[] = {
+			{ 1.0, 0.0, 0.0 },
+			{ 0.0, 1.0, 0.0 },
+			{ 1.0, 0.0, 0.0 },
+	};
+
+	create_color_fb(data, &data->fb_rgb, red_green_blue);
+	create_color_fb(data, &data->fb_rgr, red_green_red);
+}
+
+static void check_dc3co_with_videoplayback_like_load(data_t *data)
+{
+	igt_plane_t *primary;
+	uint32_t dc3co_prev_cnt;
+	int i, delay;
+
+	primary = igt_output_get_plane_type(data->output,
+					    DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(primary, NULL);
+	dc3co_prev_cnt = read_dc_counter(data->drm_fd, CHECK_DC3CO);
+	/* Calculate delay to generate idle frame in usec*/
+	delay = ((1000 * 1000) / data->mode->vrefresh);
+
+	for (i = 0; i < VIDEO_FRAMES; i++) {
+		if (i % 2 == 0) {
+			igt_plane_set_fb(primary, &data->fb_rgb);
+			igt_display_commit(&data->display);
+		} else {
+			igt_plane_set_fb(primary, &data->fb_rgr);
+			igt_display_commit(&data->display);
+		}
+
+		usleep(delay);
+		igt_assert(psr2_wait_sleep_entry(data->debugfs_fd));
+	}
+
+	check_dc_counter(data->drm_fd, CHECK_DC3CO, dc3co_prev_cnt);
+}
+
+static void setup_dc3co(data_t *data)
+{
+	data->op_psr_mode = PSR_MODE_2;
+	psr_enable(data->debugfs_fd, data->op_psr_mode);
+	igt_require_f(edp_psr2_enabled(data),
+		      "PSR2 is not enabled\n");
+	igt_require(read_dc_counter(data->drm_fd, CHECK_DC3CO));
+}
+
+static void test_dc3co_vpb_simulation(data_t *data)
+{
+	setup_output(data);
+	setup_dc3co(data);
+	setup_videoplayback(data);
+	check_dc3co_with_videoplayback_like_load(data);
+	cleanup_dc3co(data);
+}
+
 static void test_dc_state_psr(data_t *data, int dc_flag)
 {
 	uint32_t dc_counter_before_psr;
@@ -287,6 +413,13 @@ int main(int argc, char *argv[])
 			     "Can't open /dev/cpu/0/msr.\n");
 	}
 
+	igt_describe("This test simulate video playback "
+		     "in order to validate DC3CO state "
+		     "while PSR2 is active and in SLEEP state");
+	igt_subtest("dc3co-vpb-simulation") {
+		test_dc3co_vpb_simulation(&data);
+	}
+
 	igt_describe("This test validates display engine entry to DC5 state "
 		     "while PSR is active");
 	igt_subtest("dc5-psr") {
-- 
2.7.4

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

[igt-dev] ✗ Fi.CI.BAT: failure for Add a new IGT test to validate DC3CO state. (rev5)

[Patchwork]

== Series Details ==

Series: Add a new IGT test to validate DC3CO state. (rev5)
URL   : https://patchwork.freedesktop.org/series/66648/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7117 -> IGTPW_3583
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_execlists:
    - fi-icl-u2:          [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-icl-u2/igt@i915_selftest@live_execlists.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-icl-u2/igt@i915_selftest@live_execlists.html
    - fi-skl-6260u:       [PASS][3] -> [DMESG-FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-skl-6260u/igt@i915_selftest@live_execlists.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-skl-6260u/igt@i915_selftest@live_execlists.html

  
#### Suppressed ####

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

  * igt@i915_selftest@live_execlists:
    - {fi-icl-dsi}:       NOTRUN -> [DMESG-FAIL][5]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-icl-dsi/igt@i915_selftest@live_execlists.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][6] -> [FAIL][7] ([fdo#111045] / [fdo#111096])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@vgem_basic@sysfs:
    - fi-icl-u3:          [PASS][8] -> [DMESG-WARN][9] ([fdo#107724]) +1 similar issue
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-icl-u3/igt@vgem_basic@sysfs.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-icl-u3/igt@vgem_basic@sysfs.html

  
#### Possible fixes ####

  * igt@gem_ctx_switch@rcs0:
    - {fi-icl-guc}:       [INCOMPLETE][10] ([fdo#107713]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-icl-guc/igt@gem_ctx_switch@rcs0.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-icl-guc/igt@gem_ctx_switch@rcs0.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-icl-u3:          [FAIL][12] ([fdo#111699]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-icl-u3/igt@gem_exec_suspend@basic-s3.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-icl-u3/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_flink_basic@flink-lifetime:
    - fi-icl-u3:          [DMESG-WARN][14] ([fdo#107724]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-icl-u3/igt@gem_flink_basic@flink-lifetime.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-icl-u3/igt@gem_flink_basic@flink-lifetime.html

  * igt@gem_sync@basic-all:
    - {fi-tgl-u}:         [INCOMPLETE][16] ([fdo#111880]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-tgl-u/igt@gem_sync@basic-all.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-tgl-u/igt@gem_sync@basic-all.html

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         [DMESG-FAIL][18] -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-apl-guc/igt@i915_selftest@live_execlists.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-apl-guc/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_hangcheck:
    - {fi-tgl-u2}:        [INCOMPLETE][20] ([fdo#111747]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-tgl-u2/igt@i915_selftest@live_hangcheck.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-tgl-u2/igt@i915_selftest@live_hangcheck.html
    - {fi-icl-dsi}:       [INCOMPLETE][22] ([fdo#107713] / [fdo#108569]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-icl-dsi/igt@i915_selftest@live_hangcheck.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-icl-dsi/igt@i915_selftest@live_hangcheck.html

  * igt@kms_busy@basic-flip-a:
    - {fi-tgl-u2}:        [DMESG-WARN][24] ([fdo#111600]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7117/fi-tgl-u2/igt@kms_busy@basic-flip-a.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/fi-tgl-u2/igt@kms_busy@basic-flip-a.html

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

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111600]: https://bugs.freedesktop.org/show_bug.cgi?id=111600
  [fdo#111699]: https://bugs.freedesktop.org/show_bug.cgi?id=111699
  [fdo#111747]: https://bugs.freedesktop.org/show_bug.cgi?id=111747
  [fdo#111880]: https://bugs.freedesktop.org/show_bug.cgi?id=111880


Participating hosts (53 -> 46)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5231 -> IGTPW_3583

  CI-20190529: 20190529
  CI_DRM_7117: eb28e8f9386c63609c5ade1c31b6147df08cc5e0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3583: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3583/index.html
  IGT_5231: e293051f8f99c72cb01d21e4b73a5928ea351eb3 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@i915_pm_dc@dc3co-vpb-simulation

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t 2/2] Add a new IGT test to validate DC3CO state

[Arkadiusz Hiler]

On Thu, Oct 17, 2019 at 04:05:00PM +0530, Jeevan B wrote:
> Add a subtest for DC3CO video playback case
> to generate selective frame update and validate
> that system stays in DC3CO state during execution.
> 
> v2: Changed PSR2 idle check to sleep check and addressed cosmetic changes.
> v3: Renamed a function and restructured code according to Anshuman’s comments.
> v4: Cosmetic changes.
> v5: Removed DC5 check, Platform check and a function parameter. 
>     Renamed a function name as per Arek and Imre's Comments.
> 
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
>  lib/igt_psr.c           |  10 ++++
>  lib/igt_psr.h           |   1 +
>  tests/i915/i915_pm_dc.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 145 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index b92ea73..9127a0b 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -36,6 +36,16 @@ static bool psr_active_check(int debugfs_fd, enum psr_mode mode)
>  	return strstr(buf, state);
>  }
>  
> +bool psr2_active_sleep_check(int debugfs_fd)
> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +	const char *state = "SLEEP";
> +
> +	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> +				sizeof(buf));
> +	return strstr(buf, state);
> +}
> +
>  static inline const char *psr_active_state_get(enum psr_mode mode)
>  {
>  	return mode == PSR_MODE_1 ? "SRDENT" : "DEEP_SLEEP";
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index ca38573..a0627dc 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -35,6 +35,7 @@ enum psr_mode {
>  	PSR_MODE_2
>  };
>  
> +bool psr2_active_sleep_check(int debugfs_fd);
>  bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
>  bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
>  bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
> index 0ddd6b3..ac6270f 100644
> --- a/tests/i915/i915_pm_dc.c
> +++ b/tests/i915/i915_pm_dc.c
> @@ -38,13 +38,20 @@
>  #define CHECK_DC6	(1 << 1)
>  #define CHECK_DC3CO	(1 << 2)
>  
> +/*Number of Frames Video Playback*/
> +#define VIDEO_FRAMES 130
> +
> +typedef struct {
> +	double r, g, b;
> +} color_t;
> +
>  typedef struct {
>  	int drm_fd;
>  	int msr_fd;
>  	int debugfs_fd;
>  	uint32_t devid;
>  	igt_display_t display;
> -	struct igt_fb fb_white;
> +	struct igt_fb fb_white, fb_rgb, fb_rgr;
>  	enum psr_mode op_psr_mode;
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
> @@ -113,6 +120,42 @@ static void cleanup_dc_psr(data_t *data)
>  	igt_remove_fb(data->drm_fd, &data->fb_white);
>  }
>  
> +static void cleanup_dc3co(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);
> +	/* Clear Frame Buffers */
> +	igt_display_commit(&data->display);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgb);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgr);
> +}
> +
> +static void paint_rectangles(data_t *data,
> +			     drmModeModeInfo *mode,
> +			     color_t *colors,
> +			     igt_fb_t *fb)
> +{
> +	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +	int i, l = mode->hdisplay / 3;
> +	int rows_remaining = mode->hdisplay % 3;
> +
> +	/* Paint 3 solid rectangles. */
> +	for (i = 0 ; i < 3; i++) {
> +		igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
> +				colors[i].r, colors[i].g, colors[i].b);
> +	}
> +
> +	if (rows_remaining > 0)
> +		igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay,
> +				colors[i - 1].r, colors[i - 1].g,
> +				colors[i - 1].b);
> +
> +	igt_put_cairo_ctx(data->drm_fd, fb, cr);
> +}
> +
>  static void setup_primary(data_t *data)
>  {
>  	igt_plane_t *primary;
> @@ -130,6 +173,20 @@ static void setup_primary(data_t *data)
>  	igt_display_commit(&data->display);
>  }
>  
> +static void create_color_fb(data_t *data, igt_fb_t *fb, color_t *fb_color)
> +{
> +	int fb_id;
> +
> +	fb_id = igt_create_fb(data->drm_fd,
> +			      data->mode->hdisplay,
> +			      data->mode->vdisplay,
> +			      DRM_FORMAT_XRGB8888,
> +			      LOCAL_DRM_FORMAT_MOD_NONE,
> +			      fb);
> +	igt_assert(fb_id);
> +	paint_rectangles(data, data->mode, fb_color, fb);
> +}
> +
>  static uint32_t get_dc_counter(char *dc_data)
>  {
>  	char *e;
> @@ -170,6 +227,11 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
>  	return get_dc_counter(str);
>  }
>  
> +static bool psr2_wait_sleep_entry(int debugfs_fd)
> +{
> +	return igt_wait(psr2_active_sleep_check(debugfs_fd), 50, 10);
> +}
> +
>  static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count)
>  {
>  	return igt_wait(read_dc_counter(drm_fd, dc_flag) >
> @@ -186,6 +248,70 @@ static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
>  		     "%s state is not achieved\n", tmp);
>  }
>  
> +static void setup_videoplayback(data_t *data)
> +{
> +	color_t red_green_blue[] = {
> +			{ 1.0, 0.0, 0.0 },
> +			{ 0.0, 1.0, 0.0 },
> +			{ 0.0, 0.0, 1.0 },
> +	};
> +	color_t red_green_red[] = {
> +			{ 1.0, 0.0, 0.0 },
> +			{ 0.0, 1.0, 0.0 },
> +			{ 1.0, 0.0, 0.0 },
> +	};
> +
> +	create_color_fb(data, &data->fb_rgb, red_green_blue);
> +	create_color_fb(data, &data->fb_rgr, red_green_red);
> +}
> +
> +static void check_dc3co_with_videoplayback_like_load(data_t *data)
> +{
> +	igt_plane_t *primary;
> +	uint32_t dc3co_prev_cnt;
> +	int i, delay;
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	dc3co_prev_cnt = read_dc_counter(data->drm_fd, CHECK_DC3CO);
> +	/* Calculate delay to generate idle frame in usec*/
> +	delay = ((1000 * 1000) / data->mode->vrefresh);
> +
> +	for (i = 0; i < VIDEO_FRAMES; i++) {
> +		if (i % 2 == 0) {
> +			igt_plane_set_fb(primary, &data->fb_rgb);
> +			igt_display_commit(&data->display);
> +		} else {
> +			igt_plane_set_fb(primary, &data->fb_rgr);
> +			igt_display_commit(&data->display);
> +		}
> +
> +		usleep(delay);
> +		igt_assert(psr2_wait_sleep_entry(data->debugfs_fd));
> +	}
> +
> +	check_dc_counter(data->drm_fd, CHECK_DC3CO, dc3co_prev_cnt);
> +}
> +
> +static void setup_dc3co(data_t *data)
> +{
> +	data->op_psr_mode = PSR_MODE_2;
> +	psr_enable(data->debugfs_fd, data->op_psr_mode);
> +	igt_require_f(edp_psr2_enabled(data),
> +		      "PSR2 is not enabled\n");
> +	igt_require(read_dc_counter(data->drm_fd, CHECK_DC3CO));

read_dc_counter() returns the counter value and skips internally
and then this igt_require() which will skip if the counter value is 0.

This does not look like intended behavior.


I still think that we should just introduce another function,
require_dc_counter(int counter) that would check for the string presence
in the debugfs.

And then read_dc_counter() would just assert if we cannot read the
counter.

so the final test would look something like this:

igt_describe("This test simulate video playback "
	     "in order to make sure we enter DC3CO state "
	     "while PSR2 is active and we are in SLEEP state");
igt_subtest("dc3co-vpb-simulation") {
	require_dc_counter(CHECK_DC3CO);
	psr_enable(data->debugfs_fd, PSR_MODE_2);
	igt_require(is_edp_psr2_enabled(data));
	setup_videoplayback_like_load(data);
	check_dc3co_with_videoplayback_like_load(data);
	cleanup_videoplayback_fbs(data);
}


> +}
> +
> +static void test_dc3co_vpb_simulation(data_t *data)
> +{
> +	setup_output(data);
> +	setup_dc3co(data);
> +	setup_videoplayback(data);
> +	check_dc3co_with_videoplayback_like_load(data);
> +	cleanup_dc3co(data);
> +}
> +
>  static void test_dc_state_psr(data_t *data, int dc_flag)
>  {
>  	uint32_t dc_counter_before_psr;
> @@ -287,6 +413,13 @@ int main(int argc, char *argv[])
>  			     "Can't open /dev/cpu/0/msr.\n");
>  	}
>  
> +	igt_describe("This test simulate video playback "
> +		     "in order to validate DC3CO state "
> +		     "while PSR2 is active and in SLEEP state");
> +	igt_subtest("dc3co-vpb-simulation") {
> +		test_dc3co_vpb_simulation(&data);
> +	}
> +
>  	igt_describe("This test validates display engine entry to DC5 state "
>  		     "while PSR is active");
>  	igt_subtest("dc5-psr") {
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/2] igt/i915/i915_pm_dc: DC3CO PSR2 helpers

[Arkadiusz Hiler]

On Thu, Oct 17, 2019 at 04:04:59PM +0530, Jeevan B wrote:
> ontent-Length: 2643
> 
> From: Anshuman Gupta <anshuman.gupta@intel.com>
> 
> Add DC3CO IGT validation prerequisites stuff
> so we can enable DC3CO IGT test.
> 
> v2: Removed psr2_idle_wait_entry and get_psr2_status function.
> 
> v3: Changed macro definition and removed extra line.
> 
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
>  tests/i915/i915_pm_dc.c | 37 ++++++++++++++++++++++++++++++-------
>  1 file changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
> index ce3319b..0ddd6b3 100644
> --- a/tests/i915/i915_pm_dc.c
> +++ b/tests/i915/i915_pm_dc.c
> @@ -34,8 +34,9 @@
>  #include "limits.h"
>  
>  /* DC State Flags */
> -#define CHECK_DC5	1
> -#define CHECK_DC6	2
> +#define CHECK_DC5	(1 << 0)
> +#define CHECK_DC6	(1 << 1)
> +#define CHECK_DC3CO	(1 << 2)
>  
>  typedef struct {
>  	int drm_fd;
> @@ -88,6 +89,19 @@ static bool edp_psr_sink_support(data_t *data)
>  	return strstr(buf, "Sink support: yes");
>  }
>  
> +static bool edp_psr2_enabled(data_t *data)
> +{
> +	char buf[512];
> +
> +	igt_debugfs_simple_read(data->debugfs_fd, "i915_edp_psr_status",
> +				buf, sizeof(buf));
> +
> +	if (data->op_psr_mode == PSR_MODE_2)
> +		return strstr(buf, "PSR mode: PSR2 enabled");

This looks like a helper function to check whether we have PSR2 enabled,
so I don't quite get why we have this if here.

It's either enabled or not and some internal state of data struct should
not affect it.

> +
> +	return false;
> +}
> +
>  static void cleanup_dc_psr(data_t *data)
>  {
>  	igt_plane_t *primary;
> @@ -141,12 +155,18 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
>  		str = strstr(buf, "DC3 -> DC5 count");
>  	else if (dc_flag & CHECK_DC6)
>  		str = strstr(buf, "DC5 -> DC6 count");
> +	else if (dc_flag & CHECK_DC3CO)
> +		str = strstr(buf, "DC3CO count");
>  
> -	/* Check DC5/DC6 counter is available for the platform.
> +	/* Check DC counter is available for the platform.
>  	 * Skip the test if counter is not available.
>  	 */
> -	igt_skip_on_f(!str, "DC%d counter is not available\n",
> -		      dc_flag & CHECK_DC5 ? 5 : 6);
> +	if (dc_flag & CHECK_DC3CO)
> +		igt_skip_on_f(!str, "DC3CO counter is not available\n");
> +	else
> +		igt_skip_on_f(!str, "DC%d counter is not available\n",
> +			      dc_flag & CHECK_DC5 ? 5 : 6);
> +
>  	return get_dc_counter(str);
>  }
>  
> @@ -158,9 +178,12 @@ static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count)
>  
>  static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
>  {
> +	char tmp[64];
> +
> +	snprintf(tmp, sizeof(tmp), "%s", dc_flag & CHECK_DC3CO ? "DC3CO" :
> +		 (dc_flag & CHECK_DC5 ? "DC5" : "DC6"));
>  	igt_assert_f(dc_state_wait_entry(drm_fd, dc_flag, prev_dc_count),
> -		     "DC%d state is not achieved\n",
> -		     dc_flag & CHECK_DC5 ? 5 : 6);
> +		     "%s state is not achieved\n", tmp);
>  }
>  
>  static void test_dc_state_psr(data_t *data, int
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/2] Add a new IGT test to validate DC3CO state

[Imre Deak]

On Thu, Oct 17, 2019 at 04:05:00PM +0530, Jeevan B wrote:
> Add a subtest for DC3CO video playback case
> to generate selective frame update and validate
> that system stays in DC3CO state during execution.
> 
> v2: Changed PSR2 idle check to sleep check and addressed cosmetic changes.
> v3: Renamed a function and restructured code according to Anshuman’s comments.
> v4: Cosmetic changes.
> v5: Removed DC5 check, Platform check and a function parameter. 
>     Renamed a function name as per Arek and Imre's Comments.
> 
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
>  lib/igt_psr.c           |  10 ++++
>  lib/igt_psr.h           |   1 +
>  tests/i915/i915_pm_dc.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 145 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index b92ea73..9127a0b 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -36,6 +36,16 @@ static bool psr_active_check(int debugfs_fd, enum psr_mode mode)
>  	return strstr(buf, state);
>  }
>  
> +bool psr2_active_sleep_check(int debugfs_fd)
> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +	const char *state = "SLEEP";
> +
> +	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> +				sizeof(buf));
> +	return strstr(buf, state);
> +}
> +
>  static inline const char *psr_active_state_get(enum psr_mode mode)
>  {
>  	return mode == PSR_MODE_1 ? "SRDENT" : "DEEP_SLEEP";
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index ca38573..a0627dc 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -35,6 +35,7 @@ enum psr_mode {
>  	PSR_MODE_2
>  };
>  
> +bool psr2_active_sleep_check(int debugfs_fd);
>  bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
>  bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
>  bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
> index 0ddd6b3..ac6270f 100644
> --- a/tests/i915/i915_pm_dc.c
> +++ b/tests/i915/i915_pm_dc.c
> @@ -38,13 +38,20 @@
>  #define CHECK_DC6	(1 << 1)
>  #define CHECK_DC3CO	(1 << 2)
>  
> +/*Number of Frames Video Playback*/
> +#define VIDEO_FRAMES 130
> +
> +typedef struct {
> +	double r, g, b;
> +} color_t;
> +
>  typedef struct {
>  	int drm_fd;
>  	int msr_fd;
>  	int debugfs_fd;
>  	uint32_t devid;
>  	igt_display_t display;
> -	struct igt_fb fb_white;
> +	struct igt_fb fb_white, fb_rgb, fb_rgr;
>  	enum psr_mode op_psr_mode;
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
> @@ -113,6 +120,42 @@ static void cleanup_dc_psr(data_t *data)
>  	igt_remove_fb(data->drm_fd, &data->fb_white);
>  }
>  
> +static void cleanup_dc3co(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);
> +	/* Clear Frame Buffers */
> +	igt_display_commit(&data->display);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgb);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgr);
> +}
> +
> +static void paint_rectangles(data_t *data,
> +			     drmModeModeInfo *mode,
> +			     color_t *colors,
> +			     igt_fb_t *fb)
> +{
> +	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +	int i, l = mode->hdisplay / 3;
> +	int rows_remaining = mode->hdisplay % 3;
> +
> +	/* Paint 3 solid rectangles. */
> +	for (i = 0 ; i < 3; i++) {
> +		igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
> +				colors[i].r, colors[i].g, colors[i].b);
> +	}
> +
> +	if (rows_remaining > 0)
> +		igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay,
> +				colors[i - 1].r, colors[i - 1].g,
> +				colors[i - 1].b);
> +
> +	igt_put_cairo_ctx(data->drm_fd, fb, cr);
> +}
> +
>  static void setup_primary(data_t *data)
>  {
>  	igt_plane_t *primary;
> @@ -130,6 +173,20 @@ static void setup_primary(data_t *data)
>  	igt_display_commit(&data->display);
>  }
>  
> +static void create_color_fb(data_t *data, igt_fb_t *fb, color_t *fb_color)
> +{
> +	int fb_id;
> +
> +	fb_id = igt_create_fb(data->drm_fd,
> +			      data->mode->hdisplay,
> +			      data->mode->vdisplay,
> +			      DRM_FORMAT_XRGB8888,
> +			      LOCAL_DRM_FORMAT_MOD_NONE,
> +			      fb);
> +	igt_assert(fb_id);
> +	paint_rectangles(data, data->mode, fb_color, fb);
> +}
> +
>  static uint32_t get_dc_counter(char *dc_data)
>  {
>  	char *e;
> @@ -170,6 +227,11 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
>  	return get_dc_counter(str);
>  }
>  
> +static bool psr2_wait_sleep_entry(int debugfs_fd)
> +{
> +	return igt_wait(psr2_active_sleep_check(debugfs_fd), 50, 10);
> +}
> +
>  static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count)
>  {
>  	return igt_wait(read_dc_counter(drm_fd, dc_flag) >
> @@ -186,6 +248,70 @@ static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
>  		     "%s state is not achieved\n", tmp);
>  }
>  
> +static void setup_videoplayback(data_t *data)
> +{
> +	color_t red_green_blue[] = {
> +			{ 1.0, 0.0, 0.0 },
> +			{ 0.0, 1.0, 0.0 },
> +			{ 0.0, 0.0, 1.0 },
> +	};
> +	color_t red_green_red[] = {
> +			{ 1.0, 0.0, 0.0 },
> +			{ 0.0, 1.0, 0.0 },
> +			{ 1.0, 0.0, 0.0 },
> +	};
> +
> +	create_color_fb(data, &data->fb_rgb, red_green_blue);
> +	create_color_fb(data, &data->fb_rgr, red_green_red);
> +}
> +
> +static void check_dc3co_with_videoplayback_like_load(data_t *data)
> +{
> +	igt_plane_t *primary;
> +	uint32_t dc3co_prev_cnt;
> +	int i, delay;
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	dc3co_prev_cnt = read_dc_counter(data->drm_fd, CHECK_DC3CO);
> +	/* Calculate delay to generate idle frame in usec*/
> +	delay = ((1000 * 1000) / data->mode->vrefresh);
> +
> +	for (i = 0; i < VIDEO_FRAMES; i++) {

Please run the loop for a given amount of time, 130 frames is too
arbitrary.

> +		if (i % 2 == 0) {
> +			igt_plane_set_fb(primary, &data->fb_rgb);
> +			igt_display_commit(&data->display);
> +		} else {
> +			igt_plane_set_fb(primary, &data->fb_rgr);
> +			igt_display_commit(&data->display);
> +		}

You could simplify the above.

> +
> +		usleep(delay);
> +		igt_assert(psr2_wait_sleep_entry(data->debugfs_fd));

The above will wait up to 50 ms, whereas DC3co should be entered already
in the first idle frame. We also don't want to test PSR here, it belongs
to a separate test case. Waiting one and a half frame instead of one after
the flip should make sure DC3co is entered.

> +	}
> +
> +	check_dc_counter(data->drm_fd, CHECK_DC3CO, dc3co_prev_cnt);
> +}
> +
> +static void setup_dc3co(data_t *data)
> +{
> +	data->op_psr_mode = PSR_MODE_2;
> +	psr_enable(data->debugfs_fd, data->op_psr_mode);
> +	igt_require_f(edp_psr2_enabled(data),
> +		      "PSR2 is not enabled\n");
> +	igt_require(read_dc_counter(data->drm_fd, CHECK_DC3CO));
> +}
> +
> +static void test_dc3co_vpb_simulation(data_t *data)
> +{
> +	setup_output(data);
> +	setup_dc3co(data);
> +	setup_videoplayback(data);
> +	check_dc3co_with_videoplayback_like_load(data);
> +	cleanup_dc3co(data);
> +}
> +
>  static void test_dc_state_psr(data_t *data, int dc_flag)
>  {
>  	uint32_t dc_counter_before_psr;
> @@ -287,6 +413,13 @@ int main(int argc, char *argv[])
>  			     "Can't open /dev/cpu/0/msr.\n");
>  	}
>  
> +	igt_describe("This test simulate video playback "
> +		     "in order to validate DC3CO state "
> +		     "while PSR2 is active and in SLEEP state");
> +	igt_subtest("dc3co-vpb-simulation") {
> +		test_dc3co_vpb_simulation(&data);
> +	}
> +
>  	igt_describe("This test validates display engine entry to DC5 state "
>  		     "while PSR is active");
>  	igt_subtest("dc5-psr") {
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/2] Add a new IGT test to validate DC3CO state

[Ramalingam C]

On 2019-10-29 at 18:59:11 +0530, Jeevan B wrote:
> Add a subtest for DC3CO video playback case
> to generate selective frame update and validate
> that system stays in DC3CO state during execution.
> 
> v2: Changed PSR2 idle check to sleep check and addressed cosmetic changes.
> v3: Renamed a function and restructured code.
> v4: Cosmetic changes.
> v5: Removed DC5 check, Platform check and a function parameter.
>     Renamed a function name as per Arek and Imre's Comments.
> v6: Added a new function require_dc_counter as per Arek's Comments.
>     The test is now running based on time instead of the number of frames
>     increased frame delay from 1 to 1.5 as per Imre's Comments.
> v7: Removed psr2_active_sleep_check and corrected the switch indentation.
> v8: Changed skip to assert in read_dc_counter function and changed
>     IGT description
> 

Thanks for the patch. Merged with Arkadiusz's Ack.

-Ram
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  tests/i915/i915_pm_dc.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 155 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
> index 6132ebd..90d81cf 100644
> --- a/tests/i915/i915_pm_dc.c
> +++ b/tests/i915/i915_pm_dc.c
> @@ -32,6 +32,7 @@
>  #include "igt_psr.h"
>  #include "igt_sysfs.h"
>  #include "limits.h"
> +#include "time.h"
>  
>  /* DC State Flags */
>  #define CHECK_DC5	(1 << 0)
> @@ -39,12 +40,16 @@
>  #define CHECK_DC3CO	(1 << 2)
>  
>  typedef struct {
> +	double r, g, b;
> +} color_t;
> +
> +typedef struct {
>  	int drm_fd;
>  	int msr_fd;
>  	int debugfs_fd;
>  	uint32_t devid;
>  	igt_display_t display;
> -	struct igt_fb fb_white;
> +	struct igt_fb fb_white, fb_rgb, fb_rgr;
>  	enum psr_mode op_psr_mode;
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
> @@ -110,6 +115,42 @@ static void cleanup_dc_psr(data_t *data)
>  	igt_remove_fb(data->drm_fd, &data->fb_white);
>  }
>  
> +static void cleanup_dc3co_fbs(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);
> +	/* Clear Frame Buffers */
> +	igt_display_commit(&data->display);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgb);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgr);
> +}
> +
> +static void paint_rectangles(data_t *data,
> +			     drmModeModeInfo *mode,
> +			     color_t *colors,
> +			     igt_fb_t *fb)
> +{
> +	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +	int i, l = mode->hdisplay / 3;
> +	int rows_remaining = mode->hdisplay % 3;
> +
> +	/* Paint 3 solid rectangles. */
> +	for (i = 0 ; i < 3; i++) {
> +		igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
> +				colors[i].r, colors[i].g, colors[i].b);
> +	}
> +
> +	if (rows_remaining > 0)
> +		igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay,
> +				colors[i - 1].r, colors[i - 1].g,
> +				colors[i - 1].b);
> +
> +	igt_put_cairo_ctx(data->drm_fd, fb, cr);
> +}
> +
>  static void setup_primary(data_t *data)
>  {
>  	igt_plane_t *primary;
> @@ -127,6 +168,20 @@ static void setup_primary(data_t *data)
>  	igt_display_commit(&data->display);
>  }
>  
> +static void create_color_fb(data_t *data, igt_fb_t *fb, color_t *fb_color)
> +{
> +	int fb_id;
> +
> +	fb_id = igt_create_fb(data->drm_fd,
> +			      data->mode->hdisplay,
> +			      data->mode->vdisplay,
> +			      DRM_FORMAT_XRGB8888,
> +			      LOCAL_DRM_FORMAT_MOD_NONE,
> +			      fb);
> +	igt_assert(fb_id);
> +	paint_rectangles(data, data->mode, fb_color, fb);
> +}
> +
>  static uint32_t get_dc_counter(char *dc_data)
>  {
>  	char *e;
> @@ -150,13 +205,13 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
>  
>  	if (dc_flag & CHECK_DC5) {
>  		str = strstr(buf, "DC3 -> DC5 count");
> -		igt_skip_on_f(!str, "DC5 counter is not available\n");
> +		igt_assert_f(str, "DC5 counter is not available\n");
>  	} else if (dc_flag & CHECK_DC6) {
>  		str = strstr(buf, "DC5 -> DC6 count");
> -		igt_skip_on_f(!str, "DC6 counter is not available\n");
> +		igt_assert_f(str, "DC6 counter is not available\n");
>  	} else if (dc_flag & CHECK_DC3CO) {
>  		str = strstr(buf, "DC3CO count");
> -		igt_skip_on_f(!str, "DC3CO counter is not available\n");
> +		igt_assert_f(str, "DC3CO counter is not available\n");
>  	}
>  
>  	return get_dc_counter(str);
> @@ -178,10 +233,99 @@ static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
>  		     "%s state is not achieved\n", tmp);
>  }
>  
> +static void setup_videoplayback(data_t *data)
> +{
> +	color_t red_green_blue[] = {
> +		{ 1.0, 0.0, 0.0 },
> +		{ 0.0, 1.0, 0.0 },
> +		{ 0.0, 0.0, 1.0 },
> +	};
> +	color_t red_green_red[] = {
> +		{ 1.0, 0.0, 0.0 },
> +		{ 0.0, 1.0, 0.0 },
> +		{ 1.0, 0.0, 0.0 },
> +	};
> +
> +	create_color_fb(data, &data->fb_rgb, red_green_blue);
> +	create_color_fb(data, &data->fb_rgr, red_green_red);
> +}
> +
> +static void check_dc3co_with_videoplayback_like_load(data_t *data)
> +{
> +	igt_plane_t *primary;
> +	uint32_t dc3co_prev_cnt;
> +	int delay;
> +	time_t secs = 6;
> +	time_t startTime = time(NULL);
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	dc3co_prev_cnt = read_dc_counter(data->drm_fd, CHECK_DC3CO);
> +	/* Calculate delay to generate idle frame in usec*/
> +	delay = 1.5 * ((1000 * 1000) / data->mode->vrefresh);
> +
> +	while (time(NULL) - startTime < secs) {
> +		igt_plane_set_fb(primary, &data->fb_rgb);
> +		igt_display_commit(&data->display);
> +		usleep(delay);
> +
> +		igt_plane_set_fb(primary, &data->fb_rgr);
> +		igt_display_commit(&data->display);
> +		usleep(delay);
> +	}
> +
> +	check_dc_counter(data->drm_fd, CHECK_DC3CO, dc3co_prev_cnt);
> +}
> +
> +static void require_dc_counter(int drm_fd, int dc_flag)
> +{
> +	char buf[4096];
> +
> +	igt_debugfs_simple_read(drm_fd, "i915_dmc_info",
> +				buf, sizeof(buf));
> +
> +	switch (dc_flag) {
> +	case CHECK_DC3CO:
> +		igt_skip_on_f(strstr(buf, "DC3CO count"),
> +			      "DC3CO counter is not available\n");
> +		break;
> +	case CHECK_DC5:
> +		igt_skip_on_f(strstr(buf, "DC3 -> DC5 count"),
> +			      "DC5 counter is not available\n");
> +		break;
> +	case CHECK_DC6:
> +		igt_skip_on_f(strstr(buf, "DC5 -> DC6 count"),
> +			      "DC6 counter is not available\n");
> +		break;
> +	default:
> +		igt_assert_f(0, "Unknown DC counter %d\n", dc_flag);
> +	}
> +}
> +
> +static void setup_dc3co(data_t *data)
> +{
> +	data->op_psr_mode = PSR_MODE_2;
> +	psr_enable(data->debugfs_fd, data->op_psr_mode);
> +	igt_require_f(edp_psr2_enabled(data),
> +		      "PSR2 is not enabled\n");
> +}
> +
> +static void test_dc3co_vpb_simulation(data_t *data)
> +{
> +	require_dc_counter(data->drm_fd, CHECK_DC3CO);
> +	setup_output(data);
> +	setup_dc3co(data);
> +	setup_videoplayback(data);
> +	check_dc3co_with_videoplayback_like_load(data);
> +	cleanup_dc3co_fbs(data);
> +}
> +
>  static void test_dc_state_psr(data_t *data, int dc_flag)
>  {
>  	uint32_t dc_counter_before_psr;
>  
> +	require_dc_counter(data->drm_fd, dc_flag);
>  	dc_counter_before_psr = read_dc_counter(data->drm_fd, dc_flag);
>  	setup_output(data);
>  	setup_primary(data);
> @@ -243,6 +387,7 @@ static void test_dc_state_dpms(data_t *data, int dc_flag)
>  {
>  	uint32_t dc_counter;
>  
> +	require_dc_counter(data->drm_fd, dc_flag);
>  	setup_dc_dpms(data);
>  	dc_counter = read_dc_counter(data->drm_fd, dc_flag);
>  	dpms_off(data);
> @@ -279,6 +424,12 @@ int main(int argc, char *argv[])
>  			     "Can't open /dev/cpu/0/msr.\n");
>  	}
>  
> +	igt_describe("In this test we make sure that system enters DC3CO "
> +		     "when PSR2 is active and system is in SLEEP state");
> +	igt_subtest("dc3co-vpb-simulation") {
> +		test_dc3co_vpb_simulation(&data);
> +	}
> +
>  	igt_describe("This test validates display engine entry to DC5 state "
>  		     "while PSR is active");
>  	igt_subtest("dc5-psr") {
> -- 
> 2.7.4
> 
> _______________________________________________
> 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 2/2] Add a new IGT test to validate DC3CO state

[Arkadiusz Hiler]

On Tue, Oct 29, 2019 at 03:42:06PM +0530, Jeevan B wrote:
> Add a subtest for DC3CO video playback case
> to generate selective frame update and validate
> that system stays in DC3CO state during execution.
> 
> v2: Changed PSR2 idle check to sleep check and addressed cosmetic changes.
> v3: Renamed a function and restructured code according to Anshuman’s comments.
> v4: Cosmetic changes.
> v5: Removed DC5 check, Platform check and a function parameter.
>     Renamed a function name as per Arek and Imre's Comments.
> v6: Added a new function require_dc_counter as per Arek's Comments.
>     The test is now running based on time instead of the number of frames
>     increased frame delay from 1 to 1.5 as per Imre's Comments.
> v7: Removed psr2_active_sleep_check and corrected the switch indentation.
> 
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  tests/i915/i915_pm_dc.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 151 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
> index 44c6538..46db135 100644
> --- a/tests/i915/i915_pm_dc.c
> +++ b/tests/i915/i915_pm_dc.c
> @@ -32,6 +32,7 @@
>  #include "igt_psr.h"
>  #include "igt_sysfs.h"
>  #include "limits.h"
> +#include "time.h"
>  
>  /* DC State Flags */
>  #define CHECK_DC5	(1 << 0)
> @@ -39,12 +40,16 @@
>  #define CHECK_DC3CO	(1 << 2)
>  
>  typedef struct {
> +	double r, g, b;
> +} color_t;
> +
> +typedef struct {
>  	int drm_fd;
>  	int msr_fd;
>  	int debugfs_fd;
>  	uint32_t devid;
>  	igt_display_t display;
> -	struct igt_fb fb_white;
> +	struct igt_fb fb_white, fb_rgb, fb_rgr;
>  	enum psr_mode op_psr_mode;
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
> @@ -110,6 +115,42 @@ static void cleanup_dc_psr(data_t *data)
>  	igt_remove_fb(data->drm_fd, &data->fb_white);
>  }
>  
> +static void cleanup_dc3co_fbs(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);
> +	/* Clear Frame Buffers */
> +	igt_display_commit(&data->display);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgb);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgr);
> +}
> +
> +static void paint_rectangles(data_t *data,
> +			     drmModeModeInfo *mode,
> +			     color_t *colors,
> +			     igt_fb_t *fb)
> +{
> +	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +	int i, l = mode->hdisplay / 3;
> +	int rows_remaining = mode->hdisplay % 3;
> +
> +	/* Paint 3 solid rectangles. */
> +	for (i = 0 ; i < 3; i++) {
> +		igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
> +				colors[i].r, colors[i].g, colors[i].b);
> +	}
> +
> +	if (rows_remaining > 0)
> +		igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay,
> +				colors[i - 1].r, colors[i - 1].g,
> +				colors[i - 1].b);
> +
> +	igt_put_cairo_ctx(data->drm_fd, fb, cr);
> +}
> +
>  static void setup_primary(data_t *data)
>  {
>  	igt_plane_t *primary;
> @@ -127,6 +168,20 @@ static void setup_primary(data_t *data)
>  	igt_display_commit(&data->display);
>  }
>  
> +static void create_color_fb(data_t *data, igt_fb_t *fb, color_t *fb_color)
> +{
> +	int fb_id;
> +
> +	fb_id = igt_create_fb(data->drm_fd,
> +			      data->mode->hdisplay,
> +			      data->mode->vdisplay,
> +			      DRM_FORMAT_XRGB8888,
> +			      LOCAL_DRM_FORMAT_MOD_NONE,
> +			      fb);
> +	igt_assert(fb_id);
> +	paint_rectangles(data, data->mode, fb_color, fb);
> +}
> +
>  static uint32_t get_dc_counter(char *dc_data)
>  {
>  	char *e;
> @@ -183,6 +238,94 @@ static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
>  		     "%s state is not achieved\n", tmp);
>  }
>  
> +static void setup_videoplayback(data_t *data)
> +{
> +	color_t red_green_blue[] = {
> +		{ 1.0, 0.0, 0.0 },
> +		{ 0.0, 1.0, 0.0 },
> +		{ 0.0, 0.0, 1.0 },
> +	};
> +	color_t red_green_red[] = {
> +		{ 1.0, 0.0, 0.0 },
> +		{ 0.0, 1.0, 0.0 },
> +		{ 1.0, 0.0, 0.0 },
> +	};
> +
> +	create_color_fb(data, &data->fb_rgb, red_green_blue);
> +	create_color_fb(data, &data->fb_rgr, red_green_red);
> +}
> +
> +static void check_dc3co_with_videoplayback_like_load(data_t *data)
> +{
> +	igt_plane_t *primary;
> +	uint32_t dc3co_prev_cnt;
> +	int delay;
> +	time_t secs = 6;
> +	time_t startTime = time(NULL);
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	dc3co_prev_cnt = read_dc_counter(data->drm_fd, CHECK_DC3CO);
> +	/* Calculate delay to generate idle frame in usec*/
> +	delay = 1.5 * ((1000 * 1000) / data->mode->vrefresh);
> +
> +	while (time(NULL) - startTime < secs) {
> +		igt_plane_set_fb(primary, &data->fb_rgb);
> +		igt_display_commit(&data->display);
> +		usleep(delay);
> +
> +		igt_plane_set_fb(primary, &data->fb_rgr);
> +		igt_display_commit(&data->display);
> +		usleep(delay);
> +	}
> +
> +	check_dc_counter(data->drm_fd, CHECK_DC3CO, dc3co_prev_cnt);
> +}
> +
> +static void require_dc_counter(int drm_fd, int dc_flag)

Nice! Thanks for introducing this. Now we should use that for the tests
that are using other counters so they have the requirements up-front and
don't skip on reading.

As a general rule of thumb:
 * skip only from functions that have *skip* or *require* in the name
 * let *read* and other functions to fail

Something like:

diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
index 43af86d4..64d81440 100644
--- a/tests/i915/i915_pm_dc.c
+++ b/tests/i915/i915_pm_dc.c
@@ -205,13 +205,13 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
 
 	if (dc_flag & CHECK_DC5) {
 		str = strstr(buf, "DC3 -> DC5 count");
-		igt_skip_on_f(!str, "DC5 counter is not available\n");
+		igt_assert_f(str, "DC5 counter is not available\n");
 	} else if (dc_flag & CHECK_DC6) {
 		str = strstr(buf, "DC5 -> DC6 count");
-		igt_skip_on_f(!str, "DC6 counter is not available\n");
+		igt_assert_f(str, "DC6 counter is not available\n");
 	} else if (dc_flag & CHECK_DC3CO) {
 		str = strstr(buf, "DC3CO count");
-		igt_skip_on_f(!str, "DC3CO counter is not available\n");
+		igt_assert_f(str, "DC3CO counter is not available\n");
 	}
 
 	return get_dc_counter(str);
@@ -305,7 +305,6 @@ static void require_dc_counter(int drm_fd, int dc_flag)
 
 static void setup_dc3co(data_t *data)
 {
-	require_dc_counter(data->drm_fd, CHECK_DC3CO);
 	data->op_psr_mode = PSR_MODE_2;
 	psr_enable(data->debugfs_fd, data->op_psr_mode);
 	igt_require_f(edp_psr2_enabled(data),
@@ -314,6 +313,7 @@ static void setup_dc3co(data_t *data)
 
 static void test_dc3co_vpb_simulation(data_t *data)
 {
+	require_dc_counter(data->drm_fd, CHECK_DC3CO);
 	setup_output(data);
 	setup_dc3co(data);
 	setup_videoplayback(data);
@@ -325,6 +325,7 @@ static void test_dc_state_psr(data_t *data, int dc_flag)
 {
 	uint32_t dc_counter_before_psr;
 
+	require_dc_counter(data->drm_fd, dc_flag);
 	dc_counter_before_psr = read_dc_counter(data->drm_fd, dc_flag);
 	setup_output(data);
 	setup_primary(data);
@@ -386,6 +387,7 @@ static void test_dc_state_dpms(data_t *data, int dc_flag)
 {
 	uint32_t dc_counter;
 
+	require_dc_counter(data->drm_fd, dc_flag);
 	setup_dc_dpms(data);
 	dc_counter = read_dc_counter(data->drm_fd, dc_flag);
 	dpms_off(data);


> +{
> +	char buf[4096];
> +
> +	igt_debugfs_simple_read(drm_fd, "i915_dmc_info",
> +				buf, sizeof(buf));
> +
> +	switch (dc_flag) {
> +	case CHECK_DC3CO:
> +		igt_skip_on_f(strstr(buf, "DC3CO count"),
> +			      "DC3CO counter is not available\n");
> +		break;
> +	case CHECK_DC5:
> +		igt_skip_on_f(strstr(buf, "DC3 -> DC5 count"),
> +			      "DC5 counter is not available\n");
> +		break;
> +	case CHECK_DC6:
> +		igt_skip_on_f(strstr(buf, "DC5 -> DC6 count"),
> +			      "DC6 counter is not available\n");
> +		break;
> +	default:
> +		igt_assert_f(0, "Unknown DC counter %d\n", dc_flag);
> +	}
> +}
> +
> +static void setup_dc3co(data_t *data)
> +{
> +	require_dc_counter(data->drm_fd, CHECK_DC3CO);
> +	data->op_psr_mode = PSR_MODE_2;
> +	psr_enable(data->debugfs_fd, data->op_psr_mode);
> +	igt_require_f(edp_psr2_enabled(data),
> +		      "PSR2 is not enabled\n");
> +}
> +
> +static void test_dc3co_vpb_simulation(data_t *data)
> +{
> +	setup_output(data);
> +	setup_dc3co(data);
> +	setup_videoplayback(data);
> +	check_dc3co_with_videoplayback_like_load(data);
> +	cleanup_dc3co_fbs(data);
> +}
> +
>  static void test_dc_state_psr(data_t *data, int dc_flag)
>  {
>  	uint32_t dc_counter_before_psr;
> @@ -284,6 +427,13 @@ int main(int argc, char *argv[])
>  			     "Can't open /dev/cpu/0/msr.\n");
>  	}
>  
> +	igt_describe("This test simulate video playback "
> +		     "in order to validate DC3CO state "
> +		     "while PSR2 is active and in SLEEP state");

"This test" is something that the reader already knows. Using this kind of
reference makes sense when you need to address the whole thing few
sentences down the line. Here it just unecessarily inflates the word
count.

I suggest you to rephrase it like that:
"Make sure that we enter DC3CO when PSR2 is active and we are in SLEEP
state."

There are few more tests here that could use similar treatement so feel
free to do that in some follow up series later.

> +	igt_subtest("dc3co-vpb-simulation") {
> +		test_dc3co_vpb_simulation(&data);
> +	}
> +
>  	igt_describe("This test validates display engine entry to DC5 state "
>  		     "while PSR is active");
>  	igt_subtest("dc5-psr") {
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/2] Add a new IGT test to validate DC3CO state

[Imre Deak]

On Tue, Oct 29, 2019 at 12:54:29PM +0530, Jeevan B wrote:
> Add a subtest for DC3CO video playback case
> to generate selective frame update and validate
> that system stays in DC3CO state during execution.
> 
> v2: Changed PSR2 idle check to sleep check and addressed cosmetic changes.
> v3: Renamed a function and restructured code according to Anshuman’s comments.
> v4: Cosmetic changes.
> v5: Removed DC5 check, Platform check and a function parameter.
>     Renamed a function name as per Arek and Imre's Comments.
> v6: Added a new function require_dc_counter as per Arek's Comments.
>     The test is now running based on time instead of the number of frames
>     increased frame delay from 1 to 1.5 as per Imre's Comments.
> 
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  lib/igt_psr.c           |  10 +++
>  lib/igt_psr.h           |   1 +
>  tests/i915/i915_pm_dc.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 169 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index 6d19268..5276ff4 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -38,6 +38,16 @@ static bool psr_active_check(int debugfs_fd, enum psr_mode mode)
>  	return strstr(buf, state);
>  }
>  
> +bool psr2_active_sleep_check(int debugfs_fd)
> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +	const char *state = "SLEEP";
> +
> +	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> +				sizeof(buf));
> +	return strstr(buf, state);
> +}
> +
>  /*
>   * For PSR1, we wait until PSR is active. We wait until DEEP_SLEEP for PSR2.
>   */
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index ca38573..e022434 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -36,6 +36,7 @@ enum psr_mode {
>  };
>  
>  bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
> +bool psr2_active_sleep_check(int debugfs_fd);
>  bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
>  bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
>  bool psr_enable(int debugfs_fd, enum psr_mode);
> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
> index 44c6538..10148cd 100644
> --- a/tests/i915/i915_pm_dc.c
> +++ b/tests/i915/i915_pm_dc.c
> @@ -32,6 +32,7 @@
>  #include "igt_psr.h"
>  #include "igt_sysfs.h"
>  #include "limits.h"
> +#include "time.h"
>  
>  /* DC State Flags */
>  #define CHECK_DC5	(1 << 0)
> @@ -39,12 +40,16 @@
>  #define CHECK_DC3CO	(1 << 2)
>  
>  typedef struct {
> +	double r, g, b;
> +} color_t;
> +
> +typedef struct {
>  	int drm_fd;
>  	int msr_fd;
>  	int debugfs_fd;
>  	uint32_t devid;
>  	igt_display_t display;
> -	struct igt_fb fb_white;
> +	struct igt_fb fb_white, fb_rgb, fb_rgr;
>  	enum psr_mode op_psr_mode;
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
> @@ -110,6 +115,42 @@ static void cleanup_dc_psr(data_t *data)
>  	igt_remove_fb(data->drm_fd, &data->fb_white);
>  }
>  
> +static void cleanup_dc3co_fbs(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);
> +	/* Clear Frame Buffers */
> +	igt_display_commit(&data->display);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgb);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgr);
> +}
> +
> +static void paint_rectangles(data_t *data,
> +			     drmModeModeInfo *mode,
> +			     color_t *colors,
> +			     igt_fb_t *fb)
> +{
> +	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +	int i, l = mode->hdisplay / 3;
> +	int rows_remaining = mode->hdisplay % 3;
> +
> +	/* Paint 3 solid rectangles. */
> +	for (i = 0 ; i < 3; i++) {
> +		igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
> +				colors[i].r, colors[i].g, colors[i].b);
> +	}
> +
> +	if (rows_remaining > 0)
> +		igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay,
> +				colors[i - 1].r, colors[i - 1].g,
> +				colors[i - 1].b);
> +
> +	igt_put_cairo_ctx(data->drm_fd, fb, cr);
> +}
> +
>  static void setup_primary(data_t *data)
>  {
>  	igt_plane_t *primary;
> @@ -127,6 +168,20 @@ static void setup_primary(data_t *data)
>  	igt_display_commit(&data->display);
>  }
>  
> +static void create_color_fb(data_t *data, igt_fb_t *fb, color_t *fb_color)
> +{
> +	int fb_id;
> +
> +	fb_id = igt_create_fb(data->drm_fd,
> +			      data->mode->hdisplay,
> +			      data->mode->vdisplay,
> +			      DRM_FORMAT_XRGB8888,
> +			      LOCAL_DRM_FORMAT_MOD_NONE,
> +			      fb);
> +	igt_assert(fb_id);
> +	paint_rectangles(data, data->mode, fb_color, fb);
> +}
> +
>  static uint32_t get_dc_counter(char *dc_data)
>  {
>  	char *e;
> @@ -167,6 +222,11 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
>  	return get_dc_counter(str);
>  }
>  
> +static bool psr2_wait_sleep_entry(int debugfs_fd)
> +{
> +	return igt_wait(psr2_active_sleep_check(debugfs_fd), 50, 10);
> +}
> +
>  static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count)
>  {
>  	return igt_wait(read_dc_counter(drm_fd, dc_flag) >
> @@ -183,6 +243,96 @@ static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
>  		     "%s state is not achieved\n", tmp);
>  }
>  
> +static void setup_videoplayback(data_t *data)
> +{
> +	color_t red_green_blue[] = {
> +		{ 1.0, 0.0, 0.0 },
> +		{ 0.0, 1.0, 0.0 },
> +		{ 0.0, 0.0, 1.0 },
> +	};
> +	color_t red_green_red[] = {
> +		{ 1.0, 0.0, 0.0 },
> +		{ 0.0, 1.0, 0.0 },
> +		{ 1.0, 0.0, 0.0 },
> +	};
> +
> +	create_color_fb(data, &data->fb_rgb, red_green_blue);
> +	create_color_fb(data, &data->fb_rgr, red_green_red);
> +}
> +
> +static void check_dc3co_with_videoplayback_like_load(data_t *data)
> +{
> +	igt_plane_t *primary;
> +	uint32_t dc3co_prev_cnt;
> +	int delay;
> +	time_t secs = 6;
> +	time_t startTime = time(NULL);
> +
> +	primary = igt_output_get_plane_type(data->output,
> +					    DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	dc3co_prev_cnt = read_dc_counter(data->drm_fd, CHECK_DC3CO);
> +	/* Calculate delay to generate idle frame in usec*/
> +	delay = 1.5 * ((1000 * 1000) / data->mode->vrefresh);
> +
> +	while (time(NULL) - startTime < secs) {
> +		igt_plane_set_fb(primary, &data->fb_rgb);
> +		igt_display_commit(&data->display);
> +		usleep(delay);
> +
> +		igt_plane_set_fb(primary, &data->fb_rgr);
> +		igt_display_commit(&data->display);
> +		usleep(delay);
> +
> +		igt_assert(psr2_wait_sleep_entry(data->debugfs_fd));

The above wait was not remove (while you still increased the delay
between flips to 1.5 frame time as we agreed).

> +	}
> +
> +	check_dc_counter(data->drm_fd, CHECK_DC3CO, dc3co_prev_cnt);
> +}
> +
> +static void require_dc_counter(int drm_fd, int dc_flag)
> +{
> +	char buf[4096];
> +
> +	igt_debugfs_simple_read(drm_fd, "i915_dmc_info",
> +				buf, sizeof(buf));
> +
> +	switch (dc_flag) {
> +	case CHECK_DC3CO:
> +		igt_skip_on_f(strstr(buf, "DC3CO count"),
> +			      "DC3CO counter is not available\n");
> +	break;

Please use the kernel coding style, break needs one more indent.

> +	case CHECK_DC5:
> +		igt_skip_on_f(strstr(buf, "DC3 -> DC5 count"),
> +			      "DC5 counter is not available\n");
> +	break;
> +	case CHECK_DC6:
> +		igt_skip_on_f(strstr(buf, "DC5 -> DC6 count"),
> +			      "DC6 counter is not available\n");
> +		break;
> +	default:
> +		igt_assert_f(0, "Unknown DC counter %d\n", dc_flag);
> +	}
> +}
> +
> +static void setup_dc3co(data_t *data)
> +{
> +	require_dc_counter(data->drm_fd, CHECK_DC3CO);
> +	data->op_psr_mode = PSR_MODE_2;
> +	psr_enable(data->debugfs_fd, data->op_psr_mode);
> +	igt_require_f(edp_psr2_enabled(data),
> +		      "PSR2 is not enabled\n");
> +}
> +
> +static void test_dc3co_vpb_simulation(data_t *data)
> +{
> +	setup_output(data);
> +	setup_dc3co(data);
> +	setup_videoplayback(data);
> +	check_dc3co_with_videoplayback_like_load(data);
> +	cleanup_dc3co_fbs(data);
> +}
> +
>  static void test_dc_state_psr(data_t *data, int dc_flag)
>  {
>  	uint32_t dc_counter_before_psr;
> @@ -284,6 +434,13 @@ int main(int argc, char *argv[])
>  			     "Can't open /dev/cpu/0/msr.\n");
>  	}
>  
> +	igt_describe("This test simulate video playback "
> +		     "in order to validate DC3CO state "
> +		     "while PSR2 is active and in SLEEP state");
> +	igt_subtest("dc3co-vpb-simulation") {
> +		test_dc3co_vpb_simulation(&data);
> +	}
> +
>  	igt_describe("This test validates display engine entry to DC5 state "
>  		     "while PSR is active");
>  	igt_subtest("dc5-psr") {
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/2] Add a new IGT test to validate DC3CO state

[Jeevan B]

Add a subtest for DC3CO video playback case
to generate selective frame update and validate
that system stays in DC3CO state during execution.

v2: Changed PSR2 idle check to sleep check and addressed
cosmetic changes.

v3: Renamed a function and restructured code according
to Anshuman’s comments.

Signed-off-by: Jeevan B <jeevan.b@intel.com>
---
 lib/igt_psr.c           |  11 ++++
 lib/igt_psr.h           |   1 +
 tests/i915/i915_pm_dc.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 157 insertions(+), 1 deletion(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index b92ea73..7806ce9 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -36,6 +36,17 @@ static bool psr_active_check(int debugfs_fd, enum psr_mode mode)
 	return strstr(buf, state);
 }
 
+bool psr2_active_sleep_check(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+	const char *state = "SLEEP";
+
+	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				sizeof(buf));
+
+	return strstr(buf, state);
+}
+
 static inline const char *psr_active_state_get(enum psr_mode mode)
 {
 	return mode == PSR_MODE_1 ? "SRDENT" : "DEEP_SLEEP";
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index ca38573..a0627dc 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -35,6 +35,7 @@ enum psr_mode {
 	PSR_MODE_2
 };
 
+bool psr2_active_sleep_check(int debugfs_fd);
 bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
 bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
 bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
index 19d8a78..dc0a215 100644
--- a/tests/i915/i915_pm_dc.c
+++ b/tests/i915/i915_pm_dc.c
@@ -38,13 +38,20 @@
 #define CHECK_DC6	2
 #define CHECK_DC3CO     4
 
+/*Number of Frames Video Playback*/
+#define VIDEO_FRAMES 100
+
+typedef struct {
+	double r, g, b;
+} color_t;
+
 typedef struct {
 	int drm_fd;
 	int msr_fd;
 	int debugfs_fd;
 	uint32_t devid;
 	igt_display_t display;
-	struct igt_fb fb_white;
+	struct igt_fb fb_white, fb_rgb, fb_rgr;
 	enum psr_mode op_psr_mode;
 	drmModeModeInfo *mode;
 	igt_output_t *output;
@@ -114,6 +121,41 @@ static void cleanup_dc_psr(data_t *data)
 	igt_remove_fb(data->drm_fd, &data->fb_white);
 }
 
+static void cleanup_dc3co(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);
+	/*Clear Frame Buffers*/
+	igt_display_commit(&data->display);
+	igt_remove_fb(data->drm_fd, &data->fb_rgb);
+	igt_remove_fb(data->drm_fd, &data->fb_rgr);
+}
+
+static void paint_rectangles(data_t *data,
+				drmModeModeInfo *mode,
+				color_t *colors,
+				igt_fb_t *fb)
+{
+	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
+	int i, l = mode->hdisplay / 3;
+	int rows_remaining = mode->hdisplay % 3;
+
+	/* Paint 3 solid rectangles. */
+	for (i = 0 ; i < 3; i++) {
+		igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
+				colors[i].r, colors[i].g, colors[i].b);
+	}
+
+	if (rows_remaining > 0)
+		igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay,
+				colors[i-1].r, colors[i-1].g, colors[i-1].b);
+
+	igt_put_cairo_ctx(data->drm_fd, fb, cr);
+}
+
 static void setup_primary(data_t *data)
 {
 	igt_plane_t *primary;
@@ -131,6 +173,25 @@ static void setup_primary(data_t *data)
 	igt_display_commit(&data->display);
 }
 
+static void create_clr_fb(data_t *data, igt_fb_t *fb, color_t *fb_color)
+{
+	igt_plane_t *primary;
+	int fb_id;
+
+	primary = igt_output_get_plane_type(data->output,
+						DRM_PLANE_TYPE_PRIMARY);
+
+	igt_plane_set_fb(primary, NULL);
+	fb_id = igt_create_fb(data->drm_fd,
+				data->mode->hdisplay,
+				data->mode->vdisplay,
+				DRM_FORMAT_XRGB8888,
+				LOCAL_DRM_FORMAT_MOD_NONE,
+				fb);
+	igt_assert(fb_id);
+	paint_rectangles(data, data->mode, fb_color, fb);
+}
+
 static uint32_t get_dc_counter(char *dc_data)
 {
 	char *e;
@@ -171,6 +232,11 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
 	return get_dc_counter(str);
 }
 
+static bool psr2_wait_sleep_entry(int debugfs_fd)
+{
+	return igt_wait(psr2_active_sleep_check(debugfs_fd), 50, 10);
+}
+
 static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count)
 {
 	return igt_wait(read_dc_counter(drm_fd, dc_flag) >
@@ -187,6 +253,77 @@ static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
 		     "%s state is not achieved\n", tmp);
 }
 
+static void setup_vpb(data_t *data)
+{
+	color_t red_green_blue[] = {
+		{ 1.0, 0.0, 0.0 },
+		{ 0.0, 1.0, 0.0 },
+		{ 0.0, 0.0, 1.0 },
+	};
+	color_t red_green_red[] = {
+		{ 1.0, 0.0, 0.0 },
+		{ 0.0, 1.0, 0.0 },
+		{ 1.0, 0.0, 0.0 },
+	};
+
+	setup_output(data);
+
+	create_clr_fb(data, &data->fb_rgb, red_green_blue);
+	create_clr_fb(data, &data->fb_rgr, red_green_red);
+}
+
+static void run_videoplayback(data_t *data, int dc_flag)
+{
+	igt_plane_t *primary;
+	uint32_t dc3co_cnt_before_psr;
+	int i, delay;
+
+	primary = igt_output_get_plane_type(data->output,
+						DRM_PLANE_TYPE_PRIMARY);
+
+	igt_plane_set_fb(primary, NULL);
+
+	dc3co_cnt_before_psr = read_dc_counter(data->drm_fd, dc_flag);
+	/*Calculate delay to generate idle frame*/
+	delay = ((1000*1000)/data->mode->vrefresh);
+
+	for (i = 0; i < VIDEO_FRAMES; i++) {
+		if (i % 2 == 0) {
+			igt_plane_set_fb(primary, &data->fb_rgb);
+			igt_display_commit(&data->display);
+		} else {
+			igt_plane_set_fb(primary, &data->fb_rgr);
+			igt_display_commit(&data->display);
+		}
+		usleep(delay);
+		igt_assert(psr2_wait_sleep_entry(data->debugfs_fd));
+	}
+	check_dc_counter(data->drm_fd, dc_flag, dc3co_cnt_before_psr);
+}
+
+static void setup_dc3co(data_t *data)
+{
+	igt_require(IS_TIGERLAKE(data->devid));
+	data->op_psr_mode = PSR_MODE_2;
+	psr_enable(data->debugfs_fd, data->op_psr_mode);
+	igt_require_f(edp_psr2_enabled(data),
+			"PSR2 is not enabled\n");
+}
+
+static void test_dc3co_vpb_simulation(data_t *data, int dc_flag)
+{
+	uint32_t dc5_cnt_before, dc5_cnt_after;
+
+	setup_dc3co(data);
+	setup_vpb(data);
+	dc5_cnt_before = read_dc_counter(data->drm_fd, CHECK_DC5);
+	run_videoplayback(data, dc_flag);
+	dc5_cnt_after = read_dc_counter(data->drm_fd, CHECK_DC5);
+	igt_assert_f(dc5_cnt_after == dc5_cnt_before,
+			"DC State moved to DC5\n");
+	cleanup_dc3co(data);
+}
+
 static void test_dc_state_psr(data_t *data, int dc_flag)
 {
 	uint32_t dc_counter_before_psr;
@@ -288,6 +425,13 @@ int main(int argc, char *argv[])
 			     "Can't open /dev/cpu/0/msr.\n");
 	}
 
+	igt_describe("This test simulate videoplay back "
+		     "in order to validate DC3CO state "
+		     "while PSR2 is active and in SLEEP state");
+	igt_subtest("dc3co-vpb-simulation") {
+		test_dc3co_vpb_simulation(&data, CHECK_DC3CO);
+	}
+
 	igt_describe("This test validates display engine entry to DC5 state "
 		     "while PSR is active");
 	igt_subtest("dc5-psr") {
-- 
2.7.4

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

Re: [igt-dev] [PATCH i-g-t 2/2] Add a new IGT test to validate DC3CO state

[Anshuman Gupta]

On 2019-09-25 at 17:42:17 +0530, Jeevan B wrote:
> Add a subtest for DC3CO video playback case
> to generate selective frame update and validate
> that system stays in DC3CO state during execution.
> 
> v2: Changed PSR2 idle check to sleep check and addressed cosmetic changes.
> (Anshuman)
> 
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
>  lib/igt_psr.c           |  11 ++++
>  lib/igt_psr.h           |   1 +
>  tests/i915/i915_pm_dc.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 154 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index b92ea73..7806ce9 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -36,6 +36,17 @@ static bool psr_active_check(int debugfs_fd, enum psr_mode mode)
>  	return strstr(buf, state);
>  }
>  
> +bool psr2_active_sleep_check(int debugfs_fd)
> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +	const char *state = "SLEEP";
> +
> +	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> +				sizeof(buf));
> +
> +	return strstr(buf, state);
> +}
> +
>  static inline const char *psr_active_state_get(enum psr_mode mode)
>  {
>  	return mode == PSR_MODE_1 ? "SRDENT" : "DEEP_SLEEP";
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index ca38573..a0627dc 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -35,6 +35,7 @@ enum psr_mode {
>  	PSR_MODE_2
>  };
>  
> +bool psr2_active_sleep_check(int debugfs_fd);
>  bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
>  bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
>  bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
> index 19d8a78..7f93792 100644
> --- a/tests/i915/i915_pm_dc.c
> +++ b/tests/i915/i915_pm_dc.c
> @@ -38,13 +38,22 @@
>  #define CHECK_DC6	2
>  #define CHECK_DC3CO     4
>  
> +/*Number of Frames Video Playback*/
> +#define VIDEO_FRAMES 100
> +
> +typedef struct {
> +	double r, g, b;
> +} color_t;
> +
> +igt_plane_t *primary;
> +
>  typedef struct {
>  	int drm_fd;
>  	int msr_fd;
>  	int debugfs_fd;
>  	uint32_t devid;
>  	igt_display_t display;
> -	struct igt_fb fb_white;
> +	struct igt_fb fb_white, fb_rgb, fb_rgr;
>  	enum psr_mode op_psr_mode;
>  	drmModeModeInfo *mode;
>  	igt_output_t *output;
> @@ -53,6 +62,7 @@ typedef struct {
>  
>  static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count);
>  static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count);
> +static bool psr2_wait_sleep_update(int debugfs_fd);
>  
>  static void setup_output(data_t *data)
>  {
> @@ -105,8 +115,6 @@ static bool edp_psr2_enabled(data_t *data)
>  
>  static void cleanup_dc_psr(data_t *data)
>  {
> -	igt_plane_t *primary;
unwanted change, how will this build if u remove primary ?
> -
>  	primary = igt_output_get_plane_type(data->output,
>  					    DRM_PLANE_TYPE_PRIMARY);
>  	igt_plane_set_fb(primary, NULL);
> @@ -114,10 +122,43 @@ static void cleanup_dc_psr(data_t *data)
>  	igt_remove_fb(data->drm_fd, &data->fb_white);
>  }
>  
> -static void setup_primary(data_t *data)
> +static void cleanup_dc3co(data_t *data)
> +{
> +	primary = igt_output_get_plane_type(data->output,
> +						DRM_PLANE_TYPE_PRIMARY);
> +	igt_plane_set_fb(primary, NULL);
> +	/*Clear Frame Buffers*/
> +	igt_display_commit(&data->display);
> +	igt_remove_fb(data->drm_fd, &data->fb_rgb);
> +
> +	igt_display_commit(&data->display);
we do not need the second commit.
> +	igt_remove_fb(data->drm_fd, &data->fb_rgr);
> +}
> +
> +static void paint_rectangles(data_t *data,
> +				drmModeModeInfo *mode,
> +				color_t *colors,
> +				igt_fb_t *fb)
>  {
> -	igt_plane_t *primary;
> +	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
> +	int i, l = mode->hdisplay / 3;
> +	int rows_remaining = mode->hdisplay % 3;
> +
> +	/* Paint 3 solid rectangles. */
> +	for (i = 0 ; i < 3; i++) {
> +		igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
> +				colors[i].r, colors[i].g, colors[i].b);
> +	}
>  
> +	if (rows_remaining > 0)
> +		igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay,
> +				colors[i-1].r, colors[i-1].g, colors[i-1].b);
> +
> +	igt_put_cairo_ctx(data->drm_fd, fb, cr);
> +}
> +
> +static void setup_primary(data_t *data)
> +{
>  	primary = igt_output_get_plane_type(data->output,
>  					    DRM_PLANE_TYPE_PRIMARY);
>  	igt_plane_set_fb(primary, NULL);
> @@ -131,6 +172,24 @@ static void setup_primary(data_t *data)
>  	igt_display_commit(&data->display);
>  }
>  
> +static void create_clr_fb(data_t *data, igt_fb_t *fb, color_t *fb_color)
> +{
> +	int fb_id;
> +
> +	primary = igt_output_get_plane_type(data->output,
> +						DRM_PLANE_TYPE_PRIMARY);
> +
> +	igt_plane_set_fb(primary, NULL);
> +	fb_id = igt_create_fb(data->drm_fd,
> +				data->mode->hdisplay,
> +				data->mode->vdisplay,
> +				DRM_FORMAT_XRGB8888,
> +				LOCAL_DRM_FORMAT_MOD_NONE,
> +				fb);
> +	igt_assert(fb_id);
> +	paint_rectangles(data, data->mode, fb_color, fb);
> +}
> +
>  static uint32_t get_dc_counter(char *dc_data)
>  {
>  	char *e;
> @@ -171,6 +230,11 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
>  	return get_dc_counter(str);
>  }
>  
> +static bool psr2_wait_sleep_update(int debugfs_fd)
psr2_wait_sleep_entry looks suitable ?
> +{
> +	return igt_wait(psr2_active_sleep_check(debugfs_fd), 100, 10);
I think our plan was to have a total delay less then 100?
> +}
> +
>  static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count)
>  {
>  	return igt_wait(read_dc_counter(drm_fd, dc_flag) >
> @@ -187,6 +251,73 @@ static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
>  		     "%s state is not achieved\n", tmp);
>  }
>  
> +static void setup_vpb(data_t *data)
> +{
> +	color_t red_green_blue[] = {
> +		{ 1.0, 0.0, 0.0 },
> +		{ 0.0, 1.0, 0.0 },
> +		{ 0.0, 0.0, 1.0 },
> +	};
> +	color_t red_green_red[] = {
> +		{ 1.0, 0.0, 0.0 },
> +		{ 0.0, 1.0, 0.0 },
> +		{ 1.0, 0.0, 0.0 },
> +	};
> +
> +	setup_output(data);
> +
> +	create_clr_fb(data, &data->fb_rgb, red_green_blue);
> +	create_clr_fb(data, &data->fb_rgr, red_green_red);
> +}
> +
> +static void run_videoplayback(data_t *data, int dc_flag)
> +{
> +	uint32_t dc3co_cnt_before_psr;
> +	int i, delay;
> +
> +	igt_plane_set_fb(primary, NULL);
> +
> +	dc3co_cnt_before_psr = read_dc_counter(data->drm_fd, dc_flag);
> +	/*Calculate delay to generate idle frame*/
> +	delay = ((1000*1000)/data->mode->vrefresh);
> +
> +	for (i = 0; i < VIDEO_FRAMES; i++) {
> +		if (i % 2 == 0) {
> +			igt_plane_set_fb(primary, &data->fb_rgb);
> +			igt_display_commit(&data->display);
> +		} else {
> +			igt_plane_set_fb(primary, &data->fb_rgr);
> +			igt_display_commit(&data->display);
> +		}
> +		usleep(delay);
> +		igt_assert(psr2_wait_sleep_update(data->debugfs_fd));
> +	}
> +	check_dc_counter(data->drm_fd, dc_flag, dc3co_cnt_before_psr);
> +}
> +
> +static void setup_dc3co(data_t *data)
> +{
> +	igt_require(IS_TIGERLAKE(data->devid));
> +	data->op_psr_mode = PSR_MODE_2;
> +	psr_enable(data->debugfs_fd, data->op_psr_mode);
> +	igt_require_f(edp_psr2_enabled(data),
> +			"PSR2 is not enabled\n");
> +}
> +
> +static void test_dc3co_vpb_simulation(data_t *data, int dc_flag)
> +{
> +	uint32_t dc5_cnt_before, dc5_cnt_after;
> +
> +	setup_dc3co(data);
> +	setup_vpb(data);
> +	dc5_cnt_before = read_dc_counter(data->drm_fd, CHECK_DC5);
> +	run_videoplayback(data, dc_flag);
> +	dc5_cnt_after = read_dc_counter(data->drm_fd, CHECK_DC5);
> +	igt_assert_f(dc5_cnt_after == dc5_cnt_before,
> +			"DC State moved to DC5\n");
> +	cleanup_dc3co(data);
> +}
> +
>  static void test_dc_state_psr(data_t *data, int dc_flag)
>  {
>  	uint32_t dc_counter_before_psr;
> @@ -288,6 +419,12 @@ int main(int argc, char *argv[])
>  			     "Can't open /dev/cpu/0/msr.\n");
>  	}
>  
> +	igt_describe("This tests is to stimulate videoplay back "
typo it is simulate, u can use "This test simulate video playback in order to validate
DC3CO state while PSR2 is active and in SLEEP state"
Thanks,
Anshuman Gupta.
> +			"to validate DC3CO state while PSR2 is active");
> +	igt_subtest("dc3co-vpb-simulation") {
> +		test_dc3co_vpb_simulation(&data, CHECK_DC3CO);
> +	}
> +
>  	igt_describe("This test validates display engine entry to DC5 state "
>  		     "while PSR is active");
>  	igt_subtest("dc5-psr") {
> -- 
> 2.7.4
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/2] Add a new IGT test to validate DC3CO state

[Jeevan B]

Add a subtest for DC3CO video playback case
to generate selective frame update and validate
that system stays in DC3CO state during execution.

v2: Changed PSR2 idle check to sleep check and addressed cosmetic changes.
(Anshuman)

Signed-off-by: Jeevan B <jeevan.b@intel.com>
---
 lib/igt_psr.c           |  11 ++++
 lib/igt_psr.h           |   1 +
 tests/i915/i915_pm_dc.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 154 insertions(+), 5 deletions(-)

diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index b92ea73..7806ce9 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -36,6 +36,17 @@ static bool psr_active_check(int debugfs_fd, enum psr_mode mode)
 	return strstr(buf, state);
 }
 
+bool psr2_active_sleep_check(int debugfs_fd)
+{
+	char buf[PSR_STATUS_MAX_LEN];
+	const char *state = "SLEEP";
+
+	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
+				sizeof(buf));
+
+	return strstr(buf, state);
+}
+
 static inline const char *psr_active_state_get(enum psr_mode mode)
 {
 	return mode == PSR_MODE_1 ? "SRDENT" : "DEEP_SLEEP";
diff --git a/lib/igt_psr.h b/lib/igt_psr.h
index ca38573..a0627dc 100644
--- a/lib/igt_psr.h
+++ b/lib/igt_psr.h
@@ -35,6 +35,7 @@ enum psr_mode {
 	PSR_MODE_2
 };
 
+bool psr2_active_sleep_check(int debugfs_fd);
 bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
 bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
 bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
index 19d8a78..7f93792 100644
--- a/tests/i915/i915_pm_dc.c
+++ b/tests/i915/i915_pm_dc.c
@@ -38,13 +38,22 @@
 #define CHECK_DC6	2
 #define CHECK_DC3CO     4
 
+/*Number of Frames Video Playback*/
+#define VIDEO_FRAMES 100
+
+typedef struct {
+	double r, g, b;
+} color_t;
+
+igt_plane_t *primary;
+
 typedef struct {
 	int drm_fd;
 	int msr_fd;
 	int debugfs_fd;
 	uint32_t devid;
 	igt_display_t display;
-	struct igt_fb fb_white;
+	struct igt_fb fb_white, fb_rgb, fb_rgr;
 	enum psr_mode op_psr_mode;
 	drmModeModeInfo *mode;
 	igt_output_t *output;
@@ -53,6 +62,7 @@ typedef struct {
 
 static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count);
 static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count);
+static bool psr2_wait_sleep_update(int debugfs_fd);
 
 static void setup_output(data_t *data)
 {
@@ -105,8 +115,6 @@ static bool edp_psr2_enabled(data_t *data)
 
 static void cleanup_dc_psr(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);
@@ -114,10 +122,43 @@ static void cleanup_dc_psr(data_t *data)
 	igt_remove_fb(data->drm_fd, &data->fb_white);
 }
 
-static void setup_primary(data_t *data)
+static void cleanup_dc3co(data_t *data)
+{
+	primary = igt_output_get_plane_type(data->output,
+						DRM_PLANE_TYPE_PRIMARY);
+	igt_plane_set_fb(primary, NULL);
+	/*Clear Frame Buffers*/
+	igt_display_commit(&data->display);
+	igt_remove_fb(data->drm_fd, &data->fb_rgb);
+
+	igt_display_commit(&data->display);
+	igt_remove_fb(data->drm_fd, &data->fb_rgr);
+}
+
+static void paint_rectangles(data_t *data,
+				drmModeModeInfo *mode,
+				color_t *colors,
+				igt_fb_t *fb)
 {
-	igt_plane_t *primary;
+	cairo_t *cr = igt_get_cairo_ctx(data->drm_fd, fb);
+	int i, l = mode->hdisplay / 3;
+	int rows_remaining = mode->hdisplay % 3;
+
+	/* Paint 3 solid rectangles. */
+	for (i = 0 ; i < 3; i++) {
+		igt_paint_color(cr, i * l, 0, l, mode->vdisplay,
+				colors[i].r, colors[i].g, colors[i].b);
+	}
 
+	if (rows_remaining > 0)
+		igt_paint_color(cr, i * l, 0, rows_remaining, mode->vdisplay,
+				colors[i-1].r, colors[i-1].g, colors[i-1].b);
+
+	igt_put_cairo_ctx(data->drm_fd, fb, cr);
+}
+
+static void setup_primary(data_t *data)
+{
 	primary = igt_output_get_plane_type(data->output,
 					    DRM_PLANE_TYPE_PRIMARY);
 	igt_plane_set_fb(primary, NULL);
@@ -131,6 +172,24 @@ static void setup_primary(data_t *data)
 	igt_display_commit(&data->display);
 }
 
+static void create_clr_fb(data_t *data, igt_fb_t *fb, color_t *fb_color)
+{
+	int fb_id;
+
+	primary = igt_output_get_plane_type(data->output,
+						DRM_PLANE_TYPE_PRIMARY);
+
+	igt_plane_set_fb(primary, NULL);
+	fb_id = igt_create_fb(data->drm_fd,
+				data->mode->hdisplay,
+				data->mode->vdisplay,
+				DRM_FORMAT_XRGB8888,
+				LOCAL_DRM_FORMAT_MOD_NONE,
+				fb);
+	igt_assert(fb_id);
+	paint_rectangles(data, data->mode, fb_color, fb);
+}
+
 static uint32_t get_dc_counter(char *dc_data)
 {
 	char *e;
@@ -171,6 +230,11 @@ static uint32_t read_dc_counter(uint32_t drm_fd, int dc_flag)
 	return get_dc_counter(str);
 }
 
+static bool psr2_wait_sleep_update(int debugfs_fd)
+{
+	return igt_wait(psr2_active_sleep_check(debugfs_fd), 100, 10);
+}
+
 static bool dc_state_wait_entry(int drm_fd, int dc_flag, int prev_dc_count)
 {
 	return igt_wait(read_dc_counter(drm_fd, dc_flag) >
@@ -187,6 +251,73 @@ static void check_dc_counter(int drm_fd, int dc_flag, uint32_t prev_dc_count)
 		     "%s state is not achieved\n", tmp);
 }
 
+static void setup_vpb(data_t *data)
+{
+	color_t red_green_blue[] = {
+		{ 1.0, 0.0, 0.0 },
+		{ 0.0, 1.0, 0.0 },
+		{ 0.0, 0.0, 1.0 },
+	};
+	color_t red_green_red[] = {
+		{ 1.0, 0.0, 0.0 },
+		{ 0.0, 1.0, 0.0 },
+		{ 1.0, 0.0, 0.0 },
+	};
+
+	setup_output(data);
+
+	create_clr_fb(data, &data->fb_rgb, red_green_blue);
+	create_clr_fb(data, &data->fb_rgr, red_green_red);
+}
+
+static void run_videoplayback(data_t *data, int dc_flag)
+{
+	uint32_t dc3co_cnt_before_psr;
+	int i, delay;
+
+	igt_plane_set_fb(primary, NULL);
+
+	dc3co_cnt_before_psr = read_dc_counter(data->drm_fd, dc_flag);
+	/*Calculate delay to generate idle frame*/
+	delay = ((1000*1000)/data->mode->vrefresh);
+
+	for (i = 0; i < VIDEO_FRAMES; i++) {
+		if (i % 2 == 0) {
+			igt_plane_set_fb(primary, &data->fb_rgb);
+			igt_display_commit(&data->display);
+		} else {
+			igt_plane_set_fb(primary, &data->fb_rgr);
+			igt_display_commit(&data->display);
+		}
+		usleep(delay);
+		igt_assert(psr2_wait_sleep_update(data->debugfs_fd));
+	}
+	check_dc_counter(data->drm_fd, dc_flag, dc3co_cnt_before_psr);
+}
+
+static void setup_dc3co(data_t *data)
+{
+	igt_require(IS_TIGERLAKE(data->devid));
+	data->op_psr_mode = PSR_MODE_2;
+	psr_enable(data->debugfs_fd, data->op_psr_mode);
+	igt_require_f(edp_psr2_enabled(data),
+			"PSR2 is not enabled\n");
+}
+
+static void test_dc3co_vpb_simulation(data_t *data, int dc_flag)
+{
+	uint32_t dc5_cnt_before, dc5_cnt_after;
+
+	setup_dc3co(data);
+	setup_vpb(data);
+	dc5_cnt_before = read_dc_counter(data->drm_fd, CHECK_DC5);
+	run_videoplayback(data, dc_flag);
+	dc5_cnt_after = read_dc_counter(data->drm_fd, CHECK_DC5);
+	igt_assert_f(dc5_cnt_after == dc5_cnt_before,
+			"DC State moved to DC5\n");
+	cleanup_dc3co(data);
+}
+
 static void test_dc_state_psr(data_t *data, int dc_flag)
 {
 	uint32_t dc_counter_before_psr;
@@ -288,6 +419,12 @@ int main(int argc, char *argv[])
 			     "Can't open /dev/cpu/0/msr.\n");
 	}
 
+	igt_describe("This tests is to stimulate videoplay back "
+			"to validate DC3CO state while PSR2 is active");
+	igt_subtest("dc3co-vpb-simulation") {
+		test_dc3co_vpb_simulation(&data, CHECK_DC3CO);
+	}
+
 	igt_describe("This test validates display engine entry to DC5 state "
 		     "while PSR is active");
 	igt_subtest("dc5-psr") {
-- 
2.7.4

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