On 3/12/2024 8:57 PM, Lazar, Lijo wrote:
>
>
> On 3/12/2024 4:29 PM, Ma Jun wrote:
>> Sometimes user may want to enable the od feature
>> by setting ppfeaturemask when loading amdgpu driver.
>> However,not all Asics support this feature.
>> So we need to restore the ppfeature value and print
>> a warning info.
>>
>> Signed-off-by: Ma Jun <Jun.Ma2@xxxxxxx>
>> ---
>> drivers/gpu/drm/amd/pm/amdgpu_dpm.c | 15 ++++++++++++---
>> drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h | 2 +-
>> 2 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
>> index f84bfed50681..d777056b2f9d 100644
>> --- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
>> +++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
>> @@ -1548,12 +1548,14 @@ int amdgpu_dpm_get_smu_prv_buf_details(struct amdgpu_device *adev,
>> return ret;
>> }
>>
>> -int amdgpu_dpm_is_overdrive_supported(struct amdgpu_device *adev)
>> +bool amdgpu_dpm_is_overdrive_supported(struct amdgpu_device *adev)
>> {
>> + bool od_support;
>> +
>> if (is_support_sw_smu(adev)) {
>> struct smu_context *smu = adev->powerplay.pp_handle;
>>
>> - return (smu->od_enabled || smu->is_apu);
>> + od_support = (smu->od_enabled || smu->is_apu);
>> } else {
>> struct pp_hwmgr *hwmgr;
>>
>> @@ -1566,8 +1568,15 @@ int amdgpu_dpm_is_overdrive_supported(struct amdgpu_device *adev)
>>
>> hwmgr = (struct pp_hwmgr *)adev->powerplay.pp_handle;
>>
>> - return hwmgr->od_enabled;
>> + od_support = hwmgr->od_enabled;
>> + }
>> +
>> + if (!od_support && (adev->pm.pp_feature & PP_OVERDRIVE_MASK)) {
>> + adev->pm.pp_feature &= ~PP_OVERDRIVE_MASK;
>> + DRM_WARN("overdrive feature is not supported\n");
>> }
>> +
>> + return od_support;
>
> Instead of doing this inside DPM API, suggest to keep it outside towards
> the end of initialization phase.
>
This function is called before the driver creates the OD related sys interface,
which is almost the last initialization phase. I think there's probably not much
need to break the whole logic and call this function again anywhere else.
Regards,
Ma Jun
> Thanks,
> Lijo
>
>> }
>>
>> int amdgpu_dpm_set_pp_table(struct amdgpu_device *adev,
>> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
>> index 621200e0823f..0635f9d3a61a 100644
>> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
>> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h
>> @@ -551,7 +551,7 @@ int amdgpu_dpm_debugfs_print_current_performance_level(struct amdgpu_device *ade
>> int amdgpu_dpm_get_smu_prv_buf_details(struct amdgpu_device *adev,
>> void **addr,
>> size_t *size);
>> -int amdgpu_dpm_is_overdrive_supported(struct amdgpu_device *adev);
>> +bool amdgpu_dpm_is_overdrive_supported(struct amdgpu_device *adev);
>> int amdgpu_dpm_set_pp_table(struct amdgpu_device *adev,
>> const char *buf,
>> size_t size);
