[alsa-devel] [PATCH] cs4231-lib: improved waiting after mce_down
From: Krzysztof Helt krzysztof.h1@wp.pl
This patch replaces long msleeps in waiting loops with schedule_timeout() calls.
Signed-off-by: Krzysztof Helt krzysztof.h1@wp.pl ---
The cs4231 is quite fast and calibration takes about 2ms at 48kHz (the quickest). Loops wait in 10ms steps which is usually too long.
This is done after the ad1848-lib driver, which waits in this improved way. This makes the cs4231-lib and ad1848-lib more similar and should help merging of the two.
Regards, Krzysztof
diff -urp linux-2.6.22.old/sound/isa/cs423x/cs4231_lib.c linux-2.6.22/sound/isa/cs423x/cs4231_lib.c --- linux-2.6.22.old/sound/isa/cs423x/cs4231_lib.c 2007-09-09 20:33:52.000000000 +0200 +++ linux-2.6.22/sound/isa/cs423x/cs4231_lib.c 2007-09-09 20:31:27.000000000 +0200 @@ -314,6 +314,7 @@ void snd_cs4231_mce_down(struct snd_cs42 { unsigned long flags; int timeout; + long time;
snd_cs4231_busy_wait(chip); #if 0 @@ -340,31 +341,34 @@ void snd_cs4231_mce_down(struct snd_cs42
snd_printd("(2) jiffies = %li\n", jiffies);
- /* in 10 ms increments, check condition, up to 250 ms */ - timeout = 25; + time = HZ / 4; while (snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) { - if (--timeout < 0) { - snd_printk("mce_down - auto calibration time out (2)\n"); + spin_unlock_irqrestore(&chip->reg_lock, flags); + if (time <= 0) { + snd_printk(KERN_ERR "mce_down - " + "auto calibration time out (2)\n"); return; } - msleep(10); + time = schedule_timeout(time); + spin_lock_irqsave(&chip->reg_lock, flags); } -#if 0 - printk("(3) jiffies = %li\n", jiffies); -#endif - /* in 10 ms increments, check condition, up to 100 ms */ - timeout = 10; + + snd_printd("(3) jiffies = %li\n", jiffies); + + time = HZ / 10; while (cs4231_inb(chip, CS4231P(REGSEL)) & CS4231_INIT) { - if (--timeout < 0) { - snd_printk(KERN_ERR "mce_down - auto calibration time out (3)\n"); + spin_unlock_irqrestore(&chip->reg_lock, flags); + if (time <= 0) { + snd_printk(KERN_ERR "mce_down - " + "auto calibration time out (3)\n"); return; } - msleep(10); + time = schedule_timeout(time); + spin_lock_irqsave(&chip->reg_lock, flags); } -#if 0 - printk("(4) jiffies = %li\n", jiffies); - snd_printk("mce_down - exit = 0x%x\n", cs4231_inb(chip, CS4231P(REGSEL))); -#endif + snd_printd("(4) jiffies = %li\n", jiffies); + snd_printk("mce_down - exit = 0x%x\n", + cs4231_inb(chip, CS4231P(REGSEL))); }
static unsigned int snd_cs4231_get_count(unsigned char format, unsigned int size)
At Sun, 9 Sep 2007 22:11:31 +0200, Krzysztof Helt wrote:
- /* in 10 ms increments, check condition, up to 250 ms */
- timeout = 25;
- time = HZ / 4;
Use msecs_to_jiffies(250) instead.
while (snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) {
if (--timeout < 0) {snd_printk("mce_down - auto calibration time out (2)\n");
spin_unlock_irqrestore(&chip->reg_lock, flags);if (time <= 0) {snd_printk(KERN_ERR "mce_down - ""auto calibration time out (2)\n"); return;
msleep(10);
time = schedule_timeout(time);spin_lock_irqsave(&chip->reg_lock, flags);-#if 0
- printk("(3) jiffies = %li\n", jiffies);
-#endif
- /* in 10 ms increments, check condition, up to 100 ms */
- timeout = 10;
- snd_printd("(3) jiffies = %li\n", jiffies);
- time = HZ / 10;
Here, too.
thanks,
Takashi
From: Krzysztof Helt <krzysztof.h1 at wp.pl>
This patch replaces long msleeps in waiting loops with schedule_timeout() calls.
Signed-off-by: Krzysztof Helt <krzysztof.h1 at wp.pl> ---
This is done against tree with Rene's patches.
Regards, Krzysztof
At Tue, 11 Sep 2007 00:40:10 +0200, Krzysztof Helt wrote:
From: Krzysztof Helt <krzysztof.h1 at wp.pl>
This patch replaces long msleeps in waiting loops with schedule_timeout() calls.
Signed-off-by: Krzysztof Helt <krzysztof.h1 at wp.pl>
This is done against tree with Rene's patches.
Regards, Krzysztof
Applied now. Thanks.
Takashi
On 09/11/2007 12:40 AM, Krzysztof Helt wrote:
This patch replaces long msleeps in waiting loops with schedule_timeout() calls.
Hope you'll not detest me for getting on your case so much, but...
First, yes, attachments are good as far as I'm concerned, but could you make them plain text attachments? They are now Base64 which means that at least with my "casual mailer" (Thunderbird) I need to save the patch and can only then look at it.
Second -- not schedule_timeout_interruptible? A plain schedule_timeout won't work, as you first need to __set_current_state or it will not schedule. That is, schedule_timeout_interruptible or _uniterruptible and with the latter, the loop wouldn't make much sense anymore as it's just going to sleep for the full 250 ms directly.
Rene.
At Tue, 11 Sep 2007 00:43:42 +0200, Rene Herman wrote:
Second -- not schedule_timeout_interruptible? A plain schedule_timeout won't work, as you first need to __set_current_state or it will not schedule. That is, schedule_timeout_interruptible or _uniterruptible and with the latter, the loop wouldn't make much sense anymore as it's just going to sleep for the full 250 ms directly.
Ah, right. I'm obviously too sleepy to review now...
Actually, the code with msleep() was basically OK. If more finer check is needed, it can simply use msleep(1). The point is to use timer_after_eq() instead of loop count for the precise timeouts.
thanks,
Takashi
At Tue, 11 Sep 2007 01:08:07 +0200, I wrote:
At Tue, 11 Sep 2007 00:43:42 +0200, Rene Herman wrote:
Second -- not schedule_timeout_interruptible? A plain schedule_timeout won't work, as you first need to __set_current_state or it will not schedule. That is, schedule_timeout_interruptible or _uniterruptible and with the latter, the loop wouldn't make much sense anymore as it's just going to sleep for the full 250 ms directly.
Ah, right. I'm obviously too sleepy to review now...
Actually, the code with msleep() was basically OK. If more finer check is needed, it can simply use msleep(1). The point is to use timer_after_eq() instead of loop count for the precise timeouts.
Could you check whether the below patch works?
Takashi
diff -r 475132691bb1 isa/cs423x/cs4231_lib.c --- a/isa/cs423x/cs4231_lib.c Tue Sep 11 00:55:46 2007 +0200 +++ b/isa/cs423x/cs4231_lib.c Tue Sep 11 01:15:17 2007 +0200 @@ -313,6 +313,7 @@ void snd_cs4231_mce_down(struct snd_cs42 void snd_cs4231_mce_down(struct snd_cs4231 *chip) { unsigned long flags; + unsigned long end_time; int timeout;
snd_cs4231_busy_wait(chip); @@ -344,28 +345,28 @@ void snd_cs4231_mce_down(struct snd_cs42 snd_printdd("(1) jiffies = %lu\n", jiffies);
/* check condition up to 250 ms */ - timeout = msecs_to_jiffies(250); + end_time = jiffies + msecs_to_jiffies(250); while (snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) {
- if (timeout <= 0) { + if (time_after(jiffies, end_time)) { snd_printk(KERN_ERR "mce_down - " "auto calibration time out (2)\n"); return; } - timeout = schedule_timeout(timeout); + msleep(1); }
snd_printdd("(2) jiffies = %lu\n", jiffies);
/* check condition up to 100 ms */ - timeout = msecs_to_jiffies(100); + end_time = jiffies + msecs_to_jiffies(100); while (cs4231_inb(chip, CS4231P(REGSEL)) & CS4231_INIT) { - if (timeout <= 0) { + if (time_after(jiffies, end_time)) { snd_printk(KERN_ERR "mce_down - auto calibration time out (3)\n"); return; } - timeout = schedule_timeout(timeout); + msleep(1); }
snd_printdd("(3) jiffies = %lu\n", jiffies); diff -r 475132691bb1 sparc/cs4231.c --- a/sparc/cs4231.c Tue Sep 11 00:55:46 2007 +0200 +++ b/sparc/cs4231.c Tue Sep 11 01:15:17 2007 +0200 @@ -401,6 +401,7 @@ static void snd_cs4231_mce_down(struct s static void snd_cs4231_mce_down(struct snd_cs4231 *chip) { unsigned long flags; + unsigned long end_time; int timeout;
spin_lock_irqsave(&chip->lock, flags); @@ -431,30 +432,30 @@ static void snd_cs4231_mce_down(struct s msleep(1);
/* check condition up to 250ms */ - timeout = msecs_to_jiffies(250); + end_time = jiffies + msecs_to_jiffies(250); while (snd_cs4231_in(chip, CS4231_TEST_INIT) & CS4231_CALIB_IN_PROGRESS) {
spin_unlock_irqrestore(&chip->lock, flags); - if (timeout <= 0) { + if (time_after(jiffies, end_time)) { snd_printk("mce_down - " "auto calibration time out (2)\n"); return; } - timeout = schedule_timeout(timeout); + msleep(1); spin_lock_irqsave(&chip->lock, flags); }
/* check condition up to 100ms */ - timeout = msecs_to_jiffies(100); + end_time = jiffies + msecs_to_jiffies(100); while (__cs4231_readb(chip, CS4231U(chip, REGSEL)) & CS4231_INIT) { spin_unlock_irqrestore(&chip->lock, flags); - if (timeout <= 0) { + if (time_after(jiffies, end_time)) { snd_printk("mce_down - " "auto calibration time out (3)\n"); return; } - timeout = schedule_timeout(timeout); + msleep(1); spin_lock_irqsave(&chip->lock, flags); } spin_unlock_irqrestore(&chip->lock, flags);
On 09/11/2007 01:17 AM, Takashi Iwai wrote:
Actually, the code with msleep() was basically OK. If more finer check is needed, it can simply use msleep(1). The point is to use timer_after_eq() instead of loop count for the precise timeouts.
Could you check whether the below patch works?
Works for me on ISA CS4231 (behind an AZT1605). Earlier Krzysztof suggested that 2 ms may be optimum, but I'll leave that upto him.
Acked-by: Rene Herman rene.herman@gmail.com
Zzz, Rene
On Tue, 11 Sep 2007 02:40:34 +0200 Rene Herman rene.herman@gmail.com wrote:
On 09/11/2007 01:17 AM, Takashi Iwai wrote:
Works for me on ISA CS4231 (behind an AZT1605). Earlier Krzysztof suggested that 2 ms may be optimum, but I'll leave that upto him.
The 2ms is optimal before the loop. Inside the loop the less the better.
Regards, Krzysztof
On 9/11/07, Takashi Iwai tiwai@suse.de wrote:
At Tue, 11 Sep 2007 01:08:07 +0200, I wrote:
Actually, the code with msleep() was basically OK. If more finer check is needed, it can simply use msleep(1). The point is to use timer_after_eq() instead of loop count for the precise timeouts.
The patch is ok. You may change this timer_after_eq and jiffies into just number for the timeout (one pass will be 1ms) for simplicity (not required).
The ad1848_lib have been using this broken method (schedule_timeout with set_current_..). You can fix it as well or if the patch gets applied, I can sync the ad1848_lib to use exactly the same waiting loops. The ad1848 chip is slower then cs4231, so the granularity of 1ms hurts it less.
I want to have your patch applied as it is improvement (along with Rene's patch) over the old code: it is simpler, less code and lower (on average) delay on playback/record start.
Regards, Krzysztof
participants (3)
-
Krzysztof Helt -
Rene Herman -
Takashi Iwai