Re: i810 audio patch

Doug Ledford
2001-12-05T20:13:36Z

> 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 <dledford@redhat.com> http://people.redhat.com/dledford
Please check my web site for aic7xxx updates/answers before
e-mailing me about problems