| Message ID | 20241019071037.145314-6-arikalo@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: Support I6500 multi-cluster configuration | expand |
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 ?
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.
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 ?
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>
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> > --- Applied, thanks
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);