* [PATCH i-g-t 2/5] lib/igt_pm: Fix and standardize IGT PM library documentation
2024-05-13 18:55 [PATCH i-g-t 1/5] tests/intel/xe_pm: Update runtime pm conditions Rodrigo Vivi
@ 2024-05-13 18:55 ` Rodrigo Vivi
2024-05-14 6:49 ` Riana Tauro
2024-05-13 18:55 ` [PATCH i-g-t 3/5] tests/intel/xe_pm: Also disable display for mmap_system test Rodrigo Vivi
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Rodrigo Vivi @ 2024-05-13 18:55 UTC (permalink / raw)
To: igt-dev; +Cc: intel-xe, Rodrigo Vivi, Kamil Konieczny
A revamp to get some kind of standard in the various exported
functions in this library.
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
lib/igt_pm.c | 148 +++++++++++++++++++++++++--------------------------
1 file changed, 73 insertions(+), 75 deletions(-)
diff --git a/lib/igt_pm.c b/lib/igt_pm.c
index 928b72685..2c91aeb33 100644
--- a/lib/igt_pm.c
+++ b/lib/igt_pm.c
@@ -454,7 +454,7 @@ static void __igt_pm_restore_sata_link_power_management(void)
/**
* igt_pm_enable_sata_link_power_management:
*
- * Enable the min_power policy for SATA link power management.
+ * Enables the min_power policy for SATA link power management.
* Without this we cannot reach deep runtime power states.
*/
void igt_pm_enable_sata_link_power_management(void)
@@ -469,7 +469,7 @@ void igt_pm_enable_sata_link_power_management(void)
/**
* igt_pm_restore_sata_link_power_management:
*
- * Restore the link power management policies to the values
+ * Restores the link power management policies to the values
* prior to enabling min_power.
*
* Caveat: If the system supports hotplugging and hotplugging takes
@@ -566,8 +566,7 @@ static void __igt_pm_runtime_exit_handler(int sig)
* Sets up the runtime PM helper functions and enables runtime PM. To speed up
* tests the autosuspend delay is set to 0.
*
- * Returns:
- * True if runtime pm is available, false otherwise.
+ * Return: True if runtime pm is available, false otherwise.
*/
bool igt_setup_runtime_pm(int device)
{
@@ -658,7 +657,7 @@ bool igt_setup_runtime_pm(int device)
/**
* igt_disable_runtime_pm:
*
- * Disable the runtime pm for i915 device.
+ * Disables the runtime pm for i915 device.
* igt_disable_runtime_pm assumes that igt_setup_runtime_pm has already
* called to save runtime autosuspend and control attributes.
*/
@@ -683,11 +682,6 @@ void igt_disable_runtime_pm(void)
close(fd);
}
-/**
- * igt_get_runtime_pm_status:
- *
- * Returns: The current runtime PM status.
- */
static enum igt_runtime_pm_status __igt_get_runtime_pm_status(int fd)
{
ssize_t n_read;
@@ -711,6 +705,11 @@ static enum igt_runtime_pm_status __igt_get_runtime_pm_status(int fd)
return IGT_RUNTIME_PM_STATUS_UNKNOWN;
}
+/**
+ * igt_get_runtime_pm_status:
+ *
+ * Return: The current runtime PM status.
+ */
enum igt_runtime_pm_status igt_get_runtime_pm_status(void)
{
enum igt_runtime_pm_status status;
@@ -728,12 +727,6 @@ enum igt_runtime_pm_status igt_get_runtime_pm_status(void)
return status;
}
-/**
- * _pm_status_name
- * @status: runtime PM status to stringify
- *
- * Returns: The current runtime PM status as a string
- */
static const char *_pm_status_name(enum igt_runtime_pm_status status)
{
switch (status) {
@@ -757,9 +750,8 @@ static const char *_pm_status_name(enum igt_runtime_pm_status status)
* Waits until for the driver to switch to into the desired runtime PM status,
* with a 10 second timeout.
*
- * Returns:
- * True if the desired runtime PM status was attained, false if the operation
- * timed out.
+ * Return: True if the desired runtime PM status was attained, false if the
+ * operation timed out.
*/
bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
{
@@ -791,15 +783,14 @@ static const char *yesno(bool x)
}
/**
- * dmc_loaded:
- * @debugfs: fd to the debugfs dir.
+ * igt_pm_dmc_loaded:
+ * @debugfs: FD to the debugfs directory
*
* Check whether DMC FW is loaded or not. DMC FW is require for few Display C
* states like DC5 and DC6. FW does the Context Save and Restore during Display
* C States entry and exit.
*
- * Returns:
- * True if DMC FW is loaded otherwise false.
+ * Return: True if DMC FW is loaded otherwise false.
*/
bool igt_pm_dmc_loaded(int debugfs)
{
@@ -821,11 +812,11 @@ bool igt_pm_dmc_loaded(int debugfs)
/**
* igt_pm_pc8_plus_residencies_enabled:
- * @msr_fd: fd to /dev/cpu/0/msr
+ * @msr_fd: FD to /dev/cpu/0/msr
+ *
* Check whether BIOS has disabled the PC8 package deeper state.
*
- * Returns:
- * True if PC8+ package deeper state enabled on machine otherwise false.
+ * Return: True if PC8+ package deeper state enabled on machine otherwise false.
*/
bool igt_pm_pc8_plus_residencies_enabled(int msr_fd)
{
@@ -847,10 +838,10 @@ bool igt_pm_pc8_plus_residencies_enabled(int msr_fd)
* i915_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.
+ * Checks LPSP capability for a given output.
+ *
+ * Return: True if given output is LPSP capable otherwise false.
*/
bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
{
@@ -887,14 +878,13 @@ static int igt_pm_open_pci_firmware_node(struct pci_device *pci_dev)
/**
* igt_pm_get_pcie_acpihp_slot:
- * @pci_dev: pci bridge device.
- * Get pci bridge acpi hotplug slot number, if bridge's ACPI firmware_node
+ * @pci_dev: PCI bridge device struct
+ *
+ * Gets PCI bridge acpi hotplug slot number, if bridge's ACPI firmware_node
* handle supports _SUN method.
*
- * Returns:
- * PCIe bridge Slot number.
- * Returns -ENOENT number in case firmware_node/sun is not supported by the
- * bridge.
+ * Return: PCIe bridge Slot number or -ENOENT number in case firmware_node/sun
+ * is not supported by the bridge.
*/
int igt_pm_get_pcie_acpihp_slot(struct pci_device *pci_dev)
{
@@ -928,11 +918,11 @@ int igt_pm_get_pcie_acpihp_slot(struct pci_device *pci_dev)
/**
* igt_pm_acpi_d3cold_supported:
- * @pci_dev: root port pci_dev.
- * Check ACPI D3Cold support.
+ * @pci_dev: Root port PCI device struct
+ *
+ * Checks ACPI D3Cold support.
*
- * Returns:
- * True if ACPI D3Cold supported otherwise false.
+ * Return: True if ACPI D3Cold supported otherwise false.
*/
bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev)
{
@@ -958,11 +948,11 @@ bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev)
/**
* igt_pm_get_acpi_real_d_state:
- * @pci_dev: root port pci_dev.
- * Get ACPI D state for a given root port.
+ * @pci_dev: Root port PCI device struct
*
- * Returns:
- * igt_acpi_d_state state.
+ * Gets ACPI D state for a given root port.
+ *
+ * Return: igt_acpi_d_state state.
*/
enum igt_acpi_d_state
igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev)
@@ -1155,12 +1145,9 @@ igt_pm_setup_pci_card_power_attrs(struct pci_device *pci_dev, bool save_attrs, i
/**
* igt_pm_get_autosuspend_delay:
- * @pci_dev: pci_dev.
- *
- * Get pci_dev autosuspend delay value from pci sysfs.
+ * @pci_dev: PCI device struct
*
- * Returns:
- * autosuspend_delay_ms.
+ * Return: The autosuspend delay time in miliseconds.
*/
int igt_pm_get_autosuspend_delay(struct pci_device *pci_dev)
{
@@ -1177,10 +1164,10 @@ int igt_pm_get_autosuspend_delay(struct pci_device *pci_dev)
/**
* igt_pm_set_autosuspend_delay:
- * @pci_dev: pci_dev.
- * @delay_ms: autosuspend delay in ms.
+ * @pci_dev: PCI device struct
+ * @delay_ms: Autosuspend delay in miliseconds.
*
- * Set pci_dev autosuspend delay value through pci sysfs.
+ * Sets the autosuspend delay value for the PCI device through.
*/
void igt_pm_set_autosuspend_delay(struct pci_device *pci_dev, int delay_ms)
{
@@ -1201,26 +1188,28 @@ void igt_pm_set_autosuspend_delay(struct pci_device *pci_dev, int delay_ms)
/**
* igt_pm_enable_pci_card_runtime_pm:
- * @root: root port pci_dev.
- * @i915: i915 pci_dev.
- * Enable runtime PM for all PCI endpoints devices for a given root port by
+ * @root: Root port PCI device struct
+ * @gfx: PCI device struct of graphics device
+ *
+ * Enables runtime PM for all PCI endpoints devices for a given root port by
* setting power/control attr to "auto" and setting autosuspend_delay_ms
* to zero.
*/
void igt_pm_enable_pci_card_runtime_pm(struct pci_device *root,
- struct pci_device *i915)
+ struct pci_device *gfx)
{
int delay = -1;
- if (i915)
- delay = igt_pm_get_autosuspend_delay(i915);
+ if (gfx)
+ delay = igt_pm_get_autosuspend_delay(gfx);
igt_pm_setup_pci_card_power_attrs(root, false, delay);
}
/**
* igt_pm_setup_pci_card_runtime_pm:
- * @pci_dev: root port pci_dev.
+ * @pci_dev: Root port PCI device struct
+ *
* Setup runtime PM for all PCI endpoints devices for a given root port by
* enabling runtime suspend and setting autosuspend_delay_ms to zero.
* It also saves and restore power control attribute for all PCI endpoints
@@ -1234,11 +1223,10 @@ void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev)
/**
* igt_pm_get_d3cold_allowed:
- * @pci_slot_name: slot name of the pci device
- * @value: value to be read into
+ * @pci_slot_name: Slot name of the PCI device
+ * @value: Value to be read into
*
- * Reads the value of d3cold_allowed attribute
- * of the pci device
+ * Reads the value of d3cold_allowed attribute of the PCI device.
*/
void igt_pm_get_d3cold_allowed(const char *pci_slot_name, uint32_t *value)
{
@@ -1258,10 +1246,10 @@ void igt_pm_get_d3cold_allowed(const char *pci_slot_name, uint32_t *value)
/**
* igt_pm_set_d3cold_allowed:
- * @pci_slot_name: slot name of pci device
- * @value: value to be written
+ * @pci_slot_name: Slot name of PCI device
+ * @value: Value to be written
*
- * writes the value to d3cold_allowed attribute of pci device
+ * Writes the value to d3cold_allowed attribute of PCI device.
*/
void igt_pm_set_d3cold_allowed(const char *pci_slot_name, uint32_t value)
{
@@ -1294,7 +1282,8 @@ igt_pm_restore_power_attr(struct pci_device *pci_dev, const char *attr, char *va
/**
* igt_pm_restore_pci_card_runtime_pm:
- * Restore control and autosuspend_delay_ms power attribute for all
+ *
+ * Restores control and autosuspend_delay_ms power attribute for all
* PCI endpoints devices under gfx root port, which were saved earlier
* by igt_pm_setup_pci_card_runtime_pm().
*/
@@ -1342,8 +1331,9 @@ static void igt_pm_print_pci_dev_runtime_status(struct pci_device *pci_dev)
/**
* igt_pm_print_pci_card_runtime_status:
- * @pci_dev: root port pci_dev.
- * Print runtime suspend status for all PCI endpoints devices for a given
+ * @pci_dev: Root port PCI device struct
+ *
+ * Prints runtime suspend status for all PCI endpoints devices for a given
* root port.
*/
void igt_pm_print_pci_card_runtime_status(void)
@@ -1361,8 +1351,9 @@ void igt_pm_print_pci_card_runtime_status(void)
/**
* i915_is_slpc_enabled_gt:
* @drm_fd: DRM file descriptor
- * @gt: GT id
- * Check if SLPC is enabled on a GT
+ * @gt: GT ID
+ *
+ * Return: True if SLPC is enabled on a given @gt.
*/
bool i915_is_slpc_enabled_gt(int drm_fd, int gt)
{
@@ -1387,13 +1378,20 @@ bool i915_is_slpc_enabled_gt(int drm_fd, int gt)
/**
* i915_is_slpc_enabled:
* @drm_fd: DRM file descriptor
- * Check if SLPC is enabled for the device
+ *
+ * Return: True if SLPC is enabled on the device.
*/
bool i915_is_slpc_enabled(int drm_fd)
{
return i915_is_slpc_enabled_gt(drm_fd, 0);
}
+/**
+ * igt_pm_get_runtime_suspended_time:
+ * @pci_dev: PCI device struct
+ *
+ * Return: The total time that the device has been suspended.
+ */
int igt_pm_get_runtime_suspended_time(struct pci_device *pci_dev)
{
char time_str[64];
@@ -1438,9 +1436,9 @@ int igt_pm_get_runtime_active_time(struct pci_device *pci_dev)
/**
* igt_pm_get_runtime_usage:
- * @pci_dev: pci device
+ * @pci_dev: PCI device struct
*
- * Reports the runtime PM usage count of a device.
+ * Return: The runtime PM usage count of a device.
*/
int igt_pm_get_runtime_usage(struct pci_device *pci_dev)
{
@@ -1455,12 +1453,12 @@ int igt_pm_get_runtime_usage(struct pci_device *pci_dev)
}
/**
- * igt_pm_ignore_slpc_efficient_freq
+ * igt_pm_ignore_slpc_efficient_freq:
* @i915: open i915 drm file descriptor
* @gtfd: open gt sysfs fd
* @val: value to set
*
- * Ignores/un-ignores SLPC efficient frequency
+ * Ignores/un-ignores SLPC efficient frequency.
*/
void igt_pm_ignore_slpc_efficient_freq(int i915, int gtfd, bool val)
{
--
2.44.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH i-g-t 2/5] lib/igt_pm: Fix and standardize IGT PM library documentation
2024-05-13 18:55 ` [PATCH i-g-t 2/5] lib/igt_pm: Fix and standardize IGT PM library documentation Rodrigo Vivi
@ 2024-05-14 6:49 ` Riana Tauro
0 siblings, 0 replies; 13+ messages in thread
From: Riana Tauro @ 2024-05-14 6:49 UTC (permalink / raw)
To: Rodrigo Vivi, igt-dev; +Cc: intel-xe, Kamil Konieczny
Hi Rodrigo
On 5/14/2024 12:25 AM, Rodrigo Vivi wrote:
> A revamp to get some kind of standard in the various exported
> functions in this library.
>
> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> lib/igt_pm.c | 148 +++++++++++++++++++++++++--------------------------
> 1 file changed, 73 insertions(+), 75 deletions(-)
>
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index 928b72685..2c91aeb33 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -454,7 +454,7 @@ static void __igt_pm_restore_sata_link_power_management(void)
> /**
> * igt_pm_enable_sata_link_power_management:
> *
> - * Enable the min_power policy for SATA link power management.
> + * Enables the min_power policy for SATA link power management.
> * Without this we cannot reach deep runtime power states.
> */
> void igt_pm_enable_sata_link_power_management(void)
> @@ -469,7 +469,7 @@ void igt_pm_enable_sata_link_power_management(void)
> /**
> * igt_pm_restore_sata_link_power_management:
> *
> - * Restore the link power management policies to the values
> + * Restores the link power management policies to the values
> * prior to enabling min_power.
> *
> * Caveat: If the system supports hotplugging and hotplugging takes
> @@ -566,8 +566,7 @@ static void __igt_pm_runtime_exit_handler(int sig)
> * Sets up the runtime PM helper functions and enables runtime PM. To speed up
> * tests the autosuspend delay is set to 0.
> *
> - * Returns:
> - * True if runtime pm is available, false otherwise.
> + * Return: True if runtime pm is available, false otherwise.
> */
> bool igt_setup_runtime_pm(int device)
> {
> @@ -658,7 +657,7 @@ bool igt_setup_runtime_pm(int device)
> /**
> * igt_disable_runtime_pm:
> *
> - * Disable the runtime pm for i915 device.
> + * Disables the runtime pm for i915 device.
> * igt_disable_runtime_pm assumes that igt_setup_runtime_pm has already
> * called to save runtime autosuspend and control attributes.
> */
> @@ -683,11 +682,6 @@ void igt_disable_runtime_pm(void)
> close(fd);
> }
>
> -/**
> - * igt_get_runtime_pm_status:
> - *
> - * Returns: The current runtime PM status.
> - */
> static enum igt_runtime_pm_status __igt_get_runtime_pm_status(int fd)
> {
> ssize_t n_read;
> @@ -711,6 +705,11 @@ static enum igt_runtime_pm_status __igt_get_runtime_pm_status(int fd)
> return IGT_RUNTIME_PM_STATUS_UNKNOWN;
> }
>
> +/**
> + * igt_get_runtime_pm_status:
> + *
> + * Return: The current runtime PM status.
> + */
> enum igt_runtime_pm_status igt_get_runtime_pm_status(void)
> {
> enum igt_runtime_pm_status status;
> @@ -728,12 +727,6 @@ enum igt_runtime_pm_status igt_get_runtime_pm_status(void)
> return status;
> }
>
> -/**
> - * _pm_status_name
> - * @status: runtime PM status to stringify
> - *
> - * Returns: The current runtime PM status as a string
> - */
> static const char *_pm_status_name(enum igt_runtime_pm_status status)
> {
> switch (status) {
> @@ -757,9 +750,8 @@ static const char *_pm_status_name(enum igt_runtime_pm_status status)
> * Waits until for the driver to switch to into the desired runtime PM status,
> * with a 10 second timeout.
> *
> - * Returns:
> - * True if the desired runtime PM status was attained, false if the operation
> - * timed out.
> + * Return: True if the desired runtime PM status was attained, false if the
> + * operation timed out.
> */
> bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
> {
> @@ -791,15 +783,14 @@ static const char *yesno(bool x)
> }
>
> /**
> - * dmc_loaded:
> - * @debugfs: fd to the debugfs dir.
> + * igt_pm_dmc_loaded:
> + * @debugfs: FD to the debugfs directory
> *
> * Check whether DMC FW is loaded or not. DMC FW is require for few Display C
> * states like DC5 and DC6. FW does the Context Save and Restore during Display
> * C States entry and exit.
> *
> - * Returns:
> - * True if DMC FW is loaded otherwise false.
> + * Return: True if DMC FW is loaded otherwise false.
> */
> bool igt_pm_dmc_loaded(int debugfs)
> {
> @@ -821,11 +812,11 @@ bool igt_pm_dmc_loaded(int debugfs)
>
> /**
> * igt_pm_pc8_plus_residencies_enabled:
> - * @msr_fd: fd to /dev/cpu/0/msr
> + * @msr_fd: FD to /dev/cpu/0/msr
> + *
> * Check whether BIOS has disabled the PC8 package deeper state.
> *
> - * Returns:
> - * True if PC8+ package deeper state enabled on machine otherwise false.
> + * Return: True if PC8+ package deeper state enabled on machine otherwise false.
> */
> bool igt_pm_pc8_plus_residencies_enabled(int msr_fd)
> {
> @@ -847,10 +838,10 @@ bool igt_pm_pc8_plus_residencies_enabled(int msr_fd)
> * i915_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.
> + * Checks LPSP capability for a given output.
> + *
> + * Return: True if given output is LPSP capable otherwise false.
^ extra space has to be removed
> */
> bool i915_output_is_lpsp_capable(int drm_fd, igt_output_t *output)
> {
> @@ -887,14 +878,13 @@ static int igt_pm_open_pci_firmware_node(struct pci_device *pci_dev)
>
> /**
> * igt_pm_get_pcie_acpihp_slot:
> - * @pci_dev: pci bridge device.
> - * Get pci bridge acpi hotplug slot number, if bridge's ACPI firmware_node
> + * @pci_dev: PCI bridge device struct
> + *
> + * Gets PCI bridge acpi hotplug slot number, if bridge's ACPI firmware_node
> * handle supports _SUN method.
> *
> - * Returns:
> - * PCIe bridge Slot number.
> - * Returns -ENOENT number in case firmware_node/sun is not supported by the
> - * bridge.
> + * Return: PCIe bridge Slot number or -ENOENT number in case firmware_node/sun
> + * is not supported by the bridge.
> */
> int igt_pm_get_pcie_acpihp_slot(struct pci_device *pci_dev)
> {
> @@ -928,11 +918,11 @@ int igt_pm_get_pcie_acpihp_slot(struct pci_device *pci_dev)
>
> /**
> * igt_pm_acpi_d3cold_supported:
> - * @pci_dev: root port pci_dev.
> - * Check ACPI D3Cold support.
> + * @pci_dev: Root port PCI device struct
> + *
> + * Checks ACPI D3Cold support.
> *
> - * Returns:
> - * True if ACPI D3Cold supported otherwise false.
> + * Return: True if ACPI D3Cold supported otherwise false.
> */
> bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev)
> {
> @@ -958,11 +948,11 @@ bool igt_pm_acpi_d3cold_supported(struct pci_device *pci_dev)
>
> /**
> * igt_pm_get_acpi_real_d_state:
> - * @pci_dev: root port pci_dev.
> - * Get ACPI D state for a given root port.
> + * @pci_dev: Root port PCI device struct
> *
> - * Returns:
> - * igt_acpi_d_state state.
> + * Gets ACPI D state for a given root port.
> + *
> + * Return: igt_acpi_d_state state.
> */
> enum igt_acpi_d_state
> igt_pm_get_acpi_real_d_state(struct pci_device *pci_dev)
> @@ -1155,12 +1145,9 @@ igt_pm_setup_pci_card_power_attrs(struct pci_device *pci_dev, bool save_attrs, i
>
> /**
> * igt_pm_get_autosuspend_delay:
> - * @pci_dev: pci_dev.
> - *
> - * Get pci_dev autosuspend delay value from pci sysfs.
> + * @pci_dev: PCI device struct
> *
> - * Returns:
> - * autosuspend_delay_ms.
> + * Return: The autosuspend delay time in miliseconds.
> */
> int igt_pm_get_autosuspend_delay(struct pci_device *pci_dev)
> {
> @@ -1177,10 +1164,10 @@ int igt_pm_get_autosuspend_delay(struct pci_device *pci_dev)
>
> /**
> * igt_pm_set_autosuspend_delay:
> - * @pci_dev: pci_dev.
> - * @delay_ms: autosuspend delay in ms.
> + * @pci_dev: PCI device struct
> + * @delay_ms: Autosuspend delay in miliseconds.
> *
> - * Set pci_dev autosuspend delay value through pci sysfs.
> + * Sets the autosuspend delay value for the PCI device through.
> */
> void igt_pm_set_autosuspend_delay(struct pci_device *pci_dev, int delay_ms)
> {
> @@ -1201,26 +1188,28 @@ void igt_pm_set_autosuspend_delay(struct pci_device *pci_dev, int delay_ms)
>
> /**
> * igt_pm_enable_pci_card_runtime_pm:
> - * @root: root port pci_dev.
> - * @i915: i915 pci_dev.
> - * Enable runtime PM for all PCI endpoints devices for a given root port by
> + * @root: Root port PCI device struct
> + * @gfx: PCI device struct of graphics device
> + *
> + * Enables runtime PM for all PCI endpoints devices for a given root port by
> * setting power/control attr to "auto" and setting autosuspend_delay_ms
> * to zero.
> */
> void igt_pm_enable_pci_card_runtime_pm(struct pci_device *root,
> - struct pci_device *i915)
> + struct pci_device *gfx)
> {
> int delay = -1;
>
> - if (i915)
> - delay = igt_pm_get_autosuspend_delay(i915);
> + if (gfx)
> + delay = igt_pm_get_autosuspend_delay(gfx);
>
> igt_pm_setup_pci_card_power_attrs(root, false, delay);
> }
>
> /**
> * igt_pm_setup_pci_card_runtime_pm:
> - * @pci_dev: root port pci_dev.
> + * @pci_dev: Root port PCI device struct
> + *
> * Setup runtime PM for all PCI endpoints devices for a given root port by
> * enabling runtime suspend and setting autosuspend_delay_ms to zero.
> * It also saves and restore power control attribute for all PCI endpoints
> @@ -1234,11 +1223,10 @@ void igt_pm_setup_pci_card_runtime_pm(struct pci_device *pci_dev)
>
> /**
> * igt_pm_get_d3cold_allowed:
> - * @pci_slot_name: slot name of the pci device
> - * @value: value to be read into
> + * @pci_slot_name: Slot name of the PCI device
> + * @value: Value to be read into
> *
> - * Reads the value of d3cold_allowed attribute
> - * of the pci device
> + * Reads the value of d3cold_allowed attribute of the PCI device.
> */
> void igt_pm_get_d3cold_allowed(const char *pci_slot_name, uint32_t *value)
> {
> @@ -1258,10 +1246,10 @@ void igt_pm_get_d3cold_allowed(const char *pci_slot_name, uint32_t *value)
>
> /**
> * igt_pm_set_d3cold_allowed:
> - * @pci_slot_name: slot name of pci device
> - * @value: value to be written
> + * @pci_slot_name: Slot name of PCI device
> + * @value: Value to be written
> *
> - * writes the value to d3cold_allowed attribute of pci device
> + * Writes the value to d3cold_allowed attribute of PCI device.
> */
> void igt_pm_set_d3cold_allowed(const char *pci_slot_name, uint32_t value)
> {
> @@ -1294,7 +1282,8 @@ igt_pm_restore_power_attr(struct pci_device *pci_dev, const char *attr, char *va
>
> /**
> * igt_pm_restore_pci_card_runtime_pm:
> - * Restore control and autosuspend_delay_ms power attribute for all
> + *
> + * Restores control and autosuspend_delay_ms power attribute for all
s/attribute/attributes
With the above fixes
Looks good to me
Reviewed-by: Riana Tauro <riana.tauro@intel.com>
> * PCI endpoints devices under gfx root port, which were saved earlier
> * by igt_pm_setup_pci_card_runtime_pm().
> */
> @@ -1342,8 +1331,9 @@ static void igt_pm_print_pci_dev_runtime_status(struct pci_device *pci_dev)
>
> /**
> * igt_pm_print_pci_card_runtime_status:
> - * @pci_dev: root port pci_dev.
> - * Print runtime suspend status for all PCI endpoints devices for a given
> + * @pci_dev: Root port PCI device struct
> + *
> + * Prints runtime suspend status for all PCI endpoints devices for a given
> * root port.
> */
> void igt_pm_print_pci_card_runtime_status(void)
> @@ -1361,8 +1351,9 @@ void igt_pm_print_pci_card_runtime_status(void)
> /**
> * i915_is_slpc_enabled_gt:
> * @drm_fd: DRM file descriptor
> - * @gt: GT id
> - * Check if SLPC is enabled on a GT
> + * @gt: GT ID
> + *
> + * Return: True if SLPC is enabled on a given @gt.
> */
> bool i915_is_slpc_enabled_gt(int drm_fd, int gt)
> {
> @@ -1387,13 +1378,20 @@ bool i915_is_slpc_enabled_gt(int drm_fd, int gt)
> /**
> * i915_is_slpc_enabled:
> * @drm_fd: DRM file descriptor
> - * Check if SLPC is enabled for the device
> + *
> + * Return: True if SLPC is enabled on the device.
> */
> bool i915_is_slpc_enabled(int drm_fd)
> {
> return i915_is_slpc_enabled_gt(drm_fd, 0);
> }
>
> +/**
> + * igt_pm_get_runtime_suspended_time:
> + * @pci_dev: PCI device struct
> + *
> + * Return: The total time that the device has been suspended.
> + */
> int igt_pm_get_runtime_suspended_time(struct pci_device *pci_dev)
> {
> char time_str[64];
> @@ -1438,9 +1436,9 @@ int igt_pm_get_runtime_active_time(struct pci_device *pci_dev)
>
> /**
> * igt_pm_get_runtime_usage:
> - * @pci_dev: pci device
> + * @pci_dev: PCI device struct
> *
> - * Reports the runtime PM usage count of a device.
> + * Return: The runtime PM usage count of a device.
> */
> int igt_pm_get_runtime_usage(struct pci_device *pci_dev)
> {
> @@ -1455,12 +1453,12 @@ int igt_pm_get_runtime_usage(struct pci_device *pci_dev)
> }
>
> /**
> - * igt_pm_ignore_slpc_efficient_freq
> + * igt_pm_ignore_slpc_efficient_freq:
> * @i915: open i915 drm file descriptor
> * @gtfd: open gt sysfs fd
> * @val: value to set
> *
> - * Ignores/un-ignores SLPC efficient frequency
> + * Ignores/un-ignores SLPC efficient frequency.
> */
> void igt_pm_ignore_slpc_efficient_freq(int i915, int gtfd, bool val)
> {
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH i-g-t 3/5] tests/intel/xe_pm: Also disable display for mmap_system test
2024-05-13 18:55 [PATCH i-g-t 1/5] tests/intel/xe_pm: Update runtime pm conditions Rodrigo Vivi
2024-05-13 18:55 ` [PATCH i-g-t 2/5] lib/igt_pm: Fix and standardize IGT PM library documentation Rodrigo Vivi
@ 2024-05-13 18:55 ` Rodrigo Vivi
2024-05-14 6:12 ` Gupta, Anshuman
2024-05-14 6:46 ` Nilawar, Badal
2024-05-13 18:55 ` [PATCH i-g-t 4/5] tests/intel/xe_pm: Only check the rpm resume after the first mmap operation Rodrigo Vivi
` (3 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2024-05-13 18:55 UTC (permalink / raw)
To: igt-dev
Cc: intel-xe, Rodrigo Vivi, Badal Nilawar, Anshuman Gupta,
Francois Dugast
If display is enabled, the always-on components will always
keeps runtime pm references, unless you put DPMS_OFF.
Cc: Badal Nilawar <badal.nilawar@intel.com>
Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Cc: Francois Dugast <francois.dugast@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
tests/intel/xe_pm.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
index 3f963bd9b..72d484163 100644
--- a/tests/intel/xe_pm.c
+++ b/tests/intel/xe_pm.c
@@ -780,7 +780,9 @@ igt_main
igt_describe("Validate mmap memory mappings with system region,"
"when device along with parent bridge in d3");
igt_subtest("d3-mmap-system") {
+ dpms_on_off(device, DRM_MODE_DPMS_OFF);
test_mmap(device, system_memory(device.fd_xe), 0);
+ dpms_on_off(device, DRM_MODE_DPMS_ON);
}
igt_describe("Validate mmap memory mappings with vram region,"
--
2.44.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* RE: [PATCH i-g-t 3/5] tests/intel/xe_pm: Also disable display for mmap_system test
2024-05-13 18:55 ` [PATCH i-g-t 3/5] tests/intel/xe_pm: Also disable display for mmap_system test Rodrigo Vivi
@ 2024-05-14 6:12 ` Gupta, Anshuman
2024-05-14 6:46 ` Nilawar, Badal
1 sibling, 0 replies; 13+ messages in thread
From: Gupta, Anshuman @ 2024-05-14 6:12 UTC (permalink / raw)
To: Vivi, Rodrigo, igt-dev@lists.freedesktop.org
Cc: intel-xe@lists.freedesktop.org, Nilawar, Badal, Dugast, Francois
> -----Original Message-----
> From: Vivi, Rodrigo <rodrigo.vivi@intel.com>
> Sent: Tuesday, May 14, 2024 12:25 AM
> To: igt-dev@lists.freedesktop.org
> Cc: intel-xe@lists.freedesktop.org; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> Nilawar, Badal <badal.nilawar@intel.com>; Gupta, Anshuman
> <anshuman.gupta@intel.com>; Dugast, Francois <francois.dugast@intel.com>
> Subject: [PATCH i-g-t 3/5] tests/intel/xe_pm: Also disable display for
> mmap_system test
>
> If display is enabled, the always-on components will always keeps runtime pm
> references, unless you put DPMS_OFF.
>
> Cc: Badal Nilawar <badal.nilawar@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Francois Dugast <francois.dugast@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
[Gupta, Anshuman] looks good to me.
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
> tests/intel/xe_pm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c index
> 3f963bd9b..72d484163 100644
> --- a/tests/intel/xe_pm.c
> +++ b/tests/intel/xe_pm.c
> @@ -780,7 +780,9 @@ igt_main
> igt_describe("Validate mmap memory mappings with system
> region,"
> "when device along with parent bridge in d3");
> igt_subtest("d3-mmap-system") {
> + dpms_on_off(device, DRM_MODE_DPMS_OFF);
> test_mmap(device, system_memory(device.fd_xe), 0);
> + dpms_on_off(device, DRM_MODE_DPMS_ON);
> }
>
> igt_describe("Validate mmap memory mappings with vram
> region,"
> --
> 2.44.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH i-g-t 3/5] tests/intel/xe_pm: Also disable display for mmap_system test
2024-05-13 18:55 ` [PATCH i-g-t 3/5] tests/intel/xe_pm: Also disable display for mmap_system test Rodrigo Vivi
2024-05-14 6:12 ` Gupta, Anshuman
@ 2024-05-14 6:46 ` Nilawar, Badal
1 sibling, 0 replies; 13+ messages in thread
From: Nilawar, Badal @ 2024-05-14 6:46 UTC (permalink / raw)
To: Rodrigo Vivi, igt-dev, janga.rahul.kumar
Cc: intel-xe, Anshuman Gupta, Francois Dugast
On 14-05-2024 00:25, Rodrigo Vivi wrote:
> If display is enabled, the always-on components will always
> keeps runtime pm references, unless you put DPMS_OFF.
>
> Cc: Badal Nilawar <badal.nilawar@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Francois Dugast <francois.dugast@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> tests/intel/xe_pm.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
> index 3f963bd9b..72d484163 100644
> --- a/tests/intel/xe_pm.c
> +++ b/tests/intel/xe_pm.c
> @@ -780,7 +780,9 @@ igt_main
> igt_describe("Validate mmap memory mappings with system region,"
> "when device along with parent bridge in d3");
> igt_subtest("d3-mmap-system") {
> + dpms_on_off(device, DRM_MODE_DPMS_OFF);
> test_mmap(device, system_memory(device.fd_xe), 0);
> + dpms_on_off(device, DRM_MODE_DPMS_ON);
Rahul, please consider this change for mocs tests you added recently in
xe_pm.c.
Meanwhile this is
Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>
Regards,
Badal Nilawar
> }
>
> igt_describe("Validate mmap memory mappings with vram region,"
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH i-g-t 4/5] tests/intel/xe_pm: Only check the rpm resume after the first mmap operation
2024-05-13 18:55 [PATCH i-g-t 1/5] tests/intel/xe_pm: Update runtime pm conditions Rodrigo Vivi
2024-05-13 18:55 ` [PATCH i-g-t 2/5] lib/igt_pm: Fix and standardize IGT PM library documentation Rodrigo Vivi
2024-05-13 18:55 ` [PATCH i-g-t 3/5] tests/intel/xe_pm: Also disable display for mmap_system test Rodrigo Vivi
@ 2024-05-13 18:55 ` Rodrigo Vivi
2024-05-14 15:19 ` Nilawar, Badal
2024-05-13 18:55 ` [PATCH i-g-t 5/5] tests/intel/xe_pm: Convert mmap tests to use existing d3 helpers Rodrigo Vivi
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Rodrigo Vivi @ 2024-05-13 18:55 UTC (permalink / raw)
To: igt-dev; +Cc: intel-xe, Rodrigo Vivi, Badal Nilawar, Anshuman Gupta
The very first memory operation on a mmaped region after runtime suspend,
the device will be resumed if the memory used is vram.
However, after this first operation with page fault, the memory
can be migrated to the system memory.
During this migration, Xe kernel will get a notification at
xe_bo_move_notify, and the bo will be removed from the
vram_userfault list. Then, on the next suspend, we won't mark
bo for page fault and resume won't happen and the active count
won't increase. This is okay, since the bo is now in the system
memory we don't need to wake up the device. But the current
test is wrong, because it assumes that the bo keeps forever
in vram, what is the issue here.
Only checking the resume after the first operation is the
enough. But also, ensure that both read and write operations
are actually validated.
Cc: Badal Nilawar <badal.nilawar@intel.com>
Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
tests/intel/xe_pm.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
index 72d484163..81226a910 100644
--- a/tests/intel/xe_pm.c
+++ b/tests/intel/xe_pm.c
@@ -37,6 +37,11 @@
#define PREFETCH (0x1 << 1)
#define UNBIND_ALL (0x1 << 2)
+enum mem_op {
+ READ,
+ WRITE,
+};
+
typedef struct {
int fd_xe;
struct pci_device *pci_xe;
@@ -525,7 +530,8 @@ static void test_vram_d3cold_threshold(device_t device, int sysfs_fd)
*
* Functionality: pm-d3
*/
-static void test_mmap(device_t device, uint32_t placement, uint32_t flags)
+static void test_mmap(device_t device, uint32_t placement, uint32_t flags,
+ enum mem_op first_op)
{
size_t bo_size = 8192;
uint32_t *map = NULL;
@@ -563,8 +569,12 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags)
igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
active_time = igt_pm_get_runtime_active_time(device.pci_xe);
- for (i = 0; i < bo_size / sizeof(*map); i++)
- igt_assert(map[i] == MAGIC_1);
+ for (i = 0; i < bo_size / sizeof(*map); i++) {
+ if (first_op == READ)
+ igt_assert(map[i] == MAGIC_1);
+ else
+ map[i] = MAGIC_2;
+ }
/* dgfx page-fault on mmaping should wake the gpu */
if (xe_has_vram(device.fd_xe) && flags & DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)
@@ -574,12 +584,12 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags)
igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
active_time = igt_pm_get_runtime_active_time(device.pci_xe);
- for (i = 0; i < bo_size / sizeof(*map); i++)
- map[i] = MAGIC_2;
-
- if (xe_has_vram(device.fd_xe) && flags & DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)
- igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
- active_time);
+ for (i = 0; i < bo_size / sizeof(*map); i++) {
+ if (first_op == READ)
+ map[i] = MAGIC_2;
+ else
+ igt_assert(map[i] == MAGIC_2);
+ }
igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
@@ -781,7 +791,8 @@ igt_main
"when device along with parent bridge in d3");
igt_subtest("d3-mmap-system") {
dpms_on_off(device, DRM_MODE_DPMS_OFF);
- test_mmap(device, system_memory(device.fd_xe), 0);
+ test_mmap(device, system_memory(device.fd_xe), 0, READ);
+ test_mmap(device, system_memory(device.fd_xe), 0, WRITE);
dpms_on_off(device, DRM_MODE_DPMS_ON);
}
@@ -800,7 +811,10 @@ igt_main
/* Give some auto suspend delay to validate rpm active during page fault */
igt_pm_set_autosuspend_delay(device.pci_xe, 1000);
dpms_on_off(device, DRM_MODE_DPMS_OFF);
- test_mmap(device, vram_memory(device.fd_xe, 0), DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
+ test_mmap(device, vram_memory(device.fd_xe, 0),
+ DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM, READ);
+ test_mmap(device, vram_memory(device.fd_xe, 0),
+ DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM, WRITE);
dpms_on_off(device, DRM_MODE_DPMS_ON);
igt_pm_set_autosuspend_delay(device.pci_xe, delay_ms);
}
--
2.44.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH i-g-t 4/5] tests/intel/xe_pm: Only check the rpm resume after the first mmap operation
2024-05-13 18:55 ` [PATCH i-g-t 4/5] tests/intel/xe_pm: Only check the rpm resume after the first mmap operation Rodrigo Vivi
@ 2024-05-14 15:19 ` Nilawar, Badal
2024-05-14 16:39 ` Nilawar, Badal
0 siblings, 1 reply; 13+ messages in thread
From: Nilawar, Badal @ 2024-05-14 15:19 UTC (permalink / raw)
To: Rodrigo Vivi, igt-dev; +Cc: intel-xe, Anshuman Gupta
On 14-05-2024 00:25, Rodrigo Vivi wrote:
> The very first memory operation on a mmaped region after runtime suspend,
> the device will be resumed if the memory used is vram.
> However, after this first operation with page fault, the memory
> can be migrated to the system memory.
>
> During this migration, Xe kernel will get a notification at
> xe_bo_move_notify, and the bo will be removed from the
> vram_userfault list. Then, on the next suspend, we won't mark
> bo for page fault and resume won't happen and the active count
> won't increase. This is okay, since the bo is now in the system
> memory we don't need to wake up the device. But the current
> test is wrong, because it assumes that the bo keeps forever
> in vram, what is the issue here.
>
> Only checking the resume after the first operation is the
> enough. But also, ensure that both read and write operations
> are actually validated.
>
> Cc: Badal Nilawar <badal.nilawar@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> tests/intel/xe_pm.c | 36 +++++++++++++++++++++++++-----------
> 1 file changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
> index 72d484163..81226a910 100644
> --- a/tests/intel/xe_pm.c
> +++ b/tests/intel/xe_pm.c
> @@ -37,6 +37,11 @@
> #define PREFETCH (0x1 << 1)
> #define UNBIND_ALL (0x1 << 2)
>
> +enum mem_op {
> + READ,
> + WRITE,
> +};
> +
> typedef struct {
> int fd_xe;
> struct pci_device *pci_xe;
> @@ -525,7 +530,8 @@ static void test_vram_d3cold_threshold(device_t device, int sysfs_fd)
> *
> * Functionality: pm-d3
> */
> -static void test_mmap(device_t device, uint32_t placement, uint32_t flags)
> +static void test_mmap(device_t device, uint32_t placement, uint32_t flags,
> + enum mem_op first_op)
> {
> size_t bo_size = 8192;
> uint32_t *map = NULL;
> @@ -563,8 +569,12 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags)
> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> active_time = igt_pm_get_runtime_active_time(device.pci_xe);
>
> - for (i = 0; i < bo_size / sizeof(*map); i++)
> - igt_assert(map[i] == MAGIC_1);
> + for (i = 0; i < bo_size / sizeof(*map); i++) {
> + if (first_op == READ)
> + igt_assert(map[i] == MAGIC_1);
> + else
> + map[i] = MAGIC_2;
> + }
>
> /* dgfx page-fault on mmaping should wake the gpu */
> if (xe_has_vram(device.fd_xe) && flags & DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)
> @@ -574,12 +584,12 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags)
> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> active_time = igt_pm_get_runtime_active_time(device.pci_xe);
Above line can be removed.
>
> - for (i = 0; i < bo_size / sizeof(*map); i++)
> - map[i] = MAGIC_2;
> -
> - if (xe_has_vram(device.fd_xe) && flags & DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)
> - igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
> - active_time);
> + for (i = 0; i < bo_size / sizeof(*map); i++) {
> + if (first_op == READ)
> + map[i] = MAGIC_2;
> + else
> + igt_assert(map[i] == MAGIC_2);
> + }
>
> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
>
> @@ -781,7 +791,8 @@ igt_main
> "when device along with parent bridge in d3");
> igt_subtest("d3-mmap-system") {
> dpms_on_off(device, DRM_MODE_DPMS_OFF);
> - test_mmap(device, system_memory(device.fd_xe), 0);
> + test_mmap(device, system_memory(device.fd_xe), 0, READ);
> + test_mmap(device, system_memory(device.fd_xe), 0, WRITE);
> dpms_on_off(device, DRM_MODE_DPMS_ON);
> }
>
> @@ -800,7 +811,10 @@ igt_main
> /* Give some auto suspend delay to validate rpm active during page fault */
> igt_pm_set_autosuspend_delay(device.pci_xe, 1000);
> dpms_on_off(device, DRM_MODE_DPMS_OFF);
> - test_mmap(device, vram_memory(device.fd_xe, 0), DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
> + test_mmap(device, vram_memory(device.fd_xe, 0),
> + DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM, READ);
> + test_mmap(device, vram_memory(device.fd_xe, 0),
> + DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM, WRITE);With a comment this is
Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>
Regards,
Badal
> dpms_on_off(device, DRM_MODE_DPMS_ON);
> igt_pm_set_autosuspend_delay(device.pci_xe, delay_ms);
> }
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH i-g-t 4/5] tests/intel/xe_pm: Only check the rpm resume after the first mmap operation
2024-05-14 15:19 ` Nilawar, Badal
@ 2024-05-14 16:39 ` Nilawar, Badal
0 siblings, 0 replies; 13+ messages in thread
From: Nilawar, Badal @ 2024-05-14 16:39 UTC (permalink / raw)
To: Rodrigo Vivi, igt-dev; +Cc: intel-xe, Anshuman Gupta
On 14-05-2024 20:49, Nilawar, Badal wrote:
>
>
> On 14-05-2024 00:25, Rodrigo Vivi wrote:
>> The very first memory operation on a mmaped region after runtime suspend,
>> the device will be resumed if the memory used is vram.
>> However, after this first operation with page fault, the memory
>> can be migrated to the system memory.
This probably due to xe_bo_evict_all() in D3 cold.
>>
>> During this migration, Xe kernel will get a notification at
>> xe_bo_move_notify, and the bo will be removed from the
>> vram_userfault list. Then, on the next suspend, we won't mark
>> bo for page fault and resume won't happen and the active count
>> won't increase. This is okay, since the bo is now in the system
>> memory we don't need to wake up the device. But the current
>> test is wrong, because it assumes that the bo keeps forever
>> in vram, what is the issue here.
Not completely wrong in D3 Hot.
>>
>> Only checking the resume after the first operation is the
>> enough. But also, ensure that both read and write operations
>> are actually validated.
Should we keep "first operation" check only for D3 cold and previous
logic for D3 hot? May be combine patch 4 and 5 to handle this.
Regards,
Badal
>>
>> Cc: Badal Nilawar <badal.nilawar@intel.com>
>> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>> tests/intel/xe_pm.c | 36 +++++++++++++++++++++++++-----------
>> 1 file changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
>> index 72d484163..81226a910 100644
>> --- a/tests/intel/xe_pm.c
>> +++ b/tests/intel/xe_pm.c
>> @@ -37,6 +37,11 @@
>> #define PREFETCH (0x1 << 1)
>> #define UNBIND_ALL (0x1 << 2)
>> +enum mem_op {
>> + READ,
>> + WRITE,
>> +};
>> +
>> typedef struct {
>> int fd_xe;
>> struct pci_device *pci_xe;
>> @@ -525,7 +530,8 @@ static void test_vram_d3cold_threshold(device_t
>> device, int sysfs_fd)
>> *
>> * Functionality: pm-d3
>> */
>> -static void test_mmap(device_t device, uint32_t placement, uint32_t
>> flags)
>> +static void test_mmap(device_t device, uint32_t placement, uint32_t
>> flags,
>> + enum mem_op first_op)
>> {
>> size_t bo_size = 8192;
>> uint32_t *map = NULL;
>> @@ -563,8 +569,12 @@ static void test_mmap(device_t device, uint32_t
>> placement, uint32_t flags)
>>
>> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
>> active_time = igt_pm_get_runtime_active_time(device.pci_xe);
>> - for (i = 0; i < bo_size / sizeof(*map); i++)
>> - igt_assert(map[i] == MAGIC_1);
>> + for (i = 0; i < bo_size / sizeof(*map); i++) {
>> + if (first_op == READ)
>> + igt_assert(map[i] == MAGIC_1);
>> + else
>> + map[i] = MAGIC_2;
>> + }
>> /* dgfx page-fault on mmaping should wake the gpu */
>> if (xe_has_vram(device.fd_xe) && flags &
>> DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)
>> @@ -574,12 +584,12 @@ static void test_mmap(device_t device, uint32_t
>> placement, uint32_t flags)
>>
>> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
>> active_time = igt_pm_get_runtime_active_time(device.pci_xe);
> Above line can be removed.
>> - for (i = 0; i < bo_size / sizeof(*map); i++)
>> - map[i] = MAGIC_2;
>> -
>> - if (xe_has_vram(device.fd_xe) && flags &
>> DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)
>> - igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
>> - active_time);
>> + for (i = 0; i < bo_size / sizeof(*map); i++) {
>> + if (first_op == READ)
>> + map[i] = MAGIC_2;
>> + else
>> + igt_assert(map[i] == MAGIC_2);
>> + }
>>
>> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
>> @@ -781,7 +791,8 @@ igt_main
>> "when device along with parent bridge in d3");
>> igt_subtest("d3-mmap-system") {
>> dpms_on_off(device, DRM_MODE_DPMS_OFF);
>> - test_mmap(device, system_memory(device.fd_xe), 0);
>> + test_mmap(device, system_memory(device.fd_xe), 0, READ);
>> + test_mmap(device, system_memory(device.fd_xe), 0, WRITE);
>> dpms_on_off(device, DRM_MODE_DPMS_ON);
>> }
>> @@ -800,7 +811,10 @@ igt_main
>> /* Give some auto suspend delay to validate rpm active
>> during page fault */
>> igt_pm_set_autosuspend_delay(device.pci_xe, 1000);
>> dpms_on_off(device, DRM_MODE_DPMS_OFF);
>> - test_mmap(device, vram_memory(device.fd_xe, 0),
>> DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM);
>> + test_mmap(device, vram_memory(device.fd_xe, 0),
>> + DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM, READ);
>> + test_mmap(device, vram_memory(device.fd_xe, 0),
>> + DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM,
>> WRITE);With a comment this is
> Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>
>
> Regards,
> Badal
>> dpms_on_off(device, DRM_MODE_DPMS_ON);
>> igt_pm_set_autosuspend_delay(device.pci_xe, delay_ms);
>> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH i-g-t 5/5] tests/intel/xe_pm: Convert mmap tests to use existing d3 helpers
2024-05-13 18:55 [PATCH i-g-t 1/5] tests/intel/xe_pm: Update runtime pm conditions Rodrigo Vivi
` (2 preceding siblings ...)
2024-05-13 18:55 ` [PATCH i-g-t 4/5] tests/intel/xe_pm: Only check the rpm resume after the first mmap operation Rodrigo Vivi
@ 2024-05-13 18:55 ` Rodrigo Vivi
2024-05-13 19:01 ` ✗ CI.Patch_applied: failure for series starting with [i-g-t,1/5] tests/intel/xe_pm: Update runtime pm conditions Patchwork
2024-05-16 16:49 ` [PATCH i-g-t 1/5] " Francois Dugast
5 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2024-05-13 18:55 UTC (permalink / raw)
To: igt-dev; +Cc: intel-xe, Rodrigo Vivi, Badal Nilawar, Anshuman Gupta
Standardize d3 setup and ensure that it tests with D3cold and
D3hot.
Cc: Badal Nilawar <badal.nilawar@intel.com>
Cc: Anshuman Gupta <anshuman.gupta@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
tests/intel/xe_pm.c | 80 ++++++++++++++++++++++++---------------------
1 file changed, 42 insertions(+), 38 deletions(-)
diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
index 81226a910..4aeddaea8 100644
--- a/tests/intel/xe_pm.c
+++ b/tests/intel/xe_pm.c
@@ -519,19 +519,25 @@ static void test_vram_d3cold_threshold(device_t device, int sysfs_fd)
}
/**
- * SUBTEST: d3-mmap-%s
+ * SUBTEST: %s-mmap-%s
* Description:
* Validate mmap memory mapping with d3 state, for %arg[1] region,
* if supported by device.
+ *
* arg[1]:
*
+ * @d3hot: d3hot
+ * @d3cold: d3cold
+ *
+ * arg[2]:
+ *
* @vram: vram region
* @system: system region
*
* Functionality: pm-d3
*/
static void test_mmap(device_t device, uint32_t placement, uint32_t flags,
- enum mem_op first_op)
+ enum mem_op first_op, enum igt_acpi_d_state d_state)
{
size_t bo_size = 8192;
uint32_t *map = NULL;
@@ -540,7 +546,7 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags,
igt_require_f(placement, "Device doesn't support such memory region\n");
- igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+ igt_assert(in_d3(device, d_state));
active_time = igt_pm_get_runtime_active_time(device.pci_xe);
bo_size = ALIGN(bo_size, xe_get_default_alignment(device.fd_xe));
@@ -566,7 +572,7 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags,
close(fw_handle);
sleep(1);
- igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+ igt_assert(in_d3(device, d_state));
active_time = igt_pm_get_runtime_active_time(device.pci_xe);
for (i = 0; i < bo_size / sizeof(*map); i++) {
@@ -581,7 +587,7 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags,
igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
active_time);
- igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+ igt_assert(in_d3(device, d_state));
active_time = igt_pm_get_runtime_active_time(device.pci_xe);
for (i = 0; i < bo_size / sizeof(*map); i++) {
@@ -591,7 +597,7 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags,
igt_assert(map[i] == MAGIC_2);
}
- igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+ igt_assert(in_d3(device, d_state));
/* Runtime resume and check the pattern */
fw_handle = igt_debugfs_open(device.fd_xe, "forcewake_all", O_RDONLY);
@@ -772,6 +778,36 @@ igt_main
NO_SUSPEND, d->state, 0);
cleanup_d3(device);
}
+
+ igt_describe_f("Validate mmap memory mappings with system region,"
+ "when device along with parent bridge in %s", d->name);
+ igt_subtest_f("%s-mmap-system", d->name) {
+ igt_assert(setup_d3(device, d->state));
+ test_mmap(device, system_memory(device.fd_xe), 0,
+ READ, d->state);
+ test_mmap(device, system_memory(device.fd_xe), 0,
+ WRITE, d->state);
+ cleanup_d3(device);
+ }
+
+ igt_describe_f("Validate mmap memory mappings with vram region,"
+ "when device along with parent bridge in %s", d->name);
+ igt_subtest_f("%s-mmap-vram", d->name) {
+ int delay_ms = igt_pm_get_autosuspend_delay(device.pci_xe);
+
+ /* Give some auto suspend delay to validate rpm active during page fault */
+ igt_pm_set_autosuspend_delay(device.pci_xe, 1000);
+ igt_assert(setup_d3(device, d->state));
+ test_mmap(device, vram_memory(device.fd_xe, 0),
+ DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM,
+ READ, d->state);
+ test_mmap(device, vram_memory(device.fd_xe, 0),
+ DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM,
+ WRITE, d->state);
+ cleanup_d3(device);
+
+ igt_pm_set_autosuspend_delay(device.pci_xe, delay_ms);
+ }
}
igt_subtest_group {
@@ -787,38 +823,6 @@ igt_main
test_vram_d3cold_threshold(device, sysfs_fd);
}
- igt_describe("Validate mmap memory mappings with system region,"
- "when device along with parent bridge in d3");
- igt_subtest("d3-mmap-system") {
- dpms_on_off(device, DRM_MODE_DPMS_OFF);
- test_mmap(device, system_memory(device.fd_xe), 0, READ);
- test_mmap(device, system_memory(device.fd_xe), 0, WRITE);
- dpms_on_off(device, DRM_MODE_DPMS_ON);
- }
-
- igt_describe("Validate mmap memory mappings with vram region,"
- "when device along with parent bridge in d3");
- igt_subtest("d3-mmap-vram") {
- int delay_ms;
-
- if (device.pci_root != device.pci_xe) {
- igt_pm_enable_pci_card_runtime_pm(device.pci_root, NULL);
- igt_pm_set_d3cold_allowed(device.pci_slot_name, 1);
- }
-
- delay_ms = igt_pm_get_autosuspend_delay(device.pci_xe);
-
- /* Give some auto suspend delay to validate rpm active during page fault */
- igt_pm_set_autosuspend_delay(device.pci_xe, 1000);
- dpms_on_off(device, DRM_MODE_DPMS_OFF);
- test_mmap(device, vram_memory(device.fd_xe, 0),
- DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM, READ);
- test_mmap(device, vram_memory(device.fd_xe, 0),
- DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM, WRITE);
- dpms_on_off(device, DRM_MODE_DPMS_ON);
- igt_pm_set_autosuspend_delay(device.pci_xe, delay_ms);
- }
-
igt_subtest("mocs_suspend_resume")
test_mocs_suspend_resume(device, 1, 0);
}
--
2.44.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* ✗ CI.Patch_applied: failure for series starting with [i-g-t,1/5] tests/intel/xe_pm: Update runtime pm conditions
2024-05-13 18:55 [PATCH i-g-t 1/5] tests/intel/xe_pm: Update runtime pm conditions Rodrigo Vivi
` (3 preceding siblings ...)
2024-05-13 18:55 ` [PATCH i-g-t 5/5] tests/intel/xe_pm: Convert mmap tests to use existing d3 helpers Rodrigo Vivi
@ 2024-05-13 19:01 ` Patchwork
2024-05-13 20:03 ` Rodrigo Vivi
2024-05-16 16:49 ` [PATCH i-g-t 1/5] " Francois Dugast
5 siblings, 1 reply; 13+ messages in thread
From: Patchwork @ 2024-05-13 19:01 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-xe
== Series Details ==
Series: series starting with [i-g-t,1/5] tests/intel/xe_pm: Update runtime pm conditions
URL : https://patchwork.freedesktop.org/series/133566/
State : failure
== Summary ==
=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: 3d00e04692d4 drm-tip: 2024y-05m-13d-17h-13m-55s UTC integration manifest
=== git am output follows ===
error: lib/igt_pm.c: does not exist in index
error: lib/igt_pm.h: does not exist in index
error: tests/intel/xe_pm.c: does not exist in index
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: tests/intel/xe_pm: Update runtime pm conditions
Patch failed at 0001 tests/intel/xe_pm: Update runtime pm conditions
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: ✗ CI.Patch_applied: failure for series starting with [i-g-t,1/5] tests/intel/xe_pm: Update runtime pm conditions
2024-05-13 19:01 ` ✗ CI.Patch_applied: failure for series starting with [i-g-t,1/5] tests/intel/xe_pm: Update runtime pm conditions Patchwork
@ 2024-05-13 20:03 ` Rodrigo Vivi
0 siblings, 0 replies; 13+ messages in thread
From: Rodrigo Vivi @ 2024-05-13 20:03 UTC (permalink / raw)
To: intel-xe
On Mon, May 13, 2024 at 07:01:42PM -0000, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [i-g-t,1/5] tests/intel/xe_pm: Update runtime pm conditions
> URL : https://patchwork.freedesktop.org/series/133566/
> State : failure
>
> == Summary ==
>
> === Applying kernel patches on branch 'drm-tip' with base: ===
> Base commit: 3d00e04692d4 drm-tip: 2024y-05m-13d-17h-13m-55s UTC integration manifest
CI should stop trying to apply patches with subject-prefix containing "PATCH i-g-t" on a kernel tree.
> === git am output follows ===
> error: lib/igt_pm.c: does not exist in index
> error: lib/igt_pm.h: does not exist in index
> error: tests/intel/xe_pm.c: does not exist in index
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> Applying: tests/intel/xe_pm: Update runtime pm conditions
> Patch failed at 0001 tests/intel/xe_pm: Update runtime pm conditions
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH i-g-t 1/5] tests/intel/xe_pm: Update runtime pm conditions
2024-05-13 18:55 [PATCH i-g-t 1/5] tests/intel/xe_pm: Update runtime pm conditions Rodrigo Vivi
` (4 preceding siblings ...)
2024-05-13 19:01 ` ✗ CI.Patch_applied: failure for series starting with [i-g-t,1/5] tests/intel/xe_pm: Update runtime pm conditions Patchwork
@ 2024-05-16 16:49 ` Francois Dugast
5 siblings, 0 replies; 13+ messages in thread
From: Francois Dugast @ 2024-05-16 16:49 UTC (permalink / raw)
To: Rodrigo Vivi
Cc: igt-dev, intel-xe, Kamil Konieczny, Badal Nilawar, Anshuman Gupta
On Mon, May 13, 2024 at 02:55:14PM -0400, Rodrigo Vivi wrote:
> Xe is no longer holding a runtime pm reference for the life
> of a VM or exec_queue.
>
> Also, IGT changes autosuspend time to a minimal time, so we
> cannot guarantee that rpm is still suspended after the execution
> has finished.
>
> So, the reference usage is not a reliable reference.
>
> Hence, start using runtime_active_time as the indicator
> that runtime_pm resumed upon our actions.
>
> v2: Usage of runtime_active_pm and inclusion of mmap tests.
> v3: Add doc for the exported lib function (Kamil)
>
> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Cc: Badal Nilawar <badal.nilawar@intel.com>
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Francois Dugast <francois.dugast@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Seeing failures and getting some "ACPI Error: " in dmesg when running
tests for a state which is not the platform default, for example s3-*
on a laptop which defaults to s2idle. But this is not caused by this
patch, which LGTM, so:
Reviewed-by: Francois Dugast <francois.dugast@intel.com>
> ---
> lib/igt_pm.c | 24 ++++++++++++++
> lib/igt_pm.h | 1 +
> tests/intel/xe_pm.c | 77 +++++++++++++++++----------------------------
> 3 files changed, 53 insertions(+), 49 deletions(-)
>
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index fe7692960..928b72685 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -1412,6 +1412,30 @@ int igt_pm_get_runtime_suspended_time(struct pci_device *pci_dev)
> return -1;
> }
>
> +/**
> + * igt_pm_get_runtime_active_time:
> + * @pci_dev: PCI device struct
> + *
> + * Return: The total time that the device has been active.
> + */
> +int igt_pm_get_runtime_active_time(struct pci_device *pci_dev)
> +{
> + char time_str[64];
> + int time, time_fd;
> +
> + time_fd = igt_pm_get_power_attr_fd_rdonly(pci_dev, "runtime_active_time");
> + if (igt_pm_read_power_attr(time_fd, time_str, 64, false)) {
> + igt_assert(sscanf(time_str, "%d", &time) > 0);
> +
> + igt_debug("runtime active time for PCI '%04x:%02x:%02x.%01x' = %d\n",
> + pci_dev->domain, pci_dev->bus, pci_dev->dev, pci_dev->func, time);
> +
> + return time;
> + }
> +
> + return -1;
> +}
> +
> /**
> * igt_pm_get_runtime_usage:
> * @pci_dev: pci device
> diff --git a/lib/igt_pm.h b/lib/igt_pm.h
> index 91ee05cd1..b71f7c440 100644
> --- a/lib/igt_pm.h
> +++ b/lib/igt_pm.h
> @@ -94,6 +94,7 @@ void igt_pm_print_pci_card_runtime_status(void);
> bool i915_is_slpc_enabled_gt(int drm_fd, int gt);
> bool i915_is_slpc_enabled(int drm_fd);
> int igt_pm_get_runtime_suspended_time(struct pci_device *pci_dev);
> +int igt_pm_get_runtime_active_time(struct pci_device *pci_dev);
> int igt_pm_get_runtime_usage(struct pci_device *pci_dev);
> void igt_pm_ignore_slpc_efficient_freq(int i915, int gtfd, bool val);
>
> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
> index e81a75d88..3f963bd9b 100644
> --- a/tests/intel/xe_pm.c
> +++ b/tests/intel/xe_pm.c
> @@ -184,34 +184,6 @@ static bool in_d3(device_t device, enum igt_acpi_d_state state)
> return true;
> }
>
> -static bool out_of_d3(device_t device, enum igt_acpi_d_state state)
> -{
> - uint16_t val;
> -
> - /* Runtime resume needs to be immediate action without any wait */
> - if (runtime_usage_available(device.pci_xe) &&
> - igt_pm_get_runtime_usage(device.pci_xe) <= 0)
> - return false;
> -
> - if (igt_get_runtime_pm_status() != IGT_RUNTIME_PM_STATUS_ACTIVE)
> - return false;
> -
> - switch (state) {
> - case IGT_ACPI_D3Hot:
> - igt_assert_eq(pci_device_cfg_read_u16(device.pci_xe,
> - &val, 0xd4), 0);
> - return (val & 0x3) == 0;
> - case IGT_ACPI_D3Cold:
> - return igt_pm_get_acpi_real_d_state(device.pci_root) ==
> - IGT_ACPI_D0;
> - default:
> - igt_info("Invalid D3 State\n");
> - igt_assert(0);
> - }
> -
> - return true;
> -}
> -
> static void close_fw_handle(int sig)
> {
> if (fw_handle < 0)
> @@ -326,27 +298,27 @@ test_exec(device_t device, struct drm_xe_engine_class_instance *eci,
> uint64_t pad;
> uint32_t data;
> } *data;
> - int i, b, rpm_usage;
> + int i, b, active_time;
> bool check_rpm = (d_state == IGT_ACPI_D3Hot ||
> d_state == IGT_ACPI_D3Cold);
>
> igt_assert(n_exec_queues <= MAX_N_EXEC_QUEUES);
> igt_assert(n_execs > 0);
>
> - if (check_rpm)
> + if (check_rpm) {
> igt_assert(in_d3(device, d_state));
> + active_time = igt_pm_get_runtime_active_time(device.pci_xe);
> + }
>
> vm = xe_vm_create(device.fd_xe, 0, 0);
>
> if (check_rpm)
> - igt_assert(out_of_d3(device, d_state));
> + igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
> + active_time);
>
> bo_size = sizeof(*data) * n_execs;
> bo_size = xe_bb_size(device.fd_xe, bo_size);
>
> - if (check_rpm && runtime_usage_available(device.pci_xe))
> - rpm_usage = igt_pm_get_runtime_usage(device.pci_xe);
> -
> if (flags & USERPTR) {
> data = aligned_alloc(xe_get_default_alignment(device.fd_xe), bo_size);
> memset(data, 0, bo_size);
> @@ -384,8 +356,10 @@ test_exec(device_t device, struct drm_xe_engine_class_instance *eci,
> xe_vm_prefetch_async(device.fd_xe, vm, bind_exec_queues[0], 0, addr,
> bo_size, sync, 1, 0);
>
> - if (check_rpm && runtime_usage_available(device.pci_xe))
> - igt_assert(igt_pm_get_runtime_usage(device.pci_xe) > rpm_usage);
> + if (check_rpm) {
> + igt_assert(in_d3(device, d_state));
> + active_time = igt_pm_get_runtime_active_time(device.pci_xe);
> + }
>
> for (i = 0; i < n_execs; i++) {
> uint64_t batch_offset = (char *)&data[i].batch - (char *)data;
> @@ -429,9 +403,6 @@ test_exec(device_t device, struct drm_xe_engine_class_instance *eci,
> igt_assert(syncobj_wait(device.fd_xe, &sync[0].handle, 1, INT64_MAX, 0,
> NULL));
>
> - if (check_rpm && runtime_usage_available(device.pci_xe))
> - rpm_usage = igt_pm_get_runtime_usage(device.pci_xe);
> -
> sync[0].flags |= DRM_XE_SYNC_FLAG_SIGNAL;
> if (n_vmas > 1)
> xe_vm_unbind_all_async(device.fd_xe, vm, 0, bo, sync, 1);
> @@ -459,15 +430,13 @@ NULL));
> free(data);
> }
>
> - if (check_rpm && runtime_usage_available(device.pci_xe))
> - igt_assert(igt_pm_get_runtime_usage(device.pci_xe) < rpm_usage);
> - if (check_rpm)
> - igt_assert(out_of_d3(device, d_state));
> -
> xe_vm_destroy(device.fd_xe, vm);
>
> - if (check_rpm)
> + if (check_rpm) {
> + igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
> + active_time);
> igt_assert(in_d3(device, d_state));
> + }
> }
>
> /**
> @@ -561,10 +530,13 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags)
> size_t bo_size = 8192;
> uint32_t *map = NULL;
> uint32_t bo;
> - int i;
> + int i, active_time;
>
> igt_require_f(placement, "Device doesn't support such memory region\n");
>
> + igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> + active_time = igt_pm_get_runtime_active_time(device.pci_xe);
> +
> bo_size = ALIGN(bo_size, xe_get_default_alignment(device.fd_xe));
>
> bo = xe_bo_create(device.fd_xe, 0, bo_size, placement, flags);
> @@ -575,7 +547,8 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags)
> fw_handle = igt_debugfs_open(device.fd_xe, "forcewake_all", O_RDONLY);
>
> igt_assert(fw_handle >= 0);
> - igt_assert(igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_ACTIVE);
> + igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
> + active_time);
>
> for (i = 0; i < bo_size / sizeof(*map); i++)
> map[i] = MAGIC_1;
> @@ -585,22 +558,28 @@ static void test_mmap(device_t device, uint32_t placement, uint32_t flags)
>
> /* Runtime suspend and validate the pattern and changed the pattern */
> close(fw_handle);
> + sleep(1);
> +
> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> + active_time = igt_pm_get_runtime_active_time(device.pci_xe);
>
> for (i = 0; i < bo_size / sizeof(*map); i++)
> igt_assert(map[i] == MAGIC_1);
>
> /* dgfx page-fault on mmaping should wake the gpu */
> if (xe_has_vram(device.fd_xe) && flags & DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)
> - igt_assert(igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_ACTIVE);
> + igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
> + active_time);
>
> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> + active_time = igt_pm_get_runtime_active_time(device.pci_xe);
>
> for (i = 0; i < bo_size / sizeof(*map); i++)
> map[i] = MAGIC_2;
>
> if (xe_has_vram(device.fd_xe) && flags & DRM_XE_GEM_CREATE_FLAG_NEEDS_VISIBLE_VRAM)
> - igt_assert(igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_ACTIVE);
> + igt_assert(igt_pm_get_runtime_active_time(device.pci_xe) >
> + active_time);
>
> igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
>
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread