[Nouveau] [PATCH] drm/nouveau: don't touch has_pr3 for likely-non-NVIDIA device
Ratchanan Srirattanamet
peathot at hotmail.com
Thu Jul 22 19:49:21 UTC 2021
Hello,
เมื่อ 22/7/64 เวลา 23:36 Karol Herbst เขียนว่า:
> hey, thanks for the patch. But I am a bit confused on why that patch
> actually helps. It should only be called for nvidia GPUs, but are we
> ending up checking it for AMD GPUs as well?
This is the call site of nouveau_dsm_pci_probe() in nouveau_acpi.c:
/* now do DSM detection */
while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev)) != NULL) {
vga_count++;
nouveau_dsm_pci_probe(pdev, &dhandle, &has_mux, &has_optimus,
&has_optimus_flags, &has_power_resources);
}
// And repeat with PCI_CLASS_DISPLAY_3D
So, all VGA and 3D cards, Nvidia or not, has this function called. Are
you suggesting that this should not happen?
> Mind posting the output of lspci -tvnn?
In case it helps, here it is:
$ lspci -tvnn
-[0000:00]-+-00.0 Advanced Micro Devices, Inc. [AMD] Renoir Root
Complex [1022:1630]
+-00.2 Advanced Micro Devices, Inc. [AMD] Renoir IOMMU
[1022:1631]
+-01.0 Advanced Micro Devices, Inc. [AMD] Renoir PCIe Dummy
Host Bridge [1022:1632]
+-01.1-[01]--+-00.0 NVIDIA Corporation Device [10de:1f95]
| \-00.1 NVIDIA Corporation Device [10de:10fa]
+-01.2-[02]----00.0 Micron Technology Inc Device [1344:5405]
+-02.0 Advanced Micro Devices, Inc. [AMD] Renoir PCIe Dummy
Host Bridge [1022:1632]
+-02.1-[03]----00.0 Realtek Semiconductor Co., Ltd.
RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168]
+-02.2-[04]----00.0 Intel Corporation Wi-Fi 6 AX200 [8086:2723]
+-08.0 Advanced Micro Devices, Inc. [AMD] Renoir PCIe Dummy
Host Bridge [1022:1632]
+-08.1-[05]--+-00.0 Advanced Micro Devices, Inc. [AMD/ATI]
Renoir [1002:1636]
| +-00.2 Advanced Micro Devices, Inc. [AMD]
Family 17h (Models 10h-1fh) Platform Security Processor [1022:15df]
| +-00.3 Advanced Micro Devices, Inc. [AMD]
Renoir USB 3.1 [1022:1639]
| +-00.4 Advanced Micro Devices, Inc. [AMD]
Renoir USB 3.1 [1022:1639]
| +-00.5 Advanced Micro Devices, Inc. [AMD]
Raven/Raven2/FireFlight/Renoir Audio Processor [1022:15e2]
| \-00.6 Advanced Micro Devices, Inc. [AMD]
Family 17h (Models 10h-1fh) HD Audio Controller [1022:15e3]
+-08.2-[06]--+-00.0 Advanced Micro Devices, Inc. [AMD] FCH
SATA Controller [AHCI mode] [1022:7901]
| \-00.1 Advanced Micro Devices, Inc. [AMD] FCH
SATA Controller [AHCI mode] [1022:7901]
+-14.0 Advanced Micro Devices, Inc. [AMD] FCH SMBus
Controller [1022:790b]
+-14.3 Advanced Micro Devices, Inc. [AMD] FCH LPC Bridge
[1022:790e]
+-18.0 Advanced Micro Devices, Inc. [AMD] Renoir Device 24:
Function 0 [1022:1448]
+-18.1 Advanced Micro Devices, Inc. [AMD] Renoir Device 24:
Function 1 [1022:1449]
+-18.2 Advanced Micro Devices, Inc. [AMD] Renoir Device 24:
Function 2 [1022:144a]
+-18.3 Advanced Micro Devices, Inc. [AMD] Renoir Device 24:
Function 3 [1022:144b]
+-18.4 Advanced Micro Devices, Inc. [AMD] Renoir Device 24:
Function 4 [1022:144c]
+-18.5 Advanced Micro Devices, Inc. [AMD] Renoir Device 24:
Function 5 [1022:144d]
+-18.6 Advanced Micro Devices, Inc. [AMD] Renoir Device 24:
Function 6 [1022:144e]
\-18.7 Advanced Micro Devices, Inc. [AMD] Renoir Device 24:
Function 7 [1022:144f]
Ratchanan Srirattanamet
> On Thu, Jul 22, 2021 at 5:10 AM 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
>> until we think this is an NVIDIA device of interest. Otherwise, if we
>> are called with another device after the NVIDIA device, we'll clober the
>> result of the NVIDIA device.
>>
>> In this case, 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.
>>
>> As a bonus, we'll also stop preventing _PR3 usage from the bridge for
>> unrelated devices, which is always nice, I guess.
>>
>> As noted in commit ccfc2d5cdb024 ("drm/nouveau: Use generic helper to
>> check _PR3 presence"), care is taken to leave the _PR3 detection outside
>> of the optimus_func condition.
>>
>> https://gitlab.freedesktop.org/drm/nouveau/-/issues/79
>>
>> Fixes: ccfc2d5cdb024 ("drm/nouveau: Use generic helper to check _PR3
>> presence")
>> Signed-off-by: Ratchanan Srirattanamet <peathot at hotmail.com>
>> ---
>>
>> Hello,
>>
>> This is my first time submitting a Linux patch. I've done a
>> number of PR/MR workflows on GitHub or GitLab, but never done any
>> email-oriented development. So, please excuse me if I'm doing something
>> incorrectly.
>>
>> drivers/gpu/drm/nouveau/nouveau_acpi.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c
>> index 7c15f6448428..c88bda3ac820 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
>> @@ -220,15 +220,6 @@ static void nouveau_dsm_pci_probe(struct pci_dev *pdev, acpi_handle *dhandle_out
>> int optimus_funcs;
>> struct pci_dev *parent_pdev;
>>
>> - *has_pr3 = false;
>> - parent_pdev = pci_upstream_bridge(pdev);
>> - if (parent_pdev) {
>> - if (parent_pdev->bridge_d3)
>> - *has_pr3 = pci_pr3_present(parent_pdev);
>> - else
>> - pci_d3cold_disable(pdev);
>> - }
>> -
>> dhandle = ACPI_HANDLE(&pdev->dev);
>> if (!dhandle)
>> return;
>> @@ -249,6 +240,15 @@ static void nouveau_dsm_pci_probe(struct pci_dev *pdev, acpi_handle *dhandle_out
>> *has_opt = !!optimus_funcs;
>> *has_opt_flags = optimus_funcs & (1 << NOUVEAU_DSM_OPTIMUS_FLAGS);
>>
>> + *has_pr3 = false;
>> + parent_pdev = pci_upstream_bridge(pdev);
>> + if (parent_pdev) {
>> + if (parent_pdev->bridge_d3)
>> + *has_pr3 = pci_pr3_present(parent_pdev);
>> + else
>> + pci_d3cold_disable(pdev);
>> + }
>> +
>> if (optimus_funcs) {
>> uint32_t result;
>> nouveau_optimus_dsm(dhandle, NOUVEAU_DSM_OPTIMUS_CAPS, 0,
>> --
>> 2.25.1
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
>>
>
More information about the Nouveau
mailing list