From: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
Cc: Vivek Gautam
<vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
"list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS"
<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>,
Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
"Rafael J. Wysocki" <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>,
Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>,
Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dri-devel
<dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
David Airlie <airlied-cv59FeDIM0c@public.gmane.org>,
Greg KH
<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v7 3/6] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
Date: Tue, 13 Feb 2018 22:52:53 +0900 [thread overview]
Message-ID: <CAAFQd5DJtQYPg5S3Ep2bK27+D5rQiKuA-uPfMDUon3FudmGF0w@mail.gmail.com> (raw)
In-Reply-To: <906051dd-8898-ec6f-5ad4-3f37716292cf-5wv7dgnIgG8@public.gmane.org>
On Tue, Feb 13, 2018 at 9:57 PM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote:
> On 13/02/18 08:24, Tomasz Figa wrote:
>>
>> Hi Vivek,
>>
>> Thanks for the patch. Please see my comments inline.
>>
>> On Wed, Feb 7, 2018 at 7:31 PM, Vivek Gautam
>> <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>>>
>>> From: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>>
>>> The smmu device probe/remove and add/remove master device callbacks
>>> gets called when the smmu is not linked to its master, that is without
>>> the context of the master device. So calling runtime apis in those places
>>> separately.
>>>
>>> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> [vivek: Cleanup pm runtime calls]
>>> Signed-off-by: Vivek Gautam <vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> ---
>>> drivers/iommu/arm-smmu.c | 42
>>> ++++++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 38 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>> index 9e2f917e16c2..c024f69c1682 100644
>>> --- a/drivers/iommu/arm-smmu.c
>>> +++ b/drivers/iommu/arm-smmu.c
>>> @@ -913,11 +913,15 @@ static void arm_smmu_destroy_domain_context(struct
>>> iommu_domain *domain)
>>> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>>> struct arm_smmu_device *smmu = smmu_domain->smmu;
>>> struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
>>> - int irq;
>>> + int ret, irq;
>>>
>>> if (!smmu || domain->type == IOMMU_DOMAIN_IDENTITY)
>>> return;
>>>
>>> + ret = pm_runtime_get_sync(smmu->dev);
>>> + if (ret)
>>> + return;
>>
>>
>> pm_runtime_get_sync() will return 0 if the device was powered off, 1
>> if it was already/still powered on or runtime PM is not compiled in,
>> or a negative value on error, so shouldn't the test be (ret < 0)?
>>
>> Moreover, I'm actually wondering if it makes any sense to power up the
>> hardware just to program it and power it down again. In a system where
>> the IOMMU is located within a power domain, it would cause the IOMMU
>> block to lose its state anyway.
>
>
> This is generally for the case where the SMMU internal state remains active,
> but the programming interface needs to be powered up in order to access it.
That's true for Qualcomm SMMU, but I think that would be different for
existing users of the driver?
>
>> Actually, reflecting back on "[PATCH v7 2/6] iommu/arm-smmu: Add
>> pm_runtime/sleep ops", perhaps it would make more sense to just
>> control the clocks independently of runtime PM? Then, runtime PM could
>> be used for real power management, e.g. really powering the block up
>> and down, for further power saving.
>
>
> Unfortunately that ends up pretty much unmanageable, because there are
> numerous different SMMU microarchitectures with fundamentally different
> clock/power domain schemes (multiplied by individual SoC integration
> possibilities). Since this is fundamentally a generic architectural driver,
> adding explicit clock support would probably make the whole thing about 50%
> clock code, with complicated decision trees around every hardware access
> calculating which clocks are necessary for a given operation on a given
> system. That maintainability aspect is why we've already nacked such a
> fine-grained approach in the past.
Hmm, I think we are talking about different things here. My suggestion
would not add much more code to the driver than this patch does, calls
to arm_smmu_enable_clocks() instead of pm_runtime_get_sync() and
arm_smmu_disable_clocks() instead of pm_runtime_put(). The
implementation of both functions would be a simple call to clk_bulk_
API (possibly even no need to put this into functions, just call
directly).
Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2018-02-13 13:52 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-07 10:31 [PATCH v7 0/6] iommu/arm-smmu: Add runtime pm/sleep support Vivek Gautam
[not found] ` <1517999482-17317-1-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-02-07 10:31 ` [PATCH v7 1/6] base: power: runtime: Export pm_runtime_get/put_suppliers Vivek Gautam
2018-02-13 7:44 ` Tomasz Figa
[not found] ` <CAAFQd5BmroRf-C8dQkvTKHWK1psGnNi1t7g-q=Xce6KjrGTsdw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-13 12:00 ` Robin Murphy
2018-02-13 12:54 ` Tomasz Figa
2018-02-13 13:37 ` Robin Murphy
2018-02-07 10:31 ` [PATCH v7 2/6] iommu/arm-smmu: Add pm_runtime/sleep ops Vivek Gautam
2018-02-13 8:03 ` Tomasz Figa
[not found] ` <CAAFQd5B2u8RL-tdB3qgPxVUcXnsBSEhXRBZWxqO-w6rYKAiOtg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-13 10:25 ` Vivek Gautam
2018-02-14 3:45 ` Tomasz Figa
2018-02-07 10:31 ` [PATCH v7 3/6] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device Vivek Gautam
2018-02-13 8:24 ` Tomasz Figa
[not found] ` <CAAFQd5DxYhkK61VDAesby6bT+FtG2nqsbHQRvxkhrsSS0KWtog-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-13 12:57 ` Robin Murphy
[not found] ` <906051dd-8898-ec6f-5ad4-3f37716292cf-5wv7dgnIgG8@public.gmane.org>
2018-02-13 13:52 ` Tomasz Figa [this message]
[not found] ` <CAAFQd5DJtQYPg5S3Ep2bK27+D5rQiKuA-uPfMDUon3FudmGF0w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 8:24 ` Vivek Gautam
2018-02-14 8:28 ` Vivek Gautam
[not found] ` <1517999482-17317-4-git-send-email-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-02-22 23:52 ` Jordan Crouse
[not found] ` <20180222235200.GA18743-9PYrDHPZ2Orvke4nUoYGnHL1okKdlPRT@public.gmane.org>
2018-02-23 10:36 ` Vivek Gautam
[not found] ` <CAFp+6iGQ5Vckui14Jb=V0uk_Pjes95hOxo=KBijR4yxPeDDzFg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-23 15:40 ` [Freedreno] " Jordan Crouse
[not found] ` <20180223154048.GB18743-9PYrDHPZ2Orvke4nUoYGnHL1okKdlPRT@public.gmane.org>
2018-02-23 17:43 ` Vivek Gautam
2018-02-07 10:31 ` [PATCH v7 4/6] iommu/arm-smmu: Add the device_link between masters and smmu Vivek Gautam
2018-02-13 8:31 ` Tomasz Figa
[not found] ` <CAAFQd5BQAs9=N27_Z0pJNSrndFY3vFin2KBP44UtiW+tMXy5nQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-13 10:14 ` Vivek Gautam
2018-02-07 10:31 ` [PATCH v7 5/6] iommu/arm-smmu: Add support for qcom, smmu-v2 variant Vivek Gautam
2018-02-09 10:57 ` [PATCH v8 5/6] iommu/arm-smmu: Add support for qcom,smmu-v2 variant Vivek Gautam
2018-02-13 8:57 ` Tomasz Figa
2018-02-07 10:31 ` [PATCH v7 6/6] drm/msm: iommu: Replace runtime calls with runtime suppliers Vivek Gautam
2018-02-13 9:10 ` Tomasz Figa
[not found] ` <CAAFQd5AjopiX6fDgD+mO+-+d0yj-swEnVCNvccWRBSMO+XVJkA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-13 16:42 ` Jordan Crouse
2018-02-14 3:31 ` Tomasz Figa
[not found] ` <CAAFQd5CjQRFATfh-mRQv5J=WefYuxBVTkk=Ju09FoqA-Or5Cvg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 15:48 ` Jordan Crouse
[not found] ` <20180214154850.GA25422-9PYrDHPZ2Orvke4nUoYGnHL1okKdlPRT@public.gmane.org>
2018-02-14 16:12 ` [Freedreno] " Rob Clark
2018-02-15 4:09 ` Tomasz Figa
[not found] ` <CAAFQd5C-9mbd3hDSvz10a1oiO0--FT-L4EpsAYcALxxUvk6Fjg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-15 14:14 ` Rob Clark
2018-02-13 18:03 ` Rob Clark
2018-02-14 1:59 ` Tomasz Figa
[not found] ` <CAAFQd5BKRumpEfAKNF_RKS-ZZ8D671DfOz4vB2+w1SV3aG9NxQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 2:13 ` Rob Clark
[not found] ` <CAF6AEGuNZJKtwGZ5mLfqNND2jtU+HYM11UONfAtVTzoM0QVpdg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 3:01 ` Tomasz Figa
[not found] ` <CAAFQd5BZJ1G0RG32hYErNzPRvisBhhiSNCBsjbzfm0WzO=DnsQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 4:17 ` Vivek Gautam
[not found] ` <CAFp+6iHaycK=CcE1S15EeuMkaw8LnW0ebptU0hM6tUtWdeEOtA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 5:38 ` Tomasz Figa
[not found] ` <CAAFQd5Afj-Bj+3wHwmF2tT7y=46EsYEtO_mXfY6stXBgHutEUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 9:13 ` Vivek Gautam
[not found] ` <CAFp+6iGX6pr+MdPSSHHG=qOnhHky_8OHiDqAcJ9UudEUv=JMHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 9:16 ` Tomasz Figa
[not found] ` <CAAFQd5DiwAugGnPOTw0+XrEfef9x-n-vx59JFuXpNawjiXHwCw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 10:33 ` Vivek Gautam
[not found] ` <CAFp+6iEW0faeHDfzN_F1bRrHGcVo3sPCk4HSY=t9dnEvHkDkYw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-14 16:03 ` Robin Murphy
2018-02-15 3:17 ` Tomasz Figa
[not found] ` <CAAFQd5AmG1zSm+CouXOCJbs8SNGFk1-RqfU1nWGjMGJMB-qfvw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-15 4:17 ` Tomasz Figa
[not found] ` <CAAFQd5A9B-di9svtiJbvk2hz1U1xo61rTY5vt6AD+KR5iMcG-A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-15 17:14 ` Robin Murphy
[not found] ` <7406f1ce-c2c9-a6bd-2886-5a34de45add6-5wv7dgnIgG8@public.gmane.org>
2018-02-16 0:13 ` Tomasz Figa
[not found] ` <CAAFQd5DkLtq2w00=Zd4sMDB4QOWqi7R-zgydECJXLdTmaHty+g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-22 8:13 ` Tomasz Figa
[not found] ` <CAAFQd5CQoDqunAunwoVo7W=QXa=ET=eJ2s_j9j+3YgAR2EGgCg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-22 13:30 ` Rob Clark
2018-02-22 13:45 ` Robin Murphy
2018-02-22 14:12 ` Tomasz Figa
[not found] ` <CAAFQd5CzOJ0oS8LLcKP-DgyXSXgzafg5CWPAvnR-QDPS+DZAUg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-02-22 17:24 ` Vivek Gautam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAAFQd5DJtQYPg5S3Ep2bK27+D5rQiKuA-uPfMDUon3FudmGF0w@mail.gmail.com \
--to=tfiga-f7+t8e8rja9g9huczpvpmw@public.gmane.org \
--cc=airlied-cv59FeDIM0c@public.gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
--cc=freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
--cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
--cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
--cc=joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org \
--cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
--cc=rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org \
--cc=robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=robin.murphy-5wv7dgnIgG8@public.gmane.org \
--cc=sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
--cc=vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
--cc=will.deacon-5wv7dgnIgG8@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).