Ratchanan Srirattanamet
2023-May-23 21:11 UTC
[Nouveau] [PATCH v2] drm/nouveau: don't detect DSM for non-NVIDIA device
The call site of nouveau_dsm_pci_probe() uses single set of output variables for all invocations. So, we must not write anything to them unless it's an NVIDIA device. Otherwise, if we are called with another device after the NVIDIA device, we'll clober the result of the NVIDIA device. For example, if the other device doesn't have _PR3 resources, the detection later would miss the presence of power resource support, and the rest of the code will keep using Optimus DSM, breaking power management for that machine. Also, because we're detecting NVIDIA's DSM, it doesn't make sense to run this detection on a non-NVIDIA device anyway. Thus, check at the beginning of the detection code if this is an NVIDIA card, and just return if it isn't. This, together with commit d22915d22ded ("drm/nouveau/devinit/tu102-: wait for GFW_BOOT_PROGRESS == COMPLETED") developed independently and landed earlier, fixes runtime power management of the NVIDIA card in Lenovo Legion 5-15ARH05. Without this patch, the GPU resumption code will "timeout", sometimes hanging userspace. As a bonus, we'll also stop preventing _PR3 usage from the bridge for unrelated devices, which is always nice, I guess. Signed-off-by: Ratchanan Srirattanamet <peathot at hotmail.com> Closes: https://gitlab.freedesktop.org/drm/nouveau/-/issues/79 --- V1 -> V2: - Update the changelog to indicate that it now actually fixes the issue. - Add 'Closes:' tag. drivers/gpu/drm/nouveau/nouveau_acpi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c index 8cf096f841a9..a2ae8c21e4dc 100644 --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c @@ -220,6 +220,9 @@ static void nouveau_dsm_pci_probe(struct pci_dev *pdev, acpi_handle *dhandle_out int optimus_funcs; struct pci_dev *parent_pdev; + if (pdev->vendor != PCI_VENDOR_ID_NVIDIA) + return; + *has_pr3 = false; parent_pdev = pci_upstream_bridge(pdev); if (parent_pdev) { base-commit: 5fe326b4467689ef3690491ee2ad25ff4d81fe59 -- 2.34.1
Karol Herbst
2023-Jun-05 10:21 UTC
[Nouveau] [PATCH v2] drm/nouveau: don't detect DSM for non-NVIDIA device
On Tue, May 23, 2023 at 11:12?PM Ratchanan Srirattanamet <peathot at hotmail.com> wrote:> > The call site of nouveau_dsm_pci_probe() uses single set of output > variables for all invocations. So, we must not write anything to them > unless it's an NVIDIA device. Otherwise, if we are called with another > device after the NVIDIA device, we'll clober the result of the NVIDIA > device. > > For example, if the other device doesn't have _PR3 resources, the > detection later would miss the presence of power resource support, and > the rest of the code will keep using Optimus DSM, breaking power > management for that machine. > > Also, because we're detecting NVIDIA's DSM, it doesn't make sense to run > this detection on a non-NVIDIA device anyway. Thus, check at the > beginning of the detection code if this is an NVIDIA card, and just > return if it isn't. > > This, together with commit d22915d22ded ("drm/nouveau/devinit/tu102-: > wait for GFW_BOOT_PROGRESS == COMPLETED") developed independently and > landed earlier, fixes runtime power management of the NVIDIA card in > Lenovo Legion 5-15ARH05. Without this patch, the GPU resumption code > will "timeout", sometimes hanging userspace. > > As a bonus, we'll also stop preventing _PR3 usage from the bridge for > unrelated devices, which is always nice, I guess. > > Signed-off-by: Ratchanan Srirattanamet <peathot at hotmail.com> > Closes: https://gitlab.freedesktop.org/drm/nouveau/-/issues/79 > --- > V1 -> V2: > - Update the changelog to indicate that it now actually fixes the issue. > - Add 'Closes:' tag. > > drivers/gpu/drm/nouveau/nouveau_acpi.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c > index 8cf096f841a9..a2ae8c21e4dc 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c > +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c > @@ -220,6 +220,9 @@ static void nouveau_dsm_pci_probe(struct pci_dev *pdev, acpi_handle *dhandle_out > int optimus_funcs; > struct pci_dev *parent_pdev; > > + if (pdev->vendor != PCI_VENDOR_ID_NVIDIA) > + return; > + > *has_pr3 = false; > parent_pdev = pci_upstream_bridge(pdev); > if (parent_pdev) { > > base-commit: 5fe326b4467689ef3690491ee2ad25ff4d81fe59 > -- > 2.34.1 >Reviewed-by: Karol Herbst <kherbst at redhat.com> I'll add a Cc: <stable at vger.kernel.org> when pushing the change as I'll also think about how far in the past to get it applied. Though it shouldn't hurt as it shouldn't make things worse on any system.
Seemingly Similar Threads
- [PATCH] drm/nouveau/acpi: use DSM if bridge does not support D3cold
- [PATCH 1/1] drm/nouveau: Use generic helper to check _PR3 presence
- [PATCH 4/4] drm/nouveau/acpi: fix lockup with PCIe runtime PM
- [PATCH 4/4] drm/nouveau/acpi: fix lockup with PCIe runtime PM
- [PATCH 4/4] drm/nouveau/acpi: fix lockup with PCIe runtime PM