[PATCH 0/4] Extend amdgpu_device_pcie_dynamic_switching_supported()

[PATCH 0/4] Extend amdgpu_device_pcie_dynamic_switching_supported()

[Mario Limonciello]

amdgpu_device_pcie_dynamic_switching_supported() currently only covers
SMU13. It sets up the pcietables so that effectively DPM can't change
speed or lane width dynamically on problematic hosts.

Earlier quirks to SMU11 did a similar solution by looking at specific
PCI IDs typically paired with problematic products.

Even earlier dGPUS used in Intel Alder Lake and Rocket lake adopted
similar solutions that would turn off DPM.

These all come down to the same fundmental problem; Intel hosts can't
handle these features. There is nothing to stop someone from taking a
Navi14 and putting it into Sapphire Rapids system and hitting the
same problem that was observed when it was placed into an Alder
Lake-S system.

Because of this; drop all the specific Intel model + AMD dGPU matching
across the driver and instead match ALL Intel hosts to do these
quirks of setting PCIe override parameters or turning off DPM.

If a new Intel host does work well with dynamic speed switching we
can later adjust amdgpu_device_pcie_dynamic_switching_supported() to
have a switch/case where we allow list those hosts, or enumerate all
the broken ones and disallow list them.

Mario Limonciello (4):
  drm/amd: Move helper for dynamic speed switch check out of smu13
  drm/amd: Align SMU11 SMU_MSG_OverridePcieParameters implementation
    with SMU13
  drm/amd: Use amdgpu_device_pcie_dynamic_switching_supported() for SMU7
  drm/amd: Drop amdgpu_device_aspm_support_quirk()

 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    | 30 ++++---
 drivers/gpu/drm/amd/amdgpu/nv.c               |  5 +-
 drivers/gpu/drm/amd/amdgpu/vi.c               |  5 +-
 .../drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c   | 14 +--
 .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 89 ++++---------------
 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c    | 21 +----
 7 files changed, 49 insertions(+), 117 deletions(-)

-- 
2.34.1

[PATCH 1/4] drm/amd: Move helper for dynamic speed switch check out of smu13

[Mario Limonciello]

This helper is used for checking if the connected host supports
the feature, it can be moved into generic code to be used by other
smu implementations as well.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    | 19 +++++++++++++++++
 .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c    | 21 +------------------
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index dc4dc1446a19..813713f42d5e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1313,6 +1313,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
 void amdgpu_device_pci_config_reset(struct amdgpu_device *adev);
 int amdgpu_device_pci_reset(struct amdgpu_device *adev);
 bool amdgpu_device_need_post(struct amdgpu_device *adev);
+bool amdgpu_device_pcie_dynamic_switching_supported(void);
 bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev);
 bool amdgpu_device_aspm_support_quirk(void);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index fcf5f07c4775..7314529553f6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1461,6 +1461,25 @@ bool amdgpu_device_need_post(struct amdgpu_device *adev)
 	return true;
 }
 
+/*
+ * Intel hosts such as Raptor Lake and Sapphire Rapids don't support dynamic
+ * speed switching. Until we have confirmation from Intel that a specific host
+ * supports it, it's safer that we keep it disabled for all.
+ *
+ * https://edc.intel.com/content/www/us/en/design/products/platforms/details/raptor-lake-s/13th-generation-core-processors-datasheet-volume-1-of-2/005/pci-express-support/
+ * https://gitlab.freedesktop.org/drm/amd/-/issues/2663
+ */
+bool amdgpu_device_pcie_dynamic_switching_supported(void)
+{
+#if IS_ENABLED(CONFIG_X86)
+	struct cpuinfo_x86 *c = &cpu_data(0);
+
+	if (c->x86_vendor == X86_VENDOR_INTEL)
+		return false;
+#endif
+	return true;
+}
+
 /**
  * amdgpu_device_should_use_aspm - check if the device should program ASPM
  *
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
index cf7e729020ab..9b62b45ebb7f 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
@@ -2425,25 +2425,6 @@ int smu_v13_0_mode1_reset(struct smu_context *smu)
 	return ret;
 }
 
-/*
- * Intel hosts such as Raptor Lake and Sapphire Rapids don't support dynamic
- * speed switching. Until we have confirmation from Intel that a specific host
- * supports it, it's safer that we keep it disabled for all.
- *
- * https://edc.intel.com/content/www/us/en/design/products/platforms/details/raptor-lake-s/13th-generation-core-processors-datasheet-volume-1-of-2/005/pci-express-support/
- * https://gitlab.freedesktop.org/drm/amd/-/issues/2663
- */
-static bool smu_v13_0_is_pcie_dynamic_switching_supported(void)
-{
-#if IS_ENABLED(CONFIG_X86)
-	struct cpuinfo_x86 *c = &cpu_data(0);
-
-	if (c->x86_vendor == X86_VENDOR_INTEL)
-		return false;
-#endif
-	return true;
-}
-
 int smu_v13_0_update_pcie_parameters(struct smu_context *smu,
 				     uint32_t pcie_gen_cap,
 				     uint32_t pcie_width_cap)
@@ -2455,7 +2436,7 @@ int smu_v13_0_update_pcie_parameters(struct smu_context *smu,
 	uint32_t smu_pcie_arg;
 	int ret, i;
 
-	if (!smu_v13_0_is_pcie_dynamic_switching_supported()) {
+	if (!amdgpu_device_pcie_dynamic_switching_supported()) {
 		if (pcie_table->pcie_gen[num_of_levels - 1] < pcie_gen_cap)
 			pcie_gen_cap = pcie_table->pcie_gen[num_of_levels - 1];
 
-- 
2.34.1

[PATCH 2/4] drm/amd: Align SMU11 SMU_MSG_OverridePcieParameters implementation with SMU13

[Mario Limonciello]

SMU13 overrides dynamic PCIe lane width and dynamic speed by when on
certain hosts. commit 87c617c72628 ("drm/amd/pm: conditionally disable
pcie lane switching for some sienna_cichlid SKUs") worked around this
issue by setting up certain SKUs to set up certain limits, but the same
fundamental problem with those hosts affects all SMU11 implmentations
as well, so align the SMU11 and SMU13 driver handling.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   | 89 ++++---------------
 1 file changed, 18 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index 8fe2e1716da4..f6599c00a6fd 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -2077,89 +2077,36 @@ static int sienna_cichlid_display_disable_memory_clock_switch(struct smu_context
 	return ret;
 }
 
-static void sienna_cichlid_get_override_pcie_settings(struct smu_context *smu,
-						      uint32_t *gen_speed_override,
-						      uint32_t *lane_width_override)
-{
-	struct amdgpu_device *adev = smu->adev;
-
-	*gen_speed_override = 0xff;
-	*lane_width_override = 0xff;
-
-	switch (adev->pdev->device) {
-	case 0x73A0:
-	case 0x73A1:
-	case 0x73A2:
-	case 0x73A3:
-	case 0x73AB:
-	case 0x73AE:
-		/* Bit 7:0: PCIE lane width, 1 to 7 corresponds is x1 to x32 */
-		*lane_width_override = 6;
-		break;
-	case 0x73E0:
-	case 0x73E1:
-	case 0x73E3:
-		*lane_width_override = 4;
-		break;
-	case 0x7420:
-	case 0x7421:
-	case 0x7422:
-	case 0x7423:
-	case 0x7424:
-		*lane_width_override = 3;
-		break;
-	default:
-		break;
-	}
-}
-
-#define MAX(a, b)	((a) > (b) ? (a) : (b))
-
 static int sienna_cichlid_update_pcie_parameters(struct smu_context *smu,
 					 uint32_t pcie_gen_cap,
 					 uint32_t pcie_width_cap)
 {
 	struct smu_11_0_dpm_context *dpm_context = smu->smu_dpm.dpm_context;
 	struct smu_11_0_pcie_table *pcie_table = &dpm_context->dpm_tables.pcie_table;
-	uint32_t gen_speed_override, lane_width_override;
-	uint8_t *table_member1, *table_member2;
-	uint32_t min_gen_speed, max_gen_speed;
-	uint32_t min_lane_width, max_lane_width;
-	uint32_t smu_pcie_arg;
+	u32 smu_pcie_arg;
 	int ret, i;
 
-	GET_PPTABLE_MEMBER(PcieGenSpeed, &table_member1);
-	GET_PPTABLE_MEMBER(PcieLaneCount, &table_member2);
-
-	sienna_cichlid_get_override_pcie_settings(smu,
-						  &gen_speed_override,
-						  &lane_width_override);
+	/* PCIE gen speed and lane width override */
+	if (!amdgpu_device_pcie_dynamic_switching_supported()) {
+		if (pcie_table->pcie_gen[NUM_LINK_LEVELS - 1] < pcie_gen_cap)
+			pcie_gen_cap = pcie_table->pcie_gen[NUM_LINK_LEVELS - 1];
 
-	/* PCIE gen speed override */
-	if (gen_speed_override != 0xff) {
-		min_gen_speed = MIN(pcie_gen_cap, gen_speed_override);
-		max_gen_speed = MIN(pcie_gen_cap, gen_speed_override);
-	} else {
-		min_gen_speed = MAX(0, table_member1[0]);
-		max_gen_speed = MIN(pcie_gen_cap, table_member1[1]);
-		min_gen_speed = min_gen_speed > max_gen_speed ?
-				max_gen_speed : min_gen_speed;
-	}
-	pcie_table->pcie_gen[0] = min_gen_speed;
-	pcie_table->pcie_gen[1] = max_gen_speed;
+		if (pcie_table->pcie_lane[NUM_LINK_LEVELS - 1] < pcie_width_cap)
+			pcie_width_cap = pcie_table->pcie_lane[NUM_LINK_LEVELS - 1];
 
-	/* PCIE lane width override */
-	if (lane_width_override != 0xff) {
-		min_lane_width = MIN(pcie_width_cap, lane_width_override);
-		max_lane_width = MIN(pcie_width_cap, lane_width_override);
+		/* Force all levels to use the same settings */
+		for (i = 0; i < NUM_LINK_LEVELS; i++) {
+			pcie_table->pcie_gen[i] = pcie_gen_cap;
+			pcie_table->pcie_lane[i] = pcie_width_cap;
+		}
 	} else {
-		min_lane_width = MAX(1, table_member2[0]);
-		max_lane_width = MIN(pcie_width_cap, table_member2[1]);
-		min_lane_width = min_lane_width > max_lane_width ?
-				 max_lane_width : min_lane_width;
+		for (i = 0; i < NUM_LINK_LEVELS; i++) {
+			if (pcie_table->pcie_gen[i] > pcie_gen_cap)
+				pcie_table->pcie_gen[i] = pcie_gen_cap;
+			if (pcie_table->pcie_lane[i] > pcie_width_cap)
+				pcie_table->pcie_lane[i] = pcie_width_cap;
+		}
 	}
-	pcie_table->pcie_lane[0] = min_lane_width;
-	pcie_table->pcie_lane[1] = max_lane_width;
 
 	for (i = 0; i < NUM_LINK_LEVELS; i++) {
 		smu_pcie_arg = (i << 16 |
-- 
2.34.1

[PATCH 3/4] drm/amd: Use amdgpu_device_pcie_dynamic_switching_supported() for SMU7

[Mario Limonciello]

SMU7 does a check if the dGPU is inserted into a Rocket Lake system,
to turn off DPM.  Extend this check to all systems that have problems
with dynamic switching by using the
amdgpu_device_pcie_dynamic_switching_supported() helper.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 .../gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c    | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
index 6841a4bce186..1cb402264497 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c
@@ -1798,17 +1798,6 @@ static int smu7_disable_dpm_tasks(struct pp_hwmgr *hwmgr)
 	return result;
 }
 
-static bool intel_core_rkl_chk(void)
-{
-#if IS_ENABLED(CONFIG_X86_64)
-	struct cpuinfo_x86 *c = &cpu_data(0);
-
-	return (c->x86 == 6 && c->x86_model == INTEL_FAM6_ROCKETLAKE);
-#else
-	return false;
-#endif
-}
-
 static void smu7_init_dpm_defaults(struct pp_hwmgr *hwmgr)
 {
 	struct smu7_hwmgr *data = (struct smu7_hwmgr *)(hwmgr->backend);
@@ -1835,7 +1824,8 @@ static void smu7_init_dpm_defaults(struct pp_hwmgr *hwmgr)
 	data->mclk_dpm_key_disabled = hwmgr->feature_mask & PP_MCLK_DPM_MASK ? false : true;
 	data->sclk_dpm_key_disabled = hwmgr->feature_mask & PP_SCLK_DPM_MASK ? false : true;
 	data->pcie_dpm_key_disabled =
-		intel_core_rkl_chk() || !(hwmgr->feature_mask & PP_PCIE_DPM_MASK);
+		!amdgpu_device_pcie_dynamic_switching_supported() ||
+		!(hwmgr->feature_mask & PP_PCIE_DPM_MASK);
 	/* need to set voltage control types before EVV patching */
 	data->voltage_control = SMU7_VOLTAGE_CONTROL_NONE;
 	data->vddci_control = SMU7_VOLTAGE_CONTROL_NONE;
-- 
2.34.1

[PATCH 4/4] drm/amd: Drop amdgpu_device_aspm_support_quirk()

[Mario Limonciello]

NV and VI currently set up a quirk to not enable ASPM on Alder Lake
systems, but the issue appears to be tied to hosts without support
for dynamic speed switching. Migrate both of these over to use
amdgpu_device_pcie_dynamic_switching_supported() instead and drop
amdgpu_device_aspm_support_quirk().

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 -----------
 drivers/gpu/drm/amd/amdgpu/nv.c            |  5 ++++-
 drivers/gpu/drm/amd/amdgpu/vi.c            |  5 ++++-
 4 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 813713f42d5e..6ecf42c4c970 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1315,7 +1315,6 @@ int amdgpu_device_pci_reset(struct amdgpu_device *adev);
 bool amdgpu_device_need_post(struct amdgpu_device *adev);
 bool amdgpu_device_pcie_dynamic_switching_supported(void);
 bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev);
-bool amdgpu_device_aspm_support_quirk(void);
 
 void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes,
 				  u64 num_vis_bytes);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7314529553f6..a9e757f899f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1505,17 +1505,6 @@ bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev)
 	return pcie_aspm_enabled(adev->pdev);
 }
 
-bool amdgpu_device_aspm_support_quirk(void)
-{
-#if IS_ENABLED(CONFIG_X86)
-	struct cpuinfo_x86 *c = &cpu_data(0);
-
-	return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
-#else
-	return true;
-#endif
-}
-
 /* if we get transitioned to only one device, take VGA back */
 /**
  * amdgpu_device_vga_set_decode - enable/disable vga decode
diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
index 51523b27a186..71bc5b2f36cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/nv.c
+++ b/drivers/gpu/drm/amd/amdgpu/nv.c
@@ -527,7 +527,10 @@ static int nv_set_vce_clocks(struct amdgpu_device *adev, u32 evclk, u32 ecclk)
 
 static void nv_program_aspm(struct amdgpu_device *adev)
 {
-	if (!amdgpu_device_should_use_aspm(adev) || !amdgpu_device_aspm_support_quirk())
+	if (!amdgpu_device_should_use_aspm(adev))
+		return;
+
+	if (!amdgpu_device_pcie_dynamic_switching_supported())
 		return;
 
 	if (!(adev->flags & AMD_IS_APU) &&
diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index 6a8494f98d3e..f44c78e69b7f 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -1124,7 +1124,10 @@ static void vi_program_aspm(struct amdgpu_device *adev)
 	bool bL1SS = false;
 	bool bClkReqSupport = true;
 
-	if (!amdgpu_device_should_use_aspm(adev) || !amdgpu_device_aspm_support_quirk())
+	if (!amdgpu_device_should_use_aspm(adev))
+		return;
+
+	if (!amdgpu_device_pcie_dynamic_switching_supported())
 		return;
 
 	if (adev->flags & AMD_IS_APU ||
-- 
2.34.1

RE: [PATCH 4/4] drm/amd: Drop amdgpu_device_aspm_support_quirk()

[Quan, Evan]

[AMD Official Use Only - General]

Patch1, 2, 3 are reviewed-by: Evan Quan <evan.quan@amd.com>

For patch4, it seems not quite right(at least for the naming).
Since although the ASPM is the prerequisite for pcie/lclk dpm features.
But the changes involved here are really for aspm feature disablement.
I mean even if pcie dynamic lane/speed switching is not supported, aspm feature can be still enabled.
So, using "amdgpu_device_pcie_dynamic_switching_supported" for the determination whether aspm feature can be enabled seems not proper.

Evan
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Saturday, July 8, 2023 10:26 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan <Evan.Quan@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Subject: [PATCH 4/4] drm/amd: Drop amdgpu_device_aspm_support_quirk()
>
> NV and VI currently set up a quirk to not enable ASPM on Alder Lake
> systems, but the issue appears to be tied to hosts without support
> for dynamic speed switching. Migrate both of these over to use
> amdgpu_device_pcie_dynamic_switching_supported() instead and drop
> amdgpu_device_aspm_support_quirk().
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 -----------
>  drivers/gpu/drm/amd/amdgpu/nv.c            |  5 ++++-
>  drivers/gpu/drm/amd/amdgpu/vi.c            |  5 ++++-
>  4 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 813713f42d5e..6ecf42c4c970 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1315,7 +1315,6 @@ int amdgpu_device_pci_reset(struct
> amdgpu_device *adev);
>  bool amdgpu_device_need_post(struct amdgpu_device *adev);
>  bool amdgpu_device_pcie_dynamic_switching_supported(void);
>  bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev);
> -bool amdgpu_device_aspm_support_quirk(void);
>
>  void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64
> num_bytes,
>                                 u64 num_vis_bytes);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7314529553f6..a9e757f899f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1505,17 +1505,6 @@ bool amdgpu_device_should_use_aspm(struct
> amdgpu_device *adev)
>       return pcie_aspm_enabled(adev->pdev);
>  }
>
> -bool amdgpu_device_aspm_support_quirk(void)
> -{
> -#if IS_ENABLED(CONFIG_X86)
> -     struct cpuinfo_x86 *c = &cpu_data(0);
> -
> -     return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
> -#else
> -     return true;
> -#endif
> -}
> -
>  /* if we get transitioned to only one device, take VGA back */
>  /**
>   * amdgpu_device_vga_set_decode - enable/disable vga decode
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
> b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 51523b27a186..71bc5b2f36cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -527,7 +527,10 @@ static int nv_set_vce_clocks(struct amdgpu_device
> *adev, u32 evclk, u32 ecclk)
>
>  static void nv_program_aspm(struct amdgpu_device *adev)
>  {
> -     if (!amdgpu_device_should_use_aspm(adev)
> || !amdgpu_device_aspm_support_quirk())
> +     if (!amdgpu_device_should_use_aspm(adev))
> +             return;
> +
> +     if (!amdgpu_device_pcie_dynamic_switching_supported())
>               return;
>
>       if (!(adev->flags & AMD_IS_APU) &&
> diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c
> b/drivers/gpu/drm/amd/amdgpu/vi.c
> index 6a8494f98d3e..f44c78e69b7f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> @@ -1124,7 +1124,10 @@ static void vi_program_aspm(struct
> amdgpu_device *adev)
>       bool bL1SS = false;
>       bool bClkReqSupport = true;
>
> -     if (!amdgpu_device_should_use_aspm(adev)
> || !amdgpu_device_aspm_support_quirk())
> +     if (!amdgpu_device_should_use_aspm(adev))
> +             return;
> +
> +     if (!amdgpu_device_pcie_dynamic_switching_supported())
>               return;
>
>       if (adev->flags & AMD_IS_APU ||
> --
> 2.34.1

RE: [PATCH 4/4] drm/amd: Drop amdgpu_device_aspm_support_quirk()

[Limonciello, Mario]

[AMD Official Use Only - General]

OK, will pick up 1/2/3 and continue to think about 4.

> -----Original Message-----
> From: Quan, Evan <Evan.Quan@amd.com>
> Sent: Sunday, July 9, 2023 20:54
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; amd-
> gfx@lists.freedesktop.org
> Subject: RE: [PATCH 4/4] drm/amd: Drop
> amdgpu_device_aspm_support_quirk()
>
> [AMD Official Use Only - General]
>
> Patch1, 2, 3 are reviewed-by: Evan Quan <evan.quan@amd.com>
>
> For patch4, it seems not quite right(at least for the naming).
> Since although the ASPM is the prerequisite for pcie/lclk dpm features.
> But the changes involved here are really for aspm feature disablement.
> I mean even if pcie dynamic lane/speed switching is not supported, aspm
> feature can be still enabled.
> So, using "amdgpu_device_pcie_dynamic_switching_supported" for the
> determination whether aspm feature can be enabled seems not proper.
>
> Evan
> > -----Original Message-----
> > From: Limonciello, Mario <Mario.Limonciello@amd.com>
> > Sent: Saturday, July 8, 2023 10:26 AM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Quan, Evan <Evan.Quan@amd.com>; Limonciello, Mario
> > <Mario.Limonciello@amd.com>
> > Subject: [PATCH 4/4] drm/amd: Drop
> amdgpu_device_aspm_support_quirk()
> >
> > NV and VI currently set up a quirk to not enable ASPM on Alder Lake
> > systems, but the issue appears to be tied to hosts without support
> > for dynamic speed switching. Migrate both of these over to use
> > amdgpu_device_pcie_dynamic_switching_supported() instead and drop
> > amdgpu_device_aspm_support_quirk().
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 -
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 11 -----------
> >  drivers/gpu/drm/amd/amdgpu/nv.c            |  5 ++++-
> >  drivers/gpu/drm/amd/amdgpu/vi.c            |  5 ++++-
> >  4 files changed, 8 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index 813713f42d5e..6ecf42c4c970 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -1315,7 +1315,6 @@ int amdgpu_device_pci_reset(struct
> > amdgpu_device *adev);
> >  bool amdgpu_device_need_post(struct amdgpu_device *adev);
> >  bool amdgpu_device_pcie_dynamic_switching_supported(void);
> >  bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev);
> > -bool amdgpu_device_aspm_support_quirk(void);
> >
> >  void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64
> > num_bytes,
> >                                 u64 num_vis_bytes);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 7314529553f6..a9e757f899f2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -1505,17 +1505,6 @@ bool amdgpu_device_should_use_aspm(struct
> > amdgpu_device *adev)
> >       return pcie_aspm_enabled(adev->pdev);
> >  }
> >
> > -bool amdgpu_device_aspm_support_quirk(void)
> > -{
> > -#if IS_ENABLED(CONFIG_X86)
> > -     struct cpuinfo_x86 *c = &cpu_data(0);
> > -
> > -     return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
> > -#else
> > -     return true;
> > -#endif
> > -}
> > -
> >  /* if we get transitioned to only one device, take VGA back */
> >  /**
> >   * amdgpu_device_vga_set_decode - enable/disable vga decode
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
> > b/drivers/gpu/drm/amd/amdgpu/nv.c
> > index 51523b27a186..71bc5b2f36cf 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> > @@ -527,7 +527,10 @@ static int nv_set_vce_clocks(struct amdgpu_device
> > *adev, u32 evclk, u32 ecclk)
> >
> >  static void nv_program_aspm(struct amdgpu_device *adev)
> >  {
> > -     if (!amdgpu_device_should_use_aspm(adev)
> > || !amdgpu_device_aspm_support_quirk())
> > +     if (!amdgpu_device_should_use_aspm(adev))
> > +             return;
> > +
> > +     if (!amdgpu_device_pcie_dynamic_switching_supported())
> >               return;
> >
> >       if (!(adev->flags & AMD_IS_APU) &&
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c
> > b/drivers/gpu/drm/amd/amdgpu/vi.c
> > index 6a8494f98d3e..f44c78e69b7f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> > @@ -1124,7 +1124,10 @@ static void vi_program_aspm(struct
> > amdgpu_device *adev)
> >       bool bL1SS = false;
> >       bool bClkReqSupport = true;
> >
> > -     if (!amdgpu_device_should_use_aspm(adev)
> > || !amdgpu_device_aspm_support_quirk())
> > +     if (!amdgpu_device_should_use_aspm(adev))
> > +             return;
> > +
> > +     if (!amdgpu_device_pcie_dynamic_switching_supported())
> >               return;
> >
> >       if (adev->flags & AMD_IS_APU ||
> > --
> > 2.34.1
>