[PATCH] dmtimer posting

Woodruff, Richard
2008-03-19T20:00:06Z

[-- Attachment #1: Type: text/plain, Size: 4791 bytes --]

Hi,

I've fixed the bits Kevin pointed out and ran checkpatch.pl and removed
the reset of the errors.

This patch adds the use of write posting for the timer. Previously,
every write could lock the requestor for almost 3x32KHz cycles. This
now only synchronizes before writes and reads instead of after them and
it does it on a register per register basis. Doing it this way there is
some chance to hide some of the sync latency. It also removes some
needless reads when non-posted mode is there. With out this fix the
read/writes talk almost 2% CPU load @500MHz just waiting on tick timer
registers.

Signed-off-by: Richard Woodruff <r-woodruff2@ti.com>

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 302ad8d..e218ebd 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -67,6 +67,29 @@
#define OMAP_TIMER_CTRL_AR (1 << 1) /* auto-reload
enable */
#define OMAP_TIMER_CTRL_ST (1 << 0) /* start timer
*/

+/* Write posting bit shift table
+ * - Values are register shift for TWPS status bits
+ * - Use of this avoids 32KHz Synchronization penalties. The cost is
almost
+ * a 3x32KHz stall. This is comapred to a 166MHz posted access.
+ * - A value of '15' is a shift which always returns 0
+ * for registers which are not postable
+ *
+ * Note: Faster code can be generated if a larger array is used.
+ */
+#define PROW 6
+#define PCOL 4
+#define WPINDEX(off) (wpost[off >> 4][(off & 0xf) >> 2])
+
+static unsigned char wpost[PROW][PCOL] = {
+ /* 0, 4, 8, c */
+/* 00 */ {15, 15, 15, 15},
+/* 10 */ {15, 15, 15, 15},
+/* 20 */ {15, 0, 1, 2},
+/* 30 */ { 3, 15, 4, 15},
+/* 40 */ {15, 15, 5, 06},
+/* 50 */ { 7, 8, 9, 15},
+};
+
struct omap_dm_timer {
unsigned long phys_base;
int irq;
@@ -76,6 +99,7 @@ struct omap_dm_timer {
void __iomem *io_base;
unsigned reserved:1;
unsigned enabled:1;
+ unsigned posted:1;
};

#ifdef CONFIG_ARCH_OMAP1
@@ -181,16 +205,25 @@ static struct clk **dm_source_clocks;

static spinlock_t dm_timer_lock;

-static inline u32 omap_dm_timer_read_reg(struct omap_dm_timer *timer,
int reg)
+u32 omap_dm_timer_read_reg(struct omap_dm_timer *timer, int reg)
{
+ /* A read of a non completed write will be a read error */
+ if (timer->posted)
+ while (readl(timer->io_base + OMAP_TIMER_WRITE_PEND_REG)
&
+ (1 << WPINDEX(reg)))
+ ;
return readl(timer->io_base + reg);
}

-static void omap_dm_timer_write_reg(struct omap_dm_timer *timer, int
reg, u32 value)
+static void omap_dm_timer_write_reg(struct omap_dm_timer *timer, int
reg,
+ u32 value)
{
+ /* A write on a register which has a pending write will be lost
*/
+ if (timer->posted)
+ while (readl(timer->io_base + OMAP_TIMER_WRITE_PEND_REG)
&
+ (1 << WPINDEX(reg)))
+ ;
writel(value, timer->io_base + reg);
- while (omap_dm_timer_read_reg(timer, OMAP_TIMER_WRITE_PEND_REG))
- ;
}

static void omap_dm_timer_wait_for_reset(struct omap_dm_timer *timer)
@@ -217,15 +250,17 @@ static void omap_dm_timer_reset(struct
omap_dm_timer *timer)
}
omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);

- /* Set to smart-idle mode */
l = omap_dm_timer_read_reg(timer, OMAP_TIMER_OCP_CFG_REG);
- l |= 0x02 << 3;
+ l |= 0x02 << 3; /* Set to smart-idle mode */
+ l |= 0x2 << 8; /* Set clock activity to perserve f-clock on
idle */

- if (cpu_class_is_omap2() && timer == &dm_timers[0]) {
- /* Enable wake-up only for GPT1 on OMAP2 CPUs*/
+ if (cpu_class_is_omap2() && (timer == &dm_timers[0])) {
+ /* Enable wake-up only for GPT1 on OMAP2 CPUs */
+ /* FIXME: All should have this enabled and clear PRCM
status */
l |= 1 << 2;
- /* Non-posted mode */
- omap_dm_timer_write_reg(timer, OMAP_TIMER_IF_CTRL_REG,
0);
+ /* Ensure posted mode */
+ omap_dm_timer_write_reg(timer, OMAP_TIMER_IF_CTRL_REG,
+ (1 << 2));
}
omap_dm_timer_write_reg(timer, OMAP_TIMER_OCP_CFG_REG, l);
}
@@ -434,6 +469,8 @@ void omap_dm_timer_set_load(struct omap_dm_timer
*timer, int autoreload,
l &= ~OMAP_TIMER_CTRL_AR;
omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
omap_dm_timer_write_reg(timer, OMAP_TIMER_LOAD_REG, load);
+ /* ? hw feature, ttgr overtaking tldr */
+ while (readl(timer->io_base + OMAP_TIMER_WRITE_PEND_REG));
omap_dm_timer_write_reg(timer, OMAP_TIMER_TRIGGER_REG, 0);
}

@@ -568,6 +605,7 @@ int __init omap_dm_timer_init(void)
for (i = 0; i < dm_timer_count; i++) {
timer = &dm_timers[i];
timer->io_base = (void __iomem
*)io_p2v(timer->phys_base);
+ timer->posted = 1; /* post by default */
#if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
if (cpu_class_is_omap2()) {
char clk_name[16];

[-- Attachment #2: omap-git-post.diff --]
[-- Type: application/octet-stream, Size: 4543 bytes --]

This patch adds the use of write posting for the timer. Previously, every
write could lock the requestor for almost 3x32KHz cycles. This now only
synchronizes before writes and reads instead of after them and it does
it on a register per register basis. Doing it this way there is some
chance to hide some of the sync latency. It also removes some needless
reads when non-posted mode is there. With out this fix the read/writes talk
almost 2% CPU load @500MHz just waiting on tick timer registers.

Signed-off-by: Richard Woodruff <r-woodruff2@ti.com>

diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 302ad8d..e218ebd 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -67,6 +67,29 @@
#define OMAP_TIMER_CTRL_AR (1 << 1) /* auto-reload enable */
#define OMAP_TIMER_CTRL_ST (1 << 0) /* start timer */

+/* Write posting bit shift table
+ * - Values are register shift for TWPS status bits
+ * - Use of this avoids 32KHz Synchronization penalties. The cost is almost
+ * a 3x32KHz stall. This is comapred to a 166MHz posted access.
+ * - A value of '15' is a shift which always returns 0
+ * for registers which are not postable
+ *
+ * Note: Faster code can be generated if a larger array is used.
+ */
+#define PROW 6
+#define PCOL 4
+#define WPINDEX(off) (wpost[off >> 4][(off & 0xf) >> 2])
+
+static unsigned char wpost[PROW][PCOL] = {
+ /* 0, 4, 8, c */
+/* 00 */ {15, 15, 15, 15},
+/* 10 */ {15, 15, 15, 15},
+/* 20 */ {15, 0, 1, 2},
+/* 30 */ { 3, 15, 4, 15},
+/* 40 */ {15, 15, 5, 06},
+/* 50 */ { 7, 8, 9, 15},
+};
+
struct omap_dm_timer {
unsigned long phys_base;
int irq;
@@ -76,6 +99,7 @@ struct omap_dm_timer {
void __iomem *io_base;
unsigned reserved:1;
unsigned enabled:1;
+ unsigned posted:1;
};

#ifdef CONFIG_ARCH_OMAP1
@@ -181,16 +205,25 @@ static struct clk **dm_source_clocks;

static spinlock_t dm_timer_lock;

-static inline u32 omap_dm_timer_read_reg(struct omap_dm_timer *timer, int reg)
+u32 omap_dm_timer_read_reg(struct omap_dm_timer *timer, int reg)
{
+ /* A read of a non completed write will be a read error */
+ if (timer->posted)
+ while (readl(timer->io_base + OMAP_TIMER_WRITE_PEND_REG) &
+ (1 << WPINDEX(reg)))
+ ;
return readl(timer->io_base + reg);
}

-static void omap_dm_timer_write_reg(struct omap_dm_timer *timer, int reg, u32 value)
+static void omap_dm_timer_write_reg(struct omap_dm_timer *timer, int reg,
+ u32 value)
{
+ /* A write on a register which has a pending write will be lost */
+ if (timer->posted)
+ while (readl(timer->io_base + OMAP_TIMER_WRITE_PEND_REG) &
+ (1 << WPINDEX(reg)))
+ ;
writel(value, timer->io_base + reg);
- while (omap_dm_timer_read_reg(timer, OMAP_TIMER_WRITE_PEND_REG))
- ;
}

static void omap_dm_timer_wait_for_reset(struct omap_dm_timer *timer)
@@ -217,15 +250,17 @@ static void omap_dm_timer_reset(struct omap_dm_timer *timer)
}
omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);

- /* Set to smart-idle mode */
l = omap_dm_timer_read_reg(timer, OMAP_TIMER_OCP_CFG_REG);
- l |= 0x02 << 3;
+ l |= 0x02 << 3; /* Set to smart-idle mode */
+ l |= 0x2 << 8; /* Set clock activity to perserve f-clock on idle */

- if (cpu_class_is_omap2() && timer == &dm_timers[0]) {
- /* Enable wake-up only for GPT1 on OMAP2 CPUs*/
+ if (cpu_class_is_omap2() && (timer == &dm_timers[0])) {
+ /* Enable wake-up only for GPT1 on OMAP2 CPUs */
+ /* FIXME: All should have this enabled and clear PRCM status */
l |= 1 << 2;
- /* Non-posted mode */
- omap_dm_timer_write_reg(timer, OMAP_TIMER_IF_CTRL_REG, 0);
+ /* Ensure posted mode */
+ omap_dm_timer_write_reg(timer, OMAP_TIMER_IF_CTRL_REG,
+ (1 << 2));
}
omap_dm_timer_write_reg(timer, OMAP_TIMER_OCP_CFG_REG, l);
}
@@ -434,6 +469,8 @@ void omap_dm_timer_set_load(struct omap_dm_timer *timer, int autoreload,
l &= ~OMAP_TIMER_CTRL_AR;
omap_dm_timer_write_reg(timer, OMAP_TIMER_CTRL_REG, l);
omap_dm_timer_write_reg(timer, OMAP_TIMER_LOAD_REG, load);
+ /* ? hw feature, ttgr overtaking tldr */
+ while (readl(timer->io_base + OMAP_TIMER_WRITE_PEND_REG));
omap_dm_timer_write_reg(timer, OMAP_TIMER_TRIGGER_REG, 0);
}

@@ -568,6 +605,7 @@ int __init omap_dm_timer_init(void)
for (i = 0; i < dm_timer_count; i++) {
timer = &dm_timers[i];
timer->io_base = (void __iomem *)io_p2v(timer->phys_base);
+ timer->posted = 1; /* post by default */
#if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
if (cpu_class_is_omap2()) {
char clk_name[16];