From 9d97ee1b5401698ede224888028ca64f399fdae1 Mon Sep 17 00:00:00 2001 From: Michael Sevakis Date: Fri, 7 Jan 2011 20:40:36 +0000 Subject: Gigabeat S/i.MX31: Take care of an interrupt priority inversion that can happen during PCM callback lockout when DVFS switches frequecies during the lockout, preventing a thread from unlocking the callback until DVFS finishes, causing an SSI FIFO underrun. Hadn't thought of an acceptable way to deal with it before. git-svn-id: svn://svn.rockbox.org/rockbox/trunk@28996 a1c6a512-1295-4272-9138-f99709370657 --- firmware/target/arm/imx31/avic-imx31.h | 5 +++ firmware/target/arm/imx31/dvfs_dptc-imx31.c | 16 +++++++ firmware/target/arm/imx31/dvfs_dptc-imx31.h | 1 + .../arm/imx31/gigabeat-s/kernel-gigabeat-s.c | 5 +++ .../target/arm/imx31/gigabeat-s/pcm-gigabeat-s.c | 51 +++++++++++++++------- .../target/arm/imx31/gigabeat-s/system-target.h | 3 ++ 6 files changed, 65 insertions(+), 16 deletions(-) diff --git a/firmware/target/arm/imx31/avic-imx31.h b/firmware/target/arm/imx31/avic-imx31.h index 04a2856b50..ca48f85ba5 100644 --- a/firmware/target/arm/imx31/avic-imx31.h +++ b/firmware/target/arm/imx31/avic-imx31.h @@ -67,6 +67,11 @@ void avic_set_int_type(enum IMX31_INT_LIST ints, enum INT_TYPE intstype); #define AVIC_NIL_ENABLE (-1) void avic_set_ni_level(int level); +static inline void avic_mask_int(enum IMX31_INT_LIST ints) + { AVIC_INTDISNUM = ints; } + +static inline void avic_unmask_int(enum IMX31_INT_LIST ints) + { AVIC_INTENNUM = ints; } /* Call a service routine while allowing preemption by interrupts of higher * priority. Avoid using any app or other SVC stack by doing it with a mini diff --git a/firmware/target/arm/imx31/dvfs_dptc-imx31.c b/firmware/target/arm/imx31/dvfs_dptc-imx31.c index aa8d0f52fb..555e030af5 100644 --- a/firmware/target/arm/imx31/dvfs_dptc-imx31.c +++ b/firmware/target/arm/imx31/dvfs_dptc-imx31.c @@ -612,6 +612,22 @@ void dvfs_dptc_stop(void) } +/* Mask the DVFS interrupt without affecting running status */ +void dvfs_int_mask(bool mask) +{ + if (mask) + { + /* Just disable, not running = already disabled */ + avic_mask_int(INT_CCM_DVFS); + } + else if (dvfs_running) + { + /* DVFS is running; unmask it */ + avic_unmask_int(INT_CCM_DVFS); + } +} + + /* Set a signal load tracking weight */ void dvfs_set_lt_weight(enum DVFS_LT_SIGS index, unsigned long value) { diff --git a/firmware/target/arm/imx31/dvfs_dptc-imx31.h b/firmware/target/arm/imx31/dvfs_dptc-imx31.h index 844fd6ebff..6b59bffae6 100644 --- a/firmware/target/arm/imx31/dvfs_dptc-imx31.h +++ b/firmware/target/arm/imx31/dvfs_dptc-imx31.h @@ -128,6 +128,7 @@ void dvfs_wfi_monitor(bool on); void dvfs_set_lt_weight(enum DVFS_LT_SIGS index, unsigned long value); void dvfs_set_lt_detect(enum DVFS_LT_SIGS index, bool edge); void dvfs_set_gp_bit(enum DVFS_DVGPS dvgp, bool assert); +void dvfs_int_mask(bool mask); unsigned int dvfs_dptc_get_voltage(void); unsigned int dvfs_get_level(void); diff --git a/firmware/target/arm/imx31/gigabeat-s/kernel-gigabeat-s.c b/firmware/target/arm/imx31/gigabeat-s/kernel-gigabeat-s.c index 79f3eccc6b..f040d8fcf1 100644 --- a/firmware/target/arm/imx31/gigabeat-s/kernel-gigabeat-s.c +++ b/firmware/target/arm/imx31/gigabeat-s/kernel-gigabeat-s.c @@ -85,3 +85,8 @@ void tick_stop(void) ccm_module_clock_gating(CG_EPIT1, CGM_OFF); /* Turn off module clock */ } + +void kernel_audio_locking(bool locking) +{ + dvfs_int_mask(locking); +} \ No newline at end of file diff --git a/firmware/target/arm/imx31/gigabeat-s/pcm-gigabeat-s.c b/firmware/target/arm/imx31/gigabeat-s/pcm-gigabeat-s.c index 50c7da943e..c8c1283d12 100644 --- a/firmware/target/arm/imx31/gigabeat-s/pcm-gigabeat-s.c +++ b/firmware/target/arm/imx31/gigabeat-s/pcm-gigabeat-s.c @@ -109,20 +109,33 @@ static void play_dma_callback(void) void pcm_play_lock(void) { + /* Need to prevent DVFS from causing interrupt priority inversion if audio + * is locked and a DVFS interrupt fires, blocking reenabling of audio by a + * low-priority mode for at least the duration of the lengthy DVFS routine. + * Not really an issue with state changes but lockout when playing. + * + * Keep direct use of DVFS code away from here though. This could provide + * more services in the future anyway. */ + kernel_audio_locking(true); ++dma_play_data.locked; } void pcm_play_unlock(void) { - if (--dma_play_data.locked == 0 && dma_play_data.state != 0) + if (--dma_play_data.locked == 0) { - int oldstatus = disable_irq_save(); - int pending = dma_play_data.callback_pending; - dma_play_data.callback_pending = 0; - restore_irq(oldstatus); - - if (pending != 0) - play_dma_callback(); + if (dma_play_data.state != 0) + { + int oldstatus = disable_irq_save(); + int pending = dma_play_data.callback_pending; + dma_play_data.callback_pending = 0; + restore_irq(oldstatus); + + if (pending != 0) + play_dma_callback(); + } + + kernel_audio_locking(false); } } @@ -442,20 +455,26 @@ static void rec_dma_callback(void) void pcm_rec_lock(void) { + kernel_audio_locking(true); ++dma_rec_data.locked; } void pcm_rec_unlock(void) { - if (--dma_rec_data.locked == 0 && dma_rec_data.state != 0) + if (--dma_rec_data.locked == 0) { - int oldstatus = disable_irq_save(); - int pending = dma_rec_data.callback_pending; - dma_rec_data.callback_pending = 0; - restore_irq(oldstatus); - - if (pending != 0) - rec_dma_callback(); + if (dma_rec_data.state != 0) + { + int oldstatus = disable_irq_save(); + int pending = dma_rec_data.callback_pending; + dma_rec_data.callback_pending = 0; + restore_irq(oldstatus); + + if (pending != 0) + rec_dma_callback(); + } + + kernel_audio_locking(false); } } diff --git a/firmware/target/arm/imx31/gigabeat-s/system-target.h b/firmware/target/arm/imx31/gigabeat-s/system-target.h index e970109f82..f3ba719443 100644 --- a/firmware/target/arm/imx31/gigabeat-s/system-target.h +++ b/firmware/target/arm/imx31/gigabeat-s/system-target.h @@ -51,6 +51,9 @@ void tick_stop(void); void kernel_device_init(void); void system_halt(void); +/* Handle some audio lockout related tasks */ +void kernel_audio_locking(bool locking); + #define KDEV_INIT struct ARM_REGS { -- cgit v1.2.3