Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

[Zoilo Gomez]

(Sorry for cross-posting from alsa-user; however, on 2nd thought I think
this really belongs in alsa-devel):

===

I think I have found what is causing this problem: in ac97_patch.c / int
patch_vt1617a(...)  there is a line of code:

>>> snd_ac97_write_cache(a97, 0x5c, 0x20);

According to a comment, this code is supposed to "bring the analog power
consumption to normal, like WinXP driver for EPIA SP".

If I comment out this line, the problem appears to be solved ... I can
now put all volume controls to maximum values and the sound will keep
playing.

This is consistent with the apparent introduction of this bug in kernel
2.6.19 (includes alsa-driver version 1.0.12rc1): no such problem
occurred until 2.6.18, but all kernels since 2.6.19 do suffer from this
problem. The line of code above first shows up in linux-2.6.19.

Unfortunately, since I do not have a datasheet for the VT1617A chip set,
I cannot verify the exact semantics, or suggest an improvement.

Can anyone with a datasheet please suggest a proper patch to this line
of code?

TIA,

Z.

======

Zoilo Gomez wrote:
> In the mean time I have found that kernels 2.6.15 and 2.6.16 do not 
> suffer from this problem, whereas 2.6.22 till 2.6.25 do.
>
> Testing for kernels 2.6.17 till 2.6.21 is somewhat complicated in my 
> setup, because there are other problems (NFS-related) which I need to 
> sort out.
>
> So it looks like there is a bug somehow.
>
> Providing the dxs_support parameter does not make any difference, no 
> matter what value I set it to.
>
> Z.
>
>
> Zoilo Gomez wrote:
>> I have several J7F2 boards from Jetway, containing a 8237 via sound 
>> device on the south bridge.
>>
>> Using the alsa-driver, when I increase the volume then at some point 
>> all over sudden the sound stops. On some boards this happens at 75%, 
>> on others already at 60%, or 90%. Bringing the volume down again does 
>> not bring the audio back ...
>>
>> I am using the alsa-drivers with MPlayer.
>>
>> Although to me this smells like a hardware problem, I understand from 
>> googling around that this can be fixed with software, but I cannot 
>> find the correct solution anywhere. I have tested with gentoo kernels 
>> 2.6.22 and 2.6.24.
>>
>> AFAIK there are only 2 ways to get the sound back once it has stopped:
>>
>> 1) reset the mobo
>> 2) rmmod snd-via82xx, followed by modprobe snd-via82xx
>>
>> The strange thing is that in option 2), the problem seems to be gone 
>> once the driver has been reloaded; from that moment on it is working 
>> fine ....!
>>
>> There is a driver from Via Arena (viaudiocombo) that seems to work 
>> fine, but it is OSS i.o. ALSA, and also a development by Via Arena 
>> i.o. the Linux community. Besides, I need synchronization features 
>> from the audio driver, and I do not think that OSS support this (not 
>> 100% sure).
>>
>>
>> Can anyone help me and shed light on this?
>>
>>
>> TIA!
>>
>> Zoilo.
>>
>> ------------------------------------------------------------------------- 
>>
>> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
>> Don't miss this year's exciting event. There's still time to save 
>> $100. Use priority code J8TL2D2. 
>> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone 
>>
>> _______________________________________________
>> Alsa-user mailing list
>> Alsa-user@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/alsa-user
>>
>>
>>   
>
>
>

Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

[Takashi Iwai]

At Mon, 28 Apr 2008 23:39:04 +0200,
Zoilo Gomez wrote:
> 
> (Sorry for cross-posting from alsa-user; however, on 2nd thought I think
> this really belongs in alsa-devel):
> 
> ===
> 
> I think I have found what is causing this problem: in ac97_patch.c / int
> patch_vt1617a(...)  there is a line of code:
> 
> >>> snd_ac97_write_cache(a97, 0x5c, 0x20);
> 
> According to a comment, this code is supposed to "bring the analog power
> consumption to normal, like WinXP driver for EPIA SP".
> 
> If I comment out this line, the problem appears to be solved ... I can
> now put all volume controls to maximum values and the sound will keep
> playing.

Appreciated that you hunted it out.
Added Cc to the patch author.

> This is consistent with the apparent introduction of this bug in kernel
> 2.6.19 (includes alsa-driver version 1.0.12rc1): no such problem
> occurred until 2.6.18, but all kernels since 2.6.19 do suffer from this
> problem. The line of code above first shows up in linux-2.6.19.
> 
> Unfortunately, since I do not have a datasheet for the VT1617A chip set,
> I cannot verify the exact semantics, or suggest an improvement.
> 
> Can anyone with a datasheet please suggest a proper patch to this line
> of code?

The register 0x5c is the VIA specific one.  I have a VT1617 (without
A) datasheet, and it suggests that the bit corresponds to the
"headphone amplifier temperature sensing control".  And setting this
bit means to _disable_ the temperature sensing control.  This sounds
rather the correct to set.

However, I don't know whether any difference exists betweeen VIA1617
and 1617A although the codec id of both are identical.

Andrey, any comments about your patch?


In anyway, it'd be helpful if we can know which ac97 registers work
and wich not.  Please take /proc/asound/card0/codec97#*/ac97#*-regs
file in both working and non-working cases to compare.  Especially,
the registers 0x5a and 0x5c look interesting.


Thanks,

Takashi



> TIA,
> 
> Z.
> 
> ======
> 
> Zoilo Gomez wrote:
> > In the mean time I have found that kernels 2.6.15 and 2.6.16 do not 
> > suffer from this problem, whereas 2.6.22 till 2.6.25 do.
> >
> > Testing for kernels 2.6.17 till 2.6.21 is somewhat complicated in my 
> > setup, because there are other problems (NFS-related) which I need to 
> > sort out.
> >
> > So it looks like there is a bug somehow.
> >
> > Providing the dxs_support parameter does not make any difference, no 
> > matter what value I set it to.
> >
> > Z.
> >
> >
> > Zoilo Gomez wrote:
> >> I have several J7F2 boards from Jetway, containing a 8237 via sound 
> >> device on the south bridge.
> >>
> >> Using the alsa-driver, when I increase the volume then at some point 
> >> all over sudden the sound stops. On some boards this happens at 75%, 
> >> on others already at 60%, or 90%. Bringing the volume down again does 
> >> not bring the audio back ...
> >>
> >> I am using the alsa-drivers with MPlayer.
> >>
> >> Although to me this smells like a hardware problem, I understand from 
> >> googling around that this can be fixed with software, but I cannot 
> >> find the correct solution anywhere. I have tested with gentoo kernels 
> >> 2.6.22 and 2.6.24.
> >>
> >> AFAIK there are only 2 ways to get the sound back once it has stopped:
> >>
> >> 1) reset the mobo
> >> 2) rmmod snd-via82xx, followed by modprobe snd-via82xx
> >>
> >> The strange thing is that in option 2), the problem seems to be gone 
> >> once the driver has been reloaded; from that moment on it is working 
> >> fine ....!
> >>
> >> There is a driver from Via Arena (viaudiocombo) that seems to work 
> >> fine, but it is OSS i.o. ALSA, and also a development by Via Arena 
> >> i.o. the Linux community. Besides, I need synchronization features 
> >> from the audio driver, and I do not think that OSS support this (not 
> >> 100% sure).
> >>
> >>
> >> Can anyone help me and shed light on this?
> >>
> >>
> >> TIA!
> >>
> >> Zoilo.
> >>
> >> ------------------------------------------------------------------------- 
> >>
> >> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
> >> Don't miss this year's exciting event. There's still time to save 
> >> $100. Use priority code J8TL2D2. 
> >> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone 
> >>
> >> _______________________________________________
> >> Alsa-user mailing list
> >> Alsa-user@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/alsa-user
> >>
> >>
> >>   
> >
> >
> >
> 
> 
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

[Zoilo Gomez]

Takashi Iwai wrote:
> At Mon, 28 Apr 2008 23:39:04 +0200,
> Zoilo Gomez wrote:
>   
>> (Sorry for cross-posting from alsa-user; however, on 2nd thought I think
>> this really belongs in alsa-devel):
>>
>> ===
>>
>> I think I have found what is causing this problem: in ac97_patch.c / int
>> patch_vt1617a(...)  there is a line of code:
>>
>>     
>>>>> snd_ac97_write_cache(a97, 0x5c, 0x20);
>>>>>           
>> According to a comment, this code is supposed to "bring the analog power
>> consumption to normal, like WinXP driver for EPIA SP".
>>
>> If I comment out this line, the problem appears to be solved ... I can
>> now put all volume controls to maximum values and the sound will keep
>> playing.
>>     
>
> Appreciated that you hunted it out.
> Added Cc to the patch author.
>   

Thank you for your response, Takashi!

>> This is consistent with the apparent introduction of this bug in kernel
>> 2.6.19 (includes alsa-driver version 1.0.12rc1): no such problem
>> occurred until 2.6.18, but all kernels since 2.6.19 do suffer from this
>> problem. The line of code above first shows up in linux-2.6.19.
>>
>> Unfortunately, since I do not have a datasheet for the VT1617A chip set,
>> I cannot verify the exact semantics, or suggest an improvement.
>>
>> Can anyone with a datasheet please suggest a proper patch to this line
>> of code?
>>     
>
> The register 0x5c is the VIA specific one.  I have a VT1617 (without
> A) datasheet, and it suggests that the bit corresponds to the
> "headphone amplifier temperature sensing control".  And setting this
> bit means to _disable_ the temperature sensing control.  This sounds
> rather the correct to set.
>
> However, I don't know whether any difference exists betweeen VIA1617
> and 1617A although the codec id of both are identical.
>
> Andrey, any comments about your patch?
>
>
> In anyway, it'd be helpful if we can know which ac97 registers work
> and wich not.  Please take /proc/asound/card0/codec97#*/ac97#*-regs
> file in both working and non-working cases to compare.  Especially,
> the registers 0x5a and 0x5c look interesting.
>   

Code containing "snd_ac97_write_cache(a97, 0x5c, 0x20)":
0x5a = 8300
0x5c = 0000

Code with "snd_ac97_write_cache(a97, 0x5c, 0x20)" commented out:
0x5a = 8301
0x5c = 0020

Quite a surprise to me, I would have expected exactly the opposite .....!?


Can you explain this? /proc/asound/card0/codec97#0/ac97#0-0+regs does 
dump current registers, correct? Or is it a toggling bit?

Thank you,

Zoilo.




>
> Thanks,
>
> Takashi
>
>
>
>   
>> TIA,
>>
>> Z.
>>
>> ======
>>
>> Zoilo Gomez wrote:
>>     
>>> In the mean time I have found that kernels 2.6.15 and 2.6.16 do not 
>>> suffer from this problem, whereas 2.6.22 till 2.6.25 do.
>>>
>>> Testing for kernels 2.6.17 till 2.6.21 is somewhat complicated in my 
>>> setup, because there are other problems (NFS-related) which I need to 
>>> sort out.
>>>
>>> So it looks like there is a bug somehow.
>>>
>>> Providing the dxs_support parameter does not make any difference, no 
>>> matter what value I set it to.
>>>
>>> Z.
>>>
>>>
>>> Zoilo Gomez wrote:
>>>       
>>>> I have several J7F2 boards from Jetway, containing a 8237 via sound 
>>>> device on the south bridge.
>>>>
>>>> Using the alsa-driver, when I increase the volume then at some point 
>>>> all over sudden the sound stops. On some boards this happens at 75%, 
>>>> on others already at 60%, or 90%. Bringing the volume down again does 
>>>> not bring the audio back ...
>>>>
>>>> I am using the alsa-drivers with MPlayer.
>>>>
>>>> Although to me this smells like a hardware problem, I understand from 
>>>> googling around that this can be fixed with software, but I cannot 
>>>> find the correct solution anywhere. I have tested with gentoo kernels 
>>>> 2.6.22 and 2.6.24.
>>>>
>>>> AFAIK there are only 2 ways to get the sound back once it has stopped:
>>>>
>>>> 1) reset the mobo
>>>> 2) rmmod snd-via82xx, followed by modprobe snd-via82xx
>>>>
>>>> The strange thing is that in option 2), the problem seems to be gone 
>>>> once the driver has been reloaded; from that moment on it is working 
>>>> fine ....!
>>>>
>>>> There is a driver from Via Arena (viaudiocombo) that seems to work 
>>>> fine, but it is OSS i.o. ALSA, and also a development by Via Arena 
>>>> i.o. the Linux community. Besides, I need synchronization features 
>>>> from the audio driver, and I do not think that OSS support this (not 
>>>> 100% sure).
>>>>
>>>>
>>>> Can anyone help me and shed light on this?
>>>>
>>>>
>>>> TIA!
>>>>
>>>> Zoilo.
>>>>
>>>> ------------------------------------------------------------------------- 
>>>>
>>>> This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
>>>> Don't miss this year's exciting event. There's still time to save 
>>>> $100. Use priority code J8TL2D2. 
>>>> http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone 
>>>>
>>>> _______________________________________________
>>>> Alsa-user mailing list
>>>> Alsa-user@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/alsa-user
>>>>
>>>>
>>>>   
>>>>         
>>>
>>>       
>>
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>
>>     
>
>
>   

Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

[Takashi Iwai]

At Tue, 29 Apr 2008 15:25:57 +0200,
Zoilo Gomez wrote:
> 
> >> This is consistent with the apparent introduction of this bug in kernel
> >> 2.6.19 (includes alsa-driver version 1.0.12rc1): no such problem
> >> occurred until 2.6.18, but all kernels since 2.6.19 do suffer from this
> >> problem. The line of code above first shows up in linux-2.6.19.
> >>
> >> Unfortunately, since I do not have a datasheet for the VT1617A chip set,
> >> I cannot verify the exact semantics, or suggest an improvement.
> >>
> >> Can anyone with a datasheet please suggest a proper patch to this line
> >> of code?
> >>     
> >
> > The register 0x5c is the VIA specific one.  I have a VT1617 (without
> > A) datasheet, and it suggests that the bit corresponds to the
> > "headphone amplifier temperature sensing control".  And setting this
> > bit means to _disable_ the temperature sensing control.  This sounds
> > rather the correct to set.
> >
> > However, I don't know whether any difference exists betweeen VIA1617
> > and 1617A although the codec id of both are identical.
> >
> > Andrey, any comments about your patch?
> >
> >
> > In anyway, it'd be helpful if we can know which ac97 registers work
> > and wich not.  Please take /proc/asound/card0/codec97#*/ac97#*-regs
> > file in both working and non-working cases to compare.  Especially,
> > the registers 0x5a and 0x5c look interesting.
> >   
> 
> Code containing "snd_ac97_write_cache(a97, 0x5c, 0x20)":
> 0x5a = 8300
> 0x5c = 0000
> 
> Code with "snd_ac97_write_cache(a97, 0x5c, 0x20)" commented out:
> 0x5a = 8301
> 0x5c = 0020
> 
> Quite a surprise to me, I would have expected exactly the opposite .....!?

I would, too.  Could you double-check, e.g. by adding a printk?
Also, any difference in other registers?

The reg 0x5a bit 0 is a reserved bits, so no further information about
it on VT1617 datasheet.

> Can you explain this? /proc/asound/card0/codec97#0/ac97#0-0+regs does 
> dump current registers, correct? Or is it a toggling bit?

It reads the current register values.  So, it's the value right now.
And snd_ac97_write_cache() should overwrite, not toggle.  The behavior
is really puzzling.


thanks,

Takashi

Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

[Zoilo Gomez]

Takashi Iwai wrote:
> At Tue, 29 Apr 2008 15:25:57 +0200,
> Zoilo Gomez wrote:
>   
>>>> This is consistent with the apparent introduction of this bug in kernel
>>>> 2.6.19 (includes alsa-driver version 1.0.12rc1): no such problem
>>>> occurred until 2.6.18, but all kernels since 2.6.19 do suffer from this
>>>> problem. The line of code above first shows up in linux-2.6.19.
>>>>
>>>> Unfortunately, since I do not have a datasheet for the VT1617A chip set,
>>>> I cannot verify the exact semantics, or suggest an improvement.
>>>>
>>>> Can anyone with a datasheet please suggest a proper patch to this line
>>>> of code?
>>>>     
>>>>         
>>> The register 0x5c is the VIA specific one.  I have a VT1617 (without
>>> A) datasheet, and it suggests that the bit corresponds to the
>>> "headphone amplifier temperature sensing control".  And setting this
>>> bit means to _disable_ the temperature sensing control.  This sounds
>>> rather the correct to set.
>>>
>>> However, I don't know whether any difference exists betweeen VIA1617
>>> and 1617A although the codec id of both are identical.
>>>
>>> Andrey, any comments about your patch?
>>>
>>>
>>> In anyway, it'd be helpful if we can know which ac97 registers work
>>> and wich not.  Please take /proc/asound/card0/codec97#*/ac97#*-regs
>>> file in both working and non-working cases to compare.  Especially,
>>> the registers 0x5a and 0x5c look interesting.
>>>   
>>>       
>> Code containing "snd_ac97_write_cache(a97, 0x5c, 0x20)":
>> 0x5a = 8300
>> 0x5c = 0000
>>
>> Code with "snd_ac97_write_cache(a97, 0x5c, 0x20)" commented out:
>> 0x5a = 8301
>> 0x5c = 0020
>>
>> Quite a surprise to me, I would have expected exactly the opposite .....!?
>>     
>
> I would, too.  Could you double-check, e.g. by adding a printk?
>   

I modified the driver code to:

> int patch_vt1617a(struct snd_ac97 * ac97)
> {
>     int err = 0;
>
>     /* we choose to not fail out at this point, but we tell the
>        caller when we return */
>
>     err = patch_build_controls(ac97, &snd_ac97_controls_vt1617a[0],
>                    ARRAY_SIZE(snd_ac97_controls_vt1617a));
>
>     /* bring analog power consumption to normal by turning off the
>      * headphone amplifier, like WinXP driver for EPIA SP
>      */
> printk("before: snd_ac97_read(ac97, 0x5c) = %04x\n", 
> snd_ac97_read(ac97, 0x5c));
>     snd_ac97_write_cache(ac97, 0x5c, 0x20);
> printk("after: snd_ac97_read(ac97, 0x5c) = %04x\n", 
> snd_ac97_read(ac97, 0x5c));
>     ac97->ext_id |= AC97_EI_SPDIF;    /* force the detection of spdif */
>     ac97->rates[AC97_RATES_SPDIF] = SNDRV_PCM_RATE_44100 | 
> SNDRV_PCM_RATE_48000;
>     ac97->build_ops = &patch_vt1616_ops;
>
>     return err;
> }

Surprise surprise ... the printk output in dmesg reads:

> before: snd_ac97_read(ac97, 0x5c) = 0020
> after: snd_ac97_read(ac97, 0x5c) = 0000

If I write 0x00 (i.o. 0x20) into 0x5c, then the register content remains 
unchanged, and returns: 0x20.
If I write 0x20 into 0x5c twice, then the register content is also 0x00, 
so no toggling.

So it seems that the bit-value is inverted: write "1" clears the bit, 
whereas writing "0" sets the bit.

And the code is trying to set the bit (with good intentions), but the 
result is in fact the opposite.

What do you think?

Thanks and regards,

Z.

Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

[Takashi Iwai]

At Wed, 30 Apr 2008 18:49:52 +0200,
Zoilo Gomez wrote:
> 
> Takashi Iwai wrote:
> > At Tue, 29 Apr 2008 15:25:57 +0200,
> > Zoilo Gomez wrote:
> >   
> >>>> This is consistent with the apparent introduction of this bug in kernel
> >>>> 2.6.19 (includes alsa-driver version 1.0.12rc1): no such problem
> >>>> occurred until 2.6.18, but all kernels since 2.6.19 do suffer from this
> >>>> problem. The line of code above first shows up in linux-2.6.19.
> >>>>
> >>>> Unfortunately, since I do not have a datasheet for the VT1617A chip set,
> >>>> I cannot verify the exact semantics, or suggest an improvement.
> >>>>
> >>>> Can anyone with a datasheet please suggest a proper patch to this line
> >>>> of code?
> >>>>     
> >>>>         
> >>> The register 0x5c is the VIA specific one.  I have a VT1617 (without
> >>> A) datasheet, and it suggests that the bit corresponds to the
> >>> "headphone amplifier temperature sensing control".  And setting this
> >>> bit means to _disable_ the temperature sensing control.  This sounds
> >>> rather the correct to set.
> >>>
> >>> However, I don't know whether any difference exists betweeen VIA1617
> >>> and 1617A although the codec id of both are identical.
> >>>
> >>> Andrey, any comments about your patch?
> >>>
> >>>
> >>> In anyway, it'd be helpful if we can know which ac97 registers work
> >>> and wich not.  Please take /proc/asound/card0/codec97#*/ac97#*-regs
> >>> file in both working and non-working cases to compare.  Especially,
> >>> the registers 0x5a and 0x5c look interesting.
> >>>   
> >>>       
> >> Code containing "snd_ac97_write_cache(a97, 0x5c, 0x20)":
> >> 0x5a = 8300
> >> 0x5c = 0000
> >>
> >> Code with "snd_ac97_write_cache(a97, 0x5c, 0x20)" commented out:
> >> 0x5a = 8301
> >> 0x5c = 0020
> >>
> >> Quite a surprise to me, I would have expected exactly the opposite .....!?
> >>     
> >
> > I would, too.  Could you double-check, e.g. by adding a printk?
> >   
> 
> I modified the driver code to:
> 
> > int patch_vt1617a(struct snd_ac97 * ac97)
> > {
> >     int err = 0;
> >
> >     /* we choose to not fail out at this point, but we tell the
> >        caller when we return */
> >
> >     err = patch_build_controls(ac97, &snd_ac97_controls_vt1617a[0],
> >                    ARRAY_SIZE(snd_ac97_controls_vt1617a));
> >
> >     /* bring analog power consumption to normal by turning off the
> >      * headphone amplifier, like WinXP driver for EPIA SP
> >      */
> > printk("before: snd_ac97_read(ac97, 0x5c) = %04x\n", 
> > snd_ac97_read(ac97, 0x5c));
> >     snd_ac97_write_cache(ac97, 0x5c, 0x20);
> > printk("after: snd_ac97_read(ac97, 0x5c) = %04x\n", 
> > snd_ac97_read(ac97, 0x5c));
> >     ac97->ext_id |= AC97_EI_SPDIF;    /* force the detection of spdif */
> >     ac97->rates[AC97_RATES_SPDIF] = SNDRV_PCM_RATE_44100 | 
> > SNDRV_PCM_RATE_48000;
> >     ac97->build_ops = &patch_vt1616_ops;
> >
> >     return err;
> > }
> 
> Surprise surprise ... the printk output in dmesg reads:
> 
> > before: snd_ac97_read(ac97, 0x5c) = 0020
> > after: snd_ac97_read(ac97, 0x5c) = 0000
> 
> If I write 0x00 (i.o. 0x20) into 0x5c, then the register content remains 
> unchanged, and returns: 0x20.
> If I write 0x20 into 0x5c twice, then the register content is also 0x00, 
> so no toggling.
> 
> So it seems that the bit-value is inverted: write "1" clears the bit, 
> whereas writing "0" sets the bit.

Interesting.

> And the code is trying to set the bit (with good intentions), but the 
> result is in fact the opposite.
> 
> What do you think?

This sounds like a hardware bug to me...
How about the patch below then?


Takashi

---

diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c
index 726320b..e3e4dac 100644
--- a/pci/ac97/ac97_patch.c
+++ b/pci/ac97/ac97_patch.c
@@ -3448,6 +3448,7 @@ static const struct snd_kcontrol_new snd_ac97_controls_vt1617a[] = {
 int patch_vt1617a(struct snd_ac97 * ac97)
 {
 	int err = 0;
+	int val;
 
 	/* we choose to not fail out at this point, but we tell the
 	   caller when we return */
@@ -3458,7 +3459,10 @@ int patch_vt1617a(struct snd_ac97 * ac97)
 	/* bring analog power consumption to normal by turning off the
 	 * headphone amplifier, like WinXP driver for EPIA SP
 	 */
-	snd_ac97_write_cache(ac97, 0x5c, 0x20);
+	val = snd_ac97_read(ac97, 0x5c);
+	if (!(val & 0x20))
+		snd_ac97_write_cache(ac97, 0x5c, 0x20);
+
 	ac97->ext_id |= AC97_EI_SPDIF;	/* force the detection of spdif */
 	ac97->rates[AC97_RATES_SPDIF] = SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000;
 	ac97->build_ops = &patch_vt1616_ops;

Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

[Zoilo Gomez]

Takashi Iwai wrote:
> At Wed, 30 Apr 2008 18:49:52 +0200,
> Zoilo Gomez wrote:
>   
>> Takashi Iwai wrote:
>>     
>>> At Tue, 29 Apr 2008 15:25:57 +0200,
>>> Zoilo Gomez wrote:
>>>   
>>>       
>>>>>> This is consistent with the apparent introduction of this bug in kernel
>>>>>> 2.6.19 (includes alsa-driver version 1.0.12rc1): no such problem
>>>>>> occurred until 2.6.18, but all kernels since 2.6.19 do suffer from this
>>>>>> problem. The line of code above first shows up in linux-2.6.19.
>>>>>>
>>>>>> Unfortunately, since I do not have a datasheet for the VT1617A chip set,
>>>>>> I cannot verify the exact semantics, or suggest an improvement.
>>>>>>
>>>>>> Can anyone with a datasheet please suggest a proper patch to this line
>>>>>> of code?
>>>>>>     
>>>>>>         
>>>>>>             
>>>>> The register 0x5c is the VIA specific one.  I have a VT1617 (without
>>>>> A) datasheet, and it suggests that the bit corresponds to the
>>>>> "headphone amplifier temperature sensing control".  And setting this
>>>>> bit means to _disable_ the temperature sensing control.  This sounds
>>>>> rather the correct to set.
>>>>>
>>>>> However, I don't know whether any difference exists betweeen VIA1617
>>>>> and 1617A although the codec id of both are identical.
>>>>>
>>>>> Andrey, any comments about your patch?
>>>>>
>>>>>
>>>>> In anyway, it'd be helpful if we can know which ac97 registers work
>>>>> and wich not.  Please take /proc/asound/card0/codec97#*/ac97#*-regs
>>>>> file in both working and non-working cases to compare.  Especially,
>>>>> the registers 0x5a and 0x5c look interesting.
>>>>>   
>>>>>       
>>>>>           
>>>> Code containing "snd_ac97_write_cache(a97, 0x5c, 0x20)":
>>>> 0x5a = 8300
>>>> 0x5c = 0000
>>>>
>>>> Code with "snd_ac97_write_cache(a97, 0x5c, 0x20)" commented out:
>>>> 0x5a = 8301
>>>> 0x5c = 0020
>>>>
>>>> Quite a surprise to me, I would have expected exactly the opposite .....!?
>>>>     
>>>>         
>>> I would, too.  Could you double-check, e.g. by adding a printk?
>>>   
>>>       
>> I modified the driver code to:
>>
>>     
>>> int patch_vt1617a(struct snd_ac97 * ac97)
>>> {
>>>     int err = 0;
>>>
>>>     /* we choose to not fail out at this point, but we tell the
>>>        caller when we return */
>>>
>>>     err = patch_build_controls(ac97, &snd_ac97_controls_vt1617a[0],
>>>                    ARRAY_SIZE(snd_ac97_controls_vt1617a));
>>>
>>>     /* bring analog power consumption to normal by turning off the
>>>      * headphone amplifier, like WinXP driver for EPIA SP
>>>      */
>>> printk("before: snd_ac97_read(ac97, 0x5c) = %04x\n", 
>>> snd_ac97_read(ac97, 0x5c));
>>>     snd_ac97_write_cache(ac97, 0x5c, 0x20);
>>> printk("after: snd_ac97_read(ac97, 0x5c) = %04x\n", 
>>> snd_ac97_read(ac97, 0x5c));
>>>     ac97->ext_id |= AC97_EI_SPDIF;    /* force the detection of spdif */
>>>     ac97->rates[AC97_RATES_SPDIF] = SNDRV_PCM_RATE_44100 | 
>>> SNDRV_PCM_RATE_48000;
>>>     ac97->build_ops = &patch_vt1616_ops;
>>>
>>>     return err;
>>> }
>>>       
>> Surprise surprise ... the printk output in dmesg reads:
>>
>>     
>>> before: snd_ac97_read(ac97, 0x5c) = 0020
>>> after: snd_ac97_read(ac97, 0x5c) = 0000
>>>       
>> If I write 0x00 (i.o. 0x20) into 0x5c, then the register content remains 
>> unchanged, and returns: 0x20.
>> If I write 0x20 into 0x5c twice, then the register content is also 0x00, 
>> so no toggling.
>>
>> So it seems that the bit-value is inverted: write "1" clears the bit, 
>> whereas writing "0" sets the bit.
>>     
>
> Interesting.
>
>   
>> And the code is trying to set the bit (with good intentions), but the 
>> result is in fact the opposite.
>>
>> What do you think?
>>     
>
> This sounds like a hardware bug to me...
>   

Agreed.

> How about the patch below then?
>
>
> Takashi
>
> ---
>
> diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c
> index 726320b..e3e4dac 100644
> --- a/pci/ac97/ac97_patch.c
> +++ b/pci/ac97/ac97_patch.c
> @@ -3448,6 +3448,7 @@ static const struct snd_kcontrol_new snd_ac97_controls_vt1617a[] = {
>  int patch_vt1617a(struct snd_ac97 * ac97)
>  {
>  	int err = 0;
> +	int val;
>  
>  	/* we choose to not fail out at this point, but we tell the
>  	   caller when we return */
> @@ -3458,7 +3459,10 @@ int patch_vt1617a(struct snd_ac97 * ac97)
>  	/* bring analog power consumption to normal by turning off the
>  	 * headphone amplifier, like WinXP driver for EPIA SP
>  	 */
> -	snd_ac97_write_cache(ac97, 0x5c, 0x20);
> +	val = snd_ac97_read(ac97, 0x5c);
> +	if (!(val & 0x20))
> +		snd_ac97_write_cache(ac97, 0x5c, 0x20);
>   

This has no effect, since set/clear is apparently reversed here; I think 
you mean:

> +	if (!(val & 0x20))
> +		snd_ac97_write_cache(ac97, 0x5c, 0x00);
>   

But why so complicated? Why not omit the test, and write unconditionally:

> +	snd_ac97_write_cache(ac97, 0x5c, 0x00);

Or perhaps even leave bit 5 in the reset state, which is 0x20 hence OK. 
In other words: drop the entire line?

Z.

Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

[Takashi Iwai]

At Fri, 02 May 2008 17:15:13 +0200,
Zoilo Gomez wrote:
> 
> Takashi Iwai wrote:
> > At Wed, 30 Apr 2008 18:49:52 +0200,
> > Zoilo Gomez wrote:
> >   
> >> Takashi Iwai wrote:
> >>     
> >>> At Tue, 29 Apr 2008 15:25:57 +0200,
> >>> Zoilo Gomez wrote:
> >>>   
> >>>       
> >>>>>> This is consistent with the apparent introduction of this bug in kernel
> >>>>>> 2.6.19 (includes alsa-driver version 1.0.12rc1): no such problem
> >>>>>> occurred until 2.6.18, but all kernels since 2.6.19 do suffer from this
> >>>>>> problem. The line of code above first shows up in linux-2.6.19.
> >>>>>>
> >>>>>> Unfortunately, since I do not have a datasheet for the VT1617A chip set,
> >>>>>> I cannot verify the exact semantics, or suggest an improvement.
> >>>>>>
> >>>>>> Can anyone with a datasheet please suggest a proper patch to this line
> >>>>>> of code?
> >>>>>>     
> >>>>>>         
> >>>>>>             
> >>>>> The register 0x5c is the VIA specific one.  I have a VT1617 (without
> >>>>> A) datasheet, and it suggests that the bit corresponds to the
> >>>>> "headphone amplifier temperature sensing control".  And setting this
> >>>>> bit means to _disable_ the temperature sensing control.  This sounds
> >>>>> rather the correct to set.
> >>>>>
> >>>>> However, I don't know whether any difference exists betweeen VIA1617
> >>>>> and 1617A although the codec id of both are identical.
> >>>>>
> >>>>> Andrey, any comments about your patch?
> >>>>>
> >>>>>
> >>>>> In anyway, it'd be helpful if we can know which ac97 registers work
> >>>>> and wich not.  Please take /proc/asound/card0/codec97#*/ac97#*-regs
> >>>>> file in both working and non-working cases to compare.  Especially,
> >>>>> the registers 0x5a and 0x5c look interesting.
> >>>>>   
> >>>>>       
> >>>>>           
> >>>> Code containing "snd_ac97_write_cache(a97, 0x5c, 0x20)":
> >>>> 0x5a = 8300
> >>>> 0x5c = 0000
> >>>>
> >>>> Code with "snd_ac97_write_cache(a97, 0x5c, 0x20)" commented out:
> >>>> 0x5a = 8301
> >>>> 0x5c = 0020
> >>>>
> >>>> Quite a surprise to me, I would have expected exactly the opposite .....!?
> >>>>     
> >>>>         
> >>> I would, too.  Could you double-check, e.g. by adding a printk?
> >>>   
> >>>       
> >> I modified the driver code to:
> >>
> >>     
> >>> int patch_vt1617a(struct snd_ac97 * ac97)
> >>> {
> >>>     int err = 0;
> >>>
> >>>     /* we choose to not fail out at this point, but we tell the
> >>>        caller when we return */
> >>>
> >>>     err = patch_build_controls(ac97, &snd_ac97_controls_vt1617a[0],
> >>>                    ARRAY_SIZE(snd_ac97_controls_vt1617a));
> >>>
> >>>     /* bring analog power consumption to normal by turning off the
> >>>      * headphone amplifier, like WinXP driver for EPIA SP
> >>>      */
> >>> printk("before: snd_ac97_read(ac97, 0x5c) = %04x\n", 
> >>> snd_ac97_read(ac97, 0x5c));
> >>>     snd_ac97_write_cache(ac97, 0x5c, 0x20);
> >>> printk("after: snd_ac97_read(ac97, 0x5c) = %04x\n", 
> >>> snd_ac97_read(ac97, 0x5c));
> >>>     ac97->ext_id |= AC97_EI_SPDIF;    /* force the detection of spdif */
> >>>     ac97->rates[AC97_RATES_SPDIF] = SNDRV_PCM_RATE_44100 | 
> >>> SNDRV_PCM_RATE_48000;
> >>>     ac97->build_ops = &patch_vt1616_ops;
> >>>
> >>>     return err;
> >>> }
> >>>       
> >> Surprise surprise ... the printk output in dmesg reads:
> >>
> >>     
> >>> before: snd_ac97_read(ac97, 0x5c) = 0020
> >>> after: snd_ac97_read(ac97, 0x5c) = 0000
> >>>       
> >> If I write 0x00 (i.o. 0x20) into 0x5c, then the register content remains 
> >> unchanged, and returns: 0x20.
> >> If I write 0x20 into 0x5c twice, then the register content is also 0x00, 
> >> so no toggling.
> >>
> >> So it seems that the bit-value is inverted: write "1" clears the bit, 
> >> whereas writing "0" sets the bit.
> >>     
> >
> > Interesting.
> >
> >   
> >> And the code is trying to set the bit (with good intentions), but the 
> >> result is in fact the opposite.
> >>
> >> What do you think?
> >>     
> >
> > This sounds like a hardware bug to me...
> >   
> 
> Agreed.
> 
> > How about the patch below then?
> >
> >
> > Takashi
> >
> > ---
> >
> > diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c
> > index 726320b..e3e4dac 100644
> > --- a/pci/ac97/ac97_patch.c
> > +++ b/pci/ac97/ac97_patch.c
> > @@ -3448,6 +3448,7 @@ static const struct snd_kcontrol_new snd_ac97_controls_vt1617a[] = {
> >  int patch_vt1617a(struct snd_ac97 * ac97)
> >  {
> >  	int err = 0;
> > +	int val;
> >  
> >  	/* we choose to not fail out at this point, but we tell the
> >  	   caller when we return */
> > @@ -3458,7 +3459,10 @@ int patch_vt1617a(struct snd_ac97 * ac97)
> >  	/* bring analog power consumption to normal by turning off the
> >  	 * headphone amplifier, like WinXP driver for EPIA SP
> >  	 */
> > -	snd_ac97_write_cache(ac97, 0x5c, 0x20);
> > +	val = snd_ac97_read(ac97, 0x5c);
> > +	if (!(val & 0x20))
> > +		snd_ac97_write_cache(ac97, 0x5c, 0x20);
> >   
> 
> This has no effect, since set/clear is apparently reversed here;

So, the bit 0x20 isn't set as default when you didn't write to 0x5c?

> I think you mean:
> 
> > +	if (!(val & 0x20))
> > +		snd_ac97_write_cache(ac97, 0x5c, 0x00);
> >   

No I meant as is.  See below.

> But why so complicated? Why not omit the test, and write unconditionally:
> > +	snd_ac97_write_cache(ac97, 0x5c, 0x00);
> 
The patch was there for any good reason.  It was introduced to fix
some bugs on certain machines.  Removing it blindly breaks it again.


> Or perhaps even leave bit 5 in the reset state, which is 0x20 hence OK. 

Hm, they why the first test didn't pass?

> In other words: drop the entire line?

No.  The situation is really complicated:

- the current code is confirmed to work on some machines as is
- we don't know exactly whether the bit flip oocurs on every hardware
  with VT1617A (maybe not)
- we don't know exactly whether the bit is set or cleared as default


Takashi

Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

[Zoilo Gomez]

Takashi Iwai wrote:
> At Fri, 02 May 2008 17:15:13 +0200,
> Zoilo Gomez wrote:
>   
>> Takashi Iwai wrote:
>>     
>>> At Wed, 30 Apr 2008 18:49:52 +0200,
>>> Zoilo Gomez wrote:
>>>   
>>>       
>>>> Takashi Iwai wrote:
>>>>     
>>>>         
>>>>> At Tue, 29 Apr 2008 15:25:57 +0200,
>>>>> Zoilo Gomez wrote:
>>>>>   
>>>>>       
>>>>>           
>>>>>>>> This is consistent with the apparent introduction of this bug in kernel
>>>>>>>> 2.6.19 (includes alsa-driver version 1.0.12rc1): no such problem
>>>>>>>> occurred until 2.6.18, but all kernels since 2.6.19 do suffer from this
>>>>>>>> problem. The line of code above first shows up in linux-2.6.19.
>>>>>>>>
>>>>>>>> Unfortunately, since I do not have a datasheet for the VT1617A chip set,
>>>>>>>> I cannot verify the exact semantics, or suggest an improvement.
>>>>>>>>
>>>>>>>> Can anyone with a datasheet please suggest a proper patch to this line
>>>>>>>> of code?
>>>>>>>>     
>>>>>>>>         
>>>>>>>>             
>>>>>>>>                 
>>>>>>> The register 0x5c is the VIA specific one.  I have a VT1617 (without
>>>>>>> A) datasheet, and it suggests that the bit corresponds to the
>>>>>>> "headphone amplifier temperature sensing control".  And setting this
>>>>>>> bit means to _disable_ the temperature sensing control.  This sounds
>>>>>>> rather the correct to set.
>>>>>>>
>>>>>>> However, I don't know whether any difference exists betweeen VIA1617
>>>>>>> and 1617A although the codec id of both are identical.
>>>>>>>
>>>>>>> Andrey, any comments about your patch?
>>>>>>>
>>>>>>>
>>>>>>> In anyway, it'd be helpful if we can know which ac97 registers work
>>>>>>> and wich not.  Please take /proc/asound/card0/codec97#*/ac97#*-regs
>>>>>>> file in both working and non-working cases to compare.  Especially,
>>>>>>> the registers 0x5a and 0x5c look interesting.
>>>>>>>   
>>>>>>>       
>>>>>>>           
>>>>>>>               
>>>>>> Code containing "snd_ac97_write_cache(a97, 0x5c, 0x20)":
>>>>>> 0x5a = 8300
>>>>>> 0x5c = 0000
>>>>>>
>>>>>> Code with "snd_ac97_write_cache(a97, 0x5c, 0x20)" commented out:
>>>>>> 0x5a = 8301
>>>>>> 0x5c = 0020
>>>>>>
>>>>>> Quite a surprise to me, I would have expected exactly the opposite .....!?
>>>>>>     
>>>>>>         
>>>>>>             
>>>>> I would, too.  Could you double-check, e.g. by adding a printk?
>>>>>   
>>>>>       
>>>>>           
>>>> I modified the driver code to:
>>>>
>>>>     
>>>>         
>>>>> int patch_vt1617a(struct snd_ac97 * ac97)
>>>>> {
>>>>>     int err = 0;
>>>>>
>>>>>     /* we choose to not fail out at this point, but we tell the
>>>>>        caller when we return */
>>>>>
>>>>>     err = patch_build_controls(ac97, &snd_ac97_controls_vt1617a[0],
>>>>>                    ARRAY_SIZE(snd_ac97_controls_vt1617a));
>>>>>
>>>>>     /* bring analog power consumption to normal by turning off the
>>>>>      * headphone amplifier, like WinXP driver for EPIA SP
>>>>>      */
>>>>> printk("before: snd_ac97_read(ac97, 0x5c) = %04x\n", 
>>>>> snd_ac97_read(ac97, 0x5c));
>>>>>     snd_ac97_write_cache(ac97, 0x5c, 0x20);
>>>>> printk("after: snd_ac97_read(ac97, 0x5c) = %04x\n", 
>>>>> snd_ac97_read(ac97, 0x5c));
>>>>>     ac97->ext_id |= AC97_EI_SPDIF;    /* force the detection of spdif */
>>>>>     ac97->rates[AC97_RATES_SPDIF] = SNDRV_PCM_RATE_44100 | 
>>>>> SNDRV_PCM_RATE_48000;
>>>>>     ac97->build_ops = &patch_vt1616_ops;
>>>>>
>>>>>     return err;
>>>>> }
>>>>>       
>>>>>           
>>>> Surprise surprise ... the printk output in dmesg reads:
>>>>
>>>>     
>>>>         
>>>>> before: snd_ac97_read(ac97, 0x5c) = 0020
>>>>> after: snd_ac97_read(ac97, 0x5c) = 0000
>>>>>       
>>>>>           
>>>> If I write 0x00 (i.o. 0x20) into 0x5c, then the register content remains 
>>>> unchanged, and returns: 0x20.
>>>> If I write 0x20 into 0x5c twice, then the register content is also 0x00, 
>>>> so no toggling.
>>>>
>>>> So it seems that the bit-value is inverted: write "1" clears the bit, 
>>>> whereas writing "0" sets the bit.
>>>>     
>>>>         
>>> Interesting.
>>>
>>>   
>>>       
>>>> And the code is trying to set the bit (with good intentions), but the 
>>>> result is in fact the opposite.
>>>>
>>>> What do you think?
>>>>     
>>>>         
>>> This sounds like a hardware bug to me...
>>>   
>>>       
>> Agreed.
>>
>>     
>>> How about the patch below then?
>>>
>>>
>>> Takashi
>>>
>>> ---
>>>
>>> diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c
>>> index 726320b..e3e4dac 100644
>>> --- a/pci/ac97/ac97_patch.c
>>> +++ b/pci/ac97/ac97_patch.c
>>> @@ -3448,6 +3448,7 @@ static const struct snd_kcontrol_new snd_ac97_controls_vt1617a[] = {
>>>  int patch_vt1617a(struct snd_ac97 * ac97)
>>>  {
>>>  	int err = 0;
>>> +	int val;
>>>  
>>>  	/* we choose to not fail out at this point, but we tell the
>>>  	   caller when we return */
>>> @@ -3458,7 +3459,10 @@ int patch_vt1617a(struct snd_ac97 * ac97)
>>>  	/* bring analog power consumption to normal by turning off the
>>>  	 * headphone amplifier, like WinXP driver for EPIA SP
>>>  	 */
>>> -	snd_ac97_write_cache(ac97, 0x5c, 0x20);
>>> +	val = snd_ac97_read(ac97, 0x5c);
>>> +	if (!(val & 0x20))
>>> +		snd_ac97_write_cache(ac97, 0x5c, 0x20);
>>>   
>>>       
>> This has no effect, since set/clear is apparently reversed here;
>>     
>
> So, the bit 0x20 isn't set as default when you didn't write to 0x5c?
>   

Yes it is; if I do not write to 0x5c after reset, then the register 
value reads back as 0x20.

>> I think you mean:
>>     
>>> +	if (!(val & 0x20))
>>> +		snd_ac97_write_cache(ac97, 0x5c, 0x00);
>>>   
>>>       
>
> No I meant as is.  See below.
>
>   
>> But why so complicated? Why not omit the test, and write unconditionally:
>>     
>>> +	snd_ac97_write_cache(ac97, 0x5c, 0x00);
>>>       
> The patch was there for any good reason.  It was introduced to fix
> some bugs on certain machines.  Removing it blindly breaks it again.
>
>
>   
>> Or perhaps even leave bit 5 in the reset state, which is 0x20 hence OK. 
>>     
>
> Hm, they why the first test didn't pass?
>   

OK, now I get your point: you are assuming that the chip set on my 
machine can be recognized because it has bit 0x20 set after reset, and 
in that case we leave the value as is. However if bit 0x20 is not set, 
then we assume that it is a different chip set with "proper 
1/0-set/clear-behaviour", and we set the bit to 0x20. Correct?

Sorry for the confusion on my side.

> No.  The situation is really complicated:
>
> - the current code is confirmed to work on some machines as is
> - we don't know exactly whether the bit flip oocurs on every hardware
>   with VT1617A (maybe not)
> - we don't know exactly whether the bit is set or cleared as default
>   

Understood.

OK, I confirm that your patch works fine on my machine, which (for the 
record) is a Jetway J7F2 Mini-ITX motherboard.

I have also cross-checked (well ... "cross-grepped") with the 
viaudiocombo OSS-driver from the Arena web site; it seems that the 
contents of register 0x5c are left unaltered after reset (hence the 
value is 0x20). So that explains why the Arena driver does not suffer 
from this problem.

Thanks a lot for your help Takashi!

Zoilo.

Re: [Alsa-user] Jetway j7f2 via82xx volume problem: sound suddenly stops when increasing volume: "SOLVED" (well ... sort of)

[Takashi Iwai]

At Fri, 02 May 2008 19:12:57 +0200,
Zoilo Gomez wrote:
> 
> Takashi Iwai wrote:
> > At Fri, 02 May 2008 17:15:13 +0200,
> > Zoilo Gomez wrote:
> >   
> >> Takashi Iwai wrote:
> >>     
> >>> At Wed, 30 Apr 2008 18:49:52 +0200,
> >>> Zoilo Gomez wrote:
> >>>   
> >>>       
> >>>> Takashi Iwai wrote:
> >>>>     
> >>>>         
> >>>>> At Tue, 29 Apr 2008 15:25:57 +0200,
> >>>>> Zoilo Gomez wrote:
> >>>>>   
> >>>>>       
> >>>>>           
> >>>>>>>> This is consistent with the apparent introduction of this bug in kernel
> >>>>>>>> 2.6.19 (includes alsa-driver version 1.0.12rc1): no such problem
> >>>>>>>> occurred until 2.6.18, but all kernels since 2.6.19 do suffer from this
> >>>>>>>> problem. The line of code above first shows up in linux-2.6.19.
> >>>>>>>>
> >>>>>>>> Unfortunately, since I do not have a datasheet for the VT1617A chip set,
> >>>>>>>> I cannot verify the exact semantics, or suggest an improvement.
> >>>>>>>>
> >>>>>>>> Can anyone with a datasheet please suggest a proper patch to this line
> >>>>>>>> of code?
> >>>>>>>>     
> >>>>>>>>         
> >>>>>>>>             
> >>>>>>>>                 
> >>>>>>> The register 0x5c is the VIA specific one.  I have a VT1617 (without
> >>>>>>> A) datasheet, and it suggests that the bit corresponds to the
> >>>>>>> "headphone amplifier temperature sensing control".  And setting this
> >>>>>>> bit means to _disable_ the temperature sensing control.  This sounds
> >>>>>>> rather the correct to set.
> >>>>>>>
> >>>>>>> However, I don't know whether any difference exists betweeen VIA1617
> >>>>>>> and 1617A although the codec id of both are identical.
> >>>>>>>
> >>>>>>> Andrey, any comments about your patch?
> >>>>>>>
> >>>>>>>
> >>>>>>> In anyway, it'd be helpful if we can know which ac97 registers work
> >>>>>>> and wich not.  Please take /proc/asound/card0/codec97#*/ac97#*-regs
> >>>>>>> file in both working and non-working cases to compare.  Especially,
> >>>>>>> the registers 0x5a and 0x5c look interesting.
> >>>>>>>   
> >>>>>>>       
> >>>>>>>           
> >>>>>>>               
> >>>>>> Code containing "snd_ac97_write_cache(a97, 0x5c, 0x20)":
> >>>>>> 0x5a = 8300
> >>>>>> 0x5c = 0000
> >>>>>>
> >>>>>> Code with "snd_ac97_write_cache(a97, 0x5c, 0x20)" commented out:
> >>>>>> 0x5a = 8301
> >>>>>> 0x5c = 0020
> >>>>>>
> >>>>>> Quite a surprise to me, I would have expected exactly the opposite .....!?
> >>>>>>     
> >>>>>>         
> >>>>>>             
> >>>>> I would, too.  Could you double-check, e.g. by adding a printk?
> >>>>>   
> >>>>>       
> >>>>>           
> >>>> I modified the driver code to:
> >>>>
> >>>>     
> >>>>         
> >>>>> int patch_vt1617a(struct snd_ac97 * ac97)
> >>>>> {
> >>>>>     int err = 0;
> >>>>>
> >>>>>     /* we choose to not fail out at this point, but we tell the
> >>>>>        caller when we return */
> >>>>>
> >>>>>     err = patch_build_controls(ac97, &snd_ac97_controls_vt1617a[0],
> >>>>>                    ARRAY_SIZE(snd_ac97_controls_vt1617a));
> >>>>>
> >>>>>     /* bring analog power consumption to normal by turning off the
> >>>>>      * headphone amplifier, like WinXP driver for EPIA SP
> >>>>>      */
> >>>>> printk("before: snd_ac97_read(ac97, 0x5c) = %04x\n", 
> >>>>> snd_ac97_read(ac97, 0x5c));
> >>>>>     snd_ac97_write_cache(ac97, 0x5c, 0x20);
> >>>>> printk("after: snd_ac97_read(ac97, 0x5c) = %04x\n", 
> >>>>> snd_ac97_read(ac97, 0x5c));
> >>>>>     ac97->ext_id |= AC97_EI_SPDIF;    /* force the detection of spdif */
> >>>>>     ac97->rates[AC97_RATES_SPDIF] = SNDRV_PCM_RATE_44100 | 
> >>>>> SNDRV_PCM_RATE_48000;
> >>>>>     ac97->build_ops = &patch_vt1616_ops;
> >>>>>
> >>>>>     return err;
> >>>>> }
> >>>>>       
> >>>>>           
> >>>> Surprise surprise ... the printk output in dmesg reads:
> >>>>
> >>>>     
> >>>>         
> >>>>> before: snd_ac97_read(ac97, 0x5c) = 0020
> >>>>> after: snd_ac97_read(ac97, 0x5c) = 0000
> >>>>>       
> >>>>>           
> >>>> If I write 0x00 (i.o. 0x20) into 0x5c, then the register content remains 
> >>>> unchanged, and returns: 0x20.
> >>>> If I write 0x20 into 0x5c twice, then the register content is also 0x00, 
> >>>> so no toggling.
> >>>>
> >>>> So it seems that the bit-value is inverted: write "1" clears the bit, 
> >>>> whereas writing "0" sets the bit.
> >>>>     
> >>>>         
> >>> Interesting.
> >>>
> >>>   
> >>>       
> >>>> And the code is trying to set the bit (with good intentions), but the 
> >>>> result is in fact the opposite.
> >>>>
> >>>> What do you think?
> >>>>     
> >>>>         
> >>> This sounds like a hardware bug to me...
> >>>   
> >>>       
> >> Agreed.
> >>
> >>     
> >>> How about the patch below then?
> >>>
> >>>
> >>> Takashi
> >>>
> >>> ---
> >>>
> >>> diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c
> >>> index 726320b..e3e4dac 100644
> >>> --- a/pci/ac97/ac97_patch.c
> >>> +++ b/pci/ac97/ac97_patch.c
> >>> @@ -3448,6 +3448,7 @@ static const struct snd_kcontrol_new snd_ac97_controls_vt1617a[] = {
> >>>  int patch_vt1617a(struct snd_ac97 * ac97)
> >>>  {
> >>>  	int err = 0;
> >>> +	int val;
> >>>  
> >>>  	/* we choose to not fail out at this point, but we tell the
> >>>  	   caller when we return */
> >>> @@ -3458,7 +3459,10 @@ int patch_vt1617a(struct snd_ac97 * ac97)
> >>>  	/* bring analog power consumption to normal by turning off the
> >>>  	 * headphone amplifier, like WinXP driver for EPIA SP
> >>>  	 */
> >>> -	snd_ac97_write_cache(ac97, 0x5c, 0x20);
> >>> +	val = snd_ac97_read(ac97, 0x5c);
> >>> +	if (!(val & 0x20))
> >>> +		snd_ac97_write_cache(ac97, 0x5c, 0x20);
> >>>   
> >>>       
> >> This has no effect, since set/clear is apparently reversed here;
> >>     
> >
> > So, the bit 0x20 isn't set as default when you didn't write to 0x5c?
> >   
> 
> Yes it is; if I do not write to 0x5c after reset, then the register 
> value reads back as 0x20.
> 
> >> I think you mean:
> >>     
> >>> +	if (!(val & 0x20))
> >>> +		snd_ac97_write_cache(ac97, 0x5c, 0x00);
> >>>   
> >>>       
> >
> > No I meant as is.  See below.
> >
> >   
> >> But why so complicated? Why not omit the test, and write unconditionally:
> >>     
> >>> +	snd_ac97_write_cache(ac97, 0x5c, 0x00);
> >>>       
> > The patch was there for any good reason.  It was introduced to fix
> > some bugs on certain machines.  Removing it blindly breaks it again.
> >
> >
> >   
> >> Or perhaps even leave bit 5 in the reset state, which is 0x20 hence OK. 
> >>     
> >
> > Hm, they why the first test didn't pass?
> >   
> 
> OK, now I get your point: you are assuming that the chip set on my 
> machine can be recognized because it has bit 0x20 set after reset, and 
> in that case we leave the value as is. However if bit 0x20 is not set, 
> then we assume that it is a different chip set with "proper 
> 1/0-set/clear-behaviour", and we set the bit to 0x20. Correct?

Yes, exactly.

> Sorry for the confusion on my side.
> 
> > No.  The situation is really complicated:
> >
> > - the current code is confirmed to work on some machines as is
> > - we don't know exactly whether the bit flip oocurs on every hardware
> >   with VT1617A (maybe not)
> > - we don't know exactly whether the bit is set or cleared as default
> >   
> 
> Understood.
> 
> OK, I confirm that your patch works fine on my machine, which (for the 
> record) is a Jetway J7F2 Mini-ITX motherboard.

Thanks for checking.  I applied the patch (with a bit more comment) to
ALSA tree now.  It'll be on 2.6.26 tree, too.

> I have also cross-checked (well ... "cross-grepped") with the 
> viaudiocombo OSS-driver from the Arena web site; it seems that the 
> contents of register 0x5c are left unaltered after reset (hence the 
> value is 0x20). So that explains why the Arena driver does not suffer 
> from this problem.

The workaround was relatively new and I guess it's specific to some
EPIA board and possibly not included in other drivers...


thanks,

Takashi