Re: [PATCH v2] ASoC: tegra: Add per-stream Mixer Fade controls

[Sheetal .]

On 30-04-2026 11:29, Mark Brown wrote:
> On Wed, Apr 29, 2026 at 07:06:03AM +0000, Sheetal wrote:
> > Add per-stream fade controls for the Tegra mixer to allow
> > independently configuring target gain and fade duration for each of
> > the 10 input streams (RX1 through RX10).
>
> > +static int tegra210_mixer_get_fade_status(struct snd_kcontrol *kcontrol,
> > +					  struct snd_ctl_elem_value *ucontrol)
> > +{
>
> > +		if (count >= mixer->duration[id]) {
> > +			ucontrol->value.integer.value[id] = TEGRA210_MIXER_FADE_COMPLETE;
> > +			regmap_update_bits(mixer->regmap,
> > +					   MIXER_REG(TEGRA210_MIXER_RX1_CTRL, id),
> > +					   TEGRA210_MIXER_SAMPLE_COUNT_ENABLE, 0);
> > +			mixer->in_fade[id] = false;
> > +		} else {
> > +			ucontrol->value.integer.value[id] = TEGRA210_MIXER_FADE_ACTIVE;
> > +		}
>
> Is the _FADE_COMPLETE state a good idea here?  Only the first read after
> a fade will show it, and functionally it's the same as _IDLE - there's
> no current fade running.

Yes makes sense, will keep IDLE(0) and ACTIVE(1)

> Also is there any overhead to having the sample counting enabled?  This
> is the only thing that turns it off AFAICT so if userspace doesn't look
> at the control we'll just leave it running indefinitely.  Perhaps a
> timer to disable might be useful?  It's generally a bit odd that we have
> a write to the hardware in a get().


Rather than a timer (which would require sample rate tracking to convert 
the sample-based duration to wall-clock time), sample count will be 
disabled before re-enabling it in the put callback, ensuring the counter 
restarts from zero for each new fade. Does that sound reasonable, or 
would you prefer a different approach?


> > +	SOC_SINGLE_EXT("Fade Enable", SND_SOC_NOPM, 0, 1, 0,
> > +		       tegra210_mixer_get_fade_enable,
> > +		       tegra210_mixer_put_fade_enable),
>
> Fade Switch.

Ack

> >   struct tegra210_mixer {
> >   	int gain_value[TEGRA210_MIXER_RX_MAX];
> > +	u32 duration[TEGRA210_MIXER_RX_MAX];
> > +	bool in_fade[TEGRA210_MIXER_RX_MAX];
> > +	bool fade_pending[TEGRA210_MIXER_RX_MAX];
> >   	struct regmap *regmap;
> >   };
>
> gain_value is used by the existing RX n Gain Volume controls, don't we
> need separate data for the fade controls here, or should we just have
> the new Fade Gain controls?  Having the separate controls is more
> complicated.


The existing "RXn Gain Volume" applies gain immediately on each put 
call, while Fade Gain needs to be staged until the Fade Switch strobe is 
triggered. Sharing gain_value would break this staged semantics, so a 
separate fade_gain[] field keeps the two independent.