patch: RME9632 precise_ptr fix (9652 owners please read)

patch: RME9632 precise_ptr fix (9652 owners please read)

[Ed Wildgoose]

Hooray.  With some very kind help from Martin Björnsen, I have figured 
out the problem with the rme9652 driver when using the precise_ptr 
option and a fix is shown below.

I have not been able to get in touch with Thomas Charbonnel (original 
author) for a long time now, so I would be grateful if someone on the 
alsa team could commit this to cvs.  Please note that I have only tested 
this on a 9632 card, but the driver also supports the 9652 (and 
others?), so perhaps someone could retest this on those bits of 
hardware.  It's possible that a similar fix could be applied to the 
rme9652.c driver, but I don't really understand which driver is for 
which card here?

The justification for this fix being correct is that the position 
counter used here is a 16 sample counter (it's not really sample 
accurate), and it also counts up continuously, and doesn't reset each 
time the buffer is emptied.  This created a logic error in the original 
code because it was masking against the max buffer size, and not against 
the *current* buffer size. 

With the fix in below, basic testing on mplayer with some debug code 
shows extremely accurate sync rates with the video now, and life is 
rosey again!

By the way, if testing against other hardware shows that this patch 
causes no problems (and works correctly), which I think it will, then we 
can completely remove the accurate_ptr option and make this code the 
default.  If testers could copy me in directly I will submit a new patch 
once a suitable number of responses are received

Thanks

Ed Wildgoose


--- hdsp.c.orig 2004-10-06 14:58:37.052866070 +0100
+++ hdsp.c      2004-10-06 14:59:15.034213752 +0100
@@ -969,8 +969,7 @@

        position &= HDSP_BufferPositionMask;
        position /= 4;
-       position -= 32;
-       position &= (HDSP_CHANNEL_BUFFER_SAMPLES-1);
+        position &= ((hdsp->period_bytes/2)-1);
        return position;
 }




-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: patch: RME9632 precise_ptr fix (9652 owners please read)

[Takashi Iwai]

At Wed, 06 Oct 2004 15:10:18 +0100,
Ed Wildgoose wrote:
> 
> Hooray.  With some very kind help from Martin Björnsen, I have figured 
> out the problem with the rme9652 driver when using the precise_ptr 
> option and a fix is shown below.
> 
> I have not been able to get in touch with Thomas Charbonnel (original 
> author) for a long time now, so I would be grateful if someone on the 
> alsa team could commit this to cvs.

Could you provide the summary, the patch description and
signed-off-by lines for changelog?


thanks,

Takashi


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: patch: RME9632 precise_ptr fix (9652 owners please read)

[Ed Wildgoose]

Takashi Iwai wrote:

>At Wed, 06 Oct 2004 15:10:18 +0100,
>Ed Wildgoose wrote:
>  
>
>>Hooray.  With some very kind help from Martin Björnsen, I have figured 
>>out the problem with the rme9652 driver when using the precise_ptr 
>>option and a fix is shown below.
>>
>>I have not been able to get in touch with Thomas Charbonnel (original 
>>author) for a long time now, so I would be grateful if someone on the 
>>alsa team could commit this to cvs.
>>    
>>
>
>Could you provide the summary, the patch description and
>signed-off-by lines for changelog?
>  
>

Sure, sorry for the delay:

Summary:
Correct hardware position mask to mask correctly when buffer is not 
maximum size.

Signed off:  Ed Wildgoose <lists@wildgooses.com>


Thanks

P.S. Anyone with a 9652 card, I would still be interested to hear if 
this improves things for you?  I think this behaviour could be made the 
default if it works correctly.


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: patch: RME9632 precise_ptr fix (9652 owners please read)

[Takashi Iwai]

At Fri, 08 Oct 2004 23:05:04 +0100,
Ed Wildgoose wrote:
> 
> Takashi Iwai wrote:
> 
> >At Wed, 06 Oct 2004 15:10:18 +0100,
> >Ed Wildgoose wrote:
> >  
> >
> >>Hooray.  With some very kind help from Martin Björnsen, I have figured 
> >>out the problem with the rme9652 driver when using the precise_ptr 
> >>option and a fix is shown below.
> >>
> >>I have not been able to get in touch with Thomas Charbonnel (original 
> >>author) for a long time now, so I would be grateful if someone on the 
> >>alsa team could commit this to cvs.
> >>    
> >>
> >
> >Could you provide the summary, the patch description and
> >signed-off-by lines for changelog?
> >  
> >
> 
> Sure, sorry for the delay:
> 
> Summary:
> Correct hardware position mask to mask correctly when buffer is not 
> maximum size.
> 
> Signed off:  Ed Wildgoose <lists@wildgooses.com>

Thanks, applied to CVS now.


Takashi


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: patch: RME9632 precise_ptr fix (9652 owners please read)

[Ed Wildgoose]

Hi Takashi,

I was just checking the hdsp driver code for a new kernel build and I 
notice that my patch was committed incorrectly.  Notice that the 
"position /= 4;" line has been removed inadvertently in that commit:
http://cvs.sourceforge.net/viewcvs.py/alsa/alsa-kernel/pci/rme9652/hdsp.c?r1=1.71&r2=1.72

See below for original patch as sent to the list:

Summary: Fix accurate_ptr in hdsp driver
Signoff: Ed Wildgoose (lists (at) wildgooses.com)

Thanks

Ed W


> --- hdsp.c.orig 2004-10-06 14:58:37.052866070 +0100
> +++ hdsp.c      2004-10-06 14:59:15.034213752 +0100
> @@ -969,8 +969,7 @@
>
>        position &= HDSP_BufferPositionMask;
>        position /= 4;
> -       position -= 32;
> -       position &= (HDSP_CHANNEL_BUFFER_SAMPLES-1);
> +        position &= ((hdsp->period_bytes/2)-1);
>        return position;
> }




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: patch: RME9632 precise_ptr fix (9652 owners please read)

[Takashi Iwai]

At Thu, 11 Nov 2004 12:50:56 +0000,
Ed Wildgoose wrote:
> 
> Hi Takashi,
> 
> I was just checking the hdsp driver code for a new kernel build and I 
> notice that my patch was committed incorrectly.  Notice that the 
> "position /= 4;" line has been removed inadvertently in that commit:

Thanks, I fixed the CVS now.


Takashi


-------------------------------------------------------
This SF.Net email is sponsored by: InterSystems CACHE
FREE OODBMS DOWNLOAD - A multidimensional database that combines
robust object and relational technologies, making it a perfect match
for Java, C++,COM, XML, ODBC and JDBC. www.intersystems.com/match8