Hey,

On Tue, Mar 28, 2023 at 02:40:43PM +0200, Maarten Lankhorst wrote:

Display keeps PM alive, do the minimal initialization to disable
display for the PM tests entirely.

What's the reason of enforcing releasing display before starting PM
tests? Looks like introducing testing gap.

It's the easiest fix, and display is irrelevant for those tests. The system will not go into

a lower power when display keeps it in a high power state by holding a PM reference

What do you think of the below test instead?

----------->8---------

diff --git a/tests/xe/xe_pm.c b/tests/xe/xe_pm.c
index 23b8246ed..f681e253a 100644
--- a/tests/xe/xe_pm.c
+++ b/tests/xe/xe_pm.c
@@ -341,6 +341,30 @@ NULL));
 		igt_assert(in_d3(device, d_state));
 }
 
+static void test_pipe(device_t device, igt_display_t *display, enum pipe pipe,
+		      igt_output_t *output, enum igt_acpi_d_state d_state)
+{
+	struct igt_fb fb;
+
+	igt_assert(in_d3(device, d_state));
+
+	/* Just for fun, set a scaled fb as primary so scaler is active too */
+	igt_create_fb(display->drm_fd, 640, 480, DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &fb);
+
+	igt_assert(in_d3(device, d_state));
+
+	igt_display_reset(display);
+	igt_output_set_pipe(output, pipe);
+	igt_plane_set_fb(igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY), &fb);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+	igt_assert(out_of_d3(device, d_state));
+
+	igt_display_reset(display);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+	igt_remove_fb(display->drm_fd, &fb);
+	igt_assert(in_d3(device, d_state));
+}
+
 igt_main
 {
 	struct drm_xe_engine_class_instance *hwe;
@@ -363,19 +387,34 @@ igt_main
 		{ "d3cold", IGT_ACPI_D3Cold },
 		{ NULL },
 	};
+	enum pipe pipe;
+	igt_display_t display;
 
 	igt_fixture {
+		drmModeResPtr res;
+
 		memset(&device, 0, sizeof(device));
-		device.fd_xe = drm_open_driver(DRIVER_XE);
+		device.fd_xe = drm_open_driver_master(DRIVER_XE);
 		device.pci_xe = igt_device_get_pci_device(device.fd_xe);
 		device.pci_root = igt_device_get_pci_root_port(device.fd_xe);
 
+		kmstest_set_vt_graphics_mode();
 		xe_device_get(device.fd_xe);
 
 		/* Always perform initial once-basic exec checking for health */
 		xe_for_each_hw_engine(device.fd_xe, hwe)
 			test_exec(device, hwe, 1, 1, NO_SUSPEND, NO_RPM);
 
+		/* Kill display, if it exists */
+		res = drmModeGetResources(device.fd_xe);
+		if (res) {
+			drmModeFreeResources(res);
+			igt_display_require(&display, device.fd_xe);
+			igt_display_commit2(&display, COMMIT_ATOMIC);
+		} else {
+			display.drm_fd = -1;
+		}
+
 		get_d3cold_allowed(device.pci_xe, d3cold_allowed);
 		igt_assert(igt_setup_runtime_pm(device.fd_xe));
 	}
@@ -435,11 +474,31 @@ igt_main
 				test_exec(device, hwe, 16, 32,
 					  NO_SUSPEND, d->state);
 		}
+
+		for_each_pipe_static(pipe) {
+			igt_subtest_f("%s-pipe-%s", d->name, kmstest_pipe_name(pipe)) {
+				igt_output_t *output;
+				bool valid_output = false;
+
+				igt_require(display.drm_fd != -1);
+				igt_require_pipe(&display, pipe);
+				igt_assert(setup_d3(device, d->state));
+
+				for_each_valid_output_on_pipe(&display, pipe, output)
+					test_pipe(device, &display, pipe, output, d->state);
+
+				igt_require(valid_output);
+			}
+		}
 	}
 
 	igt_fixture {
 		set_d3cold_allowed(device.pci_xe, d3cold_allowed);
 		igt_restore_runtime_pm();
+
+		if (display.drm_fd != -1)
+			igt_display_fini(&display);
+
 		xe_device_put(device.fd_xe);
 		close(device.fd_xe);
 	}