[PATCH] - ALSA hda ad1988 - Add Independent HP Switch to model 6stack-dig-fp

[PATCH] - ALSA hda ad1988 - Add Independent HP Switch to model 6stack-dig-fp

[Raymond Yau]

[-- Attachment #1: Type: text/plain, Size: 693 bytes --]

Add Independent HP Switch to model 6stack-dig-fp

allow user to switch Independent HP on/off by using ALSA specific "share
stream" mode

This don't need to make "Headphone Playback Volume" inactive but this also
mean that "Headphone Playback Volume" is no longer slave of the "Virtual
Master" any more


The alternative way is to use the audio selector 0x37 but there is
1) no simple mixer api for alsamixer to display an inactive "Headphone
Playback Volume" and active "Headphone Playback Switch"
2) no api to add/remove "Headphone Playback Volume" to/from the slave list
of "Virtual Master"

snd_hda_codec_write(codec, 0x37, 0, AC_VERB_SET_CONNECT_SEL,
spec->independent_hp ? 0x00 : 0x01 );

[-- Attachment #2: 0001-Add-Independent-HP-switch-to-6stack-dig-fp.patch --]
[-- Type: application/octet-stream, Size: 3435 bytes --]

From e9d7f0aaa80d1682566f2b123ce0cb1e9b22c73b Mon Sep 17 00:00:00 2001
From: Raymond Yau <superquad.vortex2@gmail.com>
Date: Sun, 17 Apr 2011 15:55:53 +0800
Subject: [PATCH ALSA HDA 4/5] Add "Independent HP" switch to "6stack-dig-fp"

This allow switch back from multistream playback using Front Panel HP to normal but with "Headphone Playback Volume" control using share stream feature of HDA

Signed-off-by: Raymond Yau <superquad.vortex2@gmail.com>

diff --git a/sound/pci/hda/patch_analog.c b/sound/pci/hda/patch_analog.c
index 734c6ee..34c3742 100644
--- a/sound/pci/hda/patch_analog.c
+++ b/sound/pci/hda/patch_analog.c
@@ -45,6 +45,7 @@ struct ad198x_spec {
 					 */
 	unsigned int cur_eapd;
 	unsigned int need_dac_fix;
+	int independent_hp;
 
 	hda_nid_t *alt_dac_nid;
 	struct hda_pcm_stream *stream_analog_alt_playback;
@@ -300,6 +301,57 @@ static int ad198x_check_power_status(struct hda_codec *codec, hda_nid_t nid)
 }
 #endif
 
+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;
+                ctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_WRITE;
+		ctl->vd[0].access |= active
+			? SNDRV_CTL_ELEM_ACCESS_WRITE : 0;       
+		snd_ctl_notify(codec->bus->card,
+			       SNDRV_CTL_EVENT_MASK_INFO, &ctl->id);
+	}
+}
+
+static int ad1988_independent_hp_info(struct snd_kcontrol *kcontrol,
+				   struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = 1;
+	return 0;
+}
+
+static int ad1988_independent_hp_get(struct snd_kcontrol *kcontrol,
+				  struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct ad198x_spec *spec = codec->spec;
+	*ucontrol->value.integer.value = spec->independent_hp;
+	return 0;
+}
+
+static int ad1988_independent_hp_put(struct snd_kcontrol *kcontrol,
+				  struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
+	struct ad198x_spec *spec = codec->spec;
+	if ( spec->independent_hp !=  *ucontrol->value.integer.value ) {
+	        spec->independent_hp =  *ucontrol->value.integer.value;
+                spec->multiout.no_share_stream = spec->independent_hp;
+                if (spec->independent_hp)
+			spec->multiout.hp_nid = 0;
+		else
+			spec->multiout.hp_nid = spec->alt_dac_nid[0];
+		return 1;
+	}
+	return 0;
+}
+
 /*
  * Analog playback callbacks
  */
@@ -2213,7 +2265,13 @@ static struct snd_kcontrol_new ad1988_6stack_mixers2[] = {
 
 static struct snd_kcontrol_new ad1988_6stack_fp_mixers[] = {
 	HDA_CODEC_VOLUME("Headphone Playback Volume", 0x03, 0x0, HDA_OUTPUT),
-
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "Independent HP",
+		.info = ad1988_independent_hp_info,
+		.get = ad1988_independent_hp_get,
+		.put = ad1988_independent_hp_put,
+	},
 	{ } /* end */
 };
 
@@ -3232,6 +3290,7 @@ static int patch_ad1988(struct hda_codec *codec)
 			spec->alt_dac_nid = ad1988_alt_dac_nid;
 			spec->stream_analog_alt_playback =
 				&ad198x_pcm_analog_alt_playback;
+                        spec->independent_hp = 1;
 		}
 		if ((board_config == AD1988_6STACK_DIG) ||
 			(board_config == AD1988_6STACK_DIG_FP)) {
-- 
1.6.0.6


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] - ALSA hda ad1988 - Add Independent HP Switch to model 6stack-dig-fp

[Takashi Iwai]

At Tue, 19 Apr 2011 08:12:57 +0800,
Raymond Yau wrote:
> 
> Add Independent HP Switch to model 6stack-dig-fp
> 
> allow user to switch Independent HP on/off by using ALSA specific "share
> stream" mode
> 
> This don't need to make "Headphone Playback Volume" inactive but this also
> mean that "Headphone Playback Volume" is no longer slave of the "Virtual
> Master" any more

The implementation looks almost OK (although there are a few pieces
of unused code slike activate_ctl()), I prefer an enum control for it.

It's a frequently seen problem that user sets "Independent HP" mixer
switch accidentally and gets unexpected results.  An enum control
tends to be more clear than a mixer switch in general.

Could you rewrite the patch (and do a bit more clean up) and resend?


thanks,

Takashi

Re: [PATCH] - ALSA hda ad1988 - Add Independent HP Switch to model 6stack-dig-fp

[Takashi Iwai]

At Thu, 28 Apr 2011 21:10:23 +0800,
Raymond Yau wrote:
> 
> 2011/4/26 Takashi Iwai <tiwai@suse.de>
> 
> > At Tue, 19 Apr 2011 08:12:57 +0800,
> > Raymond Yau wrote:
> > >
> > > Add Independent HP Switch to model 6stack-dig-fp
> > >
> > > allow user to switch Independent HP on/off by using ALSA specific "share
> > > stream" mode
> > >
> > > This don't need to make "Headphone Playback Volume" inactive but this
> > also
> > > mean that "Headphone Playback Volume" is no longer slave of the "Virtual
> > > Master" any more
> >
> > The implementation looks almost OK (although there are a few pieces
> > of unused code slike activate_ctl()), I prefer an enum control for it.
> >
> >
> The other ad1988 models does not fully utilised five DACs
> 
> This implementation is specific to 10 channels HDA ,as Front and Headphone
> just share the stream when "Independent HP" is off
> 
> The "Independent HP" switch have to be write protected when any application
> is using multiout or alt_playback , alsamixer just display the inactive
> control in grey color.
> 
> The driver return -EBUSY if any application try to open alt_playback when
> independent HP is switch off.

If it's really implemented :)  You defined activate_ctl() but it's
called from nowhere.

> As for the upmixing feature, I wonder why no one complain about sending left
> channel to the center and right channel to subwoofer

Well, maybe because usually you set up 5.1 properly when you have such
systems, then upmixing right in the backend, not in the driver.


> > It's a frequently seen problem that user sets "Independent HP" mixer
> > switch accidentally and gets unexpected results.  An enum control
> > tends to be more clear than a mixer switch in general.
> >
> > This depend on how the mixer application treat the "inactive state" of
> control (e.g. qasmixer make the "inactive" control invisible )

No, this is a question how easily such feature can be changed
mistakenly.  It's no technical question, but a design issue.
The mixer element itself isn't so self-explanatory, and people tend to
turn on every mixer switch once when something doesn't work and wonder
what's wrong.

> Still unable to enable the unsolicited event for jack sense even if I add
> the unsol_event and verb for the audio jacks at rear panel since I don't
> have the HDA compilant front audio panel
> 
> SoundMax automatically popup immediately when jack is plug into the rear
> panel at other OS, so the hardware is capable of jack sense at rear panel
> 
> AFG Function Id: 0x1 (unsol 0)
> 
> Is there any trick to enable/debug the unsolicited event ?
> seem unsol event is disabled in the HDA controller

No, there shouldn't be big differences.  Otherwise it won't work on
any other machines.

There might be other way to trigger the jack detection, e.g. GPIO
unsolicited event, depending on the codec chip.  I don't remember how
is AD1988, though...


Takashi

Re: [PATCH] - ALSA hda ad1988 - Add Independent HP Switch to model 6stack-dig-fp

[Raymond Yau]

2011/4/28 Takashi Iwai <tiwai@suse.de>


> > As for the upmixing feature, I wonder why no one complain about sending
> left
> > channel to the center and right channel to subwoofer
>
> Well, maybe because usually you set up 5.1 properly when you have such
> systems, then upmixing right in the backend, not in the driver.
>
>
I  have Creative 4.1 speakers which only has green and black jacks.

Refer to commit d29240ce57d96db7300360d1e6f18357810a5c2b

hda-intel driver unconditionally assigns the stream to multiple output pins
in copy-front mode

I can hear the stereo in all 4 jacks in rear panel because of copy-front
mode even in OSS emulation

This is why ad1988 still can use "Headphone Playback Volume" at DAC0 when
switch off "independent Headphone"f