From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH] HDA - add "Independent HP" switch for ad1988 Date: Wed, 21 Sep 2011 10:45:07 +0200 Message-ID: References: Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 2C310103AAD for ; Wed, 21 Sep 2011 10:45:11 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: alsa-devel-bounces@alsa-project.org Errors-To: alsa-devel-bounces@alsa-project.org To: Raymond Yau Cc: ALSA Development Mailing List List-Id: alsa-devel@alsa-project.org 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