Hi,
17.08.2015, 17:16, Takashi Iwai kirjoitti:
> On Sun, 16 Aug 2015 14:50:12 +0200,
> Anssi Hannula wrote:
>>
>> AudioQuest DragonFly DAC reports a volume control range of 0..50
>> (0x0000..0x0032) which in USB Audio means a range of 0 .. 0.2dB, which
>> is obviously incorrect and causes software using the dB information in
>> e.g. volume sliders to have a massive volume difference in 100..102%
>> range.
>>
>> The actual volume mapping seems to be neither linear volume nor linear
>> dB scale, but instead quite close to the cubic mapping e.g. alsamixer
>> uses, with a range of -53...0 dB.
>>
>> Add a quirk for DragonFly to use a custom dB mapping, based on my
>> measurements, using a 10-item range TLV (so it still fits in alsa-lib
>> MAX_TLV_RANGE_SIZE).
>>
>> Tested on AudioQuest DragonFly HW v1.2. The quirk is only applied if the
>> range is 0..50, so if this gets fixed/changed in later HW revisions it
>> will no longer be applied.
>>
>> Signed-off-by: Anssi Hannula <anssi.hannula@xxxxxx>
>> Cc: <stable@xxxxxxxxxxxxxxx>
>> ---
>> sound/usb/mixer_quirks.c | 37 +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 37 insertions(+)
>>
>> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
>> index 337c317ead6f..39d7f34e44e6 100644
>> --- a/sound/usb/mixer_quirks.c
>> +++ b/sound/usb/mixer_quirks.c
>> @@ -37,6 +37,7 @@
>> #include <sound/control.h>
>> #include <sound/hwdep.h>
>> #include <sound/info.h>
>> +#include <sound/tlv.h>
>>
>> #include "usbaudio.h"
>> #include "mixer.h"
>> @@ -1733,6 +1734,38 @@ static int snd_microii_controls_create(struct usb_mixer_interface *mixer)
>> return 0;
>> }
>>
>> +static void snd_dragonfly_quirk_db_scale(struct usb_mixer_interface *mixer)
>> +{
>> + struct usb_mixer_elem_list *list;
>> + struct usb_mixer_elem_info *cval;
>> + static const int unit_id = 7;
>> +
>> + /* approximation using 10 ranges based on output measurement on hw v1.2 */
>> + static const DECLARE_TLV_DB_RANGE(scale,
>> + 0, 1, TLV_DB_MINMAX_ITEM(-5300, -4970),
>> + 2, 5, TLV_DB_MINMAX_ITEM(-4710, -4160),
>> + 6, 7, TLV_DB_MINMAX_ITEM(-3884, -3710),
>> + 8, 14, TLV_DB_MINMAX_ITEM(-3443, -2560),
>> + 15, 16, TLV_DB_MINMAX_ITEM(-2475, -2324),
>> + 17, 19, TLV_DB_MINMAX_ITEM(-2228, -2031),
>> + 20, 26, TLV_DB_MINMAX_ITEM(-1910, -1393),
>> + 27, 31, TLV_DB_MINMAX_ITEM(-1322, -1032),
>> + 32, 40, TLV_DB_MINMAX_ITEM(-968, -490),
>> + 41, 50, TLV_DB_MINMAX_ITEM(-441, 0),
>> + );
>> +
>> + for (list = mixer->id_elems[unit_id]; list; list = list->next_id_elem) {
>> + cval = (struct usb_mixer_elem_info *)list;
>> + if (cval->control == UAC_FU_VOLUME &&
>> + cval->min == 0 && cval->max == 50) {
>> + usb_audio_info(mixer->chip, "applying DragonFly dB scale quirk\n");
>> + list->kctl->tlv.p = scale;
>> + list->kctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_TLV_READ;
>> + list->kctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK;
>> + }
>> + }
>
> Instead of looking through the list, just hooking at
> build_feature_ctl() would be simpler in the end, I think.
> E.g. something like:
>
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -1334,6 +1334,7 @@ static void build_feature_ctl(struct mixer_build *state, void *raw_desc,
> SNDRV_CTL_ELEM_ACCESS_TLV_READ |
> SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK;
> }
> + mixer_fu_apply_quirk(state->mixer, cval, unitid, kctl);
> }
>
> range = (cval->max - cval->min) / cval->res;
>
> ... and the quirk implementation in mixer_quirks.c like
>
> static void snd_dragonfly_quirk_db_scale(mixer, kctl)
> {
> usb_audio_info(mixer->chip, "applying DragonFly dB scale quirk\n");
> kctl->tlv.p = scale;
> kctl->vd[0].access |= SNDRV_CTL_ELEM_ACCESS_TLV_READ;
> list->kctl->vd[0].access &= ~SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK;
> }
>
> void mixer_fu_apply_quirk(mixer, cval, unitid, kctl)
> {
> switch (mixer->chip->usb_id) {
> case USB_ID(0x21b4, 0x0081): /* AudioQuest DragonFly */
> if (unitid == 7 && cval->min == 0 && cval->max == 50)
> snd_dragonfly_quirk_db_scale(mixer, kctl);
> break;
> }
> }
OK, seems like a good idea.
However, I just noticed another volume quirk for DragonFly has already
been merged since I started looking into this:
https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/?id=2d1cb7f658fb9c3ba8f9dab8aca297d4dfdec835
It sets a fixed dB linear range map of 0...50dB via mixer_maps.c, which
doesn't seem 100% right to me. While it is much better than the
unquirked 0...0.2dB, it causes e.g. pulseaudio mixer to show the nearly
inaudible raw 0 as the "base" level. And, unless I made some silly
mistake, the volume scale is not actually linear dB AFAICS.
Yao-Wen, did you have some basis for the assumption "dB conversion
factor is 1" on DragonFly other than that it sounded approximately right?
Takashi, should I add removal of that "duplicate" quirk in the same
commit (or a separate one)? (assuming my quirk turns out to be actually
better/correct, of course)
--
Anssi Hannula
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html