SM501 driver development: problem with start of playback

SM501 driver development: problem with start of playback

[Jonas Eymann]

Hi everyone,

I'm currently developing/ enhancing an ALSA driver for a Silicon Motion SM501 compagnion chip which contains also an 8051 µC acting as AC97 controller. It doesn't support DMA and so the copy of audio data is done in the driver. There is a small hardware buffer of 1920 bytes which is devided into two halves and used as ring buffer. When the hardware has finished playing one half, it sends an interrupt to the driver so it can fill this half with new audio data while the hardware plays from the other one and so on. In the driver, I use an intermediate buffer with vmalloc().

Actually, I have a working driver now, but one small problem remains and I'm absolutely running out of ideas how to fix it: when the playback starts, the hardware expects the buffer to be filled completely (both halves), but the copy routine which does this first filling (snd_sm501_pfill()) only gets one period, not the two which are needed to fill the buffer completely. The second half it just reads zeros. It seems that the intermediate buffer is not yet filled with more data. This results in a gap of one period at the beginning of the playback (it's the second period that doesn't get copied). 
I've tried to change everything to a structure with .copy and .silence callbacks, but ran into even more problems. I read everything I could find about ALSA drivers, looked at code from other ALSA drivers, but still, I just don't get it. I know that the trigger function should be fast and only just triggering the DMA (if there was one), but I don't know where else to put the pfill call. In the prepare callback there is no sound data available yet. Maybe I should change everything completely to another structure, but I have no idea how it should then look like.

Below are some excerpts from the code.

Thank you already very much in advance!

Jonas


/* hw_params callback */
static int snd_sm501_pcm_hw_params(snd_pcm_substream_t *substream,
				   snd_pcm_hw_params_t * hw_params)
{
	KDEBUG("pcm_hw_params\n");

	return snd_sm501_pcm_alloc_vmalloc_buffer(substream,
					params_buffer_bytes(hw_params));
}

/* hw_free callback */
static int snd_sm501_pcm_hw_free(snd_pcm_substream_t *substream)
{
	KDEBUG("pcm_hw_free\n");

	return snd_sm501_pcm_free_vmalloc_buffer(substream);
}


/* prepare callback */
static int snd_sm501_pcm_playback_prepare(snd_pcm_substream_t *substream)
{
	ac97_t *ac97;
	u8 value = 0x00;
	
	snd_pcm_format_t format;
	unsigned int rate;
	unsigned int channels;
	
	psmivgx_t s = snd_pcm_substream_chip(substream);
	
	ac97 = smivgx_drv_data[smivgx_get_id(s)].ac97; 
	format = substream->runtime->format;
	rate = substream->runtime->rate;
	channels = substream->runtime->channels;

	snd_sm501_ac97_write(ac97, AC97_PCM_FRONT_DAC_RATE, rate);
	snd_sm501_ac97_write(ac97, AC97_EXTENDED_STATUS, 0x0001);
	smivgx_pokeword(s, SRAM(pb_bitrate), rate);
	
	if(format == SNDRV_PCM_FORMAT_S16_LE)
		value |= 0x01;		/* set 16 bit sample format bit (otherwise 8 bit) */
	else
		value &= 0xFE;
	if(channels == 2)
		value |= 0x02;		/* set stereo bit (otherwise mono) */
	else
		value &= 0xFD;
	smivgx_pokebyte(s, SRAM(pb_mode_select),value);

	KDEBUG("pcm_playback_prepare\n");

	smivgx_drv_data[smivgx_get_id(s)].ppointer = 0;
	return 0;
}

/* trigger callback */
static int snd_sm501_pcm_playback_trigger(snd_pcm_substream_t *substream,
					  int cmd)
{
	psmivgx_t s = snd_pcm_substream_chip(substream);
	smivgx_drv_data_t *drv_data;

	drv_data = &smivgx_drv_data[smivgx_get_id(s)];

	spin_lock(&s->lock);

	switch (cmd) {
		case SNDRV_PCM_TRIGGER_START:
			
			KDEBUG("pcm_playback_trigger: start\n");
			drv_data->psubstream = substream;
			snd_sm501_pfill(s);
			smivgx_poke(s, U8051_CPU_PROTOCOL_INTERRUPT, START_AUDIO_PLAYBACK);
			break;
		case SNDRV_PCM_TRIGGER_STOP:
			KDEBUG("pcm_playback_trigger: stop\n");
			smivgx_poke(s, U8051_CPU_PROTOCOL_INTERRUPT, STOP_AUDIO_PLAYBACK);
			drv_data->psubstream = NULL;
			break;
		default:
			KDEBUG("pcm_playback_trigger: default (%i)\n", cmd);
			return -EINVAL;
	}

	spin_unlock(&s->lock);

	return 0;
}

/*
 * Fill the playback buffers with audio data
 */
static void snd_sm501_pfill(psmivgx_t s)
{
	smivgx_drv_data_t *drv_data;
	int frames; 
	u32 *pframe;
	u32 *pdest;
	u32 *pborder;
	snd_pcm_runtime_t *runtime;

	drv_data = &smivgx_drv_data[smivgx_get_id(s)];

	if (drv_data->psubstream==NULL) {
		return;
	}
	
	runtime = drv_data->psubstream->runtime;

	if (runtime->dma_area == NULL) {
		return;
	}
	
	frames = bytes_to_frames(runtime, SM501_AC97_P_PERIOD_SIZE);
	
	pframe = (u32*) (runtime->dma_area
			     + frames_to_bytes(runtime, drv_data->ppointer));
	
	
	pdest = (u32*) SRAM(playback_buffer_0);
	
	pborder = pdest + 2*(SM501_AC97_P_PERIOD_SIZE / sizeof(*pdest));
	do {	
		smivgx_poke(drv_data->device, (unsigned int) pdest,*pframe);
		pdest++;
		pframe++;
	
	} while (pdest < pborder);
	
	drv_data->ppointer_hiddenoffset = 2*frames;
}

/* 
 * Copy playback data into the hardware buffer
 * This function can also be called as tasklet!
 */
static void snd_sm501_copy_pdata(unsigned long private_data)
{
	smivgx_drv_data_t *drv_data = (smivgx_drv_data_t *)private_data;
	int frames; 
	u32 *pframe;
	u32 *pdest;
	u32 *pborder;
	snd_pcm_runtime_t *runtime;
	unsigned char buffer_req;
	unsigned long flags; 

	spin_lock_irqsave(&drv_data->device->lock, flags);
	
	if (drv_data->psubstream==NULL) {
		spin_unlock_irqrestore(&drv_data->device->lock, flags);
		return;
	}
	
	xboardgp8_gpio_direction_set(1, 1);
	xboardgp8_gpio_set(1,1);

	runtime = drv_data->psubstream->runtime;

	if (runtime->dma_area == NULL) {
		spin_unlock_irqrestore(&drv_data->device->lock, flags);
		return;
	}
	
	frames = bytes_to_frames(runtime, SM501_AC97_P_PERIOD_SIZE);
	
	pframe = (u32*) (runtime->dma_area
				+ frames_to_bytes(runtime, drv_data->ppointer 
				+ drv_data->ppointer_hiddenoffset));
	
	
	buffer_req = smivgx_peekbyte(drv_data->device, SRAM(buffer_request));
	if (buffer_req & 0x1) 
	{
		pdest = (u32*) SRAM(playback_buffer_0);
	}
	else /* if (buffer_req & 0x2) */
	{
		pdest = (u32*) SRAM(playback_buffer_1);
	}
		
	pborder = pdest + (SM501_AC97_P_PERIOD_SIZE / sizeof(*pdest));

	do {
		smivgx_poke(drv_data->device, (unsigned int) pdest,*pframe);
		pdest++;
		pframe++;
	} while (pdest < pborder);

	drv_data->ppointer += frames;
	drv_data->ppointer_hiddenoffset = 0;
	drv_data->ppointer %= frames * runtime->periods;
	
	spin_unlock_irqrestore(&drv_data->device->lock, flags);
	
	snd_pcm_period_elapsed(drv_data->psubstream);
	
	xboardgp8_gpio_set(1,0);
}


/* interrupt service routine */
static irqreturn_t snd_sm501_interrupt(int irq, void* dev_id,
				       struct pt_regs* regs)
{
/* ... */
	
	if (buffer_req) 
	{
		smivgx_drv_data_t *drv_data;
		drv_data = &smivgx_drv_data[smivgx_get_id(s)];
		if(buffer_req & 0x03) /* Playback */
		{
			snd_sm501_copy_pdata((unsigned long) drv_data);
		}
		if(buffer_req & 0x0C) /* Capture */
		{
			snd_sm501_copy_cdata((unsigned long) drv_data);
		}
	}

	return IRQ_HANDLED;
}

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: SM501 driver development: problem with start of playback

[Takashi Iwai]

At Wed, 22 Nov 2006 10:44:55 +0100,
Jonas Eymann wrote:
> 
> Hi everyone,
> 
> I'm currently developing/ enhancing an ALSA driver for a Silicon
> Motion SM501 compagnion chip which contains also an 8051 µC acting
> as AC97 controller. It doesn't support DMA and so the copy of audio
> data is done in the driver. There is a small hardware buffer of 1920
> bytes which is devided into two halves and used as ring buffer. When
> the hardware has finished playing one half, it sends an interrupt to
> the driver so it can fill this half with new audio data while the
> hardware plays from the other one and so on. In the driver, I use an
> intermediate buffer with vmalloc(). 
> 
> Actually, I have a working driver now, but one small problem remains
> and I'm absolutely running out of ideas how to fix it: when the
> playback starts, the hardware expects the buffer to be filled
> completely (both halves), but the copy routine which does this first
> filling (snd_sm501_pfill()) only gets one period, not the two which
> are needed to fill the buffer completely. The second half it just
> reads zeros. It seems that the intermediate buffer is not yet filled
> with more data. This results in a gap of one period at the beginning
> of the playback (it's the second period that doesn't get copied).  

Hm, this kind of implementation gets more common nowadays for small
devices, so we may need a better/simpler framework in furture.

Usually, the filler routine should fill the data as much as possible
at the time it's called.  The size of available data on the
itermediate buffer can be calculated via PCM appl_ptr.

Possibly, helper functions defined in pcm-indirect.h could be used in
your case.  Check the alsa-devel list archive in the last months,
threads "Driver design question".  The discussions went on there
regarding a driver implementation without DMA.  (The pcm-indirect.h
routines didn't work for his case well, unforutnately, though...)


Takashi

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: SM501 driver development: problem with start of playback

[Jonas Eymann]

> At Wed, 22 Nov 2006 10:44:55 +0100,
> Jonas Eymann wrote:
> > 
> > Hi everyone,
> > 
> > I'm currently developing/ enhancing an ALSA driver for a Silicon
> > Motion SM501 compagnion chip which contains also an 8051 µC acting
> > as AC97 controller. It doesn't support DMA and so the copy of audio
> > data is done in the driver. There is a small hardware buffer of 1920
> > bytes which is devided into two halves and used as ring buffer. When
> > the hardware has finished playing one half, it sends an interrupt to
> > the driver so it can fill this half with new audio data while the
> > hardware plays from the other one and so on. In the driver, I use an
> > intermediate buffer with vmalloc(). 
> > 
> > Actually, I have a working driver now, but one small problem remains
> > and I'm absolutely running out of ideas how to fix it: when the
> > playback starts, the hardware expects the buffer to be filled
> > completely (both halves), but the copy routine which does this first
> > filling (snd_sm501_pfill()) only gets one period, not the two which
> > are needed to fill the buffer completely. The second half it just
> > reads zeros. It seems that the intermediate buffer is not yet filled
> > with more data. This results in a gap of one period at the beginning
> > of the playback (it's the second period that doesn't get copied).  
> 
> Hm, this kind of implementation gets more common nowadays for small
> devices, so we may need a better/simpler framework in furture.
> 
> Usually, the filler routine should fill the data as much as possible
> at the time it's called.  The size of available data on the
> itermediate buffer can be calculated via PCM appl_ptr.
> 
> Possibly, helper functions defined in pcm-indirect.h could be used in
> your case.  Check the alsa-devel list archive in the last months,
> threads "Driver design question".  The discussions went on there
> regarding a driver implementation without DMA.  (The pcm-indirect.h
> routines didn't work for his case well, unforutnately, though...)
> 
> 
> Takashi

Thanks for your answer, Takashi!

when I check the appl_ptr in the trigger callback, it gives me exactly one period of available data. As soon as we are in the copy function called from the isr (snd_sm501_copy_pdata), the appl_ptr is a few thousand periods further. Is there a simple way to change the default value for this first portion of available value?
I'm also trying to implement the driver using the pcm-indirect.h. I just thought maybe there is a simpler solution.

Thx for your answers!

Jonas

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: SM501 driver development: problem with start of playback

[Jaroslav Kysela]

On Mon, 27 Nov 2006, Jonas Eymann wrote:

> when I check the appl_ptr in the trigger callback, it gives me exactly 
> one period of available data. As soon as we are in the copy function 

Which tool do you use for tests? Could you try aplay and send us also 
debug output (aplay -v <xxxx>)? It might be sw_params setup problem.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SUSE Labs

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: SM501 driver development: problem with start of playback

[Takashi Iwai]

At Mon, 27 Nov 2006 19:28:38 +0100,
Jonas Eymann wrote:
> 
> 
> > At Wed, 22 Nov 2006 10:44:55 +0100,
> > Jonas Eymann wrote:
> > > 
> > > Hi everyone,
> > > 
> > > I'm currently developing/ enhancing an ALSA driver for a Silicon
> > > Motion SM501 compagnion chip which contains also an 8051 µC acting
> > > as AC97 controller. It doesn't support DMA and so the copy of audio
> > > data is done in the driver. There is a small hardware buffer of 1920
> > > bytes which is devided into two halves and used as ring buffer. When
> > > the hardware has finished playing one half, it sends an interrupt to
> > > the driver so it can fill this half with new audio data while the
> > > hardware plays from the other one and so on. In the driver, I use an
> > > intermediate buffer with vmalloc(). 
> > > 
> > > Actually, I have a working driver now, but one small problem remains
> > > and I'm absolutely running out of ideas how to fix it: when the
> > > playback starts, the hardware expects the buffer to be filled
> > > completely (both halves), but the copy routine which does this first
> > > filling (snd_sm501_pfill()) only gets one period, not the two which
> > > are needed to fill the buffer completely. The second half it just
> > > reads zeros. It seems that the intermediate buffer is not yet filled
> > > with more data. This results in a gap of one period at the beginning
> > > of the playback (it's the second period that doesn't get copied).  
> > 
> > Hm, this kind of implementation gets more common nowadays for small
> > devices, so we may need a better/simpler framework in furture.
> > 
> > Usually, the filler routine should fill the data as much as possible
> > at the time it's called.  The size of available data on the
> > itermediate buffer can be calculated via PCM appl_ptr.
> > 
> > Possibly, helper functions defined in pcm-indirect.h could be used in
> > your case.  Check the alsa-devel list archive in the last months,
> > threads "Driver design question".  The discussions went on there
> > regarding a driver implementation without DMA.  (The pcm-indirect.h
> > routines didn't work for his case well, unforutnately, though...)
> > 
> > 
> > Takashi
> 
> Thanks for your answer, Takashi!
> 
> when I check the appl_ptr in the trigger callback, it gives me
> exactly one period of available data.

It means that you set the minimum periods to 1.  When it's set to 2,
the trigger will be called after two periods are filled (unless the
app call snd_pcm_start() explicitly with underfilled stream).

(BTW, don't forget Cc to alsa-devel ML.)


Takashi

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV