Re: [PATCH v2 3/5] PCI/ASPM: Set ASPM_STATE_L1 when driver enables L1ss

[Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>]

On 5/2/23 12:31 PM, Ajay Agarwal wrote:
> Currently the aspm driver does not set ASPM_STATE_L1 bit in
> aspm_default when the caller requests L1SS ASPM state. This will
> lead to pcie_config_aspm_link() not enabling the requested L1SS
> state. Set ASPM_STATE_L1 when driver enables L1ss.
> 

Is there a bug associated with this issue?

> Signed-off-by: Ajay Agarwal <ajayagarwal@google.com>
> ---
> Changelog since v1:
>  - Break down the L1 and L1ss handling into separate patches
> 
>  drivers/pci/pcie/aspm.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 4ad0bf5d5838..7c9935f331f1 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -1171,14 +1171,15 @@ int pci_enable_link_state(struct pci_dev *pdev, int state)
>  		link->aspm_default |= ASPM_STATE_L0S;
>  	if (state & PCIE_LINK_STATE_L1)
>  		link->aspm_default |= ASPM_STATE_L1;
> +	/* L1 PM substates require L1 */
>  	if (state & PCIE_LINK_STATE_L1_1)
> -		link->aspm_default |= ASPM_STATE_L1_1;
> +		link->aspm_default |= ASPM_STATE_L1_1 | ASPM_STATE_L1;
>  	if (state & PCIE_LINK_STATE_L1_2)
> -		link->aspm_default |= ASPM_STATE_L1_2;
> +		link->aspm_default |= ASPM_STATE_L1_2 | ASPM_STATE_L1;
>  	if (state & PCIE_LINK_STATE_L1_1_PCIPM)
> -		link->aspm_default |= ASPM_STATE_L1_1_PCIPM;
> +		link->aspm_default |= ASPM_STATE_L1_1_PCIPM | ASPM_STATE_L1;
>  	if (state & PCIE_LINK_STATE_L1_2_PCIPM)
> -		link->aspm_default |= ASPM_STATE_L1_2_PCIPM;
> +		link->aspm_default |= ASPM_STATE_L1_2_PCIPM | ASPM_STATE_L1;
>  	pcie_config_aspm_link(link, policy_to_aspm_state(link));
>  
>  	link->clkpm_default = (state & PCIE_LINK_STATE_CLKPM) ? 1 : 0;

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer