On 1/18/2024 4:38 PM, Lazar, Lijo wrote:
> On 1/18/2024 12:57 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@xxxxxxx>
>> ---
>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 13 +++----------
>> drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c | 2 ++
>> drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c | 2 ++
>> 3 files changed, 7 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..a54663f2e2ab 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"
>> @@ -817,16 +818,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;
>> - }
>> - }
>> + adev->pm.ac_power = power_supply_is_system_supplied() > 0;
>> + smu_set_ac_dc(smu);
>
> In the older logic, driver initialization/resume fails if the message
> fails. This one doesn't care about the return value. Is there a reason
> to ignore and continue?
I think printing an error message in smu_set_ac_dc() is enough,
and stopping the driver initialization/resume seems a bit excessive.
Regards,
Ma Jun
>
> Thanks,
> Lijo
>>
>> 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:
>> /*
>
