[alsa-devel] Major reworks on patch_via.c (TESTERS WANTED)

[alex dot baldacchino dot alsasub at gmail dot com]

2011/7/4 Takashi Iwai <tiwai at suse.de>:
> At Sat, 2 Jul 2011 18:00:27 +0200,
> alex dot baldacchino dot alsasub at gmail dot com wrote:
>>
>> 2011/6/22 Takashi Iwai <tiwai at suse.de>:
>> > At Mon, 20 Jun 2011 17:06:51 +0200,
>> > Takashi Iwai wrote:
>> >>
>> >> Hi,
>> >>
>> >> as there have many problems reported for VIA codecs, I started looking
>> >> at the driver code, and decided to rework on it.
>> >> [...]
>>
>> I've installed and tested (from tarballs) both snapshot
>> alsa-driver-20110630 and latest version (alsa-driver-snapshot.tar.gz)
>> - up to commit e5e14681404ec27a422d635284bf564dabde3f81 by Lydia Wang,
>> I think.
>>
>> I've found problems with both. That's a quite huge change in the
>> implementation from older code, so I need to study it more in depth to
>> understand it better... At the moment I can only describe the problems
>> and give my first impressions on them.
>>
>> First of all, I can't get sound from the front playback (rear
>> line-out).
> ...
>
> These should be fixed now by Lydia's patch.  I updated sound git
> tree and snapshot tarball now.  Please check it later.
>

I've been far from my pc yesterday for some reasons, and haven't had
got too much time today as well, but I've gathered latest version of
patch_via.c from
http://git.kernel.org/?p=linux/kernel/git/tiwai/sound-2.6.git;a=commit;h=bac4b92cf7a444c0af8dd7b269c8791595c44052
and made a few tests in different conditions, with following results:

Booting with both front and hp connected:
- no sound from rear green jack (front pin);
- louder and noisier recording of ongoing playback (gives me an idea
of stereo mixer quality);
- louder sound on hp regardless of independent mode being set on or
off, but when nothing is connected to front pin (0x24) - of course,
since nothing connected to 0x24 means parm == AC_PWRST_D3 and HP
Independent mode ON means state of hp pin (0x28) to be disregarded,
thus 0x8 is set to D3 by set_widgets_power_state_vt1718S():


	/* PW0 (24h), AOW0 (8h) */
	parm = AC_PWRST_D3;
	set_pin_power_state(codec, 0x24, &parm);
	if (!spec->hp_independent_mode) /* check for redirected HP */
		set_pin_power_state(codec, 0x28, &parm);
	snd_hda_codec_write(codec, 0x8, 0, AC_VERB_SET_POWER_STATE, parm);


Booting with only front pin connected:
- same as above for recording and sound out of hp pin;
- now I get sound from front pin, thus it would seem that some
automuting is triggered at boot-time (and only at boot-time and/or
driver re-load after rmmod'ing - though not tested - since
detaching/attaching connectors after boot-up/after drivers are loaded
doesn't change such behaviour); such didn't happen in older
implementation (before this rework) and, personally, I preferred that
behavior;
- using same stream settings as front when stream channels are less
then line-outs (for exceeding line-outs) works as well.


Independent HP doesn't work, as before, problem (and fix) still being the same:


--- ./alsa-driver/alsa-kernel/pci/hda/patch_via.c.last	2011-07-05
16:25:35.844001455 +0200
+++ ./alsa-driver/alsa-kernel/pci/hda/patch_via.c	2011-07-05
16:25:23.652001521 +0200
@@ -2974,7 +2974,7 @@ static void set_widgets_power_state_vt17
 				    AC_VERB_SET_POWER_STATE, parm);
 		snd_hda_codec_write(codec, 0x34, 0,
 				    AC_VERB_SET_POWER_STATE, parm);
-		snd_hda_codec_write(codec, 0xc, 0,
+		snd_hda_codec_write(codec, spec->hp_dac_nid, 0,
 				    AC_VERB_SET_POWER_STATE, parm);
 	}
 }



>
>> One notably thing is that the new (for codec VT1718S family) Master
>> control misses a few slaves, mainly:
>> ...
>> - more noticeably, there's no "Headphone Playback Volume" control.
>> ...
>
> Hm, I'll check this with your alsa-info.sh output.
>

Now "Headphone Playback Volume" is back, thanks.

>
>> A final (minor) question/consideration on the code. I've noticed now
>> .put helpers validate data gathered from userspace (typically, passed
>> through ucontrol parameter), which is a good choice to protect against
>> some bug external to the driver; however, if I'm not mistaken,
>> via_mux_enum_put (and corresponding _get) could still be missing
>> something: since any data in ucontrol->id is passed as is to
>> snd_ctl_get_ioffidx, is there any chance that the result could be an
>> invalid index?
>
> No, such an invalid id will be filtered out in the upper layer.
>
>
> thanks,
>
> Takashi
>


Thanks for your clarification.

Attaching alsa-info.sh output for my last test (with the above changes
to make Independent HP work)

Regards,

Alex
-------------- next part --------------
A non-text attachment was scrubbed...
Name: alsa-info.tar.gz
Type: application/x-gzip
Size: 7219 bytes
Desc: not available
Url : http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20110706/fd506548/attachment-0001.gz