LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Oliver Upton <oupton@google.com>
To: Marc Zyngier <maz@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Mark Rutland <mark.rutland@arm.com>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Thomas Gleixner <tglx@linutronix.de>,
Peter Shier <pshier@google.com>,
Raghavendra Rao Ananta <rananta@google.com>,
Ricardo Koller <ricarkol@google.com>,
Will Deacon <will@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Linus Walleij <linus.walleij@linaro.org>,
kernel-team@android.com
Subject: Re: [PATCH 03/13] clocksource/arm_arch_timer: Move system register timer programming over to CVAL
Date: Wed, 11 Aug 2021 00:15:56 -0700 [thread overview]
Message-ID: <CAOQ_QsiV=RaYMsYou8TcTf2ePtS0g1jgqpAHA7bni8MaPtL8zA@mail.gmail.com> (raw)
In-Reply-To: <20210809152651.2297337-4-maz@kernel.org>
On Mon, Aug 9, 2021 at 8:27 AM Marc Zyngier <maz@kernel.org> wrote:
>
> In order to cope better with high frequency counters, move the
> programming of the timers from the countdown timer (TVAL) over
> to the comparator (CVAL).
>
> The programming model is slightly different, as we now need to
> read the current counter value to have an absolute deadline
> instead of a relative one.
>
> There is a small overhead to this change, which we will address
> in the following patches.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Oliver Upton <oupton@google.com>
> ---
> arch/arm/include/asm/arch_timer.h | 14 ++++++++----
> arch/arm64/include/asm/arch_timer.h | 16 +++++++++-----
> drivers/clocksource/arm_arch_timer.c | 32 +++++++++++++++++++++++++---
> include/clocksource/arm_arch_timer.h | 1 +
> 4 files changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
> index 88075c7f4bfd..b48de9d26f27 100644
> --- a/arch/arm/include/asm/arch_timer.h
> +++ b/arch/arm/include/asm/arch_timer.h
> @@ -31,18 +31,22 @@ void arch_timer_reg_write_cp15(int access, enum arch_timer_reg reg, u64 val)
> case ARCH_TIMER_REG_CTRL:
> asm volatile("mcr p15, 0, %0, c14, c2, 1" : : "r" ((u32)val));
> break;
> - case ARCH_TIMER_REG_TVAL:
> - asm volatile("mcr p15, 0, %0, c14, c2, 0" : : "r" ((u32)val));
> + case ARCH_TIMER_REG_CVAL:
> + asm volatile("mcrr p15, 2, %Q0, %R0, c14" : : "r" (val));
> break;
> + case ARCH_TIMER_REG_TVAL:
> + BUG();
> }
> } else if (access == ARCH_TIMER_VIRT_ACCESS) {
> switch (reg) {
> case ARCH_TIMER_REG_CTRL:
> asm volatile("mcr p15, 0, %0, c14, c3, 1" : : "r" ((u32)val));
> break;
> - case ARCH_TIMER_REG_TVAL:
> - asm volatile("mcr p15, 0, %0, c14, c3, 0" : : "r" ((u32)val));
> + case ARCH_TIMER_REG_CVAL:
> + asm volatile("mcrr p15, 3, %Q0, %R0, c14" : : "r" (val));
> break;
> + case ARCH_TIMER_REG_TVAL:
> + BUG();
> }
> }
>
> @@ -60,6 +64,7 @@ u32 arch_timer_reg_read_cp15(int access, enum arch_timer_reg reg)
> asm volatile("mrc p15, 0, %0, c14, c2, 1" : "=r" (val));
> break;
> case ARCH_TIMER_REG_TVAL:
> + case ARCH_TIMER_REG_CVAL:
> BUG();
> }
> } else if (access == ARCH_TIMER_VIRT_ACCESS) {
> @@ -68,6 +73,7 @@ u32 arch_timer_reg_read_cp15(int access, enum arch_timer_reg reg)
> asm volatile("mrc p15, 0, %0, c14, c3, 1" : "=r" (val));
> break;
> case ARCH_TIMER_REG_TVAL:
> + case ARCH_TIMER_REG_CVAL:
> BUG();
> }
> }
> diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
> index 107afb721749..6ceb050ae7b9 100644
> --- a/arch/arm64/include/asm/arch_timer.h
> +++ b/arch/arm64/include/asm/arch_timer.h
> @@ -96,18 +96,22 @@ void arch_timer_reg_write_cp15(int access, enum arch_timer_reg reg, u64 val)
> case ARCH_TIMER_REG_CTRL:
> write_sysreg(val, cntp_ctl_el0);
> break;
> - case ARCH_TIMER_REG_TVAL:
> - write_sysreg(val, cntp_tval_el0);
> + case ARCH_TIMER_REG_CVAL:
> + write_sysreg(val, cntp_cval_el0);
> break;
> + case ARCH_TIMER_REG_TVAL:
> + BUG();
> }
> } else if (access == ARCH_TIMER_VIRT_ACCESS) {
> switch (reg) {
> case ARCH_TIMER_REG_CTRL:
> write_sysreg(val, cntv_ctl_el0);
> break;
> - case ARCH_TIMER_REG_TVAL:
> - write_sysreg(val, cntv_tval_el0);
> + case ARCH_TIMER_REG_CVAL:
> + write_sysreg(val, cntv_cval_el0);
> break;
> + case ARCH_TIMER_REG_TVAL:
> + BUG();
> }
> }
>
> @@ -115,13 +119,14 @@ void arch_timer_reg_write_cp15(int access, enum arch_timer_reg reg, u64 val)
> }
>
> static __always_inline
> -u32 arch_timer_reg_read_cp15(int access, enum arch_timer_reg reg)
> +u64 arch_timer_reg_read_cp15(int access, enum arch_timer_reg reg)
> {
> if (access == ARCH_TIMER_PHYS_ACCESS) {
> switch (reg) {
> case ARCH_TIMER_REG_CTRL:
> return read_sysreg(cntp_ctl_el0);
> case ARCH_TIMER_REG_TVAL:
> + case ARCH_TIMER_REG_CVAL:
> break;
> }
> } else if (access == ARCH_TIMER_VIRT_ACCESS) {
> @@ -129,6 +134,7 @@ u32 arch_timer_reg_read_cp15(int access, enum arch_timer_reg reg)
> case ARCH_TIMER_REG_CTRL:
> return read_sysreg(cntv_ctl_el0);
> case ARCH_TIMER_REG_TVAL:
> + case ARCH_TIMER_REG_CVAL:
> break;
> }
> }
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 0b2bac3ef7ce..898a07dc01cd 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -112,6 +112,8 @@ void arch_timer_reg_write(int access, enum arch_timer_reg reg, u64 val,
> case ARCH_TIMER_REG_TVAL:
> writel_relaxed((u32)val, timer->base + CNTP_TVAL);
> break;
> + case ARCH_TIMER_REG_CVAL:
> + BUG();
> }
> } else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
> struct arch_timer *timer = to_arch_timer(clk);
> @@ -122,6 +124,8 @@ void arch_timer_reg_write(int access, enum arch_timer_reg reg, u64 val,
> case ARCH_TIMER_REG_TVAL:
> writel_relaxed((u32)val, timer->base + CNTV_TVAL);
> break;
> + case ARCH_TIMER_REG_CVAL:
> + BUG();
> }
> } else {
> arch_timer_reg_write_cp15(access, reg, val);
> @@ -141,6 +145,7 @@ u32 arch_timer_reg_read(int access, enum arch_timer_reg reg,
> val = readl_relaxed(timer->base + CNTP_CTL);
> break;
> case ARCH_TIMER_REG_TVAL:
> + case ARCH_TIMER_REG_CVAL:
> BUG();
> }
> } else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
> @@ -150,6 +155,7 @@ u32 arch_timer_reg_read(int access, enum arch_timer_reg reg,
> val = readl_relaxed(timer->base + CNTV_CTL);
> break;
> case ARCH_TIMER_REG_TVAL:
> + case ARCH_TIMER_REG_CVAL:
> BUG();
> }
> } else {
> @@ -687,10 +693,18 @@ static __always_inline void set_next_event(const int access, unsigned long evt,
> struct clock_event_device *clk)
> {
> unsigned long ctrl;
> + u64 cnt;
> +
> ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
> ctrl |= ARCH_TIMER_CTRL_ENABLE;
> ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
> - arch_timer_reg_write(access, ARCH_TIMER_REG_TVAL, evt, clk);
> +
> + if (access == ARCH_TIMER_PHYS_ACCESS)
> + cnt = __arch_counter_get_cntpct();
> + else
> + cnt = __arch_counter_get_cntvct();
> +
> + arch_timer_reg_write(access, ARCH_TIMER_REG_CVAL, evt + cnt, clk);
> arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
> }
>
> @@ -708,17 +722,29 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
> return 0;
> }
>
> +static __always_inline void set_next_event_mem(const int access, unsigned long evt,
> + struct clock_event_device *clk)
> +{
> + unsigned long ctrl;
> + ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
> + ctrl |= ARCH_TIMER_CTRL_ENABLE;
> + ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
> +
> + arch_timer_reg_write(access, ARCH_TIMER_REG_TVAL, evt, clk);
> + arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
> +}
> +
> static int arch_timer_set_next_event_virt_mem(unsigned long evt,
> struct clock_event_device *clk)
> {
> - set_next_event(ARCH_TIMER_MEM_VIRT_ACCESS, evt, clk);
> + set_next_event_mem(ARCH_TIMER_MEM_VIRT_ACCESS, evt, clk);
> return 0;
> }
>
> static int arch_timer_set_next_event_phys_mem(unsigned long evt,
> struct clock_event_device *clk)
> {
> - set_next_event(ARCH_TIMER_MEM_PHYS_ACCESS, evt, clk);
> + set_next_event_mem(ARCH_TIMER_MEM_PHYS_ACCESS, evt, clk);
> return 0;
> }
>
> diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
> index 73c7139c866f..d59537afb29d 100644
> --- a/include/clocksource/arm_arch_timer.h
> +++ b/include/clocksource/arm_arch_timer.h
> @@ -25,6 +25,7 @@
> enum arch_timer_reg {
> ARCH_TIMER_REG_CTRL,
> ARCH_TIMER_REG_TVAL,
> + ARCH_TIMER_REG_CVAL,
> };
>
> enum arch_timer_ppi_nr {
> --
> 2.30.2
>
next prev parent reply other threads:[~2021-08-11 7:16 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-09 15:26 [PATCH 00/13] clocksource/arm_arch_timer: Add basic ARMv8.6 support Marc Zyngier
2021-08-09 15:26 ` [PATCH 01/13] clocksource/arm_arch_timer: Drop CNT*_TVAL read accessors Marc Zyngier
2021-08-11 7:02 ` Oliver Upton
2021-08-24 16:20 ` Mark Rutland
2021-08-09 15:26 ` [PATCH 02/13] clocksource/arm_arch_timer: Extend write side of timer register accessors to u64 Marc Zyngier
2021-08-09 16:12 ` Oliver Upton
2021-08-24 16:20 ` Mark Rutland
2021-08-09 15:26 ` [PATCH 03/13] clocksource/arm_arch_timer: Move system register timer programming over to CVAL Marc Zyngier
2021-08-11 7:15 ` Oliver Upton [this message]
2021-08-24 16:21 ` Mark Rutland
2021-08-09 15:26 ` [PATCH 04/13] clocksource/arm_arch_timer: Move drop _tval from erratum function names Marc Zyngier
2021-08-09 16:16 ` Oliver Upton
2021-08-24 16:29 ` Mark Rutland
2021-08-09 15:26 ` [PATCH 05/13] clocksource/arm_arch_timer: Fix MMIO base address vs callback ordering issue Marc Zyngier
2021-08-09 16:52 ` Oliver Upton
2021-08-10 8:27 ` Marc Zyngier
2021-08-24 16:44 ` Mark Rutland
2021-08-09 15:26 ` [PATCH 06/13] clocksource/arm_arch_timer: Move MMIO timer programming over to CVAL Marc Zyngier
2021-08-09 15:26 ` [PATCH 07/13] clocksource/arm_arch_timer: Advertise 56bit timer to the core code Marc Zyngier
2021-08-09 15:26 ` [PATCH 08/13] clocksource/arm_arch_timer: Work around broken CVAL implementations Marc Zyngier
2021-08-10 12:34 ` Mark Rutland
2021-08-10 13:15 ` Marc Zyngier
2021-08-09 15:26 ` [PATCH 09/13] clocksource/arm_arch_timer: Remove any trace of the TVAL programming interface Marc Zyngier
2021-08-09 15:26 ` [PATCH 10/13] clocksource/arm_arch_timer: Drop unnecessary ISB on CVAL programming Marc Zyngier
2021-08-09 15:26 ` [PATCH 11/13] clocksource/arm_arch_timer: Fix masking for high freq counters Marc Zyngier
2021-08-09 16:45 ` Oliver Upton
2021-08-10 8:40 ` Marc Zyngier
2021-08-10 9:09 ` Oliver Upton
2021-08-09 15:26 ` [PATCH 12/13] arm64: Add a capability for FEAT_EVC Marc Zyngier
2021-08-09 16:30 ` Oliver Upton
2021-08-09 16:34 ` Oliver Upton
2021-08-09 18:02 ` Marc Zyngier
2021-08-09 18:21 ` Oliver Upton
2021-08-09 18:23 ` Oliver Upton
2021-08-09 15:26 ` [PATCH 13/13] arm64: Add CNT{P,V}CTSS_EL0 alternatives to cnt{p,v}ct_el0 Marc Zyngier
2021-08-09 16:42 ` Oliver Upton
2021-08-09 18:11 ` Marc Zyngier
2021-08-09 18:17 ` Oliver Upton
2021-08-10 7:59 ` Marc Zyngier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAOQ_QsiV=RaYMsYou8TcTf2ePtS0g1jgqpAHA7bni8MaPtL8zA@mail.gmail.com' \
--to=oupton@google.com \
--cc=catalin.marinas@arm.com \
--cc=daniel.lezcano@linaro.org \
--cc=kernel-team@android.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=maz@kernel.org \
--cc=pshier@google.com \
--cc=rananta@google.com \
--cc=ricarkol@google.com \
--cc=tglx@linutronix.de \
--cc=will@kernel.org \
--subject='Re: [PATCH 03/13] clocksource/arm_arch_timer: Move system register timer programming over to CVAL' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).