On Thu, Jul 20, 2023 at 10:13:18PM +0530, Manivannan Sadhasivam wrote: > On Thu, Jul 20, 2023 at 12:26:38PM -0400, Frank Li wrote: > > On Thu, Jul 20, 2023 at 09:37:38PM +0530, Manivannan Sadhasivam wrote: > > > On Thu, Jul 20, 2023 at 10:37:36AM -0400, Frank Li wrote: > > > > On Thu, Jul 20, 2023 at 07:55:09PM +0530, Manivannan Sadhasivam wrote: > > > > > On Tue, Jul 18, 2023 at 03:34:26PM +0530, Manivannan Sadhasivam wrote: > > > > > > On Mon, Jul 17, 2023 at 02:36:19PM -0400, Frank Li wrote: > > > > > > > On Mon, Jul 17, 2023 at 10:15:26PM +0530, Manivannan Sadhasivam wrote: > > > > > > > > On Wed, Apr 19, 2023 at 12:41:17PM -0400, Frank Li wrote: > > > > > > > > > Introduced helper function dw_pcie_get_ltssm to retrieve SMLH_LTSS_STATE. > > > > > > > > > Added API pme_turn_off and exit_from_l2 for managing L2/L3 state transitions. > > > > > > > > > > > > > > > > > > Typical L2 entry workflow: > > > > > > > > > > > > > > > > > > 1. Transmit PME turn off signal to PCI devices. > > > > > > > > > 2. Await link entering L2_IDLE state. > > > > > > > > > > > > > > > > AFAIK, typical workflow is to wait for PME_To_Ack. > > > > > > > > > > > > > > 1 Already wait for PME_to_ACK, 2, just wait for link actual enter L2. > > > > > > > I think PCI RC needs some time to set link enter L2 after get ACK from > > > > > > > PME. > > > > > > > > > > > > > > > > > One more comment. If you transition the device to L2/L3, then it can loose power > > > > > if Vaux was not provided. In that case, can all the devices work after resume? > > > > > Most notably NVMe? > > > > > > > > I have not hardware to do such test, NVMe driver will reinit everything after > > > > resume if no L1.1\L1.2 support. If there are L1.1\L1.2, NVME expect it leave > > > > at L1.2 at suspend to get better resume latency. > > > > > > > > > > To be precise, NVMe driver will shutdown the device if there is no ASPM support > > > and keep it in low power mode otherwise (there are other cases as well but we do > > > not need to worry). > > > > I supposed this should work. but I have not hardware to test it now. NMVE already > > sent shut down command to SSD, which can safely turn off. after resume, that most > > likely a cold reset. > > > > NO, it won't work and that's the reason the Qcom platforms are not transitioning > the link to L2/L3 state during suspend. This applies to other platforms > including layerscape as well. > > > > > > > But here you are not checking for ASPM state in the suspend path, and just > > > forcing the link to be in L2/L3 (thereby D3Cold) even though NVMe driver may > > > expect it to be in low power state like ASPM/APST. > > > > This function is not called defaultly and need platform driver to call it as need. > > Actually, I think PCI framework should handle L1.2 and L2 case, some devices > > or user case want to L1.2 to get better resume latency, some devices want to > > L2 to get better power saving, which out of scope of this patches. > > > > I'm referring to the platform where these helper functions are being used which > is layerscape. It doesn't matter whether you test this series with NVMe or not, > it will not work unless you disable ASPM. I tested with NVNE. You are right, ASPM is disabled at layerscape platform. > > > This patch just handle L2 case, I remember L1.2 don't expect send PME at all. > > > > > > > > So you should only put the link to L2/L3 if there is no ASPM support. Otherwise, > > > you'll ending up with bug reports when users connect NVMe to it. > > > > Platform should choose call or no call this function according to their > > user case. So far, I have not found a good mathod to let ASPM to affect > > suspend/resume. > > > > You are missing my point here. If any platform decides to use these helper > functions, they will face problems with NVMe. So please add a check for ASPM > state before doing any L2/L3 handling. > > I agree that it may not be optimal w.r.t power savings, but the PCIe controller > driver should work for all devices. I can add aspm check here. but I hope there are a flags in future, which can show if expect pci controller enter L2. Frank > > - Mani > > > > > > > - Mani > > > > > > > This API help remove duplicate codes and it can be improved gradually. > > > > > > > > > > > > > > > > > > - Mani > > > > > > > > > > > > > > > -- > > > > > மணிவண்ணன் சதாசிவம் > > > > > > -- > > > மணிவண்ணன் சதாசிவம் > > -- > மணிவண்ணன் சதாசிவம்
