Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
On Mon, Jul 23, 2018 at 7:59 AM, Marek Szyprowski wrote: > Hi Rafael, > > On 2018-07-11 22:36, Rafael J. Wysocki wrote: >> On Wed, Jul 11, 2018 at 3:40 PM, Marek Szyprowski >> wrote: [cut] >>> Frankly, if there are no other reasons I suggest to wire system >>> suspend/resume to pm_runtime_force_* helpers: >>> SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, >>> pm_runtime_force_resume). >> Not a good idea at all IMO. >> >> Use PM driver flags rather I'd say. > > Frankly, till now I wasn't aware of the DPM_FLAG_* in struct dev_pm_info > 'driver_flags'. They are a relatively recent addition. > I've briefly checked them but I don't see the equivalent > of using SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, > pm_runtime_force_resume): keep device suspend if it was runtime suspended > AND really call pm_runtime_suspend if it was not runtime suspended on > system suspend. DPM_FLAG_SMART_SUSPEND is expected to cause that to happen. If you want the device to remain in suspend after the system-wide resume from sleep (if possible), you can set DPM_FLAG_LEAVE_SUSPENDED for it too. Currently a caveat is that genpd still doesn't support the flags. I have patches for that, but I haven't got to posting them yet. If you are interested, I can push this work forward relatively quickly, so please let me know. >>> This way you will have everything related to suspending and resuming in >>> one place and you would not need to bother about all possible cases (like >>> suspending from runtime pm active and suspending from runtime pm suspended >>> cases as well as restoring proper device state on resume). This is >>> especially important in recent kernel releases, where devices are >>> system-suspended regardless their runtime pm states (in older kernels >>> devices were first runtime resumed for system suspend, what made code >>> simpler, but wasn't best from power consumption perspective). >>> >>> If you go this way, You only need to ensure that runtime resume will also >>> restore proper device state besides enabling all the clocks. This will >>> also prepare your driver to properly operate inside power domain, where it >>> is possible for device to loose its internal state after runtime suspend >>> when respective power domain has been turned off. >> I'm not sure if you are aware of the pm_runtime_force_* limitations, though. > > What are those limitations? First off, they don't work with middle-layer code implementing its own PM callbacks that actually operate on devices (like the PCI bus type or the generic ACPI PM domain). This means that drivers using them may not work on systems with ACPI, for example. Second, pm_runtime_force_resume() will always try to leave the device in suspend during system-wide resume from sleep which may not be desirable. Finally, they expect the runtime PM status to be updated by system-wide PM callbacks of devices below the one using them (eg. its children and their children etc) which generally is not required and may not take place unless the drivers of those devices use pm_runtime_force_* themselves. So careful there. ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
Hi Rafael,
On 2018-07-11 22:36, Rafael J. Wysocki wrote:
> On Wed, Jul 11, 2018 at 3:40 PM, Marek Szyprowski
> wrote:
>> Hi Tomasz,
>>
>> On 2018-07-11 14:51, Tomasz Figa wrote:
>>> On Wed, Jul 11, 2018 at 8:11 PM Rafael J. Wysocki wrote:
On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
wrote:
> On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki
> wrote:
>> On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
>>> From: Sricharan R
>>>
>>> The smmu needs to be functional only when the respective
>>> master's using it are active. The device_link feature
>>> helps to track such functional dependencies, so that the
>>> iommu gets powered when the master device enables itself
>>> using pm_runtime. So by adapting the smmu driver for
>>> runtime pm, above said dependency can be addressed.
>>>
>>> This patch adds the pm runtime/sleep callbacks to the
>>> driver and also the functions to parse the smmu clocks
>>> from DT and enable them in resume/suspend.
>>>
>>> Signed-off-by: Sricharan R
>>> Signed-off-by: Archit Taneja
>>> [vivek: Clock rework to request bulk of clocks]
>>> Signed-off-by: Vivek Gautam
>>> Reviewed-by: Tomasz Figa
>>> ---
>>>
>>>- No change since v11.
>>>
>>>drivers/iommu/arm-smmu.c | 60
>>> ++--
>>>1 file changed, 58 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>> index f7a96bcf94a6..a01d0dde21dd 100644
>>> --- a/drivers/iommu/arm-smmu.c
>>> +++ b/drivers/iommu/arm-smmu.c
>>> @@ -48,6 +48,7 @@
>>>#include
>>>#include
>>>#include
>>> +#include
>>>#include
>>>#include
>>>
>>> @@ -205,6 +206,8 @@ struct arm_smmu_device {
>>> u32 num_global_irqs;
>>> u32 num_context_irqs;
>>> unsigned int*irqs;
>>> + struct clk_bulk_data*clks;
>>> + int num_clks;
>>>
>>> u32 cavium_id_base; /* Specific to
>>> Cavium */
>>>
>>> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
>>> arm_smmu_device *smmu)
>>>struct arm_smmu_match_data {
>>> enum arm_smmu_arch_version version;
>>> enum arm_smmu_implementation model;
>>> + const char * const *clks;
>>> + int num_clks;
>>>};
>>>
>>>#define ARM_SMMU_MATCH_DATA(name, ver, imp) \
>>> -static struct arm_smmu_match_data name = { .version = ver, .model =
>>> imp }
>>> +static const struct arm_smmu_match_data name = { .version = ver,
>>> .model = imp }
>>>
>>>ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
>>>ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
>>> @@ -1919,6 +1924,23 @@ static const struct of_device_id
>>> arm_smmu_of_match[] = {
>>>};
>>>MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>>>
>>> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
>>> +const char * const *clks)
>>> +{
>>> + int i;
>>> +
>>> + if (smmu->num_clks < 1)
>>> + return;
>>> +
>>> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
>>> + sizeof(*smmu->clks), GFP_KERNEL);
>>> + if (!smmu->clks)
>>> + return;
>>> +
>>> + for (i = 0; i < smmu->num_clks; i++)
>>> + smmu->clks[i].id = clks[i];
>>> +}
>>> +
>>>#ifdef CONFIG_ACPI
>>>static int acpi_smmu_get_data(u32 model, struct arm_smmu_device
>>> *smmu)
>>>{
>>> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
>>> platform_device *pdev,
>>> data = of_device_get_match_data(dev);
>>> smmu->version = data->version;
>>> smmu->model = data->model;
>>> + smmu->num_clks = data->num_clks;
>>> +
>>> + arm_smmu_fill_clk_data(smmu, data->clks);
>>>
>>> parse_driver_options(smmu);
>>>
>>> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
>>> platform_device *pdev)
>>> smmu->irqs[i] = irq;
>>> }
>>>
>>> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
>>> + if (err)
>>> + return err;
>>> +
>>> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
>>> + if (err)
>>> + return err;
>>> +
>>> err = arm_smmu_device_cfg_probe(smmu);
>>> if (err)
>>> return err;
>>> @@ -2181,6 +2214,9 @@ static int
Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
Hi,
On Mon, Jul 16, 2018 at 12:11 PM, Vivek Gautam
wrote:
> HI Rafael,
>
>
>
> On 7/16/2018 2:21 PM, Rafael J. Wysocki wrote:
>>
>> On Thu, Jul 12, 2018 at 12:57 PM, Vivek Gautam
>> wrote:
[cut]
Although, given the PM
subsystem internals, the suspend function wouldn't be called on SMMU
implementation needed power control (since they would have runtime PM
enabled) and on others, it would be called but do nothing (since no
clocks).
> Honestly, I just don't know. :-)
>
> It just looks odd the way it is done. I think the clock should be
> gated during system-wide suspend too, because the system can spend
> much more time in a sleep state than in the working state, on average.
>
> And note that you cannot rely on runtime PM to always do it for you,
> because it may be disabled at a client device or even blocked by user
> space via power/control in sysfs and that shouldn't matter for
> system-wide PM.
User space blocking runtime PM through sysfs is a good point. I'm not
100% sure how the PM subsystem deals with that in case of system-wide
suspend. I guess for consistency and safety, we should have the
suspend callback.
>>>
>>> Will add the following suspend callback (same as
>>> arm_smmu_runtime_suspend):
>>>
>>> static int __maybe_unused arm_smmu_pm_suspend(struct device *dev)
>>> {
>>> struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>>>
>>> clk_bulk_disable(smmu->num_clks, smmu->clks);
>>>
>>> return 0;
>>> }
>>
>> I think you also need to check if the clock has already been disabled
>> by runtime PM. Otherwise you may end up disabling it twice in a row.
>
>
> Should I rather call a pm_runtime_put() in suspend callback?
That wouldn't work as runtime PM may be effectively disabled by user
space via sysfs. That's one of the reasons why you need the extra
system-wide suspend callback in the first place. :-)
> Or an expanded form something similar to:
> https://elixir.bootlin.com/linux/v4.18-rc5/source/drivers/slimbus/qcom-ctrl.c#L695
Yes, you can do something like that, but be careful to make sure that
the state of the device after system-wide resume is consistent with
its runtime PM status in all cases.
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
HI Rafael,
On 7/16/2018 2:21 PM, Rafael J. Wysocki wrote:
On Thu, Jul 12, 2018 at 12:57 PM, Vivek Gautam
wrote:
Hi,
On Wed, Jul 11, 2018 at 6:21 PM, Tomasz Figa wrote:
On Wed, Jul 11, 2018 at 8:11 PM Rafael J. Wysocki wrote:
On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
wrote:
Hi Rafael,
On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki wrote:
On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
From: Sricharan R
The smmu needs to be functional only when the respective
master's using it are active. The device_link feature
helps to track such functional dependencies, so that the
iommu gets powered when the master device enables itself
using pm_runtime. So by adapting the smmu driver for
runtime pm, above said dependency can be addressed.
This patch adds the pm runtime/sleep callbacks to the
driver and also the functions to parse the smmu clocks
from DT and enable them in resume/suspend.
Signed-off-by: Sricharan R
Signed-off-by: Archit Taneja
[vivek: Clock rework to request bulk of clocks]
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
---
- No change since v11.
drivers/iommu/arm-smmu.c | 60 ++--
1 file changed, 58 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f7a96bcf94a6..a01d0dde21dd 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
#include
@@ -205,6 +206,8 @@ struct arm_smmu_device {
u32 num_global_irqs;
u32 num_context_irqs;
unsigned int*irqs;
+ struct clk_bulk_data*clks;
+ int num_clks;
u32 cavium_id_base; /* Specific to Cavium */
@@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
arm_smmu_device *smmu)
struct arm_smmu_match_data {
enum arm_smmu_arch_version version;
enum arm_smmu_implementation model;
+ const char * const *clks;
+ int num_clks;
};
#define ARM_SMMU_MATCH_DATA(name, ver, imp) \
-static struct arm_smmu_match_data name = { .version = ver, .model = imp }
+static const struct arm_smmu_match_data name = { .version = ver, .model = imp }
ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
@@ -1919,6 +1924,23 @@ static const struct of_device_id arm_smmu_of_match[] = {
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
+static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
+const char * const *clks)
+{
+ int i;
+
+ if (smmu->num_clks < 1)
+ return;
+
+ smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
+ sizeof(*smmu->clks), GFP_KERNEL);
+ if (!smmu->clks)
+ return;
+
+ for (i = 0; i < smmu->num_clks; i++)
+ smmu->clks[i].id = clks[i];
+}
+
#ifdef CONFIG_ACPI
static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
{
@@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
platform_device *pdev,
data = of_device_get_match_data(dev);
smmu->version = data->version;
smmu->model = data->model;
+ smmu->num_clks = data->num_clks;
+
+ arm_smmu_fill_clk_data(smmu, data->clks);
parse_driver_options(smmu);
@@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
smmu->irqs[i] = irq;
}
+ err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
+ if (err)
+ return err;
+
+ err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
+ if (err)
+ return err;
+
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;
@@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct platform_device
*pdev)
/* Turn the thing off */
writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
+
+ clk_bulk_unprepare(smmu->num_clks, smmu->clks);
+
return 0;
}
@@ -2197,7 +2233,27 @@ static int __maybe_unused arm_smmu_pm_resume(struct
device *dev)
return 0;
}
-static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
+static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
+{
+ struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+
+ return clk_bulk_enable(smmu->num_clks, smmu->clks);
+}
+
+static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
+{
+ struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+
+ clk_bulk_disable(smmu->num_clks, smmu->clks);
+
+ return 0;
+}
+
+static const struct dev_pm_ops arm_smmu_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(NULL, arm_smmu_pm_resume)
This is suspicious.
If you need a runtime suspend method, why do you
Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
On Thu, Jul 12, 2018 at 12:57 PM, Vivek Gautam
wrote:
> Hi,
>
>
> On Wed, Jul 11, 2018 at 6:21 PM, Tomasz Figa wrote:
>> On Wed, Jul 11, 2018 at 8:11 PM Rafael J. Wysocki wrote:
>>>
>>> On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
>>> wrote:
>>> > Hi Rafael,
>>> >
>>> >
>>> > On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki
>>> > wrote:
>>> >> On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
>>> >>> From: Sricharan R
>>> >>>
>>> >>> The smmu needs to be functional only when the respective
>>> >>> master's using it are active. The device_link feature
>>> >>> helps to track such functional dependencies, so that the
>>> >>> iommu gets powered when the master device enables itself
>>> >>> using pm_runtime. So by adapting the smmu driver for
>>> >>> runtime pm, above said dependency can be addressed.
>>> >>>
>>> >>> This patch adds the pm runtime/sleep callbacks to the
>>> >>> driver and also the functions to parse the smmu clocks
>>> >>> from DT and enable them in resume/suspend.
>>> >>>
>>> >>> Signed-off-by: Sricharan R
>>> >>> Signed-off-by: Archit Taneja
>>> >>> [vivek: Clock rework to request bulk of clocks]
>>> >>> Signed-off-by: Vivek Gautam
>>> >>> Reviewed-by: Tomasz Figa
>>> >>> ---
>>> >>>
>>> >>> - No change since v11.
>>> >>>
>>> >>> drivers/iommu/arm-smmu.c | 60
>>> >>> ++--
>>> >>> 1 file changed, 58 insertions(+), 2 deletions(-)
>>> >>>
>>> >>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>> >>> index f7a96bcf94a6..a01d0dde21dd 100644
>>> >>> --- a/drivers/iommu/arm-smmu.c
>>> >>> +++ b/drivers/iommu/arm-smmu.c
>>> >>> @@ -48,6 +48,7 @@
>>> >>> #include
>>> >>> #include
>>> >>> #include
>>> >>> +#include
>>> >>> #include
>>> >>> #include
>>> >>>
>>> >>> @@ -205,6 +206,8 @@ struct arm_smmu_device {
>>> >>> u32 num_global_irqs;
>>> >>> u32 num_context_irqs;
>>> >>> unsigned int*irqs;
>>> >>> + struct clk_bulk_data*clks;
>>> >>> + int num_clks;
>>> >>>
>>> >>> u32 cavium_id_base; /* Specific to
>>> >>> Cavium */
>>> >>>
>>> >>> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
>>> >>> arm_smmu_device *smmu)
>>> >>> struct arm_smmu_match_data {
>>> >>> enum arm_smmu_arch_version version;
>>> >>> enum arm_smmu_implementation model;
>>> >>> + const char * const *clks;
>>> >>> + int num_clks;
>>> >>> };
>>> >>>
>>> >>> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
>>> >>> -static struct arm_smmu_match_data name = { .version = ver, .model =
>>> >>> imp }
>>> >>> +static const struct arm_smmu_match_data name = { .version = ver,
>>> >>> .model = imp }
>>> >>>
>>> >>> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
>>> >>> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
>>> >>> @@ -1919,6 +1924,23 @@ static const struct of_device_id
>>> >>> arm_smmu_of_match[] = {
>>> >>> };
>>> >>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>>> >>>
>>> >>> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
>>> >>> +const char * const *clks)
>>> >>> +{
>>> >>> + int i;
>>> >>> +
>>> >>> + if (smmu->num_clks < 1)
>>> >>> + return;
>>> >>> +
>>> >>> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
>>> >>> + sizeof(*smmu->clks), GFP_KERNEL);
>>> >>> + if (!smmu->clks)
>>> >>> + return;
>>> >>> +
>>> >>> + for (i = 0; i < smmu->num_clks; i++)
>>> >>> + smmu->clks[i].id = clks[i];
>>> >>> +}
>>> >>> +
>>> >>> #ifdef CONFIG_ACPI
>>> >>> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
>>> >>> {
>>> >>> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
>>> >>> platform_device *pdev,
>>> >>> data = of_device_get_match_data(dev);
>>> >>> smmu->version = data->version;
>>> >>> smmu->model = data->model;
>>> >>> + smmu->num_clks = data->num_clks;
>>> >>> +
>>> >>> + arm_smmu_fill_clk_data(smmu, data->clks);
>>> >>>
>>> >>> parse_driver_options(smmu);
>>> >>>
>>> >>> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
>>> >>> platform_device *pdev)
>>> >>> smmu->irqs[i] = irq;
>>> >>> }
>>> >>>
>>> >>> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
>>> >>> + if (err)
>>> >>> + return err;
>>> >>> +
>>> >>> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
>>> >>> + if (err)
>>> >>> + return err;
>>> >>> +
>>> >>> err = arm_smmu_device_cfg_probe(smmu);
>>> >>> if (err)
>>> >>> return err;
>>> >>> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
>>> >>> platform_device *pdev)
>>> >>>
>>> >>> /* Turn the thing off */
>>> >>>
Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
On Wed, Jul 11, 2018 at 3:40 PM, Marek Szyprowski
wrote:
> Hi Tomasz,
>
> On 2018-07-11 14:51, Tomasz Figa wrote:
>> On Wed, Jul 11, 2018 at 8:11 PM Rafael J. Wysocki wrote:
>>> On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
>>> wrote:
On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki
wrote:
> On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
>> From: Sricharan R
>>
>> The smmu needs to be functional only when the respective
>> master's using it are active. The device_link feature
>> helps to track such functional dependencies, so that the
>> iommu gets powered when the master device enables itself
>> using pm_runtime. So by adapting the smmu driver for
>> runtime pm, above said dependency can be addressed.
>>
>> This patch adds the pm runtime/sleep callbacks to the
>> driver and also the functions to parse the smmu clocks
>> from DT and enable them in resume/suspend.
>>
>> Signed-off-by: Sricharan R
>> Signed-off-by: Archit Taneja
>> [vivek: Clock rework to request bulk of clocks]
>> Signed-off-by: Vivek Gautam
>> Reviewed-by: Tomasz Figa
>> ---
>>
>> - No change since v11.
>>
>> drivers/iommu/arm-smmu.c | 60
>> ++--
>> 1 file changed, 58 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index f7a96bcf94a6..a01d0dde21dd 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -48,6 +48,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> @@ -205,6 +206,8 @@ struct arm_smmu_device {
>>u32 num_global_irqs;
>>u32 num_context_irqs;
>>unsigned int*irqs;
>> + struct clk_bulk_data*clks;
>> + int num_clks;
>>
>>u32 cavium_id_base; /* Specific to
>> Cavium */
>>
>> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
>> arm_smmu_device *smmu)
>> struct arm_smmu_match_data {
>>enum arm_smmu_arch_version version;
>>enum arm_smmu_implementation model;
>> + const char * const *clks;
>> + int num_clks;
>> };
>>
>> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
>> -static struct arm_smmu_match_data name = { .version = ver, .model = imp
>> }
>> +static const struct arm_smmu_match_data name = { .version = ver, .model
>> = imp }
>>
>> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
>> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
>> @@ -1919,6 +1924,23 @@ static const struct of_device_id
>> arm_smmu_of_match[] = {
>> };
>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>>
>> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
>> +const char * const *clks)
>> +{
>> + int i;
>> +
>> + if (smmu->num_clks < 1)
>> + return;
>> +
>> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
>> + sizeof(*smmu->clks), GFP_KERNEL);
>> + if (!smmu->clks)
>> + return;
>> +
>> + for (i = 0; i < smmu->num_clks; i++)
>> + smmu->clks[i].id = clks[i];
>> +}
>> +
>> #ifdef CONFIG_ACPI
>> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
>> {
>> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
>> platform_device *pdev,
>>data = of_device_get_match_data(dev);
>>smmu->version = data->version;
>>smmu->model = data->model;
>> + smmu->num_clks = data->num_clks;
>> +
>> + arm_smmu_fill_clk_data(smmu, data->clks);
>>
>>parse_driver_options(smmu);
>>
>> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
>> platform_device *pdev)
>>smmu->irqs[i] = irq;
>>}
>>
>> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
>> + if (err)
>> + return err;
>> +
>> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
>> + if (err)
>> + return err;
>> +
>>err = arm_smmu_device_cfg_probe(smmu);
>>if (err)
>>return err;
>> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
>> platform_device *pdev)
>>
>>/* Turn the thing off */
>>writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
>> +
>> + clk_bulk_unprepare(smmu->num_clks,
Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
Hi Tomasz,
On 2018-07-11 14:51, Tomasz Figa wrote:
> On Wed, Jul 11, 2018 at 8:11 PM Rafael J. Wysocki wrote:
>> On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
>> wrote:
>>> On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki
>>> wrote:
On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
> From: Sricharan R
>
> The smmu needs to be functional only when the respective
> master's using it are active. The device_link feature
> helps to track such functional dependencies, so that the
> iommu gets powered when the master device enables itself
> using pm_runtime. So by adapting the smmu driver for
> runtime pm, above said dependency can be addressed.
>
> This patch adds the pm runtime/sleep callbacks to the
> driver and also the functions to parse the smmu clocks
> from DT and enable them in resume/suspend.
>
> Signed-off-by: Sricharan R
> Signed-off-by: Archit Taneja
> [vivek: Clock rework to request bulk of clocks]
> Signed-off-by: Vivek Gautam
> Reviewed-by: Tomasz Figa
> ---
>
> - No change since v11.
>
> drivers/iommu/arm-smmu.c | 60
> ++--
> 1 file changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f7a96bcf94a6..a01d0dde21dd 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -48,6 +48,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -205,6 +206,8 @@ struct arm_smmu_device {
>u32 num_global_irqs;
>u32 num_context_irqs;
>unsigned int*irqs;
> + struct clk_bulk_data*clks;
> + int num_clks;
>
>u32 cavium_id_base; /* Specific to
> Cavium */
>
> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
> arm_smmu_device *smmu)
> struct arm_smmu_match_data {
>enum arm_smmu_arch_version version;
>enum arm_smmu_implementation model;
> + const char * const *clks;
> + int num_clks;
> };
>
> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
> -static struct arm_smmu_match_data name = { .version = ver, .model = imp }
> +static const struct arm_smmu_match_data name = { .version = ver, .model
> = imp }
>
> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
> @@ -1919,6 +1924,23 @@ static const struct of_device_id
> arm_smmu_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>
> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
> +const char * const *clks)
> +{
> + int i;
> +
> + if (smmu->num_clks < 1)
> + return;
> +
> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
> + sizeof(*smmu->clks), GFP_KERNEL);
> + if (!smmu->clks)
> + return;
> +
> + for (i = 0; i < smmu->num_clks; i++)
> + smmu->clks[i].id = clks[i];
> +}
> +
> #ifdef CONFIG_ACPI
> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
> {
> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
> platform_device *pdev,
>data = of_device_get_match_data(dev);
>smmu->version = data->version;
>smmu->model = data->model;
> + smmu->num_clks = data->num_clks;
> +
> + arm_smmu_fill_clk_data(smmu, data->clks);
>
>parse_driver_options(smmu);
>
> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
> platform_device *pdev)
>smmu->irqs[i] = irq;
>}
>
> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
> + if (err)
> + return err;
> +
> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
> + if (err)
> + return err;
> +
>err = arm_smmu_device_cfg_probe(smmu);
>if (err)
>return err;
> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
> platform_device *pdev)
>
>/* Turn the thing off */
>writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
> +
> + clk_bulk_unprepare(smmu->num_clks, smmu->clks);
> +
>return 0;
> }
>
> @@ -2197,7 +2233,27 @@ static int __maybe_unused
> arm_smmu_pm_resume(struct device *dev)
>return 0;
> }
>
Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
On Wed, Jul 11, 2018 at 8:11 PM Rafael J. Wysocki wrote:
>
> On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
> wrote:
> > Hi Rafael,
> >
> >
> > On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki
> > wrote:
> >> On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
> >>> From: Sricharan R
> >>>
> >>> The smmu needs to be functional only when the respective
> >>> master's using it are active. The device_link feature
> >>> helps to track such functional dependencies, so that the
> >>> iommu gets powered when the master device enables itself
> >>> using pm_runtime. So by adapting the smmu driver for
> >>> runtime pm, above said dependency can be addressed.
> >>>
> >>> This patch adds the pm runtime/sleep callbacks to the
> >>> driver and also the functions to parse the smmu clocks
> >>> from DT and enable them in resume/suspend.
> >>>
> >>> Signed-off-by: Sricharan R
> >>> Signed-off-by: Archit Taneja
> >>> [vivek: Clock rework to request bulk of clocks]
> >>> Signed-off-by: Vivek Gautam
> >>> Reviewed-by: Tomasz Figa
> >>> ---
> >>>
> >>> - No change since v11.
> >>>
> >>> drivers/iommu/arm-smmu.c | 60
> >>> ++--
> >>> 1 file changed, 58 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >>> index f7a96bcf94a6..a01d0dde21dd 100644
> >>> --- a/drivers/iommu/arm-smmu.c
> >>> +++ b/drivers/iommu/arm-smmu.c
> >>> @@ -48,6 +48,7 @@
> >>> #include
> >>> #include
> >>> #include
> >>> +#include
> >>> #include
> >>> #include
> >>>
> >>> @@ -205,6 +206,8 @@ struct arm_smmu_device {
> >>> u32 num_global_irqs;
> >>> u32 num_context_irqs;
> >>> unsigned int*irqs;
> >>> + struct clk_bulk_data*clks;
> >>> + int num_clks;
> >>>
> >>> u32 cavium_id_base; /* Specific to
> >>> Cavium */
> >>>
> >>> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
> >>> arm_smmu_device *smmu)
> >>> struct arm_smmu_match_data {
> >>> enum arm_smmu_arch_version version;
> >>> enum arm_smmu_implementation model;
> >>> + const char * const *clks;
> >>> + int num_clks;
> >>> };
> >>>
> >>> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
> >>> -static struct arm_smmu_match_data name = { .version = ver, .model = imp }
> >>> +static const struct arm_smmu_match_data name = { .version = ver, .model
> >>> = imp }
> >>>
> >>> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
> >>> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
> >>> @@ -1919,6 +1924,23 @@ static const struct of_device_id
> >>> arm_smmu_of_match[] = {
> >>> };
> >>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
> >>>
> >>> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
> >>> +const char * const *clks)
> >>> +{
> >>> + int i;
> >>> +
> >>> + if (smmu->num_clks < 1)
> >>> + return;
> >>> +
> >>> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
> >>> + sizeof(*smmu->clks), GFP_KERNEL);
> >>> + if (!smmu->clks)
> >>> + return;
> >>> +
> >>> + for (i = 0; i < smmu->num_clks; i++)
> >>> + smmu->clks[i].id = clks[i];
> >>> +}
> >>> +
> >>> #ifdef CONFIG_ACPI
> >>> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
> >>> {
> >>> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
> >>> platform_device *pdev,
> >>> data = of_device_get_match_data(dev);
> >>> smmu->version = data->version;
> >>> smmu->model = data->model;
> >>> + smmu->num_clks = data->num_clks;
> >>> +
> >>> + arm_smmu_fill_clk_data(smmu, data->clks);
> >>>
> >>> parse_driver_options(smmu);
> >>>
> >>> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
> >>> platform_device *pdev)
> >>> smmu->irqs[i] = irq;
> >>> }
> >>>
> >>> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
> >>> + if (err)
> >>> + return err;
> >>> +
> >>> err = arm_smmu_device_cfg_probe(smmu);
> >>> if (err)
> >>> return err;
> >>> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
> >>> platform_device *pdev)
> >>>
> >>> /* Turn the thing off */
> >>> writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
> >>> +
> >>> + clk_bulk_unprepare(smmu->num_clks, smmu->clks);
> >>> +
> >>> return 0;
> >>> }
> >>>
> >>> @@ -2197,7 +2233,27 @@ static int __maybe_unused
> >>> arm_smmu_pm_resume(struct device *dev)
> >>> return 0;
> >>> }
> >>>
> >>> -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL,
Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
On Wed, Jul 11, 2018 at 12:55 PM, Vivek Gautam
wrote:
> Hi Rafael,
>
>
> On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki wrote:
>> On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
>>> From: Sricharan R
>>>
>>> The smmu needs to be functional only when the respective
>>> master's using it are active. The device_link feature
>>> helps to track such functional dependencies, so that the
>>> iommu gets powered when the master device enables itself
>>> using pm_runtime. So by adapting the smmu driver for
>>> runtime pm, above said dependency can be addressed.
>>>
>>> This patch adds the pm runtime/sleep callbacks to the
>>> driver and also the functions to parse the smmu clocks
>>> from DT and enable them in resume/suspend.
>>>
>>> Signed-off-by: Sricharan R
>>> Signed-off-by: Archit Taneja
>>> [vivek: Clock rework to request bulk of clocks]
>>> Signed-off-by: Vivek Gautam
>>> Reviewed-by: Tomasz Figa
>>> ---
>>>
>>> - No change since v11.
>>>
>>> drivers/iommu/arm-smmu.c | 60
>>> ++--
>>> 1 file changed, 58 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>>> index f7a96bcf94a6..a01d0dde21dd 100644
>>> --- a/drivers/iommu/arm-smmu.c
>>> +++ b/drivers/iommu/arm-smmu.c
>>> @@ -48,6 +48,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>> #include
>>>
>>> @@ -205,6 +206,8 @@ struct arm_smmu_device {
>>> u32 num_global_irqs;
>>> u32 num_context_irqs;
>>> unsigned int*irqs;
>>> + struct clk_bulk_data*clks;
>>> + int num_clks;
>>>
>>> u32 cavium_id_base; /* Specific to Cavium
>>> */
>>>
>>> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
>>> arm_smmu_device *smmu)
>>> struct arm_smmu_match_data {
>>> enum arm_smmu_arch_version version;
>>> enum arm_smmu_implementation model;
>>> + const char * const *clks;
>>> + int num_clks;
>>> };
>>>
>>> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
>>> -static struct arm_smmu_match_data name = { .version = ver, .model = imp }
>>> +static const struct arm_smmu_match_data name = { .version = ver, .model =
>>> imp }
>>>
>>> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
>>> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
>>> @@ -1919,6 +1924,23 @@ static const struct of_device_id arm_smmu_of_match[]
>>> = {
>>> };
>>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>>>
>>> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
>>> +const char * const *clks)
>>> +{
>>> + int i;
>>> +
>>> + if (smmu->num_clks < 1)
>>> + return;
>>> +
>>> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
>>> + sizeof(*smmu->clks), GFP_KERNEL);
>>> + if (!smmu->clks)
>>> + return;
>>> +
>>> + for (i = 0; i < smmu->num_clks; i++)
>>> + smmu->clks[i].id = clks[i];
>>> +}
>>> +
>>> #ifdef CONFIG_ACPI
>>> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
>>> {
>>> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
>>> platform_device *pdev,
>>> data = of_device_get_match_data(dev);
>>> smmu->version = data->version;
>>> smmu->model = data->model;
>>> + smmu->num_clks = data->num_clks;
>>> +
>>> + arm_smmu_fill_clk_data(smmu, data->clks);
>>>
>>> parse_driver_options(smmu);
>>>
>>> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
>>> platform_device *pdev)
>>> smmu->irqs[i] = irq;
>>> }
>>>
>>> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
>>> + if (err)
>>> + return err;
>>> +
>>> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
>>> + if (err)
>>> + return err;
>>> +
>>> err = arm_smmu_device_cfg_probe(smmu);
>>> if (err)
>>> return err;
>>> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
>>> platform_device *pdev)
>>>
>>> /* Turn the thing off */
>>> writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
>>> +
>>> + clk_bulk_unprepare(smmu->num_clks, smmu->clks);
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -2197,7 +2233,27 @@ static int __maybe_unused arm_smmu_pm_resume(struct
>>> device *dev)
>>> return 0;
>>> }
>>>
>>> -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
>>> +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
>>> +{
>>> + struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>>> +
>>> + return clk_bulk_enable(smmu->num_clks, smmu->clks);
>>> +}
>>> +
>>> +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
>>> +{
>>> + struct
Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
Hi Rafael,
On Wed, Jul 11, 2018 at 3:20 PM, Rafael J. Wysocki wrote:
> On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
>> From: Sricharan R
>>
>> The smmu needs to be functional only when the respective
>> master's using it are active. The device_link feature
>> helps to track such functional dependencies, so that the
>> iommu gets powered when the master device enables itself
>> using pm_runtime. So by adapting the smmu driver for
>> runtime pm, above said dependency can be addressed.
>>
>> This patch adds the pm runtime/sleep callbacks to the
>> driver and also the functions to parse the smmu clocks
>> from DT and enable them in resume/suspend.
>>
>> Signed-off-by: Sricharan R
>> Signed-off-by: Archit Taneja
>> [vivek: Clock rework to request bulk of clocks]
>> Signed-off-by: Vivek Gautam
>> Reviewed-by: Tomasz Figa
>> ---
>>
>> - No change since v11.
>>
>> drivers/iommu/arm-smmu.c | 60
>> ++--
>> 1 file changed, 58 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index f7a96bcf94a6..a01d0dde21dd 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -48,6 +48,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> @@ -205,6 +206,8 @@ struct arm_smmu_device {
>> u32 num_global_irqs;
>> u32 num_context_irqs;
>> unsigned int*irqs;
>> + struct clk_bulk_data*clks;
>> + int num_clks;
>>
>> u32 cavium_id_base; /* Specific to Cavium
>> */
>>
>> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
>> arm_smmu_device *smmu)
>> struct arm_smmu_match_data {
>> enum arm_smmu_arch_version version;
>> enum arm_smmu_implementation model;
>> + const char * const *clks;
>> + int num_clks;
>> };
>>
>> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
>> -static struct arm_smmu_match_data name = { .version = ver, .model = imp }
>> +static const struct arm_smmu_match_data name = { .version = ver, .model =
>> imp }
>>
>> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
>> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
>> @@ -1919,6 +1924,23 @@ static const struct of_device_id arm_smmu_of_match[]
>> = {
>> };
>> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>>
>> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
>> +const char * const *clks)
>> +{
>> + int i;
>> +
>> + if (smmu->num_clks < 1)
>> + return;
>> +
>> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
>> + sizeof(*smmu->clks), GFP_KERNEL);
>> + if (!smmu->clks)
>> + return;
>> +
>> + for (i = 0; i < smmu->num_clks; i++)
>> + smmu->clks[i].id = clks[i];
>> +}
>> +
>> #ifdef CONFIG_ACPI
>> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
>> {
>> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
>> platform_device *pdev,
>> data = of_device_get_match_data(dev);
>> smmu->version = data->version;
>> smmu->model = data->model;
>> + smmu->num_clks = data->num_clks;
>> +
>> + arm_smmu_fill_clk_data(smmu, data->clks);
>>
>> parse_driver_options(smmu);
>>
>> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
>> platform_device *pdev)
>> smmu->irqs[i] = irq;
>> }
>>
>> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
>> + if (err)
>> + return err;
>> +
>> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
>> + if (err)
>> + return err;
>> +
>> err = arm_smmu_device_cfg_probe(smmu);
>> if (err)
>> return err;
>> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
>> platform_device *pdev)
>>
>> /* Turn the thing off */
>> writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
>> +
>> + clk_bulk_unprepare(smmu->num_clks, smmu->clks);
>> +
>> return 0;
>> }
>>
>> @@ -2197,7 +2233,27 @@ static int __maybe_unused arm_smmu_pm_resume(struct
>> device *dev)
>> return 0;
>> }
>>
>> -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
>> +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
>> +{
>> + struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>> +
>> + return clk_bulk_enable(smmu->num_clks, smmu->clks);
>> +}
>> +
>> +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
>> +{
>> + struct arm_smmu_device *smmu = dev_get_drvdata(dev);
>> +
>> + clk_bulk_disable(smmu->num_clks, smmu->clks);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dev_pm_ops arm_smmu_pm_ops = {
>> +
Re: [Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
On Sunday, July 8, 2018 7:34:10 PM CEST Vivek Gautam wrote:
> From: Sricharan R
>
> The smmu needs to be functional only when the respective
> master's using it are active. The device_link feature
> helps to track such functional dependencies, so that the
> iommu gets powered when the master device enables itself
> using pm_runtime. So by adapting the smmu driver for
> runtime pm, above said dependency can be addressed.
>
> This patch adds the pm runtime/sleep callbacks to the
> driver and also the functions to parse the smmu clocks
> from DT and enable them in resume/suspend.
>
> Signed-off-by: Sricharan R
> Signed-off-by: Archit Taneja
> [vivek: Clock rework to request bulk of clocks]
> Signed-off-by: Vivek Gautam
> Reviewed-by: Tomasz Figa
> ---
>
> - No change since v11.
>
> drivers/iommu/arm-smmu.c | 60
> ++--
> 1 file changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f7a96bcf94a6..a01d0dde21dd 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -48,6 +48,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -205,6 +206,8 @@ struct arm_smmu_device {
> u32 num_global_irqs;
> u32 num_context_irqs;
> unsigned int*irqs;
> + struct clk_bulk_data*clks;
> + int num_clks;
>
> u32 cavium_id_base; /* Specific to Cavium */
>
> @@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
> arm_smmu_device *smmu)
> struct arm_smmu_match_data {
> enum arm_smmu_arch_version version;
> enum arm_smmu_implementation model;
> + const char * const *clks;
> + int num_clks;
> };
>
> #define ARM_SMMU_MATCH_DATA(name, ver, imp) \
> -static struct arm_smmu_match_data name = { .version = ver, .model = imp }
> +static const struct arm_smmu_match_data name = { .version = ver, .model =
> imp }
>
> ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
> ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
> @@ -1919,6 +1924,23 @@ static const struct of_device_id arm_smmu_of_match[] =
> {
> };
> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>
> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
> +const char * const *clks)
> +{
> + int i;
> +
> + if (smmu->num_clks < 1)
> + return;
> +
> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
> + sizeof(*smmu->clks), GFP_KERNEL);
> + if (!smmu->clks)
> + return;
> +
> + for (i = 0; i < smmu->num_clks; i++)
> + smmu->clks[i].id = clks[i];
> +}
> +
> #ifdef CONFIG_ACPI
> static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
> {
> @@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
> platform_device *pdev,
> data = of_device_get_match_data(dev);
> smmu->version = data->version;
> smmu->model = data->model;
> + smmu->num_clks = data->num_clks;
> +
> + arm_smmu_fill_clk_data(smmu, data->clks);
>
> parse_driver_options(smmu);
>
> @@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct
> platform_device *pdev)
> smmu->irqs[i] = irq;
> }
>
> + err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
> + if (err)
> + return err;
> +
> + err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
> + if (err)
> + return err;
> +
> err = arm_smmu_device_cfg_probe(smmu);
> if (err)
> return err;
> @@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct
> platform_device *pdev)
>
> /* Turn the thing off */
> writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
> +
> + clk_bulk_unprepare(smmu->num_clks, smmu->clks);
> +
> return 0;
> }
>
> @@ -2197,7 +2233,27 @@ static int __maybe_unused arm_smmu_pm_resume(struct
> device *dev)
> return 0;
> }
>
> -static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
> +static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
> +{
> + struct arm_smmu_device *smmu = dev_get_drvdata(dev);
> +
> + return clk_bulk_enable(smmu->num_clks, smmu->clks);
> +}
> +
> +static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
> +{
> + struct arm_smmu_device *smmu = dev_get_drvdata(dev);
> +
> + clk_bulk_disable(smmu->num_clks, smmu->clks);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops arm_smmu_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(NULL, arm_smmu_pm_resume)
This is suspicious.
If you need a runtime suspend method, why do you think that it is not necessary
to suspend the device during system-wide transitions?
> +
[Freedreno] [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
From: Sricharan R
The smmu needs to be functional only when the respective
master's using it are active. The device_link feature
helps to track such functional dependencies, so that the
iommu gets powered when the master device enables itself
using pm_runtime. So by adapting the smmu driver for
runtime pm, above said dependency can be addressed.
This patch adds the pm runtime/sleep callbacks to the
driver and also the functions to parse the smmu clocks
from DT and enable them in resume/suspend.
Signed-off-by: Sricharan R
Signed-off-by: Archit Taneja
[vivek: Clock rework to request bulk of clocks]
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
---
- No change since v11.
drivers/iommu/arm-smmu.c | 60 ++--
1 file changed, 58 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f7a96bcf94a6..a01d0dde21dd 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
#include
@@ -205,6 +206,8 @@ struct arm_smmu_device {
u32 num_global_irqs;
u32 num_context_irqs;
unsigned int*irqs;
+ struct clk_bulk_data*clks;
+ int num_clks;
u32 cavium_id_base; /* Specific to Cavium */
@@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct
arm_smmu_device *smmu)
struct arm_smmu_match_data {
enum arm_smmu_arch_version version;
enum arm_smmu_implementation model;
+ const char * const *clks;
+ int num_clks;
};
#define ARM_SMMU_MATCH_DATA(name, ver, imp)\
-static struct arm_smmu_match_data name = { .version = ver, .model = imp }
+static const struct arm_smmu_match_data name = { .version = ver, .model = imp }
ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
@@ -1919,6 +1924,23 @@ static const struct of_device_id arm_smmu_of_match[] = {
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
+static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
+ const char * const *clks)
+{
+ int i;
+
+ if (smmu->num_clks < 1)
+ return;
+
+ smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
+ sizeof(*smmu->clks), GFP_KERNEL);
+ if (!smmu->clks)
+ return;
+
+ for (i = 0; i < smmu->num_clks; i++)
+ smmu->clks[i].id = clks[i];
+}
+
#ifdef CONFIG_ACPI
static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
{
@@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
platform_device *pdev,
data = of_device_get_match_data(dev);
smmu->version = data->version;
smmu->model = data->model;
+ smmu->num_clks = data->num_clks;
+
+ arm_smmu_fill_clk_data(smmu, data->clks);
parse_driver_options(smmu);
@@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
smmu->irqs[i] = irq;
}
+ err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
+ if (err)
+ return err;
+
+ err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
+ if (err)
+ return err;
+
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;
@@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct platform_device
*pdev)
/* Turn the thing off */
writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
+
+ clk_bulk_unprepare(smmu->num_clks, smmu->clks);
+
return 0;
}
@@ -2197,7 +2233,27 @@ static int __maybe_unused arm_smmu_pm_resume(struct
device *dev)
return 0;
}
-static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
+static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
+{
+ struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+
+ return clk_bulk_enable(smmu->num_clks, smmu->clks);
+}
+
+static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
+{
+ struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+
+ clk_bulk_disable(smmu->num_clks, smmu->clks);
+
+ return 0;
+}
+
+static const struct dev_pm_ops arm_smmu_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(NULL, arm_smmu_pm_resume)
+ SET_RUNTIME_PM_OPS(arm_smmu_runtime_suspend,
+ arm_smmu_runtime_resume, NULL)
+};
static struct platform_driver arm_smmu_driver = {
.driver = {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
