[alsa-devel] [RFC] [Patch 1/2] Add virtual master control

[Takashi Iwai]

At Sat, 22 Mar 2008 11:30:46 -0400,
Daniel Jacobowitz wrote:
> 
> On Sat, Mar 22, 2008 at 10:09:23AM +0100, Takashi Iwai wrote:
> > At Fri, 21 Mar 2008 15:08:55 -0400,
> > Daniel Jacobowitz wrote:
> > > 
> > > On Wed, Jan 30, 2008 at 04:27:07PM +0100, Takashi Iwai wrote:
> > > > Because HD-audio is the only user, so far.  I don't want to expand the
> > > > core stuff (and kconfig) unless really needed.
> > > 
> > > I said I'd want this for ice1724 / vt1616, and I finally got time to
> > > do it.  What do you think of this patch?  I'm happily using it.
> > 
> > Well, don't forget to check HG version before you submit a patch :)
> 
> Whoops!  This version works with the hg tree.  The new
> snd_ac97_add_vmaster is mostly similar to the hda version so it might
> make sense to put that in vmaster.c instead.
> 
> From: Daniel Jacobowitz <dan at codesourcery.com>
> 
> Enable VMASTER for VT1616 / VT1617A.
> 
> Signed-off-by: Daniel Jacobowitz <dan at codesourcery.com>

Thanks.  This kind of change is basically good to apply, I think, but
a couple of things related are in my mind:

- other AC97 codecs need similar hacks, so it could be done
  cleaner (in a more generic way) than in each patch_*()?
- headphone and line-out have the similar issue - it's handled as a
  bind-control right now, though
- what about if the device actually uses only two channels although
  codec supports more than two?  Then the additional master control
  makes little sense.  But, we have no way to know it from the codec
  itself.  Another quirk table?  That's to be avoided...

Since this is a non-urgent fix and I'll be on vacation for three weeks
from tomorrow, I'd like to keep this pending unless other developers
want to put this in.  Is it OK?


Takashi


> diff -r 0d5f43585ca7 drivers/Kconfig
> --- a/drivers/Kconfig	Sat Mar 22 10:26:05 2008 +0100
> +++ b/drivers/Kconfig	Sat Mar 22 11:02:16 2008 -0400
> @@ -44,6 +44,7 @@ config SND_AC97_CODEC
>  	tristate
>  	select SND_PCM
>  	select AC97_BUS
> +	select SND_VMASTER
>  
>  config SND_DUMMY
>  	tristate "Dummy (/dev/null) soundcard"
> diff -r 0d5f43585ca7 pci/ac97/ac97_patch.c
> --- a/pci/ac97/ac97_patch.c	Sat Mar 22 10:26:05 2008 +0100
> +++ b/pci/ac97/ac97_patch.c	Sat Mar 22 11:02:16 2008 -0400
> @@ -3332,8 +3332,66 @@ AC97_SINGLE("Downmix Surround to Front",
>  AC97_SINGLE("Downmix Surround to Front", 0x5a, 11, 1, 0),
>  };
>  
> +static const char *slave_vols_vt1616[] = {
> +	"Front Playback Volume",
> +	"Surround Playback Volume",
> +	"Center Playback Volume",
> +	"LFE Playback Volume",
> +	NULL
> +};
> +
> +static const char *slave_sws_vt1616[] = {
> +	"Front Playback Switch",
> +	"Surround Playback Switch",
> +	"Center Playback Switch",
> +	"LFE Playback Switch",
> +	NULL
> +};
> +
> +/* find a mixer control element with the given name */
> +static struct snd_kcontrol *snd_ac97_find_mixer_ctl(struct snd_ac97 *ac97,
> +						    const char *name)
> +{
> +	struct snd_ctl_elem_id id;
> +	memset(&id, 0, sizeof(id));
> +	id.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
> +	strcpy(id.name, name);
> +	return snd_ctl_find_id(ac97->bus->card, &id);
> +}
> +
> +/* create a virtual master control and add slaves */
> +int snd_ac97_add_vmaster(struct snd_ac97 *ac97, char *name,
> +			 const unsigned int *tlv, const char **slaves)
> +{
> +	struct snd_kcontrol *kctl;
> +	const char **s;
> +	int err;
> +
> +	kctl = snd_ctl_make_virtual_master(name, tlv);
> +	if (!kctl)
> +		return -ENOMEM;
> +	err = snd_ctl_add(ac97->bus->card, kctl);
> +	if (err < 0)
> +		return err;
> +	
> +	for (s = slaves; *s; s++) {
> +		struct snd_kcontrol *sctl;
> +
> +		sctl = snd_ac97_find_mixer_ctl(ac97, *s);
> +		if (!sctl) {
> +			snd_printdd("Cannot find slave %s, skipped\n", *s);
> +			continue;
> +		}
> +		err = snd_ctl_add_slave(kctl, sctl);
> +		if (err < 0)
> +			return err;
> +	}
> +	return 0;
> +}
> +
>  static int patch_vt1616_specific(struct snd_ac97 * ac97)
>  {
> +	struct snd_kcontrol *kctl;
>  	int err;
>  
>  	if (snd_ac97_try_bit(ac97, 0x5a, 9))
> @@ -3341,6 +3399,24 @@ static int patch_vt1616_specific(struct 
>  			return err;
>  	if ((err = patch_build_controls(ac97, &snd_ac97_controls_vt1616[1], ARRAY_SIZE(snd_ac97_controls_vt1616) - 1)) < 0)
>  		return err;
> +
> +	/* There is already a misnamed master switch.  Rename it.  */
> +	kctl = snd_ac97_find_mixer_ctl(ac97, "Master Playback Volume");
> +	if (!kctl)
> +		return -EINVAL;
> +
> +	snd_ac97_rename_vol_ctl(ac97, "Master Playback", "Front Playback");
> +
> +	err = snd_ac97_add_vmaster(ac97, "Master Playback Volume",
> +				   kctl->tlv.p, slave_vols_vt1616);
> +	if (err < 0)
> +		return err;
> +
> +	err = snd_ac97_add_vmaster(ac97, "Master Playback Switch",
> +				   NULL, slave_sws_vt1616);
> +	if (err < 0)
> +		return err;
> +
>  	return 0;
>  }
>  
> 
> -- 
> Daniel Jacobowitz
> CodeSourcery
>