All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini()
@ 2021-12-03  6:54 Lang Yu
  2021-12-03  6:54 ` [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled Lang Yu
  0 siblings, 1 reply; 16+ messages in thread
From: Lang Yu @ 2021-12-03  6:54 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Huang Rui, Lang Yu

Currently, we don't find some neccesities to power on/off
SDMA in SMU hw_init/fini(). It makes more sense in SDMA
hw_init/fini().

Signed-off-by: Lang Yu <lang.yu@amd.com>
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 5839918cb574..2d718c30c8eb 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1350,7 +1350,6 @@ static int smu_hw_init(void *handle)
 	}
 
 	if (smu->is_apu) {
-		smu_powergate_sdma(&adev->smu, false);
 		smu_dpm_set_vcn_enable(smu, true);
 		smu_dpm_set_jpeg_enable(smu, true);
 		smu_set_gfx_cgpg(&adev->smu, true);
@@ -1512,10 +1511,6 @@ static int smu_hw_fini(void *handle)
 	if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev))
 		return 0;
 
-	if (smu->is_apu) {
-		smu_powergate_sdma(&adev->smu, true);
-	}
-
 	smu_dpm_set_vcn_enable(smu, false);
 	smu_dpm_set_jpeg_enable(smu, false);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled
  2021-12-03  6:54 [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini() Lang Yu
@ 2021-12-03  6:54 ` Lang Yu
  2021-12-03  9:52   ` Lazar, Lijo
  0 siblings, 1 reply; 16+ messages in thread
From: Lang Yu @ 2021-12-03  6:54 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Huang Rui, Lang Yu

The general hw fini sequence is SMU-> ... ->SDMA-> ...
We need to send power gate message to power off SDMA(in SDMA hw_fini())
afer dpm is disabled(in SMU hw_fini()). Allow that for APU.

Signed-off-by: Lang Yu <lang.yu@amd.com>
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 2d718c30c8eb..285a237f3605 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
 	struct smu_context *smu = handle;
 	int ret = 0;
 
-	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
+	if (!smu->pm_enabled || (!smu->is_apu && !smu->adev->pm.dpm_enabled)) {
 		dev_WARN(smu->adev->dev,
 			 "SMU uninitialized but power %s requested for %u!\n",
 			 gate ? "gate" : "ungate", block_type);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled
  2021-12-03  6:54 ` [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled Lang Yu
@ 2021-12-03  9:52   ` Lazar, Lijo
  2021-12-06  2:49     ` Yu, Lang
  0 siblings, 1 reply; 16+ messages in thread
From: Lazar, Lijo @ 2021-12-03  9:52 UTC (permalink / raw)
  To: Lang Yu, amd-gfx; +Cc: Alex Deucher, Huang Rui



On 12/3/2021 12:24 PM, Lang Yu wrote:
> The general hw fini sequence is SMU-> ... ->SDMA-> ...
> We need to send power gate message to power off SDMA(in SDMA hw_fini())
> afer dpm is disabled(in SMU hw_fini()). Allow that for APU.

This message is not right. In APUs there is no message provided by FW to 
enable/disable DPM, it is done in BIOS. Rephrase to something like after 
smu hw_fini is completed.

> 
> Signed-off-by: Lang Yu <lang.yu@amd.com>
> ---
>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 2d718c30c8eb..285a237f3605 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
>   	struct smu_context *smu = handle;
>   	int ret = 0;
>   
> -	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
> +	if (!smu->pm_enabled || (!smu->is_apu && !smu->adev->pm.dpm_enabled)) {


This check was there before also, only the WARN is added. That means it 
was skipping sending messages in APUs also and so far this was working 
fine (until this gets noticed because of the warning).

Now this would try to send the message to APU without any check. That 
doesn't look good. Ideal way should be to fix the sequence. Otherwise, 
suggest to do something like below as the last step of smu hw cleanup 
rather than sending the message blindly.

	if (smu->is_apu)
		smu->pm.dpm_enabled = smu_is_dpm_running(smu);

Thanks,
Lijo

>   		dev_WARN(smu->adev->dev,
>   			 "SMU uninitialized but power %s requested for %u!\n",
>   			 gate ? "gate" : "ungate", block_type);
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled
  2021-12-03  9:52   ` Lazar, Lijo
@ 2021-12-06  2:49     ` Yu, Lang
  2021-12-06  3:41       ` Lazar, Lijo
  0 siblings, 1 reply; 16+ messages in thread
From: Yu, Lang @ 2021-12-06  2:49 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: Deucher, Alexander, Huang, Ray

[Public]



>-----Original Message-----
>From: Lazar, Lijo <Lijo.Lazar@amd.com>
>Sent: Friday, December 3, 2021 5:52 PM
>To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
><Ray.Huang@amd.com>
>Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message
>when dpm is disabled
>
>
>
>On 12/3/2021 12:24 PM, Lang Yu wrote:
>> The general hw fini sequence is SMU-> ... ->SDMA-> ...
>> We need to send power gate message to power off SDMA(in SDMA
>> hw_fini()) afer dpm is disabled(in SMU hw_fini()). Allow that for APU.
>
>This message is not right. In APUs there is no message provided by FW to
>enable/disable DPM, it is done in BIOS. Rephrase to something like after smu
>hw_fini is completed.

It is power on/off SDMA message. Not enable/disable DPM.

>>
>> Signed-off-by: Lang Yu <lang.yu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> index 2d718c30c8eb..285a237f3605 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>> @@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
>>   	struct smu_context *smu = handle;
>>   	int ret = 0;
>>
>> -	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
>> +	if (!smu->pm_enabled || (!smu->is_apu &&
>> +!smu->adev->pm.dpm_enabled)) {
>
>
>This check was there before also, only the WARN is added. That means it was
>skipping sending messages in APUs also and so far this was working fine (until this
>gets noticed because of the warning).
>
>Now this would try to send the message to APU without any check. That doesn't
>look good. Ideal way should be to fix the sequence. Otherwise, suggest to do
>something like below as the last step of smu hw cleanup rather than sending the
>message blindly.
>
>	if (smu->is_apu)
>		smu->pm.dpm_enabled = smu_is_dpm_running(smu);

smu_is_dpm_running(smu) will cause errors in suspend.

Here we just  send some IP power on/off messages. 
Is it necessary to enable DPM to send such messages?

Regards,
Lang

>Thanks,
>Lijo
>
>>   		dev_WARN(smu->adev->dev,
>>   			 "SMU uninitialized but power %s requested for %u!\n",
>>   			 gate ? "gate" : "ungate", block_type);
>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled
  2021-12-06  2:49     ` Yu, Lang
@ 2021-12-06  3:41       ` Lazar, Lijo
  2021-12-06  6:47         ` Yu, Lang
  0 siblings, 1 reply; 16+ messages in thread
From: Lazar, Lijo @ 2021-12-06  3:41 UTC (permalink / raw)
  To: Yu, Lang, amd-gfx; +Cc: Deucher, Alexander, Huang, Ray



On 12/6/2021 8:19 AM, Yu, Lang wrote:
> [Public]
> 
> 
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>> Sent: Friday, December 3, 2021 5:52 PM
>> To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>> <Ray.Huang@amd.com>
>> Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message
>> when dpm is disabled
>>
>>
>>
>> On 12/3/2021 12:24 PM, Lang Yu wrote:
>>> The general hw fini sequence is SMU-> ... ->SDMA-> ...
>>> We need to send power gate message to power off SDMA(in SDMA
>>> hw_fini()) afer dpm is disabled(in SMU hw_fini()). Allow that for APU.
>>
>> This message is not right. In APUs there is no message provided by FW to
>> enable/disable DPM, it is done in BIOS. Rephrase to something like after smu
>> hw_fini is completed.
> 
> It is power on/off SDMA message. Not enable/disable DPM.
> 
Bad choice of word :) I didn't mean FW message, it was about this line 
in "commit message" - "afer dpm is disabled".

>>>
>>> Signed-off-by: Lang Yu <lang.yu@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>> index 2d718c30c8eb..285a237f3605 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>> @@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
>>>    	struct smu_context *smu = handle;
>>>    	int ret = 0;
>>>
>>> -	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
>>> +	if (!smu->pm_enabled || (!smu->is_apu &&
>>> +!smu->adev->pm.dpm_enabled)) {
>>
>>
>> This check was there before also, only the WARN is added. That means it was
>> skipping sending messages in APUs also and so far this was working fine (until this
>> gets noticed because of the warning).
>>
>> Now this would try to send the message to APU without any check. That doesn't
>> look good. Ideal way should be to fix the sequence. Otherwise, suggest to do
>> something like below as the last step of smu hw cleanup rather than sending the
>> message blindly.
>>
>> 	if (smu->is_apu)
>> 		smu->pm.dpm_enabled = smu_is_dpm_running(smu);
> 
> smu_is_dpm_running(smu) will cause errors in suspend.
> 
That is interesting. What is the error you get?

Thanks,
Lijo

> Here we just  send some IP power on/off messages.
> Is it necessary to enable DPM to send such messages?
> 
> Regards,
> Lang
> 
>> Thanks,
>> Lijo
>>
>>>    		dev_WARN(smu->adev->dev,
>>>    			 "SMU uninitialized but power %s requested for %u!\n",
>>>    			 gate ? "gate" : "ungate", block_type);
>>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled
  2021-12-06  3:41       ` Lazar, Lijo
@ 2021-12-06  6:47         ` Yu, Lang
  2021-12-06  6:48           ` Yu, Lang
  0 siblings, 1 reply; 16+ messages in thread
From: Yu, Lang @ 2021-12-06  6:47 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: Deucher, Alexander, Huang, Ray

[Public]



>-----Original Message-----
>From: Lazar, Lijo <Lijo.Lazar@amd.com>
>Sent: Monday, December 6, 2021 11:41 AM
>To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
><Ray.Huang@amd.com>
>Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message
>when dpm is disabled
>
>
>
>On 12/6/2021 8:19 AM, Yu, Lang wrote:
>> [Public]
>>
>>
>>
>>> -----Original Message-----
>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>> Sent: Friday, December 3, 2021 5:52 PM
>>> To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>>> <Ray.Huang@amd.com>
>>> Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
>>> message when dpm is disabled
>>>
>>>
>>>
>>> On 12/3/2021 12:24 PM, Lang Yu wrote:
>>>> The general hw fini sequence is SMU-> ... ->SDMA-> ...
>>>> We need to send power gate message to power off SDMA(in SDMA
>>>> hw_fini()) afer dpm is disabled(in SMU hw_fini()). Allow that for APU.
>>>
>>> This message is not right. In APUs there is no message provided by FW
>>> to enable/disable DPM, it is done in BIOS. Rephrase to something like
>>> after smu hw_fini is completed.
>>
>> It is power on/off SDMA message. Not enable/disable DPM.
>>
>Bad choice of word :) I didn't mean FW message, it was about this line in "commit
>message" - "afer dpm is disabled".

Ok. I got it.

>
>>>>
>>>> Signed-off-by: Lang Yu <lang.yu@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> index 2d718c30c8eb..285a237f3605 100644
>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> @@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
>>>>    	struct smu_context *smu = handle;
>>>>    	int ret = 0;
>>>>
>>>> -	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
>>>> +	if (!smu->pm_enabled || (!smu->is_apu &&
>>>> +!smu->adev->pm.dpm_enabled)) {
>>>
>>>
>>> This check was there before also, only the WARN is added. That means
>>> it was skipping sending messages in APUs also and so far this was
>>> working fine (until this gets noticed because of the warning).
>>>
>>> Now this would try to send the message to APU without any check. That
>>> doesn't look good. Ideal way should be to fix the sequence.
>>> Otherwise, suggest to do something like below as the last step of smu
>>> hw cleanup rather than sending the message blindly.
>>>
>>> 	if (smu->is_apu)
>>> 		smu->pm.dpm_enabled = smu_is_dpm_running(smu);
>>
>> smu_is_dpm_running(smu) will cause errors in suspend.
>>
>That is interesting. What is the error you get?

[drm:amdgpu_dpm_enable_uvd [amdgpu]] *ERROR* Dpm enable uvd failed, ret = -95
That means EOPNOTSUPP.

Actually, in resume process, but adev->in_suspend  is still false.
For Renoir series APU, smu_is_dpm_running is hardcoded as following,

static bool renoir_is_dpm_running(struct smu_context *smu)
{
	struct amdgpu_device *adev = smu->adev;

	/*
	 * Until now, the pmfw hasn't exported the interface of SMU
	 * feature mask to APU SKU so just force on all the feature
	 * at early initial stage.
	 */
	if (adev->in_suspend)
		return false;
	else
		return true;

}

So we got such an error.

Regards,
Lang
  
>Thanks,
>Lijo
>
>> Here we just  send some IP power on/off messages.
>> Is it necessary to enable DPM to send such messages?
>>
>> Regards,
>> Lang
>>
>>> Thanks,
>>> Lijo
>>>
>>>>    		dev_WARN(smu->adev->dev,
>>>>    			 "SMU uninitialized but power %s requested for %u!\n",
>>>>    			 gate ? "gate" : "ungate", block_type);
>>>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled
  2021-12-06  6:47         ` Yu, Lang
@ 2021-12-06  6:48           ` Yu, Lang
  2021-12-06  6:59             ` Lazar, Lijo
  0 siblings, 1 reply; 16+ messages in thread
From: Yu, Lang @ 2021-12-06  6:48 UTC (permalink / raw)
  To: Lazar, Lijo, amd-gfx; +Cc: Deucher, Alexander, Huang, Ray

[Public]

A typo.

>-----Original Message-----
>From: Yu, Lang
>Sent: Monday, December 6, 2021 2:47 PM
>To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
><Ray.Huang@amd.com>
>Subject: RE: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message
>when dpm is disabled
>
>[Public]
>
>
>
>>-----Original Message-----
>>From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>Sent: Monday, December 6, 2021 11:41 AM
>>To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
>>Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>><Ray.Huang@amd.com>
>>Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
>>message when dpm is disabled
>>
>>
>>
>>On 12/6/2021 8:19 AM, Yu, Lang wrote:
>>> [Public]
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>>> Sent: Friday, December 3, 2021 5:52 PM
>>>> To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
>>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>>>> <Ray.Huang@amd.com>
>>>> Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
>>>> message when dpm is disabled
>>>>
>>>>
>>>>
>>>> On 12/3/2021 12:24 PM, Lang Yu wrote:
>>>>> The general hw fini sequence is SMU-> ... ->SDMA-> ...
>>>>> We need to send power gate message to power off SDMA(in SDMA
>>>>> hw_fini()) afer dpm is disabled(in SMU hw_fini()). Allow that for APU.
>>>>
>>>> This message is not right. In APUs there is no message provided by
>>>> FW to enable/disable DPM, it is done in BIOS. Rephrase to something
>>>> like after smu hw_fini is completed.
>>>
>>> It is power on/off SDMA message. Not enable/disable DPM.
>>>
>>Bad choice of word :) I didn't mean FW message, it was about this line
>>in "commit message" - "afer dpm is disabled".
>
>Ok. I got it.
>
>>
>>>>>
>>>>> Signed-off-by: Lang Yu <lang.yu@amd.com>
>>>>> ---
>>>>>    drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> index 2d718c30c8eb..285a237f3605 100644
>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>> @@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
>>>>>    	struct smu_context *smu = handle;
>>>>>    	int ret = 0;
>>>>>
>>>>> -	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
>>>>> +	if (!smu->pm_enabled || (!smu->is_apu &&
>>>>> +!smu->adev->pm.dpm_enabled)) {
>>>>
>>>>
>>>> This check was there before also, only the WARN is added. That means
>>>> it was skipping sending messages in APUs also and so far this was
>>>> working fine (until this gets noticed because of the warning).
>>>>
>>>> Now this would try to send the message to APU without any check.
>>>> That doesn't look good. Ideal way should be to fix the sequence.
>>>> Otherwise, suggest to do something like below as the last step of
>>>> smu hw cleanup rather than sending the message blindly.
>>>>
>>>> 	if (smu->is_apu)
>>>> 		smu->pm.dpm_enabled = smu_is_dpm_running(smu);
>>>
>>> smu_is_dpm_running(smu) will cause errors in suspend.
>>>
>>That is interesting. What is the error you get?
>
>[drm:amdgpu_dpm_enable_uvd [amdgpu]] *ERROR* Dpm enable uvd failed, ret =
>-95 That means EOPNOTSUPP.
>
>Actually, in resume process, but adev->in_suspend  is still true.
>For Renoir series APU, smu_is_dpm_running is hardcoded as following,
>
>static bool renoir_is_dpm_running(struct smu_context *smu) {
>	struct amdgpu_device *adev = smu->adev;
>
>	/*
>	 * Until now, the pmfw hasn't exported the interface of SMU
>	 * feature mask to APU SKU so just force on all the feature
>	 * at early initial stage.
>	 */
>	if (adev->in_suspend)
>		return false;
>	else
>		return true;
>
>}
>
>So we got such an error.
>
>Regards,
>Lang
>
>>Thanks,
>>Lijo
>>
>>> Here we just  send some IP power on/off messages.
>>> Is it necessary to enable DPM to send such messages?
>>>
>>> Regards,
>>> Lang
>>>
>>>> Thanks,
>>>> Lijo
>>>>
>>>>>    		dev_WARN(smu->adev->dev,
>>>>>    			 "SMU uninitialized but power %s requested for %u!\n",
>>>>>    			 gate ? "gate" : "ungate", block_type);
>>>>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled
  2021-12-06  6:48           ` Yu, Lang
@ 2021-12-06  6:59             ` Lazar, Lijo
  2021-12-06  8:44               ` Lang Yu
  0 siblings, 1 reply; 16+ messages in thread
From: Lazar, Lijo @ 2021-12-06  6:59 UTC (permalink / raw)
  To: Yu, Lang, amd-gfx; +Cc: Deucher, Alexander, Huang, Ray



On 12/6/2021 12:18 PM, Yu, Lang wrote:
> [Public]
> 
> A typo.
> 
>> -----Original Message-----
>> From: Yu, Lang
>> Sent: Monday, December 6, 2021 2:47 PM
>> To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>> <Ray.Huang@amd.com>
>> Subject: RE: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message
>> when dpm is disabled
>>
>> [Public]
>>
>>
>>
>>> -----Original Message-----
>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>> Sent: Monday, December 6, 2021 11:41 AM
>>> To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>>> <Ray.Huang@amd.com>
>>> Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
>>> message when dpm is disabled
>>>
>>>
>>>
>>> On 12/6/2021 8:19 AM, Yu, Lang wrote:
>>>> [Public]
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>>>> Sent: Friday, December 3, 2021 5:52 PM
>>>>> To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
>>>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>>>>> <Ray.Huang@amd.com>
>>>>> Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
>>>>> message when dpm is disabled
>>>>>
>>>>>
>>>>>
>>>>> On 12/3/2021 12:24 PM, Lang Yu wrote:
>>>>>> The general hw fini sequence is SMU-> ... ->SDMA-> ...
>>>>>> We need to send power gate message to power off SDMA(in SDMA
>>>>>> hw_fini()) afer dpm is disabled(in SMU hw_fini()). Allow that for APU.
>>>>>
>>>>> This message is not right. In APUs there is no message provided by
>>>>> FW to enable/disable DPM, it is done in BIOS. Rephrase to something
>>>>> like after smu hw_fini is completed.
>>>>
>>>> It is power on/off SDMA message. Not enable/disable DPM.
>>>>
>>> Bad choice of word :) I didn't mean FW message, it was about this line
>>> in "commit message" - "afer dpm is disabled".
>>
>> Ok. I got it.
>>
>>>
>>>>>>
>>>>>> Signed-off-by: Lang Yu <lang.yu@amd.com>
>>>>>> ---
>>>>>>     drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>>> index 2d718c30c8eb..285a237f3605 100644
>>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>>> @@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
>>>>>>     	struct smu_context *smu = handle;
>>>>>>     	int ret = 0;
>>>>>>
>>>>>> -	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
>>>>>> +	if (!smu->pm_enabled || (!smu->is_apu &&
>>>>>> +!smu->adev->pm.dpm_enabled)) {
>>>>>
>>>>>
>>>>> This check was there before also, only the WARN is added. That means
>>>>> it was skipping sending messages in APUs also and so far this was
>>>>> working fine (until this gets noticed because of the warning).
>>>>>
>>>>> Now this would try to send the message to APU without any check.
>>>>> That doesn't look good. Ideal way should be to fix the sequence.
>>>>> Otherwise, suggest to do something like below as the last step of
>>>>> smu hw cleanup rather than sending the message blindly.
>>>>>
>>>>> 	if (smu->is_apu)
>>>>> 		smu->pm.dpm_enabled = smu_is_dpm_running(smu);
>>>>
>>>> smu_is_dpm_running(smu) will cause errors in suspend.
>>>>
>>> That is interesting. What is the error you get?
>>
>> [drm:amdgpu_dpm_enable_uvd [amdgpu]] *ERROR* Dpm enable uvd failed, ret =
>> -95 That means EOPNOTSUPP.
>>
>> Actually, in resume process, but adev->in_suspend  is still true.
>> For Renoir series APU, smu_is_dpm_running is hardcoded as following,
>>
>> static bool renoir_is_dpm_running(struct smu_context *smu) {
>> 	struct amdgpu_device *adev = smu->adev;
>>
>> 	/*
>> 	 * Until now, the pmfw hasn't exported the interface of SMU
>> 	 * feature mask to APU SKU so just force on all the feature
>> 	 * at early initial stage.
>> 	 */
>> 	if (adev->in_suspend)
>> 		return false;
>> 	else

Renoir suspend shouldn't be a special case. FW should keep running with 
features enabled after driver suspend. Could you try with a return true 
all the time for this?

Thanks,
Lijo

>> 		return true;
>>
>> }
>>
>> So we got such an error.
>>
>> Regards,
>> Lang
>>
>>> Thanks,
>>> Lijo
>>>
>>>> Here we just  send some IP power on/off messages.
>>>> Is it necessary to enable DPM to send such messages?
>>>>
>>>> Regards,
>>>> Lang
>>>>
>>>>> Thanks,
>>>>> Lijo
>>>>>
>>>>>>     		dev_WARN(smu->adev->dev,
>>>>>>     			 "SMU uninitialized but power %s requested for %u!\n",
>>>>>>     			 gate ? "gate" : "ungate", block_type);
>>>>>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled
  2021-12-06  6:59             ` Lazar, Lijo
@ 2021-12-06  8:44               ` Lang Yu
  2021-12-06  9:12                 ` Lazar, Lijo
  0 siblings, 1 reply; 16+ messages in thread
From: Lang Yu @ 2021-12-06  8:44 UTC (permalink / raw)
  To: Lazar, Lijo; +Cc: Deucher, Alexander, Huang, Ray, amd-gfx

On 12/06/ , Lazar, Lijo wrote:
> 
> 
> On 12/6/2021 12:18 PM, Yu, Lang wrote:
> > [Public]
> > 
> > A typo.
> > 
> > > -----Original Message-----
> > > From: Yu, Lang
> > > Sent: Monday, December 6, 2021 2:47 PM
> > > To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
> > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
> > > <Ray.Huang@amd.com>
> > > Subject: RE: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message
> > > when dpm is disabled
> > > 
> > > [Public]
> > > 
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Lazar, Lijo <Lijo.Lazar@amd.com>
> > > > Sent: Monday, December 6, 2021 11:41 AM
> > > > To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
> > > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
> > > > <Ray.Huang@amd.com>
> > > > Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
> > > > message when dpm is disabled
> > > > 
> > > > 
> > > > 
> > > > On 12/6/2021 8:19 AM, Yu, Lang wrote:
> > > > > [Public]
> > > > > 
> > > > > 
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: Lazar, Lijo <Lijo.Lazar@amd.com>
> > > > > > Sent: Friday, December 3, 2021 5:52 PM
> > > > > > To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
> > > > > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
> > > > > > <Ray.Huang@amd.com>
> > > > > > Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
> > > > > > message when dpm is disabled
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 12/3/2021 12:24 PM, Lang Yu wrote:
> > > > > > > The general hw fini sequence is SMU-> ... ->SDMA-> ...
> > > > > > > We need to send power gate message to power off SDMA(in SDMA
> > > > > > > hw_fini()) afer dpm is disabled(in SMU hw_fini()). Allow that for APU.
> > > > > > 
> > > > > > This message is not right. In APUs there is no message provided by
> > > > > > FW to enable/disable DPM, it is done in BIOS. Rephrase to something
> > > > > > like after smu hw_fini is completed.
> > > > > 
> > > > > It is power on/off SDMA message. Not enable/disable DPM.
> > > > > 
> > > > Bad choice of word :) I didn't mean FW message, it was about this line
> > > > in "commit message" - "afer dpm is disabled".
> > > 
> > > Ok. I got it.
> > > 
> > > > 
> > > > > > > 
> > > > > > > Signed-off-by: Lang Yu <lang.yu@amd.com>
> > > > > > > ---
> > > > > > >     drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
> > > > > > >     1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > > > > > > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > > > > > > index 2d718c30c8eb..285a237f3605 100644
> > > > > > > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > > > > > > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > > > > > > @@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
> > > > > > >     	struct smu_context *smu = handle;
> > > > > > >     	int ret = 0;
> > > > > > > 
> > > > > > > -	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
> > > > > > > +	if (!smu->pm_enabled || (!smu->is_apu &&
> > > > > > > +!smu->adev->pm.dpm_enabled)) {
> > > > > > 
> > > > > > 
> > > > > > This check was there before also, only the WARN is added. That means
> > > > > > it was skipping sending messages in APUs also and so far this was
> > > > > > working fine (until this gets noticed because of the warning).
> > > > > > 
> > > > > > Now this would try to send the message to APU without any check.
> > > > > > That doesn't look good. Ideal way should be to fix the sequence.
> > > > > > Otherwise, suggest to do something like below as the last step of
> > > > > > smu hw cleanup rather than sending the message blindly.
> > > > > > 
> > > > > > 	if (smu->is_apu)
> > > > > > 		smu->pm.dpm_enabled = smu_is_dpm_running(smu);
> > > > > 
> > > > > smu_is_dpm_running(smu) will cause errors in suspend.
> > > > > 
> > > > That is interesting. What is the error you get?
> > > 
> > > [drm:amdgpu_dpm_enable_uvd [amdgpu]] *ERROR* Dpm enable uvd failed, ret =
> > > -95 That means EOPNOTSUPP.
> > > 
> > > Actually, in resume process, but adev->in_suspend  is still true.
> > > For Renoir series APU, smu_is_dpm_running is hardcoded as following,
> > > 
> > > static bool renoir_is_dpm_running(struct smu_context *smu) {
> > > 	struct amdgpu_device *adev = smu->adev;
> > > 
> > > 	/*
> > > 	 * Until now, the pmfw hasn't exported the interface of SMU
> > > 	 * feature mask to APU SKU so just force on all the feature
> > > 	 * at early initial stage.
> > > 	 */
> > > 	if (adev->in_suspend)
> > > 		return false;
> > > 	else
> 
> Renoir suspend shouldn't be a special case. FW should keep running with
> features enabled after driver suspend. Could you try with a return true all
> the time for this?

That worked.

But we just send an IP power on/off message here.

Do we really need dpm running?

Regards,
Lang

> Thanks,
> Lijo
> 
> > > 		return true;
> > > 
> > > }
> > > 
> > > So we got such an error.
> > > 
> > > Regards,
> > > Lang
> > > 
> > > > Thanks,
> > > > Lijo
> > > > 
> > > > > Here we just  send some IP power on/off messages.
> > > > > Is it necessary to enable DPM to send such messages?
> > > > > 
> > > > > Regards,
> > > > > Lang
> > > > > 
> > > > > > Thanks,
> > > > > > Lijo
> > > > > > 
> > > > > > >     		dev_WARN(smu->adev->dev,
> > > > > > >     			 "SMU uninitialized but power %s requested for %u!\n",
> > > > > > >     			 gate ? "gate" : "ungate", block_type);
> > > > > > > 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled
  2021-12-06  8:44               ` Lang Yu
@ 2021-12-06  9:12                 ` Lazar, Lijo
  2021-12-06  9:31                   ` Lang Yu
  0 siblings, 1 reply; 16+ messages in thread
From: Lazar, Lijo @ 2021-12-06  9:12 UTC (permalink / raw)
  To: Lang Yu; +Cc: Deucher, Alexander, Huang, Ray, amd-gfx



On 12/6/2021 2:14 PM, Lang Yu wrote:
> On 12/06/ , Lazar, Lijo wrote:
>>
>>
>> On 12/6/2021 12:18 PM, Yu, Lang wrote:
>>> [Public]
>>>
>>> A typo.
>>>
>>>> -----Original Message-----
>>>> From: Yu, Lang
>>>> Sent: Monday, December 6, 2021 2:47 PM
>>>> To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
>>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>>>> <Ray.Huang@amd.com>
>>>> Subject: RE: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message
>>>> when dpm is disabled
>>>>
>>>> [Public]
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>>>> Sent: Monday, December 6, 2021 11:41 AM
>>>>> To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
>>>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>>>>> <Ray.Huang@amd.com>
>>>>> Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
>>>>> message when dpm is disabled
>>>>>
>>>>>
>>>>>
>>>>> On 12/6/2021 8:19 AM, Yu, Lang wrote:
>>>>>> [Public]
>>>>>>
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>>>>>> Sent: Friday, December 3, 2021 5:52 PM
>>>>>>> To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
>>>>>>> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
>>>>>>> <Ray.Huang@amd.com>
>>>>>>> Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
>>>>>>> message when dpm is disabled
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 12/3/2021 12:24 PM, Lang Yu wrote:
>>>>>>>> The general hw fini sequence is SMU-> ... ->SDMA-> ...
>>>>>>>> We need to send power gate message to power off SDMA(in SDMA
>>>>>>>> hw_fini()) afer dpm is disabled(in SMU hw_fini()). Allow that for APU.
>>>>>>>
>>>>>>> This message is not right. In APUs there is no message provided by
>>>>>>> FW to enable/disable DPM, it is done in BIOS. Rephrase to something
>>>>>>> like after smu hw_fini is completed.
>>>>>>
>>>>>> It is power on/off SDMA message. Not enable/disable DPM.
>>>>>>
>>>>> Bad choice of word :) I didn't mean FW message, it was about this line
>>>>> in "commit message" - "afer dpm is disabled".
>>>>
>>>> Ok. I got it.
>>>>
>>>>>
>>>>>>>>
>>>>>>>> Signed-off-by: Lang Yu <lang.yu@amd.com>
>>>>>>>> ---
>>>>>>>>      drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
>>>>>>>>      1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>>>>> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>>>>> index 2d718c30c8eb..285a237f3605 100644
>>>>>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>>>>>> @@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
>>>>>>>>      	struct smu_context *smu = handle;
>>>>>>>>      	int ret = 0;
>>>>>>>>
>>>>>>>> -	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
>>>>>>>> +	if (!smu->pm_enabled || (!smu->is_apu &&
>>>>>>>> +!smu->adev->pm.dpm_enabled)) {
>>>>>>>
>>>>>>>
>>>>>>> This check was there before also, only the WARN is added. That means
>>>>>>> it was skipping sending messages in APUs also and so far this was
>>>>>>> working fine (until this gets noticed because of the warning).
>>>>>>>
>>>>>>> Now this would try to send the message to APU without any check.
>>>>>>> That doesn't look good. Ideal way should be to fix the sequence.
>>>>>>> Otherwise, suggest to do something like below as the last step of
>>>>>>> smu hw cleanup rather than sending the message blindly.
>>>>>>>
>>>>>>> 	if (smu->is_apu)
>>>>>>> 		smu->pm.dpm_enabled = smu_is_dpm_running(smu);
>>>>>>
>>>>>> smu_is_dpm_running(smu) will cause errors in suspend.
>>>>>>
>>>>> That is interesting. What is the error you get?
>>>>
>>>> [drm:amdgpu_dpm_enable_uvd [amdgpu]] *ERROR* Dpm enable uvd failed, ret =
>>>> -95 That means EOPNOTSUPP.
>>>>
>>>> Actually, in resume process, but adev->in_suspend  is still true.
>>>> For Renoir series APU, smu_is_dpm_running is hardcoded as following,
>>>>
>>>> static bool renoir_is_dpm_running(struct smu_context *smu) {
>>>> 	struct amdgpu_device *adev = smu->adev;
>>>>
>>>> 	/*
>>>> 	 * Until now, the pmfw hasn't exported the interface of SMU
>>>> 	 * feature mask to APU SKU so just force on all the feature
>>>> 	 * at early initial stage.
>>>> 	 */
>>>> 	if (adev->in_suspend)
>>>> 		return false;
>>>> 	else
>>
>> Renoir suspend shouldn't be a special case. FW should keep running with
>> features enabled after driver suspend. Could you try with a return true all
>> the time for this?
> 
> That worked.
> 
> But we just send an IP power on/off message here.
> 
> Do we really need dpm running?
> 

Yes, but it is a power management message. From a FW perspective, power 
management starts when DPM is enabled. Without that, it's not bothered 
about any power management features. Only a few non-PM related messages 
like i2c table transfer etc. work when it is not enabled. Usually for 
APUs, DPM gets enabled early through BIOS and driver doesn't have much 
control.

If dpm_enabled is not causing any SW logical errors, better to keep it 
coherent with FW DPM for swsmu based ASICs.

Thanks,
Lijo

> Regards,
> Lang
> 
>> Thanks,
>> Lijo
>>
>>>> 		return true;
>>>>
>>>> }
>>>>
>>>> So we got such an error.
>>>>
>>>> Regards,
>>>> Lang
>>>>
>>>>> Thanks,
>>>>> Lijo
>>>>>
>>>>>> Here we just  send some IP power on/off messages.
>>>>>> Is it necessary to enable DPM to send such messages?
>>>>>>
>>>>>> Regards,
>>>>>> Lang
>>>>>>
>>>>>>> Thanks,
>>>>>>> Lijo
>>>>>>>
>>>>>>>>      		dev_WARN(smu->adev->dev,
>>>>>>>>      			 "SMU uninitialized but power %s requested for %u!\n",
>>>>>>>>      			 gate ? "gate" : "ungate", block_type);
>>>>>>>>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled
  2021-12-06  9:12                 ` Lazar, Lijo
@ 2021-12-06  9:31                   ` Lang Yu
  0 siblings, 0 replies; 16+ messages in thread
From: Lang Yu @ 2021-12-06  9:31 UTC (permalink / raw)
  To: Lazar, Lijo; +Cc: Deucher, Alexander, Huang, Ray, amd-gfx

On 12/06/ , Lazar, Lijo wrote:
> 
> 
> On 12/6/2021 2:14 PM, Lang Yu wrote:
> > On 12/06/ , Lazar, Lijo wrote:
> > > 
> > > 
> > > On 12/6/2021 12:18 PM, Yu, Lang wrote:
> > > > [Public]
> > > > 
> > > > A typo.
> > > > 
> > > > > -----Original Message-----
> > > > > From: Yu, Lang
> > > > > Sent: Monday, December 6, 2021 2:47 PM
> > > > > To: Lazar, Lijo <Lijo.Lazar@amd.com>; amd-gfx@lists.freedesktop.org
> > > > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
> > > > > <Ray.Huang@amd.com>
> > > > > Subject: RE: [PATCH 2/2] drm/amdgpu: allow APU to send power gate message
> > > > > when dpm is disabled
> > > > > 
> > > > > [Public]
> > > > > 
> > > > > 
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: Lazar, Lijo <Lijo.Lazar@amd.com>
> > > > > > Sent: Monday, December 6, 2021 11:41 AM
> > > > > > To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
> > > > > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
> > > > > > <Ray.Huang@amd.com>
> > > > > > Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
> > > > > > message when dpm is disabled
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 12/6/2021 8:19 AM, Yu, Lang wrote:
> > > > > > > [Public]
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > > -----Original Message-----
> > > > > > > > From: Lazar, Lijo <Lijo.Lazar@amd.com>
> > > > > > > > Sent: Friday, December 3, 2021 5:52 PM
> > > > > > > > To: Yu, Lang <Lang.Yu@amd.com>; amd-gfx@lists.freedesktop.org
> > > > > > > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Huang, Ray
> > > > > > > > <Ray.Huang@amd.com>
> > > > > > > > Subject: Re: [PATCH 2/2] drm/amdgpu: allow APU to send power gate
> > > > > > > > message when dpm is disabled
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On 12/3/2021 12:24 PM, Lang Yu wrote:
> > > > > > > > > The general hw fini sequence is SMU-> ... ->SDMA-> ...
> > > > > > > > > We need to send power gate message to power off SDMA(in SDMA
> > > > > > > > > hw_fini()) afer dpm is disabled(in SMU hw_fini()). Allow that for APU.
> > > > > > > > 
> > > > > > > > This message is not right. In APUs there is no message provided by
> > > > > > > > FW to enable/disable DPM, it is done in BIOS. Rephrase to something
> > > > > > > > like after smu hw_fini is completed.
> > > > > > > 
> > > > > > > It is power on/off SDMA message. Not enable/disable DPM.
> > > > > > > 
> > > > > > Bad choice of word :) I didn't mean FW message, it was about this line
> > > > > > in "commit message" - "afer dpm is disabled".
> > > > > 
> > > > > Ok. I got it.
> > > > > 
> > > > > > 
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Lang Yu <lang.yu@amd.com>
> > > > > > > > > ---
> > > > > > > > >      drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 2 +-
> > > > > > > > >      1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > > > > > > > > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > > > > > > > > index 2d718c30c8eb..285a237f3605 100644
> > > > > > > > > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > > > > > > > > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > > > > > > > > @@ -277,7 +277,7 @@ static int smu_dpm_set_power_gate(void *handle,
> > > > > > > > >      	struct smu_context *smu = handle;
> > > > > > > > >      	int ret = 0;
> > > > > > > > > 
> > > > > > > > > -	if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled) {
> > > > > > > > > +	if (!smu->pm_enabled || (!smu->is_apu &&
> > > > > > > > > +!smu->adev->pm.dpm_enabled)) {
> > > > > > > > 
> > > > > > > > 
> > > > > > > > This check was there before also, only the WARN is added. That means
> > > > > > > > it was skipping sending messages in APUs also and so far this was
> > > > > > > > working fine (until this gets noticed because of the warning).
> > > > > > > > 
> > > > > > > > Now this would try to send the message to APU without any check.
> > > > > > > > That doesn't look good. Ideal way should be to fix the sequence.
> > > > > > > > Otherwise, suggest to do something like below as the last step of
> > > > > > > > smu hw cleanup rather than sending the message blindly.
> > > > > > > > 
> > > > > > > > 	if (smu->is_apu)
> > > > > > > > 		smu->pm.dpm_enabled = smu_is_dpm_running(smu);
> > > > > > > 
> > > > > > > smu_is_dpm_running(smu) will cause errors in suspend.
> > > > > > > 
> > > > > > That is interesting. What is the error you get?
> > > > > 
> > > > > [drm:amdgpu_dpm_enable_uvd [amdgpu]] *ERROR* Dpm enable uvd failed, ret =
> > > > > -95 That means EOPNOTSUPP.
> > > > > 
> > > > > Actually, in resume process, but adev->in_suspend  is still true.
> > > > > For Renoir series APU, smu_is_dpm_running is hardcoded as following,
> > > > > 
> > > > > static bool renoir_is_dpm_running(struct smu_context *smu) {
> > > > > 	struct amdgpu_device *adev = smu->adev;
> > > > > 
> > > > > 	/*
> > > > > 	 * Until now, the pmfw hasn't exported the interface of SMU
> > > > > 	 * feature mask to APU SKU so just force on all the feature
> > > > > 	 * at early initial stage.
> > > > > 	 */
> > > > > 	if (adev->in_suspend)
> > > > > 		return false;
> > > > > 	else
> > > 
> > > Renoir suspend shouldn't be a special case. FW should keep running with
> > > features enabled after driver suspend. Could you try with a return true all
> > > the time for this?
> > 
> > That worked.
> > 
> > But we just send an IP power on/off message here.
> > 
> > Do we really need dpm running?
> > 
> 
> Yes, but it is a power management message. From a FW perspective, power
> management starts when DPM is enabled. Without that, it's not bothered about
> any power management features. Only a few non-PM related messages like i2c
> table transfer etc. work when it is not enabled. Usually for APUs, DPM gets
> enabled early through BIOS and driver doesn't have much control.
> 
> If dpm_enabled is not causing any SW logical errors, better to keep it
> coherent with FW DPM for swsmu based ASICs.

As you said for APUs, DPM gets enabled early through BIOS.
Are there any cases it is disabled by diver or BIOS or itself after that?

1, If not, why we query if it is running for APUs.
2, If yes, who enable it again for APus?

Regards,
Lang

> Thanks,
> Lijo
> 
> > Regards,
> > Lang
> > 
> > > Thanks,
> > > Lijo
> > > 
> > > > > 		return true;
> > > > > 
> > > > > }
> > > > > 
> > > > > So we got such an error.
> > > > > 
> > > > > Regards,
> > > > > Lang
> > > > > 
> > > > > > Thanks,
> > > > > > Lijo
> > > > > > 
> > > > > > > Here we just  send some IP power on/off messages.
> > > > > > > Is it necessary to enable DPM to send such messages?
> > > > > > > 
> > > > > > > Regards,
> > > > > > > Lang
> > > > > > > 
> > > > > > > > Thanks,
> > > > > > > > Lijo
> > > > > > > > 
> > > > > > > > >      		dev_WARN(smu->adev->dev,
> > > > > > > > >      			 "SMU uninitialized but power %s requested for %u!\n",
> > > > > > > > >      			 gate ? "gate" : "ungate", block_type);
> > > > > > > > > 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini()
  2021-12-08 11:11     ` Wang, Yang(Kevin)
@ 2021-12-21  8:41       ` Josef Johansson
  0 siblings, 0 replies; 16+ messages in thread
From: Josef Johansson @ 2021-12-21  8:41 UTC (permalink / raw)
  To: Wang, Yang(Kevin), Yu, Lang
  Cc: Deucher, Alexander, Lazar, Lijo, Huang, Ray, amd-gfx


On 12/8/21 12:11, Wang, Yang(Kevin) wrote:
>
> [AMD Official Use Only]
>
>
> OK, I miss this call path.
>
> Reviewed-by: Kevin Wang <kevinyang.wang@amd.com>
>
> Best Regards,
> Kevin
>
> ------------------------------------------------------------------------
> *From:* Yu, Lang <Lang.Yu@amd.com>
> *Sent:* Wednesday, December 8, 2021 6:58 PM
> *To:* Wang, Yang(Kevin) <KevinYang.Wang@amd.com>
> *Cc:* amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>;
> Deucher, Alexander <Alexander.Deucher@amd.com>; Lazar, Lijo
> <Lijo.Lazar@amd.com>; Huang, Ray <Ray.Huang@amd.com>
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU
> hw_init/fini()
>  
> On 12/08/ , Wang, Yang(Kevin) wrote:
> >    [AMD Official Use Only]
> >
> >    Hi Lang,
> >    the function of smu_powergate_sdma() is only valid on renoir chip.
> >    if you want to remove it, please also remove following callback
> pointer
> >    in struct pptable_funcs{}.
> >    related macro definitions also need to be cleaned up.
> >    int (*powergate_sdma)(struct smu_context *smu, bool gate);
>
>      It will be still called by amdgpu_dpm_set_powergating_by_smu()
>      in sdma_v4_0_hw_init/fini().
>        
>      Regards,
>      Lang
>
> >    Best Regards,
> >    Kevin
> >      __________________________________________________________________
> >
> >    From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf
> of Lang
> >    Yu <lang.yu@amd.com>
> >    Sent: Wednesday, December 8, 2021 5:26 PM
> >    To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
> >    Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Yu, Lang
> >    <Lang.Yu@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Huang, Ray
> >    <Ray.Huang@amd.com>
> >    Subject: [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU
> >    hw_init/fini()
> >
> >    Currently, we don't find some neccesities to power on/off
> >    SDMA in SMU hw_init/fini(). It makes more sense in SDMA
> >    hw_init/fini().
> >    Signed-off-by: Lang Yu <lang.yu@amd.com>
> >    ---
> >     drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 5 -----
> >     1 file changed, 5 deletions(-)
> >    diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >    b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >    index 5839918cb574..2d718c30c8eb 100644
> >    --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >    +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> >    @@ -1350,7 +1350,6 @@ static int smu_hw_init(void *handle)
> >             }
> >
> >             if (smu->is_apu) {
> >    -               smu_powergate_sdma(&adev->smu, false);
> >                     smu_dpm_set_vcn_enable(smu, true);
> >                     smu_dpm_set_jpeg_enable(smu, true);
> >                     smu_set_gfx_cgpg(&adev->smu, true);
> >    @@ -1512,10 +1511,6 @@ static int smu_hw_fini(void *handle)
> >             if (amdgpu_sriov_vf(adev)&&
> !amdgpu_sriov_is_pp_one_vf(adev))
> >                     return 0;
> >
> >    -       if (smu->is_apu) {
> >    -               smu_powergate_sdma(&adev->smu, true);
> >    -       }
> >    -
> >             smu_dpm_set_vcn_enable(smu, false);
> >             smu_dpm_set_jpeg_enable(smu, false);
> >
> >    --
> >    2.25.1
Hi,

Could this patch be related to the stacktrace I get when setting
amdgpu.dpm=0 on Renoir?
I tried to clear it up as much as possible (translated via OTR).

amdgpu 0000:07:00.0: amdgpu: amdgpu_device_ip_init failed
amdgpu 0000:07:00.0: amdgpu: Fatal error during GPU init
hid-multitouch 2003:04F3:ZACC.0001: Input,hiddeu 96, hidrawo: USB HID v1.10 Device CELAN Touchscreen on 00.3-4/inputor
amdgpu 0000:07:00.0: amdgpu: amdgpu: finishing device.
[drm] free PSP TMR buffer
amdgpu: probe of 0000:07:00.0 failed with error -95
BUG: unable to handle page fault for address: ffffc90080ccc000
#PF: supervisor write access in kernel modes
#PF: error_code(0x0002) - not-present page
PGD cd7cc067 P4D cd7cc067 PUD 10d93b067 PMD 10db30067 PTE O
Oops: 0002 (#1) SMP NOPTI
CPU: 4 PID: 427 Conn: systend-udevd Tainted: G V
5.16.0-0.rcs.8.fc32.qubes.x86_64 eur
Hardware name: LENOVO 20Y1S02400/2041502400, BIOS RIBET66U(1.35 ) 07/30/2021
RIP: e030:vcn_v2_0_sw_fini+0x72/0x90 [amdgpu]
Code: 89 ef eB 41 17 FE FE 85 c0 75 08 48 69 ef e8 65 10 FE ER 48 Bb 54 24 08 65 48 2b 14 25 28 00 00 00 75 18 48 83 4 10 51 5d c3 c?> 03 00 00 00 00 66 70 24 04 e8 af 07 do ff eb be es
RSP: e02b:feffc9004070bc18 EFLAGS: 00010202
RAX: 0000000000000001 RBX: ffffc900B0ccc000 RCX: 0000000000000000
RDX: 0000607ebf40a 100 RSI: feffc9004070bcic RDI: PEEEEEEfco2760401
RBP: ffff88810fe20000 ROB: 0000000000000000 RO9: 0000000080200018
R10: 0000000040000000 R11: 0000000000000004 R12: FF188810fe35a2er
R13: fer88810fe36980 R14: 000000000000000b R15: ffff88810111137cf
FS: 00007033B0a08b80C0000) GS: PEF 88814070000000000) knlGS:000000000000
CS: e030 DS: 0000 ES: 0000 CRO: 0000000080050033
CR2: ffffc90088ccc000 CR3: 0000000100b9c000 CR4: 0000000000050660
Call Trace:
<TASK>
amdgpu_device_ip_fini.isra.O+Oxb6/0x1bo [amdgpu]
amdgpu_device_fini_sw+Ox16/Oxe0 [amdgpu]
amdgpu_driver release_kms.0x12/0x30 [amdgpu]
devm_drm_dev_init_release+Ox3d/0x60 tarnir
devres_release_all.0xb8/0x100
really probe.x100/BX3108
_driver_probe_device.exfe/0x180P
driver probe_device-Oxle/8x90
_driver_attach•OxcB/Oxice
? _device_attach_driver Oxee/exeos
? _device_attach_driver.exe/Oxeos
bus_for_each_dev@x89/exdes
Bus_add_driver Ox149/Oxices
driver register.Ox8f/xee
? Oxffffffffcobddooos
do_one_initcall.0257/8x200
do_init_module=0x5c/8x260
_do_sys_finit_module.Oxae Exi1er
do_syscall_64.0x3b/Bx90
entry_SYSCALL_64_after_huframe.8x44 Axael
RIP: 0033:0x70308ee351345
Code: 00 c3 66 Ze of 1f 84 00 00 00 00 90 83 or le fa 48 89 68 48 89 67 48 89 46 48 89 ca 40 89 c2 40 69 cb 4c 8b 4c 24 08 of 05 <48> 30  01 to ff ff 73 01 c3 48 8b od 2b 6d Oc '00 f? d8 64 89 01 4
RSP: 002b:00007fff2a5c6198 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
RAX: ffffffffffffffda RBX: 00005a0b9881a640 RCX: 0000703d8ee35131
RDX: 0000000000000000 RSI: 00005a0b98819100 RDI: 00000000000000148
RBP: 0000000000020000 ROB: 0000000000000000 RO9; 00005ab9875700N
R10: 0000000000000014 R11: 0000000000000246 R12: 0000000000000000
R13: 00005a0b98819000 R14: 00005ab9881b9a0 R15: 0000000000000006
<TASKS
Modules linked in: hid_multitouch andgpul) commu_y2 gpu_sched 
12c_algo_bit dem_ttn_helper ttn sdhcl_pcl crct1001f_pclmul crc32_pc Imul
 drm_kms_helper crc32c_intel cqhci xhci_pci cec sdhci inme 
xhci_ci_remesa
serio_raw ghash_clmulni_intel xhci_hcd muc_core ehci 
Jhcd nume_core xen_acpl_processor xen_pr lucnd xen_pelback xen_blkback 
xen_gntalloc xen_gntdeu xen_eutchn uinputs
CR2: ffffc900B0cce000
--- end trace 6bbd70000e18e7a9 )---


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini()
  2021-12-08 10:58   ` Lang Yu
@ 2021-12-08 11:11     ` Wang, Yang(Kevin)
  2021-12-21  8:41       ` Josef Johansson
  0 siblings, 1 reply; 16+ messages in thread
From: Wang, Yang(Kevin) @ 2021-12-08 11:11 UTC (permalink / raw)
  To: Yu, Lang; +Cc: Deucher, Alexander, Lazar, Lijo, Huang, Ray, amd-gfx

[-- Attachment #1: Type: text/plain, Size: 3031 bytes --]

[AMD Official Use Only]

OK, I miss this call path.

Reviewed-by: Kevin Wang <kevinyang.wang@amd.com>

Best Regards,
Kevin

________________________________
From: Yu, Lang <Lang.Yu@amd.com>
Sent: Wednesday, December 8, 2021 6:58 PM
To: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>
Cc: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Deucher, Alexander <Alexander.Deucher@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Huang, Ray <Ray.Huang@amd.com>
Subject: Re: [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini()

On 12/08/ , Wang, Yang(Kevin) wrote:
>    [AMD Official Use Only]
>
>    Hi Lang,
>    the function of smu_powergate_sdma() is only valid on renoir chip.
>    if you want to remove it, please also remove following callback pointer
>    in struct pptable_funcs{}.
>    related macro definitions also need to be cleaned up.
>    int (*powergate_sdma)(struct smu_context *smu, bool gate);

     It will be still called by amdgpu_dpm_set_powergating_by_smu()
     in sdma_v4_0_hw_init/fini().

     Regards,
     Lang

>    Best Regards,
>    Kevin
>      __________________________________________________________________
>
>    From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Lang
>    Yu <lang.yu@amd.com>
>    Sent: Wednesday, December 8, 2021 5:26 PM
>    To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
>    Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Yu, Lang
>    <Lang.Yu@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Huang, Ray
>    <Ray.Huang@amd.com>
>    Subject: [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU
>    hw_init/fini()
>
>    Currently, we don't find some neccesities to power on/off
>    SDMA in SMU hw_init/fini(). It makes more sense in SDMA
>    hw_init/fini().
>    Signed-off-by: Lang Yu <lang.yu@amd.com>
>    ---
>     drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 5 -----
>     1 file changed, 5 deletions(-)
>    diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>    b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>    index 5839918cb574..2d718c30c8eb 100644
>    --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>    +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>    @@ -1350,7 +1350,6 @@ static int smu_hw_init(void *handle)
>             }
>
>             if (smu->is_apu) {
>    -               smu_powergate_sdma(&adev->smu, false);
>                     smu_dpm_set_vcn_enable(smu, true);
>                     smu_dpm_set_jpeg_enable(smu, true);
>                     smu_set_gfx_cgpg(&adev->smu, true);
>    @@ -1512,10 +1511,6 @@ static int smu_hw_fini(void *handle)
>             if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev))
>                     return 0;
>
>    -       if (smu->is_apu) {
>    -               smu_powergate_sdma(&adev->smu, true);
>    -       }
>    -
>             smu_dpm_set_vcn_enable(smu, false);
>             smu_dpm_set_jpeg_enable(smu, false);
>
>    --
>    2.25.1

[-- Attachment #2: Type: text/html, Size: 6718 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini()
  2021-12-08 10:06 ` Wang, Yang(Kevin)
@ 2021-12-08 10:58   ` Lang Yu
  2021-12-08 11:11     ` Wang, Yang(Kevin)
  0 siblings, 1 reply; 16+ messages in thread
From: Lang Yu @ 2021-12-08 10:58 UTC (permalink / raw)
  To: Wang, Yang(Kevin); +Cc: Deucher, Alexander, Lazar, Lijo, Huang, Ray, amd-gfx

On 12/08/ , Wang, Yang(Kevin) wrote:
>    [AMD Official Use Only]
> 
>    Hi Lang,
>    the function of smu_powergate_sdma() is only valid on renoir chip.
>    if you want to remove it, please also remove following callback pointer
>    in struct pptable_funcs{}.
>    related macro definitions also need to be cleaned up.
>    int (*powergate_sdma)(struct smu_context *smu, bool gate);

     It will be still called by amdgpu_dpm_set_powergating_by_smu()
     in sdma_v4_0_hw_init/fini().
	
     Regards,
     Lang

>    Best Regards,
>    Kevin
>      __________________________________________________________________
> 
>    From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Lang
>    Yu <lang.yu@amd.com>
>    Sent: Wednesday, December 8, 2021 5:26 PM
>    To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
>    Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Yu, Lang
>    <Lang.Yu@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Huang, Ray
>    <Ray.Huang@amd.com>
>    Subject: [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU
>    hw_init/fini()
> 
>    Currently, we don't find some neccesities to power on/off
>    SDMA in SMU hw_init/fini(). It makes more sense in SDMA
>    hw_init/fini().
>    Signed-off-by: Lang Yu <lang.yu@amd.com>
>    ---
>     drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 5 -----
>     1 file changed, 5 deletions(-)
>    diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>    b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>    index 5839918cb574..2d718c30c8eb 100644
>    --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>    +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>    @@ -1350,7 +1350,6 @@ static int smu_hw_init(void *handle)
>             }
> 
>             if (smu->is_apu) {
>    -               smu_powergate_sdma(&adev->smu, false);
>                     smu_dpm_set_vcn_enable(smu, true);
>                     smu_dpm_set_jpeg_enable(smu, true);
>                     smu_set_gfx_cgpg(&adev->smu, true);
>    @@ -1512,10 +1511,6 @@ static int smu_hw_fini(void *handle)
>             if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev))
>                     return 0;
> 
>    -       if (smu->is_apu) {
>    -               smu_powergate_sdma(&adev->smu, true);
>    -       }
>    -
>             smu_dpm_set_vcn_enable(smu, false);
>             smu_dpm_set_jpeg_enable(smu, false);
> 
>    --
>    2.25.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini()
  2021-12-08  9:26 [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini() Lang Yu
@ 2021-12-08 10:06 ` Wang, Yang(Kevin)
  2021-12-08 10:58   ` Lang Yu
  0 siblings, 1 reply; 16+ messages in thread
From: Wang, Yang(Kevin) @ 2021-12-08 10:06 UTC (permalink / raw)
  To: Yu, Lang, amd-gfx; +Cc: Deucher, Alexander, Lazar, Lijo, Huang, Ray

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

[AMD Official Use Only]

Hi Lang,

the function of smu_powergate_sdma() is only valid on renoir chip.
if you want to remove it, please also remove following callback pointer in struct pptable_funcs{}.
related macro definitions also need to be cleaned up.

int (*powergate_sdma)(struct smu_context *smu, bool gate);

Best Regards,
Kevin
________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Lang Yu <lang.yu@amd.com>
Sent: Wednesday, December 8, 2021 5:26 PM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Yu, Lang <Lang.Yu@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Huang, Ray <Ray.Huang@amd.com>
Subject: [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini()

Currently, we don't find some neccesities to power on/off
SDMA in SMU hw_init/fini(). It makes more sense in SDMA
hw_init/fini().

Signed-off-by: Lang Yu <lang.yu@amd.com>
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 5839918cb574..2d718c30c8eb 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1350,7 +1350,6 @@ static int smu_hw_init(void *handle)
         }

         if (smu->is_apu) {
-               smu_powergate_sdma(&adev->smu, false);
                 smu_dpm_set_vcn_enable(smu, true);
                 smu_dpm_set_jpeg_enable(smu, true);
                 smu_set_gfx_cgpg(&adev->smu, true);
@@ -1512,10 +1511,6 @@ static int smu_hw_fini(void *handle)
         if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev))
                 return 0;

-       if (smu->is_apu) {
-               smu_powergate_sdma(&adev->smu, true);
-       }
-
         smu_dpm_set_vcn_enable(smu, false);
         smu_dpm_set_jpeg_enable(smu, false);

--
2.25.1


[-- Attachment #2: Type: text/html, Size: 7419 bytes --]

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini()
@ 2021-12-08  9:26 Lang Yu
  2021-12-08 10:06 ` Wang, Yang(Kevin)
  0 siblings, 1 reply; 16+ messages in thread
From: Lang Yu @ 2021-12-08  9:26 UTC (permalink / raw)
  To: amd-gfx; +Cc: Alex Deucher, Lang Yu, Lijo Lazar, Huang Rui

Currently, we don't find some neccesities to power on/off
SDMA in SMU hw_init/fini(). It makes more sense in SDMA
hw_init/fini().

Signed-off-by: Lang Yu <lang.yu@amd.com>
---
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index 5839918cb574..2d718c30c8eb 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -1350,7 +1350,6 @@ static int smu_hw_init(void *handle)
 	}
 
 	if (smu->is_apu) {
-		smu_powergate_sdma(&adev->smu, false);
 		smu_dpm_set_vcn_enable(smu, true);
 		smu_dpm_set_jpeg_enable(smu, true);
 		smu_set_gfx_cgpg(&adev->smu, true);
@@ -1512,10 +1511,6 @@ static int smu_hw_fini(void *handle)
 	if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev))
 		return 0;
 
-	if (smu->is_apu) {
-		smu_powergate_sdma(&adev->smu, true);
-	}
-
 	smu_dpm_set_vcn_enable(smu, false);
 	smu_dpm_set_jpeg_enable(smu, false);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-12-21  9:13 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-03  6:54 [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini() Lang Yu
2021-12-03  6:54 ` [PATCH 2/2] drm/amdgpu: allow APU to send power gate message when dpm is disabled Lang Yu
2021-12-03  9:52   ` Lazar, Lijo
2021-12-06  2:49     ` Yu, Lang
2021-12-06  3:41       ` Lazar, Lijo
2021-12-06  6:47         ` Yu, Lang
2021-12-06  6:48           ` Yu, Lang
2021-12-06  6:59             ` Lazar, Lijo
2021-12-06  8:44               ` Lang Yu
2021-12-06  9:12                 ` Lazar, Lijo
2021-12-06  9:31                   ` Lang Yu
2021-12-08  9:26 [PATCH 1/2] drm/amdgpu: remove power on/off SDMA in SMU hw_init/fini() Lang Yu
2021-12-08 10:06 ` Wang, Yang(Kevin)
2021-12-08 10:58   ` Lang Yu
2021-12-08 11:11     ` Wang, Yang(Kevin)
2021-12-21  8:41       ` Josef Johansson

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.