[alsa-devel] Via VT2020: issues with kernel 2.6.38.{2, 3} (alsa 1.0.23) - working with 2.6.33.2 (alsa 1.0.21)

alex dot baldacchino dot alsasub at gmail dot com alex.baldacchino.alsasub at gmail.com
Mon Jun 20 03:19:49 CEST 2011


2011/6/19 Raymond Yau <superquad.vortex2 at gmail.com>:
> 2011/6/12 alex dot baldacchino dot alsasub at gmail dot com
> <alex.baldacchino.alsasub at gmail.com>:
>> IMPORTANT UPDATE!!!!!
>>
>> Node 0x0b can drive HP! The problem was function
>> set_widgets_power_state_vt1718S() not updating power state for the
>> correct nid, solved as follows:
>>
>>
>> static void set_widgets_power_state_vt1718S(struct hda_codec *codec)
>> {
>> ...
>>        if (spec->hp_independent_mode) {
>> -               /* PW4 (28h), MW3 (1bh), MUX1(34h), AOW4 (ch) */
>> +               /* PW4 (28h), MW3 (1bh), MUX1(34h), AOW4 (ch) of AOW3 (bh) */
>>                parm = AC_PWRST_D3;
>>                set_pin_power_state(codec, 0x28, &parm);
>>                snd_hda_codec_write(codec, 0x1b, 0,
>>                                    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->multiout.hp_nid, 0,
>>                                    AC_VERB_SET_POWER_STATE, parm);
>>        }
>>
>
> Can your hear the same signal from black and orange jack when you
> playing stereo to hw:0,0,0 ?
>
> if you look at snd_hda_multi_out_analog_prepare() in hda_codec.c,
>
> if mout->no_share_stream=0 ,it seem that there is "upmix" feature when
> mout->num_dacs is 4 for those 5 jacks/6 jacks motherboard
>
> i.e. the black and orange jacks can have the same signal as the green jack
>
>        /* front */
>        snd_hda_codec_setup_stream(codec, nids[HDA_FRONT], stream_tag,
>                                   0, format);
>        if (!mout->no_share_stream &&
>            mout->hp_nid && mout->hp_nid != nids[HDA_FRONT])
>                /* headphone out will just decode front left/right (stereo) */
>                snd_hda_codec_setup_stream(codec, mout->hp_nid, stream_tag,
>                                           0, format);
>        /* extra outputs copied from front */
>        for (i = 0; i < ARRAY_SIZE(mout->extra_out_nid); i++)
>                if (!mout->no_share_stream && mout->extra_out_nid[i])
>                        snd_hda_codec_setup_stream(codec,
>                                                   mout->extra_out_nid[i],
>                                                   stream_tag, 0, format);
>
>        /* surrounds */
>        for (i = 1; i < mout->num_dacs; i++) {
>                if (chs >= (i + 1) * 2) /* independent out */
>                        snd_hda_codec_setup_stream(codec, nids[i], stream_tag,
>                                                   i * 2, format);
>                else if (!mout->no_share_stream) /* copy front */
>                        snd_hda_codec_setup_stream(codec, nids[i], stream_tag,
>                                                   0, format);
>        }
>        return 0;
> }
>


I fear I cannot test it the best way (I've got a 5.1 system, but have
some 'logistic' problems using it). I've made an attempt by using two
headphones, connected as follows:

1) one to (rear) green jack, one to orange,
2) one to green, one to black,
3) one to orange, one to black,

and played a short two-channels file with aplay -Dhw:0,0,0 stereo.wav
a few times, in each case I was able to here (same) sound from both
headsets.

However, it would seem to me that snd_hda_multi_out_analog_prepare()
is not used by VIA Codecs, instead, some pcm operations are defined
within patch_via.c and used by each codec, e.g.


static const struct hda_pcm_stream vt1718S_pcm_analog_playback = {
	.substreams = 2,
	.channels_min = 2,
	.channels_max = 10,
	.nid = 0x8, /* NID to query formats and rates */
	.ops = {
		.open = via_playback_pcm_open,
		.prepare = via_playback_multi_pcm_prepare,
		.cleanup = via_playback_multi_pcm_cleanup,
		.close = via_pcm_open_close,
	},
};


(not all codec use via_pcm_open_close, actually)

None of them calls snd_hda_multi_out_analog_{prepare, cleanup}() for
analog stuff, instead, part of the code in those (general) functions
is reused and modified, and spec->multiout.no_share_stream isn't used,
nor initialized (AFAICT).

By the way, my modification to set_widgets_power_state_vt1718S you've
quoted is due to 0x0b being set to D3 because side is missing:


	/* outputs */
	/* PW3 (27h), MW2 (1ah), AOW3 (bh) */
	parm = AC_PWRST_D3;
	set_pin_power_state(codec, 0x27, &parm);
	snd_hda_codec_write(codec, 0x1a, 0, AC_VERB_SET_POWER_STATE, parm);
	snd_hda_codec_write(codec, 0xb, 0, AC_VERB_SET_POWER_STATE, parm);


Since 0x27 is not present, parm is not changed  in
set_pin_power_state() and 0x0b is 'turned off'. Therefore, if 0x0b
must/can be used for independent hp, power state cannot be changed for
a fixed nid, but current value for spec->multiout.hp_nid have to be
used.


By the way, since via_playback_pcm_open() calls
snd_hda_multi_out_analog_open(), which is the last one touching
runtime->hw.channels_max, the problem you pointed out in your previous
mail to report the right number of max channels for a substream,
perhaps, could be solved the following way:

in file hda_codec.c:


/**
-* snd_hda_multi_out_analog_open - open analog outputs
+* snd_hda_multiout_analog_open_set - open analog outputs
 *
 * Open analog outputs and set up the hw-constraints.
 * If the digital outputs can be opened as slave, open the digital
 * outputs, too.
 */
-int snd_hda_multi_out_analog_open(struct hda_codec *codec,
+int snd_hda_multiout_analog_open_set(struct hda_codec *codec,
				  struct hda_multi_out *mout,
				  struct snd_pcm_substream *substream,
-				  struct hda_pcm_stream *hinfo)
+				  struct hda_pcm_stream *hinfo, int max_channels)
{
	struct snd_pcm_runtime *runtime = substream->runtime;
-	runtime->hw.channels_max = mout->max_channels;
+	if( max_channels > 0 && max_channels <= mout->max_channels)
+		runtime->hw.channels_max = max_channels;
+	else
+		runtime->hw.channels_max = mout->max_channels;
	if (mout->dig_out_nid) {
....

-EXPORT_SYMBOL_HDA(snd_hda_multi_out_analog_open);
+EXPORT_SYMBOL_HDA(snd_hda_multiout_analog_open_set);


+/**
+ * snd_hda_multi_out_analog_open - open analog outputs
+ *
+ * Open analog outputs and set up the hw-constraints.
+ * If the digital outputs can be opened as slave, open the digital
+ * outputs, too.
+ */
+int snd_hda_multi_out_analog_open(struct hda_codec *codec,
+				  struct hda_multi_out *mout,
+				  struct snd_pcm_substream *substream,
+				  struct hda_pcm_stream *hinfo)
+{
+	snd_hda_multiout_analog_open_set( codec, mout, substream, hinfo, 0 );
+}
+EXPORT_SYMBOL_HDA(snd_hda_multi_out_analog_open);


(to avoid breaking compatibility)

in file patch_via.c:


static int via_playback_pcm_open(struct hda_pcm_stream *hinfo,
				 struct hda_codec *codec,
				 struct snd_pcm_substream *substream)
{
	struct via_spec *spec = codec->spec;
	int idle = substream->pstr->substream_opened == 1
		&& substream->ref_count == 0;
	analog_low_current_mode(codec, idle);
-	return snd_hda_multi_out_analog_open(codec, &spec->multiout, substream,
-					     hinfo);
+	return snd_hda_multiout_analog_open_set( codec, &spec->multiout, substream,
+					hinfo, ( substream->number == 1 ? 2
+					: spec->multiout.max_channels ) );
}


(substream->number is the same as subdevice, because their matching is
tested in snd_pcm_attach_substream())

However, since the question is more general, and could affect other
hda (and non-hda) codecs/drivers, a careful discussion on the general
design could be needed.


>> but in via_independent_hp_put():
>>
>>
>>        unsigned int pinsel = ucontrol->value.enumerated.item[0];
>>        /* Get Independent Mode index of headphone pin widget */
>>        spec->hp_independent_mode = spec->hp_independent_mode_index == pinsel
>>                ? 1 : 0; /* <- TROUBLES! */
>>
>>
>> When turning on Independent HP mode for codec VT1718S, pinsel is 1,
>> when turning it off that's 0, thus, if spec->hp_independent_mode_index
>> is 2, spec->hp_independent_mode is always 0 (condition
>> spec->hp_independent_mode_index == pinsel alwyas fails) and
>> corresponding controls can't be activated:
>>
>>
>>        /* update HP volume/swtich active state */
>>        if (spec->codec_type == VT1708S
>>            || spec->codec_type == VT1702
>>            || spec->codec_type == VT1718S
>>            || spec->codec_type == VT1716S
>>            || VT2002P_COMPATIBLE(spec)) {
>>                activate_ctl(codec, "Headphone Playback Volume",
>>                             spec->hp_independent_mode);
>>                activate_ctl(codec, "Headphone Playback Switch",
>>                             spec->hp_independent_mode);
>>        }
>>
>
> if the hp jack can also use "copy front" mode for "redirected headphone"
>
> the driver don't need to disable "Headphone Playback Volume" for those
> 10 channels codecs or change the connection in
> via_independent_hp_put()

But if hp_independent_mode is always 0 because pinsel can never match
hp_independent_mode_index, those controls will _never_ be activated,
at least not by via_independent_hp_put, and, what's worse (as I
think), once via_independent_hp_put is called they'll become inactive,
even if setting the right connection for the "independent mode":


static void activate_ctl(struct hda_codec *codec, const char *name, int active)
{
	struct snd_kcontrol *ctl = snd_hda_find_mixer_ctl(codec, name);
	if (ctl) {
		ctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_INACTIVE;
		ctl->vd[0].access |= active
			? 0 : SNDRV_CTL_ELEM_ACCESS_INACTIVE;
		snd_ctl_notify(codec->bus->card,
			       SNDRV_CTL_EVENT_MASK_VALUE, &ctl->id);
	}
}


(active, as passed in by via_independent_hp_put is
spec->hp_independent_mode, which is always set to 0 if
spec->hp_independent_mode_index is 2, therefore the need to set
hp_independent_mode more properly)

Moreover, hp_independent_mode is used by other functions, such as
set_widgets_power_state_vtXXXX(), via_playback_multi_pcm_prepare(),
playback_multi_pcm_prep_0(), via_playback_multi_pcm_cleanup(),
therefore I think that's safer to set it properly, to avoid the risk
of breaking those algorithms counting on its value.

>
> For those 3 jacks motherobard, "channel mode" 6-8 or "Use HP as Side"
> switch may just need tot switch the connection to use 0x0b or 0x0c if
> 0b has  "Side Playback Volume" and 0x0c has "Headphone Playback
> Volume"


By the way, if there are 3 line-outs only, I'm selecting 0x0b as
multiout.hp_nid (as suggested by you), and also attaching control
"Headphone Playback Volume" to it:


static int vt1718S_auto_create_hp_ctls(struct via_spec *spec, hda_nid_t pin)
{
...
	err = via_add_control(spec, VIA_CTL_WIDGET_VOL,
			      "Headphone Playback Volume",
			   HDA_COMPOSE_AMP_VAL(spec->multiout.hp_nid, 3, 0, HDA_OUTPUT));

> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>


More information about the Alsa-devel mailing list