All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Krzysztof Helt" <krzysztof.h1@wp.pl>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel <alsa-devel@lists.sourceforge.net>
Subject: Re: sparc dbri: SMP fixes
Date: Tue, 05 Sep 2006 20:14:11 +0200	[thread overview]
Message-ID: <44fdbe73e9a3e@wp.pl> (raw)
In-Reply-To: <44f7dd09db938@wp.pl> <s5h4pvrop4h.wl%tiwai@suse.de>

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

Dnia 1-09-2006 o godz. 12:30 Takashi Iwai napisał(a):
> 
> There is only one place that calls dbri_cmdsend(lock=0), namely
> xmit_fixed().  Hence, I suggest not to add this argument but
just add
> spin_lock_irqsave()/spin_unlock_irqrestore() around the calling
side
> in xmit_fixed().  The spinlock per argument is hard to trace
whether
> the lock is really needed or not.
> 

This is a corrected patch.

Regards,
Krzysztof

----------------------------------------------------
Dzieci.wp.pl - Wydrukuj sobie plan lekcji - kliknij:
http://klik.wp.pl/?adr=http%3A%2F%2Fadv.reklama.wp.pl%2Fas%2Fd1.html&sid=860

[-- Attachment #2: dbri-patch15.diff --]
[-- Type: application/octet-stream, Size: 9140 bytes --]

--- alsa-driver-1.0.12rc2/alsa-kernel/sparc/dbri.c	2006-09-04 19:30:28.000000000 +0200
+++ linux-2.6.17a/sound/sparc/dbri.c	2006-09-04 19:36:45.000000000 +0200
@@ -635,10 +635,16 @@ to send them to the DBRI.
 static void dbri_cmdwait(struct snd_dbri *dbri)
 {
 	int maxloops = MAXLOOPS;
+	unsigned long flags;
 
 	/* Delay if previous commands are still being processed */
-	while ((--maxloops) > 0 && (sbus_readl(dbri->regs + REG0) & D_P))
+	spin_lock_irqsave(&dbri->lock, flags);
+	while ((--maxloops) > 0 && (sbus_readl(dbri->regs + REG0) & D_P)) {
+		spin_unlock_irqrestore(&dbri->lock, flags);
 		msleep_interruptible(1);
+		spin_lock_irqsave(&dbri->lock, flags);
+	}
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	if (maxloops == 0) {
 		printk(KERN_ERR "DBRI: Chip never completed command buffer\n");
@@ -671,11 +677,12 @@ static s32 *dbri_cmdlock(struct snd_dbri
  * the last WAIT cmd and force DBRI to reread the cmd.
  * The JUMP cmd points to the new cmd string.
  * It also releases the cmdlock spinlock.
+ *
+ * Lock must not be held before calling this.
  */
 static void dbri_cmdsend(struct snd_dbri * dbri, s32 * cmd,int len)
 {
 	s32 tmp, addr;
-	unsigned long flags;
 	static int wait_id = 0;
 
 	wait_id++;
@@ -706,12 +713,10 @@ static void dbri_cmdsend(struct snd_dbri
 	}
 #endif
 
-	spin_lock_irqsave(&dbri->lock, flags);
 	/* Reread the last command */
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp |= D_P;
 	sbus_writel(tmp, dbri->regs + REG0);
-	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	dbri->cmdptr = cmd;
 	spin_unlock(&dbri->cmdlock);
@@ -777,9 +782,9 @@ static void dbri_initialize(struct snd_d
 	dma_addr = dbri->dma_dvma + dbri_dma_off(cmd, 0);
 	sbus_writel(dma_addr, dbri->regs + REG8);
 	spin_unlock(&dbri->cmdlock);
-	dbri_cmdwait(dbri);
 
 	spin_unlock_irqrestore(&dbri->lock, flags);
+	dbri_cmdwait(dbri);
 }
 
 /*
@@ -840,6 +845,9 @@ static void reset_pipe(struct snd_dbri *
 	dbri->pipes[pipe].first_desc = -1;
 }
 
+/*
+ * Lock must be held before calling this.
+ */
 static void setup_pipe(struct snd_dbri * dbri, int pipe, int sdp)
 {
 	if (pipe < 0 || pipe > DBRI_MAX_PIPE) {
@@ -866,6 +874,9 @@ static void setup_pipe(struct snd_dbri *
 	reset_pipe(dbri, pipe);
 }
 
+/*
+ * Lock must be held before calling this.
+ */
 static void link_time_slot(struct snd_dbri * dbri, int pipe,
 			   int prevpipe, int nextpipe,
 			   int length, int cycle)
@@ -920,6 +931,10 @@ static void link_time_slot(struct snd_db
 	dbri_cmdsend(dbri, cmd, 4);
 }
 
+#if 0
+/*
+ * Lock must be held before calling this.
+ */
 static void unlink_time_slot(struct snd_dbri * dbri, int pipe,
 			     enum in_or_out direction, int prevpipe,
 			     int nextpipe)
@@ -952,6 +967,7 @@ static void unlink_time_slot(struct snd_
 
 	dbri_cmdsend(dbri, cmd, 4);
 }
+#endif
 
 /* xmit_fixed() / recv_fixed()
  *
@@ -965,11 +981,14 @@ static void unlink_time_slot(struct snd_
  * the actual time slot is.  The interrupt handler takes care of bit
  * ordering and alignment.  An 8-bit time slot will always end up
  * in the low-order 8 bits, filled either MSB-first or LSB-first,
- * depending on the settings passed to setup_pipe()
+ * depending on the settings passed to setup_pipe().
+ *
+ * Lock must not be held before calling it.
  */
 static void xmit_fixed(struct snd_dbri * dbri, int pipe, unsigned int data)
 {
 	s32 *cmd;
+	unsigned long flags;
 
 	if (pipe < 16 || pipe > DBRI_MAX_PIPE) {
 		printk(KERN_ERR "DBRI: xmit_fixed: Illegal pipe number\n");
@@ -1002,8 +1021,11 @@ static void xmit_fixed(struct snd_dbri *
 	*(cmd++) = data;
 	*(cmd++) = DBRI_CMD(D_PAUSE, 0, 0);
 
+	spin_lock_irqsave(&dbri->lock, flags);
 	dbri_cmdsend(dbri, cmd, 3);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 	dbri_cmdwait(dbri);
+
 }
 
 static void recv_fixed(struct snd_dbri * dbri, int pipe, volatile __u32 * ptr)
@@ -1039,6 +1061,8 @@ static void recv_fixed(struct snd_dbri *
  * be spread across multiple descriptors.
  *
  * All descriptors create a ring buffer.
+ *
+ * Lock must be held before calling this.
  */
 static int setup_descs(struct snd_dbri * dbri, int streamno, unsigned int period)
 {
@@ -1186,6 +1210,9 @@ multiplexed serial interface which the D
 
 enum master_or_slave { CHImaster, CHIslave };
 
+/*
+ * Lock must not be held before calling it.
+ */
 static void reset_chi(struct snd_dbri * dbri, enum master_or_slave master_or_slave,
 		      int bits_per_frame)
 {
@@ -1258,9 +1285,14 @@ static void reset_chi(struct snd_dbri * 
 In the standard SPARC audio configuration, the CS4215 codec is attached
 to the DBRI via the CHI interface and few of the DBRI's PIO pins.
 
+ * Lock must not be held before calling it.
+
 */
 static void cs4215_setup_pipes(struct snd_dbri * dbri)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&dbri->lock, flags);
 	/*
 	 * Data mode:
 	 * Pipe  4: Send timeslots 1-4 (audio data)
@@ -1284,6 +1316,7 @@ static void cs4215_setup_pipes(struct sn
 	setup_pipe(dbri, 17, D_SDP_FIXED | D_SDP_TO_SER | D_SDP_MSB);
 	setup_pipe(dbri, 18, D_SDP_FIXED | D_SDP_FROM_SER | D_SDP_MSB);
 	setup_pipe(dbri, 19, D_SDP_FIXED | D_SDP_FROM_SER | D_SDP_MSB);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	dbri_cmdwait(dbri);
 }
@@ -1358,6 +1391,7 @@ static void cs4215_open(struct snd_dbri 
 {
 	int data_width;
 	u32 tmp;
+	unsigned long flags;
 
 	dprintk(D_MM, "cs4215_open: %d channels, %d bits\n",
 		dbri->mm.channels, dbri->mm.precision);
@@ -1382,6 +1416,7 @@ static void cs4215_open(struct snd_dbri 
 	 * bits.  The CS4215, it seems, observes TSIN (the delayed signal)
 	 * even if it's the CHI master.  Don't ask me...
 	 */
+	spin_lock_irqsave(&dbri->lock, flags);
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp &= ~(D_C);		/* Disable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
@@ -1409,6 +1444,7 @@ static void cs4215_open(struct snd_dbri 
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp |= D_C;		/* Enable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	cs4215_setdata(dbri, 0);
 }
@@ -1420,6 +1456,7 @@ static int cs4215_setctrl(struct snd_dbr
 {
 	int i, val;
 	u32 tmp;
+	unsigned long flags;
 
 	/* FIXME - let the CPU do something useful during these delays */
 
@@ -1456,6 +1493,7 @@ static int cs4215_setctrl(struct snd_dbr
 	 * done in hardware by a TI 248 that delays the DBRI->4215
 	 * frame sync signal by eight clock cycles.  Anybody know why?
 	 */
+	spin_lock_irqsave(&dbri->lock, flags);
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp &= ~D_C;		/* Disable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
@@ -1472,14 +1510,17 @@ static int cs4215_setctrl(struct snd_dbr
 	link_time_slot(dbri, 17, 16, 16, 32, dbri->mm.offset);
 	link_time_slot(dbri, 18, 16, 16, 8, dbri->mm.offset);
 	link_time_slot(dbri, 19, 18, 16, 8, dbri->mm.offset + 48);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	/* Wait for the chip to echo back CLB (Control Latch Bit) as zero */
 	dbri->mm.ctrl[0] &= ~CS4215_CLB;
 	xmit_fixed(dbri, 17, *(int *)dbri->mm.ctrl);
 
+	spin_lock_irqsave(&dbri->lock, flags);
 	tmp = sbus_readl(dbri->regs + REG0);
 	tmp |= D_C;		/* Enable CHI */
 	sbus_writel(tmp, dbri->regs + REG0);
+	spin_unlock_irqrestore(&dbri->lock, flags);
 
 	for (i = 10; ((dbri->mm.status & 0xe4) != 0x20); --i) {
 		msleep_interruptible(1);
@@ -1688,6 +1729,7 @@ static void xmit_descs(struct snd_dbri *
 			dbri->pipes[info->pipe].desc = first_td;
 		}
 	}
+
 	spin_unlock_irqrestore(&dbri->lock, flags);
 }
 
@@ -2093,7 +2135,6 @@ static int snd_dbri_prepare(struct snd_p
 {
 	struct snd_dbri *dbri = snd_pcm_substream_chip(substream);
 	struct dbri_streaminfo *info = DBRI_STREAM(dbri, substream);
-	struct snd_pcm_runtime *runtime = substream->runtime;
 	int ret;
 
 	info->size = snd_pcm_lib_buffer_bytes(substream);
@@ -2232,7 +2273,6 @@ static int snd_cs4215_put_volume(struct 
 {
 	struct snd_dbri *dbri = snd_kcontrol_chip(kcontrol);
 	struct dbri_streaminfo *info = &dbri->stream_info[kcontrol->private_value];
-	unsigned long flags;
 	int changed = 0;
 
 	if (info->left_gain != ucontrol->value.integer.value[0]) {
@@ -2247,13 +2287,9 @@ static int snd_cs4215_put_volume(struct 
 		/* First mute outputs, and wait 1/8000 sec (125 us)
 		 * to make sure this takes.  This avoids clicking noises.
 		 */
-		spin_lock_irqsave(&dbri->lock, flags);
-
 		cs4215_setdata(dbri, 1);
 		udelay(125);
 		cs4215_setdata(dbri, 0);
-
-		spin_unlock_irqrestore(&dbri->lock, flags);
 	}
 	return changed;
 }
@@ -2300,7 +2336,6 @@ static int snd_cs4215_put_single(struct 
 				 struct snd_ctl_elem_value *ucontrol)
 {
 	struct snd_dbri *dbri = snd_kcontrol_chip(kcontrol);
-	unsigned long flags;
 	int elem = kcontrol->private_value & 0xff;
 	int shift = (kcontrol->private_value >> 8) & 0xff;
 	int mask = (kcontrol->private_value >> 16) & 0xff;
@@ -2333,13 +2368,9 @@ static int snd_cs4215_put_single(struct 
 		/* First mute outputs, and wait 1/8000 sec (125 us)
 		 * to make sure this takes.  This avoids clicking noises.
 		 */
-		spin_lock_irqsave(&dbri->lock, flags);
-
 		cs4215_setdata(dbri, 1);
 		udelay(125);
 		cs4215_setdata(dbri, 0);
-
-		spin_unlock_irqrestore(&dbri->lock, flags);
 	}
 	return changed;
 }

[-- Attachment #3: Type: text/plain, Size: 373 bytes --]

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

[-- Attachment #4: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/alsa-devel

  reply	other threads:[~2006-09-05 18:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-09-01  7:11 sparc dbri: SMP fixes Krzysztof Helt
2006-09-01 10:30 ` Takashi Iwai
2006-09-05 18:14   ` Krzysztof Helt [this message]
2006-09-05 18:22     ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44fdbe73e9a3e@wp.pl \
    --to=krzysztof.h1@wp.pl \
    --cc=alsa-devel@lists.sourceforge.net \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.