LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
To: Will Deacon <will@kernel.org>
Cc: Robin Murphy <robin.murphy@arm.com>,
	Joerg Roedel <joro@8bytes.org>,
	Rajendra Nayak <rnayak@codeaurora.org>,
	Taniya Das <tdas@codeaurora.org>, srimuc <srimuc@codeaurora.org>,
	iommu@lists.linux-foundation.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	robdclark@chromium.org
Subject: Re: [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks
Date: Tue, 03 Aug 2021 11:36:12 +0530	[thread overview]
Message-ID: <5aefcc8950ec8ced0a67815c92e985df@codeaurora.org> (raw)
In-Reply-To: <20210802161206.GA29168@willie-the-truck>

On 2021-08-02 21:42, Will Deacon wrote:
> On Tue, Jul 27, 2021 at 03:03:22PM +0530, Sai Prakash Ranjan wrote:
>> Some clocks for SMMU can have parent as XO such as 
>> gpu_cc_hub_cx_int_clk
>> of GPU SMMU in QTI SC7280 SoC and in order to enter deep sleep states 
>> in
>> such cases, we would need to drop the XO clock vote in unprepare call 
>> and
>> this unprepare callback for XO is in RPMh (Resource Power 
>> Manager-Hardened)
>> clock driver which controls RPMh managed clock resources for new QTI 
>> SoCs
>> and is a blocking call.
>> 
>> Given we cannot have a sleeping calls such as clk_bulk_prepare() and
>> clk_bulk_unprepare() in arm-smmu runtime pm callbacks since the iommu
>> operations like map and unmap can be in atomic context and are in fast
>> path, add this prepare and unprepare call to drop the XO vote only for
>> system pm callbacks since it is not a fast path and we expect the 
>> system
>> to enter deep sleep states with system pm as opposed to runtime pm.
>> 
>> This is a similar sequence of clock requests (prepare,enable and
>> disable,unprepare) in arm-smmu probe and remove.
>> 
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
>> Co-developed-by: Rajendra Nayak <rnayak@codeaurora.org>
>> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>> ---
>>  drivers/iommu/arm/arm-smmu/arm-smmu.c | 20 ++++++++++++++++++--
>>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> [+Rob]
> 
> How does this work with that funny GPU which writes to the SMMU 
> registers
> directly? Does the SMMU need to remain independently clocked for that 
> to
> work or is it all in the same clock domain?
> 

As Rob mentioned, device link should take care of all the dependencies 
between
SMMU and its consumers. But not sure how the question relates to this 
patch as this
change is for system pm and not runtime pm, so it is exactly the 
sequence of
SMMU probe/remove which if works currently for that GPU SMMU, then it 
should work
just fine for system suspend and resume as well.

>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c 
>> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> index d3c6f54110a5..9561ba4c5d39 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> @@ -2277,6 +2277,13 @@ static int __maybe_unused 
>> arm_smmu_runtime_suspend(struct device *dev)
>> 
>>  static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
>>  {
>> +	int ret;
>> +	struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>> +
>> +	ret = clk_bulk_prepare(smmu->num_clks, smmu->clks);
>> +	if (ret)
>> +		return ret;
>> +
>>  	if (pm_runtime_suspended(dev))
>>  		return 0;
> 
> If we subsequently fail to enable the clks in arm_smmu_runtime_resume()
> should we unprepare them again?
> 

If we are unable to turn on the clks then its fatal and we will not live 
for long.

Thanks,
Sai

> Will
> 
>> @@ -2285,10 +2292,19 @@ static int __maybe_unused 
>> arm_smmu_pm_resume(struct device *dev)
>> 
>>  static int __maybe_unused arm_smmu_pm_suspend(struct device *dev)
>>  {
>> +	int ret = 0;
>> +	struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>> +
>>  	if (pm_runtime_suspended(dev))
>> -		return 0;
>> +		goto clk_unprepare;
>> 
>> -	return arm_smmu_runtime_suspend(dev);
>> +	ret = arm_smmu_runtime_suspend(dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +clk_unprepare:
>> +	clk_bulk_unprepare(smmu->num_clks, smmu->clks);
>> +	return ret;
>>  }
>> 
>>  static const struct dev_pm_ops arm_smmu_pm_ops = {
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
>> member
>> of Code Aurora Forum, hosted by The Linux Foundation
>> 

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation

  parent reply	other threads:[~2021-08-03  6:06 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27  9:33 Sai Prakash Ranjan
2021-07-27 10:25 ` Robin Murphy
2021-07-27 10:33   ` Robin Murphy
2021-07-27 10:35     ` Sai Prakash Ranjan
2021-08-02 16:12 ` Will Deacon
2021-08-03  1:14   ` Rob Clark
2021-08-03  6:06   ` Sai Prakash Ranjan [this message]
2021-08-10  6:51     ` Sai Prakash Ranjan

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=5aefcc8950ec8ced0a67815c92e985df@codeaurora.org \
    --to=saiprakash.ranjan@codeaurora.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rnayak@codeaurora.org \
    --cc=robdclark@chromium.org \
    --cc=robin.murphy@arm.com \
    --cc=srimuc@codeaurora.org \
    --cc=tdas@codeaurora.org \
    --cc=will@kernel.org \
    --subject='Re: [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks' \
    /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

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).