On 9/14/20 6:01 AM, Bjorn Helgaas wrote:
> On Mon, Sep 14, 2020 at 03:36:08AM +0200, Marek Vasut wrote:
>> On 9/13/20 7:22 PM, Bjorn Helgaas wrote:
>>
>> [...]
>>
>>>> The R-Car PCIe controller is capable of handling L0s/L1 link states.
>>>> While the controller can enter and exit L0s link state, and exit L1
>>>> link state, without any additional action from the driver, to enter
>>>> L1 link state, the driver must complete the link state transition by
>>>> issuing additional commands to the controller.
>>>
>>> So IIUC an R-Car Root Port can't enter L1 without help from the
>>> driver? That *sounds* like a hardware defect, since ASPM is supposed
>>> to be "hardware-autonomous" once configured.
>>
>> The driver needs to complete the switch from L0 to L1 link state, yes.
>>
>>>> The problem is, this transition is not atomic. The controller sets
>>>> PMEL1RX bit in PMSR register upon reception of PM_ENTER_L1 DLLP from
>>>> the PCIe card, but then the controller enters some sort of inbetween
>>>> state. The driver must detect this condition and complete the link
>>>> state transition, by setting L1IATN bit in PMCTLR and waiting for
>>>> the link state transition to complete.
>>>>
>>>> If a PCIe access happens inside of this window, where the controller
>>>> is inbetween L0 and L1 link states, the access generates a fault and
>>>> the ARM 'imprecise external abort' handler is invoked.
>>>
>>> Let me see if I understand this.
>>>
>>> 1) Endpoint sends PM_ENTER_L1
>>> 2) R-Car Root Port receives PM_ENTER_L1, sets PMEL1RX
>>> 3) R-Car driver does something to complete transition to L1
>>>
>>> And if the Endpoint driver does an MMIO access between 2) and 3), it
>>> causes an external abort?
>>
>> Yes, because the link is in this inbetween state. So you need 3) to
>> complete that transition.
>>
>>> I couldn't find anything in the R-Car driver today that completes the
>>> transition to L1.
>>
>> That's because this patch adds it.
>
> So with this patch, we finish the transition to L1 and immediately
> return to L0. That must mean that even in this in-between state we
> save some power?
Yes, that's how I understand it from the datasheet.
> And the link may stay in that in-between state indefinitely, until the
> CPU does an MMIO access or the device initiates a DMA? But it will
> never spend any time in L1 because the link never actually *gets* to
> L1 until some event that will take it back to L0 occurs?
It is in power saving mode as far as I can tell. It only needs this
extra bump to get fully in L1 and then back to L0.
>>> And the MMIO access will immediately bring the link back to L0, won't
>>> it?
>>
>> Yes
>>
>>> Is there any benefit to L1 at all for this Root Port? If it can't
>>> enter L1 and just stays in the in-between state until some event that
>>> will bring it *out* of L1, maybe we just need a quirk to stop
>>> advertising support for L1 in the first place.
>>
>> Power saving I would say.
>>
>>>> Just like other PCI controller drivers, here we hook the fault handler,
>>>> perform the fixup to help the controller enter L1 link state, and then
>>>> restart the instruction which triggered the fault. Since the controller
>>>> is in L1 link state now, the link can exit from L1 link state to L0 and
>>>> successfully complete the access.
>>>>
>>>> Note that this fixup is applicable only to Aarch32 R-Car controllers,
>>>> the Aarch64 R-Car perform the same fixup in TFA, see TFA commit
>>>> 0969397f2 ("rcar_gen3: plat: Prevent PCIe hang during L1X config access")
>>>
>>> TFA? This doesn't seem to be an upstream Linux commit; can you give a
>>> more specific reference?
>>
>> This is the Trusted Firmware from ARM, in this case it behaves similar
>> to ACPI. See e.g.:
>>
>> https://github.com/ARM-software/arm-trusted-firmware/commit/0969397f295621aa26b3d14b76dd397d22be58bf
>
> Thanks for that; please include it in the commit log for the next
> version.
OK
