Re: [PATCH v5 4/4] PCI: Allow PCI bridges to go to D3Hot on all Devicetree based platforms

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Tue, Aug 06, 2024 at 03:02:39PM +0200, Lukas Wunner wrote:
> On Tue, Aug 06, 2024 at 06:11:07PM +0530, Manivannan Sadhasivam wrote:
> > On Tue, Aug 06, 2024 at 08:53:01AM +0200, Lukas Wunner wrote:
> > > AFAICS we always program the device to go to D3hot and the platform
> > > then cuts power, thereby putting it into D3cold.  So D3hot is never
> > > skipped.  See __pci_set_power_state():
> > > 
> > > 	if (state == PCI_D3cold) {
> > > 		/*
> > > 		 * To put the device in D3cold, put it into D3hot in the native
> > > 		 * way, then put it into D3cold using platform ops.
> > > 		 */
> > > 		error = pci_set_low_power_state(dev, PCI_D3hot, locked);
> > > 
> > > 		if (pci_platform_power_transition(dev, PCI_D3cold))
> > > 			return error;
> > > 
> > 
> > This is applicable only to pci_set_power_state(), but AFAIK PCIe spec
> > doesn't mandate switching to D3Hot for entering D3Cold.
> 
> Per PCI Bus Power Management Interface Specification r1.2 sec 5.5 fig 5-1,
> the only supported state transition to D3cold is from D3hot.
> 
> Per PCIe r6.2 sec 5.2, "PM is compatible with the PCI Bus Power Management
> Interface Specification".
> 
> Granted, PCI-PM is an ancient spec, so I think anyone can be forgiven
> for not knowing its intricacies off-the-cuff. :)
> 

Ah, the grand old PCI-PM... I don't remember the last time I looked into it :)

> 
> > So the PCIe host controller drivers (especically non-ACPI platforms)
> > may just send PME_Turn_Off followed by removing the slot power
> > (which again is not controlled by pci_set_power_state())
> > as there are no non-ACPI related hooks as of now.
> 
> Ideally, devicetree-based platforms should be brought into the
> platform_pci_*() fold to align them with ACPI and get common
> behavior across all platforms.
> 

Yeah, that would be the ideal case. Unfortunately, there is no ideal ground for
DT :/ We do not even have the supplies populated properly. But with the advent
of power sequencing framework, I think this can be fixed.

Regarding your comment on patch 3/4, we already have the sysfs attribute to
control whether the device can be put into D3Cold or not and that is directly
coming from userspace. So there is no guarantee to assume that D3Hot support is
considered.

- Mani

-- 
மணிவண்ணன் சதாசிவம்