2022-04-06 14:15:37

by Brian Norris

[permalink] [raw]
Subject: [RFC PATCH 2/2] PM / devfreq: rk3399_dmc: Block PMU during transitions

See the previous patch ("soc: rockchip: power-domain: Manage resource
conflicts with firmware") for a thorough explanation of the conflicts.
While ARM Trusted Firmware may be modifying memory controller and
power-domain states, we need to block the kernel's power-domain driver.

If the power-domain driver is disabled, there is no resource conflict
and this becomes a no-op.

Signed-off-by: Brian Norris <[email protected]>
---

drivers/devfreq/rk3399_dmc.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index e494d1497d60..daff40702615 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -21,6 +21,7 @@
#include <linux/rwsem.h>
#include <linux/suspend.h>

+#include <soc/rockchip/pm_domains.h>
#include <soc/rockchip/rk3399_grf.h>
#include <soc/rockchip/rockchip_sip.h>

@@ -93,6 +94,16 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,

mutex_lock(&dmcfreq->lock);

+ /*
+ * Ensure power-domain transitions don't interfere with ARM Trusted
+ * Firmware power-domain idling.
+ */
+ err = rockchip_pmu_block();
+ if (err) {
+ dev_err(dev, "Failed to block PMU: %d\n", err);
+ goto out_unlock;
+ }
+
/*
* Some idle parameters may be based on the DDR controller clock, which
* is half of the DDR frequency.
@@ -198,6 +209,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
dmcfreq->volt = target_volt;

out:
+ rockchip_pmu_unblock();
+out_unlock:
mutex_unlock(&dmcfreq->lock);
return err;
}
--
2.35.1.1094.g7c7d902a7c-goog


2022-04-06 14:52:05

by Doug Anderson

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] PM / devfreq: rk3399_dmc: Block PMU during transitions

Hi,

On Tue, Apr 5, 2022 at 6:49 PM Brian Norris <[email protected]> wrote:
>
> See the previous patch ("soc: rockchip: power-domain: Manage resource
> conflicts with firmware") for a thorough explanation of the conflicts.
> While ARM Trusted Firmware may be modifying memory controller and
> power-domain states, we need to block the kernel's power-domain driver.
>
> If the power-domain driver is disabled, there is no resource conflict
> and this becomes a no-op.
>
> Signed-off-by: Brian Norris <[email protected]>
> ---
>
> drivers/devfreq/rk3399_dmc.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)

Reviewed-by: Douglas Anderson <[email protected]>

2022-04-14 10:32:51

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] PM / devfreq: rk3399_dmc: Block PMU during transitions

Hi,

Am Donnerstag, 14. April 2022, 00:14:40 CEST schrieb Chanwoo Choi:
> On 22. 4. 6. 10:48, Brian Norris wrote:
> > See the previous patch ("soc: rockchip: power-domain: Manage resource
> > conflicts with firmware") for a thorough explanation of the conflicts.
> > While ARM Trusted Firmware may be modifying memory controller and
> > power-domain states, we need to block the kernel's power-domain driver.
> >
> > If the power-domain driver is disabled, there is no resource conflict
> > and this becomes a no-op.
> >
> > Signed-off-by: Brian Norris <[email protected]>
> > ---
> >
> > drivers/devfreq/rk3399_dmc.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
> > index e494d1497d60..daff40702615 100644
> > --- a/drivers/devfreq/rk3399_dmc.c
> > +++ b/drivers/devfreq/rk3399_dmc.c
> > @@ -21,6 +21,7 @@
> > #include <linux/rwsem.h>
> > #include <linux/suspend.h>
> >
> > +#include <soc/rockchip/pm_domains.h>
> > #include <soc/rockchip/rk3399_grf.h>
> > #include <soc/rockchip/rockchip_sip.h>
> >
> > @@ -93,6 +94,16 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
> >
> > mutex_lock(&dmcfreq->lock);
> >
> > + /*
> > + * Ensure power-domain transitions don't interfere with ARM Trusted
> > + * Firmware power-domain idling.
> > + */
> > + err = rockchip_pmu_block();
> > + if (err) {
> > + dev_err(dev, "Failed to block PMU: %d\n", err);
> > + goto out_unlock;
> > + }
> > +
> > /*
> > * Some idle parameters may be based on the DDR controller clock, which
> > * is half of the DDR frequency.
> > @@ -198,6 +209,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
> > dmcfreq->volt = target_volt;
> >
> > out:
> > + rockchip_pmu_unblock();
> > +out_unlock:
> > mutex_unlock(&dmcfreq->lock);
> > return err;
> > }
>
> Acked-by: Chanwoo Choi <[email protected]>

so I guess you're ok with me picking up both patches, right?
[Just making sure :-) ]

Thanks
Heiko


2022-04-15 18:47:35

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] PM / devfreq: rk3399_dmc: Block PMU during transitions

On 22. 4. 14. 07:45, Heiko Stübner wrote:
> Hi,
>
> Am Donnerstag, 14. April 2022, 00:14:40 CEST schrieb Chanwoo Choi:
>> On 22. 4. 6. 10:48, Brian Norris wrote:
>>> See the previous patch ("soc: rockchip: power-domain: Manage resource
>>> conflicts with firmware") for a thorough explanation of the conflicts.
>>> While ARM Trusted Firmware may be modifying memory controller and
>>> power-domain states, we need to block the kernel's power-domain driver.
>>>
>>> If the power-domain driver is disabled, there is no resource conflict
>>> and this becomes a no-op.
>>>
>>> Signed-off-by: Brian Norris <[email protected]>
>>> ---
>>>
>>> drivers/devfreq/rk3399_dmc.c | 13 +++++++++++++
>>> 1 file changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
>>> index e494d1497d60..daff40702615 100644
>>> --- a/drivers/devfreq/rk3399_dmc.c
>>> +++ b/drivers/devfreq/rk3399_dmc.c
>>> @@ -21,6 +21,7 @@
>>> #include <linux/rwsem.h>
>>> #include <linux/suspend.h>
>>>
>>> +#include <soc/rockchip/pm_domains.h>
>>> #include <soc/rockchip/rk3399_grf.h>
>>> #include <soc/rockchip/rockchip_sip.h>
>>>
>>> @@ -93,6 +94,16 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
>>>
>>> mutex_lock(&dmcfreq->lock);
>>>
>>> + /*
>>> + * Ensure power-domain transitions don't interfere with ARM Trusted
>>> + * Firmware power-domain idling.
>>> + */
>>> + err = rockchip_pmu_block();
>>> + if (err) {
>>> + dev_err(dev, "Failed to block PMU: %d\n", err);
>>> + goto out_unlock;
>>> + }
>>> +
>>> /*
>>> * Some idle parameters may be based on the DDR controller clock, which
>>> * is half of the DDR frequency.
>>> @@ -198,6 +209,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
>>> dmcfreq->volt = target_volt;
>>>
>>> out:
>>> + rockchip_pmu_unblock();
>>> +out_unlock:
>>> mutex_unlock(&dmcfreq->lock);
>>> return err;
>>> }
>>
>> Acked-by: Chanwoo Choi <[email protected]>
>
> so I guess you're ok with me picking up both patches, right?
> [Just making sure :-) ]

This patch have the dependency of latest devfreq-next branch.
So that need to make the immutable branch between rockchip and devfreq.

--
Best Regards,
Samsung Electronics
Chanwoo Choi

2022-04-16 00:30:58

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] PM / devfreq: rk3399_dmc: Block PMU during transitions

Hi Brian,

On 22. 4. 6. 10:48, Brian Norris wrote:
> See the previous patch ("soc: rockchip: power-domain: Manage resource
> conflicts with firmware") for a thorough explanation of the conflicts.
> While ARM Trusted Firmware may be modifying memory controller and
> power-domain states, we need to block the kernel's power-domain driver.
>
> If the power-domain driver is disabled, there is no resource conflict
> and this becomes a no-op.
>
> Signed-off-by: Brian Norris <[email protected]>
> ---
>
> drivers/devfreq/rk3399_dmc.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
> index e494d1497d60..daff40702615 100644
> --- a/drivers/devfreq/rk3399_dmc.c
> +++ b/drivers/devfreq/rk3399_dmc.c
> @@ -21,6 +21,7 @@
> #include <linux/rwsem.h>
> #include <linux/suspend.h>
>
> +#include <soc/rockchip/pm_domains.h>
> #include <soc/rockchip/rk3399_grf.h>
> #include <soc/rockchip/rockchip_sip.h>
>
> @@ -93,6 +94,16 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
>
> mutex_lock(&dmcfreq->lock);
>
> + /*
> + * Ensure power-domain transitions don't interfere with ARM Trusted
> + * Firmware power-domain idling.
> + */
> + err = rockchip_pmu_block();
> + if (err) {
> + dev_err(dev, "Failed to block PMU: %d\n", err);
> + goto out_unlock;
> + }
> +
> /*
> * Some idle parameters may be based on the DDR controller clock, which
> * is half of the DDR frequency.
> @@ -198,6 +209,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
> dmcfreq->volt = target_volt;
>
> out:
> + rockchip_pmu_unblock();
> +out_unlock:
> mutex_unlock(&dmcfreq->lock);
> return err;
> }

Acked-by: Chanwoo Choi <[email protected]>

--
Best Regards,
Samsung Electronics
Chanwoo Choi

2022-05-09 01:48:56

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] PM / devfreq: rk3399_dmc: Block PMU during transitions

On 22. 5. 9. 00:07, Heiko Stuebner wrote:
> Am Samstag, 7. Mai 2022, 16:21:59 CEST schrieb Chanwoo Choi:
>> On 22. 4. 14. 08:13, Chanwoo Choi wrote:
>>> On 22. 4. 14. 07:45, Heiko Stübner wrote:
>>>> Hi,
>>>>
>>>> Am Donnerstag, 14. April 2022, 00:14:40 CEST schrieb Chanwoo Choi:
>>>>> On 22. 4. 6. 10:48, Brian Norris wrote:
>>>>>> See the previous patch ("soc: rockchip: power-domain: Manage resource
>>>>>> conflicts with firmware") for a thorough explanation of the conflicts.
>>>>>> While ARM Trusted Firmware may be modifying memory controller and
>>>>>> power-domain states, we need to block the kernel's power-domain driver.
>>>>>>
>>>>>> If the power-domain driver is disabled, there is no resource conflict
>>>>>> and this becomes a no-op.
>>>>>>
>>>>>> Signed-off-by: Brian Norris <[email protected]>
>>>>>> ---
>>>>>>
>>>>>> drivers/devfreq/rk3399_dmc.c | 13 +++++++++++++
>>>>>> 1 file changed, 13 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/devfreq/rk3399_dmc.c
>>>>>> b/drivers/devfreq/rk3399_dmc.c
>>>>>> index e494d1497d60..daff40702615 100644
>>>>>> --- a/drivers/devfreq/rk3399_dmc.c
>>>>>> +++ b/drivers/devfreq/rk3399_dmc.c
>>>>>> @@ -21,6 +21,7 @@
>>>>>> #include <linux/rwsem.h>
>>>>>> #include <linux/suspend.h>
>>>>>> +#include <soc/rockchip/pm_domains.h>
>>>>>> #include <soc/rockchip/rk3399_grf.h>
>>>>>> #include <soc/rockchip/rockchip_sip.h>
>>>>>> @@ -93,6 +94,16 @@ static int rk3399_dmcfreq_target(struct device
>>>>>> *dev, unsigned long *freq,
>>>>>> mutex_lock(&dmcfreq->lock);
>>>>>> + /*
>>>>>> + * Ensure power-domain transitions don't interfere with ARM
>>>>>> Trusted
>>>>>> + * Firmware power-domain idling.
>>>>>> + */
>>>>>> + err = rockchip_pmu_block();
>>>>>> + if (err) {
>>>>>> + dev_err(dev, "Failed to block PMU: %d\n", err);
>>>>>> + goto out_unlock;
>>>>>> + }
>>>>>> +
>>>>>> /*
>>>>>> * Some idle parameters may be based on the DDR controller
>>>>>> clock, which
>>>>>> * is half of the DDR frequency.
>>>>>> @@ -198,6 +209,8 @@ static int rk3399_dmcfreq_target(struct device
>>>>>> *dev, unsigned long *freq,
>>>>>> dmcfreq->volt = target_volt;
>>>>>> out:
>>>>>> + rockchip_pmu_unblock();
>>>>>> +out_unlock:
>>>>>> mutex_unlock(&dmcfreq->lock);
>>>>>> return err;
>>>>>> }
>>>>>
>>>>> Acked-by: Chanwoo Choi <[email protected]>
>>>>
>>>> so I guess you're ok with me picking up both patches, right?
>>>> [Just making sure :-) ]
>>>
>>> This patch have the dependency of latest devfreq-next branch.
>>> So that need to make the immutable branch between rockchip and devfreq.
>>>
>>
>> Hi Heiko and Brian,
>>
>> Is there any other progress?
>>
>> IMHO, if rockchip maintainer reply the acked-by from patch1
>> and then agree these patches to be applied to devfreq.git,
>> I can take them.
>
> sounds good to me. Patch1 looks good and correct to me, so
> I've added a Reviewed-by for it and it defintily makes sense for
> both to go through the devfreq tree then, so we don't need
> additional stable-branches :-)

OK. I'll take them with your reviewed-by tag. Thanks.


--
Best Regards,
Samsung Electronics
Chanwoo Choi

2022-05-09 07:39:06

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] PM / devfreq: rk3399_dmc: Block PMU during transitions

On 22. 4. 14. 08:13, Chanwoo Choi wrote:
> On 22. 4. 14. 07:45, Heiko Stübner wrote:
>> Hi,
>>
>> Am Donnerstag, 14. April 2022, 00:14:40 CEST schrieb Chanwoo Choi:
>>> On 22. 4. 6. 10:48, Brian Norris wrote:
>>>> See the previous patch ("soc: rockchip: power-domain: Manage resource
>>>> conflicts with firmware") for a thorough explanation of the conflicts.
>>>> While ARM Trusted Firmware may be modifying memory controller and
>>>> power-domain states, we need to block the kernel's power-domain driver.
>>>>
>>>> If the power-domain driver is disabled, there is no resource conflict
>>>> and this becomes a no-op.
>>>>
>>>> Signed-off-by: Brian Norris <[email protected]>
>>>> ---
>>>>
>>>>    drivers/devfreq/rk3399_dmc.c | 13 +++++++++++++
>>>>    1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/drivers/devfreq/rk3399_dmc.c
>>>> b/drivers/devfreq/rk3399_dmc.c
>>>> index e494d1497d60..daff40702615 100644
>>>> --- a/drivers/devfreq/rk3399_dmc.c
>>>> +++ b/drivers/devfreq/rk3399_dmc.c
>>>> @@ -21,6 +21,7 @@
>>>>    #include <linux/rwsem.h>
>>>>    #include <linux/suspend.h>
>>>> +#include <soc/rockchip/pm_domains.h>
>>>>    #include <soc/rockchip/rk3399_grf.h>
>>>>    #include <soc/rockchip/rockchip_sip.h>
>>>> @@ -93,6 +94,16 @@ static int rk3399_dmcfreq_target(struct device
>>>> *dev, unsigned long *freq,
>>>>        mutex_lock(&dmcfreq->lock);
>>>> +    /*
>>>> +     * Ensure power-domain transitions don't interfere with ARM
>>>> Trusted
>>>> +     * Firmware power-domain idling.
>>>> +     */
>>>> +    err = rockchip_pmu_block();
>>>> +    if (err) {
>>>> +        dev_err(dev, "Failed to block PMU: %d\n", err);
>>>> +        goto out_unlock;
>>>> +    }
>>>> +
>>>>        /*
>>>>         * Some idle parameters may be based on the DDR controller
>>>> clock, which
>>>>         * is half of the DDR frequency.
>>>> @@ -198,6 +209,8 @@ static int rk3399_dmcfreq_target(struct device
>>>> *dev, unsigned long *freq,
>>>>        dmcfreq->volt = target_volt;
>>>>    out:
>>>> +    rockchip_pmu_unblock();
>>>> +out_unlock:
>>>>        mutex_unlock(&dmcfreq->lock);
>>>>        return err;
>>>>    }
>>>
>>> Acked-by: Chanwoo Choi <[email protected]>
>>
>> so I guess you're ok with me picking up both patches, right?
>> [Just making sure :-) ]
>
> This patch have the dependency of latest devfreq-next branch.
> So that need to make the immutable branch between rockchip and devfreq.
>

Hi Heiko and Brian,

Is there any other progress?

IMHO, if rockchip maintainer reply the acked-by from patch1
and then agree these patches to be applied to devfreq.git,
I can take them.

--
Best Regards,
Samsung Electronics
Chanwoo Choi

2022-05-09 07:44:10

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [RFC PATCH 2/2] PM / devfreq: rk3399_dmc: Block PMU during transitions

Am Samstag, 7. Mai 2022, 16:21:59 CEST schrieb Chanwoo Choi:
> On 22. 4. 14. 08:13, Chanwoo Choi wrote:
> > On 22. 4. 14. 07:45, Heiko St?bner wrote:
> >> Hi,
> >>
> >> Am Donnerstag, 14. April 2022, 00:14:40 CEST schrieb Chanwoo Choi:
> >>> On 22. 4. 6. 10:48, Brian Norris wrote:
> >>>> See the previous patch ("soc: rockchip: power-domain: Manage resource
> >>>> conflicts with firmware") for a thorough explanation of the conflicts.
> >>>> While ARM Trusted Firmware may be modifying memory controller and
> >>>> power-domain states, we need to block the kernel's power-domain driver.
> >>>>
> >>>> If the power-domain driver is disabled, there is no resource conflict
> >>>> and this becomes a no-op.
> >>>>
> >>>> Signed-off-by: Brian Norris <[email protected]>
> >>>> ---
> >>>>
> >>>> drivers/devfreq/rk3399_dmc.c | 13 +++++++++++++
> >>>> 1 file changed, 13 insertions(+)
> >>>>
> >>>> diff --git a/drivers/devfreq/rk3399_dmc.c
> >>>> b/drivers/devfreq/rk3399_dmc.c
> >>>> index e494d1497d60..daff40702615 100644
> >>>> --- a/drivers/devfreq/rk3399_dmc.c
> >>>> +++ b/drivers/devfreq/rk3399_dmc.c
> >>>> @@ -21,6 +21,7 @@
> >>>> #include <linux/rwsem.h>
> >>>> #include <linux/suspend.h>
> >>>> +#include <soc/rockchip/pm_domains.h>
> >>>> #include <soc/rockchip/rk3399_grf.h>
> >>>> #include <soc/rockchip/rockchip_sip.h>
> >>>> @@ -93,6 +94,16 @@ static int rk3399_dmcfreq_target(struct device
> >>>> *dev, unsigned long *freq,
> >>>> mutex_lock(&dmcfreq->lock);
> >>>> + /*
> >>>> + * Ensure power-domain transitions don't interfere with ARM
> >>>> Trusted
> >>>> + * Firmware power-domain idling.
> >>>> + */
> >>>> + err = rockchip_pmu_block();
> >>>> + if (err) {
> >>>> + dev_err(dev, "Failed to block PMU: %d\n", err);
> >>>> + goto out_unlock;
> >>>> + }
> >>>> +
> >>>> /*
> >>>> * Some idle parameters may be based on the DDR controller
> >>>> clock, which
> >>>> * is half of the DDR frequency.
> >>>> @@ -198,6 +209,8 @@ static int rk3399_dmcfreq_target(struct device
> >>>> *dev, unsigned long *freq,
> >>>> dmcfreq->volt = target_volt;
> >>>> out:
> >>>> + rockchip_pmu_unblock();
> >>>> +out_unlock:
> >>>> mutex_unlock(&dmcfreq->lock);
> >>>> return err;
> >>>> }
> >>>
> >>> Acked-by: Chanwoo Choi <[email protected]>
> >>
> >> so I guess you're ok with me picking up both patches, right?
> >> [Just making sure :-) ]
> >
> > This patch have the dependency of latest devfreq-next branch.
> > So that need to make the immutable branch between rockchip and devfreq.
> >
>
> Hi Heiko and Brian,
>
> Is there any other progress?
>
> IMHO, if rockchip maintainer reply the acked-by from patch1
> and then agree these patches to be applied to devfreq.git,
> I can take them.

sounds good to me. Patch1 looks good and correct to me, so
I've added a Reviewed-by for it and it defintily makes sense for
both to go through the devfreq tree then, so we don't need
additional stable-branches :-)

Thanks
Heiko