Message-ID: <57D71FD9.9070007@codeaurora.org>
Date:   Mon, 12 Sep 2016 15:36:25 -0600
From:   Karthikeyan Ramasubramanian <kramasub@...eaurora.org>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Jeffrey Hugo <jhugo@...eaurora.org>
Cc:     Lina Iyer <lina.iyer@...aro.org>, Ohad Ben-Cohen <ohad@...ery.com>,
        linux-remoteproc@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 00/17] Make rpmsg a framework

On 9/12/2016 1:58 PM, Bjorn Andersson wrote:
> On Mon 12 Sep 12:21 PDT 2016, Jeffrey Hugo wrote:
>
>> On 9/12/2016 12:49 PM, Bjorn Andersson wrote:
>>> On Mon 12 Sep 11:13 PDT 2016, Jeffrey Hugo wrote:
>>>
>>>> On 9/12/2016 12:00 PM, Bjorn Andersson wrote:
> [..]
>>>>> Can you point me to the downstream code where this is implemented so I
>>>>> can have a look? Do you expect to get the response on that request?
>>>>
>>>> Have a look at -
>>>> smd_mask_receive_interrupt()
>>>> smd_is_pkt_avail()
>>>>
>>>
>>> In msm-3.18 these still seems to only come from either
>>> msm_rpm_enter_sleep() and the rpm-clock driver, related to flushing
>>> cached sleep state requests.
>>>
>>>> Every request to the RPM generates a response.  The Linux RPM driver may
>>>> decide to let the response sit in the fifo, or it may need to read and
>>>> process it.
>>>>
>>>
>>> Right, I presume we save some time by not waiting for these responses as
>>> we want to reach sleep as soon as possible. The answer I got last time
>>> this was discussed was that it was an optimization, not a functional
>>> requirement.
>>
>> Two optimizations in play here.
>>
>> First, disabling interrupts prevents an immediate wakeup.  When the system
>> is entering sleep, IRQs are disabled.  The sleep request to RPM will trigger
>> a response, and the IRQ for that response will be queued. Once the sleep
>> processing is done, IRQs get enabled, so the pending IRQ from RPM will cause
>> an immediate wakeup.  The system will process the wakeup, and then go back
>> to sleep (sans request because nothing has changed).  This down-up-down
>> processing burns a lot of power.
>>
>
> But which "sleep request" is this? The only one I can find is the
> flushing of sleep state values from the rpm resource tables.
>
>> Second is not waiting for the response.  Linux doesn't really do anything
>> with the sleep request response, so we can enter sleep faster by not waiting
>> for the response and processing (discarding) it when the system wakes up as
>> scheduled.
>
> Right, as long as the RPM code doesn't consider it a timeout don't have
> a problem if those ack's are handled after the resume.
>
>> However, Linux needs to ensure there is enough fifo space to
>> hold that response while asleep, otherwise the RPM will panic and crash the
>> system.  Therefore, if there are a number of outstanding requests that would
>> fill the fifo, then the RPM driver on Linux needs to spin and drain requests
>> from the fifo until a minimum free space buffer to hold additional expected
>> pending responses is established.  This has to occur with IRQs disabled.
>>
>
> Right. Which means that the RPM driver needs to know how large the rx
> fifo is, what overhead the underlaying transport mechanism has and then
> calculate how many responses it should leave room for.
>
> [..]
>>>> If I recall correctly, there was a parameter in the RPM driver
>>>> for the transmit function that indicated if the request was being made in
>>>> atomic context or not, which would change the behavior of how the transmit
>>>> was handled.
>>>>
>>>
>>> You're correct, the question is still which of these code paths are
>>> actually needed and to motivate the endless maintenance of the extra
>>> code.
>>
>> If we are just talking about transmitting in atomic context (not necessarily
>> related to sleep), if I recall correctly, some bus requests are sent to RPM
>> in atomic context, some APR requests to the Audio DSP are done in atomic
>> context, and I think IPC Router uses atomic context in some cases.  As a
>> generic framework that should support usecases to all processors/subsystems,
>> I don't think transmitting in atomic context is a special case for
>> RPM/sleep.
>>
>
> I have not looked through all of APR yet and don't know where msm_bus is
> heading, but for IPC-router your correct that the downstream driver does
> indeed require this; but that's a side effect of the downstream
> ipcrouter implementation, not the problem itself.
>
APR does send messages in atomic context in addition to the RPM Driver, 
but IPC Router does not to the extent of my knowledge.
> Regards,
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Regards,
Karthik.
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
  hosted by The Linux Foundation

Powered by blists - more mailing lists