From: Paul Burton <paulburton@kernel.org>
In a multi-cluster MIPS system, there are multiple GICs - one in each
cluster - each of which has its independent counter. The counters in
each GIC are not synchronized in any way, so they can drift relative
to one another through the lifetime of the system. This is problematic
for a clock source which ought to be global.
Avoid problems by always accessing cluster 0's counter, using
cross-cluster register access. This adds overhead so it is applied only
on multi-cluster systems.
Signed-off-by: Paul Burton <paulburton@kernel.org>
Signed-off-by: Chao-ying Fu <cfu@wavecomp.com>
Signed-off-by: Dragan Mladjenovic <dragan.mladjenovic@syrmia.com>
Signed-off-by: Aleksandar Rikalo <arikalo@gmail.com>
Tested-by: Serge Semin <fancer.lancer@gmail.com>
---
drivers/clocksource/mips-gic-timer.c | 39 +++++++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index 110347707ff9..7907b740497a 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -166,6 +166,37 @@ static u64 gic_hpt_read(struct clocksource *cs)
return gic_read_count();
}
+static u64 gic_hpt_read_multicluster(struct clocksource *cs)
+{
+ unsigned int hi, hi2, lo;
+ u64 count;
+
+ mips_cm_lock_other(0, 0, 0, CM_GCR_Cx_OTHER_BLOCK_GLOBAL);
+
+ if (mips_cm_is64) {
+ count = read_gic_redir_counter();
+ goto out;
+ }
+
+ hi = read_gic_redir_counter_32h();
+ while (true) {
+ lo = read_gic_redir_counter_32l();
+
+ /* If hi didn't change then lo didn't wrap & we're done */
+ hi2 = read_gic_redir_counter_32h();
+ if (hi2 == hi)
+ break;
+
+ /* Otherwise, repeat with the latest hi value */
+ hi = hi2;
+ }
+
+ count = (((u64)hi) << 32) + lo;
+out:
+ mips_cm_unlock_other();
+ return count;
+}
+
static struct clocksource gic_clocksource = {
.name = "GIC",
.read = gic_hpt_read,
@@ -203,6 +234,11 @@ static int __init __gic_clocksource_init(void)
gic_clocksource.rating = 200;
gic_clocksource.rating += clamp(gic_frequency / 10000000, 0, 99);
+ if (mips_cps_multicluster_cpus()) {
+ gic_clocksource.read = &gic_hpt_read_multicluster;
+ gic_clocksource.vdso_clock_mode = VDSO_CLOCKMODE_NONE;
+ }
+
ret = clocksource_register_hz(&gic_clocksource, gic_frequency);
if (ret < 0)
pr_warn("Unable to register clocksource\n");
@@ -261,7 +297,8 @@ static int __init gic_clocksource_of_init(struct device_node *node)
* stable CPU frequency or on the platforms with CM3 and CPU frequency
* change performed by the CPC core clocks divider.
*/
- if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ)) {
+ if ((mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ)) &&
+ !mips_cps_multicluster_cpus()) {
sched_clock_register(mips_cm_is64 ?
gic_read_count_64 : gic_read_count_2x32,
gic_count_width, gic_frequency);
--
2.25.1On 19/10/2024 09:10, Aleksandar Rikalo wrote: > From: Paul Burton <paulburton@kernel.org> > > In a multi-cluster MIPS system, there are multiple GICs - one in each > cluster - each of which has its independent counter. The counters in > each GIC are not synchronized in any way, so they can drift relative > to one another through the lifetime of the system. This is problematic > for a clock source which ought to be global. > > Avoid problems by always accessing cluster 0's counter, using > cross-cluster register access. This adds overhead so it is applied only > on multi-cluster systems. > > Signed-off-by: Paul Burton <paulburton@kernel.org> > Signed-off-by: Chao-ying Fu <cfu@wavecomp.com> > Signed-off-by: Dragan Mladjenovic <dragan.mladjenovic@syrmia.com> > Signed-off-by: Aleksandar Rikalo <arikalo@gmail.com> > Tested-by: Serge Semin <fancer.lancer@gmail.com> > --- Applied, thanks -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
On 19/10/2024 09:10, Aleksandar Rikalo wrote: > From: Paul Burton <paulburton@kernel.org> > > In a multi-cluster MIPS system, there are multiple GICs - one in each > cluster - each of which has its independent counter. The counters in > each GIC are not synchronized in any way, so they can drift relative > to one another through the lifetime of the system. This is problematic > for a clock source which ought to be global. > > Avoid problems by always accessing cluster 0's counter, using > cross-cluster register access. This adds overhead so it is applied only > on multi-cluster systems. > > Signed-off-by: Paul Burton <paulburton@kernel.org> > Signed-off-by: Chao-ying Fu <cfu@wavecomp.com> > Signed-off-by: Dragan Mladjenovic <dragan.mladjenovic@syrmia.com> > Signed-off-by: Aleksandar Rikalo <arikalo@gmail.com> > Tested-by: Serge Semin <fancer.lancer@gmail.com> > --- May I take this patch through the clocksource tree ? -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
On Mon, Oct 28, 2024 at 03:54:48PM +0100, Daniel Lezcano wrote: > On 19/10/2024 09:10, Aleksandar Rikalo wrote: > > From: Paul Burton <paulburton@kernel.org> > > > > In a multi-cluster MIPS system, there are multiple GICs - one in each > > cluster - each of which has its independent counter. The counters in > > each GIC are not synchronized in any way, so they can drift relative > > to one another through the lifetime of the system. This is problematic > > for a clock source which ought to be global. > > > > Avoid problems by always accessing cluster 0's counter, using > > cross-cluster register access. This adds overhead so it is applied only > > on multi-cluster systems. > > > > Signed-off-by: Paul Burton <paulburton@kernel.org> > > Signed-off-by: Chao-ying Fu <cfu@wavecomp.com> > > Signed-off-by: Dragan Mladjenovic <dragan.mladjenovic@syrmia.com> > > Signed-off-by: Aleksandar Rikalo <arikalo@gmail.com> > > Tested-by: Serge Semin <fancer.lancer@gmail.com> > > --- > > May I take this patch through the clocksource tree ? sure, should be the best option. Thomas. -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea. [ RFC1925, 2.3 ]
On 28/10/2024 16:15, Thomas Bogendoerfer wrote: > On Mon, Oct 28, 2024 at 03:54:48PM +0100, Daniel Lezcano wrote: >> On 19/10/2024 09:10, Aleksandar Rikalo wrote: >>> From: Paul Burton <paulburton@kernel.org> >>> >>> In a multi-cluster MIPS system, there are multiple GICs - one in each >>> cluster - each of which has its independent counter. The counters in >>> each GIC are not synchronized in any way, so they can drift relative >>> to one another through the lifetime of the system. This is problematic >>> for a clock source which ought to be global. >>> >>> Avoid problems by always accessing cluster 0's counter, using >>> cross-cluster register access. This adds overhead so it is applied only >>> on multi-cluster systems. >>> >>> Signed-off-by: Paul Burton <paulburton@kernel.org> >>> Signed-off-by: Chao-ying Fu <cfu@wavecomp.com> >>> Signed-off-by: Dragan Mladjenovic <dragan.mladjenovic@syrmia.com> >>> Signed-off-by: Aleksandar Rikalo <arikalo@gmail.com> >>> Tested-by: Serge Semin <fancer.lancer@gmail.com> >>> --- >> >> May I take this patch through the clocksource tree ? > > sure, should be the best option. Ok, thanks Can you add your tag ? -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
On Mon, Oct 28, 2024 at 04:22:55PM +0100, Daniel Lezcano wrote: > On 28/10/2024 16:15, Thomas Bogendoerfer wrote: > > On Mon, Oct 28, 2024 at 03:54:48PM +0100, Daniel Lezcano wrote: > > > On 19/10/2024 09:10, Aleksandar Rikalo wrote: > > > > From: Paul Burton <paulburton@kernel.org> > > > > > > > > In a multi-cluster MIPS system, there are multiple GICs - one in each > > > > cluster - each of which has its independent counter. The counters in > > > > each GIC are not synchronized in any way, so they can drift relative > > > > to one another through the lifetime of the system. This is problematic > > > > for a clock source which ought to be global. > > > > > > > > Avoid problems by always accessing cluster 0's counter, using > > > > cross-cluster register access. This adds overhead so it is applied only > > > > on multi-cluster systems. > > > > > > > > Signed-off-by: Paul Burton <paulburton@kernel.org> > > > > Signed-off-by: Chao-ying Fu <cfu@wavecomp.com> > > > > Signed-off-by: Dragan Mladjenovic <dragan.mladjenovic@syrmia.com> > > > > Signed-off-by: Aleksandar Rikalo <arikalo@gmail.com> > > > > Tested-by: Serge Semin <fancer.lancer@gmail.com> > > > > --- > > > > > > May I take this patch through the clocksource tree ? > > > > sure, should be the best option. > > Ok, thanks > > Can you add your tag ? it's only touching drivers/clocksource, but if you want Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de> -- Crap can work. Given enough thrust pigs will fly, but it's not necessarily a good idea. [ RFC1925, 2.3 ]
The following commit has been merged into the timers/core branch of tip:
Commit-ID: dfe101bcad840d025deb5e43150d54050ab7724d
Gitweb: https://git.kernel.org/tip/dfe101bcad840d025deb5e43150d54050ab7724d
Author: Paul Burton <paulburton@kernel.org>
AuthorDate: Sat, 19 Oct 2024 09:10:30 +02:00
Committer: Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Wed, 13 Nov 2024 13:49:33 +01:00
clocksource/drivers/mips-gic-timer: Always use cluster 0 counter as clocksource
In a multi-cluster MIPS system, there are multiple GICs - one in each
cluster - each of which has its independent counter. The counters in
each GIC are not synchronized in any way, so they can drift relative
to one another through the lifetime of the system. This is problematic
for a clock source which ought to be global.
Avoid problems by always accessing cluster 0's counter, using
cross-cluster register access. This adds overhead so it is applied only
on multi-cluster systems.
Signed-off-by: Paul Burton <paulburton@kernel.org>
Signed-off-by: Chao-ying Fu <cfu@wavecomp.com>
Signed-off-by: Dragan Mladjenovic <dragan.mladjenovic@syrmia.com>
Signed-off-by: Aleksandar Rikalo <arikalo@gmail.com>
Tested-by: Serge Semin <fancer.lancer@gmail.com>
Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Tested-by: Gregory CLEMENT <gregory.clement@bootlin.com>
Link: https://lore.kernel.org/r/20241019071037.145314-6-arikalo@gmail.com
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/clocksource/mips-gic-timer.c | 39 ++++++++++++++++++++++++++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index 1103477..7907b74 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -166,6 +166,37 @@ static u64 gic_hpt_read(struct clocksource *cs)
return gic_read_count();
}
+static u64 gic_hpt_read_multicluster(struct clocksource *cs)
+{
+ unsigned int hi, hi2, lo;
+ u64 count;
+
+ mips_cm_lock_other(0, 0, 0, CM_GCR_Cx_OTHER_BLOCK_GLOBAL);
+
+ if (mips_cm_is64) {
+ count = read_gic_redir_counter();
+ goto out;
+ }
+
+ hi = read_gic_redir_counter_32h();
+ while (true) {
+ lo = read_gic_redir_counter_32l();
+
+ /* If hi didn't change then lo didn't wrap & we're done */
+ hi2 = read_gic_redir_counter_32h();
+ if (hi2 == hi)
+ break;
+
+ /* Otherwise, repeat with the latest hi value */
+ hi = hi2;
+ }
+
+ count = (((u64)hi) << 32) + lo;
+out:
+ mips_cm_unlock_other();
+ return count;
+}
+
static struct clocksource gic_clocksource = {
.name = "GIC",
.read = gic_hpt_read,
@@ -203,6 +234,11 @@ static int __init __gic_clocksource_init(void)
gic_clocksource.rating = 200;
gic_clocksource.rating += clamp(gic_frequency / 10000000, 0, 99);
+ if (mips_cps_multicluster_cpus()) {
+ gic_clocksource.read = &gic_hpt_read_multicluster;
+ gic_clocksource.vdso_clock_mode = VDSO_CLOCKMODE_NONE;
+ }
+
ret = clocksource_register_hz(&gic_clocksource, gic_frequency);
if (ret < 0)
pr_warn("Unable to register clocksource\n");
@@ -261,7 +297,8 @@ static int __init gic_clocksource_of_init(struct device_node *node)
* stable CPU frequency or on the platforms with CM3 and CPU frequency
* change performed by the CPC core clocks divider.
*/
- if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ)) {
+ if ((mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ)) &&
+ !mips_cps_multicluster_cpus()) {
sched_clock_register(mips_cm_is64 ?
gic_read_count_64 : gic_read_count_2x32,
gic_count_width, gic_frequency);© 2016 - 2024 Red Hat, Inc.