Re: i810 audio patch

[Doug Ledford]

Nathan Bryant wrote:

> Doug Ledford wrote:
> 
>> If that fixes it, then the real fix is to find the bug in 
>> i810_get_free_Wwrite_space() and i810_update_lvi(). 
> 
> 
> It does fix it.
> 
> By the way, I'm confused about something. i810_write appears to overrun 
> the end of the DMA buffer instead of wrapping around to the beginning 
> when it does copy_from_user. which makes no sense to me.

This didn't used to be a bug, but it is now (all these changes and 
merges have let some thigns slip in I'm afraid, and I'm not just 
referring to the work in the last few days, I'm also referring to 
changes that were lost in the time of the S/PDIF and PM merges).

Anyway, in i810_write, the lines that read:

if (cnt > (dmabuf->dmasize - dmabuf->count))
	cnt = dmabuf->dmasize - dmabuf->count;

should read:

if (cnt > (dmabuf->dmasize - swptr))
	cnt = dmabuf->dmasize - swptr;

> so ok, the correct number is 31/32nds, not 3/4ths - not so bad.
> 
>> 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.
> 
> 
> yeah, i think that's what's happening

Can you add a debug check to update_lvi()?  Something like:

#ifdef DEBUG_MMAP
         if (dmabuf->count > dmabuf->fragsize && inb(port+OFF_CIV) == x)
                 printk(KERN_DEBUG,"i810_audio: update_lvi - CIV == LVI\n");
#endif

and see if that triggers with the original mmap code?

-- 

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