* [igt-dev] [PATCH i-g-t 0/3] Add a new IGT test to validate DC3CO state.
@ 2019-10-03 7:45 Jeevan B
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 1/3] igt/i915/i915_pm_dc: DC3CO PSR2 helpers Jeevan B
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Jeevan B @ 2019-10-03 7:45 UTC (permalink / raw)
To: igt-dev; +Cc: 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 (2):
igt/i915/i915_pm_dc: DC3CO PSR2 helpers
HAX: Run igt@i915_pm_dc@dc3co-vpb-simulation in BAT
Jeevan B (1):
Add a new IGT test to validate DC3CO state
lib/igt_psr.c | 11 ++
lib/igt_psr.h | 1 +
tests/i915/i915_pm_dc.c | 183 ++++++++++++++++++++++++++++++++--
tests/intel-ci/fast-feedback.testlist | 1 +
4 files changed, 190 insertions(+), 6 deletions(-)
--
2.7.4
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH i-g-t 1/3] igt/i915/i915_pm_dc: DC3CO PSR2 helpers
2019-10-03 7:45 [igt-dev] [PATCH i-g-t 0/3] Add a new IGT test to validate DC3CO state Jeevan B
@ 2019-10-03 7:45 ` Jeevan B
2019-10-04 12:43 ` Animesh Manna
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state Jeevan B
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Jeevan B @ 2019-10-03 7:45 UTC (permalink / raw)
To: igt-dev; +Cc: 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.
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Jeevan B <jeevan.b@intel.com>
---
tests/i915/i915_pm_dc.c | 34 +++++++++++++++++++++++++++++-----
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
index ce3319b..19d8a78 100644
--- a/tests/i915/i915_pm_dc.c
+++ b/tests/i915/i915_pm_dc.c
@@ -36,6 +36,7 @@
/* DC State Flags */
#define CHECK_DC5 1
#define CHECK_DC6 2
+#define CHECK_DC3CO 4
typedef struct {
int drm_fd;
@@ -88,6 +89,20 @@ 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 +156,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 +179,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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state
2019-10-03 7:45 [igt-dev] [PATCH i-g-t 0/3] Add a new IGT test to validate DC3CO state Jeevan B
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 1/3] igt/i915/i915_pm_dc: DC3CO PSR2 helpers Jeevan B
@ 2019-10-03 7:45 ` Jeevan B
2019-10-03 11:25 ` Anshuman Gupta
` (2 more replies)
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 3/3] HAX: Run igt@i915_pm_dc@dc3co-vpb-simulation in BAT Jeevan B
` (2 subsequent siblings)
4 siblings, 3 replies; 12+ messages in thread
From: Jeevan B @ 2019-10-03 7:45 UTC (permalink / raw)
To: igt-dev; +Cc: 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.
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 | 149 +++++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 160 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..c02c02e 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,43 @@ 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 +175,25 @@ 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)
+{
+ 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 +234,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 +255,78 @@ 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_color_fb(data, &data->fb_rgb, red_green_blue);
+ create_color_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_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, 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_prev_cnt);
+}
+
+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 +428,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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [igt-dev] [PATCH i-g-t 3/3] HAX: Run igt@i915_pm_dc@dc3co-vpb-simulation in BAT
2019-10-03 7:45 [igt-dev] [PATCH i-g-t 0/3] Add a new IGT test to validate DC3CO state Jeevan B
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 1/3] igt/i915/i915_pm_dc: DC3CO PSR2 helpers Jeevan B
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state Jeevan B
@ 2019-10-03 7:45 ` Jeevan B
2019-10-03 9:24 ` [igt-dev] ✓ Fi.CI.BAT: success for Add a new IGT test to validate DC3CO state. (rev4) Patchwork
2019-10-03 15:56 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Jeevan B @ 2019-10-03 7:45 UTC (permalink / raw)
To: igt-dev; +Cc: Jeevan B
From: Anshuman Gupta <anshuman.gupta@intel.com>
We want to cover igt@i915_pm_dc@dc3co-vpb-simulation test on
all TGL platform because dc3co-vpg-simulation tests requires PSR2
panel support. so adding this test to fast feedback list.
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Jeevan B <jeevan.b@intel.com>
---
tests/intel-ci/fast-feedback.testlist | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index e78e7fd..5569a2d 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -188,6 +188,7 @@ igt@kms_psr@primary_mmap_gtt
igt@kms_setmode@basic-clone-single-crtc
igt@i915_pm_backlight@basic-brightness
igt@i915_pm_rpm@basic-pci-d3-state
+igt@i915_pm_dc@dc3co-vpb-simulation
igt@i915_pm_rpm@basic-rte
igt@i915_pm_rps@basic-api
igt@prime_busy@basic-after-default
--
2.7.4
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for Add a new IGT test to validate DC3CO state. (rev4)
2019-10-03 7:45 [igt-dev] [PATCH i-g-t 0/3] Add a new IGT test to validate DC3CO state Jeevan B
` (2 preceding siblings ...)
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 3/3] HAX: Run igt@i915_pm_dc@dc3co-vpb-simulation in BAT Jeevan B
@ 2019-10-03 9:24 ` Patchwork
2019-10-03 15:56 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-10-03 9:24 UTC (permalink / raw)
To: Jeevan B; +Cc: igt-dev
== Series Details ==
Series: Add a new IGT test to validate DC3CO state. (rev4)
URL : https://patchwork.freedesktop.org/series/66648/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6996 -> IGTPW_3531
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_3531:
### IGT changes ###
#### Possible regressions ####
* {igt@i915_pm_dc@dc3co-vpb-simulation} (NEW):
- {fi-tgl-u}: NOTRUN -> [SKIP][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-tgl-u/igt@i915_pm_dc@dc3co-vpb-simulation.html
- {fi-icl-dsi}: NOTRUN -> [SKIP][2]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-icl-dsi/igt@i915_pm_dc@dc3co-vpb-simulation.html
- {fi-icl-guc}: NOTRUN -> [SKIP][3]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-icl-guc/igt@i915_pm_dc@dc3co-vpb-simulation.html
- {fi-icl-u4}: NOTRUN -> [SKIP][4]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-icl-u4/igt@i915_pm_dc@dc3co-vpb-simulation.html
- fi-icl-u2: NOTRUN -> [SKIP][5]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-icl-u2/igt@i915_pm_dc@dc3co-vpb-simulation.html
- fi-cml-u2: NOTRUN -> [SKIP][6]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-cml-u2/igt@i915_pm_dc@dc3co-vpb-simulation.html
- {fi-cml-s}: NOTRUN -> [SKIP][7]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-cml-s/igt@i915_pm_dc@dc3co-vpb-simulation.html
- fi-icl-u3: NOTRUN -> [SKIP][8]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-icl-u3/igt@i915_pm_dc@dc3co-vpb-simulation.html
New tests
---------
New tests have been introduced between CI_DRM_6996 and IGTPW_3531:
### New IGT tests (1) ###
* igt@i915_pm_dc@dc3co-vpb-simulation:
- Statuses : 45 skip(s)
- Exec time: [0.0, 0.08] s
Known issues
------------
Here are the changes found in IGTPW_3531 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_mmap_gtt@basic-read-write-distinct:
- fi-icl-u3: [PASS][9] -> [DMESG-WARN][10] ([fdo#107724])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write-distinct.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write-distinct.html
* igt@i915_module_load@reload:
- fi-icl-u3: [PASS][11] -> [DMESG-WARN][12] ([fdo#107724] / [fdo#111214])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/fi-icl-u3/igt@i915_module_load@reload.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-icl-u3/igt@i915_module_load@reload.html
#### Possible fixes ####
* igt@gem_flink_basic@double-flink:
- fi-icl-u3: [DMESG-WARN][13] ([fdo#107724]) -> [PASS][14] +1 similar issue
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/fi-icl-u3/igt@gem_flink_basic@double-flink.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-icl-u3/igt@gem_flink_basic@double-flink.html
* igt@i915_selftest@live_gem_contexts:
- {fi-tgl-u}: [INCOMPLETE][15] -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/fi-tgl-u/igt@i915_selftest@live_gem_contexts.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-tgl-u/igt@i915_selftest@live_gem_contexts.html
* igt@i915_selftest@live_hangcheck:
- fi-cfl-8109u: [INCOMPLETE][17] ([fdo#106070]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/fi-cfl-8109u/igt@i915_selftest@live_hangcheck.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-cfl-8109u/igt@i915_selftest@live_hangcheck.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-icl-u2: [FAIL][19] ([fdo#109483]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
#### Warnings ####
* igt@i915_module_load@reload:
- fi-icl-u2: [DMESG-WARN][21] ([fdo#110595]) -> [DMESG-WARN][22] ([fdo#110595] / [fdo#111214])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/fi-icl-u2/igt@i915_module_load@reload.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/fi-icl-u2/igt@i915_module_load@reload.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#106070]: https://bugs.freedesktop.org/show_bug.cgi?id=106070
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
[fdo#110595]: https://bugs.freedesktop.org/show_bug.cgi?id=110595
[fdo#111214]: https://bugs.freedesktop.org/show_bug.cgi?id=111214
Participating hosts (52 -> 45)
------------------------------
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_5210 -> IGTPW_3531
CI-20190529: 20190529
CI_DRM_6996: 98596d29a3cff9d996c42468eb606036faf42954 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_3531: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/index.html
IGT_5210: 74f55119f9920b65996535210a09147997804136 @ 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_3531/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state Jeevan B
@ 2019-10-03 11:25 ` Anshuman Gupta
2019-10-03 14:50 ` Arkadiusz Hiler
2019-10-07 13:48 ` Imre Deak
2 siblings, 0 replies; 12+ messages in thread
From: Anshuman Gupta @ 2019-10-03 11:25 UTC (permalink / raw)
To: Jeevan B, ankit.k.nautiyal; +Cc: igt-dev
On 2019-10-03 at 13:15:08 +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.
>
> 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 | 149 +++++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 160 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..c02c02e 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,43 @@ 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 +175,25 @@ 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)
> +{
> + 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);
Do we need to pair primary plane with NULL framebuffer here?
We are not doing any commit here, i am not sure about it.
@ankit can u please comment here.
> + 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 +234,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 +255,78 @@ 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);
please move this to after setup_dc3co() in test_dc3co_vpb_simulation()
> +
we do not need space here.
> + create_color_fb(data, &data->fb_rgb, red_green_blue);
> + create_color_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_prev_cnt;
> + int i, delay;
> +
> + primary = igt_output_get_plane_type(data->output,
> + DRM_PLANE_TYPE_PRIMARY);
> +
> + igt_plane_set_fb(primary, NULL);
> +
we do not need space here too.
> + dc3co_prev_cnt = read_dc_counter(data->drm_fd, dc_flag);
> + /* Calculate delay to generate idle frame */
Please mention the delay is for an idle frame in usec in comment.
> + 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));
> + }
A space require here after each block of code.
> + check_dc_counter(data->drm_fd, dc_flag, dc3co_prev_cnt);
> +}
> +
> +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");
Align this to open parenthesis.
> + cleanup_dc3co(data);
> +}
> +
> static void test_dc_state_psr(data_t *data, int dc_flag)
> {
> uint32_t dc_counter_before_psr;
> @@ -288,6 +428,13 @@ int main(int argc, char *argv[])
> "Can't open /dev/cpu/0/msr.\n");
> }
>
> + igt_describe("This test simulate videoplay back "
Typo 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, CHECK_DC3CO);
> + }
> +
> igt_describe("This test validates display engine entry to DC5 state "
> "while PSR is active");
> igt_subtest("dc5-psr") {
With above feedback and comment patch is looks good to me.
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> --
> 2.7.4
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state Jeevan B
2019-10-03 11:25 ` Anshuman Gupta
@ 2019-10-03 14:50 ` Arkadiusz Hiler
2019-10-04 10:39 ` Jeevan B
2019-10-07 13:48 ` Imre Deak
2 siblings, 1 reply; 12+ messages in thread
From: Arkadiusz Hiler @ 2019-10-03 14:50 UTC (permalink / raw)
To: Jeevan B; +Cc: igt-dev
On Thu, Oct 03, 2019 at 01:15:08PM +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.
>
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
<SNIP>
> +static void setup_dc3co(data_t *data)
> +{
> + igt_require(IS_TIGERLAKE(data->devid));
Is there a reason to make a check specific to TIGERLAKE here?
I think that better solution would be to add
igt_require_dc_counter(CHECK_DC3CO) that would look for "DC3CO count"
presence in i915_dmc_info.
This way you have feature check instead of encoding what is supported by
which platforms in the tests - kernel should not advertise counters for
things it does not support.
> +static void run_videoplayback(data_t *data, int dc_flag)
> +{
> + 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, dc_flag);
I don't quite get why this is parametrized with dc_flag, when the name
clearly says that we are testing for DC3CO. Just use CHECK_DC3CO
directly.
> +static void test_dc3co_vpb_simulation(data_t *data, int dc_flag)
> +{
Same here with dc_flag, you pass it few levels down to functions that
are not generic and won't work with anything else than CHECK_DC3CO.
> static void test_dc_state_psr(data_t *data, int dc_flag)
> {
> uint32_t dc_counter_before_psr;
> @@ -288,6 +428,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") {
This does not quite help me to understand what and why we are checking
here.
As far as I understand we want to make sure that we reach DC3CO state
between frames of videoplayback-like workload (when we have some idle
frames but not enough of them to reach deeper C-state).
So, if my understanding is correct (and please correct me if I am wrong)
something like this would work better:
"Make sure that we reach DC3CO power-saving state state with PSR2 panel
when we have videoplayback-like display workload."
I would also like to see a normal C comment that explains DC3CO a bit,
especially how it differs from DC5 and DC6 and expecations/interactions
with idle frames.
--
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* [igt-dev] ✗ Fi.CI.IGT: failure for Add a new IGT test to validate DC3CO state. (rev4)
2019-10-03 7:45 [igt-dev] [PATCH i-g-t 0/3] Add a new IGT test to validate DC3CO state Jeevan B
` (3 preceding siblings ...)
2019-10-03 9:24 ` [igt-dev] ✓ Fi.CI.BAT: success for Add a new IGT test to validate DC3CO state. (rev4) Patchwork
@ 2019-10-03 15:56 ` Patchwork
4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2019-10-03 15:56 UTC (permalink / raw)
To: Jeevan B; +Cc: igt-dev
== Series Details ==
Series: Add a new IGT test to validate DC3CO state. (rev4)
URL : https://patchwork.freedesktop.org/series/66648/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6996_full -> IGTPW_3531_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with IGTPW_3531_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_3531_full, 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_3531/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_3531_full:
### IGT changes ###
#### Possible regressions ####
* {igt@i915_pm_dc@dc3co-vpb-simulation} (NEW):
- shard-iclb: NOTRUN -> [SKIP][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb8/igt@i915_pm_dc@dc3co-vpb-simulation.html
* igt@i915_selftest@live_execlists:
- shard-apl: [PASS][2] -> [DMESG-FAIL][3]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-apl1/igt@i915_selftest@live_execlists.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-apl1/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@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
- {shard-tglb}: [PASS][4] -> [INCOMPLETE][5]
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-tglb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
* igt@kms_rotation_crc@bad-tiling:
- {shard-tglb}: NOTRUN -> [INCOMPLETE][6] +1 similar issue
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-tglb1/igt@kms_rotation_crc@bad-tiling.html
New tests
---------
New tests have been introduced between CI_DRM_6996_full and IGTPW_3531_full:
### New IGT tests (1) ###
* igt@i915_pm_dc@dc3co-vpb-simulation:
- Statuses : 6 skip(s)
- Exec time: [0.0] s
Known issues
------------
Here are the changes found in IGTPW_3531_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_schedule@in-order-bsd2:
- shard-iclb: [PASS][7] -> [SKIP][8] ([fdo#109276]) +14 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb1/igt@gem_exec_schedule@in-order-bsd2.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb3/igt@gem_exec_schedule@in-order-bsd2.html
* igt@gem_exec_schedule@preempt-self-bsd:
- shard-iclb: [PASS][9] -> [SKIP][10] ([fdo#111325]) +2 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb6/igt@gem_exec_schedule@preempt-self-bsd.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb4/igt@gem_exec_schedule@preempt-self-bsd.html
* igt@gem_userptr_blits@sync-unmap-after-close:
- shard-apl: [PASS][11] -> [DMESG-WARN][12] ([fdo#109385] / [fdo#111870]) +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-apl3/igt@gem_userptr_blits@sync-unmap-after-close.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-apl7/igt@gem_userptr_blits@sync-unmap-after-close.html
* igt@gem_userptr_blits@sync-unmap-cycles:
- shard-snb: [PASS][13] -> [DMESG-WARN][14] ([fdo#111870]) +2 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-snb2/igt@gem_userptr_blits@sync-unmap-cycles.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-snb5/igt@gem_userptr_blits@sync-unmap-cycles.html
* igt@kms_frontbuffer_tracking@fbc-modesetfrombusy:
- shard-iclb: [PASS][15] -> [FAIL][16] ([fdo#103167]) +3 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-modesetfrombusy.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-modesetfrombusy.html
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-apl: [PASS][17] -> [DMESG-WARN][18] ([fdo#108566]) +2 similar issues
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-apl8/igt@kms_frontbuffer_tracking@fbc-suspend.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-apl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
* igt@kms_plane_lowres@pipe-a-tiling-y:
- shard-iclb: [PASS][19] -> [FAIL][20] ([fdo#103166])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-y.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-y.html
* igt@kms_psr2_su@frontbuffer:
- shard-iclb: [PASS][21] -> [SKIP][22] ([fdo#109642] / [fdo#111068])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb3/igt@kms_psr2_su@frontbuffer.html
* igt@kms_psr@psr2_suspend:
- shard-iclb: [PASS][23] -> [SKIP][24] ([fdo#109441])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb2/igt@kms_psr@psr2_suspend.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb4/igt@kms_psr@psr2_suspend.html
#### Possible fixes ####
* igt@gem_exec_schedule@in-order-bsd:
- shard-iclb: [SKIP][25] ([fdo#111325]) -> [PASS][26] +7 similar issues
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb2/igt@gem_exec_schedule@in-order-bsd.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb6/igt@gem_exec_schedule@in-order-bsd.html
* igt@gem_mocs_settings@mocs-rc6-ctx-render:
- {shard-tglb}: [FAIL][27] ([fdo#111723]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-tglb7/igt@gem_mocs_settings@mocs-rc6-ctx-render.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-tglb7/igt@gem_mocs_settings@mocs-rc6-ctx-render.html
* igt@gem_userptr_blits@map-fixed-invalidate-busy:
- shard-kbl: [DMESG-WARN][29] ([fdo#111870]) -> [PASS][30] +1 similar issue
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-kbl4/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-kbl7/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
- {shard-tglb}: [DMESG-WARN][31] ([fdo#111870]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-tglb3/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-tglb7/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
- shard-hsw: [DMESG-WARN][33] ([fdo#111870]) -> [PASS][34] +2 similar issues
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-hsw7/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
- shard-apl: [DMESG-WARN][35] ([fdo#109385] / [fdo#111870]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-apl3/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-apl3/igt@gem_userptr_blits@map-fixed-invalidate-busy.html
* igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup:
- shard-iclb: [DMESG-WARN][37] ([fdo#111870]) -> [PASS][38] +1 similar issue
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb4/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb1/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup.html
* igt@gem_userptr_blits@sync-unmap:
- shard-snb: [DMESG-WARN][39] ([fdo#111870]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-snb5/igt@gem_userptr_blits@sync-unmap.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-snb4/igt@gem_userptr_blits@sync-unmap.html
* igt@gem_userptr_blits@sync-unmap-cycles:
- shard-glk: [DMESG-WARN][41] ([fdo#111870]) -> [PASS][42] +3 similar issues
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-glk1/igt@gem_userptr_blits@sync-unmap-cycles.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-glk6/igt@gem_userptr_blits@sync-unmap-cycles.html
* igt@kms_atomic_transition@plane-all-modeset-transition-fencing:
- shard-apl: [INCOMPLETE][43] ([fdo#103927]) -> [PASS][44] +2 similar issues
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-apl3/igt@kms_atomic_transition@plane-all-modeset-transition-fencing.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-apl7/igt@kms_atomic_transition@plane-all-modeset-transition-fencing.html
* igt@kms_flip@2x-flip-vs-suspend:
- shard-hsw: [INCOMPLETE][45] ([fdo#103540]) -> [PASS][46] +1 similar issue
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-hsw6/igt@kms_flip@2x-flip-vs-suspend.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-hsw4/igt@kms_flip@2x-flip-vs-suspend.html
* igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
- shard-iclb: [FAIL][47] ([fdo#103167]) -> [PASS][48] +5 similar issues
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff:
- {shard-tglb}: [FAIL][49] ([fdo#103167]) -> [PASS][50] +2 similar issues
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-tglb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-tglb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-onoff.html
* igt@kms_frontbuffer_tracking@fbc-suspend:
- {shard-tglb}: [INCOMPLETE][51] ([fdo#111832]) -> [PASS][52] +3 similar issues
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-tglb5/igt@kms_frontbuffer_tracking@fbc-suspend.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-tglb6/igt@kms_frontbuffer_tracking@fbc-suspend.html
* igt@kms_psr2_su@page_flip:
- shard-iclb: [SKIP][53] ([fdo#109642] / [fdo#111068]) -> [PASS][54]
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb8/igt@kms_psr2_su@page_flip.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb2/igt@kms_psr2_su@page_flip.html
* igt@kms_psr@psr2_cursor_plane_move:
- shard-iclb: [SKIP][55] ([fdo#109441]) -> [PASS][56] +1 similar issue
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb3/igt@kms_psr@psr2_cursor_plane_move.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html
* igt@kms_setmode@invalid-clone-exclusive-crtc:
- {shard-tglb}: [DMESG-WARN][57] ([fdo#111600]) -> [PASS][58] +1 similar issue
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-tglb2/igt@kms_setmode@invalid-clone-exclusive-crtc.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-tglb5/igt@kms_setmode@invalid-clone-exclusive-crtc.html
* igt@kms_vblank@pipe-a-ts-continuation-suspend:
- shard-apl: [DMESG-WARN][59] ([fdo#108566]) -> [PASS][60] +4 similar issues
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-apl2/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-apl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
* igt@prime_busy@hang-bsd2:
- shard-iclb: [SKIP][61] ([fdo#109276]) -> [PASS][62] +16 similar issues
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb5/igt@prime_busy@hang-bsd2.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb2/igt@prime_busy@hang-bsd2.html
#### Warnings ####
* igt@gem_mocs_settings@mocs-rc6-bsd2:
- shard-iclb: [FAIL][63] ([fdo#111330]) -> [SKIP][64] ([fdo#109276])
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-iclb2/igt@gem_mocs_settings@mocs-rc6-bsd2.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-iclb5/igt@gem_mocs_settings@mocs-rc6-bsd2.html
* igt@gem_userptr_blits@dmabuf-unsync:
- shard-snb: [DMESG-WARN][65] ([fdo#111870]) -> [DMESG-WARN][66] ([fdo#110789] / [fdo#111870])
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6996/shard-snb2/igt@gem_userptr_blits@dmabuf-unsync.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/shard-snb1/igt@gem_userptr_blits@dmabuf-unsync.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109385]: https://bugs.freedesktop.org/show_bug.cgi?id=109385
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#110789]: https://bugs.freedesktop.org/show_bug.cgi?id=110789
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
[fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
[fdo#111597]: https://bugs.freedesktop.org/show_bug.cgi?id=111597
[fdo#111600]: https://bugs.freedesktop.org/show_bug.cgi?id=111600
[fdo#111646]: https://bugs.freedesktop.org/show_bug.cgi?id=111646
[fdo#111671]: https://bugs.freedesktop.org/show_bug.cgi?id=111671
[fdo#111672]: https://bugs.freedesktop.org/show_bug.cgi?id=111672
[fdo#111723]: https://bugs.freedesktop.org/show_bug.cgi?id=111723
[fdo#111832]: https://bugs.freedesktop.org/show_bug.cgi?id=111832
[fdo#111867]: https://bugs.freedesktop.org/show_bug.cgi?id=111867
[fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
Participating hosts (11 -> 7)
------------------------------
Missing (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5210 -> IGTPW_3531
* Piglit: piglit_4509 -> None
CI-20190529: 20190529
CI_DRM_6996: 98596d29a3cff9d996c42468eb606036faf42954 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_3531: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3531/index.html
IGT_5210: 74f55119f9920b65996535210a09147997804136 @ 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_3531/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state
2019-10-03 14:50 ` Arkadiusz Hiler
@ 2019-10-04 10:39 ` Jeevan B
2019-10-04 13:59 ` Arkadiusz Hiler
0 siblings, 1 reply; 12+ messages in thread
From: Jeevan B @ 2019-10-04 10:39 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: igt-dev
On 2019-10-03 at 17:50:04 +0300, Arkadiusz Hiler wrote:
> On Thu, Oct 03, 2019 at 01:15:08PM +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.
> >
> > Signed-off-by: Jeevan B <jeevan.b@intel.com>
> <SNIP>
> > +static void setup_dc3co(data_t *data)
> > +{
> > + igt_require(IS_TIGERLAKE(data->devid));
>
> Is there a reason to make a check specific to TIGERLAKE here?
>
> I think that better solution would be to add
> igt_require_dc_counter(CHECK_DC3CO) that would look for "DC3CO count"
> presence in i915_dmc_info.
We are checking DC3CO count in read_dc_counter(), it will check DC state count if counter
not available then the test will skip.
In run_videoplayback we are using read_dc_counter() to check DC3CO count.
Here we added TGL check as we didn't want to check for any other platform. As DC3CO is only supported on TGL.
If you want us to remove TGL check, we will remove it.
>
> This way you have feature check instead of encoding what is supported by
> which platforms in the tests - kernel should not advertise counters for
> things it does not support.
>
> > +static void run_videoplayback(data_t *data, int dc_flag)
> > +{
> > + 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, dc_flag);
>
> I don't quite get why this is parametrized with dc_flag, when the name
> clearly says that we are testing for DC3CO. Just use CHECK_DC3CO
> directly.
>
> > +static void test_dc3co_vpb_simulation(data_t *data, int dc_flag)
> > +{
>
> Same here with dc_flag, you pass it few levels down to functions that
> are not generic and won't work with anything else than CHECK_DC3CO.
>
we need this flag because read_dc_counter() and check_dc_counter() are used for
each DC states. (DC5:DC6:DC3CO)
> > static void test_dc_state_psr(data_t *data, int dc_flag)
> > {
> > uint32_t dc_counter_before_psr;
> > @@ -288,6 +428,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") {
>
> This does not quite help me to understand what and why we are checking
> here.
>
> As far as I understand we want to make sure that we reach DC3CO state
> between frames of videoplayback-like workload (when we have some idle
> frames but not enough of them to reach deeper C-state).
>
> So, if my understanding is correct (and please correct me if I am wrong)
> something like this would work better:
>
Yes, your understanding is correct. I will do the necessary change accordingly.
Shall i change the test name to dc3co-vpb what is your opinion?
> "Make sure that we reach DC3CO power-saving state state with PSR2 panel
> when we have videoplayback-like display workload."
>
> I would also like to see a normal C comment that explains DC3CO a bit,
> especially how it differs from DC5 and DC6 and expecations/interactions
> with idle frames.
>
Sure, I will add it.
> --
> Cheers,
> Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/3] igt/i915/i915_pm_dc: DC3CO PSR2 helpers
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 1/3] igt/i915/i915_pm_dc: DC3CO PSR2 helpers Jeevan B
@ 2019-10-04 12:43 ` Animesh Manna
0 siblings, 0 replies; 12+ messages in thread
From: Animesh Manna @ 2019-10-04 12:43 UTC (permalink / raw)
To: Jeevan B, igt-dev
On 10/3/2019 1:15 PM, Jeevan B wrote:
> 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.
>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
> tests/i915/i915_pm_dc.c | 34 +++++++++++++++++++++++++++++-----
> 1 file changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/tests/i915/i915_pm_dc.c b/tests/i915/i915_pm_dc.c
> index ce3319b..19d8a78 100644
> --- a/tests/i915/i915_pm_dc.c
> +++ b/tests/i915/i915_pm_dc.c
> @@ -36,6 +36,7 @@
> /* DC State Flags */
> #define CHECK_DC5 1
> #define CHECK_DC6 2
> +#define CHECK_DC3CO 4
As I understood each bit of dc_flag represent a dc-state.
Good to change macro definition like below which represent the bit
position for specific dc-state.
#define CHECK_DC5 (1 << 0)
#define CHECK_DC6 (1 << 1)
#define CHECK_DC3CO (1 << 2)
>
> typedef struct {
> int drm_fd;
> @@ -88,6 +89,20 @@ static bool edp_psr_sink_support(data_t *data)
> return strstr(buf, "Sink support: yes");
> }
>
> +static bool edp_psr2_enabled(data_t *data)
> +
Remove the extra line if added.
Can add Reviewed-by: Animesh Manna <animesh.manna@intel.com>
> +{
> + 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 +156,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 +179,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)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state
2019-10-04 10:39 ` Jeevan B
@ 2019-10-04 13:59 ` Arkadiusz Hiler
0 siblings, 0 replies; 12+ messages in thread
From: Arkadiusz Hiler @ 2019-10-04 13:59 UTC (permalink / raw)
To: Jeevan B; +Cc: igt-dev
On Fri, Oct 04, 2019 at 04:09:08PM +0530, Jeevan B wrote:
> On 2019-10-03 at 17:50:04 +0300, Arkadiusz Hiler wrote:
> > On Thu, Oct 03, 2019 at 01:15:08PM +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.
> > >
> > > Signed-off-by: Jeevan B <jeevan.b@intel.com>
> > <SNIP>
> > > +static void setup_dc3co(data_t *data)
> > > +{
> > > + igt_require(IS_TIGERLAKE(data->devid));
> >
> > Is there a reason to make a check specific to TIGERLAKE here?
> >
> > I think that better solution would be to add
> > igt_require_dc_counter(CHECK_DC3CO) that would look for "DC3CO count"
> > presence in i915_dmc_info.
> We are checking DC3CO count in read_dc_counter(), it will check DC state count if counter
> not available then the test will skip.
Right. That sounds better.
Requiring DC3CO counter explicitly upfront would help with few things
though:
1. you have check for DC3CO being available early in the test
instead of having one burried deep within some function - if someone
reads the test it is much more obvious what are the requirements and
how the checks are done
2. you don't get surprise skips form function that has read_ in the
name, require_ makes it more explicit that it may skip
3. you do that way ealirer in the process, before speding time on
setting up FBs/KMS
> In run_videoplayback we are using read_dc_counter() to check DC3CO count.
>
> Here we added TGL check as we didn't want to check for any other
> platform. As DC3CO is only supported on TGL.
> If you want us to remove TGL check, we will remove it.
Adding platorm checks makes maintainence much harder. If any other
future platform will support DC3CO, someone will have to come here and
expand this check.
If you check for DC3CO counter presence instead then you will get this
test running on all DC3CO supporting HW for free.
It's up to kernel to not falsely advertise counters for C-states it does
not support.
I hope this convinces you.
> > This way you have feature check instead of encoding what is supported by
> > which platforms in the tests - kernel should not advertise counters for
> > things it does not support.
> >
> > > +static void run_videoplayback(data_t *data, int dc_flag)
> > > +{
> > > + 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, dc_flag);
> >
> > I don't quite get why this is parametrized with dc_flag, when the name
> > clearly says that we are testing for DC3CO. Just use CHECK_DC3CO
> > directly.
> >
> > > +static void test_dc3co_vpb_simulation(data_t *data, int dc_flag)
> > > +{
> >
> > Same here with dc_flag, you pass it few levels down to functions that
> > are not generic and won't work with anything else than CHECK_DC3CO.
> >
> we need this flag because read_dc_counter() and check_dc_counter() are used for
> each DC states. (DC5:DC6:DC3CO)
You need that flag when you are calling check_dc_counter yes, but I
don't think you should make test_dc3co_vpb_siulation() parametrizable
with it.
Let me explain:
test_dc3co_vpb_simulation says that it test dc3co but yet it's
parametrized with dc_flag which is confusing.
The only valid way to call it is:
test_dc3co_vpb_simulation(&data, CHECK_DC3CO);
If you try to do:
test_dc3co_vpb_simulation(&data, CHECK_DC5);
or
test_dc3co_vpb_simulation(&data, CHECK_DC6);
It will not work. So the parameter is completetly unecessary.
Same goes for the line:
dc3co_prev_cnt = read_dc_counter(data->drm_fd, dc_flag);
The variable is even has dc3co in the name, so you can just pass
CHECK_DC3CO directly instead of passing it through dc_flag all the way
from the top.
static void test_dc3co_vpb_simulation(data_t *data)
{
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);
check_dc3co_with_videoplayback_like_load(data);
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);
}
and
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 */
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);
}
Note the suggested name change for run_videoplayback - it actually do
checks (igt_asserts) for dc3co with videoplayback-like load, so the name
here seems more fitting.
> > > static void test_dc_state_psr(data_t *data, int dc_flag)
> > > {
> > > uint32_t dc_counter_before_psr;
> > > @@ -288,6 +428,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") {
> >
> > This does not quite help me to understand what and why we are checking
> > here.
> >
> > As far as I understand we want to make sure that we reach DC3CO state
> > between frames of videoplayback-like workload (when we have some idle
> > frames but not enough of them to reach deeper C-state).
> >
> > So, if my understanding is correct (and please correct me if I am wrong)
> > something like this would work better:
> >
> Yes, your understanding is correct. I will do the necessary change accordingly.
> Shall i change the test name to dc3co-vpb what is your opinion?
Looks good, but I have no strong opinion on this one :-)
> > "Make sure that we reach DC3CO power-saving state state with PSR2 panel
> > when we have videoplayback-like display workload."
> >
> > I would also like to see a normal C comment that explains DC3CO a bit,
> > especially how it differs from DC5 and DC6 and expecations/interactions
> > with idle frames.
> >
> Sure, I will add it.
Thanks!
--
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state Jeevan B
2019-10-03 11:25 ` Anshuman Gupta
2019-10-03 14:50 ` Arkadiusz Hiler
@ 2019-10-07 13:48 ` Imre Deak
2 siblings, 0 replies; 12+ messages in thread
From: Imre Deak @ 2019-10-07 13:48 UTC (permalink / raw)
To: Jeevan B; +Cc: igt-dev
On Thu, Oct 03, 2019 at 01:15:08PM +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.
>
> 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 | 149 +++++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 160 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..c02c02e 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,43 @@ 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 +175,25 @@ 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)
> +{
> + 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 +234,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 +255,78 @@ 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_color_fb(data, &data->fb_rgb, red_green_blue);
> + create_color_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_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, dc_flag);
> + /* Calculate delay to generate idle frame */
> + delay = ((1000 * 1000) / data->mode->vrefresh);
> +
> + for (i = 0; i < VIDEO_FRAMES; i++) {
100 frames may not be enough to get to DC3co. DC states may be disabled
after a (full) modeset due to the delayed disabling of panel VDD (and
VDD requires DC states to be off). Since that's 3 sec on the panel I
tried with a 600ms power cycle delay (VDD off delay=5 * power cycle
delay), let's run the test for some time (let's say 6 seconds) instead
of a number of frames.
> + 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_prev_cnt);
> +}
> +
> +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");
We can't assume that we won't enter DC5 at all. There may some
scheduling latency that results the DC5 idle thread to be called and
re-enable DC5.
> + cleanup_dc3co(data);
> +}
> +
> static void test_dc_state_psr(data_t *data, int dc_flag)
> {
> uint32_t dc_counter_before_psr;
> @@ -288,6 +428,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
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-10-07 13:49 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-03 7:45 [igt-dev] [PATCH i-g-t 0/3] Add a new IGT test to validate DC3CO state Jeevan B
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 1/3] igt/i915/i915_pm_dc: DC3CO PSR2 helpers Jeevan B
2019-10-04 12:43 ` Animesh Manna
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 2/3] Add a new IGT test to validate DC3CO state Jeevan B
2019-10-03 11:25 ` Anshuman Gupta
2019-10-03 14:50 ` Arkadiusz Hiler
2019-10-04 10:39 ` Jeevan B
2019-10-04 13:59 ` Arkadiusz Hiler
2019-10-07 13:48 ` Imre Deak
2019-10-03 7:45 ` [igt-dev] [PATCH i-g-t 3/3] HAX: Run igt@i915_pm_dc@dc3co-vpb-simulation in BAT Jeevan B
2019-10-03 9:24 ` [igt-dev] ✓ Fi.CI.BAT: success for Add a new IGT test to validate DC3CO state. (rev4) Patchwork
2019-10-03 15:56 ` [igt-dev] ✗ Fi.CI.IGT: failure " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.