Re: [patch] voluntary-preempt-2.6.9-rc1-bk4-Q5

[Takashi Iwai <tiwai@suse.de>]

At Tue, 31 Aug 2004 20:48:46 +0200,
Ingo Molnar wrote:
> 
> 
> * Takashi Iwai <tiwai@suse.de> wrote:
> 
> > Does the attached patch fix this problem?
> > 
> > 
> > Takashi
> > 
> > --- linux/sound/pci/ens1370.c	25 Aug 2004 09:57:03 -0000	1.64
> > +++ linux/sound/pci/ens1370.c	31 Aug 2004 18:17:45 -0000
> > @@ -513,6 +513,7 @@
> >  		r = inl(ES_REG(ensoniq, 1371_SMPRATE));
> >  		if ((r & ES_1371_SRC_RAM_BUSY) == 0)
> >  			return r;
> > +		cond_resched();
> 
> but ... snd_es1371_wait_src_ready() is being called with
> ensoniq->reg_lock held and you must not reschedule with a spinlock held. 

Oh yes, you're right.  I overlooked that.

> cond_resched_lock(&ensoniq->req_lock) might not crash immediately, but 
> is it really safe to release the driver lock at this point?

The lock can be released, but then *_rate() functions may become racy
with each other.

Ok, the second try.


Takashi

--- linux/sound/pci/ens1370.c	25 Aug 2004 09:57:03 -0000	1.64
+++ linux/sound/pci/ens1370.c	31 Aug 2004 19:00:33 -0000
@@ -372,6 +372,7 @@
 
 struct _snd_ensoniq {
 	spinlock_t reg_lock;
+	struct semaphore src_mutex;
 
 	int irq;
 
@@ -513,6 +514,7 @@
 		r = inl(ES_REG(ensoniq, 1371_SMPRATE));
 		if ((r & ES_1371_SRC_RAM_BUSY) == 0)
 			return r;
+		cond_resched();
 	}
 	snd_printk("wait source ready timeout 0x%lx [0x%x]\n", ES_REG(ensoniq, 1371_SMPRATE), r);
 	return 0;
@@ -696,6 +698,7 @@
 {
 	unsigned int n, truncm, freq, result;
 
+	down(&ensoniq->src_mutex);
 	n = rate / 3000;
 	if ((1 << n) & ((1 << 15) | (1 << 13) | (1 << 11) | (1 << 9)))
 		n--;
@@ -719,12 +722,14 @@
 	snd_es1371_src_write(ensoniq, ES_SMPREG_ADC + ES_SMPREG_VFREQ_FRAC, freq & 0x7fff);
 	snd_es1371_src_write(ensoniq, ES_SMPREG_VOL_ADC, n << 8);
 	snd_es1371_src_write(ensoniq, ES_SMPREG_VOL_ADC + 1, n << 8);
+	up(&ensoniq->src_mutex);
 }
 
 static void snd_es1371_dac1_rate(ensoniq_t * ensoniq, unsigned int rate)
 {
 	unsigned int freq, r;
 
+	down(&ensoniq->src_mutex);
 	freq = ((rate << 15) + 1500) / 3000;
 	r = (snd_es1371_wait_src_ready(ensoniq) & (ES_1371_SRC_DISABLE | ES_1371_DIS_P2 | ES_1371_DIS_R1)) | ES_1371_DIS_P1;
 	outl(r, ES_REG(ensoniq, 1371_SMPRATE));
@@ -734,12 +739,14 @@
 	snd_es1371_src_write(ensoniq, ES_SMPREG_DAC1 + ES_SMPREG_VFREQ_FRAC, freq & 0x7fff);
 	r = (snd_es1371_wait_src_ready(ensoniq) & (ES_1371_SRC_DISABLE | ES_1371_DIS_P2 | ES_1371_DIS_R1));
 	outl(r, ES_REG(ensoniq, 1371_SMPRATE));
+	up(&ensoniq->src_mutex);
 }
 
 static void snd_es1371_dac2_rate(ensoniq_t * ensoniq, unsigned int rate)
 {
 	unsigned int freq, r;
 
+	down(&ensoniq->src_mutex);
 	freq = ((rate << 15) + 1500) / 3000;
 	r = (snd_es1371_wait_src_ready(ensoniq) & (ES_1371_SRC_DISABLE | ES_1371_DIS_P1 | ES_1371_DIS_R1)) | ES_1371_DIS_P2;
 	outl(r, ES_REG(ensoniq, 1371_SMPRATE));
@@ -749,6 +756,7 @@
 	snd_es1371_src_write(ensoniq, ES_SMPREG_DAC2 + ES_SMPREG_VFREQ_FRAC, freq & 0x7fff);
 	r = (snd_es1371_wait_src_ready(ensoniq) & (ES_1371_SRC_DISABLE | ES_1371_DIS_P1 | ES_1371_DIS_R1));
 	outl(r, ES_REG(ensoniq, 1371_SMPRATE));
+	up(&ensoniq->src_mutex);
 }
 
 #endif /* CHIP1371 */
@@ -870,11 +878,12 @@
 	case 44100: ensoniq->ctrl |= ES_1370_WTSRSEL(3); break;
 	default: snd_BUG();
 	}
-#else
-	snd_es1371_dac1_rate(ensoniq, runtime->rate);
 #endif
 	outl(ensoniq->ctrl, ES_REG(ensoniq, CONTROL));
 	spin_unlock_irq(&ensoniq->reg_lock);
+#ifndef CHIP1370
+	snd_es1371_dac1_rate(ensoniq, runtime->rate);
+#endif
 	return 0;
 }
 
@@ -908,11 +917,12 @@
 		ensoniq->ctrl |= ES_1370_PCLKDIVO(ES_1370_SRTODIV(runtime->rate));
 		ensoniq->u.es1370.pclkdiv_lock |= ES_MODE_PLAY2;
 	}
-#else
-	snd_es1371_dac2_rate(ensoniq, runtime->rate);
 #endif
 	outl(ensoniq->ctrl, ES_REG(ensoniq, CONTROL));
 	spin_unlock_irq(&ensoniq->reg_lock);
+#ifndef CHIP1370
+	snd_es1371_dac2_rate(ensoniq, runtime->rate);
+#endif
 	return 0;
 }
 
@@ -944,11 +954,12 @@
 		ensoniq->ctrl |= ES_1370_PCLKDIVO(ES_1370_SRTODIV(runtime->rate));
 		ensoniq->u.es1370.pclkdiv_lock |= ES_MODE_CAPTURE;
 	}
-#else
-	snd_es1371_adc_rate(ensoniq, runtime->rate);
 #endif
 	outl(ensoniq->ctrl, ES_REG(ensoniq, CONTROL));
 	spin_unlock_irq(&ensoniq->reg_lock);
+#ifndef CHIP1370
+	snd_es1371_adc_rate(ensoniq, runtime->rate);
+#endif
 	return 0;
 }
 
@@ -1886,6 +1897,7 @@
 	if (ensoniq == NULL)
 		return -ENOMEM;
 	spin_lock_init(&ensoniq->reg_lock);
+	init_MUTEX(&ensoniq->src_mutex);
 	ensoniq->card = card;
 	ensoniq->pci = pci;
 	ensoniq->irq = -1;