Re: [PATCH 0/5] drm/amd/pm: Fix laptop issues on SMU6-7

["Timur Kristóf" <timur.kristof@gmail.com>]

Hi Jeremy & Alex,

> Apologies for my late reply. I tested the patch series (SI laptop
> 1002:6606) and the problem remains where the clock speeds don't boost upon
> switching to AC. Timur and I investigated this and found 2 problems

Thanks for getting back to us on this topic.
At Alex's suggestion, I removed the clock recalculation and added the check to 
verify ATOM_PP_PLATFORM_CAP_HARDWAREDC. I'm sad to hear that this broke your 
patches. I apologize for that.

Unfortunately I don't have a SI laptop GPU to test this stuff, so there was no 
way for me to verify the correctness of those changes before I sent the 
patches to the mailing list.

> 1. It seems that it is necessary after all to recompute clock speeds when
> toggling AC/DC. Sending PPSMC_MSG_RunningOnAC on its own has no effect.
> Each ASIC family's apply_state_adjust_rules appears to be responsible for
> the switch by setting the max_limits, and this function is only called as
> part of computing clocks.

That's right. I took another look at:
si_apply_state_adjust_rules()
smu7_apply_state_adjust_rules()

Both of these rely on adev->pm.ac_power when determining max_limits, and they 
set the maximum clocks accordingly. We should indeed re-calculate these clocks 
on both SI and SMU7 when there is an AC/DC switch to make sure to apply the 
updated max_limits. Additionally I think we should probably lock the mutexes 
to ensure that we are sending only one message at a time.

My suggestion would be to call pm_compute_clocks() inside notify_ac_dc(), and 
also to lock the mutexes:
https://gitlab.freedesktop.org/Venemo/linux/-/commit/
e98279dff480cc297cbb1fe50c2b71ebd65b9576

if that works, I'd like to submit that patch (and will also port it to SMU7).

> I'm considering removing the .notify_ac_dc field
> from the IP block entirely and just calling .pm_compute_clocks from
> amdgpu_pm_acpi_event_handler, but I only know for certain that this works
> for my GPU.

I don't agree with that. amdgpu_dpm is generic between all supported HW 
generations and shouldn't contain HW generation specific code. Also, it clearly 
doesn't work the same way on every GPU generation, so we shouldn't generalize.

Furthermore, we should minimize the amount of messages we send to the SMU, so 
we shouldn't send the RunningOnAC message every time we recompute the clocks, 
only when it actually switches to AC.

> 2. The ATOM_PP_PLATFORM_CAP_HARDWAREDC flag is enabled for my GPU, causing
> PPSMC_MSG_RunningOnAC to never be sent. Either the flag is enabled
> erroneously, or we're interpreting its intended usage incorrectly.

It's hard to judge that without having access to the hardware or docs.
Are you actually sure that the PPSMC_MSG_RunningOnAC is necessary on your 
laptop? Isn't it enough to just re-compute the clocks?

Can you check what exactly is the value of adev->pm.dpm.platform_caps on your 
laptop? Maybe we are looking at the wrong flag, or maybe the HARDWAREDC flag 
only refers to the AC->DC transition and not the DC->AC transition.

This is just guesswork on my part, but maybe we should look at the 
SBIOSPOWERSOURCE flag instead, which is explained in pptable_v1_0.h:
/* This cap indicates whether power source notificaiton is done by SBIOS 
directly. */
Can you check if this flag is set on your laptop?

Thanks & best regards,
Timur