search for: pci_restore_config_space

...more verbose info was moved to bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=201069 This patch is aimed at v4.19 (and maybe v4.18-stable); we may follow up with more intrusive improvements for v4.20+. v2: reimplement the register restore within the existing pci_restore_config_space() code. drivers/pci/pci.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 29ff9619b5fa..e1704100e72d 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -1289,13 +1289,15 @@ int pci_save_state(...

...AR, I/O Base + > Limit, Memory Base + Limit, etc. from top to bottom), see annotations: > https://www.spinics.net/lists/linux-pci/msg75856.html > > Linux has a generic "restore" operation that works backwards from the > end of the PCI config space to the beginning, see > pci_restore_config_space. Do you have a dmesg where you see the > "restoring config space at offset" messages? Interesting, I had not spotted this code. The logs for the affected bridge on Asus X542UQ: 0000:00:1c.0: restoring config space at offset 0x3c (was 0x100, writing 0x1001ff) 0000:00:1c.0: restoring...

...d_val, int retry, bool force) { u32 val; pci_read_config_dword(pdev, offset, &val); - if (val == saved_val) + if (!force && val == saved_val) return; for (;;) { @@ -1313,25 +1313,34 @@ static void pci_restore_config_dword(struct pci_dev *pdev, int offset, } static void pci_restore_config_space_range(struct pci_dev *pdev, - int start, int end, int retry) + int start, int end, int retry, + bool force) { int index; for (index = end; index >= start; index--) pci_restore_config_dword(pdev, 4 * index, pdev->saved_config_space[index], - retry); +...

...ed to bugzilla: > https://bugzilla.kernel.org/show_bug.cgi?id=201069 > > This patch is aimed at v4.19 (and maybe v4.18-stable); we may follow > up with more intrusive improvements for v4.20+. > > v2: reimplement the register restore within the existing > pci_restore_config_space() code. > > drivers/pci/pci.c | 31 +++++++++++++++++++++---------- > 1 file changed, 21 insertions(+), 10 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 29ff9619b5fa..e1704100e72d 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c >...

...R, I/O Base + > Limit, Memory Base + Limit, etc. from top to bottom), see annotations: > https://www.spinics.net/lists/linux-pci/msg75856.html > > Linux has a generic "restore" operation that works backwards from the > end of the PCI config space to the beginning, see > pci_restore_config_space. Do you have a dmesg where you see the > "restoring config space at offset" messages? > > Would it be reasonable to unconditionally write these registers in > pci_restore_config_dword, like Windows does? That sounds reasonable to me. We did write them unconditionally, prior...

...evice may have > + * been in D3cold as well and require restoration of its BARs. > */ > - if (!pci_dev->driver) > + if (!pci_dev->driver) { > + pci_restore_bars(pci_dev); If we do decide not to do a full-blown restore, how do we decide whether to use pci_restore_bars() or pci_restore_config_space()? I'm not sure why we have both. The pci_restore_bars() path looks a little smarter in that it is more careful when updating 64-bit BARs that can't be updated atomically. > return 0; > + } > > if (!pm || !pm->runtime_resume) > return -ENOSYS; > diff --git...

...y rewrites these registers (BAR, I/O Base + Limit, Memory Base + Limit, etc. from top to bottom), see annotations: https://www.spinics.net/lists/linux-pci/msg75856.html Linux has a generic "restore" operation that works backwards from the end of the PCI config space to the beginning, see pci_restore_config_space. Do you have a dmesg where you see the "restoring config space at offset" messages? Would it be reasonable to unconditionally write these registers in pci_restore_config_dword, like Windows does? Kind regards, Peter

...al); > > - if (val == saved_val) > > + if (!force && val == saved_val) > > return; > > > > for (;;) { > > @@ -1313,25 +1313,34 @@ static void pci_restore_config_dword(struct pci_dev *pdev, int offset, > > } > > > > static void pci_restore_config_space_range(struct pci_dev *pdev, > > - int start, int end, int retry) > > + int start, int end, int retry, > > + bool force) > > { > > int index; > > > > for (index = end; index >= start; index--) > > pci_restore_config_dw...

...ev, offset, &val); > - if (val == saved_val) > + if (!force && val == saved_val) > return; > > for (;;) { > @@ -1313,25 +1313,34 @@ static void pci_restore_config_dword(struct pci_dev *pdev, int offset, > } > > static void pci_restore_config_space_range(struct pci_dev *pdev, > - int start, int end, int retry) > + int start, int end, int retry, > + bool force) > { > int index; > > for...

...pci_read_config_dword(pdev, offset, &val); > - if (val == saved_val) > + if (!force && val == saved_val) > return; > > for (;;) { > @@ -1313,25 +1313,34 @@ static void pci_restore_config_dword(struct pci_dev *pdev, int offset, > } > > static void pci_restore_config_space_range(struct pci_dev *pdev, > - int start, int end, int retry) > + int start, int end, int retry, > + bool force) > { > int index; > > for (index = end; index >= start; index--) > pci_restore_config_dword(pdev, 4 * index, > pdev-&g...

...equire restoration of its BARs. >> */ >> - if (!pci_dev->driver) >> + if (!pci_dev->driver) { >> + pci_restore_bars(pci_dev); > > If we do decide not to do a full-blown restore, how do we decide > whether to use pci_restore_bars() or pci_restore_config_space()? > > I'm not sure why we have both. Me neither. > The pci_restore_bars() path looks a > little smarter in that it is more careful when updating 64-bit BARs > that can't be updated atomically. > >> return 0; >> + } >> >> i...

On 38+ Intel-based Asus products, the nvidia GPU becomes unusable after S3 suspend/resume. The affected products include multiple generations of nvidia GPUs and Intel SoCs. After resume, nouveau logs many errors such as: fifo: fault 00 [READ] at 0000005555555000 engine 00 [GR] client 04 [HUB/FE] reason 4a [] on channel -1 [007fa91000 unknown] DRM: failed to idle channel 0 [DRM]

...ved_val) >>> + if (!force && val == saved_val) >>> return; >>> >>> for (;;) { >>> @@ -1313,25 +1313,34 @@ static void pci_restore_config_dword(struct pci_dev *pdev, int offset, >>> } >>> >>> static void pci_restore_config_space_range(struct pci_dev *pdev, >>> - int start, int end, int retry) >>> + int start, int end, int retry, >>> + bool force) >>> { >>> int index; >>> >>> for (index = end; index >= start; index--) >>&...

...; >> */ > >> - if (!pci_dev->driver) > >> + if (!pci_dev->driver) { > >> + pci_restore_bars(pci_dev); > > > > If we do decide not to do a full-blown restore, how do we decide > > whether to use pci_restore_bars() or pci_restore_config_space()? > > > > I'm not sure why we have both. > > Me neither. > > > The pci_restore_bars() path looks a > > little smarter in that it is more careful when updating 64-bit BARs > > that can't be updated atomically. > > > >> re...

Modernize vga_switcheroo by using a "device link" to enforce a runtime PM dependency from an HDA controller to the GPU it's integrated into. Remove thereby obsoleted code and fix a bunch of bugs. Device links were introduced in v4.10. Users might see a small power saving if the discrete GPU is in use and its HDA controller is not, because the HDA controller is now allowed to runtime