[4321] First cut at WM8750 volume control (Jan Kiszka).

Qemu-devel New Andrzej Zaborowski 2 participants 9 messages May 4, 2008 – May 5, 2008

Share & tools

Andrzej Zaborowski

author

2008-05-04 10:21 UTC #0 lore raw

Revision: 4321

          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=4321
Author:   balrog
Date:     2008-05-04 10:21:03 +0000 (Sun, 04 May 2008)

Log Message:
-----------
First cut at WM8750 volume control (Jan Kiszka).

Modified Paths:
--------------

    trunk/audio/audio.c
    trunk/audio/audio.h
    trunk/hw/wm8750.c

Modified: trunk/audio/audio.c
===================================================================

audio/audio.c +20 -2

@@ -117,8 +117,8 @@
     1.0,
     1.0
 #else
-    UINT_MAX,
-    UINT_MAX
+    1ULL << 32,
+    1ULL << 32
 #endif
 };

@@ -1955,3 +1955,21 @@
         }
     }
 }
+
+void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol)
+{
+    if (sw) {
+        sw->vol.mute = mute;
+        sw->vol.l = nominal_volume.l * lvol / 255;
+        sw->vol.r = nominal_volume.r * rvol / 255;
+    }
+}
+
+void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol)
+{
+    if (sw) {
+        sw->vol.mute = mute;
+        sw->vol.l = nominal_volume.l * lvol / 255;
+        sw->vol.r = nominal_volume.r * rvol / 255;
+    }
+}

Modified: trunk/audio/audio.h
===================================================================

audio/audio.h +3 -0

@@ -124,6 +124,9 @@
 void     AUD_init_time_stamp_out (SWVoiceOut *sw, QEMUAudioTimeStamp *ts);
 uint64_t AUD_get_elapsed_usec_out (SWVoiceOut *sw, QEMUAudioTimeStamp *ts);
 
+void AUD_set_volume_out (SWVoiceOut *sw, int mute, uint8_t lvol, uint8_t rvol);
+void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol);
+
 SWVoiceIn *AUD_open_in (
     QEMUSoundCard *card,
     SWVoiceIn *sw,

Modified: trunk/hw/wm8750.c
===================================================================

hw/wm8750.c +47 ~~-30~~

165 lines not on this page · raw message

Jan Kiszka

2008-05-04 10:47 UTC #1 lore raw

Quoted 4 lines · Andrzej Zaborowski

Modified: trunk/hw/wm8750.c
===================================================================
hw/wm8750.c +0 -0

...

Quoted 10 lines

hw/wm8750.c +6 -0

@@ -125,6 +133,32 @@
     {  192, 88200,  192, 88200 },	/* SR: 11111 */
 };
 
+static void wm8750_vol_update(struct wm8750_s *s)
+{
+    /* FIXME: multiply all volumes by s->invol[2], s->invol[3] */
+
+    AUD_set_volume_in(*s->in[0], s->mute,
+                    s->inmute[0] ? 0 : 0xff,

hw/wm8750.c line 151

Quoted 10 lines

hw/wm8750.c +10 -0

+                    s->inmute[1] ? 0 : 0xff);
+
+    /* FIXME: multiply all volumes by s->outvol[0], s->outvol[1] */
+
+    /* Speaker: LOUT2VOL ROUT2VOL */
+    AUD_set_volume_out(s->dac_voice[0], s->mute,
+                    s->outmute[0] ? 0 : WM8750_VOL_TRANSFORM(s->outvol[4]),
+                    s->outmute[1] ? 0 : WM8750_VOL_TRANSFORM(s->outvol[5]));
+
+    /* Headphone: LOUT2VOL ROUT2VOL */

LOUT1VOL / ROUT1VOL

Quoted 13 lines

hw/wm8750.c +10 -0

+    AUD_set_volume_out(s->dac_voice[1], s->mute,
+                    s->outmute[0] ? 0 : WM8750_VOL_TRANSFORM(s->outvol[2]),
+                    s->outmute[1] ? 0 : WM8750_VOL_TRANSFORM(s->outvol[3]));
+
+    /* MONOOUT: MONOVOL MONOVOL */
+    AUD_set_volume_out(s->dac_voice[2], s->mute,
+                    s->outmute[0] ? 0 : WM8750_VOL_TRANSFORM(s->outvol[6]),
+                    s->outmute[1] ? 0 : WM8750_VOL_TRANSFORM(s->outvol[6]));
+}
+
 static void wm8750_set_format(struct wm8750_s *s)
 {
     int i;

...

hw/wm8750.c line 460

Quoted 6 lines

hw/wm8750.c +1 -1

@@ -437,19 +457,21 @@
         break;
 
     case WM8750_LOUT1V:	/* LOUT1 Volume */
-        s->outvol[2] = value & 0x7f;		/* LOUT2VOL */
+        s->outvol[2] = value & 0x7f;		/* LOUT1VOL */

You probably want to add wm8750_vol_update here (and to the other channels that are now included in volume control).

Great! This just leaves up with

audio/mixeng.c +1 -1

Index: qemu/audio/mixeng.c
===================================================================
@@ -28,7 +28,7 @@
 #define AUDIO_CAP "mixeng"
 #include "audio_int.h"
 
-#define NOVOL
+//#define NOVOL
 
 /* 8 bit */
 #define ENDIAN_CONVERSION natural

to make the MusicPal work out-of-the-box (muting is now broken again, causing loud noise during channel switches and while in suspended mode).

Could you comment on the remaining issuing of non-NOVOL and a roadmap to finally overcome that switch?

Jan

andrzej zaborowski

author

2008-05-04 12:31 UTC #2 lore raw

Quoted 8 lines · Jan Kiszka

-#define NOVOL
+//#define NOVOL
  /* 8 bit */
  #define ENDIAN_CONVERSION natural
to make the MusicPal work out-of-the-box (muting is now broken again, causing loud noise during channel switches and while in suspended mode).

Right, as I said you need to disable NOVOL manually if you want volume control. Changing this would affect users of all machines of all architectures and until now everyone was fine with using the host mixer instead of software mixing in qemu.

BTW, I noticed that on some radio stations I get double speed playback, not sure what's causing this. My recent commit, while correct (I think), doesn't fix this. For a test case try connecting to:

Internet Radio -> Countries -> Europe -> Spain -> All Stations -> Caroline International,

the firmware sets both the WM8750 sample rate and the CPUs audio iface to 24 KHz but correct value is 12KHz or so. (I by no means recommend that station for listening).

Also BTW, do you want to add a qemu-doc.texi piece describing -M musicpal? Otherwise I'll add something.

Regards

Signature · 2 lines

Please do not print this email unless absolutely necessary. Spread
environmental awareness.

andrzej zaborowski

author

2008-05-04 15:49 UTC #3 lore raw

Quoted 11 lines · andrzej zaborowski

Quoted 8 lines · Jan Kiszka
-#define NOVOL
+//#define NOVOL
  /* 8 bit */
  #define ENDIAN_CONVERSION natural
to make the MusicPal work out-of-the-box (muting is now broken again, causing loud noise during channel switches and while in suspended mode).

Quoted 8 lines

Right, as I said you need to disable NOVOL manually if you want volume
control. Changing this would affect users of all machines of all architectures and until now everyone was fine with using the host mixer instead of software mixing in qemu.
BTW, I noticed that on some radio stations I get double speed playback, not sure what's causing this. My recent commit, while correct (I think), doesn't fix this. For a test case try connecting

Ok, seems to be fixed now - it (logically) happened everytime Linux was trying to play a mono channel stream.

Regards

Signature · 2 lines

Please do not print this email unless absolutely necessary. Spread
environmental awareness.

Jan Kiszka

2008-05-04 17:32 UTC #4 lore raw

Quoted 17 lines · andrzej zaborowski

Quoted 16 lines · andrzej zaborowski
Quoted 8 lines · Jan Kiszka
-#define NOVOL
+//#define NOVOL
  /* 8 bit */
  #define ENDIAN_CONVERSION natural
to make the MusicPal work out-of-the-box (muting is now broken again, causing loud noise during channel switches and while in suspended mode).
Right, as I said you need to disable NOVOL manually if you want volume
control. Changing this would affect users of all machines of all architectures and until now everyone was fine with using the host mixer instead of software mixing in qemu.

Quoted 6 lines

Quoted 3 lines
BTW, I noticed that on some radio stations I get double speed playback, not sure what's causing this. My recent commit, while correct (I think), doesn't fix this. For a test case try connecting
Ok, seems to be fixed now - it (logically) happened everytime Linux was trying to play a mono channel stream.

Cool! Just a minor nit (to avoid that the compiler gets upset):

hw/musicpal.c +9 -7

Index: qemu/hw/musicpal.c
===================================================================
@@ -255,7 +255,8 @@ typedef struct musicpal_audio_state {
 static void audio_callback(void *opaque, int free_out, int free_in)
 {
     musicpal_audio_state *s = opaque;
-    int16_t *codec_buffer, *mem_buffer;
+    int16_t *codec_buffer;
+    void *mem_buffer;
     int pos, block_size;
 
     if (!(s->playback_mode & MP_AUDIO_PLAYBACK_EN))

@@ -276,8 +277,9 @@ static void audio_callback(void *opaque,
         if (s->playback_mode & MP_AUDIO_MONO) {
             codec_buffer = wm8750_dac_buffer(s->wm, block_size >> 1);
             for (pos = 0; pos < block_size; pos += 2) {
-                *codec_buffer++ = *mem_buffer;
-                *codec_buffer++ = *mem_buffer++;
+                *codec_buffer++ = *(uint16_t *)mem_buffer;
+                *codec_buffer++ = *(uint16_t *)mem_buffer;
+                mem_buffer += 2;
             }
         } else
             memcpy(wm8750_dac_buffer(s->wm, block_size >> 2),

@@ -286,14 +288,14 @@ static void audio_callback(void *opaque,
         if (s->playback_mode & MP_AUDIO_MONO) {
             codec_buffer = wm8750_dac_buffer(s->wm, block_size);
             for (pos = 0; pos < block_size; pos++) {
-                *codec_buffer++ = cpu_to_le16(256 * *((int8_t *)mem_buffer));
-                *codec_buffer++ = cpu_to_le16(256 * *((int8_t *)mem_buffer)++);
+                *codec_buffer++ = cpu_to_le16(256 * *(int8_t *)mem_buffer);
+                *codec_buffer++ = cpu_to_le16(256 * *(int8_t *)mem_buffer++);
             }
         } else {
             codec_buffer = wm8750_dac_buffer(s->wm, block_size >> 1);
             for (pos = 0; pos < block_size; pos += 2) {
-                *codec_buffer++ = cpu_to_le16(256 * *((int8_t *)mem_buffer)++);
-                *codec_buffer++ = cpu_to_le16(256 * *((int8_t *)mem_buffer)++);
+                *codec_buffer++ = cpu_to_le16(256 * *(int8_t *)mem_buffer++);
+                *codec_buffer++ = cpu_to_le16(256 * *(int8_t *)mem_buffer++);
             }
         }
     }

Jan Kiszka

2008-05-04 17:31 UTC #5 lore raw

In reply to andrzej zaborowski

Quoted 7 lines · andrzej zaborowski

Quoted 6 lines · Jan Kiszka
-#define NOVOL
+//#define NOVOL
  /* 8 bit */
  #define ENDIAN_CONVERSION natural

Quoted 7 lines

Quoted 2 lines
to make the MusicPal work out-of-the-box (muting is now broken again, causing loud noise during channel switches and while in suspended mode).
Right, as I said you need to disable NOVOL manually if you want volume control. Changing this would affect users of all machines of all architectures and until now everyone was fine with using the host mixer instead of software mixing in qemu.

Well, this is a weak argument. So far there was no alternative available.

I don't let you go with this ;): How should we find out what needs to be fixed - given that hardly anyone is able to test on its own all the supported boards with appropriate sound scenarios? What regressions are you aware of when we switch NOVOL off? That's why I was asking for a
roadmap: If there are known one, then keep NOVOL set for those arch. But we won't make progress if we just stick with "this was OK so far."

Jan

andrzej zaborowski

author

2008-05-04 23:59 UTC #6 lore raw

Quoted 11 lines · Jan Kiszka

Quoted 10 lines · andrzej zaborowski
Quoted 8 lines · Jan Kiszka
-#define NOVOL
+//#define NOVOL
  /* 8 bit */
  #define ENDIAN_CONVERSION natural
to make the MusicPal work out-of-the-box (muting is now broken again, causing loud noise during channel switches and while in suspended mode).

Quoted 7 lines

Quoted 4 lines
Right, as I said you need to disable NOVOL manually if you want volume control. Changing this would affect users of all machines of all architectures and until now everyone was fine with using the host mixer instead of software mixing in qemu.
Well, this is a weak argument. So far there was no alternative available.

I'm sure there would be an alternative if anybody wanted it.

Quoted 5 lines

I don't let you go with this ;): How should we find out what needs to be fixed - given that hardly anyone is able to test on its own all the supported boards with appropriate sound scenarios? What regressions are you aware of when we switch NOVOL off?

The regression is the overhead of two 32bit multiplications per sample per channel even if the virtual sound card you use has no volume control. For the wm8750 that's not easily noticeable but I'm personally not making it a default and not globally. (But I won't oppose if there's a wider agreement).

Regards

Jan Kiszka

2008-05-05 18:44 UTC #7 lore raw

PATCH 1/1 Enable volume control globally (was: First cut at WM8750 volume control)

Quoted 27 lines · andrzej zaborowski

Quoted 11 lines · Jan Kiszka
Quoted 10 lines · andrzej zaborowski
Quoted 8 lines · Jan Kiszka
-#define NOVOL
+//#define NOVOL
  /* 8 bit */
  #define ENDIAN_CONVERSION natural
to make the MusicPal work out-of-the-box (muting is now broken again, causing loud noise during channel switches and while in suspended mode).
Quoted 7 lines
Quoted 4 lines
Right, as I said you need to disable NOVOL manually if you want volume control. Changing this would affect users of all machines of all architectures and until now everyone was fine with using the host mixer instead of software mixing in qemu.
Well, this is a weak argument. So far there was no alternative available.
I'm sure there would be an alternative if anybody wanted it.
Quoted 4 lines
I don't let you go with this ;): How should we find out what needs to be fixed - given that hardly anyone is able to test on its own all the supported boards with appropriate sound scenarios? What regressions are you aware of when we switch NOVOL off?

Quoted 5 lines

The regression is the overhead of two 32bit multiplications per sample per channel even if the virtual sound card you use has no volume control. For the wm8750 that's not easily noticeable but I'm personally not making it a default and not globally. (But I won't oppose if there's a wider agreement).

Well, let's start like below, and if someone complains, think about a step-wise move towards out-of-the-box mixer support in QEMU.

----------

As we now have the possibility to set per-channel volume levels, thus to emulated sound hardware more realistically, we just need to enable the application of this values by disabling 'NOVOL'. Users can still disable this support if desired or required, but it should be the aim now to fix potential remaining issues of the soft-mixer.

This patch also unbreaks the MusicPal emulation, which depends on the mute control so that users have to tweak the source code right now.

Signed-off-by Jan Kiszka <[email protected]>

---
 audio/mixeng.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

audio/mixeng.c +1 -1

Index: b/audio/mixeng.c
===================================================================
@@ -28,7 +28,7 @@
 #define AUDIO_CAP "mixeng"
 #include "audio_int.h"
 
-#define NOVOL
+//#define NOVOL
 
 /* 8 bit */
 #define ENDIAN_CONVERSION natural

Jan Kiszka

2008-05-04 21:33 UTC #8 lore raw

PATCH 1/1 musicpal: doc fragment

In reply to andrzej zaborowski

Quoted 3 lines · andrzej zaborowski

...
Also BTW, do you want to add a qemu-doc.texi piece describing -M musicpal? Otherwise I'll add something.

Something like this?

qemu-doc.texi +20 -0

Index: qemu/qemu-doc.texi
===================================================================
@@ -87,6 +87,7 @@ For system emulation, the following hard
 @item Freescale MCF5208EVB (ColdFire V2).
 @item Arnewsh MCF5206 evaluation board (ColdFire V2).
 @item Palm Tungsten|E PDA (OMAP310 processor)
+@item MusicPal (MV88W8618 ARM processor)
 @end itemize
 
 For user emulation, x86, PowerPC, ARM, 32-bit MIPS, Sparc32/64 and ColdFire(m68k) CPUs are supported.

@@ -2466,6 +2467,25 @@ Timers, UARTs, ADC, I@math{^2}C and SSI 
 OSRAM Pictiva 128x64 OLED with SSD0323 controller connected via SSI.
 @end itemize
 
+The Freecom MusicPal emulation includes the following elements:
+
+@itemize @minus
+@item
+Marvel MV88W8618 ARM core.
+@item
+32 MB RAM, 256 KB SRAM, 8 MB flash.
+@item
+up to 2 16550 UARTs
+@item
+MV88W8xx8 Ethernet controller
+@item
+MV88W8618 audio controller, WM8750 CODEC and mixer
+@item
+128×64 display with brightness control
+@item
+2 buttons, 2 navigation wheels with button function
+@end itemize
+
 A Linux 2.6 test image is available on the QEMU web site. More
 information is available in the QEMU mailing-list archive.