Clemens Ladisch wrote:
Benoît Thébaudeau wrote:
Clemens Ladisch wrote:
Benoît Thébaudeau wrote:
In case of a TLV dB range with all items having raw value ranges strictly within the main raw value range reported by the driver, snd_tlv_convert_from_dB() returned one of the main raw range boundaries, which was outside all dB range items.
But the main raw range boundaries _are_ valid values.
Not always. For instance, if a codec register has the following volume values: 0x00 to 0x20: prohibited 0x21 to 0x40: some dB values 0x41 to 0x60: prohibited 0x61 to 0xff: some dB values
then the kernel driver will use the existing SOC_SINGLE_TLV macro, which, like most TLV macros, automatically sets min to 0. The driver then relies on the associated TLV range description to filter out invalid raw values.
All raw values described by the .info callback, returned by the .get callback, and especially accepted by the .put callback, are valid.
The TLV information just _describes_ values, it cannot _restrict_ them.
If a register has invalid values, the driver must somehow map between raw control values and valid register values.
Now I understand how you see things. But is it an official and absolute ALSA rule, known by everybody, that may cause issues if not applied?
Here is my point of view. Let's take a real example, which corresponds to the volume table of the ROHM BD37534 on page 26: http://www.rohm.com/products/databook/audio/pdf/bd37531fv-e.pdf
This is what gave the 1st example of my patch comment: static const unsigned int bd3753x_vol_fader_gain_att_tlv[] = { TLV_DB_RANGE_HEAD(2), 0, 0, TLV_DB_SCALE_ITEM(TLV_DB_GAIN_MUTE, 0, 1), 48, 142, TLV_DB_SCALE_ITEM(-7900, 100, 0), }; SOC_SINGLE_TLV("Main Volume", BD3753X_VOL_GAIN, 0, 255, 1, bd3753x_vol_fader_gain_att_tlv),
This solution is the only possible one using existing ALSA SOC TLV macros. This indeed breaks the rule that you gave above, but is it really an issue? See below.
The hardware values are: 0x00 to 0x70: prohibited 0x71 to 0xcf: +15 dB to -79 dB by -1 dB steps 0xd0 to 0xfe: prohibited 0xff: mute
To respect your rule, the driver would have to implement its own .info/.get/.put callbacks, giving the following mapping: SW 0x00: HW 0xff: mute SW 0x01 to 0x5f: HW 0x71 to 0xcf: +15 dB to -79 dB by -1 dB steps
But why compel the driver to complicate things this way if it can work with standard macros and holes in the TLV ranges?
Do you think that it is not a so common case, and hence that it does not deserve to be handled by the general macros?
It is also the case for the TLV320AIC3104, page 0/register 19/field D6-D3 on page 51: http://www.ti.com/lit/ds/symlink/tlv320aic3104.pdf
It is also probably the case for many other existing hardware.
Do you think that the solution would be rather to add common kernel ALSA stuff to handle such mappings?
As for the patch, while I'm not sure whether it is needed at all, I also don't quite understand the reason for all the changes.
--- alsa-lib/src/control/tlv.c +++ alsa-lib/src/control/tlv.c @@ -291,41 +291,37 @@ int snd_tlv_convert_from_dB(unsigned int { switch (tlv[0]) { case SND_CTL_TLVT_DB_RANGE: {
long dbmin, dbmax, prev_submax;if (len < 6 || len > MAX_TLV_RANGE_SIZE) return -EINVAL;prev_submax = 0;do {Why is this not a while loop?
Because the added "if (len < 6" makes it pointless.
Then why did you add the len<6 check? This is, in effect, just the same as the old while condition.
Because it's useless to wait until the end of the loop to return -EINVAL, and with my patch, the after-loop code becomes the fallback case for beyond-final- range values, so it avoids adding a weird extra check there.
Regards, Benoît