Expecting to revert commit 55285e21f045 "fbdev/efifb: Release PCI device ..."

[Deucher, Alexander]

[Public]

> -----Original Message-----
> From: Linus Torvalds <torvalds at linux-foundation.org>
> Sent: Monday, December 20, 2021 5:21 PM
> To: Imre Deak <imre.deak at intel.com>; Koenig, Christian
> <Christian.Koenig at amd.com>; Pan, Xinhui <Xinhui.Pan at amd.com>;
> Wentland, Harry <Harry.Wentland at amd.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Kai-Heng Feng
> <kai.heng.feng at canonical.com>; amd-gfx list <amd-
> gfx at lists.freedesktop.org>
> Subject: Re: Expecting to revert commit 55285e21f045 "fbdev/efifb: Release
> PCI device ..."
> 
> [ Adding back in more amd people and the amd list, the people Daniel added
> seem to have gotten lost again, but I think people at least saw my original
> report thanks to Daniel ]
> 
> With "amdgpu.runpm=0", things are better, but not perfect. With that I can
> lock the screen, and it has to go through *two* cycles of "No signal, turning
> off", but on the second cycle it does finally work.
> 
> This was exposed by commit 55285e21f045 ("fbdev/efifb: Release PCI
> device's runtime PM ref during FB destroy"), probably because that made
> runtime PM actually potentially work, but it is then broken on amdgpu.
> 
> Absolutely nothing odd in my setup. Two monitors, one GPU. PCI ID
> 1002:67df rev e7, subsystem ID 1da2:e353.
> 
> I'd expect pretty much any amdgpu person to see this.
> 
> On Mon, Dec 20, 2021 at 2:04 PM Linus Torvalds <torvalds at linux-
> foundation.org> wrote:
> >
> > Note: on my machine, I get that
> >
> >    amdgpu 0000:49:00.0: amdgpu: Using BACO for runtime pm
> >
> > so maybe the other possible runtime pm models (ARPX and BOCO) are ok,
> > and it's only that BACO case that is broken.
> 
> Hmm. The *documentation* says:
> 
>     PX runtime pm
>         2 = force enable with BAMACO,
>         1 = force enable with BACO,
>         0 = disable,
>         -1 = PX only default
> 
> but the code actually makes anything != 0 enable it, except on VEGA20 and
> ARCTURUS, where it needs to be positive.
> 
> My card is apparently "POLARIS10", whatever that means, which means that
> any non-zero value of amdgpu_runtime_pm will enable runtime PM as long
> as "amdgpu_device_supports_baco()" is true. Which it is.
> 
> Whatever. Now I'm just kwetching about the documentation not matching
> what I see the code doing, which is never a great sign when things don't
> work.

Apologies on the documentation.  -1 is the default and is enabled for all dGPUs which support runtime D3.  It was never fixed up when we extended support for runtime pm beyond PX/HG laptops.  Fixed up the documentation here:
https://patchwork.freedesktop.org/patch/467681/

Alex