Skip to contentSkip to conversation

V4L (926) Saa7134 alsa can only be autoloaded after saa7134 is active

PATCH In review [email protected] 4 participants 6 messages Nov 7, 2005 – Nov 9, 2005 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 V4L (926) Saa7134 alsa can only be autoloaded after saa7134 is active

From: Ricardo Cerqueira <[email protected]>

- Saa7134-alsa can only be autoloaded after saa7134 is active
- Applied pertinent changes proposed by the ALSA team
- dsp_nr replaced by ALSA's index[]

-----------------

 drivers/media/video/saa7134/saa7134-alsa.c |  240 ++++++++++------------
 drivers/media/video/saa7134/saa7134-core.c |   27 +-
 drivers/media/video/saa7134/saa7134-oss.c  |  308 ++++++++++++++---------------
 drivers/media/video/saa7134/saa7134.h      |    6 
 4 files changed, 289 insertions(+), 292 deletions(-)
drivers/media/video/saa7134/saa7134-core.c +17 -10
@@ -194,6 +194,7 @@ void saa7134_track_gpio(struct saa7134_d
 
 static int need_empress;
 static int need_dvb;
+static int need_alsa;
 
 static int pending_call(struct notifier_block *self, unsigned long state,
 			void *module)
@@ -205,6 +206,8 @@ static int pending_call(struct notifier_
 		request_module("saa7134-empress");
 	if (need_dvb)
 		request_module("saa7134-dvb");
+	if (need_alsa)
+		request_module("saa7134-alsa");
 	return NOTIFY_DONE;
 }
 
@@ -469,7 +472,7 @@ int saa7134_set_dmabits(struct saa7134_d
 	}
 
 	/* audio capture -- dma 3 */
-	if (dev->oss.dma_running) {
+	if (dev->dmasound.dma_running) {
 		ctrl |= SAA7134_MAIN_CTRL_TE6;
 		irq  |= SAA7134_IRQ1_INTE_RA3_1 |
 			SAA7134_IRQ1_INTE_RA3_0;
@@ -983,6 +986,12 @@ static int __devinit saa7134_initdev(str
 	if (card_is_dvb(dev))
 		request_module_depend("saa7134-dvb",&need_dvb);
 
+	if (!oss && alsa) {
+		dprintk("Requesting ALSA module\n");
+		request_module_depend("saa7134-alsa",&need_alsa);
+	}
+
+
 	v4l2_prio_init(&dev->prio);
 
 	/* register v4l devices */
@@ -1021,24 +1030,22 @@ static int __devinit saa7134_initdev(str
 	case PCI_DEVICE_ID_PHILIPS_SAA7133:
 	case PCI_DEVICE_ID_PHILIPS_SAA7135:
 		if (oss) {
-			err = dev->oss.minor_dsp =
+			err = dev->dmasound.minor_dsp =
 				register_sound_dsp(&saa7134_dsp_fops,
 						   dsp_nr[dev->nr]);
 			if (err < 0) {
 				goto fail4;
 			}
 			printk(KERN_INFO "%s: registered device dsp%d\n",
-			       dev->name,dev->oss.minor_dsp >> 4);
+			       dev->name,dev->dmasound.minor_dsp >> 4);
 
-			err = dev->oss.minor_mixer =
+			err = dev->dmasound.minor_mixer =
 				register_sound_mixer(&saa7134_mixer_fops,
 						     mixer_nr[dev->nr]);
 			if (err < 0)
 				goto fail5;
 			printk(KERN_INFO "%s: registered device mixer%d\n",
-			       dev->name,dev->oss.minor_mixer >> 4);
-		} else if (alsa) {
-			request_module("saa7134-alsa");
+			       dev->name,dev->dmasound.minor_mixer >> 4);
 		}
 		break;
 	}
@@ -1065,7 +1072,7 @@ static int __devinit saa7134_initdev(str
 	case PCI_DEVICE_ID_PHILIPS_SAA7133:
 	case PCI_DEVICE_ID_PHILIPS_SAA7135:
 		if (oss)
-			unregister_sound_dsp(dev->oss.minor_dsp);
+			unregister_sound_dsp(dev->dmasound.minor_dsp);
 		break;
 	}
  fail4:
@@ -1123,8 +1130,8 @@ static void __devexit saa7134_finidev(st
 	case PCI_DEVICE_ID_PHILIPS_SAA7133:
 	case PCI_DEVICE_ID_PHILIPS_SAA7135:
 		if (oss) {
-			unregister_sound_mixer(dev->oss.minor_mixer);
-			unregister_sound_dsp(dev->oss.minor_dsp);
+			unregister_sound_mixer(dev->dmasound.minor_mixer);
+			unregister_sound_dsp(dev->dmasound.minor_dsp);
 		}
 		break;
 	}
drivers/media/video/saa7134/saa7134-alsa.c +115 -125

514 lines not on this page · raw message

drivers/media/video/saa7134/saa7134-oss.c +154 -154

675 lines not on this page · raw message

drivers/media/video/saa7134/saa7134.h +3 -3
@@ -355,8 +355,8 @@ struct saa7134_fh {
 	struct saa7134_pgtable     pt_vbi;
 };
 
-/* oss dsp status */
-struct saa7134_oss {
+/* dmasound dsp status */
+struct saa7134_dmasound {
 	struct semaphore           lock;
 	int                        minor_mixer;
 	int                        minor_dsp;
@@ -431,7 +431,7 @@ struct saa7134_dev {
 	struct video_device        *video_dev;
 	struct video_device        *radio_dev;
 	struct video_device        *vbi_dev;
-	struct saa7134_oss         oss;
+	struct saa7134_dmasound    dmasound;
 
 	/* infrared remote */
 	int                        has_remote;

_______________________________________________________
Yahoo! Acesso Gr�tis: Internet r�pida e gr�tis.
Instale o discador agora!
http://br.acesso.yahoo.com/

#1 lore raw

At Mon, 07 Nov 2005 18:48:18 -0200,

Quoted 9 lines · [email protected] wrote:

From: Ricardo Cerqueira <[email protected]>

- Saa7134-alsa can only be autoloaded after saa7134 is active
- Applied pertinent changes proposed by the ALSA team
- dsp_nr replaced by ALSA's index[]

Thanks for the quick rewrite!

Quoted 6 lines
drivers/media/video/saa7134/saa7134-alsa.c +0 -0
@@ -186,30 +171,31 @@ void saa7134_irq_alsa_done(struct saa713
 		goto done;
 	}
 

drivers/media/video/saa7134/saa7134-alsa.c void saa7134_irq_alsa_done(struct saa713 · line 180

Quoted 10 lines
drivers/media/video/saa7134/saa7134-alsa.c +4 -3
-	if (dev->oss.read_count >= dev->oss.blksize * (dev->oss.blocks-2)) {
-		dprintk("irq: overrun [full=%d/%d] - Blocks in %d\n",dev->oss.read_count,
-			dev->oss.bufsize, dev->oss.blocks);
+	if (dev->dmasound.read_count >= dev->dmasound.blksize * (dev->dmasound.blocks-2)) {
+		dprintk("irq: overrun [full=%d/%d] - Blocks in %d\n",dev->dmasound.read_count,
+			dev->dmasound.bufsize, dev->dmasound.blocks);
+		snd_pcm_stop(dev->dmasound.substream,SNDRV_PCM_STATE_XRUN);
 		saa7134_dma_stop(dev);
 		goto done;
 	}

You need spin_unlock(&dev->slock) before calling snd_pcm_stop() since this lock is used in trigger callback, too. Note that "goto done" may require the lock again. Also, the succeeding saa7134_dma_stop() is superfluous.

Quoted 8 lines
drivers/media/video/saa7134/saa7134-alsa.c +1 -2
@@ -374,13 +376,13 @@ static int snd_card_saa7134_capture_prep
 		goto fail2;
 
 	/* prepare buffer */
-	if (0 != (err = videobuf_dma_pci_map(dev->pci,&dev->oss.dma)))
+	if (0 != (err = videobuf_dma_pci_map(dev->pci,&dev->dmasound.dma)))
 		return err;
-	if (0 != (err = saa7134_pgtable_alloc(dev->pci,&dev->oss.pt)))

drivers/media/video/saa7134/saa7134-alsa.c static int snd_card_saa7134_capture_prep · line 387

Quoted 10 lines
drivers/media/video/saa7134/saa7134-alsa.c +4 -3
+	if (0 != (err = saa7134_pgtable_alloc(dev->pci,&dev->dmasound.pt)))
 		goto fail1;
-	if (0 != (err = saa7134_pgtable_build(dev->pci,&dev->oss.pt,
-					      dev->oss.dma.sglist,
-					      dev->oss.dma.sglen,
+	if (0 != (err = saa7134_pgtable_build(dev->pci,&dev->dmasound.pt,
+					      dev->dmasound.dma.sglist,
+					      dev->dmasound.dma.sglen,
 					      0)))
 		goto fail2;

The buffer allocation would be better to be done in hw_params callback because prepare callback can be frequently called (although it works in prepare callback, too).
Or, at least, you shoud cache the value in the same stream.

drivers/media/video/saa7134/saa7134-alsa.c static int snd_saa7134_capsrc_put(snd_kc · line 815

Quoted 7 lines
drivers/media/video/saa7134/saa7134-alsa.c +1 -1
@@ -818,7 +811,7 @@ static int snd_saa7134_capsrc_put(snd_kc
 		 chip->capture_source[addr][1] != right;
 	chip->capture_source[addr][0] = left;
 	chip->capture_source[addr][1] = right;
-	dev->oss.input=addr;
+	dev->dmasound.input=addr;
 	spin_unlock_irqrestore(&chip->mixer_lock, flags);

You can use spin_unlock_irq() safely here.

drivers/media/video/saa7134/saa7134-alsa.c int alsa_card_saa7134_create (struct saa · line 967

Quoted 7 lines
drivers/media/video/saa7134/saa7134-alsa.c +1 -1
@@ -973,18 +963,17 @@ int alsa_card_saa7134_create (struct saa
 	chip->irq = saadev->pci->irq;
 	chip->iobase = pci_resource_start(saadev->pci, 0);
 
-	err = request_irq(chip->pci->irq, saa7134_alsa_irq,
+	err = request_irq(saadev->pci->irq, saa7134_alsa_irq,
 				SA_SHIRQ | SA_INTERRUPT, saadev->name, saadev);

Hmm, still I don't see any call of free_irq() in the driver?

drivers/media/video/saa7134/saa7134-alsa.c int alsa_card_saa7134_create (struct saa · line 1000

Quoted 6 lines
drivers/media/video/saa7134/saa7134-alsa.c +1 -1
@@ -1010,7 +997,7 @@ int alsa_card_saa7134_create (struct saa
 
 __nodev:
 	snd_card_free(card);
-	kfree(card);
+	kfree(chip);

It's not safe to call kfree(chip) here, too. This can be already released, once after snd_device_new() is succeeded.
In your case, it's easier to use the following style:

	err = snd_card_new(index[dev], id[dev], THIS_MODULE,
				sizeof(snd_card_saa7134_t));
	chip = card->private_data;
	card->private_free = snd_saa7134_free;

so that no extra kfree() is needed.

Takashi

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#2 lore raw

On Tue, 2005-11-08 at 12:36 +0100, Takashi Iwai wrote:
[...]

Quoted 4 lines

Note that "goto done" may
require the lock again. Also, the succeeding saa7134_dma_stop() is superfluous.

No, it's not. dma_stop also affects the main saa7134 module, and it's needed there.

[...]

Quoted 10 lines
(quoted diff) +0 -0
@@ -973,18 +963,17 @@ int alsa_card_saa7134_create (struct saa
 >  	chip->irq = saadev->pci->irq;
 >  	chip->iobase = pci_resource_start(saadev->pci, 0);

drivers/media/video/saa7134/saa7134-alsa.c int alsa_card_saa7134_create (struct saa · line 967

Quoted 4 lines
drivers/media/video/saa7134/saa7134-alsa.c +1 -1
-	err = request_irq(chip->pci->irq, saa7134_alsa_irq,
+	err = request_irq(saadev->pci->irq, saa7134_alsa_irq,
 				SA_SHIRQ | SA_INTERRUPT, saadev->name, saadev);

Hmm, still I don't see any call of free_irq() in the driver?

It was added to v4l CVS shortly after Mauro sent the last patchset, should be sent upstream in the next patchset.
[...]

--
RC

#3 lore raw

At Tue, 08 Nov 2005 12:07:50 +0000,

Quoted 11 lines · R C

On Tue, 2005-11-08 at 12:36 +0100, Takashi Iwai wrote:
[...]

Quoted 4 lines

Note that "goto done" may
require the lock again. Also, the succeeding saa7134_dma_stop() is superfluous.

No, it's not. dma_stop also affects the main saa7134 module, and it's needed there.

But saa7134_dma_stop() should be already called in trigger callback, which is called via snd_pcm_stop().

Takashi

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#4 lore raw
Quoted 2 lines · Takashi Iwai

But saa7134_dma_stop() should be already called in trigger callback, which is called via snd_pcm_stop().

You're right. I wasn't aware pcm_stop() caused a trigger. You may want to check the current version at
http://linuxtv.org/cgi-bin/viewcvs.cgi/v4l-kernel/linux/drivers/media/video/saa7134/saa7134-alsa.c?rev=1.21&root=v4l&view=auto

--
RC

--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

#5 lore raw
Quoted 8 lines · R C
Quoted 2 lines · Takashi Iwai

But saa7134_dma_stop() should be already called in trigger callback, which is called via snd_pcm_stop().

You're right. I wasn't aware pcm_stop() caused a trigger. You may want to check the current version at
http://linuxtv.org/cgi-bin/viewcvs.cgi/v4l-kernel/linux/drivers/media/video/saa7134/saa7134-alsa.c?rev=1.21&root=v4l&view=auto

Did you not read Takashi-san's ALSA driver guide? If not I'm impressed you made it work at all...

http://www.alsa-project.org/~iwai/writing-an-alsa-driver/

Lee