* [PATCH] drm/amdgpu/pm: Fix the ppfeature value
@ 2024-03-12 10:59 Ma Jun
2024-03-12 12:57 ` Lazar, Lijo
0 siblings, 1 reply; 4+ messages in thread
From: Ma Jun @ 2024-03-12 10:59 UTC (permalink / raw)
To: amd-gfx; +Cc: Kenneth.Feng, Alexander.Deucher, kevinyang.wang, Ma Jun
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@amd.com>
---
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;
}
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);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/amdgpu/pm: Fix the ppfeature value
2024-03-12 10:59 [PATCH] drm/amdgpu/pm: Fix the ppfeature value Ma Jun
@ 2024-03-12 12:57 ` Lazar, Lijo
2024-03-13 2:45 ` Ma, Jun
0 siblings, 1 reply; 4+ messages in thread
From: Lazar, Lijo @ 2024-03-12 12:57 UTC (permalink / raw)
To: Ma Jun, amd-gfx; +Cc: Kenneth.Feng, Alexander.Deucher, kevinyang.wang
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@amd.com>
> ---
> 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.
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);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/amdgpu/pm: Fix the ppfeature value
2024-03-12 12:57 ` Lazar, Lijo
@ 2024-03-13 2:45 ` Ma, Jun
2024-03-13 3:27 ` Lazar, Lijo
0 siblings, 1 reply; 4+ messages in thread
From: Ma, Jun @ 2024-03-13 2:45 UTC (permalink / raw)
To: Lazar, Lijo, Ma Jun, amd-gfx
Cc: majun, Kenneth.Feng, Alexander.Deucher, kevinyang.wang
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@amd.com>
>> ---
>> 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);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/amdgpu/pm: Fix the ppfeature value
2024-03-13 2:45 ` Ma, Jun
@ 2024-03-13 3:27 ` Lazar, Lijo
0 siblings, 0 replies; 4+ messages in thread
From: Lazar, Lijo @ 2024-03-13 3:27 UTC (permalink / raw)
To: Ma, Jun, Ma Jun, amd-gfx; +Cc: Kenneth.Feng, Alexander.Deucher, kevinyang.wang
On 3/13/2024 8:15 AM, Ma, Jun wrote:
>
>
> 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@amd.com>
>>> ---
>>> 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.
>
amdgpu_dpm_* are meant as APIs and not meant to do internal manipulation
of pp_feature with every invocation.
This is indeed not the right way to do this. There is already another
place during driver initialization -
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/pm/amdgpu_pm.c#L4277
This could be done even there.
Thanks,
Lijo
> 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);
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-13 3:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-12 10:59 [PATCH] drm/amdgpu/pm: Fix the ppfeature value Ma Jun
2024-03-12 12:57 ` Lazar, Lijo
2024-03-13 2:45 ` Ma, Jun
2024-03-13 3:27 ` Lazar, Lijo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox