[alsa-devel] Codec VIA VT2020/VT1718S and questions about commit 4ab2d53a99b6dcee86837d2a9739bfb9f468db45

[alex dot baldacchino dot alsasub at gmail dot com]

Hello,

I've been looking into the code of alsa drivers (mainly hda and
patch_via.c), with help by Raymond Yau (many thanks to him) after a
problem making my system work with a sound driver newer than version
1.0.21 (at last, I fixed it by recompiling alsa-lib to use device
files in /dev as created by udev with correct minor numbers - original
subject of that discussion on alsa-devel mailing list is "Via VT2020:
issues with kernel 2.6.38.{2, 3} (alsa 1.0.23) - working with 2.6.33.2
(alsa 1.0.21)").

My system is a no-brand desktop, with an asrock motherbord (model
890GX Extreme3) embedding an audio codec VIA VT2020 (beside ATI HDMI).
That should support 7.1 speaker systems, plus eventually a headset
connected to a front audio panel in multistream mode (since VT1718S
seems to support as much as 10 playback channles, as per channels_max
field in static const struct hda_pcm_stream
vt1718S_pcm_analog_playback, file patch_via.c). Though, my motherboard
holds no grey (side) jack (would be nid 0x27), and it would seem that
blue (line-in) jack (0x2a) should be retasked as side output (it would
use AOW 0x0c, actually used for Independent HP mode, but I've tested,
with help by Raymond Yau, and can confirm 0x0b can be used instead by
front panel line-out - it's in the connection list for nid 0x34,
labeled "Independent HP", which connects to 0x1b and thus to 0x28,
"Headphone Playback Switch"). We were trying to fix it (most of the
code, about all the code about that, coming from him, I'm just
testing/reviewing it and looking into some other, perhaps more general
and maybe minor, issues). But now we'd have a concern about a piece of
code within struct hda_verb vt1718S_volume_init_verbs[], as modified
by you in commit 4ab2d53a99b6dcee86837d2a9739bfb9f468db45,
specifically the following part:


@@ -4264,7 +4265,7 @@ static struct hda_verb vt1718S_volume_init_verbs[] = {
        {0x21, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_MUTE(1)},
        {0x21, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_MUTE(2)},
        {0x21, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_MUTE(3)},
-       {0x21, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_MUTE(5)},
+       {0x21, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_UNMUTE(5)},

        /* Setup default input of Front HP to MW9 */
        {0x28, AC_VERB_SET_CONNECT_SEL, 0x1},


My system (codec vt2020) has only 5 connections to nid 0x21:


Node 0x21 [Audio Mixer] wcaps 0x20050b: Stereo Amp-In
  Control: name="Front Mic Playback Volume", index=0, device=0
    ControlAmp: chs=3, dir=In, idx=3, ofs=0
  Control: name="Front Mic Playback Switch", index=0, device=0
    ControlAmp: chs=3, dir=In, idx=3, ofs=0
  Control: name="Rear Mic Playback Volume", index=0, device=0
    ControlAmp: chs=3, dir=In, idx=1, ofs=0
  Control: name="Rear Mic Playback Switch", index=0, device=0
    ControlAmp: chs=3, dir=In, idx=1, ofs=0
  Control: name="Line Playback Volume", index=0, device=0
    ControlAmp: chs=3, dir=In, idx=2, ofs=0
  Control: name="Line Playback Switch", index=0, device=0
    ControlAmp: chs=3, dir=In, idx=2, ofs=0
  Amp-In caps: ofs=0x17, nsteps=0x1f, stepsize=0x05, mute=1
  Amp-In vals:  [0x80 0x80] [0x1f 0x1f] [0x1f 0x1f] [0x1f 0x1f] [0x97 0x97]
  Power states:  D0 D1 D2 D3
  Power: setting=D0, actual=D0
  Connection: 5
     0x2c 0x2b 0x2a 0x29 0x28


and that seems to be the same for other vt1718s codec implementations
(by asrock): https://launchpadlibrarian.net/70606587/Card0.Codecs.codec.0.txt
 (motherboard ASRock H55M Pro, more info at
https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/772638 ),
therefore your code seems to access a non-existing connection. Since
such looks like not very safe/correct, Raymond Yau had suggested to
change it into:


-       {0x21, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_UNMUTE(5)},
+       {0x21, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_MUTE(4)},


which seems to work (or at least to cause no problem) on my system.
However, after finding your commit, we wonder if that could be
necessary for the machine (e.g. because of internal functioning, I'm
_not_ asking you to disclose, of course), so I'm writing you to ask
for some clarification, as far as possible.

Moreover, I have some doubts about the rest of that patch:


@@ -4255,7 +4255,8 @@ static struct hda_verb vt1718S_volume_init_verbs[] = {
        {0x10, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_UNMUTE(0)},
        {0x11, AC_VERB_SET_AMP_GAIN_MUTE, AMP_IN_UNMUTE(0)},

-
+       /* Enable MW0 adjust Gain 5 */
+       {0x1, 0xfb2, 0x10},
        /* Mute input amps (CD, Line In, Mic 1 & Mic 2) of the analog-loopback
         * mixer widget
         */


In my system, MW0 (first audio mixer widget) seems to be nid 0x18 (0x1
being AFG Function Id) and I haven't been able to find infos about
verb 0xfb2; commenting it out seems to cause no problem, however that
may look like an implementation-specific verb to be sent to a special
connection (as a part of VIA internals I'll never ask you to
disclose), so I'll be grateful to you if you can just tell me if
that's specifically needed and must be there.

Furthermore:


        /* Setup default input of Front HP to MW9 */
        {0x28, AC_VERB_SET_CONNECT_SEL, 0x1},


(this part not created by that commit of yours)

My system seems to have only 6 Audio Mixer Widgets (0x18, 0x19, 0x1a,
0x1b, 0x1c and 0x21 - unless any Vendor Defined Widget is taken into
account), as it seems to be for vt1718S codec as described in links
above; in both cases 0x28 has one only connection:

vt2020 (mine):

Node 0x28 [Pin Complex] wcaps 0x40058d: Stereo Amp-Out
  Control: name="Headphone Playback Switch", index=0, device=0
    ControlAmp: chs=3, dir=Out, idx=0, ofs=0
  Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
  Amp-Out vals:  [0x00 0x00]
  Pincap 0x0000233c: IN OUT HP Detect
    Vref caps: HIZ 50 100
  Pin Default 0x0221401f: [Jack] HP Out at Ext Front
    Conn = 1/8, Color = Green
    DefAssociation = 0x1, Sequence = 0xf
  Pin-ctls: 0xc0: OUT HP VREF_HIZ
  Unsolicited: tag=21, enabled=1
  Power states:  D0 D1 D2 D3
  Power: setting=D0, actual=D0
  Connection: 1
     0x1b


vt1718s (linked above):

Node 0x28 [Pin Complex] wcaps 0x40058d: Stereo Amp-Out
  Control: name="Headphone Playback Switch", index=0, device=0
    ControlAmp: chs=3, dir=Out, idx=0, ofs=0
  Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
  Amp-Out vals:  [0x00 0x00]
  Pincap 0x0000233c: IN OUT HP Detect
    Vref caps: HIZ 50 100
  Pin Default 0x0221401f: [Jack] HP Out at Ext Front
    Conn = 1/8, Color = Green
    DefAssociation = 0x1, Sequence = 0xf
  Pin-ctls: 0xc0: OUT HP VREF_HIZ
  Unsolicited: tag=05, enabled=1
  Power states:  D0 D1 D2 D3
  Power: setting=D0, actual=D0
  Connection: 1
     0x1b


Thus, it would seem (to me, at least) more natural (and correct) to
initialize it by selecting connection 0 (the only existing one - or
none at all, since the only one should be selected by the hardware by
default), unless there's some specific reason for that (e.g. other
codec implementations with more than one connection to 0x28, or
internal need for that).

Could there be the need to create different initialization verb arrays
for different flavours of VT1718S?

Many thanks in advance for your answers.
Best regards.