On 3/7/2024 7:42 AM, Ma, Jun wrote:
> Hi Lijo,
>
> On 3/6/2024 7:16 PM, Lazar, Lijo wrote:
>>
>>
>> On 3/6/2024 3:56 PM, Ma Jun wrote:
>>> Because powerplay_table initialization is skipped under
>>> sriov case, We set default lower and upper OD value to
>>> avoid NULL pointer issue.
>>
>> pp_od_clk_voltage is not enabled in SRIOV (except for GC 9.4.3 one VF
>> mode). Since the interface is not available for SRIOV, a better fix may
>> be to set smu->od_enabled itself to false for SRIOV.
>>
> This is not about pp_od_clk_voltage and od_enabled. This problem only occurs
> when getting power1_cap_* value, because we need to read the od_percent_lower
> value from poweplay_table.
od_enabled = false means there is no OD capability and then OD
lower/higher percentage should be zero. For SRIOV, this will be the case.
Instead you are putting some default values which doesn't necessarily
work for all SKUs (that's the reason they are part of powerplay table).
Thanks,
Lijo
>
> Regards,
> Ma Jun
>> Thanks,
>> Lijo
>>
>>>
>>> Fixes: 7968e9748fbb ("drm/amdgpu/pm: Fix the power1_min_cap value")
>>> Signed-off-by: Ma Jun <Jun.Ma2@xxxxxxx>
>>> Reported-by: Yin Zhenguo <zhenguo.yin@xxxxxxx>
>>> ---
>>> .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 15 ++++++++++-----
>>> drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 16 ++++++++++------
>>> .../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 15 ++++++++++-----
>>> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 15 ++++++++++-----
>>> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 15 ++++++++++-----
>>> 5 files changed, 50 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>>> index 1d96eb274d72..42c5e6b103e8 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>>> @@ -1303,12 +1303,17 @@ static int arcturus_get_power_limit(struct smu_context *smu,
>>> if (default_power_limit)
>>> *default_power_limit = power_limit;
>>>
>>> - if (smu->od_enabled)
>>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> - else
>>> - od_percent_upper = 0;
>>> + if (powerplay_table) {
>>> + if (smu->od_enabled)
>>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + else
>>> + od_percent_upper = 0;
>>>
>>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + } else {
>>> + od_percent_upper = 0;
>>> + od_percent_lower = 10;
>>> + }
>>>
>>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>>> od_percent_upper, od_percent_lower, power_limit);
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>>> index ed189a3878eb..6cc2e2d27a0d 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>>> @@ -2356,13 +2356,17 @@ static int navi10_get_power_limit(struct smu_context *smu,
>>> if (default_power_limit)
>>> *default_power_limit = power_limit;
>>>
>>> - if (smu->od_enabled &&
>>> - navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_POWER_LIMIT))
>>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> - else
>>> - od_percent_upper = 0;
>>> + if (powerplay_table) {
>>> + if (smu->od_enabled && navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_POWER_LIMIT))
>>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + else
>>> + od_percent_upper = 0;
>>>
>>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + } else {
>>> + od_percent_upper = 0;
>>> + od_percent_lower = 10;
>>> + }
>>>
>>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>>> od_percent_upper, od_percent_lower, power_limit);
>>> 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 e2ad2b972ab0..5daeac2e6239 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
>>> @@ -640,12 +640,17 @@ static int sienna_cichlid_get_power_limit(struct smu_context *smu,
>>> if (default_power_limit)
>>> *default_power_limit = power_limit;
>>>
>>> - if (smu->od_enabled)
>>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>>> - else
>>> - od_percent_upper = 0;
>>> + if (powerplay_table) {
>>> + if (smu->od_enabled)
>>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + else
>>> + od_percent_upper = 0;
>>>
>>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + } else {
>>> + od_percent_upper = 0;
>>> + od_percent_lower = 10;
>>> + }
>>>
>>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>>> od_percent_upper, od_percent_lower, power_limit);
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>>> index 215f7c91ca73..271151c518e1 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>>> @@ -2369,12 +2369,17 @@ static int smu_v13_0_0_get_power_limit(struct smu_context *smu,
>>> if (default_power_limit)
>>> *default_power_limit = power_limit;
>>>
>>> - if (smu->od_enabled)
>>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>>> - else
>>> - od_percent_upper = 0;
>>> + if (powerplay_table) {
>>> + if (smu->od_enabled)
>>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>>> + else
>>> + od_percent_upper = 0;
>>>
>>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>>> + } else {
>>> + od_percent_upper = 0;
>>> + od_percent_lower = 10;
>>> + }
>>>
>>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>>> od_percent_upper, od_percent_lower, power_limit);
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
>>> index 3dc7b60cb075..533a3c1ba41e 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
>>> @@ -2333,12 +2333,17 @@ static int smu_v13_0_7_get_power_limit(struct smu_context *smu,
>>> if (default_power_limit)
>>> *default_power_limit = power_limit;
>>>
>>> - if (smu->od_enabled)
>>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>>> - else
>>> - od_percent_upper = 0;
>>> + if (powerplay_table) {
>>> + if (smu->od_enabled)
>>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + else
>>> + od_percent_upper = 0;
>>>
>>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + } else {
>>> + od_percent_upper = 0;
>>> + od_percent_lower = 10;
>>> + }
>>>
>>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>>> od_percent_upper, od_percent_lower, power_limit);
