Re: [PATCH] ASoC: tegra: Add Mixer Fade controls

[Mark Brown]

On Wed, Apr 22, 2026 at 10:29:57AM +0000, Sheetal wrote:
> Add fade controls for the Tegra mixer to allow the user to configure
> the gain and duration for each mixer stream. The mixer supports up to
> 10 input streams (RX1 through RX10), each with independently

>   amixer -c <card> cset name="MIXER1 Fade" \
>     "<id1>,<gain1>,<duration1>[,<id2>,<gain2>,<duration2>,...]"
>   id       - mixer input RX stream index (1 to 10)
>   gain     - target gain level
>   duration - fade duration in samples (N3 parameter)

This would be more idomatically represented as 10 pairs of controls, one
with the gain and one with the duration.  Possibly also with an
additional strobe control which actually makes the changes take effect
if there's a need to do a block of them at once.

> +static int tegra210_mixer_put_fade(struct snd_kcontrol *kcontrol,
> +				   struct snd_ctl_elem_value *ucontrol)
> +{

> +	struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
> +	struct tegra210_mixer *mixer = snd_soc_component_get_drvdata(cmpnt);
> +	u32 rx_id, rx_gain, rx_duration;
> +	int id, base, err = 0;
> +
> +	/* Process array of RX stream parameters (id, gain, duration) */
> +	for (id = 0; id < TEGRA210_MIXER_RX_MAX; id++) {
> +		base = 3 * id;
> +		rx_id = ucontrol->value.integer.value[base];
> +		rx_gain = ucontrol->value.integer.value[base + 1];
> +		rx_duration = ucontrol->value.integer.value[base + 2];

I'm not seeing much validation of these, for example it looks like you
can set a duration of 0 quite happily which seems to be a problem as we
divide by the duration when configuring.

> +               /* Only update if values are different */
> +               if (mixer->duration[rx_id] != rx_duration ||
 +                   mixer->gain_value[rx_id] != rx_gain) {
> +                       err = tegra210_mixer_configure_gain(cmpnt, rx_id, false);

...

> +                       mixer->duration[rx_id] = rx_duration;
> +                       mixer->gain_value[rx_id] = rx_gain;

Doesn't this mean that the values that get configured are the ones that
were previously set, not the new ones?

Attachment: signature.asc
Description: PGP signature