Sorry for taking this long to take a look, it's just that I don't want this to get merged without testing, and I'd like to test it on the different kinds of hybrid GPU setups we have so that nothing unexpected happens here. I am not 100% sure how all of that works before optimus, so I have to find a laptop covering this generation of hardware. I think my oldest one here actually is, but the entire system on it is quite broken and it still has some private stuff on it so I would rather not touch that one :( But if others would be able to test that, that would be awesome. I will see if I can find a clean machine somewhere at work I can use for testing. On Sun, Aug 8, 2021 at 9:24 PM Ratchanan Srirattanamet <peathot@xxxxxxxxxxx> 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. In particular, this fixes power management > of the NVIDIA card in Lenovo Legion 5-15ARH05... well, at least > partially. New error shows up, but this patch is correct in itself > anyway. > > 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. > > As a bonus, we'll also stop preventing _PR3 usage from the bridge for > unrelated devices, which is always nice, I guess. > > https://gitlab.freedesktop.org/drm/nouveau/-/issues/79 > > Signed-off-by: Ratchanan Srirattanamet <peathot@xxxxxxxxxxx> > --- > > It's been discussed in the "previous" patch series ("[PATCH] drm/ > nouveau: don't touch has_pr3 for likely-non-NVIDIA device" [1]) that, > instead of changing how the detection works, it's better to check if > the device's vendor is NVIDIA, or if it's a secondary device. I don't > find a feasible way to detect the secondary-ness of a device, so I > opted for checking the vendor code [2]. > > Thus, this makes the patch different enough to warant a different > summary phrase and description. > > [1] https://lore.kernel.org/nouveau/CACO55tsYWW_dfAFWBFLrH51SVivUeN72Omc6DRTCtzVWSLE68w@xxxxxxxxxxxxxx/T/#mb710275dc1dd7e9d83028c218b886400d3a43bfc > [2] It's been suggested to use `nouveau_dsm_get_client_id()`, however, > on my machine it would return _DIS on both cards, so it can't be > used. > > 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 7c15f6448428..9c55f205ab66 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) { > -- > 2.25.1 >
