diff mbox series

[RFC,2/2] PM / devfreq: rk3399_dmc: Block PMU during transitions

Message ID 20220405184816.RFC.2.I2d73b403944f0b8b5871a77585b73f31ccc62999@changeid
State Accepted
Commit 2e691421a2c9e0462175fe98171afa632861b199
Headers show
Series rockchip / devfreq: Coordinate DRAM controller resources between ATF and kernel | expand

Commit Message

Brian Norris April 6, 2022, 1:48 a.m. UTC
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 <briannorris@chromium.org>
---

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

Comments

Doug Anderson April 6, 2022, 2:26 a.m. UTC | #1
Hi,

On Tue, Apr 5, 2022 at 6:49 PM Brian Norris <briannorris@chromium.org> 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 <briannorris@chromium.org>
> ---
>
>  drivers/devfreq/rk3399_dmc.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Chanwoo Choi April 13, 2022, 10:14 p.m. UTC | #2
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 <briannorris@chromium.org>
> ---
> 
>   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 <cw00.choi@samsung.com>
Heiko Stuebner April 13, 2022, 10:45 p.m. UTC | #3
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 <briannorris@chromium.org>
> > ---
> > 
> >   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 <cw00.choi@samsung.com>

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

Thanks
Heiko
Chanwoo Choi April 13, 2022, 11:13 p.m. UTC | #4
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 <briannorris@chromium.org>
>>> ---
>>>
>>>    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 <cw00.choi@samsung.com>
> 
> 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.
Chanwoo Choi May 7, 2022, 2:21 p.m. UTC | #5
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 <briannorris@chromium.org>
>>>> ---
>>>>
>>>>    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 <cw00.choi@samsung.com>
>>
>> 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.
Chanwoo Choi May 8, 2022, 6:42 p.m. UTC | #6
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 <briannorris@chromium.org>
>>>>>> ---
>>>>>>
>>>>>>     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 <cw00.choi@samsung.com>
>>>>
>>>> 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.
diff mbox series

Patch

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;
 }