Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Pranay Samala <pranay.samala@intel.com>
To: igt-dev@lists.freedesktop.org
Cc: karthik.b.s@intel.com, sameer.lattannavar@intel.com,
	pranay.samala@intel.com
Subject: [PATCH i-g-t 7/7] tests/chamelium/kms_chamelium_hpd: Add HPD from runtime suspended D3hot
Date: Mon,  7 Sep 2026 19:52:59 +0530	[thread overview]
Message-ID: <20260907142259.750528-8-pranay.samala@intel.com> (raw)
In-Reply-To: <20260907142259.750528-1-pranay.samala@intel.com>

Add dp-hpd-after-runtime-suspend-d3hot and
hdmi-hpd-after-runtime-suspend-d3hot, covering a hotplug signalled by a
PME while the device is runtime suspended in D3hot. The existing
*-hpd-after-suspend subtests use system suspend instead.

Checking only that the display comes back would pass whether or not the
driver fell back to the ~10s connector poll, so the subtest runs two
phases against the same suspended device.

Phase 1 parks the device in D3hot with PME_En set and samples the D
state from the PCI Power Management Control/Status register across a
window longer than two poll periods, requiring that it never leaves
D3hot. Any resume there means polling is still enabled.

Phase 2 toggles the HPD from the chamelium and requires a prompt uevent,
an updated connector status, and an increase in
power/wakeup_active_count. The counter is what shows the device
signalled its own resume rather than the host initiating one.

Both phases report the D state, PME_Status and the counter delta on
failure rather than asserting a cause, which separates a device that
never signalled from one whose PME was not delivered and from one that
resumed and dropped the HPD afterwards.

Assisted-by: GitHub_Copilot:claude-opus-5
Signed-off-by: Pranay Samala <pranay.samala@intel.com>
---
 tests/chamelium/kms_chamelium_hpd.c | 344 ++++++++++++++++++++++++++++
 1 file changed, 344 insertions(+)

diff --git a/tests/chamelium/kms_chamelium_hpd.c b/tests/chamelium/kms_chamelium_hpd.c
index bd8731c02..1135019c4 100644
--- a/tests/chamelium/kms_chamelium_hpd.c
+++ b/tests/chamelium/kms_chamelium_hpd.c
@@ -33,6 +33,7 @@
  */
 
 #include "kms_chamelium_helper.h"
+#include "igt_device.h"
 
 /**
  * SUBTEST: dp-hpd-fast
@@ -114,6 +115,18 @@
  * Description: Toggle HPD during Suspend, check that uevents are sent and
  *              connector status is updated
  *
+ * SUBTEST: dp-hpd-after-runtime-suspend-d3hot
+ * Description: Toggle HPD while runtime suspended in D3hot with PME signalling
+ *              available, check that connector polling stayed disabled, that the
+ *              device woke itself up and that uevents are sent and connector
+ *              status is updated
+ *
+ * SUBTEST: hdmi-hpd-after-runtime-suspend-d3hot
+ * Description: Toggle HPD while runtime suspended in D3hot with PME signalling
+ *              available, check that connector polling stayed disabled, that the
+ *              device woke itself up and that uevents are sent and connector
+ *              status is updated
+ *
  * SUBTEST: common-hpd-after-suspend
  * Description: Toggle HPD during suspend on all connectors, check that uevents
  * 		are sent and connector status is updated
@@ -152,6 +165,33 @@
 #define HPD_TOGGLE_COUNT_DP_HDMI 15
 #define HPD_TOGGLE_COUNT_FAST 3
 
+/*
+ * The KMS helper polls connectors every 10s, so watch for longer than two poll
+ * periods: a single delayed or coalesced poll then cannot make the test pass.
+ */
+#define PME_POLL_PROOF_WINDOW_MS 25000
+/*
+ * How often to sample the D state during that window. Config space reads do not
+ * resume a device suspended into D3hot, so this catches a poll induced resume
+ * that is too brief to stand out in the runtime PM accounting.
+ */
+#define PME_D_STATE_SAMPLE_MS 100
+/*
+ * If the device never leaves D3hot, all elapsed time is accounted to
+ * runtime_suspended_time and runtime_active_time does not move at all. Allow a
+ * small amount for one unrelated wakeup rather than requiring exactly zero,
+ * while staying well under what two or three poll induced resumes would cost.
+ */
+#define PME_ACTIVE_TIME_SLACK_MS 100
+/* Tolerance for accounting granularity and usleep() overshoot. */
+#define PME_POLL_PROOF_SLACK_MS 500
+/*
+ * A PME is delivered as an interrupt, so the wake is expected to be prompt. The
+ * whole point of the feature is that this is not the ~10s a poll would take.
+ */
+#define PME_WAKE_LATENCY_MAX_MS 3000
+#define PME_HPD_TOGGLE_DELAY_MS 1000
+
 enum test_modeset_mode {
 	TEST_MODESET_ON,
 	TEST_MODESET_ON_OFF,
@@ -383,6 +423,302 @@ static void test_suspend_resume_hpd_common(chamelium_data_t *data,
 	igt_cleanup_uevents(mon);
 }
 
+/*
+ * Name a D state read back from config space, for failure messages. PMCS cannot
+ * report D3cold - a device in D3cold is resumed by the read itself and answers
+ * as D0 - so an unknown state here means the read failed outright, i.e. the
+ * device has gone away.
+ */
+static const char *pme_d_state_name(enum igt_acpi_d_state state)
+{
+	switch (state) {
+	case IGT_ACPI_D0:
+		return "D0";
+	case IGT_ACPI_D1:
+		return "D1";
+	case IGT_ACPI_D2:
+		return "D2";
+	case IGT_ACPI_D3Hot:
+		return "D3hot";
+	case IGT_ACPI_D3Cold:
+		return "D3cold";
+	default:
+		return "an unreadable state";
+	}
+}
+
+static void try_hpd_runtime_suspend_pme(chamelium_data_t *data,
+					struct chamelium_port *port,
+					struct pci_device *pci_dev,
+					struct udev_monitor *mon,
+					bool connected)
+{
+	drmModeConnection target_state = connected ? DRM_MODE_DISCONNECTED :
+						     DRM_MODE_CONNECTED;
+	int timeout = CHAMELIUM_HOTPLUG_TIMEOUT;
+	uint64_t susp_before, active_before, wakeups_before, wakeups_after;
+	uint64_t wakeups_at_toggle;
+	uint64_t susp_delta, active_delta;
+	struct timespec start, end;
+	int latency;
+
+	igt_flush_uevents(mon);
+
+	igt_assert_f(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED),
+		     "Device did not runtime suspend\n");
+
+	/*
+	 * Intel graphics devices support PME in D3hot but not in D3cold, and xe
+	 * chooses the target state itself, so there is nothing to force from
+	 * userspace. In practice an integrated device always lands in D3hot,
+	 * since xe needs an ACPI _PR3 power resource on the upstream port before
+	 * it considers D3cold at all. Check anyway: on a device that did drop to
+	 * D3cold there is no PME to observe and everything below would only
+	 * exercise the polling fallback, so skip.
+	 *
+	 * A device in D3cold answers this read as D0 rather than as D3cold, for
+	 * the reason described above pme_d_state_name(), which fails the check
+	 * just the same.
+	 */
+	igt_require_f(igt_pm_pci_get_d_state(pci_dev) == IGT_ACPI_D3Hot,
+		      "Device is not suspended in D3hot, so there is no PME to "
+		      "observe\n");
+
+	/*
+	 * PME_En is the direct sign that the feature engaged; without it, the test
+	 * would only be measuring the polling fallback.
+	 */
+	igt_assert_f(igt_pm_pci_pme_enabled(pci_dev),
+		     "PME_En is not set after suspending into D3hot\n");
+
+	/*
+	 * Phase 1: ensure the device stays in D3hot while idle. If polling remains on,
+	 * the KMS helper will resume it to probe connectors.
+	 */
+	susp_before = igt_pm_get_runtime_suspended_time(pci_dev);
+	active_before = igt_pm_get_runtime_active_time(pci_dev);
+	wakeups_before = igt_pm_get_wakeup_active_count(pci_dev);
+
+	igt_assert_eq(igt_gettime(&start), 0);
+	do {
+		enum igt_acpi_d_state d_state = igt_pm_pci_get_d_state(pci_dev);
+
+		/*
+		 * Distinguish self-woken resumes from host-driven ones. A self-wakeup bumps
+		 * wakeup_active_count; a host-driven resume does not.
+		 */
+		if (d_state != IGT_ACPI_D3Hot) {
+			bool self_woke;
+
+			wakeups_after = igt_pm_get_wakeup_active_count(pci_dev);
+			self_woke = wakeups_after > wakeups_before;
+
+			igt_assert_eq(igt_gettime(&end), 0);
+			igt_assert_f(false,
+				     "Device left D3hot into %s after %.0fms of "
+				     "a %dms idle window.\n"
+				     "wakeup_active_count %" PRIu64 " -> %" PRIu64
+				     ", so the resume was %s.\n%s\n",
+				     pme_d_state_name(d_state),
+				     igt_time_elapsed(&start, &end) * 1000,
+				     PME_POLL_PROOF_WINDOW_MS,
+				     wakeups_before, wakeups_after,
+				     self_woke ? "signalled by the device" :
+						 "initiated by the host",
+				     self_woke ?
+				     "No hotplug was requested yet, so this is a "
+				     "spurious or still pending PME rather than "
+				     "the polling fallback." :
+				     "The polling fallback resumes on a 10s period, "
+				     "or 1s while a delayed event is pending, so "
+				     "compare the elapsed time above: a wake at an "
+				     "unrelated delay is some other client or "
+				     "driver taking a display power reference "
+				     "rather than connector polling.");
+		}
+
+		usleep(PME_D_STATE_SAMPLE_MS * 1000);
+
+		igt_assert_eq(igt_gettime(&end), 0);
+	} while (igt_time_elapsed(&start, &end) * 1000 < PME_POLL_PROOF_WINDOW_MS);
+
+	susp_delta = igt_pm_get_runtime_suspended_time(pci_dev) - susp_before;
+	active_delta = igt_pm_get_runtime_active_time(pci_dev) - active_before;
+
+	igt_info("Idle for %dms in D3hot: suspended +%" PRIu64 "ms, active +%" PRIu64 "ms\n",
+		 PME_POLL_PROOF_WINDOW_MS, susp_delta, active_delta);
+
+	/*
+	 * Belt and braces on top of the D state sampling above, which covers
+	 * resumes shorter than the sample interval: with the device parked in
+	 * D3hot for the whole window, all of the elapsed time is accounted to
+	 * runtime_suspended_time and runtime_active_time should barely move.
+	 */
+	igt_assert_f(active_delta <= PME_ACTIVE_TIME_SLACK_MS,
+		     "Device was active for %" PRIu64 "ms of a %dms idle window, "
+		     "so it resumed for shorter than the %dms sample interval. "
+		     "Something is still waking the device while idle and the "
+		     "feature saves no power\n",
+		     active_delta, PME_POLL_PROOF_WINDOW_MS,
+		     PME_D_STATE_SAMPLE_MS);
+	igt_assert_lte(PME_POLL_PROOF_WINDOW_MS - PME_POLL_PROOF_SLACK_MS,
+		       susp_delta);
+
+	/*
+	 * Phase 2: fire the HPD and check the device wakes up promptly. The
+	 * toggle is scheduled over XMLRPC, which travels over the network and so
+	 * does not disturb the GPU while we wait for the uevent.
+	 *
+	 * Take a fresh wakeup baseline rather than reusing the one from phase 1,
+	 * which is a whole idle window old by now. Phase 1 only requires that
+	 * the device stayed in D3hot, which a PME that resumed nothing at all
+	 * would not violate, so the counter is not guaranteed to be unchanged.
+	 */
+	wakeups_at_toggle = igt_pm_get_wakeup_active_count(pci_dev);
+
+	chamelium_schedule_hpd_toggle(data->chamelium, port,
+				      PME_HPD_TOGGLE_DELAY_MS, !connected);
+
+	igt_assert_eq(igt_gettime(&start), 0);
+	if (!chamelium_wait_for_hotplug(mon, &timeout)) {
+		enum igt_acpi_d_state d_state = igt_pm_pci_get_d_state(pci_dev);
+		bool pending = igt_pm_pci_pme_status(pci_dev);
+		const char *cause;
+
+		wakeups_after = igt_pm_get_wakeup_active_count(pci_dev);
+
+		/*
+		 * If the device stays in D3hot, the wake is from PME. If it returns to D0,
+		 * the HPD was handled by the driver resume path.
+		 */
+		if (d_state == IGT_ACPI_D3Hot)
+			cause = pending ?
+				"The device signalled a PME that was never "
+				"delivered, so the PME is not reaching the OS." :
+				"The device never signalled a PME, so the HPD "
+				"is not reaching the PME logic.";
+		else
+			cause = "The device resumed but sent no uevent, so the "
+				"HPD was dropped on the resume path.";
+
+		igt_assert_f(false,
+			     "No hotplug uevent %ds after an HPD toggle in D3hot.\n"
+			     "Device is in %s, PME_Status is %s, "
+			     "wakeup_active_count %" PRIu64 " -> %" PRIu64 ".\n%s\n",
+			     CHAMELIUM_HOTPLUG_TIMEOUT,
+			     pme_d_state_name(d_state),
+			     pending ? "set" : "clear",
+			     wakeups_at_toggle, wakeups_after, cause);
+	}
+	igt_assert_eq(igt_gettime(&end), 0);
+
+	latency = igt_time_elapsed(&start, &end) * 1000 -
+		  PME_HPD_TOGGLE_DELAY_MS;
+
+	/*
+	 * wakeup_active_count incrementing means the resume was signalled by the
+	 * device itself, i.e. by a PME. Unlike PME_Status, which the PCI/PM core
+	 * clears on the way back to D0, this counter survives the resume and so
+	 * is not racy to sample here.
+	 */
+	wakeups_after = igt_pm_get_wakeup_active_count(pci_dev);
+
+	igt_info("HPD to uevent latency %dms, wakeup_active_count %" PRIu64 " -> %" PRIu64 "\n",
+		 latency, wakeups_at_toggle, wakeups_after);
+
+	igt_assert_f(wakeups_after > wakeups_at_toggle,
+		     "power/wakeup_active_count did not increment, so the resume "
+		     "was not triggered by a PME from the device\n");
+	igt_assert_lt(latency, PME_WAKE_LATENCY_MAX_MS);
+
+	chamelium_assert_reachable(data->chamelium, ONLINE_TIMEOUT);
+	igt_assert_eq(chamelium_reprobe_connector(&data->display,
+						  data->chamelium, port),
+		      target_state);
+}
+
+/*
+ * Mirror of the driver's own HAS_PM_PME_SUPPORT(): only xe reports PME
+ * capability to the display code, and only from graphics IP 35.10 on.
+ */
+#define PME_HPD_MIN_GRAPHICS_VERX100 3500
+
+static bool pme_hpd_supported(int fd)
+{
+	const struct intel_device_info *info;
+
+	if (!is_xe_device(fd))
+		return false;
+
+	info = intel_get_device_info(intel_get_drm_devid(fd));
+
+	return info->graphics_ver * 100 + info->graphics_rel >=
+	       PME_HPD_MIN_GRAPHICS_VERX100;
+}
+
+static const char test_hpd_runtime_suspend_pme_desc[] =
+	"Toggle HPD while runtime suspended in D3hot with PME signalling "
+	"available, check that connector polling stayed disabled, that the "
+	"device woke itself up and that uevents are sent and connector status "
+	"is updated";
+static void test_hpd_runtime_suspend_pme(chamelium_data_t *data,
+					 struct chamelium_port *port)
+{
+	struct pci_device *pci_dev;
+	struct udev_monitor *mon;
+
+	pci_dev = igt_device_get_pci_device(data->drm_fd);
+
+	/*
+	 * Without driver support the device keeps HPD polling enabled and phase
+	 * 1 would fail rather than skip, so gate on the platform before looking
+	 * at the hardware capability at all.
+	 */
+	igt_require_f(pme_hpd_supported(data->drm_fd),
+		      "Driver does not use PME for HPD on this platform\n");
+
+	igt_require_f(igt_pm_pci_pme_supported(pci_dev, IGT_ACPI_D3Hot),
+		      "Device does not advertise PME support in D3hot\n");
+	igt_require_f(igt_pm_has_wakeup_support(pci_dev),
+		      "Device does not expose power/wakeup\n");
+	igt_require(igt_setup_runtime_pm(data->drm_fd));
+	igt_require_hpd_storm_ctl(data->drm_fd);
+
+	/*
+	 * Need both: PME is only armed when device_may_wakeup() is true, and
+	 * without wakeup support the PM core never records the PME wakeup.
+	 */
+	igt_pm_set_wakeup_enabled(pci_dev, true);
+
+	/*
+	 * Save the poll setting so a previous test cannot leave it disabled and
+	 * fake a pass. Polling must stay enabled; the test checks that the driver
+	 * does not use it when PME is available.
+	 */
+	igt_require(igt_pm_kms_poll_save());
+	igt_pm_kms_poll_set(true);
+
+	/* Don't let our own toggles trip the storm detection into polling. */
+	igt_hpd_storm_set_threshold(data->drm_fd, 0);
+
+	igt_modeset_disable_all_outputs(&data->display);
+
+	mon = igt_watch_uevents();
+	chamelium_reset_state(&data->display, data->chamelium, port,
+			      data->ports, data->port_count);
+
+	/* Ports are left disconnected by the reset, so plug first. */
+	try_hpd_runtime_suspend_pme(data, port, pci_dev, mon, false);
+
+	/* Now check we notice a disconnect signalled from D3hot too. */
+	try_hpd_runtime_suspend_pme(data, port, pci_dev, mon, true);
+
+	igt_cleanup_uevents(mon);
+	igt_hpd_storm_reset(data->drm_fd);
+	igt_pm_kms_poll_restore();
+	igt_pm_restore_wakeup();
+}
+
 static const char test_hpd_without_ddc_desc[] =
 	"Disable DDC on a VGA connector, check we still get a uevent on hotplug";
 static void test_hpd_without_ddc(chamelium_data_t *data,
@@ -500,6 +836,10 @@ int igt_main()
 	connector_subtest("dp-hpd-after-hibernate", DisplayPort, &data, test_suspend_resume_hpd,
 			  SUSPEND_STATE_DISK, SUSPEND_TEST_DEVICES);
 
+	igt_describe(test_hpd_runtime_suspend_pme_desc);
+	connector_subtest("dp-hpd-after-runtime-suspend-d3hot", DisplayPort,
+			  &data, test_hpd_runtime_suspend_pme);
+
 	igt_describe(test_hpd_storm_detect_desc);
 	connector_subtest("dp-hpd-storm", DisplayPort, &data, test_hpd_storm_detect,
 			  HPD_STORM_PULSE_INTERVAL_DP);
@@ -538,6 +878,10 @@ int igt_main()
 	connector_subtest("hdmi-hpd-after-hibernate", HDMIA, &data, test_suspend_resume_hpd,
 			  SUSPEND_STATE_DISK, SUSPEND_TEST_DEVICES);
 
+	igt_describe(test_hpd_runtime_suspend_pme_desc);
+	connector_subtest("hdmi-hpd-after-runtime-suspend-d3hot", HDMIA,
+			  &data, test_hpd_runtime_suspend_pme);
+
 	igt_describe(test_hpd_storm_detect_desc);
 	connector_subtest("hdmi-hpd-storm", HDMIA, &data, test_hpd_storm_detect,
 			  HPD_STORM_PULSE_INTERVAL_HDMI);
-- 
2.53.0


  parent reply	other threads:[~2026-09-07 14:12 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07 14:22 [PATCH i-g-t 0/7] Validate PM_PME signalling on display hotplug Pranay Samala
2026-09-07 14:22 ` [PATCH i-g-t 1/7] lib/igt_pci: Add PCI Power Management capability register layout Pranay Samala
2026-09-07 14:22 ` [PATCH i-g-t 2/7] lib/igt_pm: Add PCI PME capability and D state accessors Pranay Samala
2026-09-07 14:22 ` [PATCH i-g-t 3/7] lib/igt_pm: Factor out power attribute path construction Pranay Samala
2026-09-07 14:22 ` [PATCH i-g-t 4/7] lib/igt_pm: Add power/wakeup accessors Pranay Samala
2026-09-07 14:22 ` [PATCH i-g-t 5/7] lib/igt_pm: Add power/wakeup_active_count accessor Pranay Samala
2026-09-07 14:22 ` [PATCH i-g-t 6/7] lib/igt_pm: Add drm_kms_helper.poll save/restore helpers Pranay Samala
2026-09-07 14:22 ` Pranay Samala [this message]
2026-09-16 15:40   ` [PATCH i-g-t 7/7] tests/chamelium/kms_chamelium_hpd: Add HPD from runtime suspended D3hot Govindapillai, Vinod
2026-09-07 19:49 ` ✓ Xe.CI.BAT: success for Validate PM_PME signalling on display hotplug (rev2) Patchwork
2026-09-07 20:02 ` ✓ i915.CI.BAT: " Patchwork
2026-09-08  0:17 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-09-08  6:28 ` ✗ i915.CI.Full: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260907142259.750528-8-pranay.samala@intel.com \
    --to=pranay.samala@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=karthik.b.s@intel.com \
    --cc=sameer.lattannavar@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox