[Nouveau] [PATCH] drm/nouveau: don't detect DSM for non-NVIDIA device

[Karol Herbst]

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 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. 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 at hotmail.com>
> ---
>
> 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@mail.gmail.com/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
>