[PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited

Linux PCI subsystem development
 help / color / mirror / Atom feed

* [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited
@ 2022-04-05  9:38 Mika Westerberg
  2022-04-05 12:49 ` Rodrigo Vivi
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Mika Westerberg @ 2022-04-05  9:38 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Rodrigo Vivi, Mika Westerberg, linux-pci

Intel DG2 discrete graphics PCIe endpoints hard-code their acceptable L1
ASPM latency to be < 1us even though the hardware actually supports
higher latencies (> 64 us) just fine. In order to allow the links to go
into L1 and save power, quirk the acceptable L1 ASPM latency for these
endpoints to be unlimited.

Note this does not have any effect unless the user requested the kernel
to enable ASPM in the first place (by default we don't enable it). This
is done with "pcie_aspm=force pcie_aspm.policy=powersupsersave" command
line parameters.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/quirks.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index da829274fc66..e97b5daa00eb 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5895,3 +5895,47 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1533, rom_bar_overlap_defect);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect);
+
+#ifdef CONFIG_PCIEASPM
+/*
+ * Intel DG2 graphics card has hard-coded acceptable L1 latency that is
+ * too low which prevents ASPM to be enabled. It does support ASPM L1
+ * and tolerates higher latencies so quirk it to be unlimited.
+ */
+static void quirk_aspm_accepted_l1_latency(struct pci_dev *dev)
+{
+	if ((dev->devcap & PCI_EXP_DEVCAP_L1) >> 9 < 7) {
+		u32 devcap = dev->devcap;
+
+		dev->devcap |= 7 << 9;
+		pci_info(dev, "quirking devcap for L1 accepted latency 0x%08x -> 0x%08x\n",
+			 devcap, dev->devcap);
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f80, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f81, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f82, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f83, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f84, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f85, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f86, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f87, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f88, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5690, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5691, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5692, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5693, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5694, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5695, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a0, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a1, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a2, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a3, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a4, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a5, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a6, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b0, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b1, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c0, quirk_aspm_accepted_l1_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c1, quirk_aspm_accepted_l1_latency);
+#endif
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited
  2022-04-05  9:38 [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited Mika Westerberg
@ 2022-04-05 12:49 ` Rodrigo Vivi
  2022-04-05 16:01 ` Bjorn Helgaas
  2022-04-07 17:06 ` Bjorn Helgaas
  2 siblings, 0 replies; 7+ messages in thread
From: Rodrigo Vivi @ 2022-04-05 12:49 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: Bjorn Helgaas, linux-pci

On Tue, Apr 05, 2022 at 12:38:10PM +0300, Mika Westerberg wrote:
> Intel DG2 discrete graphics PCIe endpoints hard-code their acceptable L1
> ASPM latency to be < 1us even though the hardware actually supports
> higher latencies (> 64 us) just fine. In order to allow the links to go
> into L1 and save power, quirk the acceptable L1 ASPM latency for these
> endpoints to be unlimited.
> 
> Note this does not have any effect unless the user requested the kernel
> to enable ASPM in the first place (by default we don't enable it). This
> is done with "pcie_aspm=force pcie_aspm.policy=powersupsersave" command
> line parameters.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pci/quirks.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index da829274fc66..e97b5daa00eb 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5895,3 +5895,47 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1533, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect);
> +
> +#ifdef CONFIG_PCIEASPM
> +/*
> + * Intel DG2 graphics card has hard-coded acceptable L1 latency that is
> + * too low which prevents ASPM to be enabled. It does support ASPM L1
> + * and tolerates higher latencies so quirk it to be unlimited.
> + */
> +static void quirk_aspm_accepted_l1_latency(struct pci_dev *dev)
> +{
> +	if ((dev->devcap & PCI_EXP_DEVCAP_L1) >> 9 < 7) {
> +		u32 devcap = dev->devcap;
> +
> +		dev->devcap |= 7 << 9;
> +		pci_info(dev, "quirking devcap for L1 accepted latency 0x%08x -> 0x%08x\n",
> +			 devcap, dev->devcap);
> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f80, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f81, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f82, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f83, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f84, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f85, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f86, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f87, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f88, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5690, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5691, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5692, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5693, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5694, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5695, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a0, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a1, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a2, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a3, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a4, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a5, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a6, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b0, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b1, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c0, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c1, quirk_aspm_accepted_l1_latency);
> +#endif

This matches our expectations and IDs.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited
  2022-04-05  9:38 [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited Mika Westerberg
  2022-04-05 12:49 ` Rodrigo Vivi
@ 2022-04-05 16:01 ` Bjorn Helgaas
  2022-04-05 16:23   ` Vivi, Rodrigo
  2022-04-05 16:24   ` Mika Westerberg
  2022-04-07 17:06 ` Bjorn Helgaas
  2 siblings, 2 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2022-04-05 16:01 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: Bjorn Helgaas, Rodrigo Vivi, linux-pci

On Tue, Apr 05, 2022 at 12:38:10PM +0300, Mika Westerberg wrote:
> Intel DG2 discrete graphics PCIe endpoints hard-code their acceptable L1
> ASPM latency to be < 1us even though the hardware actually supports
> higher latencies (> 64 us) just fine. In order to allow the links to go
> into L1 and save power, quirk the acceptable L1 ASPM latency for these
> endpoints to be unlimited.

Is there a plan to fix this in future DG2 hardware/firmware?
Obviously the point of Dev Cap is to make the device self-describing
so we can avoid updates like this every time new hardware comes out.

> Note this does not have any effect unless the user requested the kernel
> to enable ASPM in the first place (by default we don't enable it). 

I think this depends on the platform and kernel config, doesn't it?
If CONFIG_PCIEASPM_POWERSAVE=y or CONFIG_PCIEASPM_POWER_SUPERSAVE=y
I suspect we would enable ASPM L1 even without the parameters below.

> This is done with "pcie_aspm=force pcie_aspm.policy=powersupsersave"
> command line parameters.

s/powersupsersave/powersupersave/

This should affect "powersave" as well as "powersupersave", right?
Both enable L1.  "powersupersave" enables the L1 substates.

We *should* be able to enable/disable ASPM L1 using the sysfs "l1_aspm
file, too.

> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pci/quirks.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index da829274fc66..e97b5daa00eb 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5895,3 +5895,47 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1533, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect);
> +
> +#ifdef CONFIG_PCIEASPM
> +/*
> + * Intel DG2 graphics card has hard-coded acceptable L1 latency that is
> + * too low which prevents ASPM to be enabled. It does support ASPM L1
> + * and tolerates higher latencies so quirk it to be unlimited.
> + */
> +static void quirk_aspm_accepted_l1_latency(struct pci_dev *dev)
> +{
> +	if ((dev->devcap & PCI_EXP_DEVCAP_L1) >> 9 < 7) {
> +		u32 devcap = dev->devcap;
> +
> +		dev->devcap |= 7 << 9;
> +		pci_info(dev, "quirking devcap for L1 accepted latency 0x%08x -> 0x%08x\n",
> +			 devcap, dev->devcap);
> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f80, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f81, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f82, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f83, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f84, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f85, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f86, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f87, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f88, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5690, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5691, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5692, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5693, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5694, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5695, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a0, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a1, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a2, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a3, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a4, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a5, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a6, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b0, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b1, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c0, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c1, quirk_aspm_accepted_l1_latency);
> +#endif
> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited
  2022-04-05 16:01 ` Bjorn Helgaas
@ 2022-04-05 16:23   ` Vivi, Rodrigo
  2022-04-05 16:24   ` Mika Westerberg
  1 sibling, 0 replies; 7+ messages in thread
From: Vivi, Rodrigo @ 2022-04-05 16:23 UTC (permalink / raw)
  To: mika.westerberg@linux.intel.com, helgaas@kernel.org
  Cc: linux-pci@vger.kernel.org, bhelgaas@google.com

On Tue, 2022-04-05 at 11:01 -0500, Bjorn Helgaas wrote:
> On Tue, Apr 05, 2022 at 12:38:10PM +0300, Mika Westerberg wrote:
> > Intel DG2 discrete graphics PCIe endpoints hard-code their
> > acceptable L1
> > ASPM latency to be < 1us even though the hardware actually supports
> > higher latencies (> 64 us) just fine. In order to allow the links
> > to go
> > into L1 and save power, quirk the acceptable L1 ASPM latency for
> > these
> > endpoints to be unlimited.
> 
> Is there a plan to fix this in future DG2 hardware/firmware?
> Obviously the point of Dev Cap is to make the device self-describing
> so we can avoid updates like this every time new hardware comes out.

Unfortunately no fix possible in DG2 hw (pci id list below), but it
will be fixed for other platforms coming after those.

> 
> > Note this does not have any effect unless the user requested the
> > kernel
> > to enable ASPM in the first place (by default we don't enable it). 
> 
> I think this depends on the platform and kernel config, doesn't it?
> If CONFIG_PCIEASPM_POWERSAVE=y or CONFIG_PCIEASPM_POWER_SUPERSAVE=y
> I suspect we would enable ASPM L1 even without the parameters below.
> 
> > This is done with "pcie_aspm=force
> > pcie_aspm.policy=powersupsersave"
> > command line parameters.
> 
> s/powersupsersave/powersupersave/
> 
> This should affect "powersave" as well as "powersupersave", right?
> Both enable L1.  "powersupersave" enables the L1 substates.
> 
> We *should* be able to enable/disable ASPM L1 using the sysfs
> "l1_aspm
> file, too.
> 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  drivers/pci/quirks.c | 44
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> > 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index da829274fc66..e97b5daa00eb 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -5895,3 +5895,47 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL,
> > 0x1533, rom_bar_overlap_defect);
> >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536,
> > rom_bar_overlap_defect);
> >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537,
> > rom_bar_overlap_defect);
> >  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538,
> > rom_bar_overlap_defect);
> > +
> > +#ifdef CONFIG_PCIEASPM
> > +/*
> > + * Intel DG2 graphics card has hard-coded acceptable L1 latency
> > that is
> > + * too low which prevents ASPM to be enabled. It does support ASPM
> > L1
> > + * and tolerates higher latencies so quirk it to be unlimited.
> > + */
> > +static void quirk_aspm_accepted_l1_latency(struct pci_dev *dev)
> > +{
> > +       if ((dev->devcap & PCI_EXP_DEVCAP_L1) >> 9 < 7) {
> > +               u32 devcap = dev->devcap;
> > +
> > +               dev->devcap |= 7 << 9;
> > +               pci_info(dev, "quirking devcap for L1 accepted
> > latency 0x%08x -> 0x%08x\n",
> > +                        devcap, dev->devcap);
> > +       }
> > +}
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f80,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f81,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f82,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f83,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f84,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f85,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f86,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f87,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f88,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5690,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5691,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5692,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5693,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5694,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5695,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a0,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a1,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a2,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a3,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a4,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a5,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a6,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b0,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b1,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c0,
> > quirk_aspm_accepted_l1_latency);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c1,
> > quirk_aspm_accepted_l1_latency);
> > +#endif
> > -- 
> > 2.35.1
> > 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited
  2022-04-05 16:01 ` Bjorn Helgaas
  2022-04-05 16:23   ` Vivi, Rodrigo
@ 2022-04-05 16:24   ` Mika Westerberg
  1 sibling, 0 replies; 7+ messages in thread
From: Mika Westerberg @ 2022-04-05 16:24 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Bjorn Helgaas, Rodrigo Vivi, linux-pci

Hi Bjorn,

On Tue, Apr 05, 2022 at 11:01:51AM -0500, Bjorn Helgaas wrote:
> On Tue, Apr 05, 2022 at 12:38:10PM +0300, Mika Westerberg wrote:
> > Intel DG2 discrete graphics PCIe endpoints hard-code their acceptable L1
> > ASPM latency to be < 1us even though the hardware actually supports
> > higher latencies (> 64 us) just fine. In order to allow the links to go
> > into L1 and save power, quirk the acceptable L1 ASPM latency for these
> > endpoints to be unlimited.
> 
> Is there a plan to fix this in future DG2 hardware/firmware?
> Obviously the point of Dev Cap is to make the device self-describing
> so we can avoid updates like this every time new hardware comes out.

Yes, I think that's the plan.

> > Note this does not have any effect unless the user requested the kernel
> > to enable ASPM in the first place (by default we don't enable it). 
> 
> I think this depends on the platform and kernel config, doesn't it?
> If CONFIG_PCIEASPM_POWERSAVE=y or CONFIG_PCIEASPM_POWER_SUPERSAVE=y
> I suspect we would enable ASPM L1 even without the parameters below.
> 
> > This is done with "pcie_aspm=force pcie_aspm.policy=powersupsersave"
> > command line parameters.
> 
> s/powersupsersave/powersupersave/
> 
> This should affect "powersave" as well as "powersupersave", right?
> Both enable L1.  "powersupersave" enables the L1 substates.
> 
> We *should* be able to enable/disable ASPM L1 using the sysfs "l1_aspm
> file, too.

Indeed you are right. I think we can drop that paragraph completely from
the commit log. Do you want me to send v2 with that corrected or you
will do that while applying?

Thanks!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited
  2022-04-05  9:38 [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited Mika Westerberg
  2022-04-05 12:49 ` Rodrigo Vivi
  2022-04-05 16:01 ` Bjorn Helgaas
@ 2022-04-07 17:06 ` Bjorn Helgaas
  2022-04-08  6:44   ` Mika Westerberg
  2 siblings, 1 reply; 7+ messages in thread
From: Bjorn Helgaas @ 2022-04-07 17:06 UTC (permalink / raw)
  To: Mika Westerberg; +Cc: Bjorn Helgaas, Rodrigo Vivi, linux-pci

On Tue, Apr 05, 2022 at 12:38:10PM +0300, Mika Westerberg wrote:
> Intel DG2 discrete graphics PCIe endpoints hard-code their acceptable L1
> ASPM latency to be < 1us even though the hardware actually supports
> higher latencies (> 64 us) just fine. In order to allow the links to go
> into L1 and save power, quirk the acceptable L1 ASPM latency for these
> endpoints to be unlimited.
> 
> Note this does not have any effect unless the user requested the kernel
> to enable ASPM in the first place (by default we don't enable it). This
> is done with "pcie_aspm=force pcie_aspm.policy=powersupsersave" command
> line parameters.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

I wordsmithed as below and applied to pci/aspm for v5.19, thanks!

Please double-check that I didn't screw up the FIELD_GET/PREP usage.

> ---
>  drivers/pci/quirks.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index da829274fc66..e97b5daa00eb 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5895,3 +5895,47 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1533, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect);
> +
> +#ifdef CONFIG_PCIEASPM
> +/*
> + * Intel DG2 graphics card has hard-coded acceptable L1 latency that is
> + * too low which prevents ASPM to be enabled. It does support ASPM L1
> + * and tolerates higher latencies so quirk it to be unlimited.
> + */
> +static void quirk_aspm_accepted_l1_latency(struct pci_dev *dev)
> +{
> +	if ((dev->devcap & PCI_EXP_DEVCAP_L1) >> 9 < 7) {
> +		u32 devcap = dev->devcap;
> +
> +		dev->devcap |= 7 << 9;
> +		pci_info(dev, "quirking devcap for L1 accepted latency 0x%08x -> 0x%08x\n",
> +			 devcap, dev->devcap);
> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f80, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f81, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f82, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f83, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f84, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f85, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f86, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f87, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f88, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5690, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5691, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5692, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5693, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5694, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5695, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a0, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a1, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a2, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a3, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a4, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a5, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a6, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b0, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b1, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c0, quirk_aspm_accepted_l1_latency);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c1, quirk_aspm_accepted_l1_latency);
> +#endif


commit 03038d84ace7 ("PCI/ASPM: Make Intel DG2 L1 acceptable latency unlimited")
Author: Mika Westerberg <mika.westerberg@linux.intel.com>
Date:   Tue Apr 5 12:38:10 2022 +0300

    PCI/ASPM: Make Intel DG2 L1 acceptable latency unlimited
    
    Intel DG2 discrete graphics PCIe endpoints advertise L1 acceptable exit
    latency to be < 1us even though they can actually tolerate unlimited exit
    latencies just fine. Quirk the L1 acceptable exit latency for these
    endpoints to be unlimited so ASPM L1 can be enabled.
    
    [bhelgaas: use FIELD_GET/FIELD_PREP, wordsmith comment & commit log]
    Link: https://lore.kernel.org/r/20220405093810.76613-1-mika.westerberg@linux.intel.com
    Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index da829274fc66..41aeaa235132 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -12,6 +12,7 @@
  * file, where their drivers can use them.
  */
 
+#include <linux/bitfield.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
 #include <linux/export.h>
@@ -5895,3 +5896,49 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1533, rom_bar_overlap_defect);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1536, rom_bar_overlap_defect);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1537, rom_bar_overlap_defect);
 DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1538, rom_bar_overlap_defect);
+
+#ifdef CONFIG_PCIEASPM
+/*
+ * Several Intel DG2 graphics devices advertise that they can only tolerate
+ * 1us latency when transitioning from L1 to L0, which may prevent ASPM L1
+ * from being enabled.  But in fact these devices can tolerate unlimited
+ * latency.  Override their Device Capabilities value to allow ASPM L1 to
+ * be enabled.
+ */
+static void aspm_l1_acceptable_latency(struct pci_dev *dev)
+{
+	u32 l1_lat = FIELD_GET(PCI_EXP_DEVCAP_L1, dev->devcap);
+
+	if (l1_lat < 7) {
+		dev->devcap |= FIELD_PREP(PCI_EXP_DEVCAP_L1, 7);
+		pci_info(dev, "ASPM: overriding L1 acceptable latency from %#x to 0x7\n",
+			 l1_lat);
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f80, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f81, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f82, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f83, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f84, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f85, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f86, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f87, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x4f88, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5690, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5691, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5692, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5693, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5694, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x5695, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a0, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a1, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a2, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a3, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a4, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a5, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56a6, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b0, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b1, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c0, aspm_l1_acceptable_latency);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c1, aspm_l1_acceptable_latency);
+#endif

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited
  2022-04-07 17:06 ` Bjorn Helgaas
@ 2022-04-08  6:44   ` Mika Westerberg
  0 siblings, 0 replies; 7+ messages in thread
From: Mika Westerberg @ 2022-04-08  6:44 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Bjorn Helgaas, Rodrigo Vivi, linux-pci

On Thu, Apr 07, 2022 at 12:06:55PM -0500, Bjorn Helgaas wrote:
> On Tue, Apr 05, 2022 at 12:38:10PM +0300, Mika Westerberg wrote:
> > Intel DG2 discrete graphics PCIe endpoints hard-code their acceptable L1
> > ASPM latency to be < 1us even though the hardware actually supports
> > higher latencies (> 64 us) just fine. In order to allow the links to go
> > into L1 and save power, quirk the acceptable L1 ASPM latency for these
> > endpoints to be unlimited.
> > 
> > Note this does not have any effect unless the user requested the kernel
> > to enable ASPM in the first place (by default we don't enable it). This
> > is done with "pcie_aspm=force pcie_aspm.policy=powersupsersave" command
> > line parameters.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> I wordsmithed as below and applied to pci/aspm for v5.19, thanks!

Thanks!

> Please double-check that I didn't screw up the FIELD_GET/PREP usage.

Looks good to me. I did not even know we have such macros, thanks for
pointing that out :)

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-04-08  6:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-05  9:38 [PATCH] PCI: Quirk Intel DG2 ASPM L1 acceptable latency to be unlimited Mika Westerberg
2022-04-05 12:49 ` Rodrigo Vivi
2022-04-05 16:01 ` Bjorn Helgaas
2022-04-05 16:23   ` Vivi, Rodrigo
2022-04-05 16:24   ` Mika Westerberg
2022-04-07 17:06 ` Bjorn Helgaas
2022-04-08  6:44   ` Mika Westerberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox