* [igt-dev] [PATCH i-g-t v2 0/5] lpsp platform agnostic support
@ 2020-03-23 6:32 Anshuman Gupta
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 1/5] lib/igt_pm: Add lib func to get lpsp capability Anshuman Gupta
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Anshuman Gupta @ 2020-03-23 6:32 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
This series adds lpsp igt val platform agnostic support.
This has been a big gap on igt validation side in order to
validate low power single pipe use cases.
V2 update of this series had fixed CI failures observed with v1.
This should be test with i915 lpsp support, which adds
i915_lpsp_info debugfs.
Anshuman Gupta (5):
lib/igt_pm: Add lib func to get lpsp capability
tests/i915_pm_lpsp: lpsp platform agnostic support
tests/i915_pm_lpsp: Skip panel-fitter subtest for 1024x768 panels
tests/i915_pm_lpsp: screens-disabled subtest use igt_wait
tests/i915_pm_rpm: lpsp/non-lpsp screen mode_set_data
lib/igt_pm.c | 27 ++++
lib/igt_pm.h | 1 +
tests/i915/i915_pm_lpsp.c | 311 +++++++++++++++++++++++---------------
tests/i915/i915_pm_rpm.c | 53 ++++---
4 files changed, 248 insertions(+), 144 deletions(-)
--
2.25.2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* [igt-dev] [PATCH i-g-t v2 1/5] lib/igt_pm: Add lib func to get lpsp capability
2020-03-23 6:32 [igt-dev] [PATCH i-g-t v2 0/5] lpsp platform agnostic support Anshuman Gupta
@ 2020-03-23 6:32 ` Anshuman Gupta
2020-03-23 6:55 ` Peres, Martin
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support Anshuman Gupta
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Anshuman Gupta @ 2020-03-23 6:32 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
This lib function evaluate the lpsp capability from
the connector specific debugfs attribute i915_lpsp_info.
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
lib/igt_pm.c | 27 +++++++++++++++++++++++++++
lib/igt_pm.h | 1 +
2 files changed, 28 insertions(+)
diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index 9d441e1b..7a6cab7c 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -37,6 +37,7 @@
#include <dirent.h>
#include "drmtest.h"
+#include "igt_kms.h"
#include "igt_pm.h"
#include "igt_aux.h"
#include "igt_sysfs.h"
@@ -827,3 +828,29 @@ bool igt_pm_pc8_plus_residencies_enabled(int msr_fd)
return true;
}
+
+/**
+ * igt_output_is_lpsp_capable:
+ * @drm_fd: fd to drm device
+ * @output: igt output for which lpsp capability need to be evaluated
+ * Check lpsp capability for a given output.
+ *
+ * Returns:
+ * True if given output is lpsp capable otherwise false.
+ */
+bool igt_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
+{
+ char buf[256];
+ int fd, len;
+
+ fd = igt_debugfs_connector_dir(drm_fd, output->name, O_RDONLY);
+ igt_require(fd >= 0);
+ len = igt_debugfs_simple_read(fd, "i915_lpsp_info", buf, sizeof(buf));
+
+ if (len < 0)
+ igt_assert_eq(len, -ENODEV);
+
+ close(fd);
+
+ return strstr(buf, "LPSP capable");
+}
diff --git a/lib/igt_pm.h b/lib/igt_pm.h
index 5e438452..076d8c27 100644
--- a/lib/igt_pm.h
+++ b/lib/igt_pm.h
@@ -53,5 +53,6 @@ enum igt_runtime_pm_status igt_get_runtime_pm_status(void);
bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
bool igt_pm_dmc_loaded(int debugfs);
bool igt_pm_pc8_plus_residencies_enabled(int msr_fd);
+bool igt_output_is_lpsp_capable(int drm_fd, igt_output_t *output);
#endif /* IGT_PM_H */
--
2.25.2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support
2020-03-23 6:32 [igt-dev] [PATCH i-g-t v2 0/5] lpsp platform agnostic support Anshuman Gupta
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 1/5] lib/igt_pm: Add lib func to get lpsp capability Anshuman Gupta
@ 2020-03-23 6:32 ` Anshuman Gupta
2020-03-23 7:00 ` Peres, Martin
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 3/5] tests/i915_pm_lpsp: Skip panel-fitter subtest for 1024x768 panels Anshuman Gupta
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Anshuman Gupta @ 2020-03-23 6:32 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
Current implementation of lpsp igt test, assumed that every non-edp
panel isn't a lpsp panel but it is not true on TGL anymore,
any HDMI/DP/DSI panel connected on pipe A and connected to PORT_{A,B,C}
can drive LPSP.
Even on older Gen9 platform a DP panel can drive lpsp on Port A.
This requires complete design change in current lpsp igt for a platform
agnostic support.
The new igt approach is relies on connector specific debugfs
attribute i915_lpsp_info, which exposes whether an output is capable
of driving lpsp and whether lpsp is enabled.
v2:
- CI failures fixup.
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
tests/i915/i915_pm_lpsp.c | 303 +++++++++++++++++++++++---------------
1 file changed, 186 insertions(+), 117 deletions(-)
diff --git a/tests/i915/i915_pm_lpsp.c b/tests/i915/i915_pm_lpsp.c
index 42938e10..e400c92e 100644
--- a/tests/i915/i915_pm_lpsp.c
+++ b/tests/i915/i915_pm_lpsp.c
@@ -25,49 +25,125 @@
*/
#include "igt.h"
+#include "igt_kmod.h"
+#include "igt_pm.h"
+#include "igt_sysfs.h"
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
+#define MAX_SINK_LPSP_INFO_BUF_LEN 5000
-static bool supports_lpsp(uint32_t devid)
+#define PWR_DOMAIN_INFO "i915_power_domain_info"
+
+const char *snd_module[10] = {"snd_hda_codec_hdmi", "snd_hda_intel", "snd_hda_codec", "snd_hda_core", NULL};
+
+typedef struct {
+ int drm_fd;
+ int debugfs_fd;
+ uint32_t devid;
+ char *pwr_dmn_info;
+ igt_display_t display;
+ struct igt_fb fb;
+ drmModeModeInfo *mode;
+ igt_output_t *output;
+} data_t;
+
+static void debugfs_read(int fd, const char *param, char *buf, int len)
{
- return IS_HASWELL(devid) || IS_BROADWELL(devid);
+ len = igt_debugfs_simple_read(fd, param, buf, len);
+ if (len < 0)
+ igt_assert_eq(len, -ENODEV);
}
-static bool lpsp_is_enabled(int drm_fd)
+static bool lpsp_is_enabled(data_t *data)
{
- uint32_t val;
+ char buf[MAX_SINK_LPSP_INFO_BUF_LEN];
+ int fd;
+
+ fd = igt_debugfs_connector_dir(data->drm_fd, data->output->name,
+ O_RDONLY);
+ igt_require(fd >= 0);
+
+ debugfs_read(fd, "i915_lpsp_info", buf, sizeof(buf));
+ close(fd);
- val = INREG(HSW_PWR_WELL_CTL2);
- return !(val & HSW_PWR_WELL_STATE_ENABLED);
+ return strstr(buf, "LPSP enabled");
}
-/* The LPSP mode is all about an enabled pipe, but we expect to also be in the
- * low power mode when no pipes are enabled, so do this check anyway. */
-static void screens_disabled_subtest(int drm_fd, drmModeResPtr drm_res)
+/*
+ * The LPSP mode is all about an enabled pipe, but we expect to also be in the
+ * low power mode when no pipes are enabled, so do this check anyway.
+ */
+static void screens_disabled_subtest(data_t *data)
{
- kmstest_unset_all_crtcs(drm_fd, drm_res);
- igt_assert(lpsp_is_enabled(drm_fd));
+ igt_output_t *output;
+ enum pipe pipe;
+ igt_plane_t *primary;
+
+ for_each_pipe_with_single_output(&data->display, pipe, output) {
+ data->output = output;
+ igt_output_set_pipe(data->output, pipe);
+ primary = igt_output_get_plane_type(data->output,
+ DRM_PLANE_TYPE_PRIMARY);
+ igt_plane_set_fb(primary, NULL);
+ igt_display_commit(&data->display);
+ }
+
+ igt_assert(lpsp_is_enabled(data));
}
-static uint32_t create_fb(int drm_fd, int width, int height)
+static void check_output_lpsp(data_t *data)
{
- struct igt_fb fb;
+ if (igt_output_is_lpsp_capable(data->drm_fd, data->output))
+ igt_assert_f(lpsp_is_enabled(data), "%s: lpsp is not enabled\n%s:\n%s\n",
+ data->output->name, PWR_DOMAIN_INFO, data->pwr_dmn_info =
+ igt_sysfs_get(data->debugfs_fd, PWR_DOMAIN_INFO));
+ else
+ igt_assert(!lpsp_is_enabled(data));
+}
- return igt_create_pattern_fb(drm_fd, width, height, DRM_FORMAT_XRGB8888,
- LOCAL_DRM_FORMAT_MOD_NONE, &fb);
+static void setup_lpsp_output(data_t *data)
+{
+ igt_plane_t *primary;
+
+ /* set output pipe = PIPE_A for LPSP */
+ igt_output_set_pipe(data->output, PIPE_A);
+ primary = igt_output_get_plane_type(data->output,
+ DRM_PLANE_TYPE_PRIMARY);
+ igt_plane_set_fb(primary, NULL);
+ igt_create_pattern_fb(data->drm_fd,
+ data->mode->hdisplay, data->mode->vdisplay,
+ DRM_FORMAT_XRGB8888,
+ LOCAL_DRM_FORMAT_MOD_NONE,
+ &data->fb);
+ igt_plane_set_fb(primary, &data->fb);
+ igt_display_commit(&data->display);
+}
+
+static void cleanup_lpsp_output(data_t *data)
+{
+ igt_plane_t *primary;
+
+ if (!data->output)
+ return;
+
+ primary = igt_output_get_plane_type(data->output,
+ DRM_PLANE_TYPE_PRIMARY);
+ igt_plane_set_fb(primary, NULL);
+ igt_output_set_pipe(data->output, PIPE_NONE);
+ igt_display_commit(&data->display);
+ igt_remove_fb(data->drm_fd, &data->fb);
+ data->output = NULL;
}
-static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
- drmModeConnectorPtr *drm_connectors, uint32_t devid,
- bool use_panel_fitter)
+static void edp_subtest(data_t *data, bool use_panel_fitter)
{
- int i, rc;
- uint32_t crtc_id = 0, buffer_id = 0;
- drmModeConnectorPtr connector = NULL;
- drmModeModeInfoPtr mode = NULL;
+ igt_display_t *display = &data->display;
+ igt_output_t *output;
+ int valid_output;
+
drmModeModeInfo std_1024_mode = {
.clock = 65000,
.hdisplay = 1024,
@@ -86,23 +162,17 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
.name = "Custom 1024x768",
};
- kmstest_unset_all_crtcs(drm_fd, drm_res);
-
- for (i = 0; i < drm_res->count_connectors; i++) {
- drmModeConnectorPtr c = drm_connectors[i];
+ for_each_connected_output(display, output) {
+ drmModeConnectorPtr c = output->config.connector;
if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
continue;
- if (c->connection != DRM_MODE_CONNECTED)
+ if (!igt_pipe_connector_valid(PIPE_A, output))
continue;
- if (!use_panel_fitter && c->count_modes) {
- connector = c;
- mode = &c->modes[0];
- break;
- }
+ data->output = output;
+
if (use_panel_fitter) {
- connector = c;
/* This is one of the modes Xorg creates for panels, so
* it should work just fine. Notice that Gens that
@@ -113,124 +183,123 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
c->modes[0].hdisplay > 1024);
igt_assert(c->count_modes &&
c->modes[0].vdisplay > 768);
- mode = &std_1024_mode;
- break;
+ data->mode = &std_1024_mode;
+ igt_output_override_mode(output, data->mode);
+ } else {
+ data->mode = igt_output_get_mode(output);
}
- }
- igt_require(connector);
- crtc_id = kmstest_find_crtc_for_connector(drm_fd, drm_res, connector,
- 0);
- buffer_id = create_fb(drm_fd, mode->hdisplay, mode->vdisplay);
+ setup_lpsp_output(data);
+
+ if (use_panel_fitter && IS_HASWELL(data->devid))
+ igt_assert(!lpsp_is_enabled(data));
+ else
+ check_output_lpsp(data);
- igt_assert(buffer_id);
- igt_assert(connector);
- igt_assert(mode);
+ cleanup_lpsp_output(data);
+ valid_output++;
+ }
- rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0,
- &connector->connector_id, 1, mode);
- igt_assert_eq(rc, 0);
+ igt_require_f(valid_output, "No edp connector found\n");
+}
- if (use_panel_fitter) {
- if (IS_HASWELL(devid))
- igt_assert(!lpsp_is_enabled(drm_fd));
- else
- igt_assert(lpsp_is_enabled(drm_fd));
- } else {
- igt_assert(lpsp_is_enabled(drm_fd));
+static bool unload_snd_hda_core_module(void)
+{
+ int i;
+
+ /* unbind snd_hda_intel */
+ kick_snd_hda_intel();
+
+ for (i = 0; snd_module[i]; i++) {
+ if (igt_kmod_is_loaded(snd_module[i]) &&
+ igt_kmod_unload(snd_module[i], KMOD_REMOVE_FORCE)) {
+ igt_warn("Could not unload %s\n", snd_module[i]);
+ igt_kmod_list_loaded();
+ igt_lsof("/dev/snd");
+ return false;
+ }
}
+
+ return true;
}
-static void non_edp_subtest(int drm_fd, drmModeResPtr drm_res,
- drmModeConnectorPtr *drm_connectors)
+static void load_snd_hda_core_module(void)
{
- int i, rc;
- uint32_t crtc_id = 0, buffer_id = 0;
- drmModeConnectorPtr connector = NULL;
- drmModeModeInfoPtr mode = NULL;
+ igt_kmod_load("snd_hda_core", NULL);
+}
+
+static void non_edp_subtest(data_t *data)
+{
+ igt_display_t *display = &data->display;
+ igt_output_t *output;
+ int valid_output;
- kmstest_unset_all_crtcs(drm_fd, drm_res);
+ /* DP/HDMI panel requires to drive lpsp without audio */
+ igt_require(unload_snd_hda_core_module());
- for (i = 0; i < drm_res->count_connectors; i++) {
- drmModeConnectorPtr c = drm_connectors[i];
+ for_each_connected_output(display, output) {
+ drmModeConnectorPtr c = output->config.connector;
if (c->connector_type == DRM_MODE_CONNECTOR_eDP)
continue;
if (c->connection != DRM_MODE_CONNECTED)
continue;
+ if (!igt_pipe_connector_valid(PIPE_A, output))
+ continue;
- if (c->count_modes) {
- connector = c;
- mode = &c->modes[0];
- break;
- }
+ data->output = output;
+ data->mode = igt_output_get_mode(output);
+ setup_lpsp_output(data);
+ check_output_lpsp(data);
+ cleanup_lpsp_output(data);
+ load_snd_hda_core_module();
+ valid_output++;
}
- igt_require(connector);
-
- crtc_id = kmstest_find_crtc_for_connector(drm_fd, drm_res, connector,
- 0);
- buffer_id = create_fb(drm_fd, mode->hdisplay, mode->vdisplay);
- igt_assert(buffer_id);
- igt_assert(mode);
-
- rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0,
- &connector->connector_id, 1, mode);
- igt_assert_eq(rc, 0);
-
- igt_assert(!lpsp_is_enabled(drm_fd));
+ igt_require_f(valid_output, "No non-edp connector found\n");
}
-#define MAX_CONNECTORS 32
-
-int drm_fd;
-uint32_t devid;
-drmModeResPtr drm_res;
-drmModeConnectorPtr drm_connectors[MAX_CONNECTORS];
-struct intel_mmio_data mmio_data;
+IGT_TEST_DESCRIPTION("These tests validates display Low Power Single Pipe configurations");
igt_main
{
- igt_fixture {
- int i;
-
- drm_fd = drm_open_driver_master(DRIVER_INTEL);
- igt_require(drm_fd >= 0);
-
- devid = intel_get_drm_devid(drm_fd);
+ data_t data = {};
- drm_res = drmModeGetResources(drm_fd);
- igt_require(drm_res);
- igt_assert(drm_res->count_connectors <= MAX_CONNECTORS);
-
- for (i = 0; i < drm_res->count_connectors; i++)
- drm_connectors[i] = drmModeGetConnectorCurrent(drm_fd,
- drm_res->connectors[i]);
+ igt_fixture {
+ data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+ igt_require(data.drm_fd >= 0);
+ data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
+ igt_require(data.debugfs_fd >= 0);
igt_pm_enable_audio_runtime_pm();
-
- igt_require(supports_lpsp(devid));
-
- intel_register_access_init(&mmio_data, intel_get_pci_device(), 0, drm_fd);
-
kmstest_set_vt_graphics_mode();
+ data.devid = intel_get_drm_devid(data.drm_fd);
+ igt_display_require(&data.display, data.drm_fd);
+ igt_require(igt_pm_dmc_loaded(data.debugfs_fd));
}
+ igt_describe("This test validates lpsp while all crtc are disabled");
igt_subtest("screens-disabled")
- screens_disabled_subtest(drm_fd, drm_res);
+ screens_disabled_subtest(&data);
+ igt_describe("This test validates lpsp on eDP panel");
igt_subtest("edp-native")
- edp_subtest(drm_fd, drm_res, drm_connectors, devid, false);
+ edp_subtest(&data, false);
+ igt_fixture
+ cleanup_lpsp_output(&data);
+ igt_describe("This test validates lpsp on eDP panel while forcing panel_fitter");
igt_subtest("edp-panel-fitter")
- edp_subtest(drm_fd, drm_res, drm_connectors, devid, true);
+ edp_subtest(&data, true);
+ igt_fixture
+ cleanup_lpsp_output(&data);
+ igt_describe("This test validates lpsp on DP/HDMI/DSI panels");
igt_subtest("non-edp")
- non_edp_subtest(drm_fd, drm_res, drm_connectors);
+ non_edp_subtest(&data);
+ igt_fixture
+ cleanup_lpsp_output(&data);
igt_fixture {
- int i;
-
- intel_register_access_fini(&mmio_data);
- for (i = 0; i < drm_res->count_connectors; i++)
- drmModeFreeConnector(drm_connectors[i]);
- drmModeFreeResources(drm_res);
- close(drm_fd);
+ free(data.pwr_dmn_info);
+ load_snd_hda_core_module();
+ close(data.drm_fd);
+ igt_display_fini(&data.display);
}
}
--
2.25.2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] [PATCH i-g-t v2 3/5] tests/i915_pm_lpsp: Skip panel-fitter subtest for 1024x768 panels
2020-03-23 6:32 [igt-dev] [PATCH i-g-t v2 0/5] lpsp platform agnostic support Anshuman Gupta
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 1/5] lib/igt_pm: Add lib func to get lpsp capability Anshuman Gupta
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support Anshuman Gupta
@ 2020-03-23 6:32 ` Anshuman Gupta
2020-03-23 7:00 ` Peres, Martin
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 4/5] tests/i915_pm_lpsp: screens-disabled subtest use igt_wait Anshuman Gupta
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: Anshuman Gupta @ 2020-03-23 6:32 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
It makes sense to skip the test if edp-panel native resoultion
is limted to 1024x768 rather than failing the test.
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
tests/i915/i915_pm_lpsp.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/i915/i915_pm_lpsp.c b/tests/i915/i915_pm_lpsp.c
index e400c92e..0ce694e6 100644
--- a/tests/i915/i915_pm_lpsp.c
+++ b/tests/i915/i915_pm_lpsp.c
@@ -179,10 +179,10 @@ static void edp_subtest(data_t *data, bool use_panel_fitter)
* support LPSP are too new for panels with native
* 1024x768 resolution, so this should force the panel
* fitter. */
- igt_assert(c->count_modes &&
- c->modes[0].hdisplay > 1024);
- igt_assert(c->count_modes &&
- c->modes[0].vdisplay > 768);
+ igt_require(c->count_modes &&
+ c->modes[0].hdisplay > 1024);
+ igt_require(c->count_modes &&
+ c->modes[0].vdisplay > 768);
data->mode = &std_1024_mode;
igt_output_override_mode(output, data->mode);
} else {
--
2.25.2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] [PATCH i-g-t v2 4/5] tests/i915_pm_lpsp: screens-disabled subtest use igt_wait
2020-03-23 6:32 [igt-dev] [PATCH i-g-t v2 0/5] lpsp platform agnostic support Anshuman Gupta
` (2 preceding siblings ...)
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 3/5] tests/i915_pm_lpsp: Skip panel-fitter subtest for 1024x768 panels Anshuman Gupta
@ 2020-03-23 6:32 ` Anshuman Gupta
2020-03-23 7:05 ` Peres, Martin
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 5/5] tests/i915_pm_rpm: lpsp/non-lpsp screen mode_set_data Anshuman Gupta
2020-03-23 7:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for lpsp platform agnostic support (rev3) Patchwork
5 siblings, 1 reply; 18+ messages in thread
From: Anshuman Gupta @ 2020-03-23 6:32 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
Some times delayed audio codec disabling causes failure
of test, using igt_wait to check lpsp after disabling all outputs.
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
tests/i915/i915_pm_lpsp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/i915/i915_pm_lpsp.c b/tests/i915/i915_pm_lpsp.c
index 0ce694e6..5f15c034 100644
--- a/tests/i915/i915_pm_lpsp.c
+++ b/tests/i915/i915_pm_lpsp.c
@@ -91,7 +91,7 @@ static void screens_disabled_subtest(data_t *data)
igt_display_commit(&data->display);
}
- igt_assert(lpsp_is_enabled(data));
+ igt_assert(igt_wait(lpsp_is_enabled(data), 1000, 100));
}
static void check_output_lpsp(data_t *data)
--
2.25.2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] [PATCH i-g-t v2 5/5] tests/i915_pm_rpm: lpsp/non-lpsp screen mode_set_data
2020-03-23 6:32 [igt-dev] [PATCH i-g-t v2 0/5] lpsp platform agnostic support Anshuman Gupta
` (3 preceding siblings ...)
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 4/5] tests/i915_pm_lpsp: screens-disabled subtest use igt_wait Anshuman Gupta
@ 2020-03-23 6:32 ` Anshuman Gupta
2020-03-23 7:10 ` Peres, Martin
2020-03-23 7:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for lpsp platform agnostic support (rev3) Patchwork
5 siblings, 1 reply; 18+ messages in thread
From: Anshuman Gupta @ 2020-03-23 6:32 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
Initialize the mode set params for lpsp/non-lpsp screen
based upon their output lpsp capability instead
of edp/non-edp output type.
v2:
- CI failures fixup.
Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
---
tests/i915/i915_pm_rpm.c | 53 +++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 23 deletions(-)
diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index 4f8124dc..e2fb732c 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -94,6 +94,7 @@ struct mode_set_data {
drmModeResPtr res;
drmModeConnectorPtr connectors[MAX_CONNECTORS];
drmModePropertyBlobPtr edids[MAX_CONNECTORS];
+ igt_display_t display;
uint32_t devid;
};
@@ -254,29 +255,29 @@ static bool init_modeset_params_for_type(struct mode_set_data *data,
{
drmModeConnectorPtr connector = NULL;
drmModeModeInfoPtr mode = NULL;
+ igt_output_t *output = NULL;
+ igt_display_t *display = &data->display;
- if (!data->res)
+ if (!data->res || !display)
return false;
- for (int i = 0; i < data->res->count_connectors; i++) {
- drmModeConnectorPtr c = data->connectors[i];
+ for_each_connected_output(display, output) {
+ drmModeConnectorPtr c = output->config.connector;
if (type == SCREEN_TYPE_LPSP &&
- c->connector_type != DRM_MODE_CONNECTOR_eDP)
+ !igt_output_is_lpsp_capable(drm_fd, output))
continue;
if (type == SCREEN_TYPE_NON_LPSP &&
- c->connector_type == DRM_MODE_CONNECTOR_eDP)
+ igt_output_is_lpsp_capable(drm_fd, output))
continue;
- if (c->connection == DRM_MODE_CONNECTED && c->count_modes) {
- connector = c;
- mode = &c->modes[0];
- break;
- }
+ connector = c;
+ mode = igt_output_get_mode(output);
+ break;
}
- if (!connector)
+ if (!connector || !mode)
return false;
igt_create_pattern_fb(drm_fd, mode->hdisplay, mode->vdisplay,
@@ -397,6 +398,7 @@ static void init_mode_set_data(struct mode_set_data *data)
kmstest_set_vt_graphics_mode();
}
+ igt_display_require(&data->display, drm_fd);
data->devid = intel_get_drm_devid(drm_fd);
init_modeset_cached_params(&ms_data);
}
@@ -410,6 +412,8 @@ static void fini_mode_set_data(struct mode_set_data *data)
}
drmModeFreeResources(data->res);
}
+
+ igt_display_fini(&data->display);
}
static void get_drm_info(struct compare_data *data)
@@ -760,7 +764,7 @@ static void dump_file(int dir, const char *filename)
free(contents);
}
-static bool setup_environment(void)
+static bool setup_environment(bool display_disabled)
{
if (has_runtime_pm)
goto out;
@@ -772,7 +776,8 @@ static bool setup_environment(void)
debugfs = igt_debugfs_dir(drm_fd);
igt_require(debugfs != -1);
- init_mode_set_data(&ms_data);
+ if (!display_disabled)
+ init_mode_set_data(&ms_data);
igt_pm_enable_sata_link_power_management();
@@ -785,13 +790,14 @@ static bool setup_environment(void)
igt_require(igt_pm_dmc_loaded(debugfs));
out:
- disable_all_screens(&ms_data);
+ if (!display_disabled)
+ disable_all_screens(&ms_data);
dump_file(debugfs, "i915_runtime_pm_status");
return wait_for_suspended();
}
-static void teardown_environment(void)
+static void teardown_environment(bool display_disabled)
{
close(msr_fd);
if (has_pc8)
@@ -801,7 +807,8 @@ static void teardown_environment(void)
igt_pm_restore_sata_link_power_management();
- fini_mode_set_data(&ms_data);
+ if (!display_disabled)
+ fini_mode_set_data(&ms_data);
close(debugfs);
close(drm_fd);
@@ -2008,7 +2015,7 @@ static struct option long_options[] = {
igt_main_args("", long_options, help_str, opt_handler, NULL)
{
igt_subtest("basic-rte") {
- igt_assert(setup_environment());
+ igt_assert(setup_environment(false));
basic_subtest();
}
@@ -2016,7 +2023,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
* PC8+. We don't want bug reports from cases where the machine is just
* not properly configured. */
igt_fixture
- igt_require(setup_environment());
+ igt_require(setup_environment(false));
if (stay)
igt_subtest("stay")
@@ -2146,7 +2153,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
}
igt_fixture
- teardown_environment();
+ teardown_environment(false);
igt_subtest("module-reload") {
igt_debug("Reload w/o display\n");
@@ -2155,9 +2162,9 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
igt_kmsg(KMSG_INFO "Reloading i915 w/o display\n");
igt_assert_eq(igt_i915_driver_load("disable_display=1 mmio_debug=-1"), 0);
- igt_assert(setup_environment());
+ igt_assert(setup_environment(true));
igt_assert(igt_wait(device_in_pci_d3(), 2000, 100));
- teardown_environment();
+ teardown_environment(true);
igt_debug("Reload as normal\n");
igt_i915_driver_unload();
@@ -2165,11 +2172,11 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
igt_kmsg(KMSG_INFO "Reloading i915 as normal\n");
igt_assert_eq(igt_i915_driver_load("mmio_debug=-1"), 0);
- igt_assert(setup_environment());
+ igt_assert(setup_environment(false));
igt_assert(igt_wait(device_in_pci_d3(), 2000, 100));
if (enable_one_screen_with_type(&ms_data, SCREEN_TYPE_ANY))
drm_resources_equal_subtest();
- teardown_environment();
+ teardown_environment(false);
/* Remove our mmio_debugging module */
igt_i915_driver_unload();
--
2.25.2
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 1/5] lib/igt_pm: Add lib func to get lpsp capability
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 1/5] lib/igt_pm: Add lib func to get lpsp capability Anshuman Gupta
@ 2020-03-23 6:55 ` Peres, Martin
2020-03-24 6:05 ` Anshuman Gupta
0 siblings, 1 reply; 18+ messages in thread
From: Peres, Martin @ 2020-03-23 6:55 UTC (permalink / raw)
To: Gupta, Anshuman, igt-dev@lists.freedesktop.org; +Cc: Peres, Martin
[-- Attachment #1: Type: text/plain, Size: 2118 bytes --]
On 2020-03-23 08:32, Anshuman Gupta wrote:
> This lib function evaluate the lpsp capability from
> the connector specific debugfs attribute i915_lpsp_info.
>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> lib/igt_pm.c | 27 +++++++++++++++++++++++++++
> lib/igt_pm.h | 1 +
> 2 files changed, 28 insertions(+)
>
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index 9d441e1b..7a6cab7c 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -37,6 +37,7 @@
> #include <dirent.h>
>
> #include "drmtest.h"
> +#include "igt_kms.h"
> #include "igt_pm.h"
> #include "igt_aux.h"
> #include "igt_sysfs.h"
> @@ -827,3 +828,29 @@ bool igt_pm_pc8_plus_residencies_enabled(int msr_fd)
>
> return true;
> }
> +
> +/**
> + * igt_output_is_lpsp_capable:
> + * @drm_fd: fd to drm device
> + * @output: igt output for which lpsp capability need to be evaluated
> + * Check lpsp capability for a given output.
> + *
> + * Returns:
> + * True if given output is lpsp capable otherwise false.
> + */
> +bool igt_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
Do we have a file for i915-specific features? If not, maybe prefixing
the function with i915 would help readers understand that this is an
i915-specific function.
> +{
> + char buf[256];
> + int fd, len;
> +
> + fd = igt_debugfs_connector_dir(drm_fd, output->name, O_RDONLY);
> + igt_require(fd >= 0);
> + len = igt_debugfs_simple_read(fd, "i915_lpsp_info", buf, sizeof(buf));
> +
> + if (len < 0)
> + igt_assert_eq(len, -ENODEV);
> +
> + close(fd);
> +
> + return strstr(buf, "LPSP capable");
> +}
> diff --git a/lib/igt_pm.h b/lib/igt_pm.h
> index 5e438452..076d8c27 100644
> --- a/lib/igt_pm.h
> +++ b/lib/igt_pm.h
> @@ -53,5 +53,6 @@ enum igt_runtime_pm_status igt_get_runtime_pm_status(void);
> bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
> bool igt_pm_dmc_loaded(int debugfs);
> bool igt_pm_pc8_plus_residencies_enabled(int msr_fd);
> +bool igt_output_is_lpsp_capable(int drm_fd, igt_output_t *output);
>
> #endif /* IGT_PM_H */
>
[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1774 bytes --]
[-- Attachment #3: Type: text/plain, Size: 154 bytes --]
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support Anshuman Gupta
@ 2020-03-23 7:00 ` Peres, Martin
2020-03-23 7:46 ` Anshuman Gupta
0 siblings, 1 reply; 18+ messages in thread
From: Peres, Martin @ 2020-03-23 7:00 UTC (permalink / raw)
To: Gupta, Anshuman, igt-dev@lists.freedesktop.org; +Cc: Peres, Martin
[-- Attachment #1: Type: text/plain, Size: 13558 bytes --]
On 2020-03-23 08:32, Anshuman Gupta wrote:
> Current implementation of lpsp igt test, assumed that every non-edp
> panel isn't a lpsp panel but it is not true on TGL anymore,
> any HDMI/DP/DSI panel connected on pipe A and connected to PORT_{A,B,C}
> can drive LPSP.
> Even on older Gen9 platform a DP panel can drive lpsp on Port A.
> This requires complete design change in current lpsp igt for a platform
> agnostic support.
>
> The new igt approach is relies on connector specific debugfs
> attribute i915_lpsp_info, which exposes whether an output is capable
> of driving lpsp and whether lpsp is enabled.
>
> v2:
> - CI failures fixup.
>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> tests/i915/i915_pm_lpsp.c | 303 +++++++++++++++++++++++---------------
> 1 file changed, 186 insertions(+), 117 deletions(-)
>
> diff --git a/tests/i915/i915_pm_lpsp.c b/tests/i915/i915_pm_lpsp.c
> index 42938e10..e400c92e 100644
> --- a/tests/i915/i915_pm_lpsp.c
> +++ b/tests/i915/i915_pm_lpsp.c
> @@ -25,49 +25,125 @@
> */
>
> #include "igt.h"
> +#include "igt_kmod.h"
> +#include "igt_pm.h"
> +#include "igt_sysfs.h"
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <unistd.h>
>
> +#define MAX_SINK_LPSP_INFO_BUF_LEN 5000
>
> -static bool supports_lpsp(uint32_t devid)
> +#define PWR_DOMAIN_INFO "i915_power_domain_info"
> +
> +const char *snd_module[10] = {"snd_hda_codec_hdmi", "snd_hda_intel", "snd_hda_codec", "snd_hda_core", NULL};
> +
> +typedef struct {
> + int drm_fd;
> + int debugfs_fd;
> + uint32_t devid;
> + char *pwr_dmn_info;
> + igt_display_t display;
> + struct igt_fb fb;
> + drmModeModeInfo *mode;
> + igt_output_t *output;
> +} data_t;
> +
> +static void debugfs_read(int fd, const char *param, char *buf, int len)
> {
> - return IS_HASWELL(devid) || IS_BROADWELL(devid);
> + len = igt_debugfs_simple_read(fd, param, buf, len);
> + if (len < 0)
> + igt_assert_eq(len, -ENODEV);
> }
>
> -static bool lpsp_is_enabled(int drm_fd)
> +static bool lpsp_is_enabled(data_t *data)
> {
> - uint32_t val;
> + char buf[MAX_SINK_LPSP_INFO_BUF_LEN];
> + int fd;
> +
> + fd = igt_debugfs_connector_dir(data->drm_fd, data->output->name,
> + O_RDONLY);
> + igt_require(fd >= 0);
> +
> + debugfs_read(fd, "i915_lpsp_info", buf, sizeof(buf));
> + close(fd);
>
> - val = INREG(HSW_PWR_WELL_CTL2);
> - return !(val & HSW_PWR_WELL_STATE_ENABLED);
> + return strstr(buf, "LPSP enabled");
> }
>
> -/* The LPSP mode is all about an enabled pipe, but we expect to also be in the
> - * low power mode when no pipes are enabled, so do this check anyway. */
> -static void screens_disabled_subtest(int drm_fd, drmModeResPtr drm_res)
> +/*
> + * The LPSP mode is all about an enabled pipe, but we expect to also be in the
> + * low power mode when no pipes are enabled, so do this check anyway.
> + */
> +static void screens_disabled_subtest(data_t *data)
> {
> - kmstest_unset_all_crtcs(drm_fd, drm_res);
> - igt_assert(lpsp_is_enabled(drm_fd));
> + igt_output_t *output;
> + enum pipe pipe;
> + igt_plane_t *primary;
> +
> + for_each_pipe_with_single_output(&data->display, pipe, output) {
> + data->output = output;
> + igt_output_set_pipe(data->output, pipe);
> + primary = igt_output_get_plane_type(data->output,
> + DRM_PLANE_TYPE_PRIMARY);
> + igt_plane_set_fb(primary, NULL);
> + igt_display_commit(&data->display);
> + }
> +
> + igt_assert(lpsp_is_enabled(data));
> }
>
> -static uint32_t create_fb(int drm_fd, int width, int height)
> +static void check_output_lpsp(data_t *data)
> {
> - struct igt_fb fb;
> + if (igt_output_is_lpsp_capable(data->drm_fd, data->output))
> + igt_assert_f(lpsp_is_enabled(data), "%s: lpsp is not enabled\n%s:\n%s\n",
> + data->output->name, PWR_DOMAIN_INFO, data->pwr_dmn_info =
> + igt_sysfs_get(data->debugfs_fd, PWR_DOMAIN_INFO));
> + else
> + igt_assert(!lpsp_is_enabled(data));
> +}
>
> - return igt_create_pattern_fb(drm_fd, width, height, DRM_FORMAT_XRGB8888,
> - LOCAL_DRM_FORMAT_MOD_NONE, &fb);
> +static void setup_lpsp_output(data_t *data)
> +{
> + igt_plane_t *primary;
> +
> + /* set output pipe = PIPE_A for LPSP */
> + igt_output_set_pipe(data->output, PIPE_A);
> + primary = igt_output_get_plane_type(data->output,
> + DRM_PLANE_TYPE_PRIMARY);
> + igt_plane_set_fb(primary, NULL);
> + igt_create_pattern_fb(data->drm_fd,
> + data->mode->hdisplay, data->mode->vdisplay,
> + DRM_FORMAT_XRGB8888,
> + LOCAL_DRM_FORMAT_MOD_NONE,
> + &data->fb);
> + igt_plane_set_fb(primary, &data->fb);
> + igt_display_commit(&data->display);
> +}
> +
> +static void cleanup_lpsp_output(data_t *data)
> +{
> + igt_plane_t *primary;
> +
> + if (!data->output)
> + return;
> +
> + primary = igt_output_get_plane_type(data->output,
> + DRM_PLANE_TYPE_PRIMARY);
> + igt_plane_set_fb(primary, NULL);
> + igt_output_set_pipe(data->output, PIPE_NONE);
> + igt_display_commit(&data->display);
> + igt_remove_fb(data->drm_fd, &data->fb);
> + data->output = NULL;
> }
>
> -static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
> - drmModeConnectorPtr *drm_connectors, uint32_t devid,
> - bool use_panel_fitter)
> +static void edp_subtest(data_t *data, bool use_panel_fitter)
> {
> - int i, rc;
> - uint32_t crtc_id = 0, buffer_id = 0;
> - drmModeConnectorPtr connector = NULL;
> - drmModeModeInfoPtr mode = NULL;
> + igt_display_t *display = &data->display;
> + igt_output_t *output;
> + int valid_output;
> +
> drmModeModeInfo std_1024_mode = {
> .clock = 65000,
> .hdisplay = 1024,
> @@ -86,23 +162,17 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
> .name = "Custom 1024x768",
> };
>
> - kmstest_unset_all_crtcs(drm_fd, drm_res);
> -
> - for (i = 0; i < drm_res->count_connectors; i++) {
> - drmModeConnectorPtr c = drm_connectors[i];
> + for_each_connected_output(display, output) {
> + drmModeConnectorPtr c = output->config.connector;
>
> if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> continue;
> - if (c->connection != DRM_MODE_CONNECTED)
> + if (!igt_pipe_connector_valid(PIPE_A, output))
> continue;
>
> - if (!use_panel_fitter && c->count_modes) {
> - connector = c;
> - mode = &c->modes[0];
> - break;
> - }
> + data->output = output;
> +
> if (use_panel_fitter) {
> - connector = c;
>
> /* This is one of the modes Xorg creates for panels, so
> * it should work just fine. Notice that Gens that
> @@ -113,124 +183,123 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
> c->modes[0].hdisplay > 1024);
> igt_assert(c->count_modes &&
> c->modes[0].vdisplay > 768);
> - mode = &std_1024_mode;
> - break;
> + data->mode = &std_1024_mode;
> + igt_output_override_mode(output, data->mode);
> + } else {
> + data->mode = igt_output_get_mode(output);
> }
> - }
> - igt_require(connector);
>
> - crtc_id = kmstest_find_crtc_for_connector(drm_fd, drm_res, connector,
> - 0);
> - buffer_id = create_fb(drm_fd, mode->hdisplay, mode->vdisplay);
> + setup_lpsp_output(data);
> +
> + if (use_panel_fitter && IS_HASWELL(data->devid))
> + igt_assert(!lpsp_is_enabled(data));
> + else
> + check_output_lpsp(data);
>
> - igt_assert(buffer_id);
> - igt_assert(connector);
> - igt_assert(mode);
> + cleanup_lpsp_output(data);
> + valid_output++;
> + }
>
> - rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0,
> - &connector->connector_id, 1, mode);
> - igt_assert_eq(rc, 0);
> + igt_require_f(valid_output, "No edp connector found\n");
> +}
>
> - if (use_panel_fitter) {
> - if (IS_HASWELL(devid))
> - igt_assert(!lpsp_is_enabled(drm_fd));
> - else
> - igt_assert(lpsp_is_enabled(drm_fd));
> - } else {
> - igt_assert(lpsp_is_enabled(drm_fd));
> +static bool unload_snd_hda_core_module(void)
> +{
> + int i;
> +
> + /* unbind snd_hda_intel */
> + kick_snd_hda_intel();
> +
> + for (i = 0; snd_module[i]; i++) {
> + if (igt_kmod_is_loaded(snd_module[i]) &&
> + igt_kmod_unload(snd_module[i], KMOD_REMOVE_FORCE)) {
> + igt_warn("Could not unload %s\n", snd_module[i]);
> + igt_kmod_list_loaded();
> + igt_lsof("/dev/snd");
> + return false;
> + }
> }
> +
> + return true;
> }
>
> -static void non_edp_subtest(int drm_fd, drmModeResPtr drm_res,
> - drmModeConnectorPtr *drm_connectors)
> +static void load_snd_hda_core_module(void)
> {
> - int i, rc;
> - uint32_t crtc_id = 0, buffer_id = 0;
> - drmModeConnectorPtr connector = NULL;
> - drmModeModeInfoPtr mode = NULL;
> + igt_kmod_load("snd_hda_core", NULL);
> +}
> +
> +static void non_edp_subtest(data_t *data)
> +{
> + igt_display_t *display = &data->display;
> + igt_output_t *output;
> + int valid_output;
>
> - kmstest_unset_all_crtcs(drm_fd, drm_res);
> + /* DP/HDMI panel requires to drive lpsp without audio */
> + igt_require(unload_snd_hda_core_module());
I'm not super happy with having to unload modules that it would be very
easy to forget to re-add and would make audio tests fail :s
IMO, if the DUT is not playing sound, we should be able to enter LPSP.
If no, then I would consider this a driver bug for wasting energy
sending 0s to the screen.
Thoughts on this?
Martin
>
> - for (i = 0; i < drm_res->count_connectors; i++) {
> - drmModeConnectorPtr c = drm_connectors[i];
> + for_each_connected_output(display, output) {
> + drmModeConnectorPtr c = output->config.connector;
>
> if (c->connector_type == DRM_MODE_CONNECTOR_eDP)
> continue;
> if (c->connection != DRM_MODE_CONNECTED)
> continue;
> + if (!igt_pipe_connector_valid(PIPE_A, output))
> + continue;
>
> - if (c->count_modes) {
> - connector = c;
> - mode = &c->modes[0];
> - break;
> - }
> + data->output = output;
> + data->mode = igt_output_get_mode(output);
> + setup_lpsp_output(data);
> + check_output_lpsp(data);
> + cleanup_lpsp_output(data);
> + load_snd_hda_core_module();
> + valid_output++;
> }
> - igt_require(connector);
> -
> - crtc_id = kmstest_find_crtc_for_connector(drm_fd, drm_res, connector,
> - 0);
> - buffer_id = create_fb(drm_fd, mode->hdisplay, mode->vdisplay);
>
> - igt_assert(buffer_id);
> - igt_assert(mode);
> -
> - rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0,
> - &connector->connector_id, 1, mode);
> - igt_assert_eq(rc, 0);
> -
> - igt_assert(!lpsp_is_enabled(drm_fd));
> + igt_require_f(valid_output, "No non-edp connector found\n");
> }
>
> -#define MAX_CONNECTORS 32
> -
> -int drm_fd;
> -uint32_t devid;
> -drmModeResPtr drm_res;
> -drmModeConnectorPtr drm_connectors[MAX_CONNECTORS];
> -struct intel_mmio_data mmio_data;
> +IGT_TEST_DESCRIPTION("These tests validates display Low Power Single Pipe configurations");
> igt_main
> {
> - igt_fixture {
> - int i;
> -
> - drm_fd = drm_open_driver_master(DRIVER_INTEL);
> - igt_require(drm_fd >= 0);
> -
> - devid = intel_get_drm_devid(drm_fd);
> + data_t data = {};
>
> - drm_res = drmModeGetResources(drm_fd);
> - igt_require(drm_res);
> - igt_assert(drm_res->count_connectors <= MAX_CONNECTORS);
> -
> - for (i = 0; i < drm_res->count_connectors; i++)
> - drm_connectors[i] = drmModeGetConnectorCurrent(drm_fd,
> - drm_res->connectors[i]);
> + igt_fixture {
>
> + data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> + igt_require(data.drm_fd >= 0);
> + data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> + igt_require(data.debugfs_fd >= 0);
> igt_pm_enable_audio_runtime_pm();
> -
> - igt_require(supports_lpsp(devid));
> -
> - intel_register_access_init(&mmio_data, intel_get_pci_device(), 0, drm_fd);
> -
> kmstest_set_vt_graphics_mode();
> + data.devid = intel_get_drm_devid(data.drm_fd);
> + igt_display_require(&data.display, data.drm_fd);
> + igt_require(igt_pm_dmc_loaded(data.debugfs_fd));
> }
>
> + igt_describe("This test validates lpsp while all crtc are disabled");
> igt_subtest("screens-disabled")
> - screens_disabled_subtest(drm_fd, drm_res);
> + screens_disabled_subtest(&data);
> + igt_describe("This test validates lpsp on eDP panel");
> igt_subtest("edp-native")
> - edp_subtest(drm_fd, drm_res, drm_connectors, devid, false);
> + edp_subtest(&data, false);
> + igt_fixture
> + cleanup_lpsp_output(&data);
> + igt_describe("This test validates lpsp on eDP panel while forcing panel_fitter");
> igt_subtest("edp-panel-fitter")
> - edp_subtest(drm_fd, drm_res, drm_connectors, devid, true);
> + edp_subtest(&data, true);
> + igt_fixture
> + cleanup_lpsp_output(&data);
> + igt_describe("This test validates lpsp on DP/HDMI/DSI panels");
> igt_subtest("non-edp")
> - non_edp_subtest(drm_fd, drm_res, drm_connectors);
> + non_edp_subtest(&data);
> + igt_fixture
> + cleanup_lpsp_output(&data);
>
> igt_fixture {
> - int i;
> -
> - intel_register_access_fini(&mmio_data);
> - for (i = 0; i < drm_res->count_connectors; i++)
> - drmModeFreeConnector(drm_connectors[i]);
> - drmModeFreeResources(drm_res);
> - close(drm_fd);
> + free(data.pwr_dmn_info);
> + load_snd_hda_core_module();
> + close(data.drm_fd);
> + igt_display_fini(&data.display);
> }
> }
>
[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1774 bytes --]
[-- Attachment #3: Type: text/plain, Size: 154 bytes --]
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 3/5] tests/i915_pm_lpsp: Skip panel-fitter subtest for 1024x768 panels
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 3/5] tests/i915_pm_lpsp: Skip panel-fitter subtest for 1024x768 panels Anshuman Gupta
@ 2020-03-23 7:00 ` Peres, Martin
0 siblings, 0 replies; 18+ messages in thread
From: Peres, Martin @ 2020-03-23 7:00 UTC (permalink / raw)
To: Gupta, Anshuman, igt-dev@lists.freedesktop.org; +Cc: Peres, Martin
[-- Attachment #1: Type: text/plain, Size: 1214 bytes --]
On 2020-03-23 08:32, Anshuman Gupta wrote:
> It makes sense to skip the test if edp-panel native resoultion
> is limted to 1024x768 rather than failing the test.
Acked-by: Martin Peres <martin.peres@linux.intel.com>
>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> tests/i915/i915_pm_lpsp.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tests/i915/i915_pm_lpsp.c b/tests/i915/i915_pm_lpsp.c
> index e400c92e..0ce694e6 100644
> --- a/tests/i915/i915_pm_lpsp.c
> +++ b/tests/i915/i915_pm_lpsp.c
> @@ -179,10 +179,10 @@ static void edp_subtest(data_t *data, bool use_panel_fitter)
> * support LPSP are too new for panels with native
> * 1024x768 resolution, so this should force the panel
> * fitter. */
> - igt_assert(c->count_modes &&
> - c->modes[0].hdisplay > 1024);
> - igt_assert(c->count_modes &&
> - c->modes[0].vdisplay > 768);
> + igt_require(c->count_modes &&
> + c->modes[0].hdisplay > 1024);
> + igt_require(c->count_modes &&
> + c->modes[0].vdisplay > 768);
> data->mode = &std_1024_mode;
> igt_output_override_mode(output, data->mode);
> } else {
>
[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1774 bytes --]
[-- Attachment #3: Type: text/plain, Size: 154 bytes --]
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 4/5] tests/i915_pm_lpsp: screens-disabled subtest use igt_wait
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 4/5] tests/i915_pm_lpsp: screens-disabled subtest use igt_wait Anshuman Gupta
@ 2020-03-23 7:05 ` Peres, Martin
2020-03-23 9:37 ` Anshuman Gupta
0 siblings, 1 reply; 18+ messages in thread
From: Peres, Martin @ 2020-03-23 7:05 UTC (permalink / raw)
To: Gupta, Anshuman, igt-dev@lists.freedesktop.org; +Cc: Peres, Martin
[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]
On 2020-03-23 08:32, Anshuman Gupta wrote:
> Some times delayed audio codec disabling causes failure
> of test, using igt_wait to check lpsp after disabling all outputs.
Seems like this answers my question on patch 2/5: We are sending audio
to the screen even when we do not have anything to send...
I know there are some screens that take ages to re-enable sound after
not getting any for a while, but waiting 10 or even 30s without sound
before disabling it does not sound like a terrible idea. My TV
automatically disables sound after way less than that anyway.
So, to me, this is a driver bug.
That being said, the patch is fine even if your reason for it is not
acceptable.
Martin
>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> tests/i915/i915_pm_lpsp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/i915/i915_pm_lpsp.c b/tests/i915/i915_pm_lpsp.c
> index 0ce694e6..5f15c034 100644
> --- a/tests/i915/i915_pm_lpsp.c
> +++ b/tests/i915/i915_pm_lpsp.c
> @@ -91,7 +91,7 @@ static void screens_disabled_subtest(data_t *data)
> igt_display_commit(&data->display);
> }
>
> - igt_assert(lpsp_is_enabled(data));
> + igt_assert(igt_wait(lpsp_is_enabled(data), 1000, 100));
> }
>
> static void check_output_lpsp(data_t *data)
>
[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1774 bytes --]
[-- Attachment #3: Type: text/plain, Size: 154 bytes --]
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 5/5] tests/i915_pm_rpm: lpsp/non-lpsp screen mode_set_data
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 5/5] tests/i915_pm_rpm: lpsp/non-lpsp screen mode_set_data Anshuman Gupta
@ 2020-03-23 7:10 ` Peres, Martin
0 siblings, 0 replies; 18+ messages in thread
From: Peres, Martin @ 2020-03-23 7:10 UTC (permalink / raw)
To: Gupta, Anshuman, igt-dev@lists.freedesktop.org; +Cc: Peres, Martin
[-- Attachment #1: Type: text/plain, Size: 6397 bytes --]
On 2020-03-23 08:32, Anshuman Gupta wrote:
> Initialize the mode set params for lpsp/non-lpsp screen
> based upon their output lpsp capability instead
> of edp/non-edp output type.
>
> v2:
> - CI failures fixup.
This patch is:
Acked-by: Martin Peres <martin.peres@linux.intel.com>
Congrats on your series, Anshuman! My only gripe with it is letting the
audio free-wheel even when we don't need it and could reach LPSP.
That being said, if you were to remove the sound drivers unloading, the
series would be very close to a mergeable state. It's OK if the new
tests fail, they can be fixed with a corresponding i915 patch series.
Thanks again!
Martin
>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> tests/i915/i915_pm_rpm.c | 53 +++++++++++++++++++++++-----------------
> 1 file changed, 30 insertions(+), 23 deletions(-)
>
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index 4f8124dc..e2fb732c 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -94,6 +94,7 @@ struct mode_set_data {
> drmModeResPtr res;
> drmModeConnectorPtr connectors[MAX_CONNECTORS];
> drmModePropertyBlobPtr edids[MAX_CONNECTORS];
> + igt_display_t display;
>
> uint32_t devid;
> };
> @@ -254,29 +255,29 @@ static bool init_modeset_params_for_type(struct mode_set_data *data,
> {
> drmModeConnectorPtr connector = NULL;
> drmModeModeInfoPtr mode = NULL;
> + igt_output_t *output = NULL;
> + igt_display_t *display = &data->display;
>
> - if (!data->res)
> + if (!data->res || !display)
> return false;
>
> - for (int i = 0; i < data->res->count_connectors; i++) {
> - drmModeConnectorPtr c = data->connectors[i];
> + for_each_connected_output(display, output) {
> + drmModeConnectorPtr c = output->config.connector;
>
> if (type == SCREEN_TYPE_LPSP &&
> - c->connector_type != DRM_MODE_CONNECTOR_eDP)
> + !igt_output_is_lpsp_capable(drm_fd, output))
> continue;
>
> if (type == SCREEN_TYPE_NON_LPSP &&
> - c->connector_type == DRM_MODE_CONNECTOR_eDP)
> + igt_output_is_lpsp_capable(drm_fd, output))
> continue;
>
> - if (c->connection == DRM_MODE_CONNECTED && c->count_modes) {
> - connector = c;
> - mode = &c->modes[0];
> - break;
> - }
> + connector = c;
> + mode = igt_output_get_mode(output);
> + break;
> }
>
> - if (!connector)
> + if (!connector || !mode)
> return false;
>
> igt_create_pattern_fb(drm_fd, mode->hdisplay, mode->vdisplay,
> @@ -397,6 +398,7 @@ static void init_mode_set_data(struct mode_set_data *data)
> kmstest_set_vt_graphics_mode();
> }
>
> + igt_display_require(&data->display, drm_fd);
> data->devid = intel_get_drm_devid(drm_fd);
> init_modeset_cached_params(&ms_data);
> }
> @@ -410,6 +412,8 @@ static void fini_mode_set_data(struct mode_set_data *data)
> }
> drmModeFreeResources(data->res);
> }
> +
> + igt_display_fini(&data->display);
> }
>
> static void get_drm_info(struct compare_data *data)
> @@ -760,7 +764,7 @@ static void dump_file(int dir, const char *filename)
> free(contents);
> }
>
> -static bool setup_environment(void)
> +static bool setup_environment(bool display_disabled)
> {
> if (has_runtime_pm)
> goto out;
> @@ -772,7 +776,8 @@ static bool setup_environment(void)
> debugfs = igt_debugfs_dir(drm_fd);
> igt_require(debugfs != -1);
>
> - init_mode_set_data(&ms_data);
> + if (!display_disabled)
> + init_mode_set_data(&ms_data);
>
> igt_pm_enable_sata_link_power_management();
>
> @@ -785,13 +790,14 @@ static bool setup_environment(void)
> igt_require(igt_pm_dmc_loaded(debugfs));
>
> out:
> - disable_all_screens(&ms_data);
> + if (!display_disabled)
> + disable_all_screens(&ms_data);
> dump_file(debugfs, "i915_runtime_pm_status");
>
> return wait_for_suspended();
> }
>
> -static void teardown_environment(void)
> +static void teardown_environment(bool display_disabled)
> {
> close(msr_fd);
> if (has_pc8)
> @@ -801,7 +807,8 @@ static void teardown_environment(void)
>
> igt_pm_restore_sata_link_power_management();
>
> - fini_mode_set_data(&ms_data);
> + if (!display_disabled)
> + fini_mode_set_data(&ms_data);
>
> close(debugfs);
> close(drm_fd);
> @@ -2008,7 +2015,7 @@ static struct option long_options[] = {
> igt_main_args("", long_options, help_str, opt_handler, NULL)
> {
> igt_subtest("basic-rte") {
> - igt_assert(setup_environment());
> + igt_assert(setup_environment(false));
> basic_subtest();
> }
>
> @@ -2016,7 +2023,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
> * PC8+. We don't want bug reports from cases where the machine is just
> * not properly configured. */
> igt_fixture
> - igt_require(setup_environment());
> + igt_require(setup_environment(false));
>
> if (stay)
> igt_subtest("stay")
> @@ -2146,7 +2153,7 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
> }
>
> igt_fixture
> - teardown_environment();
> + teardown_environment(false);
>
> igt_subtest("module-reload") {
> igt_debug("Reload w/o display\n");
> @@ -2155,9 +2162,9 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
> igt_kmsg(KMSG_INFO "Reloading i915 w/o display\n");
> igt_assert_eq(igt_i915_driver_load("disable_display=1 mmio_debug=-1"), 0);
>
> - igt_assert(setup_environment());
> + igt_assert(setup_environment(true));
> igt_assert(igt_wait(device_in_pci_d3(), 2000, 100));
> - teardown_environment();
> + teardown_environment(true);
>
> igt_debug("Reload as normal\n");
> igt_i915_driver_unload();
> @@ -2165,11 +2172,11 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
> igt_kmsg(KMSG_INFO "Reloading i915 as normal\n");
> igt_assert_eq(igt_i915_driver_load("mmio_debug=-1"), 0);
>
> - igt_assert(setup_environment());
> + igt_assert(setup_environment(false));
> igt_assert(igt_wait(device_in_pci_d3(), 2000, 100));
> if (enable_one_screen_with_type(&ms_data, SCREEN_TYPE_ANY))
> drm_resources_equal_subtest();
> - teardown_environment();
> + teardown_environment(false);
>
> /* Remove our mmio_debugging module */
> igt_i915_driver_unload();
>
[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1774 bytes --]
[-- Attachment #3: Type: text/plain, Size: 154 bytes --]
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* [igt-dev] ✗ Fi.CI.BAT: failure for lpsp platform agnostic support (rev3)
2020-03-23 6:32 [igt-dev] [PATCH i-g-t v2 0/5] lpsp platform agnostic support Anshuman Gupta
` (4 preceding siblings ...)
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 5/5] tests/i915_pm_rpm: lpsp/non-lpsp screen mode_set_data Anshuman Gupta
@ 2020-03-23 7:27 ` Patchwork
5 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-03-23 7:27 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: igt-dev
== Series Details ==
Series: lpsp platform agnostic support (rev3)
URL : https://patchwork.freedesktop.org/series/74647/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_8172 -> IGTPW_4338
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with IGTPW_4338 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_4338, 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_4338/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_4338:
### IGT changes ###
#### Possible regressions ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-glk-dsi: [PASS][1] -> [FAIL][2] +2 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-glk-dsi/igt@i915_pm_rpm@basic-pci-d3-state.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-glk-dsi/igt@i915_pm_rpm@basic-pci-d3-state.html
- fi-icl-u2: [PASS][3] -> [FAIL][4] +1 similar issue
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-icl-u2/igt@i915_pm_rpm@basic-pci-d3-state.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-icl-u2/igt@i915_pm_rpm@basic-pci-d3-state.html
- fi-kbl-r: [PASS][5] -> [FAIL][6] +2 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-kbl-r/igt@i915_pm_rpm@basic-pci-d3-state.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-kbl-r/igt@i915_pm_rpm@basic-pci-d3-state.html
- fi-icl-guc: [PASS][7] -> [FAIL][8] +2 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-icl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-icl-guc/igt@i915_pm_rpm@basic-pci-d3-state.html
- fi-bdw-5557u: [PASS][9] -> [FAIL][10] +2 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-bdw-5557u/igt@i915_pm_rpm@basic-pci-d3-state.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-bdw-5557u/igt@i915_pm_rpm@basic-pci-d3-state.html
- fi-kbl-soraka: [PASS][11] -> [FAIL][12] +2 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-kbl-soraka/igt@i915_pm_rpm@basic-pci-d3-state.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-kbl-soraka/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_pm_rpm@basic-rte:
- fi-kbl-7500u: NOTRUN -> [FAIL][13] +2 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-kbl-7500u/igt@i915_pm_rpm@basic-rte.html
- fi-gdg-551: NOTRUN -> [FAIL][14] +2 similar issues
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-gdg-551/igt@i915_pm_rpm@basic-rte.html
- fi-skl-lmem: NOTRUN -> [FAIL][15] +2 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-skl-lmem/igt@i915_pm_rpm@basic-rte.html
- fi-cml-u2: [PASS][16] -> [FAIL][17] +2 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-cml-u2/igt@i915_pm_rpm@basic-rte.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-cml-u2/igt@i915_pm_rpm@basic-rte.html
- fi-bxt-dsi: [PASS][18] -> [FAIL][19] +2 similar issues
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-bxt-dsi/igt@i915_pm_rpm@basic-rte.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-bxt-dsi/igt@i915_pm_rpm@basic-rte.html
- fi-byt-j1900: [PASS][20] -> [FAIL][21] +2 similar issues
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-byt-j1900/igt@i915_pm_rpm@basic-rte.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-byt-j1900/igt@i915_pm_rpm@basic-rte.html
- fi-hsw-4770: [PASS][22] -> [FAIL][23] +2 similar issues
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-hsw-4770/igt@i915_pm_rpm@basic-rte.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-hsw-4770/igt@i915_pm_rpm@basic-rte.html
- fi-snb-2520m: NOTRUN -> [FAIL][24] +2 similar issues
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-snb-2520m/igt@i915_pm_rpm@basic-rte.html
- fi-apl-guc: [PASS][25] -> [FAIL][26] +2 similar issues
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-apl-guc/igt@i915_pm_rpm@basic-rte.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-apl-guc/igt@i915_pm_rpm@basic-rte.html
* igt@i915_pm_rpm@module-reload:
- fi-cfl-8109u: [PASS][27] -> [FAIL][28] +2 similar issues
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
- fi-skl-6600u: [PASS][29] -> [FAIL][30] +2 similar issues
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-skl-6600u/igt@i915_pm_rpm@module-reload.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-skl-6600u/igt@i915_pm_rpm@module-reload.html
- fi-cfl-8700k: [PASS][31] -> [FAIL][32] +2 similar issues
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-cfl-8700k/igt@i915_pm_rpm@module-reload.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-cfl-8700k/igt@i915_pm_rpm@module-reload.html
- fi-bsw-kefka: [PASS][33] -> [FAIL][34] +2 similar issues
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-bsw-kefka/igt@i915_pm_rpm@module-reload.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-bsw-kefka/igt@i915_pm_rpm@module-reload.html
- fi-kbl-x1275: [PASS][35] -> [FAIL][36] +2 similar issues
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
- fi-icl-dsi: [PASS][37] -> [FAIL][38] +2 similar issues
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-icl-dsi/igt@i915_pm_rpm@module-reload.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-icl-dsi/igt@i915_pm_rpm@module-reload.html
- fi-skl-guc: [PASS][39] -> [FAIL][40] +2 similar issues
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-skl-guc/igt@i915_pm_rpm@module-reload.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-skl-guc/igt@i915_pm_rpm@module-reload.html
- fi-bsw-n3050: NOTRUN -> [FAIL][41] +2 similar issues
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-bsw-n3050/igt@i915_pm_rpm@module-reload.html
- fi-skl-6700k2: [PASS][42] -> [FAIL][43] +2 similar issues
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-skl-6700k2/igt@i915_pm_rpm@module-reload.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-skl-6700k2/igt@i915_pm_rpm@module-reload.html
- fi-cfl-guc: [PASS][44] -> [FAIL][45] +2 similar issues
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-cfl-guc/igt@i915_pm_rpm@module-reload.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-cfl-guc/igt@i915_pm_rpm@module-reload.html
- fi-icl-y: [PASS][46] -> [FAIL][47] +2 similar issues
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-icl-y/igt@i915_pm_rpm@module-reload.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-icl-y/igt@i915_pm_rpm@module-reload.html
- fi-cml-s: [PASS][48] -> [FAIL][49] +2 similar issues
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-cml-s/igt@i915_pm_rpm@module-reload.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-cml-s/igt@i915_pm_rpm@module-reload.html
#### Warnings ####
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-bwr-2160: [SKIP][50] ([fdo#109271]) -> [FAIL][51] +2 similar issues
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-bwr-2160/igt@i915_pm_rpm@basic-pci-d3-state.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-bwr-2160/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_pm_rpm@basic-rte:
- fi-ivb-3770: [SKIP][52] ([fdo#109271]) -> [FAIL][53] +2 similar issues
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-ivb-3770/igt@i915_pm_rpm@basic-rte.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-ivb-3770/igt@i915_pm_rpm@basic-rte.html
* igt@i915_pm_rpm@module-reload:
- fi-pnv-d510: [SKIP][54] ([fdo#109271]) -> [FAIL][55] +2 similar issues
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-pnv-d510/igt@i915_pm_rpm@module-reload.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-pnv-d510/igt@i915_pm_rpm@module-reload.html
- fi-ilk-650: [SKIP][56] ([fdo#109271]) -> [FAIL][57] +2 similar issues
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-ilk-650/igt@i915_pm_rpm@module-reload.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-ilk-650/igt@i915_pm_rpm@module-reload.html
- fi-blb-e6850: [SKIP][58] ([fdo#109271]) -> [FAIL][59] +2 similar issues
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-blb-e6850/igt@i915_pm_rpm@module-reload.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-blb-e6850/igt@i915_pm_rpm@module-reload.html
- fi-elk-e7500: [SKIP][60] ([fdo#109271]) -> [FAIL][61] +2 similar issues
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-elk-e7500/igt@i915_pm_rpm@module-reload.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-elk-e7500/igt@i915_pm_rpm@module-reload.html
- fi-icl-u2: [DMESG-WARN][62] ([i915#289]) -> [FAIL][63]
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-icl-u2/igt@i915_pm_rpm@module-reload.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-icl-u2/igt@i915_pm_rpm@module-reload.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@i915_pm_rpm@basic-pci-d3-state:
- {fi-kbl-7560u}: NOTRUN -> [FAIL][64] +2 similar issues
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-kbl-7560u/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_pm_rpm@basic-rte:
- {fi-ehl-1}: [PASS][65] -> [FAIL][66] +2 similar issues
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-ehl-1/igt@i915_pm_rpm@basic-rte.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-ehl-1/igt@i915_pm_rpm@basic-rte.html
- {fi-tgl-u}: [PASS][67] -> [FAIL][68] +2 similar issues
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-tgl-u/igt@i915_pm_rpm@basic-rte.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-tgl-u/igt@i915_pm_rpm@basic-rte.html
Known issues
------------
Here are the changes found in IGTPW_4338 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_pm_rpm@module-reload:
- fi-kbl-8809g: [PASS][69] -> [SKIP][70] ([fdo#109271])
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-kbl-8809g/igt@i915_pm_rpm@module-reload.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-kbl-8809g/igt@i915_pm_rpm@module-reload.html
#### Possible fixes ####
* igt@i915_selftest@live@hangcheck:
- {fi-ehl-1}: [INCOMPLETE][71] -> [PASS][72]
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8172/fi-ehl-1/igt@i915_selftest@live@hangcheck.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/fi-ehl-1/igt@i915_selftest@live@hangcheck.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#289]: https://gitlab.freedesktop.org/drm/intel/issues/289
Participating hosts (39 -> 40)
------------------------------
Additional (7): fi-bsw-n3050 fi-snb-2520m fi-kbl-7500u fi-gdg-551 fi-skl-lmem fi-kbl-7560u fi-bsw-nick
Missing (6): fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5526 -> IGTPW_4338
CI-20190529: 20190529
CI_DRM_8172: b53e00acaa550df13d601f258011b180013c5d38 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_4338: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/index.html
IGT_5526: f49ebeee9f54d6f23c60a842f75f65561d452ab0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4338/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support
2020-03-23 7:00 ` Peres, Martin
@ 2020-03-23 7:46 ` Anshuman Gupta
2020-03-23 8:04 ` Peres, Martin
2020-03-23 12:48 ` Kai Vehmanen
0 siblings, 2 replies; 18+ messages in thread
From: Anshuman Gupta @ 2020-03-23 7:46 UTC (permalink / raw)
To: Peres, Martin, Kai Vehmanen; +Cc: igt-dev@lists.freedesktop.org
On 2020-03-23 at 12:30:08 +0530, Peres, Martin wrote:
> On 2020-03-23 08:32, Anshuman Gupta wrote:
> > Current implementation of lpsp igt test, assumed that every non-edp
> > panel isn't a lpsp panel but it is not true on TGL anymore,
> > any HDMI/DP/DSI panel connected on pipe A and connected to PORT_{A,B,C}
> > can drive LPSP.
> > Even on older Gen9 platform a DP panel can drive lpsp on Port A.
> > This requires complete design change in current lpsp igt for a platform
> > agnostic support.
> >
> > The new igt approach is relies on connector specific debugfs
> > attribute i915_lpsp_info, which exposes whether an output is capable
> > of driving lpsp and whether lpsp is enabled.
> >
> > v2:
> > - CI failures fixup.
> >
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> > tests/i915/i915_pm_lpsp.c | 303 +++++++++++++++++++++++---------------
> > 1 file changed, 186 insertions(+), 117 deletions(-)
> >
> > diff --git a/tests/i915/i915_pm_lpsp.c b/tests/i915/i915_pm_lpsp.c
> > index 42938e10..e400c92e 100644
> > --- a/tests/i915/i915_pm_lpsp.c
> > +++ b/tests/i915/i915_pm_lpsp.c
> > @@ -25,49 +25,125 @@
> > */
> >
> > #include "igt.h"
> > +#include "igt_kmod.h"
> > +#include "igt_pm.h"
> > +#include "igt_sysfs.h"
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > #include <fcntl.h>
> > #include <unistd.h>
> >
> > +#define MAX_SINK_LPSP_INFO_BUF_LEN 5000
> >
> > -static bool supports_lpsp(uint32_t devid)
> > +#define PWR_DOMAIN_INFO "i915_power_domain_info"
> > +
> > +const char *snd_module[10] = {"snd_hda_codec_hdmi", "snd_hda_intel", "snd_hda_codec", "snd_hda_core", NULL};
> > +
> > +typedef struct {
> > + int drm_fd;
> > + int debugfs_fd;
> > + uint32_t devid;
> > + char *pwr_dmn_info;
> > + igt_display_t display;
> > + struct igt_fb fb;
> > + drmModeModeInfo *mode;
> > + igt_output_t *output;
> > +} data_t;
> > +
> > +static void debugfs_read(int fd, const char *param, char *buf, int len)
> > {
> > - return IS_HASWELL(devid) || IS_BROADWELL(devid);
> > + len = igt_debugfs_simple_read(fd, param, buf, len);
> > + if (len < 0)
> > + igt_assert_eq(len, -ENODEV);
> > }
> >
> > -static bool lpsp_is_enabled(int drm_fd)
> > +static bool lpsp_is_enabled(data_t *data)
> > {
> > - uint32_t val;
> > + char buf[MAX_SINK_LPSP_INFO_BUF_LEN];
> > + int fd;
> > +
> > + fd = igt_debugfs_connector_dir(data->drm_fd, data->output->name,
> > + O_RDONLY);
> > + igt_require(fd >= 0);
> > +
> > + debugfs_read(fd, "i915_lpsp_info", buf, sizeof(buf));
> > + close(fd);
> >
> > - val = INREG(HSW_PWR_WELL_CTL2);
> > - return !(val & HSW_PWR_WELL_STATE_ENABLED);
> > + return strstr(buf, "LPSP enabled");
> > }
> >
> > -/* The LPSP mode is all about an enabled pipe, but we expect to also be in the
> > - * low power mode when no pipes are enabled, so do this check anyway. */
> > -static void screens_disabled_subtest(int drm_fd, drmModeResPtr drm_res)
> > +/*
> > + * The LPSP mode is all about an enabled pipe, but we expect to also be in the
> > + * low power mode when no pipes are enabled, so do this check anyway.
> > + */
> > +static void screens_disabled_subtest(data_t *data)
> > {
> > - kmstest_unset_all_crtcs(drm_fd, drm_res);
> > - igt_assert(lpsp_is_enabled(drm_fd));
> > + igt_output_t *output;
> > + enum pipe pipe;
> > + igt_plane_t *primary;
> > +
> > + for_each_pipe_with_single_output(&data->display, pipe, output) {
> > + data->output = output;
> > + igt_output_set_pipe(data->output, pipe);
> > + primary = igt_output_get_plane_type(data->output,
> > + DRM_PLANE_TYPE_PRIMARY);
> > + igt_plane_set_fb(primary, NULL);
> > + igt_display_commit(&data->display);
> > + }
> > +
> > + igt_assert(lpsp_is_enabled(data));
> > }
> >
> > -static uint32_t create_fb(int drm_fd, int width, int height)
> > +static void check_output_lpsp(data_t *data)
> > {
> > - struct igt_fb fb;
> > + if (igt_output_is_lpsp_capable(data->drm_fd, data->output))
> > + igt_assert_f(lpsp_is_enabled(data), "%s: lpsp is not enabled\n%s:\n%s\n",
> > + data->output->name, PWR_DOMAIN_INFO, data->pwr_dmn_info =
> > + igt_sysfs_get(data->debugfs_fd, PWR_DOMAIN_INFO));
> > + else
> > + igt_assert(!lpsp_is_enabled(data));
> > +}
> >
> > - return igt_create_pattern_fb(drm_fd, width, height, DRM_FORMAT_XRGB8888,
> > - LOCAL_DRM_FORMAT_MOD_NONE, &fb);
> > +static void setup_lpsp_output(data_t *data)
> > +{
> > + igt_plane_t *primary;
> > +
> > + /* set output pipe = PIPE_A for LPSP */
> > + igt_output_set_pipe(data->output, PIPE_A);
> > + primary = igt_output_get_plane_type(data->output,
> > + DRM_PLANE_TYPE_PRIMARY);
> > + igt_plane_set_fb(primary, NULL);
> > + igt_create_pattern_fb(data->drm_fd,
> > + data->mode->hdisplay, data->mode->vdisplay,
> > + DRM_FORMAT_XRGB8888,
> > + LOCAL_DRM_FORMAT_MOD_NONE,
> > + &data->fb);
> > + igt_plane_set_fb(primary, &data->fb);
> > + igt_display_commit(&data->display);
> > +}
> > +
> > +static void cleanup_lpsp_output(data_t *data)
> > +{
> > + igt_plane_t *primary;
> > +
> > + if (!data->output)
> > + return;
> > +
> > + primary = igt_output_get_plane_type(data->output,
> > + DRM_PLANE_TYPE_PRIMARY);
> > + igt_plane_set_fb(primary, NULL);
> > + igt_output_set_pipe(data->output, PIPE_NONE);
> > + igt_display_commit(&data->display);
> > + igt_remove_fb(data->drm_fd, &data->fb);
> > + data->output = NULL;
> > }
> >
> > -static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
> > - drmModeConnectorPtr *drm_connectors, uint32_t devid,
> > - bool use_panel_fitter)
> > +static void edp_subtest(data_t *data, bool use_panel_fitter)
> > {
> > - int i, rc;
> > - uint32_t crtc_id = 0, buffer_id = 0;
> > - drmModeConnectorPtr connector = NULL;
> > - drmModeModeInfoPtr mode = NULL;
> > + igt_display_t *display = &data->display;
> > + igt_output_t *output;
> > + int valid_output;
> > +
> > drmModeModeInfo std_1024_mode = {
> > .clock = 65000,
> > .hdisplay = 1024,
> > @@ -86,23 +162,17 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
> > .name = "Custom 1024x768",
> > };
> >
> > - kmstest_unset_all_crtcs(drm_fd, drm_res);
> > -
> > - for (i = 0; i < drm_res->count_connectors; i++) {
> > - drmModeConnectorPtr c = drm_connectors[i];
> > + for_each_connected_output(display, output) {
> > + drmModeConnectorPtr c = output->config.connector;
> >
> > if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
> > continue;
> > - if (c->connection != DRM_MODE_CONNECTED)
> > + if (!igt_pipe_connector_valid(PIPE_A, output))
> > continue;
> >
> > - if (!use_panel_fitter && c->count_modes) {
> > - connector = c;
> > - mode = &c->modes[0];
> > - break;
> > - }
> > + data->output = output;
> > +
> > if (use_panel_fitter) {
> > - connector = c;
> >
> > /* This is one of the modes Xorg creates for panels, so
> > * it should work just fine. Notice that Gens that
> > @@ -113,124 +183,123 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
> > c->modes[0].hdisplay > 1024);
> > igt_assert(c->count_modes &&
> > c->modes[0].vdisplay > 768);
> > - mode = &std_1024_mode;
> > - break;
> > + data->mode = &std_1024_mode;
> > + igt_output_override_mode(output, data->mode);
> > + } else {
> > + data->mode = igt_output_get_mode(output);
> > }
> > - }
> > - igt_require(connector);
> >
> > - crtc_id = kmstest_find_crtc_for_connector(drm_fd, drm_res, connector,
> > - 0);
> > - buffer_id = create_fb(drm_fd, mode->hdisplay, mode->vdisplay);
> > + setup_lpsp_output(data);
> > +
> > + if (use_panel_fitter && IS_HASWELL(data->devid))
> > + igt_assert(!lpsp_is_enabled(data));
> > + else
> > + check_output_lpsp(data);
> >
> > - igt_assert(buffer_id);
> > - igt_assert(connector);
> > - igt_assert(mode);
> > + cleanup_lpsp_output(data);
> > + valid_output++;
> > + }
> >
> > - rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0,
> > - &connector->connector_id, 1, mode);
> > - igt_assert_eq(rc, 0);
> > + igt_require_f(valid_output, "No edp connector found\n");
> > +}
> >
> > - if (use_panel_fitter) {
> > - if (IS_HASWELL(devid))
> > - igt_assert(!lpsp_is_enabled(drm_fd));
> > - else
> > - igt_assert(lpsp_is_enabled(drm_fd));
> > - } else {
> > - igt_assert(lpsp_is_enabled(drm_fd));
> > +static bool unload_snd_hda_core_module(void)
> > +{
> > + int i;
> > +
> > + /* unbind snd_hda_intel */
> > + kick_snd_hda_intel();
> > +
> > + for (i = 0; snd_module[i]; i++) {
> > + if (igt_kmod_is_loaded(snd_module[i]) &&
> > + igt_kmod_unload(snd_module[i], KMOD_REMOVE_FORCE)) {
> > + igt_warn("Could not unload %s\n", snd_module[i]);
> > + igt_kmod_list_loaded();
> > + igt_lsof("/dev/snd");
> > + return false;
> > + }
> > }
> > +
> > + return true;
> > }
> >
> > -static void non_edp_subtest(int drm_fd, drmModeResPtr drm_res,
> > - drmModeConnectorPtr *drm_connectors)
> > +static void load_snd_hda_core_module(void)
> > {
> > - int i, rc;
> > - uint32_t crtc_id = 0, buffer_id = 0;
> > - drmModeConnectorPtr connector = NULL;
> > - drmModeModeInfoPtr mode = NULL;
> > + igt_kmod_load("snd_hda_core", NULL);
> > +}
> > +
> > +static void non_edp_subtest(data_t *data)
> > +{
> > + igt_display_t *display = &data->display;
> > + igt_output_t *output;
> > + int valid_output;
> >
> > - kmstest_unset_all_crtcs(drm_fd, drm_res);
> > + /* DP/HDMI panel requires to drive lpsp without audio */
> > + igt_require(unload_snd_hda_core_module());
>
> I'm not super happy with having to unload modules that it would be very
> easy to forget to re-add and would make audio tests fail :s
>
> IMO, if the DUT is not playing sound, we should be able to enter LPSP.
> If no, then I would consider this a driver bug for wasting energy
> sending 0s to the screen.
i915 contorls AUDIO power domain on request of i915_audio_component_ops
get_power/put_power, which is an external dependency.
Despite unloading the module, it fails to enter lpsp for HDMI panels,
audio driver didn't invoke the put_power to release the i915 power resources.
It seems bug with audio driver interface.
My plan was to raise a gitlab bug based upon lpsp failure due to
AUDIO_POWER_DOMAIN non-zero ref count despite there was no audio
module.
>
> Thoughts on this?
I agree with you, if there is no audio is being played from DP/hdmi
we should be able to enter lpsp, but it seems audio codec requires
power at the time of codec probe itself.
@Kai Vehmanen may be the best one to confirm hdmi-codec behavior?
IMO this should not block our igt validation,
therefore i think in order to validate i915 lpsp use cases
it would be a better approach to unload the snd module.
These igt are pending since time of haswell/broadwell.
Please suggest how to proceed this?
Thanks,
Anshuman Gupta.
>
> Martin
>
> >
> > - for (i = 0; i < drm_res->count_connectors; i++) {
> > - drmModeConnectorPtr c = drm_connectors[i];
> > + for_each_connected_output(display, output) {
> > + drmModeConnectorPtr c = output->config.connector;
> >
> > if (c->connector_type == DRM_MODE_CONNECTOR_eDP)
> > continue;
> > if (c->connection != DRM_MODE_CONNECTED)
> > continue;
> > + if (!igt_pipe_connector_valid(PIPE_A, output))
> > + continue;
> >
> > - if (c->count_modes) {
> > - connector = c;
> > - mode = &c->modes[0];
> > - break;
> > - }
> > + data->output = output;
> > + data->mode = igt_output_get_mode(output);
> > + setup_lpsp_output(data);
> > + check_output_lpsp(data);
> > + cleanup_lpsp_output(data);
> > + load_snd_hda_core_module();
> > + valid_output++;
> > }
> > - igt_require(connector);
> > -
> > - crtc_id = kmstest_find_crtc_for_connector(drm_fd, drm_res, connector,
> > - 0);
> > - buffer_id = create_fb(drm_fd, mode->hdisplay, mode->vdisplay);
> >
> > - igt_assert(buffer_id);
> > - igt_assert(mode);
> > -
> > - rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0,
> > - &connector->connector_id, 1, mode);
> > - igt_assert_eq(rc, 0);
> > -
> > - igt_assert(!lpsp_is_enabled(drm_fd));
> > + igt_require_f(valid_output, "No non-edp connector found\n");
> > }
> >
> > -#define MAX_CONNECTORS 32
> > -
> > -int drm_fd;
> > -uint32_t devid;
> > -drmModeResPtr drm_res;
> > -drmModeConnectorPtr drm_connectors[MAX_CONNECTORS];
> > -struct intel_mmio_data mmio_data;
> > +IGT_TEST_DESCRIPTION("These tests validates display Low Power Single Pipe configurations");
> > igt_main
> > {
> > - igt_fixture {
> > - int i;
> > -
> > - drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > - igt_require(drm_fd >= 0);
> > -
> > - devid = intel_get_drm_devid(drm_fd);
> > + data_t data = {};
> >
> > - drm_res = drmModeGetResources(drm_fd);
> > - igt_require(drm_res);
> > - igt_assert(drm_res->count_connectors <= MAX_CONNECTORS);
> > -
> > - for (i = 0; i < drm_res->count_connectors; i++)
> > - drm_connectors[i] = drmModeGetConnectorCurrent(drm_fd,
> > - drm_res->connectors[i]);
> > + igt_fixture {
> >
> > + data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > + igt_require(data.drm_fd >= 0);
> > + data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
> > + igt_require(data.debugfs_fd >= 0);
> > igt_pm_enable_audio_runtime_pm();
> > -
> > - igt_require(supports_lpsp(devid));
> > -
> > - intel_register_access_init(&mmio_data, intel_get_pci_device(), 0, drm_fd);
> > -
> > kmstest_set_vt_graphics_mode();
> > + data.devid = intel_get_drm_devid(data.drm_fd);
> > + igt_display_require(&data.display, data.drm_fd);
> > + igt_require(igt_pm_dmc_loaded(data.debugfs_fd));
> > }
> >
> > + igt_describe("This test validates lpsp while all crtc are disabled");
> > igt_subtest("screens-disabled")
> > - screens_disabled_subtest(drm_fd, drm_res);
> > + screens_disabled_subtest(&data);
> > + igt_describe("This test validates lpsp on eDP panel");
> > igt_subtest("edp-native")
> > - edp_subtest(drm_fd, drm_res, drm_connectors, devid, false);
> > + edp_subtest(&data, false);
> > + igt_fixture
> > + cleanup_lpsp_output(&data);
> > + igt_describe("This test validates lpsp on eDP panel while forcing panel_fitter");
> > igt_subtest("edp-panel-fitter")
> > - edp_subtest(drm_fd, drm_res, drm_connectors, devid, true);
> > + edp_subtest(&data, true);
> > + igt_fixture
> > + cleanup_lpsp_output(&data);
> > + igt_describe("This test validates lpsp on DP/HDMI/DSI panels");
> > igt_subtest("non-edp")
> > - non_edp_subtest(drm_fd, drm_res, drm_connectors);
> > + non_edp_subtest(&data);
> > + igt_fixture
> > + cleanup_lpsp_output(&data);
> >
> > igt_fixture {
> > - int i;
> > -
> > - intel_register_access_fini(&mmio_data);
> > - for (i = 0; i < drm_res->count_connectors; i++)
> > - drmModeFreeConnector(drm_connectors[i]);
> > - drmModeFreeResources(drm_res);
> > - close(drm_fd);
> > + free(data.pwr_dmn_info);
> > + load_snd_hda_core_module();
> > + close(data.drm_fd);
> > + igt_display_fini(&data.display);
> > }
> > }
> >
>
> pub 2048R/33A53379 2019-12-02 Martin Peres <martin.peres@linux.intel.com>
> sub 2048R/C5E7DE5F 2019-12-02 [expires: 2020-12-01]
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support
2020-03-23 7:46 ` Anshuman Gupta
@ 2020-03-23 8:04 ` Peres, Martin
2020-03-23 12:48 ` Kai Vehmanen
1 sibling, 0 replies; 18+ messages in thread
From: Peres, Martin @ 2020-03-23 8:04 UTC (permalink / raw)
To: Gupta, Anshuman, Kai Vehmanen; +Cc: igt-dev@lists.freedesktop.org
[-- Attachment #1: Type: text/plain, Size: 16503 bytes --]
On 2020-03-23 09:56, Gupta, Anshuman wrote:
> On 2020-03-23 at 12:30:08 +0530, Peres, Martin wrote:
>> On 2020-03-23 08:32, Anshuman Gupta wrote:
>>> Current implementation of lpsp igt test, assumed that every non-edp
>>> panel isn't a lpsp panel but it is not true on TGL anymore,
>>> any HDMI/DP/DSI panel connected on pipe A and connected to PORT_{A,B,C}
>>> can drive LPSP.
>>> Even on older Gen9 platform a DP panel can drive lpsp on Port A.
>>> This requires complete design change in current lpsp igt for a platform
>>> agnostic support.
>>>
>>> The new igt approach is relies on connector specific debugfs
>>> attribute i915_lpsp_info, which exposes whether an output is capable
>>> of driving lpsp and whether lpsp is enabled.
>>>
>>> v2:
>>> - CI failures fixup.
>>>
>>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>>> ---
>>> tests/i915/i915_pm_lpsp.c | 303 +++++++++++++++++++++++---------------
>>> 1 file changed, 186 insertions(+), 117 deletions(-)
>>>
>>> diff --git a/tests/i915/i915_pm_lpsp.c b/tests/i915/i915_pm_lpsp.c
>>> index 42938e10..e400c92e 100644
>>> --- a/tests/i915/i915_pm_lpsp.c
>>> +++ b/tests/i915/i915_pm_lpsp.c
>>> @@ -25,49 +25,125 @@
>>> */
>>>
>>> #include "igt.h"
>>> +#include "igt_kmod.h"
>>> +#include "igt_pm.h"
>>> +#include "igt_sysfs.h"
>>> #include <sys/types.h>
>>> #include <sys/stat.h>
>>> #include <fcntl.h>
>>> #include <unistd.h>
>>>
>>> +#define MAX_SINK_LPSP_INFO_BUF_LEN 5000
>>>
>>> -static bool supports_lpsp(uint32_t devid)
>>> +#define PWR_DOMAIN_INFO "i915_power_domain_info"
>>> +
>>> +const char *snd_module[10] = {"snd_hda_codec_hdmi", "snd_hda_intel", "snd_hda_codec", "snd_hda_core", NULL};
>>> +
>>> +typedef struct {
>>> + int drm_fd;
>>> + int debugfs_fd;
>>> + uint32_t devid;
>>> + char *pwr_dmn_info;
>>> + igt_display_t display;
>>> + struct igt_fb fb;
>>> + drmModeModeInfo *mode;
>>> + igt_output_t *output;
>>> +} data_t;
>>> +
>>> +static void debugfs_read(int fd, const char *param, char *buf, int len)
>>> {
>>> - return IS_HASWELL(devid) || IS_BROADWELL(devid);
>>> + len = igt_debugfs_simple_read(fd, param, buf, len);
>>> + if (len < 0)
>>> + igt_assert_eq(len, -ENODEV);
>>> }
>>>
>>> -static bool lpsp_is_enabled(int drm_fd)
>>> +static bool lpsp_is_enabled(data_t *data)
>>> {
>>> - uint32_t val;
>>> + char buf[MAX_SINK_LPSP_INFO_BUF_LEN];
>>> + int fd;
>>> +
>>> + fd = igt_debugfs_connector_dir(data->drm_fd, data->output->name,
>>> + O_RDONLY);
>>> + igt_require(fd >= 0);
>>> +
>>> + debugfs_read(fd, "i915_lpsp_info", buf, sizeof(buf));
>>> + close(fd);
>>>
>>> - val = INREG(HSW_PWR_WELL_CTL2);
>>> - return !(val & HSW_PWR_WELL_STATE_ENABLED);
>>> + return strstr(buf, "LPSP enabled");
>>> }
>>>
>>> -/* The LPSP mode is all about an enabled pipe, but we expect to also be in the
>>> - * low power mode when no pipes are enabled, so do this check anyway. */
>>> -static void screens_disabled_subtest(int drm_fd, drmModeResPtr drm_res)
>>> +/*
>>> + * The LPSP mode is all about an enabled pipe, but we expect to also be in the
>>> + * low power mode when no pipes are enabled, so do this check anyway.
>>> + */
>>> +static void screens_disabled_subtest(data_t *data)
>>> {
>>> - kmstest_unset_all_crtcs(drm_fd, drm_res);
>>> - igt_assert(lpsp_is_enabled(drm_fd));
>>> + igt_output_t *output;
>>> + enum pipe pipe;
>>> + igt_plane_t *primary;
>>> +
>>> + for_each_pipe_with_single_output(&data->display, pipe, output) {
>>> + data->output = output;
>>> + igt_output_set_pipe(data->output, pipe);
>>> + primary = igt_output_get_plane_type(data->output,
>>> + DRM_PLANE_TYPE_PRIMARY);
>>> + igt_plane_set_fb(primary, NULL);
>>> + igt_display_commit(&data->display);
>>> + }
>>> +
>>> + igt_assert(lpsp_is_enabled(data));
>>> }
>>>
>>> -static uint32_t create_fb(int drm_fd, int width, int height)
>>> +static void check_output_lpsp(data_t *data)
>>> {
>>> - struct igt_fb fb;
>>> + if (igt_output_is_lpsp_capable(data->drm_fd, data->output))
>>> + igt_assert_f(lpsp_is_enabled(data), "%s: lpsp is not enabled\n%s:\n%s\n",
>>> + data->output->name, PWR_DOMAIN_INFO, data->pwr_dmn_info =
>>> + igt_sysfs_get(data->debugfs_fd, PWR_DOMAIN_INFO));
>>> + else
>>> + igt_assert(!lpsp_is_enabled(data));
>>> +}
>>>
>>> - return igt_create_pattern_fb(drm_fd, width, height, DRM_FORMAT_XRGB8888,
>>> - LOCAL_DRM_FORMAT_MOD_NONE, &fb);
>>> +static void setup_lpsp_output(data_t *data)
>>> +{
>>> + igt_plane_t *primary;
>>> +
>>> + /* set output pipe = PIPE_A for LPSP */
>>> + igt_output_set_pipe(data->output, PIPE_A);
>>> + primary = igt_output_get_plane_type(data->output,
>>> + DRM_PLANE_TYPE_PRIMARY);
>>> + igt_plane_set_fb(primary, NULL);
>>> + igt_create_pattern_fb(data->drm_fd,
>>> + data->mode->hdisplay, data->mode->vdisplay,
>>> + DRM_FORMAT_XRGB8888,
>>> + LOCAL_DRM_FORMAT_MOD_NONE,
>>> + &data->fb);
>>> + igt_plane_set_fb(primary, &data->fb);
>>> + igt_display_commit(&data->display);
>>> +}
>>> +
>>> +static void cleanup_lpsp_output(data_t *data)
>>> +{
>>> + igt_plane_t *primary;
>>> +
>>> + if (!data->output)
>>> + return;
>>> +
>>> + primary = igt_output_get_plane_type(data->output,
>>> + DRM_PLANE_TYPE_PRIMARY);
>>> + igt_plane_set_fb(primary, NULL);
>>> + igt_output_set_pipe(data->output, PIPE_NONE);
>>> + igt_display_commit(&data->display);
>>> + igt_remove_fb(data->drm_fd, &data->fb);
>>> + data->output = NULL;
>>> }
>>>
>>> -static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
>>> - drmModeConnectorPtr *drm_connectors, uint32_t devid,
>>> - bool use_panel_fitter)
>>> +static void edp_subtest(data_t *data, bool use_panel_fitter)
>>> {
>>> - int i, rc;
>>> - uint32_t crtc_id = 0, buffer_id = 0;
>>> - drmModeConnectorPtr connector = NULL;
>>> - drmModeModeInfoPtr mode = NULL;
>>> + igt_display_t *display = &data->display;
>>> + igt_output_t *output;
>>> + int valid_output;
>>> +
>>> drmModeModeInfo std_1024_mode = {
>>> .clock = 65000,
>>> .hdisplay = 1024,
>>> @@ -86,23 +162,17 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
>>> .name = "Custom 1024x768",
>>> };
>>>
>>> - kmstest_unset_all_crtcs(drm_fd, drm_res);
>>> -
>>> - for (i = 0; i < drm_res->count_connectors; i++) {
>>> - drmModeConnectorPtr c = drm_connectors[i];
>>> + for_each_connected_output(display, output) {
>>> + drmModeConnectorPtr c = output->config.connector;
>>>
>>> if (c->connector_type != DRM_MODE_CONNECTOR_eDP)
>>> continue;
>>> - if (c->connection != DRM_MODE_CONNECTED)
>>> + if (!igt_pipe_connector_valid(PIPE_A, output))
>>> continue;
>>>
>>> - if (!use_panel_fitter && c->count_modes) {
>>> - connector = c;
>>> - mode = &c->modes[0];
>>> - break;
>>> - }
>>> + data->output = output;
>>> +
>>> if (use_panel_fitter) {
>>> - connector = c;
>>>
>>> /* This is one of the modes Xorg creates for panels, so
>>> * it should work just fine. Notice that Gens that
>>> @@ -113,124 +183,123 @@ static void edp_subtest(int drm_fd, drmModeResPtr drm_res,
>>> c->modes[0].hdisplay > 1024);
>>> igt_assert(c->count_modes &&
>>> c->modes[0].vdisplay > 768);
>>> - mode = &std_1024_mode;
>>> - break;
>>> + data->mode = &std_1024_mode;
>>> + igt_output_override_mode(output, data->mode);
>>> + } else {
>>> + data->mode = igt_output_get_mode(output);
>>> }
>>> - }
>>> - igt_require(connector);
>>>
>>> - crtc_id = kmstest_find_crtc_for_connector(drm_fd, drm_res, connector,
>>> - 0);
>>> - buffer_id = create_fb(drm_fd, mode->hdisplay, mode->vdisplay);
>>> + setup_lpsp_output(data);
>>> +
>>> + if (use_panel_fitter && IS_HASWELL(data->devid))
>>> + igt_assert(!lpsp_is_enabled(data));
>>> + else
>>> + check_output_lpsp(data);
>>>
>>> - igt_assert(buffer_id);
>>> - igt_assert(connector);
>>> - igt_assert(mode);
>>> + cleanup_lpsp_output(data);
>>> + valid_output++;
>>> + }
>>>
>>> - rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0,
>>> - &connector->connector_id, 1, mode);
>>> - igt_assert_eq(rc, 0);
>>> + igt_require_f(valid_output, "No edp connector found\n");
>>> +}
>>>
>>> - if (use_panel_fitter) {
>>> - if (IS_HASWELL(devid))
>>> - igt_assert(!lpsp_is_enabled(drm_fd));
>>> - else
>>> - igt_assert(lpsp_is_enabled(drm_fd));
>>> - } else {
>>> - igt_assert(lpsp_is_enabled(drm_fd));
>>> +static bool unload_snd_hda_core_module(void)
>>> +{
>>> + int i;
>>> +
>>> + /* unbind snd_hda_intel */
>>> + kick_snd_hda_intel();
>>> +
>>> + for (i = 0; snd_module[i]; i++) {
>>> + if (igt_kmod_is_loaded(snd_module[i]) &&
>>> + igt_kmod_unload(snd_module[i], KMOD_REMOVE_FORCE)) {
>>> + igt_warn("Could not unload %s\n", snd_module[i]);
>>> + igt_kmod_list_loaded();
>>> + igt_lsof("/dev/snd");
>>> + return false;
>>> + }
>>> }
>>> +
>>> + return true;
>>> }
>>>
>>> -static void non_edp_subtest(int drm_fd, drmModeResPtr drm_res,
>>> - drmModeConnectorPtr *drm_connectors)
>>> +static void load_snd_hda_core_module(void)
>>> {
>>> - int i, rc;
>>> - uint32_t crtc_id = 0, buffer_id = 0;
>>> - drmModeConnectorPtr connector = NULL;
>>> - drmModeModeInfoPtr mode = NULL;
>>> + igt_kmod_load("snd_hda_core", NULL);
>>> +}
>>> +
>>> +static void non_edp_subtest(data_t *data)
>>> +{
>>> + igt_display_t *display = &data->display;
>>> + igt_output_t *output;
>>> + int valid_output;
>>>
>>> - kmstest_unset_all_crtcs(drm_fd, drm_res);
>>> + /* DP/HDMI panel requires to drive lpsp without audio */
>>> + igt_require(unload_snd_hda_core_module());
>>
>> I'm not super happy with having to unload modules that it would be very
>> easy to forget to re-add and would make audio tests fail :s
>>
>> IMO, if the DUT is not playing sound, we should be able to enter LPSP.
>> If no, then I would consider this a driver bug for wasting energy
>> sending 0s to the screen.
> i915 contorls AUDIO power domain on request of i915_audio_component_ops
> get_power/put_power, which is an external dependency.
> Despite unloading the module, it fails to enter lpsp for HDMI panels,
> audio driver didn't invoke the put_power to release the i915 power resources.
> It seems bug with audio driver interface.
> My plan was to raise a gitlab bug based upon lpsp failure due to
> AUDIO_POWER_DOMAIN non-zero ref count despite there was no audio
> module.
Yet another reason not to remove the modules then ;)
>>
>> Thoughts on this?
> I agree with you, if there is no audio is being played from DP/hdmi
> we should be able to enter lpsp, but it seems audio codec requires
> power at the time of codec probe itself.
> @Kai Vehmanen may be the best one to confirm hdmi-codec behavior?
>
> IMO this should not block our igt validation,
> therefore i think in order to validate i915 lpsp use cases
> it would be a better approach to unload the snd module.
> These igt are pending since time of haswell/broadwell.
> Please suggest how to proceed this?
I agree that this series is a step forward, and I want to see this
series merged sooner rather than later. What I don't want to see is
painting the grass green by working around actual bugs. I suggest you
stop unloading the modules and let the tests fail until the kernel gets
fixed. It should be easy to do and we can merge the series.
As for validation, painting the grass green by working around real
issues is not acceptable. You can say in the bug we will file that the
cause is related to the audio codec, but working around it in the test
should be a no-no. We want normal users to be able to reach LPSP, not
just the ones who force-disable audio at boot. So, our IGT tests should
reflect the normal case :)
Martin
> Thanks,
> Anshuman Gupta.
>>
>> Martin
>>
>>>
>>> - for (i = 0; i < drm_res->count_connectors; i++) {
>>> - drmModeConnectorPtr c = drm_connectors[i];
>>> + for_each_connected_output(display, output) {
>>> + drmModeConnectorPtr c = output->config.connector;
>>>
>>> if (c->connector_type == DRM_MODE_CONNECTOR_eDP)
>>> continue;
>>> if (c->connection != DRM_MODE_CONNECTED)
>>> continue;
>>> + if (!igt_pipe_connector_valid(PIPE_A, output))
>>> + continue;
>>>
>>> - if (c->count_modes) {
>>> - connector = c;
>>> - mode = &c->modes[0];
>>> - break;
>>> - }
>>> + data->output = output;
>>> + data->mode = igt_output_get_mode(output);
>>> + setup_lpsp_output(data);
>>> + check_output_lpsp(data);
>>> + cleanup_lpsp_output(data);
>>> + load_snd_hda_core_module();
>>> + valid_output++;
>>> }
>>> - igt_require(connector);
>>> -
>>> - crtc_id = kmstest_find_crtc_for_connector(drm_fd, drm_res, connector,
>>> - 0);
>>> - buffer_id = create_fb(drm_fd, mode->hdisplay, mode->vdisplay);
>>>
>>> - igt_assert(buffer_id);
>>> - igt_assert(mode);
>>> -
>>> - rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0,
>>> - &connector->connector_id, 1, mode);
>>> - igt_assert_eq(rc, 0);
>>> -
>>> - igt_assert(!lpsp_is_enabled(drm_fd));
>>> + igt_require_f(valid_output, "No non-edp connector found\n");
>>> }
>>>
>>> -#define MAX_CONNECTORS 32
>>> -
>>> -int drm_fd;
>>> -uint32_t devid;
>>> -drmModeResPtr drm_res;
>>> -drmModeConnectorPtr drm_connectors[MAX_CONNECTORS];
>>> -struct intel_mmio_data mmio_data;
>>> +IGT_TEST_DESCRIPTION("These tests validates display Low Power Single Pipe configurations");
>>> igt_main
>>> {
>>> - igt_fixture {
>>> - int i;
>>> -
>>> - drm_fd = drm_open_driver_master(DRIVER_INTEL);
>>> - igt_require(drm_fd >= 0);
>>> -
>>> - devid = intel_get_drm_devid(drm_fd);
>>> + data_t data = {};
>>>
>>> - drm_res = drmModeGetResources(drm_fd);
>>> - igt_require(drm_res);
>>> - igt_assert(drm_res->count_connectors <= MAX_CONNECTORS);
>>> -
>>> - for (i = 0; i < drm_res->count_connectors; i++)
>>> - drm_connectors[i] = drmModeGetConnectorCurrent(drm_fd,
>>> - drm_res->connectors[i]);
>>> + igt_fixture {
>>>
>>> + data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
>>> + igt_require(data.drm_fd >= 0);
>>> + data.debugfs_fd = igt_debugfs_dir(data.drm_fd);
>>> + igt_require(data.debugfs_fd >= 0);
>>> igt_pm_enable_audio_runtime_pm();
>>> -
>>> - igt_require(supports_lpsp(devid));
>>> -
>>> - intel_register_access_init(&mmio_data, intel_get_pci_device(), 0, drm_fd);
>>> -
>>> kmstest_set_vt_graphics_mode();
>>> + data.devid = intel_get_drm_devid(data.drm_fd);
>>> + igt_display_require(&data.display, data.drm_fd);
>>> + igt_require(igt_pm_dmc_loaded(data.debugfs_fd));
>>> }
>>>
>>> + igt_describe("This test validates lpsp while all crtc are disabled");
>>> igt_subtest("screens-disabled")
>>> - screens_disabled_subtest(drm_fd, drm_res);
>>> + screens_disabled_subtest(&data);
>>> + igt_describe("This test validates lpsp on eDP panel");
>>> igt_subtest("edp-native")
>>> - edp_subtest(drm_fd, drm_res, drm_connectors, devid, false);
>>> + edp_subtest(&data, false);
>>> + igt_fixture
>>> + cleanup_lpsp_output(&data);
>>> + igt_describe("This test validates lpsp on eDP panel while forcing panel_fitter");
>>> igt_subtest("edp-panel-fitter")
>>> - edp_subtest(drm_fd, drm_res, drm_connectors, devid, true);
>>> + edp_subtest(&data, true);
>>> + igt_fixture
>>> + cleanup_lpsp_output(&data);
>>> + igt_describe("This test validates lpsp on DP/HDMI/DSI panels");
>>> igt_subtest("non-edp")
>>> - non_edp_subtest(drm_fd, drm_res, drm_connectors);
>>> + non_edp_subtest(&data);
>>> + igt_fixture
>>> + cleanup_lpsp_output(&data);
>>>
>>> igt_fixture {
>>> - int i;
>>> -
>>> - intel_register_access_fini(&mmio_data);
>>> - for (i = 0; i < drm_res->count_connectors; i++)
>>> - drmModeFreeConnector(drm_connectors[i]);
>>> - drmModeFreeResources(drm_res);
>>> - close(drm_fd);
>>> + free(data.pwr_dmn_info);
>>> + load_snd_hda_core_module();
>>> + close(data.drm_fd);
>>> + igt_display_fini(&data.display);
>>> }
>>> }
>>>
>>
>
>> pub 2048R/33A53379 2019-12-02 Martin Peres <martin.peres@linux.intel.com>
>> sub 2048R/C5E7DE5F 2019-12-02 [expires: 2020-12-01]
>
>
[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1765 bytes --]
[-- Attachment #3: Type: text/plain, Size: 154 bytes --]
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 4/5] tests/i915_pm_lpsp: screens-disabled subtest use igt_wait
2020-03-23 7:05 ` Peres, Martin
@ 2020-03-23 9:37 ` Anshuman Gupta
0 siblings, 0 replies; 18+ messages in thread
From: Anshuman Gupta @ 2020-03-23 9:37 UTC (permalink / raw)
To: Peres, Martin; +Cc: igt-dev@lists.freedesktop.org
On 2020-03-23 at 12:35:29 +0530, Peres, Martin wrote:
> On 2020-03-23 08:32, Anshuman Gupta wrote:
> > Some times delayed audio codec disabling causes failure
> > of test, using igt_wait to check lpsp after disabling all outputs.
>
> Seems like this answers my question on patch 2/5: We are sending audio
> to the screen even when we do not have anything to send...
>
> I know there are some screens that take ages to re-enable sound after
> not getting any for a while, but waiting 10 or even 30s without sound
> before disabling it does not sound like a terrible idea. My TV
> automatically disables sound after way less than that anyway.
>
> So, to me, this is a driver bug.
>
> That being said, the patch is fine even if your reason for it is not
> acceptable.
Thanks Martin for comment, i will drop this patch, remove the
unloading of sound moudles from 2/5 patch, and will send an update.
Thanks,
Anshuman Gupta,
>
> Martin
> >
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> > tests/i915/i915_pm_lpsp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/i915/i915_pm_lpsp.c b/tests/i915/i915_pm_lpsp.c
> > index 0ce694e6..5f15c034 100644
> > --- a/tests/i915/i915_pm_lpsp.c
> > +++ b/tests/i915/i915_pm_lpsp.c
> > @@ -91,7 +91,7 @@ static void screens_disabled_subtest(data_t *data)
> > igt_display_commit(&data->display);
> > }
> >
> > - igt_assert(lpsp_is_enabled(data));
> > + igt_assert(igt_wait(lpsp_is_enabled(data), 1000, 100));
> > }
> >
> > static void check_output_lpsp(data_t *data)
> >
>
> pub 2048R/33A53379 2019-12-02 Martin Peres <martin.peres@linux.intel.com>
> sub 2048R/C5E7DE5F 2019-12-02 [expires: 2020-12-01]
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support
2020-03-23 7:46 ` Anshuman Gupta
2020-03-23 8:04 ` Peres, Martin
@ 2020-03-23 12:48 ` Kai Vehmanen
2020-03-23 15:33 ` Anshuman Gupta
1 sibling, 1 reply; 18+ messages in thread
From: Kai Vehmanen @ 2020-03-23 12:48 UTC (permalink / raw)
To: Anshuman Gupta; +Cc: igt-dev@lists.freedesktop.org, Peres, Martin, Kai Vehmanen
Hey,
On Mon, 23 Mar 2020, Anshuman Gupta wrote:
> On 2020-03-23 at 12:30:08 +0530, Peres, Martin wrote:
> > On 2020-03-23 08:32, Anshuman Gupta wrote:
> > > + /* DP/HDMI panel requires to drive lpsp without audio */
> > > + igt_require(unload_snd_hda_core_module());
> >
> > I'm not super happy with having to unload modules that it would be very
> > easy to forget to re-add and would make audio tests fail :s
at least this may be painful to maintain. We have three audio drivers in
active use (HDA snd-hda-intel, SST DSP and SOF DSP -> drivers is selected
at runtime dependsing on hw generation, enabled features and the available
DSP FW), and the modules you need to unload are different for all three,
and may change over time. So not so nice to maintain if you need to
support a large variety of hardware targets.
> > IMO, if the DUT is not playing sound, we should be able to enter LPSP.
> > If no, then I would consider this a driver bug for wasting energy
> > sending 0s to the screen.
> i915 contorls AUDIO power domain on request of i915_audio_component_ops
> get_power/put_power, which is an external dependency.
> Despite unloading the module, it fails to enter lpsp for HDMI panels,
> audio driver didn't invoke the put_power to release the i915 power resources.
> It seems bug with audio driver interface.
Yes, this sounds like a plain bug. Audio should never keep powers
on unless it needs it for some actual use-case.
> My plan was to raise a gitlab bug based upon lpsp failure due to
> AUDIO_POWER_DOMAIN non-zero ref count despite there was no audio
> module.
> >
> > Thoughts on this?
> I agree with you, if there is no audio is being played from DP/hdmi
> we should be able to enter lpsp, but it seems audio codec requires
> power at the time of codec probe itself.
> @Kai Vehmanen may be the best one to confirm hdmi-codec behavior?
Yes, audio codec asks for i915 power when it is probed, and also whenever
it uses the HDA bus. But when it is idle, it will release i915 power via
put_power. One possible problem could be that the audio driver is uilt
without runtime pm support. Then it will indeed keep a ref to i915
power all the time. Can you provide a bit of data about the
case (which hardware, kernel config used, possible dmesg log of error)?
Br, Kai (Sound Open Firmware (SOF) team)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support
2020-03-23 12:48 ` Kai Vehmanen
@ 2020-03-23 15:33 ` Anshuman Gupta
0 siblings, 0 replies; 18+ messages in thread
From: Anshuman Gupta @ 2020-03-23 15:33 UTC (permalink / raw)
To: Kai Vehmanen; +Cc: igt-dev@lists.freedesktop.org, Peres, Martin
On 2020-03-23 at 14:48:03 +0200, Kai Vehmanen wrote:
> Hey,
>
> On Mon, 23 Mar 2020, Anshuman Gupta wrote:
>
> > On 2020-03-23 at 12:30:08 +0530, Peres, Martin wrote:
> > > On 2020-03-23 08:32, Anshuman Gupta wrote:
> > > > + /* DP/HDMI panel requires to drive lpsp without audio */
> > > > + igt_require(unload_snd_hda_core_module());
> > >
> > > I'm not super happy with having to unload modules that it would be very
> > > easy to forget to re-add and would make audio tests fail :s
>
> at least this may be painful to maintain. We have three audio drivers in
> active use (HDA snd-hda-intel, SST DSP and SOF DSP -> drivers is selected
> at runtime dependsing on hw generation, enabled features and the available
> DSP FW), and the modules you need to unload are different for all three,
> and may change over time. So not so nice to maintain if you need to
> support a large variety of hardware targets.
>
> > > IMO, if the DUT is not playing sound, we should be able to enter LPSP.
> > > If no, then I would consider this a driver bug for wasting energy
> > > sending 0s to the screen.
> > i915 contorls AUDIO power domain on request of i915_audio_component_ops
> > get_power/put_power, which is an external dependency.
> > Despite unloading the module, it fails to enter lpsp for HDMI panels,
> > audio driver didn't invoke the put_power to release the i915 power resources.
> > It seems bug with audio driver interface.
>
> Yes, this sounds like a plain bug. Audio should never keep powers
> on unless it needs it for some actual use-case.
Thanks kai for your comment based upon your comment, i investigated
it and found, while enabling crtc i915 also gets ref count
for AUDIO_POWER_DOMAIN if there is valid ELD.
This was failing the test.
Thanks,
Anshuman Gupta.
>
> > My plan was to raise a gitlab bug based upon lpsp failure due to
> > AUDIO_POWER_DOMAIN non-zero ref count despite there was no audio
> > module.
> > >
> > > Thoughts on this?
> > I agree with you, if there is no audio is being played from DP/hdmi
> > we should be able to enter lpsp, but it seems audio codec requires
> > power at the time of codec probe itself.
> > @Kai Vehmanen may be the best one to confirm hdmi-codec behavior?
>
> Yes, audio codec asks for i915 power when it is probed, and also whenever
> it uses the HDA bus. But when it is idle, it will release i915 power via
> put_power. One possible problem could be that the audio driver is uilt
> without runtime pm support. Then it will indeed keep a ref to i915
> power all the time. Can you provide a bit of data about the
> case (which hardware, kernel config used, possible dmesg log of error)?
>
> Br, Kai (Sound Open Firmware (SOF) team)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t v2 1/5] lib/igt_pm: Add lib func to get lpsp capability
2020-03-23 6:55 ` Peres, Martin
@ 2020-03-24 6:05 ` Anshuman Gupta
0 siblings, 0 replies; 18+ messages in thread
From: Anshuman Gupta @ 2020-03-24 6:05 UTC (permalink / raw)
To: Peres, Martin; +Cc: igt-dev@lists.freedesktop.org
On 2020-03-23 at 12:25:25 +0530, Peres, Martin wrote:
> On 2020-03-23 08:32, Anshuman Gupta wrote:
> > This lib function evaluate the lpsp capability from
> > the connector specific debugfs attribute i915_lpsp_info.
> >
> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> > ---
> > lib/igt_pm.c | 27 +++++++++++++++++++++++++++
> > lib/igt_pm.h | 1 +
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> > index 9d441e1b..7a6cab7c 100644
> > --- a/lib/igt_pm.c
> > +++ b/lib/igt_pm.c
> > @@ -37,6 +37,7 @@
> > #include <dirent.h>
> >
> > #include "drmtest.h"
> > +#include "igt_kms.h"
> > #include "igt_pm.h"
> > #include "igt_aux.h"
> > #include "igt_sysfs.h"
> > @@ -827,3 +828,29 @@ bool igt_pm_pc8_plus_residencies_enabled(int msr_fd)
> >
> > return true;
> > }
> > +
> > +/**
> > + * igt_output_is_lpsp_capable:
> > + * @drm_fd: fd to drm device
> > + * @output: igt output for which lpsp capability need to be evaluated
> > + * Check lpsp capability for a given output.
> > + *
> > + * Returns:
> > + * True if given output is lpsp capable otherwise false.
> > + */
> > +bool igt_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
>
> Do we have a file for i915-specific features? If not, maybe prefixing
> the function with i915 would help readers understand that this is an
> i915-specific function.
I am unable to find any igt lib file for i915-features.
IMO this is a power feature, igt_pm.c suits it.
I will change the prefix to i915_ but all other
function in this file are using igt_ prefix,
i hope that will not be an issue.
Thanks,
Anshuman Gupta.
>
> > +{
> > + char buf[256];
> > + int fd, len;
> > +
> > + fd = igt_debugfs_connector_dir(drm_fd, output->name, O_RDONLY);
> > + igt_require(fd >= 0);
> > + len = igt_debugfs_simple_read(fd, "i915_lpsp_info", buf, sizeof(buf));
> > +
> > + if (len < 0)
> > + igt_assert_eq(len, -ENODEV);
> > +
> > + close(fd);
> > +
> > + return strstr(buf, "LPSP capable");
> > +}
> > diff --git a/lib/igt_pm.h b/lib/igt_pm.h
> > index 5e438452..076d8c27 100644
> > --- a/lib/igt_pm.h
> > +++ b/lib/igt_pm.h
> > @@ -53,5 +53,6 @@ enum igt_runtime_pm_status igt_get_runtime_pm_status(void);
> > bool igt_wait_for_pm_status(enum igt_runtime_pm_status status);
> > bool igt_pm_dmc_loaded(int debugfs);
> > bool igt_pm_pc8_plus_residencies_enabled(int msr_fd);
> > +bool igt_output_is_lpsp_capable(int drm_fd, igt_output_t *output);
> >
> > #endif /* IGT_PM_H */
> >
>
> pub 2048R/33A53379 2019-12-02 Martin Peres <martin.peres@linux.intel.com>
> sub 2048R/C5E7DE5F 2019-12-02 [expires: 2020-12-01]
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-03-24 6:15 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-23 6:32 [igt-dev] [PATCH i-g-t v2 0/5] lpsp platform agnostic support Anshuman Gupta
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 1/5] lib/igt_pm: Add lib func to get lpsp capability Anshuman Gupta
2020-03-23 6:55 ` Peres, Martin
2020-03-24 6:05 ` Anshuman Gupta
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 2/5] tests/i915_pm_lpsp: lpsp platform agnostic support Anshuman Gupta
2020-03-23 7:00 ` Peres, Martin
2020-03-23 7:46 ` Anshuman Gupta
2020-03-23 8:04 ` Peres, Martin
2020-03-23 12:48 ` Kai Vehmanen
2020-03-23 15:33 ` Anshuman Gupta
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 3/5] tests/i915_pm_lpsp: Skip panel-fitter subtest for 1024x768 panels Anshuman Gupta
2020-03-23 7:00 ` Peres, Martin
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 4/5] tests/i915_pm_lpsp: screens-disabled subtest use igt_wait Anshuman Gupta
2020-03-23 7:05 ` Peres, Martin
2020-03-23 9:37 ` Anshuman Gupta
2020-03-23 6:32 ` [igt-dev] [PATCH i-g-t v2 5/5] tests/i915_pm_rpm: lpsp/non-lpsp screen mode_set_data Anshuman Gupta
2020-03-23 7:10 ` Peres, Martin
2020-03-23 7:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for lpsp platform agnostic support (rev3) Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox