On Thu, Aug 03, 2023 at 08:02:29PM -0500, Mario Limonciello wrote:
> Since commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
> PCIe ports from modern machines (>=2015) are allowed to be put into D3 by
> storing a value to the `bridge_d3` variable in the `struct pci_dev`
> structure.
>
> pci_power_manageable() uses this variable to indicate a PCIe port can
> enter D3.
> pci_pm_suspend_noirq() uses the return from pci_power_manageable() to
> decide whether to try to put a device into its target state for a sleep
> cycle via pci_prepare_to_sleep().
>
> For devices that support D3, the target state is selected by this policy:
> 1. If platform_pci_power_manageable():
> Use platform_pci_choose_state()
> 2. If the device is armed for wakeup:
> Select the deepest D-state that supports a PME.
> 3. Else:
> Use D3hot.
>
> Devices are considered power manageable by the platform when they have
> one or more objects described in the table in section 7.3 of the ACPI 6.5
> specification.
>
> When devices are not considered power manageable; specs are ambiguous as
> to what should happen. In this situation Windows 11 leaves PCIe
> ports in D0 while Linux puts them into D3 due to the above mentioned
> commit.
>
> In Windows systems that support Modern Standby specify hardware
> pre-conditions for the SoC to achieve the lowest power state by device
> constraints in a SOC specific "Power Engine Plugin" (PEP) [2] [3].
> They can be marked as disabled or enabled and when enabled can specify
> the minimum power state required for an ACPI device.
>
> When it is ambiguous what should happen, adjust the logic for
> pci_target_state() to check whether a device constraint is present
> and enabled.
> * If power manageable by ACPI use this to get to select target state
> * If a device constraint is present but disabled then choose D0
> * If a device constraint is present and enabled then use it
> * If a device constraint is not present, then continue to existing
> logic (if marked for wakeup use deepest state that PME works)
> * If not marked for wakeup choose D3hot
...
> +/**
> + * acpi_get_lps0_constraint - get any LPS0 constraint for a device
> + * @dev: device to get constraint for
> + *
> + * If a constraint has been specified in the _DSM method for the device,
> + * and the constraint is enabled return it. If the constraint is disabled,
> + * return 0. Otherwise, return -ENODEV.
> + */
I believe you get a kernel-doc warning. Always test kernel doc with
scripts/kernel-doc -v -none -Wall ...your file...
...
> +/**
> + * acpi_pci_device_constraint - determine if the platform has a contraint for the device
> + * @dev: PCI device to check
> + * @result (out): the constraint specified by the platform
> + *
> + * If the platform has specified a constraint for a device, this function will
> + * return 0 and set @result to the constraint.
> + * Otherwise, it will return an error code.
> + */
Ditto.
...
> +int acpi_pci_device_constraint(struct pci_dev *dev, int *result)
> +{
> + int constraint;
> +
> + constraint = acpi_get_lps0_constraint(&dev->dev);
> + pci_dbg(dev, "ACPI device constraint: %d\n", constraint);
Does it make sense before the below check? Why can we be interested in the
_exact_ negative values? (Note that non-printing is already a sign that either
we don't call this or have negative constraint.)
> + if (constraint < 0)
> + return constraint;
> + *result = constraint;
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
