| 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 |
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>
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>
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
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.
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.
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 --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; }
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(+)