On Tue, May 02, 2023 at 06:18:56PM -0700, Sathyanarayanan Kuppuswamy wrote: > > > 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? > There is no bug associated. I found this through my dry run of the code. > > Signed-off-by: Ajay Agarwal <ajayagarwal@xxxxxxxxxx> > > --- > > 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
