[PATCH] drm/amd/pm: avoid duplicate powergate/ungate setting

[Quan, Evan]

[AMD Official Use Only]



> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar at amd.com>
> Sent: Monday, November 8, 2021 10:41 PM
> To: Koenig, Christian <Christian.Koenig at amd.com>; Borislav Petkov
> <bp at suse.de>; Paul Menzel <pmenzel at molgen.mpg.de>
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Quan, Evan
> <Evan.Quan at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/amd/pm: avoid duplicate powergate/ungate
> setting
> 
> 
> 
> On 11/8/2021 7:44 PM, Christian König wrote:
> > Am 08.11.21 um 12:15 schrieb Borislav Petkov:
> >> On Mon, Nov 08, 2021 at 09:51:03AM +0100, Paul Menzel wrote:
> >>> Please elaborate the kind of issues.
> >> It fails to reboot on Carrizo-based laptops.
> >
> > That doesn't necessary sounds like a good idea to me then.
> >
> > What exactly is going wrong here? And what is the rational that we
> > must fix this by avoiding updating the current state?
> >
> 
> Reboot will trigger a suspend of IPs. As part of UVD/VCE suspend, now there
> is an added logic to power gate the IP as part of suspend sequence. In case of
> UVD/VCE, power gating would have already happened as part of idle work
> execution.
[Quan, Evan] Thanks Lijo. Some supplement for the added-on power gate logic on suspend:
1. if the UVD/VCE is still using on reboot triggered, idle work will be not triggered and the add-on power gate logic
can help to put the Ips into correct gate state.
2. If the UVD/VCE is already idle on reboot triggered, then as Lijo mentioned, the idle work will be already triggered
and the iP is in gate state. Then the add-on power gate logic on suspend will be extra and even harmful(on Carrizo platform).

So, to address the issue of 2 and not break 1, we will track the pg state internally and just bail out on 2nd consecutive gate request.

P.S. gfxoff feature suffers the same issue. The patch can address it also.
 
BR
Evan
> 
> In any case, power gating is done by SMU FW. The assumption here is - the
> logic to power gate IP could involve register programming. AFAIK, accessing
> some UVD/VCE regs during powergate state could cause a hang unless the
> anti-hang mechanism is not programmed. That means either FW or driver
> needs to track the state of IP before accessing those regs and in this case
> probably FW is assuming driver to be responsible. i.e., not to call power off
> again if it's already powered down.
> 
> Though that seems to be a bad assumption on part of FW, it is still a
> possibility. Haven't seen the actual FW code, it's a very old program.
> 
> Thanks,
> Lijo
> 
> > See we usually assume that updating to the already set state is
> > unproblematic and that here sounds like just trying to mitigated some
> > issues instead of fixing the root cause.
> >
> > Regards,
> > Christian.
> >
> >>
> >> Whoever commits this, pls add
> >>
> >> Link: https://lore.kernel.org/r/YV81vidWQLWvATMM@zn.tnic
> >>
> >> so that it is clear what the whole story way.
> >>
> >> Thx.
> >>
> >