From 094da2fad43ba74895a84c469dcb35cac0a46822 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Mon, 2 Sep 2019 19:25:49 +0200 Subject: [PATCH 1/3] codeowners: add @tlauda and @mrajwa as codeowners of schedule directory This adds @tlauda and @mrajwa as codeowners of src\scheduler directory so they will be auto-requested for review in all changes related to files under this directory. Signed-off-by: Marcin Rajwa --- CODEOWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/CODEOWNERS b/CODEOWNERS index ed35fd9921fc..cd28e8929586 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -48,6 +48,7 @@ src/ipc/* @xiulipan @bardliao src/ipc/sue-ipc.c @lyakh src/lib/* @libinyang src/gdb/* @mrajwa +src/schedule @tlauda @mrajwa # other helpers test/** @jajanusz From 0cde4bcf418de66b462a9c70c50d0b102f6ad1d9 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Wed, 11 Sep 2019 14:30:12 +0200 Subject: [PATCH 2/3] timer: rework timer_set() so it verifies requested ticks This patch reworks cAVS timer_set() interface used to write into timer's compare register. This change takes care of ticks request which are "past time". If such condition occur we set compare register to current time so the latency is minimum. Signed-off-by: Marcin Rajwa --- src/arch/host/include/arch/drivers/timer.h | 4 ++-- src/arch/xtensa/drivers/timer.c | 2 +- src/arch/xtensa/include/arch/drivers/timer.h | 2 +- src/drivers/imx/timer.c | 2 +- src/drivers/intel/baytrail/timer.c | 2 +- src/drivers/intel/cavs/timer.c | 13 +++++++++---- src/drivers/intel/haswell/timer.c | 2 +- src/include/sof/drivers/timer.h | 4 ++-- 8 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/arch/host/include/arch/drivers/timer.h b/src/arch/host/include/arch/drivers/timer.h index 3dd1e91fc8aa..76305224afed 100644 --- a/src/arch/host/include/arch/drivers/timer.h +++ b/src/arch/host/include/arch/drivers/timer.h @@ -21,8 +21,8 @@ static inline void arch_timer_unregister(struct timer *timer) {} static inline void arch_timer_enable(struct timer *timer) {} static inline void arch_timer_disable(struct timer *timer) {} static inline uint32_t arch_timer_get_system(struct timer *timer) {return 0; } -static inline int arch_timer_set(struct timer *timer, - uint64_t ticks) {return 0; } +static inline int64_t arch_timer_set(struct timer *timer, + uint64_t ticks) {return 0; } static inline void arch_timer_clear(struct timer *timer) {} #endif /* __ARCH_DRIVERS_TIMER_H__ */ diff --git a/src/arch/xtensa/drivers/timer.c b/src/arch/xtensa/drivers/timer.c index f84d7b215d14..f242a6023c51 100644 --- a/src/arch/xtensa/drivers/timer.c +++ b/src/arch/xtensa/drivers/timer.c @@ -103,7 +103,7 @@ uint64_t arch_timer_get_system(struct timer *timer) return time; } -int arch_timer_set(struct timer *timer, uint64_t ticks) +int64_t arch_timer_set(struct timer *timer, uint64_t ticks) { uint32_t time = 1; uint32_t hitimeout = ticks >> 32; diff --git a/src/arch/xtensa/include/arch/drivers/timer.h b/src/arch/xtensa/include/arch/drivers/timer.h index b8841b08864c..799e43104054 100644 --- a/src/arch/xtensa/include/arch/drivers/timer.h +++ b/src/arch/xtensa/include/arch/drivers/timer.h @@ -63,7 +63,7 @@ static inline void arch_timer_disable(struct timer *timer) uint64_t arch_timer_get_system(struct timer *timer); -int arch_timer_set(struct timer *timer, uint64_t ticks); +int64_t arch_timer_set(struct timer *timer, uint64_t ticks); static inline void arch_timer_clear(struct timer *timer) { diff --git a/src/drivers/imx/timer.c b/src/drivers/imx/timer.c index cc725a3374ae..35f7c1b68e5a 100644 --- a/src/drivers/imx/timer.c +++ b/src/drivers/imx/timer.c @@ -22,7 +22,7 @@ void platform_timer_stop(struct timer *timer) arch_timer_disable(timer); } -int platform_timer_set(struct timer *timer, uint64_t ticks) +int64_t platform_timer_set(struct timer *timer, uint64_t ticks) { return arch_timer_set(timer, ticks); } diff --git a/src/drivers/intel/baytrail/timer.c b/src/drivers/intel/baytrail/timer.c index 5d484446f50d..f6738e0a45cd 100644 --- a/src/drivers/intel/baytrail/timer.c +++ b/src/drivers/intel/baytrail/timer.c @@ -75,7 +75,7 @@ void platform_timer_stop(struct timer *timer) shim_write(SHIM_EXT_TIMER_CNTLH, SHIM_EXT_TIMER_CLEAR); } -int platform_timer_set(struct timer *timer, uint64_t ticks) +int64_t platform_timer_set(struct timer *timer, uint64_t ticks) { uint32_t time = 1; uint32_t hitimeout = ticks >> 32; diff --git a/src/drivers/intel/cavs/timer.c b/src/drivers/intel/cavs/timer.c index d381e076d0ab..cd653d566753 100644 --- a/src/drivers/intel/cavs/timer.c +++ b/src/drivers/intel/cavs/timer.c @@ -32,17 +32,22 @@ void platform_timer_stop(struct timer *timer) shim_read(SHIM_DSPWCTCS) & ~SHIM_DSPWCTCS_T0A); } -int platform_timer_set(struct timer *timer, uint64_t ticks) +int64_t platform_timer_set(struct timer *timer, uint64_t ticks) { /* a tick value of 0 will not generate an IRQ */ if (ticks == 0) ticks = 1; - /* set new value and run */ - shim_write64(SHIM_DSPWCT0C, ticks); + /* Check if requested time is not past time */ + if (ticks > shim_read64(SHIM_DSPWC)) + shim_write64(SHIM_DSPWCT0C, ticks); + else + shim_write64(SHIM_DSPWCT0C, shim_read64(SHIM_DSPWC) + 1); + + /* Enable IRQ */ shim_write(SHIM_DSPWCTCS, SHIM_DSPWCTCS_T0A); - return 0; + return shim_read64(SHIM_DSPWCT0C); } void platform_timer_clear(struct timer *timer) diff --git a/src/drivers/intel/haswell/timer.c b/src/drivers/intel/haswell/timer.c index d9a55cf52697..05e3bc96ac37 100644 --- a/src/drivers/intel/haswell/timer.c +++ b/src/drivers/intel/haswell/timer.c @@ -23,7 +23,7 @@ void platform_timer_stop(struct timer *timer) arch_timer_disable(timer); } -int platform_timer_set(struct timer *timer, uint64_t ticks) +int64_t platform_timer_set(struct timer *timer, uint64_t ticks) { return arch_timer_set(timer, ticks); } diff --git a/src/include/sof/drivers/timer.h b/src/include/sof/drivers/timer.h index f91fb7f4ea36..b307c18516ef 100644 --- a/src/include/sof/drivers/timer.h +++ b/src/include/sof/drivers/timer.h @@ -25,7 +25,7 @@ void timer_unregister(struct timer *timer, void *arg); void timer_enable(struct timer *timer, void *arg, int core); void timer_disable(struct timer *timer, void *arg, int core); -static inline int timer_set(struct timer *timer, uint64_t ticks) +static inline int64_t timer_set(struct timer *timer, uint64_t ticks) { return arch_timer_set(timer, ticks); } @@ -46,7 +46,7 @@ static inline uint64_t timer_get_system(struct timer *timer) return arch_timer_get_system(timer); } -int platform_timer_set(struct timer *timer, uint64_t ticks); +int64_t platform_timer_set(struct timer *timer, uint64_t ticks); void platform_timer_clear(struct timer *timer); uint64_t platform_timer_get(struct timer *timer); void platform_timer_start(struct timer *timer); From cc148e0d00e6724c35f201bec12be9fbf46cac04 Mon Sep 17 00:00:00 2001 From: Marcin Rajwa Date: Tue, 10 Sep 2019 00:04:13 +0200 Subject: [PATCH 3/3] timer_domain: check if delay occurred during scheduling This patch checks if during scheduling we were delayed. If such situation happens we log error message. Signed-off-by: Marcin Rajwa --- src/schedule/timer_domain.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/schedule/timer_domain.c b/src/schedule/timer_domain.c index 37c623fcb6cb..ae818c4c7f19 100644 --- a/src/schedule/timer_domain.c +++ b/src/schedule/timer_domain.c @@ -24,6 +24,16 @@ struct timer_domain { struct ll_schedule_domain_ops timer_domain_ops; +static inline void timer_report_delay(int id, uint64_t delay) +{ + uint32_t ll_delay_us = (delay * 1000) / + clock_ms_to_ticks(PLATFORM_DEFAULT_CLOCK, 1); + + trace_schedule_error("timer_report_delay(): timer %d " + "delayed by %d uS %d ticks", id, ll_delay_us, + delay); +} + static int timer_domain_register(struct ll_schedule_domain *domain, uint64_t period, struct task *task, void (*handler)(void *arg), void *arg) @@ -72,12 +82,20 @@ static void timer_domain_disable(struct ll_schedule_domain *domain, int core) static void timer_domain_set(struct ll_schedule_domain *domain, uint64_t start) { struct timer_domain *timer_domain = ll_sch_domain_get_pdata(domain); - uint64_t ticks = domain->ticks_per_ms * timer_domain->timeout / 1000 + - start; + uint64_t ticks_req = domain->ticks_per_ms * timer_domain->timeout / + 1000 + start; + uint64_t ticks_set; + + ticks_set = platform_timer_set(timer_domain->timer, ticks_req); - platform_timer_set(timer_domain->timer, ticks); + /* Was timer set to the value we requested? If no it means some + * delay occurred and we should report that in error log. + */ + if (ticks_req < ticks_set) + timer_report_delay(timer_domain->timer->id, + ticks_set - ticks_req); - domain->last_tick = ticks; + domain->last_tick = ticks_set; } static void timer_domain_clear(struct ll_schedule_domain *domain)