[PATCH i-g-t 2/5] lib/igt_pm: Fix and standardize IGT PM library documentation
Riana Tauro
riana.tauro at intel.com
Tue May 14 06:49:03 UTC 2024
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 at linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at 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 at 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)
> {
More information about the igt-dev
mailing list