Hi Lorenzo, On 20 August 2018 15:48 Lorenzo Pieralisi wrote: > On Mon, Aug 20, 2018 at 01:44:48PM +0000, Phil Edworthy wrote: > > [...] > > > However, both before and after this patch, the RP does not transition > > L1 when the endpoints change to L1. > > This patch only transitions the RP to L1 during accessing a card's > > config registers, if the RP is not in L1 link state and has received > > PM_ENTER_L1 DLLP (e.g. resume). After this, the hardware will handle > > the transition out of L1. > > > > The relevant part of the rcar manual says: "After a recovery to L0, if > > the device is in the Non-D0 state and PM_Enter_L1 DLLP is transmitted > > from the downstream device, software should confirm that hardware is > > in the L0 state (PMSR.PMSTATE = L0) and initiate the L1 transition > > sequence again (write 1 to PMCTLR.L1IATN). In this case, the sequence > > is: L0 → L1 → L0 recovery → L1 again." > > Can you map these FSM steps to this patch code please ? I would like to > understand what Link state maps to which command written and when. I don’t think I can because we are not initially entering L1. Looking at this again, I think this section of the manual only helps in indicating how to detect we should have gone into L1 and how to poke the HW to initiate the transition to L1. On system suspend, the EP sends PM_Enter_L1 DLLP and enters L1 state. The rcar RP cannot enter L1 by HW alone, so is still in L0. The only way out of this from the PCIe spec FSM is for both EP and RP to enter the Recovery state. The patch simply detects that we should have gone into L1, and so initiates that state change, and the HW will then handle the transition from L1 to Recovery and then on to L0. > > I don’t think the potential issue that Bjorn talked about can happen > > because the RP does go into L1. I could be wrong though... > > I do not understand this paragraph, mind elaborating on it ? As rcar RP only supports D0 and D3hot/cold, (the manual says it supports D3cold, but I cannot see how if it doesn’t support L2 or L3 states), if you force the link to D3, we can only be in L1 state. > > The driver should also have a runtime-PM hook to transition to L1 on > > suspend in order to save power. However, that is somewhat separate to > > the problem the patch fixes. > > Yes that's a separate patch. > > Thanks for chiming in, let's try to get to the bottom of this thread. > > Lorenzo Thanks Phil
