Re: [PATCH] HDA - add "Independent HP" switch for ad1988

[Takashi Iwai <tiwai@suse.de>]

At Sun, 18 Sep 2011 07:18:28 +0800,
Raymond Yau wrote:
> 
> Add "Independent HP" switch for ad1988
> 
> - add playback device 2 "AD1988 HP" for 7.1+2 multi streaming playback
> - add "Independent HP" switch to enable/disable the feature
>   switch is inactive and write protect when device 0 or device 2 is opened
> - remove 6stack-dig-fp model

Any rationale to remove this model?
I'm fine with the removal, but need to know the reason.

Also, regarding the patch:

> @@ -302,6 +304,71 @@ 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;

Please use tabs.

> +		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 void set_stream_active(struct hda_codec *codec, bool active)
> +{
> +	struct ad198x_spec *spec = codec->spec;
> +	if (active)
> +		spec->num_active_streams++;
> +	else
> +		spec->num_active_streams--;
> +	activate_ctl(codec, "Independent HP", spec->num_active_streams == 0);
> +	printk(KERN_INFO "set stream active : active stream  %d \n", spec->num_active_streams);

Avoid debug prints in the final code.
If any, use snd_printdd() or such.

> +}
+
+static int ad1988_independent_hp_info(struct snd_kcontrol *kcontrol,
+				   struct snd_ctl_elem_info *uinfo)
+{
+	static const char * const texts[] = { "OFF", "ON", NULL};
+	int index;
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
+	uinfo->count = 1;
+	uinfo->value.enumerated.items = 2;
+	index = uinfo->value.enumerated.item;
+	if (index >= 2)
+		index = 1;
+	strcpy(uinfo->value.enumerated.name, texts[index]);
+	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.enumerated.item[0] = 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;
+	unsigned int select = ucontrol->value.enumerated.item[0];
+	if (spec->independent_hp != select) {
+	        spec->independent_hp = select;
+		if (spec->independent_hp)
+			spec->multiout.hp_nid = 0; 
+		else
+			spec->multiout.hp_nid = spec->alt_dac_nid[0];
+		return 1;
+	}
+	return 0;
+}

Changing spec->multiout.hp_nid dynamically here is racy.  If the value
is changed during the PCM stream is opened, the HP-setup might be kept
inconsistent at close.


thanks,

Takashi