[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hda-codec: use smaller dynamic range for input
|
From: |
Marc-André Lureau |
|
Subject: |
Re: [Qemu-devel] [PATCH] hda-codec: use smaller dynamic range for input amplifier |
|
Date: |
Tue, 21 Apr 2015 13:14:32 -0400 (EDT) |
Hi
----- Original Message -----
> This patch addresses the following problem:
>
> Moving the input volume slider 0% to 1% in a Win7 guest causes the
> the "gain" values in the emulated HDA codec HW to jump from 0 to 40.
> This makes it impossible to control sound volume in the low-gain
> range using the windows guest.
> This is related to how the Windows HDA codec driver and the QEMU
> emulated HW interact. It seems that Windows uses a ~30dB overall
> scale for the min-max range of the input volume slider.
>
> The "right" solution for this problem would be to implement
> proper dB scaling in QEMU and the audio backends (such as spice).
>
> While this clean solution is not available, I suggest to decrease
> the dynamic range for the the emulated Amps in the QEMU hda codec.
> Experiments showed that with 32dB dynamic range with a 0.5 dB step,
> it was possible to set gain values as low as 5 (-29.5dB / 8%).
> Actual HW seems to use similar ranges for input amplifiers.
>
> Signed-off-by: Martin Wilck <address@hidden>
> ---
> hw/audio/hda-codec-common.h | 23 +++++++++++++++--------
> hw/audio/hda-codec.c | 23 +++++++++++++++++------
> 2 files changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/hw/audio/hda-codec-common.h b/hw/audio/hda-codec-common.h
> index b4fdb51..d59781b 100644
> --- a/hw/audio/hda-codec-common.h
> +++ b/hw/audio/hda-codec-common.h
> @@ -28,16 +28,22 @@
> #define QEMU_HDA_ID_OUTPUT ((QEMU_HDA_ID_VENDOR << 16) | 0x12)
> #define QEMU_HDA_ID_DUPLEX ((QEMU_HDA_ID_VENDOR << 16) | 0x22)
> #define QEMU_HDA_ID_MICRO ((QEMU_HDA_ID_VENDOR << 16) | 0x32)
> -#define QEMU_HDA_AMP_CAPS \
> +#define QEMU_HDA_AMP_OUT_CAPS
> \
> (AC_AMPCAP_MUTE | \
> - (QEMU_HDA_AMP_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
> - (QEMU_HDA_AMP_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
> - (3 << AC_AMPCAP_STEP_SIZE_SHIFT))
> + (QEMU_HDA_AMP_OUT_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
> + (QEMU_HDA_AMP_OUT_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
> + (QEMU_HDA_AMP_OUT_STEP_SIZE << AC_AMPCAP_STEP_SIZE_SHIFT))
> +#define QEMU_HDA_AMP_IN_CAPS \
> + (AC_AMPCAP_MUTE | \
> + (QEMU_HDA_AMP_IN_STEPS << AC_AMPCAP_OFFSET_SHIFT) | \
> + (QEMU_HDA_AMP_IN_STEPS << AC_AMPCAP_NUM_STEPS_SHIFT) | \
> + (QEMU_HDA_AMP_IN_STEP_SIZE << AC_AMPCAP_STEP_SIZE_SHIFT))
> #else
> #define QEMU_HDA_ID_OUTPUT ((QEMU_HDA_ID_VENDOR << 16) | 0x11)
> #define QEMU_HDA_ID_DUPLEX ((QEMU_HDA_ID_VENDOR << 16) | 0x21)
> #define QEMU_HDA_ID_MICRO ((QEMU_HDA_ID_VENDOR << 16) | 0x31)
> -#define QEMU_HDA_AMP_CAPS QEMU_HDA_AMP_NONE
> +#define QEMU_HDA_AMP_OUT_CAPS QEMU_HDA_AMP_NONE
> +#define QEMU_HDA_AMP_IN_CAPS QEMU_HDA_AMP_NONE
> #endif
>
>
> @@ -61,7 +67,7 @@ static const desc_param glue(common_params_audio_dac_,
> PARAM)[] = {
> .val = QEMU_HDA_AMP_NONE,
> },{
> .id = AC_PAR_AMP_OUT_CAP,
> - .val = QEMU_HDA_AMP_CAPS,
> + .val = QEMU_HDA_AMP_OUT_CAPS,
> },
> };
>
> @@ -86,7 +92,7 @@ static const desc_param glue(common_params_audio_adc_,
> PARAM)[] = {
> .val = AC_SUPFMT_PCM,
> },{
> .id = AC_PAR_AMP_IN_CAP,
> - .val = QEMU_HDA_AMP_CAPS,
> + .val = QEMU_HDA_AMP_IN_CAPS,
> },{
> .id = AC_PAR_AMP_OUT_CAP,
> .val = QEMU_HDA_AMP_NONE,
I am afraid guest OS won't like it the card description changes after a
migration.
This will likely need a new VMState field, for the steps, or the amp caps.
> @@ -453,4 +459,5 @@ static const desc_codec glue(micro_, PARAM) = {
> #undef QEMU_HDA_ID_OUTPUT
> #undef QEMU_HDA_ID_DUPLEX
> #undef QEMU_HDA_ID_MICRO
> -#undef QEMU_HDA_AMP_CAPS
> +#undef QEMU_HDA_AMP_IN_CAPS
> +#undef QEMU_HDA_AMP_OUT_CAPS
> diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
> index 3c03ff5..64b4d98 100644
> --- a/hw/audio/hda-codec.c
> +++ b/hw/audio/hda-codec.c
> @@ -116,7 +116,17 @@ static void hda_codec_parse_fmt(uint32_t format, struct
> audsettings *as)
> #define QEMU_HDA_PCM_FORMATS (AC_SUPPCM_BITS_16 | \
> 0x1fc /* 16 -> 96 kHz */)
> #define QEMU_HDA_AMP_NONE (0)
> -#define QEMU_HDA_AMP_STEPS 0x4a
> +/* Amplifier properties */
> +/* Step size: 0: 0.25dB 1: 0.5dB 2: 0.75dB 3: 1dB */
> +/*
> + Until we have a way to propagate dB values cleanly to the host
> + e.g. through spice, it is better to use a smaller dynamic range
> + for input. Here we use 0x40 * 0.5 dB = 32dB.
> +*/
> +#define QEMU_HDA_AMP_OUT_STEPS 0x4a
> +#define QEMU_HDA_AMP_OUT_STEP_SIZE 3
> +#define QEMU_HDA_AMP_IN_STEPS 0x40
> +#define QEMU_HDA_AMP_IN_STEP_SIZE 1
>
> #define PARAM mixemu
> #define HDA_MIXER
> @@ -258,15 +268,16 @@ static void hda_audio_set_amp(HDAAudioStream *st)
> left = st->mute_left ? 0 : st->gain_left;
> right = st->mute_right ? 0 : st->gain_right;
>
> - left = left * 255 / QEMU_HDA_AMP_STEPS;
> - right = right * 255 / QEMU_HDA_AMP_STEPS;
> -
> if (!st->state->mixer) {
> return;
> }
> if (st->output) {
> + left = left * 255 / QEMU_HDA_AMP_OUT_STEPS;
> + right = right * 255 / QEMU_HDA_AMP_OUT_STEPS;
> AUD_set_volume_out(st->voice.out, muted, left, right);
> } else {
> + left = left * 255 / QEMU_HDA_AMP_IN_STEPS;
> + right = right * 255 / QEMU_HDA_AMP_IN_STEPS;
> AUD_set_volume_in(st->voice.in, muted, left, right);
> }
> }
> @@ -502,8 +513,8 @@ static int hda_audio_init(HDACodecDevice *hda, const
> struct desc_codec *desc)
> st->node = node;
> if (type == AC_WID_AUD_OUT) {
> /* unmute output by default */
> - st->gain_left = QEMU_HDA_AMP_STEPS;
> - st->gain_right = QEMU_HDA_AMP_STEPS;
> + st->gain_left = QEMU_HDA_AMP_OUT_STEPS;
> + st->gain_right = QEMU_HDA_AMP_OUT_STEPS;
> st->bpos = sizeof(st->buf);
> st->output = true;
> } else {
> --
> 2.1.0
>
>