The busy loop in rpmh_rsc_send_data() is written with the assumption
that the udelay will be preempted by the tcs_tx_done() irq handler when
the TCS slots are all full. This doesn't hold true when the calling
thread is an irqthread and the tcs_tx_done() irq is also an irqthread.
That's because kernel irqthreads are SCHED_FIFO and thus need to
voluntarily give up priority by calling into the scheduler so that other
threads can run.
I see RCU stalls when I boot with irqthreads on the kernel commandline
because the modem remoteproc driver is trying to send an rpmh async
message from an irqthread that needs to give up the CPU for the rpmh
irqthread to run and clear out tcs slots.
rcu: INFO: rcu_preempt self-detected stall on CPU
rcu: 0-....: (1 GPs behind) idle=402/1/0x4000000000000002 softirq=2108/2109 fqs=4920
(t=21016 jiffies g=2933 q=590)
Task dump for CPU 0:
irq/11-smp2p R running task 0 148 2 0x00000028
Call trace:
dump_backtrace+0x0/0x154
show_stack+0x20/0x2c
sched_show_task+0xfc/0x108
dump_cpu_task+0x44/0x50
rcu_dump_cpu_stacks+0xa4/0xf8
rcu_sched_clock_irq+0x7dc/0xaa8
update_process_times+0x30/0x54
tick_sched_handle+0x50/0x64
tick_sched_timer+0x4c/0x8c
__hrtimer_run_queues+0x21c/0x36c
hrtimer_interrupt+0xf0/0x22c
arch_timer_handler_phys+0x40/0x50
handle_percpu_devid_irq+0x114/0x25c
__handle_domain_irq+0x84/0xc4
gic_handle_irq+0xd0/0x178
el1_irq+0xbc/0x180
save_return_addr+0x18/0x28
return_address+0x54/0x88
preempt_count_sub+0x40/0x88
_raw_spin_unlock_irqrestore+0x4c/0x6c
___ratelimit+0xd0/0x128
rpmh_rsc_send_data+0x24c/0x378
__rpmh_write+0x1b0/0x208
rpmh_write_async+0x90/0xbc
rpmhpd_send_corner+0x60/0x8c
rpmhpd_aggregate_corner+0x8c/0x124
rpmhpd_set_performance_state+0x8c/0xbc
_genpd_set_performance_state+0xdc/0x1b8
dev_pm_genpd_set_performance_state+0xb8/0xf8
q6v5_pds_disable+0x34/0x60 [qcom_q6v5_mss]
qcom_msa_handover+0x38/0x44 [qcom_q6v5_mss]
q6v5_handover_interrupt+0x24/0x3c [qcom_q6v5]
handle_nested_irq+0xd0/0x138
qcom_smp2p_intr+0x188/0x200
irq_thread_fn+0x2c/0x70
irq_thread+0xfc/0x14c
kthread+0x11c/0x12c
ret_from_fork+0x10/0x18
This busy loop naturally lends itself to using a wait queue so that each
thread that tries to send a message will sleep waiting on the waitqueue
and only be woken up when a free slot is available. This should make
things more predictable too because the scheduler will be able to sleep
tasks that are waiting on a free tcs instead of the busy loop we
currently have today.
Reviewed-by: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Maulik Shah <mkshah@codeaurora.org>
Cc: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20200724211711.810009-1-sboyd@kernel.org
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
It has been postulated that the pm_lock is bad for performance because
a CPU currently running rpmh_flush() could block other CPUs from
coming out of idle. Similarly CPUs coming out of / going into idle
all need to contend with each other for the spinlock just to update
the variable tracking who's in PM.
Let's optimize this a bit. Specifically:
- Use a count rather than a bitmask. This is faster to access and
also means we can use the atomic_inc_return() function to really
detect who the last one to enter PM was.
- Accept that it's OK if we race and are doing the flush (because we
think we're last) while another CPU is coming out of idle. As long
as we block that CPU if/when it tries to do an active-only transfer
we're OK.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20200504104917.v6.5.I295cb72bc5334a2af80313cbe97cb5c9dcb1442c@changeid
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
The rpmh-rsc code had both a driver-level lock (sometimes referred to
in comments as drv->lock) and a lock per-TCS. The idea was supposed
to be that there would be times where you could get by with just
locking a TCS lock and therefor other RPMH users wouldn't be blocked.
The above didn't work out so well.
Looking at tcs_write() the bigger drv->lock was held for most of the
function anyway. Only the __tcs_buffer_write() and
__tcs_set_trigger() calls were called without holding the drv->lock.
It actually turns out that in tcs_write() we don't need to hold the
drv->lock for those function calls anyway even if the per-TCS lock
isn't there anymore. From the newly added comments in the code, this
is because:
- We marked "tcs_in_use" under lock.
- Once "tcs_in_use" has been marked nobody else could be writing
to these registers until the interrupt goes off.
- The interrupt can't go off until we trigger w/ the last line
of __tcs_set_trigger().
Thus, from a tcs_write() point of view, the per-TCS lock was useless.
Looking at rpmh_rsc_write_ctrl_data(), only the per-TCS lock was held.
It turns out, though, that this function already needs to be called
with the equivalent of the drv->lock held anyway (we either need to
hold drv->lock as we will in a future patch or we need to know no
other CPUs could be running as happens today). Specifically
rpmh_rsc_write_ctrl_data() might be writing to a TCS that has been
borrowed for writing an active transation but it never checks this.
Let's eliminate this extra overhead and avoid possible AB BA locking
headaches.
Suggested-by: Maulik Shah <mkshah@codeaurora.org>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20200504104917.v6.4.Ib8dccfdb10bf6b1fb1d600ca1c21d9c0db1ef746@changeid
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Auditing tcs_invalidate() made me worried. Specifically I saw that it
used spin_lock(), not spin_lock_irqsave(). That always worries me
unless I can trace for sure that I'm in the interrupt handler or that
someone else already disabled interrupts.
Looking more at it, there is actually no reason for these locks
anyway. Specifically the only reason you'd ever call
rpmh_rsc_invalidate() is if you cared that the sleep/wake TCSes were
empty. That means that they need to continue to be empty even after
rpmh_rsc_invalidate() returns. The only way that can happen is if the
caller already has done something to keep all other RPMH users out.
It should be noted that even though the caller is only worried about
making sleep/wake TCSes empty, they also need to worry about stopping
active-only transfers if they need to handle the case where
active-only transfers might borrow the wake TCS.
At the moment rpmh_rsc_invalidate() is only called in PM code from the
last CPU. If that later changes the caller will still need to solve
the above problems themselves, so these locks will never be useful.
Continuing to audit tcs_invalidate(), I found a bug. The function
didn't properly check for a borrowed TCS if we hadn't recently written
anything into the TCS. Specifically, if we've never written to the
WAKE_TCS (or we've flushed it recently) then tcs->slots is empty.
We'll early-out and we'll never call tcs_is_free().
I thought about fixing this bug by either deleting the early check for
bitmap_empty() or possibly only doing it if we knew we weren't on a
TCS that could be borrowed. However, I think it's better to just
delete the checks.
As argued above it's up to the caller to make sure that all other
users of RPMH are quiet before tcs_invalidate() is called. Since
callers need to handle the zero-active-TCS case anyway that means they
need to make sure that the active-only transfers are quiet before
calling too. The one way tcs_invalidate() gets called today is
through rpmh_rsc_cpu_pm_callback() which calls
rpmh_rsc_ctrlr_is_busy() to handle this. When we have another path to
get to tcs_invalidate() it will also need to come up with something
similar and it won't need this extra check either. If we later find
some code path that actually needs this check back in (and somehow
manages to be race free) we can always add it back in.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Maulik Shah <mkshah@codeaurora.org>
Link: https://lore.kernel.org/r/20200413100321.v4.9.I07c1f70e0e8f2dc0004bd38970b4e258acdc773e@changeid
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
I've been pouring through the rpmh-rsc code and trying to understand
it. Document everything to the best of my ability. All documentation
here is strictly from code analysis--no actual knowledge of the
hardware was used. If something is wrong in here I either
misunderstood the code, had a typo, or the code has a bug in it
leading to my incorrect understanding.
In a few places here I have documented things that don't make tons of
sense. A future patch will try to address this. While this means I'm
adding comments / todos and then later fixing them in the series, it
seemed more urgent to get things documented first so that people could
understand the later patches.
Any comments I adjusted I also tried to make match kernel-doc better.
Specifically:
- kernel-doc says do not leave a blank line between the function
description and the arguments
- kernel-doc examples always have things starting w/ a capital and
ending with a period.
This should be a no-op. It's just comment changes.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20200413100321.v4.6.I52653eb85d7dc8981ee0dafcd0b6cc0f273e9425@changeid
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
The "cmd_cache" in RPMH wasn't terribly sensible. Specifically:
- The current code doesn't really detect "conflicts" properly any case
where the sequence being checked has more than one entry. One
simple way to see this in the current code is that if cmd[0].addr
isn't found then cmd[1].addr is never checked.
- The code attempted to use the "cmd_cache" to update an existing
message in a sleep/wake TCS with new data. The goal appeared to be
to update part of a TCS while leaving the rest of the TCS alone. We
never actually do this. We always fully invalidate and re-write
everything.
- If/when we try to optimize things to not fully invalidate / re-write
every time we update the TCSes we'll need to think it through very
carefully. Specifically requirement of find_match() that the new
sequence of addrs must match exactly the old sequence of addrs seems
inflexible. It's also not documented in rpmh_write() and
rpmh_write_batch(). In any case, if we do decide to require updates
to keep the exact same sequence and length then presumably the API
and data structures should be updated to understand groups more
properly. The current algorithm doesn't really keep track of the
length of the old sequence and there are several boundary-condition
bugs because of that. Said another way: if we decide to do
something like this in the future we should start from scratch and
thus find_match() isn't useful to keep around.
This patch isn't quite a no-op. Specifically:
- It should be a slight performance boost of not searching through so
many arrays.
- The old code would have done something useful in one case: it would
allow someone calling rpmh_write() to override the data that came
from rpmh_write_batch(). I don't believe that actually happens in
reality.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Tested-by: Maulik Shah <mkshah@codeaurora.org>
Link: https://lore.kernel.org/r/20200413100321.v4.5.I6d3d0a3ec810dc72ff1df3cbf97deefdcdeb8eef@changeid
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Add changes to invoke rpmh flush() from CPU PM notification.
This is done when the last the cpu is entering deep CPU idle
states and controller is not busy.
Controllers that have 'HW solver' mode like display RSC do not need
to register for CPU PM notification. They may be in autonomous mode
executing low power mode and do not require rpmh_flush() to happen
from CPU PM notification.
Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/1586703004-13674-5-git-send-email-mkshah@codeaurora.org
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
rpmh_flush() was exported with the idea that an external entity
operation during CPU idle would know when to flush the sleep and wake
TCS. Since, this is not the case when defining a power domain for the
RSC. Remove the function export and instead allow the function to be
called internally.
Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/1580736940-6985-3-git-send-email-mkshah@codeaurora.org
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Platform drivers need make a lot of resource state requests at the same
time, say, at the start or end of an usecase. It can be quite
inefficient to send each request separately. Instead they can give the
RPMH library a batch of requests to be sent and wait on the whole
transaction to be complete.
rpmh_write_batch() is a blocking call that can be used to send multiple
RPMH command sets. Each RPMH command set is set asynchronously and the
API blocks until all the command sets are complete and receive their
tx_done callbacks.
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Andy Gross <andy.gross@linaro.org>
Platform drivers that want to send a request but do not want to block
until the RPMH request completes have now a new API -
rpmh_write_async().
The API allocates memory and send the requests and returns the control
back to the platform driver. The tx_done callback from the controller is
handled in the context of the controller's thread and frees the
allocated memory. This API allows RPMH requests from atomic contexts as
well.
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
Signed-off-by: Andy Gross <andy.gross@linaro.org>
Active state requests are sent immediately to the RSC controller, while
sleep and wake state requests are cached in this driver to avoid taxing
the RSC controller repeatedly. The cached values will be sent to the
controller when the rpmh_flush() is called.
Generally, flushing is a system PM activity and may be called from the
system PM drivers when the system is entering suspend or deeper sleep
modes during cpuidle.
Also allow invalidating the cached requests, so they may be re-populated
again.
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
[rplsssn: remove unneeded semicolon, address line over 80chars error]
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
Reviewed-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Andy Gross <andy.gross@linaro.org>
Sleep and wake requests are sent when the application processor
subsystem of the SoC is entering deep sleep states like in suspend.
These requests help lower the system power requirements when the
resources are not in use.
Sleep and wake requests are written to the TCS slots but are not
triggered at the time of writing. The TCS are triggered by the firmware
after the last of the CPUs has executed its WFI. Since these requests
may come in different batches of requests, it is the job of this
controller driver to find and arrange the requests into the available
TCSes.
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
Reviewed-by: Evan Green <evgreen@chromium.org>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Andy Gross <andy.gross@linaro.org>
Sending RPMH requests and waiting for response from the controller
through a callback is common functionality across all platform drivers.
To simplify drivers, add a library functions to create RPMH client and
send resource state requests.
rpmh_write() is a synchronous blocking call that can be used to send
active state requests.
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
Signed-off-by: Andy Gross <andy.gross@linaro.org>
Add controller driver for QCOM SoCs that have hardware based shared
resource management. The hardware IP known as RSC (Resource State
Coordinator) houses multiple Direct Resource Voter (DRV) for different
execution levels. A DRV is a unique voter on the state of a shared
resource. A Trigger Control Set (TCS) is a bunch of slots that can house
multiple resource state requests, that when triggered will issue those
requests through an internal bus to the Resource Power Manager Hardened
(RPMH) blocks. These hardware blocks are capable of adjusting clocks,
voltages, etc. The resource state request from a DRV are aggregated
along with state requests from other processors in the SoC and the
aggregate value is applied on the resource.
Some important aspects of the RPMH communication -
- Requests are <addr, value> with some header information
- Multiple requests (upto 16) may be sent through a TCS, at a time
- Requests in a TCS are sent in sequence
- Requests may be fire-n-forget or completion (response expected)
- Multiple TCS from the same DRV may be triggered simultaneously
- Cannot send a request if another request for the same addr is in
progress from the same DRV
- When all the requests from a TCS are complete, an IRQ is raised
- The IRQ handler needs to clear the TCS before it is available for
reuse
- TCS configuration is specific to a DRV
- Platform drivers may use DRV from different RSCs to make requests
Resource state requests made when CPUs are active are called 'active'
state requests. Requests made when all the CPUs are powered down (idle
state) are called 'sleep' state requests. They are matched by a
corresponding 'wake' state requests which puts the resources back in to
previously requested active state before resuming any CPU. TCSes are
dedicated for each type of requests. Active mode TCSes (AMC) are used to
send requests immediately to the resource, while control TCS are used to
provide specific information to the controller. Sleep and Wake TCS send
sleep and wake requests, after and before the system halt respectively.
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org>
Signed-off-by: Andy Gross <andy.gross@linaro.org>
Stephen Boyd