Re: [PATCH v2] ALSA: usb-audio: Add input gain and master output mixer elements for RME Babyface Pro

[Takashi Iwai <tiwai@xxxxxxx>]

On Wed, 07 Aug 2024 21:11:21 +0200,
Stefan Stistrup wrote:
> 
> Add missing input gain and master output mixer controls for RME Babyface Pro
> 
> This patch implements:
> 
> 1. Input gain controls for 2 mic and 2 line inputs
> 2. Master output volume controls for all 12 output channels
> 
> These additions allow for more complete control of the Babyface Pro under Linux.
> 
> Signed-off-by: Stefan Stistrup <sstistrup@xxxxxxxxx>
> ---
> Changes in v2:
>  - Check invalid negative values in snd_bbfpro_gain_put
>  - Coding style fixes, blank line in snd_bbfpro_gain_update and checkpatch.pl errors

There are a few other errors and warnings detected by checkpatch.pl.
Please fix those and resubmit.


thanks,

Takashi

> 
>  sound/usb/mixer_quirks.c | 164 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 163 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index 065a4be0d771..4c050110004b 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -2541,14 +2541,23 @@ enum {
>  #define SND_BBFPRO_CTL_REG2_PAD_AN1 4
>  #define SND_BBFPRO_CTL_REG2_PAD_AN2 5
> 
> -#define SND_BBFPRO_MIXER_IDX_MASK 0x1ff
> +#define SND_BBFPRO_MIXER_MAIN_OUT_CH_OFFSET 992
> +#define SND_BBFPRO_MIXER_IDX_MASK 0x3ff
>  #define SND_BBFPRO_MIXER_VAL_MASK 0x3ffff
>  #define SND_BBFPRO_MIXER_VAL_SHIFT 9
>  #define SND_BBFPRO_MIXER_VAL_MIN 0 // -inf
>  #define SND_BBFPRO_MIXER_VAL_MAX 65536 // +6dB
> 
> +#define SND_BBFPRO_GAIN_CHANNEL_MASK 0x03
> +#define SND_BBFPRO_GAIN_CHANNEL_SHIFT 7
> +#define SND_BBFPRO_GAIN_VAL_MASK 0x7f
> +#define SND_BBFPRO_GAIN_VAL_MIN 0
> +#define SND_BBFPRO_GAIN_VAL_MIC_MAX 65
> +#define SND_BBFPRO_GAIN_VAL_LINE_MAX 18 // 9db in 0.5db incraments
> +
>  #define SND_BBFPRO_USBREQ_CTL_REG1 0x10
>  #define SND_BBFPRO_USBREQ_CTL_REG2 0x17
> +#define SND_BBFPRO_USBREQ_GAIN 0x1a
>  #define SND_BBFPRO_USBREQ_MIXER 0x12
> 
>  static int snd_bbfpro_ctl_update(struct usb_mixer_interface *mixer, u8 reg,
> @@ -2695,6 +2704,115 @@ static int snd_bbfpro_ctl_resume(struct usb_mixer_elem_list *list)
>  	return snd_bbfpro_ctl_update(list->mixer, reg, idx, value);
>  }
> 
> +static int snd_bbfpro_gain_update(struct usb_mixer_interface *mixer,
> +				  u8 channel, u8 gain)
> +{
> +	int err;
> +	struct snd_usb_audio *chip = mixer->chip;
> +
> +	if (channel < 2) {
> +		// XLR preamp: 3-bit fine, 5-bit coarse; special case >60
> +		if (gain < 60)
> +			gain = ((gain % 3) << 5) | (gain / 3);
> +		else
> +			gain = ((gain % 6) << 5) | (60 / 3);
> +	}
> +
> +	err = snd_usb_lock_shutdown(chip);
> +	if (err < 0)
> +		return err;
> +
> +	err = snd_usb_ctl_msg(chip->dev,
> +				usb_sndctrlpipe(chip->dev, 0),
> +				SND_BBFPRO_USBREQ_GAIN,
> +				USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> +				gain, channel, NULL, 0);
> +
> +	snd_usb_unlock_shutdown(chip);
> +	return err;
> +}
> +
> +static int snd_bbfpro_gain_get(struct snd_kcontrol *kcontrol,
> +                               struct snd_ctl_elem_value *ucontrol)
> +{
> +	int value = kcontrol->private_value & SND_BBFPRO_GAIN_VAL_MASK;
> +	ucontrol->value.integer.value[0] = value;
> +	return 0;
> +}
> +
> +static int snd_bbfpro_gain_info(struct snd_kcontrol *kcontrol,
> +                                struct snd_ctl_elem_info *uinfo)
> +{
> +	int pv, channel;
> +
> +	pv = kcontrol->private_value;
> +	channel = (pv >> SND_BBFPRO_GAIN_CHANNEL_SHIFT) &
> +		  SND_BBFPRO_GAIN_CHANNEL_MASK;
> +
> +	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
> +	uinfo->count = 1;
> +	uinfo->value.integer.min = SND_BBFPRO_GAIN_VAL_MIN;
> +
> +	if (channel < 2) {
> +		uinfo->value.integer.max = SND_BBFPRO_GAIN_VAL_MIC_MAX;
> +	} else {
> +		uinfo->value.integer.max = SND_BBFPRO_GAIN_VAL_LINE_MAX;
> +	}
> +
> +	return 0;
> +}
> +
> +static int snd_bbfpro_gain_put(struct snd_kcontrol *kcontrol,
> +                               struct snd_ctl_elem_value *ucontrol)
> +{
> +	int pv, channel, old_value, value, err;
> +
> +	struct usb_mixer_elem_list *list = snd_kcontrol_chip(kcontrol);
> +	struct usb_mixer_interface *mixer = list->mixer;
> +
> +	pv = kcontrol->private_value;
> +	channel = (pv >> SND_BBFPRO_GAIN_CHANNEL_SHIFT) &
> +		  SND_BBFPRO_GAIN_CHANNEL_MASK;
> +	old_value = pv & SND_BBFPRO_GAIN_VAL_MASK;
> +	value = ucontrol->value.integer.value[0];
> +
> +	if (value < SND_BBFPRO_GAIN_VAL_MIN)
> +		return -EINVAL;
> +
> +	if (channel < 2) {
> +		if (value > SND_BBFPRO_GAIN_VAL_MIC_MAX)
> +			return -EINVAL;
> +	} else {
> +		if (value > SND_BBFPRO_GAIN_VAL_LINE_MAX)
> +			return -EINVAL;
> +	}
> +
> +	if (value == old_value)
> +		return 0;
> +
> +	err = snd_bbfpro_gain_update(mixer, channel, value);
> +	if (err < 0)
> +		return err;
> +
> +	kcontrol->private_value =
> +		  (channel << SND_BBFPRO_GAIN_CHANNEL_SHIFT) | value;
> +	return 1;
> +}
> +
> +static int snd_bbfpro_gain_resume(struct usb_mixer_elem_list *list)
> +{
> +	int pv, channel, value;
> +
> +	struct snd_kcontrol *kctl = list->kctl;
> +
> +	pv = kctl->private_value;
> +	channel = (pv >> SND_BBFPRO_GAIN_CHANNEL_SHIFT) &
> +		  SND_BBFPRO_GAIN_CHANNEL_MASK;
> +	value = pv & SND_BBFPRO_GAIN_VAL_MASK;
> +
> +	return snd_bbfpro_gain_update(list->mixer, channel, value);
> +}
> +
>  static int snd_bbfpro_vol_update(struct usb_mixer_interface *mixer, u16 index,
>  				 u32 value)
>  {
> @@ -2790,6 +2908,15 @@ static const struct snd_kcontrol_new snd_bbfpro_ctl_control = {
>  	.put = snd_bbfpro_ctl_put
>  };
> 
> +static const struct snd_kcontrol_new snd_bbfpro_gain_control = {
> +	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
> +	.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
> +	.index = 0,
> +	.info = snd_bbfpro_gain_info,
> +	.get = snd_bbfpro_gain_get,
> +	.put = snd_bbfpro_gain_put
> +};
> +
>  static const struct snd_kcontrol_new snd_bbfpro_vol_control = {
>  	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
>  	.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
> @@ -2813,6 +2940,18 @@ static int snd_bbfpro_ctl_add(struct usb_mixer_interface *mixer, u8 reg,
>  		&knew, NULL);
>  }
> 
> +static int snd_bbfpro_gain_add(struct usb_mixer_interface *mixer, u8 channel,
> +                               char *name)
> +{
> +	struct snd_kcontrol_new knew = snd_bbfpro_gain_control;
> +
> +	knew.name = name;
> +	knew.private_value = channel << SND_BBFPRO_GAIN_CHANNEL_SHIFT;
> +
> +	return add_single_ctl_with_resume(mixer, 0, snd_bbfpro_gain_resume,
> +		&knew, NULL);
> +}
> +
>  static int snd_bbfpro_vol_add(struct usb_mixer_interface *mixer, u16 index,
>  			      char *name)
>  {
> @@ -2860,6 +2999,29 @@ static int snd_bbfpro_controls_create(struct usb_mixer_interface *mixer)
>  		}
>  	}
> 
> +	// Main out volume
> +	for (i = 0 ; i < 12 ; ++i) {
> +		snprintf(name, sizeof(name), "Main-Out %s", output[i]);
> +		// Main outs are offset to 992
> +		err = snd_bbfpro_vol_add(mixer,
> +					 i + SND_BBFPRO_MIXER_MAIN_OUT_CH_OFFSET,
> +					 name);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	// Input gain
> +	for (i = 0 ; i < 4 ; ++i) {
> +		if (i < 2) {
> +			snprintf(name, sizeof(name), "Mic-%s Gain", input[i]);
> +		} else {
> +			snprintf(name, sizeof(name), "Line-%s Gain", input[i]);
> +		}
> +		err = snd_bbfpro_gain_add(mixer, i, name);
> +		if (err < 0)
> +			return err;
> +	}
> +
>  	// Control Reg 1
>  	err = snd_bbfpro_ctl_add(mixer, SND_BBFPRO_CTL_REG1,
>  				 SND_BBFPRO_CTL_REG1_CLK_OPTICAL,
> --
> 2.45.2
>