[alsa-devel] [PATCH 2/5] HDA : add L/R swap of center/LFE for sigmatel codecs

[Takashi Iwai]

At Tue, 28 Aug 2007 05:07:56 +0300,
Maxim Levitsky wrote:
> 
> From 50e9864ecccb70aa6ba72fe542cf898add11f43a Mon Sep 17 00:00:00 2001
> From: Maxim Levitsky <maximlevitsky at gmail.com>
> Date: Mon, 27 Aug 2007 20:21:29 +0300
> Subject: [PATCH] HDA : add L/R swap of center/LFE for sigmatel codecs
> 
> ---
>  sound/pci/hda/patch_sigmatel.c |   64 ++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 61 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c
> index 1627ca3..12b318e 100644
> --- a/sound/pci/hda/patch_sigmatel.c
> +++ b/sound/pci/hda/patch_sigmatel.c
> @@ -127,6 +127,7 @@ struct sigmatel_spec {
>  
>  	/* i/o switches */
>  	unsigned int io_switch[2];
> +	unsigned int clfe_swap;
>  
>  	struct hda_pcm pcm_rec[2];	/* PCM information */
>  
> @@ -1078,8 +1079,43 @@ static int stac92xx_io_switch_put(struct snd_kcontrol *kcontrol, struct snd_ctl_
>          return 1;
>  }
>  
> +static int stac92xx_clfe_switch_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;
> +}

You can use a common helper snd_ctl_boolean_mono_info() that was added
recently.

> +
> +static int stac92xx_clfe_switch_get(struct snd_kcontrol *kcontrol,
> +		struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> +	struct sigmatel_spec *spec = codec->spec;
> +
> +	ucontrol->value.integer.value[0] = spec->clfe_swap;
> +	return 0;
> +}
> +
> +static int stac92xx_clfe_switch_put(struct snd_kcontrol *kcontrol,
> +		struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
> +	struct sigmatel_spec *spec = codec->spec;
> +	hda_nid_t nid = kcontrol->private_value & 0xff;
> +
> +	spec->clfe_swap = ucontrol->value.integer.value[0];
> +
> +	snd_hda_codec_write(codec, nid, 0, AC_VERB_SET_EAPD_BTLENABLE,
> +		spec->clfe_swap ? 0x4 : 0x0);
> +
> +	return 1;
> +}

The put callback should check whether the value is changed or not.
It should change the value only when changed, and then return 1.
If unchanged, it returns 0.

Also, the recent hda-intel driver uses snd_hda_codec_write_cache() for
the register values to be resumed after suspend or power-save.  I
think it's the case, too.  Simply use snd_hda_codec_write_cache()
there.


>  #define STAC_CODEC_IO_SWITCH(xname, xpval) \
> -	{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
> +	{.iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
>  	  .name = xname, \
>  	  .index = 0, \
>            .info = stac92xx_io_switch_info, \
> @@ -1088,17 +1124,28 @@ static int stac92xx_io_switch_put(struct snd_kcontrol *kcontrol, struct snd_ctl_
>            .private_value = xpval, \
>  	}
>  
> +#define STAC_CODEC_CLFE_SWITCH(xname, xpval) \
> +	{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
> +	  .name = xname, \
> +	  .index = 0, \
> +	  .info = stac92xx_clfe_switch_info, \
> +	  .get = stac92xx_clfe_switch_get, \
> +	  .put = stac92xx_clfe_switch_put, \
> +	  .private_value = xpval, \
> +	}
>  
>  enum {
>  	STAC_CTL_WIDGET_VOL,
>  	STAC_CTL_WIDGET_MUTE,
>  	STAC_CTL_WIDGET_IO_SWITCH,
> +	STAC_CTL_WIDGET_CLFE_SWITCH
>  };
>  
>  static struct snd_kcontrol_new stac92xx_control_templates[] = {
>  	HDA_CODEC_VOLUME(NULL, 0, 0, 0),
>  	HDA_CODEC_MUTE(NULL, 0, 0, 0),
>  	STAC_CODEC_IO_SWITCH(NULL, 0),
> +	STAC_CODEC_CLFE_SWITCH(NULL, 0),
>  };
>  
>  /* add dynamic controls */
> @@ -1287,7 +1334,7 @@ static int create_controls(struct sigmatel_spec *spec, const char *pfx, hda_nid_
>  }
>  
>  /* add playback controls from the parsed DAC table */
> -static int stac92xx_auto_create_multi_out_ctls(struct sigmatel_spec *spec,
> +static int stac92xx_auto_create_multi_out_ctls(struct hda_codec *codec,
>  					       const struct auto_pin_cfg *cfg)
>  {
>  	static const char *chname[4] = {
> @@ -1296,6 +1343,10 @@ static int stac92xx_auto_create_multi_out_ctls(struct sigmatel_spec *spec,
>  	hda_nid_t nid;
>  	int i, err;
>  
> +	struct sigmatel_spec *spec = codec->spec;
> +	unsigned int wid_caps;
> +
> +
>  	for (i = 0; i < cfg->line_outs; i++) {
>  		if (!spec->multiout.dac_nids[i])
>  			continue;
> @@ -1310,6 +1361,13 @@ static int stac92xx_auto_create_multi_out_ctls(struct sigmatel_spec *spec,
>  			err = create_controls(spec, "LFE", nid, 2);
>  			if (err < 0)
>  				return err;
> +
> +			wid_caps = get_wcaps(codec, nid);
> +
> +			if (wid_caps & AC_WCAP_LR_SWAP)
> +				if ((err = stac92xx_add_control(spec, STAC_CTL_WIDGET_CLFE_SWITCH, "Swap Center/LFE Playback Switch", nid)) < 0)
> +					return err;

Please avoid if ((err = xxx) < 0) style.
Split err = xxx and if (err < 0).
Also, try to keep the line within 80 chars (at least for the new stuff
:)


Takashi