Re: [Nouveau] [PATCH 4/4] drm/nouveau/acpi: fix lockup with PCIe runtime PM

[Lukas Wunner <lukas@wunner.de>]

Hi Peter,

On Fri, May 27, 2016 at 11:31:23PM +0200, Peter Wu wrote:
> On Fri, May 27, 2016 at 02:01:39PM +0100, Emil Velikov wrote:
> > On 24 May 2016 at 23:53, Peter Wu <peter@lekensteyn.nl> wrote:
[snip]
> > > @@ -273,14 +296,14 @@ static bool nouveau_dsm_detect(void)
> > >                 vga_count++;
> > >
> > >                 nouveau_dsm_pci_probe(pdev, &has_mux, &has_optimus,
> > > -                                     &has_optimus_flags);
> > > +                                     &has_optimus_flags, &has_power_resources);
> > >         }
> > >
> > >         while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL) {
> > >                 vga_count++;
> > >
> > >                 nouveau_dsm_pci_probe(pdev, &has_mux, &has_optimus,
> > > -                                     &has_optimus_flags);
> > > +                                     &has_optimus_flags, &has_power_resources);
> > >         }
> > >
> > This and earlier patch break things in a subtle way.
> > 
> > Namely: upon the second (and any later) call into the
> > nouveau_dsm_pci_probe() function, the had_foo flags are reset. Thus
> > only the specifics of the _final_ device are being used (at a later
> > stage). IMHO one should change that to "_any_ device", which will
> > match the original code and the actual intent further down in the
> > file.
> 
> The flags are only reset if any of the MUX or Optimus handles are found.
> If both are missing, the flags are not overridden. This is from patch 1:
> 
> +       /* Does not look like a Nvidia device. */
> +       if (!supports_mux && !supports_opt)
> +               return;
> 
> The reason why later calls override early ones is because some Optimus
> laptops have the _DSM method on both the Intel GPU (00:02.0) and the
> Nvidia one (01:00.0).

Sounds like you may want to check for pdev->vendor == PCI_VENDOR_ID_NVIDIA
or export pci_get_dev_by_id() and use that to match for class and vendor.

Best regards,

Lukas