[PATCH] drm/amdgpu/pm: Fix the power source flag error

[Lazar, Lijo]

On 1/18/2024 7:54 AM, Ma, Jun wrote:
> Hi Lijo,
> 
> On 1/17/2024 5:41 PM, Lazar, Lijo wrote:
>> On 1/17/2024 2:22 PM, Ma Jun wrote:
>>> The power source flag should be updated when
>>> [1] System receives an interrupt indicating that the power source
>>> has changed.
>>> [2] System resumes from suspend or runtime suspend
>>>
>>> Signed-off-by: Ma Jun <Jun.Ma2 at amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     | 24 +++++++++++--------
>>>    .../gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c    |  2 ++
>>>    .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c    |  2 ++
>>>    3 files changed, 18 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>> index c16703868e5c..e16d22e30a8a 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>> @@ -24,6 +24,7 @@
>>>    
>>>    #include <linux/firmware.h>
>>>    #include <linux/pci.h>
>>> +#include <linux/power_supply.h>
>>>    #include <linux/reboot.h>
>>>    
>>>    #include "amdgpu.h"
>>> @@ -793,6 +794,17 @@ static int smu_apply_default_config_table_settings(struct smu_context *smu)
>>>    	return smu_set_config_table(smu, &adev->pm.config_table);
>>>    }
>>>    
>>> +static void smu_update_power_source(struct amdgpu_device *adev)
>>> +{
>>> +	if (power_supply_is_system_supplied() > 0)
>>> +		adev->pm.ac_power = true;
>>> +	else
>>> +		adev->pm.ac_power = false;
>>> +
>>> +	if (is_support_sw_smu(adev))
>>> +		smu_set_ac_dc(adev->powerplay.pp_handle);
>>> +}
>>> +
>>>    static int smu_late_init(void *handle)
>>>    {
>>>    	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>> @@ -817,16 +829,8 @@ static int smu_late_init(void *handle)
>>>    	 * handle the switch automatically. Driver involvement
>>>    	 * is unnecessary.
>>>    	 */
>>> -	if (!smu->dc_controlled_by_gpio) {
>>> -		ret = smu_set_power_source(smu,
>>> -					   adev->pm.ac_power ? SMU_POWER_SOURCE_AC :
>>> -					   SMU_POWER_SOURCE_DC);
>>> -		if (ret) {
>>> -			dev_err(adev->dev, "Failed to switch to %s mode!\n",
>>> -				adev->pm.ac_power ? "AC" : "DC");
>>> -			return ret;
>>> -		}
>>> -	}
>>
>> For this part of the change - driver already updates FW with the initial
>> detected power state or the last detected power state before going for
>> suspend. Isn't that good enough?
>>
> 
> The power source may change during system suspend, so it's necessary to detect the
> power source again before system reads the power related data, such as max_power_limit.
> 

Ok. For dGPUs, then refresh of power state after resume will be required 
in GPIO controlled case also. Since FW is reloaded, FW may not detect it 
as a power source change.

Rather than creating another wrapper function, it is simpler to do 
something like
	adev->pm.ac_power = power_supply_is_system_supplied() > 0;
	ret = smu_set_ac_dc(smu);

Thanks,
Lijo

> Regards,
> Ma Jun
> 
> 
>> Thanks,
>> Lijo
>>
>>> +	if (!smu->dc_controlled_by_gpio)
>>> +		smu_update_power_source(adev);
>>>    
>>>    	if ((amdgpu_ip_version(adev, MP1_HWIP, 0) == IP_VERSION(13, 0, 1)) ||
>>>    	    (amdgpu_ip_version(adev, MP1_HWIP, 0) == IP_VERSION(13, 0, 3)))
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>> index 2e7f8d5cfc28..8047150fddd4 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
>>> @@ -1442,10 +1442,12 @@ static int smu_v11_0_irq_process(struct amdgpu_device *adev,
>>>    			case 0x3:
>>>    				dev_dbg(adev->dev, "Switched to AC mode!\n");
>>>    				schedule_work(&smu->interrupt_work);
>>> +				adev->pm.ac_power = true;
>>>    				break;
>>>    			case 0x4:
>>>    				dev_dbg(adev->dev, "Switched to DC mode!\n");
>>>    				schedule_work(&smu->interrupt_work);
>>> +				adev->pm.ac_power = false;
>>>    				break;
>>>    			case 0x7:
>>>    				/*
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
>>> index 771a3d457c33..c486182ff275 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
>>> @@ -1379,10 +1379,12 @@ static int smu_v13_0_irq_process(struct amdgpu_device *adev,
>>>    			case 0x3:
>>>    				dev_dbg(adev->dev, "Switched to AC mode!\n");
>>>    				smu_v13_0_ack_ac_dc_interrupt(smu);
>>> +				adev->pm.ac_power = true;
>>>    				break;
>>>    			case 0x4:
>>>    				dev_dbg(adev->dev, "Switched to DC mode!\n");
>>>    				smu_v13_0_ack_ac_dc_interrupt(smu);
>>> +				adev->pm.ac_power = false;
>>>    				break;
>>>    			case 0x7:
>>>    				/*
>>