Re: [RFC PATCH 2/3] ASoC: tegra: Update ASRC ratio controls

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

On 21-09-2026 17:20, Thierry Reding wrote:
> On Mon, Sep 21, 2026 at 08:57:03AM +0000, Sheetal wrote:
>> Replace the separate integer and fractional ASRC ratio controls with a
>> single two-value control for each stream. This is a userspace-visible
>> control ABI change, but:
>>
>> 1. Keeping writable compatibility controls would preserve the non-atomic
>>     update path that can expose a transient mixed ratio to hardware.
>> 2. This is limited to only the Tegra ASRC driver.
>>
>> Validate both values before programming hardware. The fractional field
>> keeps the existing 32-bit maximum, while the integer field is rejected
>> when it exceeds the hardware field width.
>>
>> ALSA exposes one min and max range for all values in an integer-array
>> control, so the paired ratio controls advertise the fractional field's
>> full 32-bit range. The put callback validates the integer part separately
>> against the hardware field width.
>>
>> Serialize ratio-source changes, stream setup, and ratio updates with a
>> driver mutex so cached ratio state and paired hardware programming remain
>> consistent.
>>
>> Unlock and re-lock the ASRC stream before programming a software ratio
>> when the new ratio differs from the cached ratio by more than 20%.
>> Large ratio jumps require the hardware lock to be refreshed around the
>> paired integer/fractional update.
>>
>> Roll back the previous hardware pair and lock state if a paired ratio
>> update fails after partially programming the hardware.
>>
>> Signed-off-by: Sheetal <sheetal@nvidia.com>
>> ---
>>   sound/soc/tegra/tegra186_asrc.c | 443 ++++++++++++++++++++------------
>>   sound/soc/tegra/tegra186_asrc.h |   4 +
>>   2 files changed, 276 insertions(+), 171 deletions(-)
>>
>> diff --git a/sound/soc/tegra/tegra186_asrc.c b/sound/soc/tegra/tegra186_asrc.c
>> index d8ae5d997615..2b67d23f5fa7 100644
>> --- a/sound/soc/tegra/tegra186_asrc.c
>> +++ b/sound/soc/tegra/tegra186_asrc.c
>> @@ -19,6 +19,9 @@
>>   #include "tegra186_asrc.h"
>>   #include "tegra_cif.h"
>>   
>> +#define TEGRA186_ASRC_RATIO_PERCENT_SCALE		100
>> +#define TEGRA186_ASRC_RATIO_UNLOCK_THRESHOLD_PERCENT	20
>> +
>>   #define ASRC_STREAM_SOURCE_SELECT(id)					       \
>>   	(TEGRA186_ASRC_CFG + ((id) * TEGRA186_ASRC_STREAM_STRIDE))
>>   
>> @@ -65,13 +68,125 @@ static const struct reg_default tegra186_asrc_reg_defaults[] = {
>>   	{ TEGRA186_ASRC_CYA, 0x0},
>>   };
>>   
>> -static void tegra186_asrc_lock_stream(struct tegra186_asrc *asrc,
>> -				      unsigned int id)
>> +static int tegra186_asrc_set_stream_lock(struct tegra186_asrc *asrc,
>> +					 unsigned int id, bool lock)
>>   {
>> -	regmap_write(asrc->regmap,
>> -		     ASRC_STREAM_REG(TEGRA186_ASRC_RATIO_LOCK_STATUS,
>> -				     id),
>> -		     1);
>> +	return regmap_write(asrc->regmap,
>> +			    ASRC_STREAM_REG(TEGRA186_ASRC_RATIO_LOCK_STATUS,
>> +					    id),
>> +			    lock);
>> +}
>> +
>> +static int tegra186_asrc_write_ratio_pair(struct tegra186_asrc *asrc,
>> +					  unsigned int id,
>> +					  unsigned int int_part,
>> +					  unsigned int frac_part)
>> +{
>> +	int ret;
>> +
>> +	ret = regmap_write(asrc->regmap,
>> +			   ASRC_STREAM_REG(TEGRA186_ASRC_RATIO_INT_PART, id),
>> +			   int_part);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return regmap_write(asrc->regmap,
>> +			    ASRC_STREAM_REG(TEGRA186_ASRC_RATIO_FRAC_PART, id),
>> +			    frac_part);
>> +}
>> +
>> +static int tegra186_asrc_read_ratio_pair(struct tegra186_asrc *asrc,
>> +					 unsigned int id, unsigned int *int_part,
>> +					 unsigned int *frac_part)
>> +{
>> +	int ret;
>> +
>> +	ret = regmap_read(asrc->regmap,
>> +			  ASRC_STREAM_REG(TEGRA186_ASRC_RATIO_INT_PART, id),
>> +			  int_part);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return regmap_read(asrc->regmap,
>> +			   ASRC_STREAM_REG(TEGRA186_ASRC_RATIO_FRAC_PART, id),
>> +			   frac_part);
>> +}
>> +
>> +static void tegra186_asrc_cache_ratio(struct tegra186_asrc *asrc,
>> +				      unsigned int id, unsigned int int_part,
>> +				      unsigned int frac_part)
>> +{
>> +	asrc->lane[id].int_part = int_part;
>> +	asrc->lane[id].frac_part = frac_part;
>> +}
>> +
>> +static int tegra186_asrc_apply_ratio(struct tegra186_asrc *asrc,
>> +				     unsigned int id, unsigned int int_part,
>> +				     unsigned int frac_part, bool unlock)
>> +{
>> +	unsigned int old_int_part, old_frac_part, old_lock;
>> +	int ret, restore_ret;
>> +
>> +	ret = tegra186_asrc_read_ratio_pair(asrc, id, &old_int_part,
>> +					    &old_frac_part);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_read(asrc->regmap,
>> +			  ASRC_STREAM_REG(TEGRA186_ASRC_RATIO_LOCK_STATUS, id),
>> +			  &old_lock);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (unlock) {
>> +		ret = tegra186_asrc_set_stream_lock(asrc, id, false);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	ret = tegra186_asrc_write_ratio_pair(asrc, id, int_part, frac_part);
>> +	if (ret)
>> +		goto restore_ratio;
>> +
>> +	ret = tegra186_asrc_set_stream_lock(asrc, id, true);
>> +	if (!ret)
>> +		return 0;
>> +
>> +restore_ratio:
>> +	restore_ret = tegra186_asrc_write_ratio_pair(asrc, id,
>> +						     old_int_part,
>> +						     old_frac_part);
>> +	if (!restore_ret) {
>> +		if (old_lock)
>> +			restore_ret = tegra186_asrc_set_stream_lock(asrc, id,
>> +								    true);
>> +		else
>> +			restore_ret = tegra186_asrc_set_stream_lock(asrc, id,
>> +								    false);
>> +	}
>> +
>> +	return ret ?: restore_ret;
>> +}
>> +
>> +static int tegra186_asrc_apply_cached_ratio(struct tegra186_asrc *asrc,
>> +					    unsigned int id)
>> +{
>> +	return tegra186_asrc_apply_ratio(asrc, id, asrc->lane[id].int_part,
>> +					asrc->lane[id].frac_part, false);
>> +}
>> +
>> +static bool tegra186_asrc_need_unlock(u64 old_ratio, u64 new_ratio)
>> +{
>> +	u64 ratio_diff;
>> +
>> +	if (!old_ratio || !new_ratio)
>> +		return false;
>> +
>> +	ratio_diff = (old_ratio > new_ratio) ?
>> +		     (old_ratio - new_ratio) : (new_ratio - old_ratio);
>> +
>> +	return ((ratio_diff * TEGRA186_ASRC_RATIO_PERCENT_SCALE) >
>> +		(old_ratio * TEGRA186_ASRC_RATIO_UNLOCK_THRESHOLD_PERCENT));
>>   }
> 
> There's a fair bit of complication introduced by the concept of locking
> here, so I wonder what the impact of this is. The function name implies
> that sometimes unlocking is needed and sometimes it isn't.
> 
> What if we unlock unconditionally? That doesn't get rid of all the
> locking and unlocking, but at least it would avoid the need to track the
> current locking status.
> 
> The unlock operation doesn't seem overly expensive, so maybe it isn't
> worth trying to avoid it?
> 
> [...]


This follows the ASRC hardware programming sequence. SW can update 
ratios directly for changes below 20%, but must unlock/relock the stream 
for changes above 20%. I would prefer not to unlock/relock for every 
ratio change because stream locking takes some time and would delay 
configuration unnecessarily for small ratio updates.


>> diff --git a/sound/soc/tegra/tegra186_asrc.h b/sound/soc/tegra/tegra186_asrc.h
>> index 0c98e26d5e72..9cc8cec6c204 100644
>> --- a/sound/soc/tegra/tegra186_asrc.h
>> +++ b/sound/soc/tegra/tegra186_asrc.h
>> @@ -7,6 +7,8 @@
>>   #ifndef __TEGRA186_ASRC_H__
>>   #define __TEGRA186_ASRC_H__
>>   
>> +#include <linux/mutex.h>
>> +
>>   /* ASRC stream related offset */
>>   #define TEGRA186_ASRC_CFG				0x0
>>   #define TEGRA186_ASRC_RATIO_INT_PART			0x4
>> @@ -111,6 +113,8 @@ struct tegra186_asrc {
>>   	const struct tegra_asrc_soc_data *soc_data;
>>   	struct tegra186_asrc_lane lane[TEGRA186_ASRC_STREAM_MAX];
>>   	struct regmap *regmap;
>> +	/* Serializes ratio source, cache, and paired register updates. */
>> +	struct mutex ratio_lock;
> 
> All of the operations that are serialized by this lock seem to be non-
> sleeping register or cache updates. Could the lock be turned into a
> spinlock instead?


I think this should remain a mutex. The protected paths call regmap and 
runtime PM helpers, which can sleep.


> 
> Thierry