* [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks @ 2021-07-27 9:33 Sai Prakash Ranjan 2021-07-27 10:25 ` Robin Murphy 2021-08-02 16:12 ` Will Deacon 0 siblings, 2 replies; 8+ messages in thread From: Sai Prakash Ranjan @ 2021-07-27 9:33 UTC (permalink / raw) To: Will Deacon, Robin Murphy, Joerg Roedel, Rajendra Nayak, Taniya Das, srimuc Cc: iommu, linux-arm-kernel, linux-kernel, linux-arm-msm, Sai Prakash Ranjan 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(-) 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; @@ -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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks 2021-07-27 9:33 [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks Sai Prakash Ranjan @ 2021-07-27 10:25 ` Robin Murphy 2021-07-27 10:33 ` Robin Murphy 2021-08-02 16:12 ` Will Deacon 1 sibling, 1 reply; 8+ messages in thread From: Robin Murphy @ 2021-07-27 10:25 UTC (permalink / raw) To: Sai Prakash Ranjan, Will Deacon, Joerg Roedel, Rajendra Nayak, Taniya Das, srimuc Cc: linux-arm-msm, iommu, linux-kernel, linux-arm-kernel On 2021-07-27 10:33, 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. Nope. We call arm_smmu_rpm_get(), which may resume the device, from atomic contexts. clk_prepare() may sleep. This doesn't work. Robin. > 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(-) > > 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; > > @@ -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 = { > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks 2021-07-27 10:25 ` Robin Murphy @ 2021-07-27 10:33 ` Robin Murphy 2021-07-27 10:35 ` Sai Prakash Ranjan 0 siblings, 1 reply; 8+ messages in thread From: Robin Murphy @ 2021-07-27 10:33 UTC (permalink / raw) To: Sai Prakash Ranjan, Will Deacon, Joerg Roedel, Rajendra Nayak, Taniya Das, srimuc Cc: linux-arm-msm, iommu, linux-kernel, linux-arm-kernel On 2021-07-27 11:25, Robin Murphy wrote: > On 2021-07-27 10:33, 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. > > Nope. We call arm_smmu_rpm_get(), which may resume the device, from > atomic contexts. clk_prepare() may sleep. This doesn't work. Urgh, or maybe I skimmed the commit message too lightly *and* managed to totally misread the patch, sorry :( I'll wake up some more and try again later... Robin. >> 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(-) >> >> 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; >> @@ -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 = { >> > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks 2021-07-27 10:33 ` Robin Murphy @ 2021-07-27 10:35 ` Sai Prakash Ranjan 0 siblings, 0 replies; 8+ messages in thread From: Sai Prakash Ranjan @ 2021-07-27 10:35 UTC (permalink / raw) To: Robin Murphy Cc: Will Deacon, Joerg Roedel, Rajendra Nayak, Taniya Das, srimuc, linux-arm-msm, iommu, linux-kernel, linux-arm-kernel Hi Robin, On 2021-07-27 16:03, Robin Murphy wrote: > On 2021-07-27 11:25, Robin Murphy wrote: >> On 2021-07-27 10:33, 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. >> >> Nope. We call arm_smmu_rpm_get(), which may resume the device, from >> atomic contexts. clk_prepare() may sleep. This doesn't work. > > Urgh, or maybe I skimmed the commit message too lightly *and* managed > to totally misread the patch, sorry :( > > I'll wake up some more and try again later... > No worries, we took our time looking through that many times before posting this :) Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks 2021-07-27 9:33 [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks Sai Prakash Ranjan 2021-07-27 10:25 ` Robin Murphy @ 2021-08-02 16:12 ` Will Deacon 2021-08-03 1:14 ` Rob Clark 2021-08-03 6:06 ` Sai Prakash Ranjan 1 sibling, 2 replies; 8+ messages in thread From: Will Deacon @ 2021-08-02 16:12 UTC (permalink / raw) To: Sai Prakash Ranjan Cc: Robin Murphy, Joerg Roedel, Rajendra Nayak, Taniya Das, srimuc, iommu, linux-arm-kernel, linux-kernel, linux-arm-msm, robdclark 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? > 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? 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks 2021-08-02 16:12 ` Will Deacon @ 2021-08-03 1:14 ` Rob Clark 2021-08-03 6:06 ` Sai Prakash Ranjan 1 sibling, 0 replies; 8+ messages in thread From: Rob Clark @ 2021-08-03 1:14 UTC (permalink / raw) To: Will Deacon Cc: Sai Prakash Ranjan, Taniya Das, Rob Clark, Rajendra Nayak, linux-arm-msm, Linux Kernel Mailing List, list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>,, Robin Murphy, srimuc, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On Mon, Aug 2, 2021 at 9:12 AM Will Deacon <will@kernel.org> 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? AFAIU the device_link stuff should keep the SMMU clocked as long as the GPU is alive, so I think this should work out ok.. ie. the SMMU won't suspend while the GPU is not suspended. BR, -R > > 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? > > 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 > > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks 2021-08-02 16:12 ` Will Deacon 2021-08-03 1:14 ` Rob Clark @ 2021-08-03 6:06 ` Sai Prakash Ranjan 2021-08-10 6:51 ` Sai Prakash Ranjan 1 sibling, 1 reply; 8+ messages in thread From: Sai Prakash Ranjan @ 2021-08-03 6:06 UTC (permalink / raw) To: Will Deacon Cc: Robin Murphy, Joerg Roedel, Rajendra Nayak, Taniya Das, srimuc, iommu, linux-arm-kernel, linux-kernel, linux-arm-msm, robdclark 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks 2021-08-03 6:06 ` Sai Prakash Ranjan @ 2021-08-10 6:51 ` Sai Prakash Ranjan 0 siblings, 0 replies; 8+ messages in thread From: Sai Prakash Ranjan @ 2021-08-10 6:51 UTC (permalink / raw) To: Will Deacon Cc: Robin Murphy, Joerg Roedel, Rajendra Nayak, Taniya Das, srimuc, iommu, linux-arm-kernel, linux-kernel, linux-arm-msm, robdclark On 2021-08-03 11:36, Sai Prakash Ranjan wrote: > 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. > Nonetheless, it won't hurt to unprepare if clk enable fails as that is the correct thing anyway, so I have added it and sent a v2. 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-08-10 6:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-27 9:33 [PATCH] iommu/arm-smmu: Add clk_bulk_{prepare/unprepare} to system pm callbacks 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 2021-08-10 6:51 ` Sai Prakash Ranjan
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).