* [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs
@ 2023-04-21 7:28 Evan Quan
2023-04-21 7:32 ` Quan, Evan
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Evan Quan @ 2023-04-21 7:28 UTC (permalink / raw)
To: amd-gfx; +Cc: Alexander.Deucher, Evan Quan, Mario.Limonciello
Disable the pcie lane switching for some sienna_cichlid SKUs since it
might not work well on some platforms.
Signed-off-by: Evan Quan <evan.quan@amd.com>
Change-Id: Iea9ceaa146c8706768ee077c10e5d33bce9bc1c2
---
.../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 92 +++++++++++++++----
1 file changed, 74 insertions(+), 18 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 4b91cdc3eaa0..e7223513e384 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
@@ -2067,33 +2067,94 @@ 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;
-
- uint32_t smu_pcie_arg;
+ 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;
int ret, i;
GET_PPTABLE_MEMBER(PcieGenSpeed, &table_member1);
GET_PPTABLE_MEMBER(PcieLaneCount, &table_member2);
- /* lclk dpm table setup */
- for (i = 0; i < MAX_PCIE_CONF; i++) {
- dpm_context->dpm_tables.pcie_table.pcie_gen[i] = table_member1[i];
- dpm_context->dpm_tables.pcie_table.pcie_lane[i] = table_member2[i];
+ sienna_cichlid_get_override_pcie_settings(smu,
+ &gen_speed_override,
+ &lane_width_override);
+
+ /* 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;
+
+ /* 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);
+ } 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;
+ }
+ 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) |
- ((table_member1[i] <= pcie_gen_cap) ?
- (table_member1[i] << 8) :
- (pcie_gen_cap << 8)) |
- ((table_member2[i] <= pcie_width_cap) ?
- table_member2[i] :
- pcie_width_cap);
+ smu_pcie_arg = (i << 16 |
+ pcie_table->pcie_gen[i] << 8 |
+ pcie_table->pcie_lane[i]);
ret = smu_cmn_send_smc_msg_with_param(smu,
SMU_MSG_OverridePcieParameters,
@@ -2101,11 +2162,6 @@ static int sienna_cichlid_update_pcie_parameters(struct smu_context *smu,
NULL);
if (ret)
return ret;
-
- if (table_member1[i] > pcie_gen_cap)
- dpm_context->dpm_tables.pcie_table.pcie_gen[i] = pcie_gen_cap;
- if (table_member2[i] > pcie_width_cap)
- dpm_context->dpm_tables.pcie_table.pcie_lane[i] = pcie_width_cap;
}
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs
2023-04-21 7:28 [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs Evan Quan
@ 2023-04-21 7:32 ` Quan, Evan
2023-04-21 9:30 ` Lazar, Lijo
2023-05-31 9:54 ` Quan, Evan
2023-04-21 13:39 ` Limonciello, Mario
2023-04-21 13:44 ` Alex Deucher
2 siblings, 2 replies; 9+ messages in thread
From: Quan, Evan @ 2023-04-21 7:32 UTC (permalink / raw)
To: amd-gfx; +Cc: Deucher, Alexander, Limonciello, Mario
[AMD Official Use Only - General]
This seems able to address some audio noise issue observed per customer's feedback.
Evan
> -----Original Message-----
> From: Quan, Evan <Evan.Quan@amd.com>
> Sent: Friday, April 21, 2023 3:29 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Quan, Evan <Evan.Quan@amd.com>
> Subject: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for
> some sienna_cichlid SKUs
>
> Disable the pcie lane switching for some sienna_cichlid SKUs since it
> might not work well on some platforms.
>
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> Change-Id: Iea9ceaa146c8706768ee077c10e5d33bce9bc1c2
> ---
> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 92 +++++++++++++++---
> -
> 1 file changed, 74 insertions(+), 18 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 4b91cdc3eaa0..e7223513e384 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
> @@ -2067,33 +2067,94 @@ 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;
> -
> - uint32_t smu_pcie_arg;
> + 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;
> int ret, i;
>
> GET_PPTABLE_MEMBER(PcieGenSpeed, &table_member1);
> GET_PPTABLE_MEMBER(PcieLaneCount, &table_member2);
>
> - /* lclk dpm table setup */
> - for (i = 0; i < MAX_PCIE_CONF; i++) {
> - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> table_member1[i];
> - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> table_member2[i];
> + sienna_cichlid_get_override_pcie_settings(smu,
> + &gen_speed_override,
> + &lane_width_override);
> +
> + /* 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;
> +
> + /* 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);
> + } 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;
> + }
> + 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) |
> - ((table_member1[i] <= pcie_gen_cap) ?
> - (table_member1[i] << 8) :
> - (pcie_gen_cap << 8)) |
> - ((table_member2[i] <= pcie_width_cap) ?
> - table_member2[i] :
> - pcie_width_cap);
> + smu_pcie_arg = (i << 16 |
> + pcie_table->pcie_gen[i] << 8 |
> + pcie_table->pcie_lane[i]);
>
> ret = smu_cmn_send_smc_msg_with_param(smu,
> SMU_MSG_OverridePcieParameters,
> @@ -2101,11 +2162,6 @@ static int
> sienna_cichlid_update_pcie_parameters(struct smu_context *smu,
> NULL);
> if (ret)
> return ret;
> -
> - if (table_member1[i] > pcie_gen_cap)
> - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> pcie_gen_cap;
> - if (table_member2[i] > pcie_width_cap)
> - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> pcie_width_cap;
> }
>
> return 0;
> --
> 2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs
2023-04-21 7:32 ` Quan, Evan
@ 2023-04-21 9:30 ` Lazar, Lijo
2023-05-31 9:54 ` Quan, Evan
1 sibling, 0 replies; 9+ messages in thread
From: Lazar, Lijo @ 2023-04-21 9:30 UTC (permalink / raw)
To: Quan, Evan, amd-gfx; +Cc: Deucher, Alexander, Limonciello, Mario
On 4/21/2023 1:02 PM, Quan, Evan wrote:
> [AMD Official Use Only - General]
>
> This seems able to address some audio noise issue observed per customer's feedback.
>
> Evan
>> -----Original Message-----
>> From: Quan, Evan <Evan.Quan@amd.com>
>> Sent: Friday, April 21, 2023 3:29 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario
>> <Mario.Limonciello@amd.com>; Quan, Evan <Evan.Quan@amd.com>
>> Subject: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for
>> some sienna_cichlid SKUs
>>
>> Disable the pcie lane switching for some sienna_cichlid SKUs since it
>> might not work well on some platforms.
>>
>> Signed-off-by: Evan Quan <evan.quan@amd.com>
>> Change-Id: Iea9ceaa146c8706768ee077c10e5d33bce9bc1c2
>> ---
>> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 92 +++++++++++++++---
>> -
>> 1 file changed, 74 insertions(+), 18 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 4b91cdc3eaa0..e7223513e384 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
>> @@ -2067,33 +2067,94 @@ 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;
>> + }
>> +}
>> +
Isn't this applicable only on Intel platforms? If so, better to do a
processor check rather than maintaining max width based on DID.
Thanks,
Lijo
>> +#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;
>> -
>> - uint32_t smu_pcie_arg;
>> + 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;
>> int ret, i;
>>
>> GET_PPTABLE_MEMBER(PcieGenSpeed, &table_member1);
>> GET_PPTABLE_MEMBER(PcieLaneCount, &table_member2);
>>
>> - /* lclk dpm table setup */
>> - for (i = 0; i < MAX_PCIE_CONF; i++) {
>> - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
>> table_member1[i];
>> - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
>> table_member2[i];
>> + sienna_cichlid_get_override_pcie_settings(smu,
>> + &gen_speed_override,
>> + &lane_width_override);
>> +
>> + /* 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;
>> +
>> + /* 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);
>> + } 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;
>> + }
>> + 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) |
>> - ((table_member1[i] <= pcie_gen_cap) ?
>> - (table_member1[i] << 8) :
>> - (pcie_gen_cap << 8)) |
>> - ((table_member2[i] <= pcie_width_cap) ?
>> - table_member2[i] :
>> - pcie_width_cap);
>> + smu_pcie_arg = (i << 16 |
>> + pcie_table->pcie_gen[i] << 8 |
>> + pcie_table->pcie_lane[i]);
>>
>> ret = smu_cmn_send_smc_msg_with_param(smu,
>> SMU_MSG_OverridePcieParameters,
>> @@ -2101,11 +2162,6 @@ static int
>> sienna_cichlid_update_pcie_parameters(struct smu_context *smu,
>> NULL);
>> if (ret)
>> return ret;
>> -
>> - if (table_member1[i] > pcie_gen_cap)
>> - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
>> pcie_gen_cap;
>> - if (table_member2[i] > pcie_width_cap)
>> - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
>> pcie_width_cap;
>> }
>>
>> return 0;
>> --
>> 2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs
2023-04-21 7:28 [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs Evan Quan
2023-04-21 7:32 ` Quan, Evan
@ 2023-04-21 13:39 ` Limonciello, Mario
2023-04-24 7:33 ` Quan, Evan
2023-04-21 13:44 ` Alex Deucher
2 siblings, 1 reply; 9+ messages in thread
From: Limonciello, Mario @ 2023-04-21 13:39 UTC (permalink / raw)
To: Quan, Evan, amd-gfx; +Cc: Deucher, Alexander
[Public]
> -----Original Message-----
> From: Quan, Evan <Evan.Quan@amd.com>
> Sent: Friday, April 21, 2023 02:29
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Quan, Evan <Evan.Quan@amd.com>
> Subject: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for
> some sienna_cichlid SKUs
>
> Disable the pcie lane switching for some sienna_cichlid SKUs since it
> might not work well on some platforms.
>
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> Change-Id: Iea9ceaa146c8706768ee077c10e5d33bce9bc1c2
You can drop the Gerrit Change-Id here
> ---
> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 92 +++++++++++++++----
> 1 file changed, 74 insertions(+), 18 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 4b91cdc3eaa0..e7223513e384 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
> @@ -2067,33 +2067,94 @@ 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;
> -
> - uint32_t smu_pcie_arg;
> + 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;
> int ret, i;
>
> GET_PPTABLE_MEMBER(PcieGenSpeed, &table_member1);
> GET_PPTABLE_MEMBER(PcieLaneCount, &table_member2);
>
> - /* lclk dpm table setup */
> - for (i = 0; i < MAX_PCIE_CONF; i++) {
> - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> table_member1[i];
> - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> table_member2[i];
> + sienna_cichlid_get_override_pcie_settings(smu,
> + &gen_speed_override,
> + &lane_width_override);
> +
> + /* 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;
> +
> + /* 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);
> + } 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;
> + }
> + 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) |
> - ((table_member1[i] <= pcie_gen_cap) ?
> - (table_member1[i] << 8) :
> - (pcie_gen_cap << 8)) |
> - ((table_member2[i] <= pcie_width_cap) ?
> - table_member2[i] :
> - pcie_width_cap);
> + smu_pcie_arg = (i << 16 |
> + pcie_table->pcie_gen[i] << 8 |
> + pcie_table->pcie_lane[i]);
>
> ret = smu_cmn_send_smc_msg_with_param(smu,
> SMU_MSG_OverridePcieParameters,
> @@ -2101,11 +2162,6 @@ static int
> sienna_cichlid_update_pcie_parameters(struct smu_context *smu,
> NULL);
> if (ret)
> return ret;
> -
> - if (table_member1[i] > pcie_gen_cap)
> - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> pcie_gen_cap;
> - if (table_member2[i] > pcie_width_cap)
> - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> pcie_width_cap;
> }
>
> return 0;
> --
> 2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs
2023-04-21 7:28 [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs Evan Quan
2023-04-21 7:32 ` Quan, Evan
2023-04-21 13:39 ` Limonciello, Mario
@ 2023-04-21 13:44 ` Alex Deucher
2 siblings, 0 replies; 9+ messages in thread
From: Alex Deucher @ 2023-04-21 13:44 UTC (permalink / raw)
To: Evan Quan; +Cc: Alexander.Deucher, Mario.Limonciello, amd-gfx
On Fri, Apr 21, 2023 at 3:30 AM Evan Quan <evan.quan@amd.com> wrote:
>
> Disable the pcie lane switching for some sienna_cichlid SKUs since it
> might not work well on some platforms.
>
> Signed-off-by: Evan Quan <evan.quan@amd.com>
> Change-Id: Iea9ceaa146c8706768ee077c10e5d33bce9bc1c2
> ---
> .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 92 +++++++++++++++----
> 1 file changed, 74 insertions(+), 18 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 4b91cdc3eaa0..e7223513e384 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
> @@ -2067,33 +2067,94 @@ 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;
> + }
> +}
Why not just apply this to all SKUs? Or use the pcie info we fetched
at init time?
> +
> +#define MAX(a, b) ((a) > (b) ? (a) : (b))
There is already a max macro in the kernel you can use.
> +
> 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;
> -
> - uint32_t smu_pcie_arg;
> + 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;
> int ret, i;
>
> GET_PPTABLE_MEMBER(PcieGenSpeed, &table_member1);
> GET_PPTABLE_MEMBER(PcieLaneCount, &table_member2);
>
> - /* lclk dpm table setup */
> - for (i = 0; i < MAX_PCIE_CONF; i++) {
> - dpm_context->dpm_tables.pcie_table.pcie_gen[i] = table_member1[i];
> - dpm_context->dpm_tables.pcie_table.pcie_lane[i] = table_member2[i];
> + sienna_cichlid_get_override_pcie_settings(smu,
> + &gen_speed_override,
> + &lane_width_override);
> +
> + /* 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;
> +
> + /* 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);
> + } 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;
> + }
> + 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) |
> - ((table_member1[i] <= pcie_gen_cap) ?
> - (table_member1[i] << 8) :
> - (pcie_gen_cap << 8)) |
> - ((table_member2[i] <= pcie_width_cap) ?
> - table_member2[i] :
> - pcie_width_cap);
> + smu_pcie_arg = (i << 16 |
> + pcie_table->pcie_gen[i] << 8 |
> + pcie_table->pcie_lane[i]);
>
> ret = smu_cmn_send_smc_msg_with_param(smu,
> SMU_MSG_OverridePcieParameters,
> @@ -2101,11 +2162,6 @@ static int sienna_cichlid_update_pcie_parameters(struct smu_context *smu,
> NULL);
> if (ret)
> return ret;
> -
> - if (table_member1[i] > pcie_gen_cap)
> - dpm_context->dpm_tables.pcie_table.pcie_gen[i] = pcie_gen_cap;
> - if (table_member2[i] > pcie_width_cap)
> - dpm_context->dpm_tables.pcie_table.pcie_lane[i] = pcie_width_cap;
> }
>
> return 0;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs
2023-04-21 13:39 ` Limonciello, Mario
@ 2023-04-24 7:33 ` Quan, Evan
0 siblings, 0 replies; 9+ messages in thread
From: Quan, Evan @ 2023-04-24 7:33 UTC (permalink / raw)
To: Limonciello, Mario, amd-gfx; +Cc: Deucher, Alexander
[Public]
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Friday, April 21, 2023 9:40 PM
> To: Quan, Evan <Evan.Quan@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
> Subject: RE: [PATCH] drm/amd/pm: conditionally disable pcie lane switching
> for some sienna_cichlid SKUs
>
> [Public]
>
>
>
> > -----Original Message-----
> > From: Quan, Evan <Evan.Quan@amd.com>
> > Sent: Friday, April 21, 2023 02:29
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello,
> Mario
> > <Mario.Limonciello@amd.com>; Quan, Evan <Evan.Quan@amd.com>
> > Subject: [PATCH] drm/amd/pm: conditionally disable pcie lane switching
> > for some sienna_cichlid SKUs
> >
> > Disable the pcie lane switching for some sienna_cichlid SKUs since it
> > might not work well on some platforms.
> >
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > Change-Id: Iea9ceaa146c8706768ee077c10e5d33bce9bc1c2
>
> You can drop the Gerrit Change-Id here
Sure, thanks.
Evan
>
> > ---
> > .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 92
> +++++++++++++++----
> > 1 file changed, 74 insertions(+), 18 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 4b91cdc3eaa0..e7223513e384 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
> > @@ -2067,33 +2067,94 @@ 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;
> > -
> > - uint32_t smu_pcie_arg;
> > + 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;
> > int ret, i;
> >
> > GET_PPTABLE_MEMBER(PcieGenSpeed, &table_member1);
> > GET_PPTABLE_MEMBER(PcieLaneCount, &table_member2);
> >
> > - /* lclk dpm table setup */
> > - for (i = 0; i < MAX_PCIE_CONF; i++) {
> > - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> > table_member1[i];
> > - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> > table_member2[i];
> > + sienna_cichlid_get_override_pcie_settings(smu,
> > + &gen_speed_override,
> > + &lane_width_override);
> > +
> > + /* 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;
> > +
> > + /* 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);
> > + } 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;
> > + }
> > + 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) |
> > - ((table_member1[i] <= pcie_gen_cap) ?
> > - (table_member1[i] << 8) :
> > - (pcie_gen_cap << 8)) |
> > - ((table_member2[i] <= pcie_width_cap) ?
> > - table_member2[i] :
> > - pcie_width_cap);
> > + smu_pcie_arg = (i << 16 |
> > + pcie_table->pcie_gen[i] << 8 |
> > + pcie_table->pcie_lane[i]);
> >
> > ret = smu_cmn_send_smc_msg_with_param(smu,
> > SMU_MSG_OverridePcieParameters,
> > @@ -2101,11 +2162,6 @@ static int
> > sienna_cichlid_update_pcie_parameters(struct smu_context *smu,
> > NULL);
> > if (ret)
> > return ret;
> > -
> > - if (table_member1[i] > pcie_gen_cap)
> > - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> > pcie_gen_cap;
> > - if (table_member2[i] > pcie_width_cap)
> > - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> > pcie_width_cap;
> > }
> >
> > return 0;
> > --
> > 2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs
2023-04-21 7:32 ` Quan, Evan
2023-04-21 9:30 ` Lazar, Lijo
@ 2023-05-31 9:54 ` Quan, Evan
2023-05-31 12:12 ` Alex Deucher
1 sibling, 1 reply; 9+ messages in thread
From: Quan, Evan @ 2023-05-31 9:54 UTC (permalink / raw)
To: amd-gfx; +Cc: Deucher, Alexander, Limonciello, Mario
[AMD Official Use Only - General]
Hi Alex,
Can we land this as a temporary solution while we are seeking a more proper one?
This is gating our customer and I was pushed for a solution.
BR,
Evan
> -----Original Message-----
> From: Quan, Evan
> Sent: Friday, April 21, 2023 3:32 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>
> Subject: RE: [PATCH] drm/amd/pm: conditionally disable pcie lane switching
> for some sienna_cichlid SKUs
>
> [AMD Official Use Only - General]
>
> This seems able to address some audio noise issue observed per customer's
> feedback.
>
> Evan
> > -----Original Message-----
> > From: Quan, Evan <Evan.Quan@amd.com>
> > Sent: Friday, April 21, 2023 3:29 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario
> > <Mario.Limonciello@amd.com>; Quan, Evan <Evan.Quan@amd.com>
> > Subject: [PATCH] drm/amd/pm: conditionally disable pcie lane switching
> > for some sienna_cichlid SKUs
> >
> > Disable the pcie lane switching for some sienna_cichlid SKUs since it
> > might not work well on some platforms.
> >
> > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > Change-Id: Iea9ceaa146c8706768ee077c10e5d33bce9bc1c2
> > ---
> > .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 92 +++++++++++++++-
> --
> > -
> > 1 file changed, 74 insertions(+), 18 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 4b91cdc3eaa0..e7223513e384 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
> > @@ -2067,33 +2067,94 @@ 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;
> > -
> > - uint32_t smu_pcie_arg;
> > + 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;
> > int ret, i;
> >
> > GET_PPTABLE_MEMBER(PcieGenSpeed, &table_member1);
> > GET_PPTABLE_MEMBER(PcieLaneCount, &table_member2);
> >
> > - /* lclk dpm table setup */
> > - for (i = 0; i < MAX_PCIE_CONF; i++) {
> > - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> > table_member1[i];
> > - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> > table_member2[i];
> > + sienna_cichlid_get_override_pcie_settings(smu,
> > + &gen_speed_override,
> > + &lane_width_override);
> > +
> > + /* 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;
> > +
> > + /* 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);
> > + } 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;
> > + }
> > + 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) |
> > - ((table_member1[i] <= pcie_gen_cap) ?
> > - (table_member1[i] << 8) :
> > - (pcie_gen_cap << 8)) |
> > - ((table_member2[i] <= pcie_width_cap) ?
> > - table_member2[i] :
> > - pcie_width_cap);
> > + smu_pcie_arg = (i << 16 |
> > + pcie_table->pcie_gen[i] << 8 |
> > + pcie_table->pcie_lane[i]);
> >
> > ret = smu_cmn_send_smc_msg_with_param(smu,
> > SMU_MSG_OverridePcieParameters,
> > @@ -2101,11 +2162,6 @@ static int
> > sienna_cichlid_update_pcie_parameters(struct smu_context *smu,
> > NULL);
> > if (ret)
> > return ret;
> > -
> > - if (table_member1[i] > pcie_gen_cap)
> > - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> > pcie_gen_cap;
> > - if (table_member2[i] > pcie_width_cap)
> > - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> > pcie_width_cap;
> > }
> >
> > return 0;
> > --
> > 2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs
2023-05-31 9:54 ` Quan, Evan
@ 2023-05-31 12:12 ` Alex Deucher
2023-05-31 12:48 ` Alex Deucher
0 siblings, 1 reply; 9+ messages in thread
From: Alex Deucher @ 2023-05-31 12:12 UTC (permalink / raw)
To: Quan, Evan; +Cc: Deucher, Alexander, Limonciello, Mario, amd-gfx
Sure. Please go ahead.
Alex
On Wed, May 31, 2023 at 5:54 AM Quan, Evan <Evan.Quan@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> Hi Alex,
>
> Can we land this as a temporary solution while we are seeking a more proper one?
> This is gating our customer and I was pushed for a solution.
>
> BR,
> Evan
> > -----Original Message-----
> > From: Quan, Evan
> > Sent: Friday, April 21, 2023 3:32 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario
> > <Mario.Limonciello@amd.com>
> > Subject: RE: [PATCH] drm/amd/pm: conditionally disable pcie lane switching
> > for some sienna_cichlid SKUs
> >
> > [AMD Official Use Only - General]
> >
> > This seems able to address some audio noise issue observed per customer's
> > feedback.
> >
> > Evan
> > > -----Original Message-----
> > > From: Quan, Evan <Evan.Quan@amd.com>
> > > Sent: Friday, April 21, 2023 3:29 PM
> > > To: amd-gfx@lists.freedesktop.org
> > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario
> > > <Mario.Limonciello@amd.com>; Quan, Evan <Evan.Quan@amd.com>
> > > Subject: [PATCH] drm/amd/pm: conditionally disable pcie lane switching
> > > for some sienna_cichlid SKUs
> > >
> > > Disable the pcie lane switching for some sienna_cichlid SKUs since it
> > > might not work well on some platforms.
> > >
> > > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > > Change-Id: Iea9ceaa146c8706768ee077c10e5d33bce9bc1c2
> > > ---
> > > .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 92 +++++++++++++++-
> > --
> > > -
> > > 1 file changed, 74 insertions(+), 18 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 4b91cdc3eaa0..e7223513e384 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
> > > @@ -2067,33 +2067,94 @@ 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;
> > > -
> > > - uint32_t smu_pcie_arg;
> > > + 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;
> > > int ret, i;
> > >
> > > GET_PPTABLE_MEMBER(PcieGenSpeed, &table_member1);
> > > GET_PPTABLE_MEMBER(PcieLaneCount, &table_member2);
> > >
> > > - /* lclk dpm table setup */
> > > - for (i = 0; i < MAX_PCIE_CONF; i++) {
> > > - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> > > table_member1[i];
> > > - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> > > table_member2[i];
> > > + sienna_cichlid_get_override_pcie_settings(smu,
> > > + &gen_speed_override,
> > > + &lane_width_override);
> > > +
> > > + /* 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;
> > > +
> > > + /* 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);
> > > + } 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;
> > > + }
> > > + 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) |
> > > - ((table_member1[i] <= pcie_gen_cap) ?
> > > - (table_member1[i] << 8) :
> > > - (pcie_gen_cap << 8)) |
> > > - ((table_member2[i] <= pcie_width_cap) ?
> > > - table_member2[i] :
> > > - pcie_width_cap);
> > > + smu_pcie_arg = (i << 16 |
> > > + pcie_table->pcie_gen[i] << 8 |
> > > + pcie_table->pcie_lane[i]);
> > >
> > > ret = smu_cmn_send_smc_msg_with_param(smu,
> > > SMU_MSG_OverridePcieParameters,
> > > @@ -2101,11 +2162,6 @@ static int
> > > sienna_cichlid_update_pcie_parameters(struct smu_context *smu,
> > > NULL);
> > > if (ret)
> > > return ret;
> > > -
> > > - if (table_member1[i] > pcie_gen_cap)
> > > - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> > > pcie_gen_cap;
> > > - if (table_member2[i] > pcie_width_cap)
> > > - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> > > pcie_width_cap;
> > > }
> > >
> > > return 0;
> > > --
> > > 2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs
2023-05-31 12:12 ` Alex Deucher
@ 2023-05-31 12:48 ` Alex Deucher
0 siblings, 0 replies; 9+ messages in thread
From: Alex Deucher @ 2023-05-31 12:48 UTC (permalink / raw)
To: Quan, Evan; +Cc: Deucher, Alexander, Limonciello, Mario, amd-gfx
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
On Wed, May 31, 2023 at 8:12 AM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> Sure. Please go ahead.
>
> Alex
>
> On Wed, May 31, 2023 at 5:54 AM Quan, Evan <Evan.Quan@amd.com> wrote:
> >
> > [AMD Official Use Only - General]
> >
> > Hi Alex,
> >
> > Can we land this as a temporary solution while we are seeking a more proper one?
> > This is gating our customer and I was pushed for a solution.
> >
> > BR,
> > Evan
> > > -----Original Message-----
> > > From: Quan, Evan
> > > Sent: Friday, April 21, 2023 3:32 PM
> > > To: amd-gfx@lists.freedesktop.org
> > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario
> > > <Mario.Limonciello@amd.com>
> > > Subject: RE: [PATCH] drm/amd/pm: conditionally disable pcie lane switching
> > > for some sienna_cichlid SKUs
> > >
> > > [AMD Official Use Only - General]
> > >
> > > This seems able to address some audio noise issue observed per customer's
> > > feedback.
> > >
> > > Evan
> > > > -----Original Message-----
> > > > From: Quan, Evan <Evan.Quan@amd.com>
> > > > Sent: Friday, April 21, 2023 3:29 PM
> > > > To: amd-gfx@lists.freedesktop.org
> > > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Limonciello, Mario
> > > > <Mario.Limonciello@amd.com>; Quan, Evan <Evan.Quan@amd.com>
> > > > Subject: [PATCH] drm/amd/pm: conditionally disable pcie lane switching
> > > > for some sienna_cichlid SKUs
> > > >
> > > > Disable the pcie lane switching for some sienna_cichlid SKUs since it
> > > > might not work well on some platforms.
> > > >
> > > > Signed-off-by: Evan Quan <evan.quan@amd.com>
> > > > Change-Id: Iea9ceaa146c8706768ee077c10e5d33bce9bc1c2
> > > > ---
> > > > .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 92 +++++++++++++++-
> > > --
> > > > -
> > > > 1 file changed, 74 insertions(+), 18 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 4b91cdc3eaa0..e7223513e384 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
> > > > @@ -2067,33 +2067,94 @@ 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;
> > > > -
> > > > - uint32_t smu_pcie_arg;
> > > > + 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;
> > > > int ret, i;
> > > >
> > > > GET_PPTABLE_MEMBER(PcieGenSpeed, &table_member1);
> > > > GET_PPTABLE_MEMBER(PcieLaneCount, &table_member2);
> > > >
> > > > - /* lclk dpm table setup */
> > > > - for (i = 0; i < MAX_PCIE_CONF; i++) {
> > > > - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> > > > table_member1[i];
> > > > - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> > > > table_member2[i];
> > > > + sienna_cichlid_get_override_pcie_settings(smu,
> > > > + &gen_speed_override,
> > > > + &lane_width_override);
> > > > +
> > > > + /* 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;
> > > > +
> > > > + /* 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);
> > > > + } 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;
> > > > + }
> > > > + 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) |
> > > > - ((table_member1[i] <= pcie_gen_cap) ?
> > > > - (table_member1[i] << 8) :
> > > > - (pcie_gen_cap << 8)) |
> > > > - ((table_member2[i] <= pcie_width_cap) ?
> > > > - table_member2[i] :
> > > > - pcie_width_cap);
> > > > + smu_pcie_arg = (i << 16 |
> > > > + pcie_table->pcie_gen[i] << 8 |
> > > > + pcie_table->pcie_lane[i]);
> > > >
> > > > ret = smu_cmn_send_smc_msg_with_param(smu,
> > > > SMU_MSG_OverridePcieParameters,
> > > > @@ -2101,11 +2162,6 @@ static int
> > > > sienna_cichlid_update_pcie_parameters(struct smu_context *smu,
> > > > NULL);
> > > > if (ret)
> > > > return ret;
> > > > -
> > > > - if (table_member1[i] > pcie_gen_cap)
> > > > - dpm_context->dpm_tables.pcie_table.pcie_gen[i] =
> > > > pcie_gen_cap;
> > > > - if (table_member2[i] > pcie_width_cap)
> > > > - dpm_context->dpm_tables.pcie_table.pcie_lane[i] =
> > > > pcie_width_cap;
> > > > }
> > > >
> > > > return 0;
> > > > --
> > > > 2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-05-31 12:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-21 7:28 [PATCH] drm/amd/pm: conditionally disable pcie lane switching for some sienna_cichlid SKUs Evan Quan
2023-04-21 7:32 ` Quan, Evan
2023-04-21 9:30 ` Lazar, Lijo
2023-05-31 9:54 ` Quan, Evan
2023-05-31 12:12 ` Alex Deucher
2023-05-31 12:48 ` Alex Deucher
2023-04-21 13:39 ` Limonciello, Mario
2023-04-24 7:33 ` Quan, Evan
2023-04-21 13:44 ` Alex Deucher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.