On Thu, Mar 09, 2023 at 06:57:38PM -0600, Mario Limonciello wrote: > On 3/9/23 16:30, Bjorn Helgaas wrote: > > On Thu, Mar 09, 2023 at 12:32:41PM -0600, Limonciello, Mario wrote: > > > On 3/9/2023 12:25, Bjorn Helgaas wrote: > > > ... > > > > > > > > https://gitlab.freedesktop.org/agd5f/linux/-/commit/07494a25fc8881e122c242a46b5c53e0e4403139 > > > > > > > > That nbio_v7.2.c patch and this patch don't look anything alike. It > > > > looks like the nbio_v7.2.c patch might run once? Could *this* be done > > > > once at enumeration-time, too? > > > > > > They don't look anything alike because they're attacking the problem from > > > different angles. > > > > Why do we need different angles? > > The GPU driver approach only works if the GPU is enabled. If the GPU could > never be disabled then it alone would be sufficient. > > > > The NBIO patch fixes the initialization value for the internal registers. > > > This is what the BIOS "should" have done. When the internal registers are > > > configured properly then the behavior the kernel expects works as well. > > > > > > The NBIO patch will run both at amdgpu startup as well as when resuming from > > > suspend. > > > > If initializing something as BIOS should have done makes the hardware > > work correctly, isn't once enough? Why does the NBIO patch need to > > run at resume-time? > > During suspend some internal registers are in a power domain that the state > will be lost. These are typically restored by the BIOS to the values > defined in initialization tables before handing control back to the OS. I don't quite get this. I thought I read that if BIOS had initialized the hardware correctly, a D0->D3hot->D0 transition would work without any issues. Linux can do this with PMCSR writes and BIOS isn't involved at all. Bjorn
