lkml.org 
[lkml]   [2018]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v13 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
From
Date
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 7/26/2018 9:00 PM, Robin Murphy
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:401d8509-b9ad-913b-334e-f4ac853472e3@arm.com">On
26/07/18 08:12, Vivek Gautam wrote:
<br>
<blockquote type="cite">On Wed, Jul 25, 2018 at 11:46 PM, Vivek
Gautam
<br>
<a class="moz-txt-link-rfc2396E" href="mailto:vivek.gautam@codeaurora.org">&lt;vivek.gautam@codeaurora.org&gt;</a> wrote:
<br>
<blockquote type="cite">On Tue, Jul 24, 2018 at 8:51 PM, Robin
Murphy <a class="moz-txt-link-rfc2396E" href="mailto:robin.murphy@arm.com">&lt;robin.murphy@arm.com&gt;</a> wrote:
<br>
<blockquote type="cite">On 19/07/18 11:15, Vivek Gautam wrote:
<br>
<blockquote type="cite">
<br>
From: Sricharan R <a class="moz-txt-link-rfc2396E" href="mailto:sricharan@codeaurora.org">&lt;sricharan@codeaurora.org&gt;</a>
<br>
<br>
The smmu needs to be functional only when the respective
<br>
master's using it are active. The device_link feature
<br>
helps to track such functional dependencies, so that the
<br>
iommu gets powered when the master device enables itself
<br>
using pm_runtime. So by adapting the smmu driver for
<br>
runtime pm, above said dependency can be addressed.
<br>
<br>
This patch adds the pm runtime/sleep callbacks to the
<br>
driver and also the functions to parse the smmu clocks
<br>
from DT and enable them in resume/suspend.
<br>
<br>
Also, while we enable the runtime pm add a pm sleep
suspend
<br>
callback that pushes devices to low power state by turning
<br>
the clocks off in a system sleep.
<br>
Also add corresponding clock enable path in resume
callback.
<br>
<br>
Signed-off-by: Sricharan R
<a class="moz-txt-link-rfc2396E" href="mailto:sricharan@codeaurora.org">&lt;sricharan@codeaurora.org&gt;</a>
<br>
Signed-off-by: Archit Taneja
<a class="moz-txt-link-rfc2396E" href="mailto:architt@codeaurora.org">&lt;architt@codeaurora.org&gt;</a>
<br>
[vivek: rework for clock and pm ops]
<br>
Signed-off-by: Vivek Gautam
<a class="moz-txt-link-rfc2396E" href="mailto:vivek.gautam@codeaurora.org">&lt;vivek.gautam@codeaurora.org&gt;</a>
<br>
Reviewed-by: Tomasz Figa <a class="moz-txt-link-rfc2396E" href="mailto:tfiga@chromium.org">&lt;tfiga@chromium.org&gt;</a>
<br>
---
<br>
<br>
Changes since v12:
<br>
   - Added pm sleep .suspend callback. This disables the
clocks.
<br>
   - Added corresponding change to enable clocks in
.resume
<br>
    pm sleep callback.
<br>
<br>
   drivers/iommu/arm-smmu.c | 75
<br>
++++++++++++++++++++++++++++++++++++++++++++++--
<br>
   1 file changed, 73 insertions(+), 2 deletions(-)
<br>
<br>
diff --git a/drivers/iommu/arm-smmu.c
b/drivers/iommu/arm-smmu.c
<br>
index c73cfce1ccc0..9138a6fffe04 100644
<br>
--- a/drivers/iommu/arm-smmu.c
<br>
+++ b/drivers/iommu/arm-smmu.c
<br>
</blockquote>
</blockquote>
</blockquote>
<br>
[snip]
<br>
<br>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">platform_device *pdev)
<br>
         arm_smmu_device_remove(pdev);
<br>
   }
<br>
   +static int __maybe_unused
arm_smmu_runtime_resume(struct device *dev)
<br>
+{
<br>
+       struct arm_smmu_device *smmu =
dev_get_drvdata(dev);
<br>
+
<br>
+       return clk_bulk_enable(smmu-&gt;num_clks,
smmu-&gt;clks);
<br>
</blockquote>
<br>
<br>
If there's a power domain being automatically switched by
genpd then we need
<br>
a reset here because we may have lost state entirely. Since
I remembered the
<br>
otherwise-useless GPU SMMU on Juno is in a separate power
domain, I gave it
<br>
a poking via sysfs with some debug stuff to dump sCR0 in
these callbacks,
<br>
and the problem is clear:
<br>
<br>
...
<br>
[    4.625551] arm-smmu 2b400000.iommu:
genpd_runtime_suspend()
<br>
[    4.631163] arm-smmu 2b400000.iommu:
arm_smmu_runtime_suspend: 0x00201936
<br>
[    4.637897] arm-smmu 2b400000.iommu: suspend latency
exceeded, 6733980 ns
<br>
[   21.566983] arm-smmu 2b400000.iommu:
genpd_runtime_resume()
<br>
[   21.584796] arm-smmu 2b400000.iommu:
arm_smmu_runtime_resume: 0x00220101
<br>
[   21.591452] arm-smmu 2b400000.iommu: resume latency
exceeded, 6658020 ns
<br>
...
<br>
</blockquote>
<br>
Qualcomm SoCs have retention enabled for SMMU registers so
they don't
<br>
lose state.
<br>
...
<br>
[  256.013367] arm-smmu b40000.arm,smmu:
arm_smmu_runtime_suspend
<br>
SCR0 = 0x201e36
<br>
[  256.013367]
<br>
[  256.019160] arm-smmu b40000.arm,smmu:
arm_smmu_runtime_resume
<br>
SCR0 = 0x201e36
<br>
[  256.019160]
<br>
[  256.027368] arm-smmu b40000.arm,smmu:
arm_smmu_runtime_suspend
<br>
SCR0 = 0x201e36
<br>
[  256.027368]
<br>
[  256.036786] arm-smmu b40000.arm,smmu:
arm_smmu_runtime_resume
<br>
SCR0 = 0x201e36
<br>
...
<br>
<br>
However after adding arm_smmu_device_reset() in
runtime_resume() I observe
<br>
some performance degradation when kill an instance of
'kmscube' and
<br>
start it again.
<br>
The launch time with arm_smmu_device_reset() in
runtime_resume() change is
<br>
more.
<br>
Could this be because of frequent TLB invalidation and sync?
<br>
</blockquote>
</blockquote>
<br>
Probably. Plus the reset procedure is a big chunk of MMIO
accesses, which for a non-trivial SMMU configuration probably
isn't negligible in itself. Unfortunately, unless you know for
absolute certain that you don't need to do that, you do.
<br>
<br>
<blockquote type="cite">Some more information that i gathered.
<br>
On Qcom SoCs besides the registers retention, TCU invalidates
TLB cache on
<br>
a CX power collapse exit, which is the system wide suspend case.
<br>
The arm-smmu software is not aware of this CX power collapse /
<br>
auto-invalidation.
<br>
<br>
So wouldn't doing an explicit TLB invalidations during runtime
resume be
<br>
detrimental to performance?
<br>
</blockquote>
<br>
Indeed it would be, but resuming with TLBs full of random
valid-looking junk is even more so.
<br>
<br>
<blockquote type="cite">I have one more doubt here -
<br>
We do runtime power cycle around arm_smmu_map/unmap() too.
<br>
Now during map/unmap we selectively do TLB maintenance (either
<br>
tlb_sync or tlb_add_flush).
<br>
But with runtime pm we want to do TLBIALL*. Is that a problem?
<br>
</blockquote>
<br>
It's technically redundant to do both, true, but as we've covered
in previous rounds of discussion it's very difficult to know
*which* one is sufficient at any given time, so in order to make
progress for now I think we have to settle with doing both.
<br>
</blockquote>
<br>
Thanks Robin. I will respin the patches as Tomasz also suggested;<span
style="color: rgb(34, 34, 34); font-family: arial, sans-serif;
font-size: 12.8px; font-style: normal; font-variant-ligatures:
normal; font-variant-caps: normal; font-weight: 400;
letter-spacing: normal; orphans: 2; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial; display: inline
!important; float: none;"><br>
<br>
arm_smmu_runtime_resume() will look like:</span><br style="color:
rgb(34, 34, 34); font-family: arial, sans-serif; font-size:
12.8px; font-style: normal; font-variant-ligatures: normal;
font-variant-caps: normal; font-weight: 400; letter-spacing:
normal; orphans: 2; text-align: start; text-indent: 0px;
text-transform: none; white-space: normal; widows: 2;
word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial;">
<br style="color: rgb(34, 34, 34); font-family: arial, sans-serif;
font-size: 12.8px; font-style: normal; font-variant-ligatures:
normal; font-variant-caps: normal; font-weight: 400;
letter-spacing: normal; orphans: 2; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial;">
<span style="color: rgb(34, 34, 34); font-family: arial, sans-serif;
font-size: 12.8px; font-style: normal; font-variant-ligatures:
normal; font-variant-caps: normal; font-weight: 400;
letter-spacing: normal; orphans: 2; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial; display: inline
!important; float: none;">    if (pm_runtime_suspended(dev))</span><br
style="color: rgb(34, 34, 34); font-family: arial, sans-serif;
font-size: 12.8px; font-style: normal; font-variant-ligatures:
normal; font-variant-caps: normal; font-weight: 400;
letter-spacing: normal; orphans: 2; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial;">
<span style="color: rgb(34, 34, 34); font-family: arial, sans-serif;
font-size: 12.8px; font-style: normal; font-variant-ligatures:
normal; font-variant-caps: normal; font-weight: 400;
letter-spacing: normal; orphans: 2; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial; display: inline
!important; float: none;">        return 0;</span><br
style="color: rgb(34, 34, 34); font-family: arial, sans-serif;
font-size: 12.8px; font-style: normal; font-variant-ligatures:
normal; font-variant-caps: normal; font-weight: 400;
letter-spacing: normal; orphans: 2; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial;">
<span style="color: rgb(34, 34, 34); font-family: arial, sans-serif;
font-size: 12.8px; font-style: normal; font-variant-ligatures:
normal; font-variant-caps: normal; font-weight: 400;
letter-spacing: normal; orphans: 2; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial; display: inline
!important; float: none;">    return arm_smmu_runtime_resume(dev);<br>
<br>
and,<br>
</span><span style="color: rgb(34, 34, 34); font-family: arial,
sans-serif; font-size: 12.8px; font-style: normal;
font-variant-ligatures: normal; font-variant-caps: normal;
font-weight: 400; letter-spacing: normal; orphans: 2; text-align:
start; text-indent: 0px; text-transform: none; white-space:
normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width:
0px; background-color: rgb(255, 255, 255); text-decoration-style:
initial; text-decoration-color: initial; display: inline
!important; float: none;"><span style="color: rgb(34, 34, 34);
font-family: arial, sans-serif; font-size: 12.8px; font-style:
normal; font-variant-ligatures: normal; font-variant-caps:
normal; font-weight: 400; letter-spacing: normal; orphans: 2;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; widows: 2; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color: rgb(255, 255,
255); text-decoration-style: initial; text-decoration-color:
initial; display: inline !important; float: none;">arm_smmu_runtime_resume()
will have arm_smmu_device_reset().<br>
</span></span><br>
Best regards<br>
Vivek<br>
<blockquote type="cite"
cite="mid:401d8509-b9ad-913b-334e-f4ac853472e3@arm.com">
<br>
Robin.
<br>
--
<br>
To unsubscribe from this list: send the line "unsubscribe
linux-arm-msm" in
<br>
the body of a message to <a class="moz-txt-link-abbreviated" href="mailto:majordomo@vger.kernel.org">majordomo@vger.kernel.org</a>
<br>
More majordomo info at  <a class="moz-txt-link-freetext" href="http://vger.kernel.org/majordomo-info.html">http://vger.kernel.org/majordomo-info.html</a>
<br>
</blockquote>
<br>
</body>
</html>
\
 
 \ /
  Last update: 2018-07-27 07:07    [W:0.071 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site