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

["Sheetal ." <sheetal@nvidia.com>]

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.