From: Karol Herbst <kherbst@redhat.com>
To: Ratchanan Srirattanamet <peathot@hotmail.com>
Cc: Ben Skeggs <bskeggs@redhat.com>, nouveau <nouveau@lists.freedesktop.org>
Subject: Re: [Nouveau] [PATCH] drm/nouveau: don't detect DSM for non-NVIDIA device
Date: Mon, 13 Sep 2021 14:25:07 +0200 [thread overview]
Message-ID: <CACO55tuzHmwrgPEnzHf-xVHt3n4gqrzQ5NJD63xMnsHYNFHiSw@mail.gmail.com> (raw)
In-Reply-To: <DM6PR19MB278000DDEC936A67E85752E5BCF59@DM6PR19MB2780.namprd19.prod.outlook.com>
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@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@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
>
prev parent reply other threads:[~2021-09-13 12:25 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-08 19:23 [Nouveau] [PATCH] drm/nouveau: don't detect DSM for non-NVIDIA device Ratchanan Srirattanamet
2021-09-13 12:25 ` Karol Herbst [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CACO55tuzHmwrgPEnzHf-xVHt3n4gqrzQ5NJD63xMnsHYNFHiSw@mail.gmail.com \
--to=kherbst@redhat.com \
--cc=bskeggs@redhat.com \
--cc=nouveau@lists.freedesktop.org \
--cc=peathot@hotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).