Re: [PATCH v4] PCI: Call _REG when transitioning D-states

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Jun 22, 2023 at 12:43:56PM -0500, Limonciello, Mario wrote:
> On 6/21/2023 5:52 PM, Limonciello, Mario wrote:
> > On 6/21/2023 5:28 PM, Bjorn Helgaas wrote:
> > > On Tue, Jun 20, 2023 at 09:04:51AM -0500, Mario Limonciello wrote:
> > > > Section 6.5.4 of the ACPI 6.4 spec describes how AML is unable
> > > > to access an OperationRegion unless `_REG` has been called.

> > > If so, I guess AML that uses ACPI_ADR_SPACE_PCI_CONFIG won't work
> > > until after we set the relevant device to D0?
> >
> > The particular problem that that exposed this issue doesn't
> > happen until suspend/resume time, but yes I think this should
> > be called when setting the device to D0.

> > > Do we explicitly set devices to D0 during enumeration, e.g., somewhere
> > > in the pci_scan_device() path?  If not, should we?
> >
> > AFAICT it's happening for PCIe ports as part of:
> > pcie_portdrv_probe
> > ->pcie_port_device_register
> > ->->pci_enable_device
> > ->->->pci_enable_device_flags
> > ->->->->do_pci_enable_device
> > ->->->->->pci_set_power_state(pci_dev, PCI_D0)
> 
> Just to add to this; I double checked and one of the devices that
> doesn't have a driver on my system has the power_state set to
> "unknown". I don't think it would be appropriate to explicitly put
> "all devices without drivers" into D0 as this could block low power
> states for the SOC.

I think we probably *should* be evaluating _REG for bridges and
endpoints during enumeration, but I don't think we should depend on
pci_enable_device() to do it.

If a method uses the PCI config OpRegion, I think it should work at
any time, but with this patch it looks like it will only work after a
transition to D0.

> On the Intel side, there is some special stuff in intel-pmc-core for example
> that explicitly puts specific driverless devices into D3 to ensure low power
> states.  If we put everything without a driver into D0 we may break stuff
> like
> that.