diff mbox

ALSA: hda/via - Add beep controls to VIA codecs

Message ID 492ac5c1f00a4e07b44835f87fd5f05e8c4e6253.1427266376.git.wking@tremily.us (mailing list archive)
State New, archived
Headers show

Commit Message

W. Trevor King March 25, 2015, 7:01 a.m. UTC 
My codec has a beep-generating node:

  $ cat /proc/asound/card1/codec#0
  Codec: VIA VT1802
  ...
  Vendor Id: 0x11068446
  Subsystem Id: 0x15587410
  Revision Id: 0x100000
  ...
  Node 0x22 [Beep Generator Widget] wcaps 0x70040c: Mono Amp-Out
    Amp-Out caps: ofs=0x0a, nsteps=0x12, stepsize=0x05, mute=1
    Amp-Out vals:  [0x0a]
    Power states:  D0 D1 D2 D3
    Power: setting=D0, actual=D0
  ...

But I was missing the:

  Control: name=...

entries that I need to manage this widget from alsamixer.  With this
patch (based on the similar Mono Amp-Out handling in
patch_conexant.c), I get a new:

  input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/sound/card1/hdaudioC1D0/input15

entry in dmesg and controls to manage that beep:

  $ cat /proc/asound/card1/codec#0 | grep -A5 Beep
  Node 0x22 [Beep Generator Widget] wcaps 0x70040c: Mono Amp-Out
    Control: name="Beep Playback Volume", index=0, device=0
      ControlAmp: chs=1, dir=Out, idx=0, ofs=0
    Control: name="Beep Playback Switch", index=0, device=0
      ControlAmp: chs=1, dir=Out, idx=0, ofs=0
    Amp-Out caps: ofs=0x0a, nsteps=0x12, stepsize=0x05, mute=1
    Amp-Out vals:  [0x12]
    Power states:  D0 D1 D2 D3
    Power: setting=D0, actual=D0

Signed-off-by: W. Trevor King <wking@tremily.us>
---
On Sun, Mar 22, 2015 at 09:27:24PM +0800, Raymond Yau wrote:
> Why beep generator cannot automically found by auto parser  ?
> …
> end_nid = codec->start_nid + codec->num_nodes;
> for (nid = codec->start_nid; nid < end_nid; nid++) {
> unsigned int wid_caps = get_wcaps(codec, nid);
> unsigned int wid_type = get_wcaps_type(wid_caps);
> if (wid_type == AC_WID_BEEP)
>              spec->gen.beep_nid = nid;
>   }

This is pretty much what patch_conexant.c's cx_auto_parse_beep() does,
so I've just shifted that code over here to handle beeps in VIA
codecs.  It works on my VT1802, but I don't have any other VIA codecs
around to check that it works on all of the VIA codecs (Takashi
pointed out that they can be buggy [1]).  Do I need to shift the
auto_parse_beep() check into the VT1802-specific portion of
patch_vt2002P(), or can I leave it here in the generic
via_parse_auto_config()?  Alternatively, we could wedge it into
via_new_spec(), which would give the per-codec patches time to clear
spec->gen.beep_nid for broken codecs before via_parse_auto_config()
calls snd_hda_gen_parse_auto_config() which consumes beep_nid.

Cheers,
Trevor

[1]: id:s5hpp813vva.wl-tiwai@suse.de
     http://thread.gmane.org/gmane.linux.alsa.devel/135819/focus=135823

 sound/pci/hda/patch_via.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

Comments

W. Trevor King March 25, 2015, 7:03 a.m. UTC | #1 
On Wed, Mar 25, 2015 at 12:01:04AM -0700, W. Trevor King wrote:
>  sound/pci/hda/patch_via.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c
> index 3de6d3d..e4919f8 100644
> --- a/sound/pci/hda/patch_via.c
> +++ b/sound/pci/hda/patch_via.c

Oh, this patch was based on v3.19, but I'm happy to rebase it if it
doesn't apply to whichever tip the ALSA development work is done in.

Cheers,
Trevor
Takashi Iwai March 25, 2015, 7:15 a.m. UTC | #2 
At Wed, 25 Mar 2015 00:01:04 -0700,
W. Trevor King wrote:
> 
> My codec has a beep-generating node:
> 
>   $ cat /proc/asound/card1/codec#0
>   Codec: VIA VT1802
>   ...
>   Vendor Id: 0x11068446
>   Subsystem Id: 0x15587410
>   Revision Id: 0x100000
>   ...
>   Node 0x22 [Beep Generator Widget] wcaps 0x70040c: Mono Amp-Out
>     Amp-Out caps: ofs=0x0a, nsteps=0x12, stepsize=0x05, mute=1
>     Amp-Out vals:  [0x0a]
>     Power states:  D0 D1 D2 D3
>     Power: setting=D0, actual=D0
>   ...
> 
> But I was missing the:
> 
>   Control: name=...
> 
> entries that I need to manage this widget from alsamixer.  With this
> patch (based on the similar Mono Amp-Out handling in
> patch_conexant.c), I get a new:
> 
>   input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/sound/card1/hdaudioC1D0/input15
> 
> entry in dmesg and controls to manage that beep:
> 
>   $ cat /proc/asound/card1/codec#0 | grep -A5 Beep
>   Node 0x22 [Beep Generator Widget] wcaps 0x70040c: Mono Amp-Out
>     Control: name="Beep Playback Volume", index=0, device=0
>       ControlAmp: chs=1, dir=Out, idx=0, ofs=0
>     Control: name="Beep Playback Switch", index=0, device=0
>       ControlAmp: chs=1, dir=Out, idx=0, ofs=0
>     Amp-Out caps: ofs=0x0a, nsteps=0x12, stepsize=0x05, mute=1
>     Amp-Out vals:  [0x12]
>     Power states:  D0 D1 D2 D3
>     Power: setting=D0, actual=D0
> 
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
> On Sun, Mar 22, 2015 at 09:27:24PM +0800, Raymond Yau wrote:
> > Why beep generator cannot automically found by auto parser  ?
> > …
> > end_nid = codec->start_nid + codec->num_nodes;
> > for (nid = codec->start_nid; nid < end_nid; nid++) {
> > unsigned int wid_caps = get_wcaps(codec, nid);
> > unsigned int wid_type = get_wcaps_type(wid_caps);
> > if (wid_type == AC_WID_BEEP)
> >              spec->gen.beep_nid = nid;
> >   }
> 
> This is pretty much what patch_conexant.c's cx_auto_parse_beep() does,
> so I've just shifted that code over here to handle beeps in VIA
> codecs.  It works on my VT1802, but I don't have any other VIA codecs
> around to check that it works on all of the VIA codecs (Takashi
> pointed out that they can be buggy [1]).  Do I need to shift the
> auto_parse_beep() check into the VT1802-specific portion of
> patch_vt2002P(), or can I leave it here in the generic
> via_parse_auto_config()?  Alternatively, we could wedge it into
> via_new_spec(), which would give the per-codec patches time to clear
> spec->gen.beep_nid for broken codecs before via_parse_auto_config()
> calls snd_hda_gen_parse_auto_config() which consumes beep_nid.

I guess it's OK as is.  Not all VIA codecs seem to have the beep
widget, and the codecs with the beep widget have all the same
functionality (digital beep with the output amp).

Care to brush up your patch as a formal form to be merged to upstream?
There are a few conflicts with the latest sound.git tree code, but I
can manage to resolve them.

Later on, we can move the common code to the generic parser (with yet
some behavior flag), but it can be done by another patch.


thanks,

Takashi

> 
> Cheers,
> Trevor
> 
> [1]: id:s5hpp813vva.wl-tiwai@suse.de
>      http://thread.gmane.org/gmane.linux.alsa.devel/135819/focus=135823
> 
>  sound/pci/hda/patch_via.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c
> index 3de6d3d..e4919f8 100644
> --- a/sound/pci/hda/patch_via.c
> +++ b/sound/pci/hda/patch_via.c
> @@ -109,6 +109,8 @@ struct via_spec {
>  	int hp_work_active;
>  	int vt1708_jack_detect;
>  
> +	unsigned int beep_amp;
> +
>  	void (*set_widgets_power_state)(struct hda_codec *codec);
>  	unsigned int dac_stream_tag[4];
>  };
> @@ -355,6 +357,59 @@ static const struct snd_kcontrol_new via_pin_power_ctl_enum[] = {
>  	{} /* terminator */
>  };
>  
> +#ifdef CONFIG_SND_HDA_INPUT_BEEP
> +static inline void set_beep_amp(struct via_spec *spec, hda_nid_t nid,
> +				int idx, int dir)
> +{
> +	spec->gen.beep_nid = nid;
> +	spec->beep_amp = HDA_COMPOSE_AMP_VAL(nid, 1, idx, dir);
> +}
> +/* additional beep mixers; the actual parameters are overwritten at build */
> +static const struct snd_kcontrol_new cxt_beep_mixer[] = {
> +	HDA_CODEC_VOLUME_MONO("Beep Playback Volume", 0, 1, 0, HDA_OUTPUT),
> +	HDA_CODEC_MUTE_BEEP_MONO("Beep Playback Switch", 0, 1, 0, HDA_OUTPUT),
> +	{ } /* end */
> +};
> +
> +/* create beep controls if needed */
> +static int add_beep_ctls(struct hda_codec *codec)
> +{
> +	struct via_spec *spec = codec->spec;
> +	int err;
> +
> +	if (spec->beep_amp) {
> +		const struct snd_kcontrol_new *knew;
> +		for (knew = cxt_beep_mixer; knew->name; knew++) {
> +			struct snd_kcontrol *kctl;
> +			kctl = snd_ctl_new1(knew, codec);
> +			if (!kctl)
> +				return -ENOMEM;
> +			kctl->private_value = spec->beep_amp;
> +			err = snd_hda_ctl_add(codec, 0, kctl);
> +			if (err < 0)
> +				return err;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static void auto_parse_beep(struct hda_codec *codec)
> +{
> +	struct via_spec *spec = codec->spec;
> +	hda_nid_t nid, end_nid;
> +
> +	end_nid = codec->start_nid + codec->num_nodes;
> +	for (nid = codec->start_nid; nid < end_nid; nid++)
> +		if (get_wcaps_type(get_wcaps(codec, nid)) == AC_WID_BEEP) {
> +			set_beep_amp(spec, nid, 0, HDA_OUTPUT);
> +			break;
> +		}
> +}
> +#else
> +#define set_beep_amp(spec, nid, idx, dir) /* NOP */
> +#define add_beep_ctls(codec)	0
> +#define auto_parse_beep(codec)
> +#endif
>  
>  /* check AA path's mute status */
>  static bool is_aa_path_mute(struct hda_codec *codec)
> @@ -441,6 +496,10 @@ static int via_build_controls(struct hda_codec *codec)
>  	if (err < 0)
>  		return err;
>  
> +	err = add_beep_ctls(codec);
> +	if (err < 0)
> +		return err;
> +
>  	if (spec->set_widgets_power_state)
>  		spec->mixers[spec->num_mixers++] = via_pin_power_ctl_enum;
>  
> @@ -631,6 +690,8 @@ static int via_parse_auto_config(struct hda_codec *codec)
>  	if (err < 0)
>  		return err;
>  
> +	auto_parse_beep(codec);
> +
>  	err = snd_hda_gen_parse_auto_config(codec, &spec->gen.autocfg);
>  	if (err < 0)
>  		return err;
> -- 
> 2.1.0.60.g85f0837
>
diff mbox

Patch

diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c
index 3de6d3d..e4919f8 100644
--- a/sound/pci/hda/patch_via.c
+++ b/sound/pci/hda/patch_via.c
@@ -109,6 +109,8 @@  struct via_spec {
 	int hp_work_active;
 	int vt1708_jack_detect;
 
+	unsigned int beep_amp;
+
 	void (*set_widgets_power_state)(struct hda_codec *codec);
 	unsigned int dac_stream_tag[4];
 };
@@ -355,6 +357,59 @@  static const struct snd_kcontrol_new via_pin_power_ctl_enum[] = {
 	{} /* terminator */
 };
 
+#ifdef CONFIG_SND_HDA_INPUT_BEEP
+static inline void set_beep_amp(struct via_spec *spec, hda_nid_t nid,
+				int idx, int dir)
+{
+	spec->gen.beep_nid = nid;
+	spec->beep_amp = HDA_COMPOSE_AMP_VAL(nid, 1, idx, dir);
+}
+/* additional beep mixers; the actual parameters are overwritten at build */
+static const struct snd_kcontrol_new cxt_beep_mixer[] = {
+	HDA_CODEC_VOLUME_MONO("Beep Playback Volume", 0, 1, 0, HDA_OUTPUT),
+	HDA_CODEC_MUTE_BEEP_MONO("Beep Playback Switch", 0, 1, 0, HDA_OUTPUT),
+	{ } /* end */
+};
+
+/* create beep controls if needed */
+static int add_beep_ctls(struct hda_codec *codec)
+{
+	struct via_spec *spec = codec->spec;
+	int err;
+
+	if (spec->beep_amp) {
+		const struct snd_kcontrol_new *knew;
+		for (knew = cxt_beep_mixer; knew->name; knew++) {
+			struct snd_kcontrol *kctl;
+			kctl = snd_ctl_new1(knew, codec);
+			if (!kctl)
+				return -ENOMEM;
+			kctl->private_value = spec->beep_amp;
+			err = snd_hda_ctl_add(codec, 0, kctl);
+			if (err < 0)
+				return err;
+		}
+	}
+	return 0;
+}
+
+static void auto_parse_beep(struct hda_codec *codec)
+{
+	struct via_spec *spec = codec->spec;
+	hda_nid_t nid, end_nid;
+
+	end_nid = codec->start_nid + codec->num_nodes;
+	for (nid = codec->start_nid; nid < end_nid; nid++)
+		if (get_wcaps_type(get_wcaps(codec, nid)) == AC_WID_BEEP) {
+			set_beep_amp(spec, nid, 0, HDA_OUTPUT);
+			break;
+		}
+}
+#else
+#define set_beep_amp(spec, nid, idx, dir) /* NOP */
+#define add_beep_ctls(codec)	0
+#define auto_parse_beep(codec)
+#endif
 
 /* check AA path's mute status */
 static bool is_aa_path_mute(struct hda_codec *codec)
@@ -441,6 +496,10 @@  static int via_build_controls(struct hda_codec *codec)
 	if (err < 0)
 		return err;
 
+	err = add_beep_ctls(codec);
+	if (err < 0)
+		return err;
+
 	if (spec->set_widgets_power_state)
 		spec->mixers[spec->num_mixers++] = via_pin_power_ctl_enum;
 
@@ -631,6 +690,8 @@  static int via_parse_auto_config(struct hda_codec *codec)
 	if (err < 0)
 		return err;
 
+	auto_parse_beep(codec);
+
 	err = snd_hda_gen_parse_auto_config(codec, &spec->gen.autocfg);
 	if (err < 0)
 		return err;