ASoC: snd_soc_info_volsw and platfrom_max

[Mark Brown]

On Mon, Aug 15, 2022 at 10:22:37AM +0100, Srinivas Kandagatla wrote:

> before this patch the controls max value was calculated considering the min
> value, but with this patch this calculation has changed resulting in low
> volume on most of the codecs that are using SOC_SINGLE_S8_TLV.

Right, the whole situation is unclear.  At various points the code
hasn't agreed with itself ragarding what the platform_max is relative
to, if it's taken into account and all both within individual control
types and also between control types.

> snd_soc_put_volsw does the right thing by considering mc->min, but
> info_volsw does it differently.

> Below change fixes the issue for me, but I am bit confused with the first
> line of this function that calculates max value as max = mc->max - mc->min
> and then limits it to platform_max.

The issue is that it's not clear if the platform_max value should be a
value for the register or a value for the control.  For some reason
(which frankly is the source of a lot of the problems here) the controls
adjust the user visible range to be zero based even though the ABI would
be totally fine with any range.  There were out of tree patches that
changed things for some of the control types too.

> diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
> index bd88de056358..49fb34609202 100644
> --- a/sound/soc/soc-ops.c
> +++ b/sound/soc/soc-ops.c
> @@ -196,7 +196,7 @@ int snd_soc_info_volsw(struct snd_kcontrol *kcontrol,
> 
>  	uinfo->count = snd_soc_volsw_is_stereo(mc) ? 2 : 1;
>  	uinfo->value.integer.min = 0;
> -	uinfo->value.integer.max = max;
> +	uinfo->value.integer.max = max  - mc->min;

That'll then break anything that doesn't set platform_max since we'll
apply mc->min twice, you'd need to do the adjustment at the point where
we apply the platform_max override.  Keeping platform_max a register
value is *probably* the least bad thing here.

>  	return 0;
>  }
> 
> 
> Or should we fix the macro to set platform_max to be max - min.

We shouldn't be changing the static data at all, that's one of the
problems.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20220816/735e4d7d/attachment.sig>