[PATCH 06/13] ASoC: tegra: Add Tegra210 based MVC driver

[Sameer Pujar]

Hi Mark,

Thank you for review.

On 9/3/2021 11:43 PM, Mark Brown wrote:
> On Fri, Aug 27, 2021 at 03:03:52PM +0530, Sameer Pujar wrote:
>> The Master Volume Control (MVC) provides gain or attenuation to a digital
>> signal path. It can be used in input or output signal path for per-stream
>> volume control or it can be used as master volume control. The MVC block
>> has one input and one output. The input digital stream can be mono or
>> multi-channel (up to 7.1 channels) stream. An independent mute control is
>> also included in the MVC block.
> Looks like it's also got a little bit of other DSP in there (a simple
> EQ?).  Not that it really matters.

MVC does not have EQ block. In fact it is part of another processing 
module, the driver for which is not part of this series.

>> +	if (reg == TEGRA210_MVC_CTRL) {
>> +		u32 val;
>> +		u8 mute_mask;
>> +	} else {
>> +		u8 chan = (reg - TEGRA210_MVC_TARGET_VOL) / REG_SIZE;
>> +		s32 val = mvc->volume[chan];
> It's not clear to me why we're using the same callbacks for the volume
> and mute settings - there's no shared code on the read path and only a
> tiny bit on the write path.

The volume switch trigger is common in put(). The get() is probably 
influenced from the put() and common function is maintained. If it makes 
more clear I will separate these out.

>> +	err |= regmap_update_bits(mvc->regmap, TEGRA210_MVC_SWITCH,
>> +			TEGRA210_MVC_VOLUME_SWITCH_MASK,
>> +			TEGRA210_MVC_VOLUME_SWITCH_TRIGGER);
>> +
>> +end:
>> +	pm_runtime_put(cmpnt->dev);
>> +	return err;
>> +}
> _put() should return 0 if there's no change or 1 for a change.

done

>> +	/* SW reset */
>> +	regmap_write(mvc->regmap, TEGRA210_MVC_SOFT_RESET, 1);
> What about all the cached values in the regmap, won't they get out of
> sync?  Especially things like volume and mute, it looks like the mute
> just gets written directly to the regmap and not otherwise saved.

1. I think original idea was to disable MVC (forcefully) before new 
configurations are applied and eventually it gets enabled via DAPM path. 
DAPM path handles disable also, but sometimes MVC may get stuck in 
enable state (due to a previous usage) when all the data in pipeline is 
not processed. My understanding is that reset is restricted to this 
alone. The volume/mute registers seem not to be affected by reset. 
However I will confirm with HW about the reset behavior and affected 
register map. Thanks for pointing this.

2. The mute is part of volatile register and volume is not. If the 
system is muted and any volume settings are made, the system is unmuted 
automatically. For this reason mute is always read from HW. For volume 
it is saved for local computation.