On Thu, Jul 22, 2021 at 9:54 PM Karol Herbst <kherbst@xxxxxxxxxx> wrote:
>
> On Thu, Jul 22, 2021 at 9:49 PM Ratchanan Srirattanamet
> <peathot@xxxxxxxxxxx> wrote:
> >
> > 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?
> >
>
> yeah.. it shouldn't. Although I am not quite sure how pci_get_class
> works... I was under the impression it only touches children but I
> guess it just iterates through the list of all devices? uhhhh mhhh...
> Yeah then I'd see why we never hit this on intel systems as the intel
> GPU is usually further up the list.
>
ohh, actually I missunderstood the entire code. Yeah.. seems you are
right and we are just stupid here.. okay.
> > > 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@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
> > >> 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@xxxxxxxxxxx>
> > >> ---
> > >>
> > >> 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@xxxxxxxxxxxxxxxxxxxxx
> > >> https://lists.freedesktop.org/mailman/listinfo/nouveau
> > >>
> > >
> >
_______________________________________________
Nouveau mailing list
Nouveau@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/nouveau
