Hey,

On 2023-04-03 07:37, Zbigniew Kempczyński wrote:
> 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);
  	}