[PATCH v2] ASoC: rt5677: make volume TLV closer to reality

[PATCH v2] ASoC: rt5677: make volume TLV closer to reality

[Dylan Reid]

The volume blocks have an step of 0.375dB, but TLV uses 0.01dB for
units.  Switch to use TLV_DB_MINMAX as this allows the minimum to be
off by only 0.005dB.  This is an improvement from the ~500dB is was
misreported by before.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
---
 sound/soc/codecs/rt5677.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 16aa4d9..a4c71f9 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -538,9 +538,9 @@ static bool rt5677_readable_register(struct device *dev, unsigned int reg)
 }
 
 static const DECLARE_TLV_DB_SCALE(out_vol_tlv, -4650, 150, 0);
-static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -65625, 375, 0);
+static const DECLARE_TLV_DB_MINMAX(dac_vol_tlv, -6562, 0);
 static const DECLARE_TLV_DB_SCALE(in_vol_tlv, -3450, 150, 0);
-static const DECLARE_TLV_DB_SCALE(adc_vol_tlv, -17625, 375, 0);
+static const DECLARE_TLV_DB_MINMAX(adc_vol_tlv, -1762, 0);
 static const DECLARE_TLV_DB_SCALE(adc_bst_tlv, 0, 1200, 0);
 static const DECLARE_TLV_DB_SCALE(st_vol_tlv, -4650, 150, 0);
 
-- 
2.1.2.330.g565301e

Re: [PATCH v2] ASoC: rt5677: make volume TLV closer to reality

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 613 bytes --]

On Thu, Oct 09, 2014 at 03:11:27PM -0700, Dylan Reid wrote:
> The volume blocks have an step of 0.375dB, but TLV uses 0.01dB for
> units.  Switch to use TLV_DB_MINMAX as this allows the minimum to be
> off by only 0.005dB.  This is an improvement from the ~500dB is was
> misreported by before.

The usual thing to do with these is just to halve the resolution of the
control by ignoring the low bit so that you get exact values, though for
this device that does result in a relatively large step (step sizes like
0.125dB are more common).  I dunno, doesn't make much difference either
way from my point of view.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2] ASoC: rt5677: make volume TLV closer to reality

[Dylan Reid]

On Thu, Oct 9, 2014 at 4:10 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Oct 09, 2014 at 03:11:27PM -0700, Dylan Reid wrote:
>> The volume blocks have an step of 0.375dB, but TLV uses 0.01dB for
>> units.  Switch to use TLV_DB_MINMAX as this allows the minimum to be
>> off by only 0.005dB.  This is an improvement from the ~500dB is was
>> misreported by before.
>
> The usual thing to do with these is just to halve the resolution of the
> control by ignoring the low bit so that you get exact values, though for
> this device that does result in a relatively large step (step sizes like
> 0.125dB are more common).  I dunno, doesn't make much difference either
> way from my point of view.

I can easily add a flag to the UCM config to hint userspace to divide by 10, so
I'm not strongly tied to making this change if it isn't bothering anyone else.

Re: [PATCH v2] ASoC: rt5677: make volume TLV closer to reality

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 320 bytes --]

On Thu, Oct 09, 2014 at 07:27:40PM -0700, Dylan Reid wrote:

> I can easily add a flag to the UCM config to hint userspace to divide by 10, so
> I'm not strongly tied to making this change if it isn't bothering anyone else.

I was thinking about just doing this in the driver rather than working
around it in userspace.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2] ASoC: rt5677: make volume TLV closer to reality

[Anatol Pomozov]

Hi

On Mon, Oct 13, 2014 at 1:01 AM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Oct 09, 2014 at 07:27:40PM -0700, Dylan Reid wrote:
>
>> I can easily add a flag to the UCM config to hint userspace to divide by 10, so
>> I'm not strongly tied to making this change if it isn't bothering anyone else.
>
> I was thinking about just doing this in the driver rather than working
> around it in userspace.

If kernel solution is preferable are you going to accept Dylan's
solution then? Or you think it should be implemented other way?

Re: [PATCH v2] ASoC: rt5677: make volume TLV closer to reality

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 565 bytes --]

On Wed, Dec 03, 2014 at 12:10:52PM -0800, Anatol Pomozov wrote:

> If kernel solution is preferable are you going to accept Dylan's
> solution then? Or you think it should be implemented other way?

To repeat my original review comment:

| The usual thing to do with these is just to halve the resolution of the
| control by ignoring the low bit so that you get exact values, though for
| this device that does result in a relatively large step (step sizes like
| 0.125dB are more common).  I dunno, doesn't make much difference either
| way from my point of view.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v2] ASoC: rt5677: make volume TLV closer to reality

[Dylan Reid]

On Wed, Dec 3, 2014 at 12:13 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Dec 03, 2014 at 12:10:52PM -0800, Anatol Pomozov wrote:
>
>> If kernel solution is preferable are you going to accept Dylan's
>> solution then? Or you think it should be implemented other way?
>
> To repeat my original review comment:
>
> | The usual thing to do with these is just to halve the resolution of the
> | control by ignoring the low bit so that you get exact values, though for
> | this device that does result in a relatively large step (step sizes like
> | 0.125dB are more common).  I dunno, doesn't make much difference either
> | way from my point of view.

Sorry, My fault here.  I forgot to send a v2 after I got back from
ELCE.  I'll try to dig up the machine I have that change on and get it
sent out later in the week.