[PATCH V2 1/2] PCI/ASPM: Add host-bridge API to override default ASPM/CLKPM link state

[PATCH V2 1/2] PCI/ASPM: Add host-bridge API to override default ASPM/CLKPM link state

[David E. Box]

Synthetic PCIe hierarchies, such as those created by Intel VMD, are not
enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM
defaults. Devices in such domains may therefore run without the intended
power management.

Add a host-bridge mechanism that lets controller drivers supply their own
defaults. A new aspm_default_link_state field in struct pci_host_bridge is
set via pci_host_set_default_pcie_link_state(). During link initialization,
if this field is non-zero, ASPM and CLKPM defaults come from it instead of
BIOS.

This enables drivers like VMD to align link power management with platform
expectations and avoids embedding controller-specific quirks in ASPM core
logic.

Link: https://patchwork.ozlabs.org/project/linux-pci/patch/20250720190140.2639200-1-david.e.box%40linux.intel.com/
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---

Changes in V1 from RFC:

  -- Rename field to aspm_dflt_link_state since it stores
     PCIE_LINK_STATE_XXX flags, not a policy enum.
  -- Move the field to struct pci_host_bridge since it's being applied to
     the entire host bridge per Mani's suggestion.
  -- During testing noticed that clkpm remained disabled and this was
     also handled by the formerly used pci_enable_link_state(). Add a
     check in pcie_clkpm_cap_init() as well to enable clkpm during init.

Changes in V2:

  -- Host field name changed to aspm_default_link_state.
  -- Added get/set functions for aspm_default_link_state. Only the
     setter is exported. Added a kernel-doc describing usage and
     particulars around meaning of 0.

 drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
 include/linux/pci.h     |  9 +++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 919a05b97647..b4f0b4805a35 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
 	pcie_set_clkpm_nocheck(link, enable);
 }
 
+/**
+ * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
+ * @host: host bridge on which to apply the defaults
+ * @state: PCIE_LINK_STATE_XXX flags
+ *
+ * Allows a PCIe controller driver to specify the default ASPM and/or
+ * Clock Power Management (CLKPM) link state mask that will be used
+ * for links under this host bridge during ASPM/CLKPM capability init.
+ *
+ * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
+ * to override the firmware-discovered defaults.
+ *
+ * Interpretation of aspm_default_link_state:
+ *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
+ *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
+ *              values discovered in hardware/firmware
+ *
+ * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
+ */
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state)
+{
+	host->aspm_default_link_state = state;
+}
+EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
+
+static u32 pci_host_get_default_pcie_link_state(struct pci_dev *parent)
+{
+	struct pci_host_bridge *host = pci_find_host_bridge(parent->bus);
+
+	return host ? host->aspm_default_link_state : 0;
+}
+
 static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	int capable = 1, enabled = 1;
@@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 			enabled = 0;
 	}
 	link->clkpm_enabled = enabled;
-	link->clkpm_default = enabled;
+	if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
+		link->clkpm_default = 1;
+	else
+		link->clkpm_default = enabled;
 	link->clkpm_capable = capable;
 	link->clkpm_disable = blacklist ? 1 : 0;
 }
@@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 	}
 
 	/* Save default state */
-	link->aspm_default = link->aspm_enabled;
+	link->aspm_default = pci_host_get_default_pcie_link_state(parent);
+	if (!link->aspm_default)
+		link->aspm_default = link->aspm_enabled;
 
 	/* Setup initial capable state. Will be updated later */
 	link->aspm_capable = link->aspm_support;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 59876de13860..8947cbaf9fa6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -620,6 +620,10 @@ struct pci_host_bridge {
 	unsigned int	size_windows:1;		/* Enable root bus sizing */
 	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
 
+#ifdef CONFIG_PCIEASPM
+	unsigned int	aspm_default_link_state;	/* Controller-provided default */
+#endif
+
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,
 			const struct resource *res,
@@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
 int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
 int pci_enable_link_state(struct pci_dev *pdev, int state);
 int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
+void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+					  unsigned int state);
 void pcie_no_aspm(void);
 bool pcie_aspm_support_enabled(void);
 bool pcie_aspm_enabled(struct pci_dev *pdev);
@@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
 { return 0; }
 static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
 { return 0; }
+static inline void
+pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
+				     unsigned int state) { }
 static inline void pcie_no_aspm(void) { }
 static inline bool pcie_aspm_support_enabled(void) { return false; }
 static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }

base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
-- 
2.43.0

[PATCH V2 2/2] PCI: vmd: Use pci_host_set_default_pcie_link_state() to set ASPM defaults

[David E. Box]

Now that pci_host_set_default_pcie_link_state() exists, set the VMD child
domain with PCIE_LINK_STATE_ALL at bridge creation so core ASPM uses those
defaults during ASPM and CLKPM capability init.

Also remove the unneeded pci_set_power_state_locked(pdev, PCI_D0) and
pci_enable_link_state_locked() calls now that the links are configured
during enumeration.

This aligns VMD behavior with platform expectations without per-controller
ASPM tweaks at runtime.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
Changes in V2:

  -- Separated VMD changes into new patch.
  -- Changed comment for VMD_FEAT_BIOS_PM_QUIRK to remove ASPM
  -- Removed pci_set_power_state() and pci_enable_link_state_locked()
     calls in vmd_pm_enable_quirk()
  -- Use pci_host_set_default_pcie_link_state()

 drivers/pci/controller/vmd.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index b679c7f28f51..b99e01a57ddb 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -71,10 +71,9 @@ enum vmd_features {
 	VMD_FEAT_CAN_BYPASS_MSI_REMAP		= (1 << 4),
 
 	/*
-	 * Enable ASPM on the PCIE root ports and set the default LTR of the
-	 * storage devices on platforms where these values are not configured by
-	 * BIOS. This is needed for laptops, which require these settings for
-	 * proper power management of the SoC.
+	 * Program default LTR values for storage devices on platforms where
+	 * firmware did not. Required on many laptops for proper SoC power
+	 * management.
 	 */
 	VMD_FEAT_BIOS_PM_QUIRK		= (1 << 5),
 };
@@ -733,7 +732,7 @@ static void vmd_copy_host_bridge_flags(struct pci_host_bridge *root_bridge,
 }
 
 /*
- * Enable ASPM and LTR settings on devices that aren't configured by BIOS.
+ * Enable LTR settings on devices that aren't configured by BIOS.
  */
 static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 {
@@ -747,7 +746,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 
 	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
 	if (!pos)
-		goto out_state_change;
+		return 0;
 
 	/*
 	 * Skip if the max snoop LTR is non-zero, indicating BIOS has set it
@@ -755,7 +754,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	 */
 	pci_read_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, &ltr_reg);
 	if (!!(ltr_reg & (PCI_LTR_VALUE_MASK | PCI_LTR_SCALE_MASK)))
-		goto out_state_change;
+		return 0;
 
 	/*
 	 * Set the default values to the maximum required by the platform to
@@ -767,13 +766,6 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	pci_write_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, ltr_reg);
 	pci_info(pdev, "VMD: Default LTR value set by driver\n");
 
-out_state_change:
-	/*
-	 * Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per
-	 * PCIe r6.0, sec 5.5.4.
-	 */
-	pci_set_power_state_locked(pdev, PCI_D0);
-	pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL);
 	return 0;
 }
 
@@ -921,6 +913,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	WARN(sysfs_create_link(&vmd->dev->dev.kobj, &vmd->bus->dev.kobj,
 			       "domain"), "Can't create symlink to domain\n");
 
+	pci_host_set_default_pcie_link_state(to_pci_host_bridge(vmd->bus->bridge),
+					     PCIE_LINK_STATE_ALL);
 	vmd_acpi_begin();
 
 	pci_scan_child_bus(vmd->bus);
-- 
2.43.0

Re: [PATCH V2 1/2] PCI/ASPM: Add host-bridge API to override default ASPM/CLKPM link state

[David Box]

On Thu, Aug 21, 2025 at 08:11:57PM -0700, David E. Box wrote:
> Synthetic PCIe hierarchies, such as those created by Intel VMD, are not
> enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM
> defaults. Devices in such domains may therefore run without the intended
> power management.
> 
> Add a host-bridge mechanism that lets controller drivers supply their own
> defaults. A new aspm_default_link_state field in struct pci_host_bridge is
> set via pci_host_set_default_pcie_link_state(). During link initialization,
> if this field is non-zero, ASPM and CLKPM defaults come from it instead of
> BIOS.
> 
> This enables drivers like VMD to align link power management with platform
> expectations and avoids embedding controller-specific quirks in ASPM core
> logic.
> 
> Link: https://patchwork.ozlabs.org/project/linux-pci/patch/20250720190140.2639200-1-david.e.box%40linux.intel.com/
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---

Besides the review I'd appreciate a Tested-by on this patch. Thanks.

David

> 
> Changes in V1 from RFC:
> 
>   -- Rename field to aspm_dflt_link_state since it stores
>      PCIE_LINK_STATE_XXX flags, not a policy enum.
>   -- Move the field to struct pci_host_bridge since it's being applied to
>      the entire host bridge per Mani's suggestion.
>   -- During testing noticed that clkpm remained disabled and this was
>      also handled by the formerly used pci_enable_link_state(). Add a
>      check in pcie_clkpm_cap_init() as well to enable clkpm during init.
> 
> Changes in V2:
> 
>   -- Host field name changed to aspm_default_link_state.
>   -- Added get/set functions for aspm_default_link_state. Only the
>      setter is exported. Added a kernel-doc describing usage and
>      particulars around meaning of 0.
> 
>  drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
>  include/linux/pci.h     |  9 +++++++++
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 919a05b97647..b4f0b4805a35 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
>  	pcie_set_clkpm_nocheck(link, enable);
>  }
>  
> +/**
> + * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
> + * @host: host bridge on which to apply the defaults
> + * @state: PCIE_LINK_STATE_XXX flags
> + *
> + * Allows a PCIe controller driver to specify the default ASPM and/or
> + * Clock Power Management (CLKPM) link state mask that will be used
> + * for links under this host bridge during ASPM/CLKPM capability init.
> + *
> + * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
> + * to override the firmware-discovered defaults.
> + *
> + * Interpretation of aspm_default_link_state:
> + *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
> + *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
> + *              values discovered in hardware/firmware
> + *
> + * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
> + */
> +void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +					  unsigned int state)
> +{
> +	host->aspm_default_link_state = state;
> +}
> +EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
> +
> +static u32 pci_host_get_default_pcie_link_state(struct pci_dev *parent)
> +{
> +	struct pci_host_bridge *host = pci_find_host_bridge(parent->bus);
> +
> +	return host ? host->aspm_default_link_state : 0;
> +}
> +
>  static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  {
>  	int capable = 1, enabled = 1;
> @@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  			enabled = 0;
>  	}
>  	link->clkpm_enabled = enabled;
> -	link->clkpm_default = enabled;
> +	if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
> +		link->clkpm_default = 1;
> +	else
> +		link->clkpm_default = enabled;
>  	link->clkpm_capable = capable;
>  	link->clkpm_disable = blacklist ? 1 : 0;
>  }
> @@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
>  	}
>  
>  	/* Save default state */
> -	link->aspm_default = link->aspm_enabled;
> +	link->aspm_default = pci_host_get_default_pcie_link_state(parent);
> +	if (!link->aspm_default)
> +		link->aspm_default = link->aspm_enabled;
>  
>  	/* Setup initial capable state. Will be updated later */
>  	link->aspm_capable = link->aspm_support;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 59876de13860..8947cbaf9fa6 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -620,6 +620,10 @@ struct pci_host_bridge {
>  	unsigned int	size_windows:1;		/* Enable root bus sizing */
>  	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
>  
> +#ifdef CONFIG_PCIEASPM
> +	unsigned int	aspm_default_link_state;	/* Controller-provided default */
> +#endif
> +
>  	/* Resource alignment requirements */
>  	resource_size_t (*align_resource)(struct pci_dev *dev,
>  			const struct resource *res,
> @@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
>  int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
>  int pci_enable_link_state(struct pci_dev *pdev, int state);
>  int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
> +void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +					  unsigned int state);
>  void pcie_no_aspm(void);
>  bool pcie_aspm_support_enabled(void);
>  bool pcie_aspm_enabled(struct pci_dev *pdev);
> @@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
>  { return 0; }
>  static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
>  { return 0; }
> +static inline void
> +pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +				     unsigned int state) { }
>  static inline void pcie_no_aspm(void) { }
>  static inline bool pcie_aspm_support_enabled(void) { return false; }
>  static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }
> 
> base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
> -- 
> 2.43.0
> 
> 

Re: [PATCH V2 1/2] PCI/ASPM: Add host-bridge API to override default ASPM/CLKPM link state

[Kenneth Crudup]

On 8/21/25 20:30, David Box wrote:
> Besides the review I'd appreciate a Tested-by on this patch. Thanks.

Oh, don't worry! :)

-K

-- 
Kenneth R. Crudup / Sr. SW Engineer, Scott County Consulting, Orange 
County CA

Re: [PATCH V2 1/2] PCI/ASPM: Add host-bridge API to override default ASPM/CLKPM link state

[Kenneth Crudup]

Tested-By: Kenneth R. Crudup <kenny@panix.com>

----
Bzy_MHz C1E%    C10%    CPU%c7  PkgTmp  GFX%rc6 GFXMHz  CPUGFX% Pkg%pc8 
Pk%pc10 CPU%LPI SYS%LPI PkgWatt CorWatt GFXWatt
909     1.37    98.28   42.74   45      99.79   300     0.15    21.61 
47.17   47.24   38.78   1.07    0.19    0.00
1788    3.86    95.43   42.24   46      99.84   300     0.14    23.97 
32.84   32.89   26.52   1.80    0.68    0.00
1014    0.01    99.70   42.74   45      99.84   300     0.12    26.23 
59.45   59.54   48.82   0.59    0.12    0.00
1887    0.86    98.41   42.25   45      99.83   300     0.13    24.62 
47.84   47.92   40.02   1.42    0.63    0.00
951     0.01    99.70   42.74   46      99.83   300     0.13    27.87 
56.99   57.08   46.82   0.63    0.12    0.00
Bzy_MHz C1E%    C10%    CPU%c7  PkgTmp  GFX%rc6 GFXMHz  CPUGFX% Pkg%pc8 
Pk%pc10 CPU%LPI SYS%LPI PkgWatt CorWatt GFXWatt
1814    4.11    95.08   42.26   46      99.81   300     0.14    18.76 
37.64   37.70   30.90   1.93    0.77    0.00
1009    0.30    99.41   42.73   45      99.89   300     0.13    27.08 
55.32   55.42   45.56   0.70    0.13    0.00
1896    2.06    97.25   42.25   46      99.82   300     0.13    20.58 
41.54   41.61   35.22   1.66    0.67    0.00
972     0.01    99.69   42.74   45      99.86   300     0.14    28.82 
56.29   56.38   47.29   0.59    0.11    0.00
1859    1.66    97.61   42.24   45      99.86   300     0.12    22.99 
40.33   40.40   33.22   1.69    0.71    0.00
----

(Hope I've done that properly!)

-K

On 8/21/25 20:11, David E. Box wrote:
> Synthetic PCIe hierarchies, such as those created by Intel VMD, are not
> enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM
> defaults. Devices in such domains may therefore run without the intended
> power management.
> 
> Add a host-bridge mechanism that lets controller drivers supply their own
> defaults. A new aspm_default_link_state field in struct pci_host_bridge is
> set via pci_host_set_default_pcie_link_state(). During link initialization,
> if this field is non-zero, ASPM and CLKPM defaults come from it instead of
> BIOS.
> 
> This enables drivers like VMD to align link power management with platform
> expectations and avoids embedding controller-specific quirks in ASPM core
> logic.
> 
> Link: https://patchwork.ozlabs.org/project/linux-pci/patch/20250720190140.2639200-1-david.e.box%40linux.intel.com/
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> 
> Changes in V1 from RFC:
> 
>    -- Rename field to aspm_dflt_link_state since it stores
>       PCIE_LINK_STATE_XXX flags, not a policy enum.
>    -- Move the field to struct pci_host_bridge since it's being applied to
>       the entire host bridge per Mani's suggestion.
>    -- During testing noticed that clkpm remained disabled and this was
>       also handled by the formerly used pci_enable_link_state(). Add a
>       check in pcie_clkpm_cap_init() as well to enable clkpm during init.
> 
> Changes in V2:
> 
>    -- Host field name changed to aspm_default_link_state.
>    -- Added get/set functions for aspm_default_link_state. Only the
>       setter is exported. Added a kernel-doc describing usage and
>       particulars around meaning of 0.
> 
>   drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
>   include/linux/pci.h     |  9 +++++++++
>   2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 919a05b97647..b4f0b4805a35 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
>   	pcie_set_clkpm_nocheck(link, enable);
>   }
>   
> +/**
> + * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
> + * @host: host bridge on which to apply the defaults
> + * @state: PCIE_LINK_STATE_XXX flags
> + *
> + * Allows a PCIe controller driver to specify the default ASPM and/or
> + * Clock Power Management (CLKPM) link state mask that will be used
> + * for links under this host bridge during ASPM/CLKPM capability init.
> + *
> + * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
> + * to override the firmware-discovered defaults.
> + *
> + * Interpretation of aspm_default_link_state:
> + *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
> + *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
> + *              values discovered in hardware/firmware
> + *
> + * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
> + */
> +void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +					  unsigned int state)
> +{
> +	host->aspm_default_link_state = state;
> +}
> +EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
> +
> +static u32 pci_host_get_default_pcie_link_state(struct pci_dev *parent)
> +{
> +	struct pci_host_bridge *host = pci_find_host_bridge(parent->bus);
> +
> +	return host ? host->aspm_default_link_state : 0;
> +}
> +
>   static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>   {
>   	int capable = 1, enabled = 1;
> @@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>   			enabled = 0;
>   	}
>   	link->clkpm_enabled = enabled;
> -	link->clkpm_default = enabled;
> +	if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
> +		link->clkpm_default = 1;
> +	else
> +		link->clkpm_default = enabled;
>   	link->clkpm_capable = capable;
>   	link->clkpm_disable = blacklist ? 1 : 0;
>   }
> @@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
>   	}
>   
>   	/* Save default state */
> -	link->aspm_default = link->aspm_enabled;
> +	link->aspm_default = pci_host_get_default_pcie_link_state(parent);
> +	if (!link->aspm_default)
> +		link->aspm_default = link->aspm_enabled;
>   
>   	/* Setup initial capable state. Will be updated later */
>   	link->aspm_capable = link->aspm_support;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 59876de13860..8947cbaf9fa6 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -620,6 +620,10 @@ struct pci_host_bridge {
>   	unsigned int	size_windows:1;		/* Enable root bus sizing */
>   	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
>   
> +#ifdef CONFIG_PCIEASPM
> +	unsigned int	aspm_default_link_state;	/* Controller-provided default */
> +#endif
> +
>   	/* Resource alignment requirements */
>   	resource_size_t (*align_resource)(struct pci_dev *dev,
>   			const struct resource *res,
> @@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
>   int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
>   int pci_enable_link_state(struct pci_dev *pdev, int state);
>   int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
> +void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +					  unsigned int state);
>   void pcie_no_aspm(void);
>   bool pcie_aspm_support_enabled(void);
>   bool pcie_aspm_enabled(struct pci_dev *pdev);
> @@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
>   { return 0; }
>   static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
>   { return 0; }
> +static inline void
> +pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +				     unsigned int state) { }
>   static inline void pcie_no_aspm(void) { }
>   static inline bool pcie_aspm_support_enabled(void) { return false; }
>   static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }
> 
> base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9

-- 
Kenneth R. Crudup / Sr. SW Engineer, Scott County Consulting, Orange 
County CA

Re: [PATCH V2 1/2] PCI/ASPM: Add host-bridge API to override default ASPM/CLKPM link state

[Rafael J. Wysocki]

On Fri, Aug 22, 2025 at 5:12 AM David E. Box
<david.e.box@linux.intel.com> wrote:
>
> Synthetic PCIe hierarchies, such as those created by Intel VMD, are not
> enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM
> defaults. Devices in such domains may therefore run without the intended
> power management.
>
> Add a host-bridge mechanism that lets controller drivers supply their own
> defaults. A new aspm_default_link_state field in struct pci_host_bridge is
> set via pci_host_set_default_pcie_link_state(). During link initialization,
> if this field is non-zero, ASPM and CLKPM defaults come from it instead of
> BIOS.
>
> This enables drivers like VMD to align link power management with platform
> expectations and avoids embedding controller-specific quirks in ASPM core
> logic.
>
> Link: https://patchwork.ozlabs.org/project/linux-pci/patch/20250720190140.2639200-1-david.e.box%40linux.intel.com/
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>

No issues found, so

Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

There is a very minor nit below, but it has no bearing on the above.

> ---
>
> Changes in V1 from RFC:
>
>   -- Rename field to aspm_dflt_link_state since it stores
>      PCIE_LINK_STATE_XXX flags, not a policy enum.
>   -- Move the field to struct pci_host_bridge since it's being applied to
>      the entire host bridge per Mani's suggestion.
>   -- During testing noticed that clkpm remained disabled and this was
>      also handled by the formerly used pci_enable_link_state(). Add a
>      check in pcie_clkpm_cap_init() as well to enable clkpm during init.
>
> Changes in V2:
>
>   -- Host field name changed to aspm_default_link_state.
>   -- Added get/set functions for aspm_default_link_state. Only the
>      setter is exported. Added a kernel-doc describing usage and
>      particulars around meaning of 0.
>
>  drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
>  include/linux/pci.h     |  9 +++++++++
>  2 files changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 919a05b97647..b4f0b4805a35 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
>         pcie_set_clkpm_nocheck(link, enable);
>  }
>
> +/**
> + * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
> + * @host: host bridge on which to apply the defaults
> + * @state: PCIE_LINK_STATE_XXX flags
> + *
> + * Allows a PCIe controller driver to specify the default ASPM and/or
> + * Clock Power Management (CLKPM) link state mask that will be used
> + * for links under this host bridge during ASPM/CLKPM capability init.
> + *
> + * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
> + * to override the firmware-discovered defaults.
> + *
> + * Interpretation of aspm_default_link_state:
> + *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
> + *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
> + *              values discovered in hardware/firmware
> + *
> + * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
> + */
> +void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +                                         unsigned int state)
> +{
> +       host->aspm_default_link_state = state;
> +}
> +EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
> +
> +static u32 pci_host_get_default_pcie_link_state(struct pci_dev *parent)

The parameter need not be called "parent".

> +{
> +       struct pci_host_bridge *host = pci_find_host_bridge(parent->bus);
> +
> +       return host ? host->aspm_default_link_state : 0;
> +}
> +
>  static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  {
>         int capable = 1, enabled = 1;
> @@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>                         enabled = 0;
>         }
>         link->clkpm_enabled = enabled;
> -       link->clkpm_default = enabled;
> +       if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
> +               link->clkpm_default = 1;
> +       else
> +               link->clkpm_default = enabled;
>         link->clkpm_capable = capable;
>         link->clkpm_disable = blacklist ? 1 : 0;
>  }
> @@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
>         }
>
>         /* Save default state */
> -       link->aspm_default = link->aspm_enabled;
> +       link->aspm_default = pci_host_get_default_pcie_link_state(parent);
> +       if (!link->aspm_default)
> +               link->aspm_default = link->aspm_enabled;
>
>         /* Setup initial capable state. Will be updated later */
>         link->aspm_capable = link->aspm_support;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 59876de13860..8947cbaf9fa6 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -620,6 +620,10 @@ struct pci_host_bridge {
>         unsigned int    size_windows:1;         /* Enable root bus sizing */
>         unsigned int    msi_domain:1;           /* Bridge wants MSI domain */
>
> +#ifdef CONFIG_PCIEASPM
> +       unsigned int    aspm_default_link_state;        /* Controller-provided default */
> +#endif
> +
>         /* Resource alignment requirements */
>         resource_size_t (*align_resource)(struct pci_dev *dev,
>                         const struct resource *res,
> @@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
>  int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
>  int pci_enable_link_state(struct pci_dev *pdev, int state);
>  int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
> +void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +                                         unsigned int state);
>  void pcie_no_aspm(void);
>  bool pcie_aspm_support_enabled(void);
>  bool pcie_aspm_enabled(struct pci_dev *pdev);
> @@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
>  { return 0; }
>  static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
>  { return 0; }
> +static inline void
> +pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +                                    unsigned int state) { }
>  static inline void pcie_no_aspm(void) { }
>  static inline bool pcie_aspm_support_enabled(void) { return false; }
>  static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }
>
> base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
> --
> 2.43.0
>

Re: [PATCH V2 2/2] PCI: vmd: Use pci_host_set_default_pcie_link_state() to set ASPM defaults

[Rafael J. Wysocki]

On Fri, Aug 22, 2025 at 5:12 AM David E. Box
<david.e.box@linux.intel.com> wrote:
>
> Now that pci_host_set_default_pcie_link_state() exists, set the VMD child
> domain with PCIE_LINK_STATE_ALL at bridge creation so core ASPM uses those
> defaults during ASPM and CLKPM capability init.
>
> Also remove the unneeded pci_set_power_state_locked(pdev, PCI_D0) and
> pci_enable_link_state_locked() calls now that the links are configured
> during enumeration.
>
> This aligns VMD behavior with platform expectations without per-controller
> ASPM tweaks at runtime.
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>

No issues found, so

Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

> ---
> Changes in V2:
>
>   -- Separated VMD changes into new patch.
>   -- Changed comment for VMD_FEAT_BIOS_PM_QUIRK to remove ASPM
>   -- Removed pci_set_power_state() and pci_enable_link_state_locked()
>      calls in vmd_pm_enable_quirk()
>   -- Use pci_host_set_default_pcie_link_state()
>
>  drivers/pci/controller/vmd.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index b679c7f28f51..b99e01a57ddb 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -71,10 +71,9 @@ enum vmd_features {
>         VMD_FEAT_CAN_BYPASS_MSI_REMAP           = (1 << 4),
>
>         /*
> -        * Enable ASPM on the PCIE root ports and set the default LTR of the
> -        * storage devices on platforms where these values are not configured by
> -        * BIOS. This is needed for laptops, which require these settings for
> -        * proper power management of the SoC.
> +        * Program default LTR values for storage devices on platforms where
> +        * firmware did not. Required on many laptops for proper SoC power
> +        * management.
>          */
>         VMD_FEAT_BIOS_PM_QUIRK          = (1 << 5),
>  };
> @@ -733,7 +732,7 @@ static void vmd_copy_host_bridge_flags(struct pci_host_bridge *root_bridge,
>  }
>
>  /*
> - * Enable ASPM and LTR settings on devices that aren't configured by BIOS.
> + * Enable LTR settings on devices that aren't configured by BIOS.
>   */
>  static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
>  {
> @@ -747,7 +746,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
>
>         pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
>         if (!pos)
> -               goto out_state_change;
> +               return 0;
>
>         /*
>          * Skip if the max snoop LTR is non-zero, indicating BIOS has set it
> @@ -755,7 +754,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
>          */
>         pci_read_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, &ltr_reg);
>         if (!!(ltr_reg & (PCI_LTR_VALUE_MASK | PCI_LTR_SCALE_MASK)))
> -               goto out_state_change;
> +               return 0;
>
>         /*
>          * Set the default values to the maximum required by the platform to
> @@ -767,13 +766,6 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
>         pci_write_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, ltr_reg);
>         pci_info(pdev, "VMD: Default LTR value set by driver\n");
>
> -out_state_change:
> -       /*
> -        * Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per
> -        * PCIe r6.0, sec 5.5.4.
> -        */
> -       pci_set_power_state_locked(pdev, PCI_D0);
> -       pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL);
>         return 0;
>  }
>
> @@ -921,6 +913,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>         WARN(sysfs_create_link(&vmd->dev->dev.kobj, &vmd->bus->dev.kobj,
>                                "domain"), "Can't create symlink to domain\n");
>
> +       pci_host_set_default_pcie_link_state(to_pci_host_bridge(vmd->bus->bridge),
> +                                            PCIE_LINK_STATE_ALL);
>         vmd_acpi_begin();
>
>         pci_scan_child_bus(vmd->bus);
> --
> 2.43.0
>

Re: [PATCH V2 1/2] PCI/ASPM: Add host-bridge API to override default ASPM/CLKPM link state

[Manivannan Sadhasivam]

On Thu, Aug 21, 2025 at 08:11:57PM GMT, David E. Box wrote:
> Synthetic PCIe hierarchies, such as those created by Intel VMD, are not
> enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM
> defaults. Devices in such domains may therefore run without the intended
> power management.
> 
> Add a host-bridge mechanism that lets controller drivers supply their own
> defaults. A new aspm_default_link_state field in struct pci_host_bridge is
> set via pci_host_set_default_pcie_link_state(). During link initialization,
> if this field is non-zero, ASPM and CLKPM defaults come from it instead of
> BIOS.
> 
> This enables drivers like VMD to align link power management with platform
> expectations and avoids embedding controller-specific quirks in ASPM core
> logic.
> 
> Link: https://patchwork.ozlabs.org/project/linux-pci/patch/20250720190140.2639200-1-david.e.box%40linux.intel.com/
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
> 
> Changes in V1 from RFC:
> 
>   -- Rename field to aspm_dflt_link_state since it stores
>      PCIE_LINK_STATE_XXX flags, not a policy enum.
>   -- Move the field to struct pci_host_bridge since it's being applied to
>      the entire host bridge per Mani's suggestion.
>   -- During testing noticed that clkpm remained disabled and this was
>      also handled by the formerly used pci_enable_link_state(). Add a
>      check in pcie_clkpm_cap_init() as well to enable clkpm during init.
> 
> Changes in V2:
> 
>   -- Host field name changed to aspm_default_link_state.
>   -- Added get/set functions for aspm_default_link_state. Only the
>      setter is exported. Added a kernel-doc describing usage and
>      particulars around meaning of 0.
> 
>  drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
>  include/linux/pci.h     |  9 +++++++++
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 919a05b97647..b4f0b4805a35 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
>  	pcie_set_clkpm_nocheck(link, enable);
>  }
>  
> +/**
> + * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
> + * @host: host bridge on which to apply the defaults
> + * @state: PCIE_LINK_STATE_XXX flags
> + *
> + * Allows a PCIe controller driver to specify the default ASPM and/or
> + * Clock Power Management (CLKPM) link state mask that will be used
> + * for links under this host bridge during ASPM/CLKPM capability init.
> + *
> + * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
> + * to override the firmware-discovered defaults.
> + *
> + * Interpretation of aspm_default_link_state:
> + *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
> + *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
> + *              values discovered in hardware/firmware
> + *
> + * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
> + */
> +void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +					  unsigned int state)
> +{
> +	host->aspm_default_link_state = state;
> +}
> +EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
> +
> +static u32 pci_host_get_default_pcie_link_state(struct pci_dev *parent)
> +{
> +	struct pci_host_bridge *host = pci_find_host_bridge(parent->bus);
> +
> +	return host ? host->aspm_default_link_state : 0;
> +}
> +
>  static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  {
>  	int capable = 1, enabled = 1;
> @@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  			enabled = 0;
>  	}
>  	link->clkpm_enabled = enabled;
> -	link->clkpm_default = enabled;
> +	if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
> +		link->clkpm_default = 1;
> +	else
> +		link->clkpm_default = enabled;
>  	link->clkpm_capable = capable;
>  	link->clkpm_disable = blacklist ? 1 : 0;
>  }
> @@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
>  	}
>  
>  	/* Save default state */
> -	link->aspm_default = link->aspm_enabled;
> +	link->aspm_default = pci_host_get_default_pcie_link_state(parent);
> +	if (!link->aspm_default)
> +		link->aspm_default = link->aspm_enabled;
>  
>  	/* Setup initial capable state. Will be updated later */
>  	link->aspm_capable = link->aspm_support;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 59876de13860..8947cbaf9fa6 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -620,6 +620,10 @@ struct pci_host_bridge {
>  	unsigned int	size_windows:1;		/* Enable root bus sizing */
>  	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
>  
> +#ifdef CONFIG_PCIEASPM
> +	unsigned int	aspm_default_link_state;	/* Controller-provided default */
> +#endif
> +
>  	/* Resource alignment requirements */
>  	resource_size_t (*align_resource)(struct pci_dev *dev,
>  			const struct resource *res,
> @@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
>  int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
>  int pci_enable_link_state(struct pci_dev *pdev, int state);
>  int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
> +void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +					  unsigned int state);
>  void pcie_no_aspm(void);
>  bool pcie_aspm_support_enabled(void);
>  bool pcie_aspm_enabled(struct pci_dev *pdev);
> @@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
>  { return 0; }
>  static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
>  { return 0; }
> +static inline void
> +pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +				     unsigned int state) { }
>  static inline void pcie_no_aspm(void) { }
>  static inline bool pcie_aspm_support_enabled(void) { return false; }
>  static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }
> 
> base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
> -- 
> 2.43.0
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH V2 2/2] PCI: vmd: Use pci_host_set_default_pcie_link_state() to set ASPM defaults

[Manivannan Sadhasivam]

On Thu, Aug 21, 2025 at 08:11:58PM GMT, David E. Box wrote:
> Now that pci_host_set_default_pcie_link_state() exists, set the VMD child
> domain with PCIE_LINK_STATE_ALL at bridge creation so core ASPM uses those
> defaults during ASPM and CLKPM capability init.
> 
> Also remove the unneeded pci_set_power_state_locked(pdev, PCI_D0) and
> pci_enable_link_state_locked() calls now that the links are configured
> during enumeration.
> 
> This aligns VMD behavior with platform expectations without per-controller
> ASPM tweaks at runtime.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>


Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani


> ---
> Changes in V2:
> 
>   -- Separated VMD changes into new patch.
>   -- Changed comment for VMD_FEAT_BIOS_PM_QUIRK to remove ASPM
>   -- Removed pci_set_power_state() and pci_enable_link_state_locked()
>      calls in vmd_pm_enable_quirk()
>   -- Use pci_host_set_default_pcie_link_state()
> 
>  drivers/pci/controller/vmd.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index b679c7f28f51..b99e01a57ddb 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -71,10 +71,9 @@ enum vmd_features {
>  	VMD_FEAT_CAN_BYPASS_MSI_REMAP		= (1 << 4),
>  
>  	/*
> -	 * Enable ASPM on the PCIE root ports and set the default LTR of the
> -	 * storage devices on platforms where these values are not configured by
> -	 * BIOS. This is needed for laptops, which require these settings for
> -	 * proper power management of the SoC.
> +	 * Program default LTR values for storage devices on platforms where
> +	 * firmware did not. Required on many laptops for proper SoC power
> +	 * management.
>  	 */
>  	VMD_FEAT_BIOS_PM_QUIRK		= (1 << 5),
>  };
> @@ -733,7 +732,7 @@ static void vmd_copy_host_bridge_flags(struct pci_host_bridge *root_bridge,
>  }
>  
>  /*
> - * Enable ASPM and LTR settings on devices that aren't configured by BIOS.
> + * Enable LTR settings on devices that aren't configured by BIOS.
>   */
>  static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
>  {
> @@ -747,7 +746,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
>  
>  	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
>  	if (!pos)
> -		goto out_state_change;
> +		return 0;
>  
>  	/*
>  	 * Skip if the max snoop LTR is non-zero, indicating BIOS has set it
> @@ -755,7 +754,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
>  	 */
>  	pci_read_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, &ltr_reg);
>  	if (!!(ltr_reg & (PCI_LTR_VALUE_MASK | PCI_LTR_SCALE_MASK)))
> -		goto out_state_change;
> +		return 0;
>  
>  	/*
>  	 * Set the default values to the maximum required by the platform to
> @@ -767,13 +766,6 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
>  	pci_write_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, ltr_reg);
>  	pci_info(pdev, "VMD: Default LTR value set by driver\n");
>  
> -out_state_change:
> -	/*
> -	 * Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per
> -	 * PCIe r6.0, sec 5.5.4.
> -	 */
> -	pci_set_power_state_locked(pdev, PCI_D0);
> -	pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL);
>  	return 0;
>  }
>  
> @@ -921,6 +913,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>  	WARN(sysfs_create_link(&vmd->dev->dev.kobj, &vmd->bus->dev.kobj,
>  			       "domain"), "Can't create symlink to domain\n");
>  
> +	pci_host_set_default_pcie_link_state(to_pci_host_bridge(vmd->bus->bridge),
> +					     PCIE_LINK_STATE_ALL);
>  	vmd_acpi_begin();
>  
>  	pci_scan_child_bus(vmd->bus);
> -- 
> 2.43.0
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH V2 1/2] PCI/ASPM: Add host-bridge API to override default ASPM/CLKPM link state

[Manivannan Sadhasivam]

On Thu, Aug 21, 2025 at 08:11:57PM GMT, David E. Box wrote:
> Synthetic PCIe hierarchies, such as those created by Intel VMD, are not
> enumerated by firmware and do not receive BIOS-provided ASPM or CLKPM
> defaults. Devices in such domains may therefore run without the intended
> power management.
> 
> Add a host-bridge mechanism that lets controller drivers supply their own
> defaults. A new aspm_default_link_state field in struct pci_host_bridge is
> set via pci_host_set_default_pcie_link_state(). During link initialization,
> if this field is non-zero, ASPM and CLKPM defaults come from it instead of
> BIOS.
> 
> This enables drivers like VMD to align link power management with platform
> expectations and avoids embedding controller-specific quirks in ASPM core
> logic.
> 
> Link: https://patchwork.ozlabs.org/project/linux-pci/patch/20250720190140.2639200-1-david.e.box%40linux.intel.com/
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>

Tested this patch along with the change in the Qcom controller driver. So,

Tested-by: Manivannan Sadhasivam <mani@kernel.org>

Thanks for the work!

- Mani

> ---
> 
> Changes in V1 from RFC:
> 
>   -- Rename field to aspm_dflt_link_state since it stores
>      PCIE_LINK_STATE_XXX flags, not a policy enum.
>   -- Move the field to struct pci_host_bridge since it's being applied to
>      the entire host bridge per Mani's suggestion.
>   -- During testing noticed that clkpm remained disabled and this was
>      also handled by the formerly used pci_enable_link_state(). Add a
>      check in pcie_clkpm_cap_init() as well to enable clkpm during init.
> 
> Changes in V2:
> 
>   -- Host field name changed to aspm_default_link_state.
>   -- Added get/set functions for aspm_default_link_state. Only the
>      setter is exported. Added a kernel-doc describing usage and
>      particulars around meaning of 0.
> 
>  drivers/pci/pcie/aspm.c | 42 +++++++++++++++++++++++++++++++++++++++--
>  include/linux/pci.h     |  9 +++++++++
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 919a05b97647..b4f0b4805a35 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -373,6 +373,39 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
>  	pcie_set_clkpm_nocheck(link, enable);
>  }
>  
> +/**
> + * pci_host_set_default_pcie_link_state - set controller-provided default ASPM/CLKPM mask
> + * @host: host bridge on which to apply the defaults
> + * @state: PCIE_LINK_STATE_XXX flags
> + *
> + * Allows a PCIe controller driver to specify the default ASPM and/or
> + * Clock Power Management (CLKPM) link state mask that will be used
> + * for links under this host bridge during ASPM/CLKPM capability init.
> + *
> + * The value is consumed in pcie_aspm_cap_init() and pcie_clkpm_cap_init()
> + * to override the firmware-discovered defaults.
> + *
> + * Interpretation of aspm_default_link_state:
> + *   - Nonzero: bitmask of PCIE_LINK_STATE_* values to be used as defaults
> + *   - Zero:    no override provided; ASPM/CLKPM defaults fall back to
> + *              values discovered in hardware/firmware
> + *
> + * Note: zero is always treated as "unset", not as "force ASPM/CLKPM off".
> + */
> +void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +					  unsigned int state)
> +{
> +	host->aspm_default_link_state = state;
> +}
> +EXPORT_SYMBOL_GPL(pci_host_set_default_pcie_link_state);
> +
> +static u32 pci_host_get_default_pcie_link_state(struct pci_dev *parent)
> +{
> +	struct pci_host_bridge *host = pci_find_host_bridge(parent->bus);
> +
> +	return host ? host->aspm_default_link_state : 0;
> +}
> +
>  static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  {
>  	int capable = 1, enabled = 1;
> @@ -394,7 +427,10 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  			enabled = 0;
>  	}
>  	link->clkpm_enabled = enabled;
> -	link->clkpm_default = enabled;
> +	if (pci_host_get_default_pcie_link_state(link->pdev) & PCIE_LINK_STATE_CLKPM)
> +		link->clkpm_default = 1;
> +	else
> +		link->clkpm_default = enabled;
>  	link->clkpm_capable = capable;
>  	link->clkpm_disable = blacklist ? 1 : 0;
>  }
> @@ -866,7 +902,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
>  	}
>  
>  	/* Save default state */
> -	link->aspm_default = link->aspm_enabled;
> +	link->aspm_default = pci_host_get_default_pcie_link_state(parent);
> +	if (!link->aspm_default)
> +		link->aspm_default = link->aspm_enabled;
>  
>  	/* Setup initial capable state. Will be updated later */
>  	link->aspm_capable = link->aspm_support;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 59876de13860..8947cbaf9fa6 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -620,6 +620,10 @@ struct pci_host_bridge {
>  	unsigned int	size_windows:1;		/* Enable root bus sizing */
>  	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
>  
> +#ifdef CONFIG_PCIEASPM
> +	unsigned int	aspm_default_link_state;	/* Controller-provided default */
> +#endif
> +
>  	/* Resource alignment requirements */
>  	resource_size_t (*align_resource)(struct pci_dev *dev,
>  			const struct resource *res,
> @@ -1849,6 +1853,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
>  int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
>  int pci_enable_link_state(struct pci_dev *pdev, int state);
>  int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
> +void pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +					  unsigned int state);
>  void pcie_no_aspm(void);
>  bool pcie_aspm_support_enabled(void);
>  bool pcie_aspm_enabled(struct pci_dev *pdev);
> @@ -1861,6 +1867,9 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
>  { return 0; }
>  static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
>  { return 0; }
> +static inline void
> +pci_host_set_default_pcie_link_state(struct pci_host_bridge *host,
> +				     unsigned int state) { }
>  static inline void pcie_no_aspm(void) { }
>  static inline bool pcie_aspm_support_enabled(void) { return false; }
>  static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }
> 
> base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
> -- 
> 2.43.0
> 

-- 
மணிவண்ணன் சதாசிவம்