Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Umm, duh, here's the actual patch. :-)
> 
> Nathan Bryant wrote:
> 
>> Doug Ledford wrote:
>>
>>> The attached patch should get me the debugging output I need to solve 
>>> the problem.  If you'll get me the output, then I can likely have a 
>>> working version in short order.
>>
>>
>>
>> Here is a fix. It is diffed against your original 0.08 version, Doug.
>>
>> It makes GETOPTR set the LVI to the hardware fragment preceding the 
>> one that's currently playing. In the case of Quake, that means Quake 
>> must call GETOPTR at least every 3/4ths of a DMA buffer. Hopefully 
>> that requirement should be relaxed enough. The alternate fix is to 
>> modify the completion handlers.
>>
>> I don't see anything obvious in the databook about how to make the 
>> hardware loop infinitely without taking any additional input from us.
>>
>> Comments, please.

If that fixes it, then the real fix is to find the bug in 
i810_get_free_Wwrite_space() and i810_update_lvi().  By using the first 
function to find the available data in the GETOPTR ioctl, then using 
update_lvi(), we *should* be setting the lvi fragment to exactly 31 sg 
segments away from the current index.  If that's failing, and we are 
instead setting lvi == civ, then things will stop and not work.

> 
> 
> 
> ------------------------------------------------------------------------
> 
> --- i810_audio.c.08	Tue Dec  4 19:43:21 2001
> +++ linux/drivers/sound/i810_audio.c	Wed Dec  5 14:52:28 2001
> @@ -197,7 +197,7 @@
>  #define INT_MASK (INT_SEC|INT_PRI|INT_MC|INT_PO|INT_PI|INT_MO|INT_NI|INT_GPI)
>  
>  
> -#define DRIVER_VERSION "0.07"
> +#define DRIVER_VERSION "0.08n"
>  
>  /* magic numbers to protect our data structures */
>  #define I810_CARD_MAGIC		0x5072696E /* "Prin" */
> @@ -1651,7 +1651,10 @@
>  #endif
>  		if (dmabuf->enable != DAC_RUNNING || file->f_flags & O_NONBLOCK)
>  			return 0;
> -		drain_dac(state, 0);
> +		if (!dmabuf->mapped)
> +			drain_dac(state, 0);
> +		else
> +			stop_dac(state);
>  		dmabuf->ready = 0;
>  		dmabuf->swptr = dmabuf->hwptr = 0;
>  		dmabuf->count = dmabuf->total_bytes = 0;
> @@ -1913,16 +1916,31 @@
>  		if (!dmabuf->ready && (val = prog_dmabuf(state, 0)) != 0)
>  			return val;
>  		spin_lock_irqsave(&state->card->lock, flags);
> -		val = i810_get_free_write_space(state);
> +		i810_update_ptr(state);
>  		cinfo.bytes = dmabuf->total_bytes;
>  		cinfo.ptr = dmabuf->hwptr;
> -		cinfo.blocks = val/dmabuf->userfragsize;
> +		/* blocks is only valid in mmap mode, according to API doc */
> +		cinfo.blocks = 0;
>  		if (dmabuf->mapped && (dmabuf->trigger & PCM_ENABLE_OUTPUT)) {
> -			dmabuf->count += val;
> -			dmabuf->swptr = (dmabuf->swptr + val) % dmabuf->dmasize;
> +			/* blocks is supposed to reset to 0 on every call to GETOPTR */
> +			/* hopefully nobody else destroys count so we can use it for this purpose
> +			   in mmap mode */
> +			cinfo.blocks = (dmabuf->dmasize - dmabuf->count - dmabuf->fragsize) /
> +				dmabuf->userfragsize;
> +			dmabuf->count = dmabuf->dmasize - dmabuf->fragsize;
> +			dmabuf->swptr = (dmabuf->dmasize + dmabuf->hwptr - dmabuf->fragsize) 
> +				% dmabuf->dmasize;
> +#ifdef DEBUG
> +			printk("SNDCTL_DSP_GETOPTR: calling __i810_update_lvi for swptr %d\n",
> +				dmabuf->swptr);
> +#endif
>  			__i810_update_lvi(state, 0);
> -			if (!dmabuf->enable)
> +			if (!dmabuf->enable) {
> +#ifdef DEBUG
> +				printk("SNDCTL_DSP_GETOPTR: calling __start_dac\n");
> +#endif
>  				__start_dac(state);
> +			}
>  		}
>  		spin_unlock_irqrestore(&state->card->lock, flags);
>  #ifdef DEBUG
> @@ -2324,7 +2342,12 @@
>  	/* stop DMA state machine and free DMA buffers/channels */
>  	if(dmabuf->enable & DAC_RUNNING ||
>  	   (dmabuf->count && (dmabuf->trigger & PCM_ENABLE_OUTPUT))) {
> -		drain_dac(state,0);
> +		if (!dmabuf->mapped)
> +			drain_dac(state,0);
> +		else {
> +			stop_dac(state);
> +			synchronize_irq();
> +		}
>  	}
>  	if(dmabuf->enable & ADC_RUNNING) {
>  		stop_adc(state);
> 

-- 

  Doug Ledford <[e-mail]>  http://people.redhat.com/dledford
       Please check my web site for aic7xxx updates/answers before
                       e-mailing me about problems