Add support for Marvell 88w8618 / MusicPal
RFC PATCH 4/4 Add support for Marvell 88w8618 / MusicPal
This is the board emulation for Freecom's MusicPal, featuring
- rudimentary PIT and PIC
- up to 2 UARTs
- 88W8xx8 Ethernet controller
- 88W8618 audio controller
- Wolfson WM8750 mixer chip (volume control and mute only)
- 128×64 display with brightness control
- all input buttons
Using up to 32 MB flash, I hit a limit /wrt phys_ram_size. I worked around this for now by extending MAX_BIOS_SIZE to 32 MB, surely not a nice solution.
The emulation suffers a bit from limited time, specifically to implement details of hardware setup/shutdown. Another problem - not only for the emulation, but also for native Linux support - is that Marvell yet hides their specs from the community, and existing Linux code [1] is not that clear in every required detail. In case the specs remain Marvell secret, maybe the emulation can help a bit to reverse-engineer the remaining bits (I'm thinking of the binary-blobbed WLAN driver e.g.).
[1] http://www.musicpal.info/gpl.htm
--- Makefile.target | 1 hw/boards.h | 3 hw/musicpal.c | 1266 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ sysemu.h | 2 vl.c | 1 5 files changed, 1272 insertions(+), 1 deletion(-)
Makefile.target +1 -0
@@ -612,6 +612,7 @@ OBJS+= spitz.o ide.o serial.o nand.o ecc OBJS+= omap1.o omap_lcdc.o omap_dma.o omap_clk.o omap_mmc.o omap_i2c.o OBJS+= palm.o tsc210x.o OBJS+= mst_fpga.o mainstone.o +OBJS+= musicpal.o pflash_cfi02.o CPPFLAGS += -DHAS_AUDIO endif ifeq ($(TARGET_BASE_ARCH), sh4)
hw/boards.h +3 -0
@@ -101,4 +101,7 @@ extern QEMUMachine dummy_m68k_machine;/* mainstone.c */ extern QEMUMachine mainstone2_machine; +/* musicpal.c */ +extern QEMUMachine musicpal_machine; +#endif
vl.c +1 -0
@@ -8056,6 +8056,7 @@ static void register_machines(void)qemu_register_machine (&connex_machine);qemu_register_machine (&verdex_machine);qemu_register_machine (&mainstone2_machine); +qemu_register_machine (&musicpal_machine);#elif defined(TARGET_SH4 )qemu_register_machine (&shix_machine);qemu_register_machine (&r2d_machine);
sysemu.h +1 -1
@@ -109,7 +109,7 @@ extern unsigned int nb_prom_envs;#endif /* XXX: make it dynamic */ -#define MAX_BIOS_SIZE (4 * 1024 * 1024) +#define MAX_BIOS_SIZE (32 * 1024 * 1024)#if defined (TARGET_PPC )#define BIOS_SIZE (1024 * 1024)#elif defined (TARGET_SPARC64 )
hw/musicpal.c new file +1266 -0
1,266 lines not on this page · raw message
Quoted 9 lines · Jan Kiszka
This is the board emulation for Freecom's MusicPal, featuring
- rudimentary PIT and PIC
- up to 2 UARTs
- 88W8xx8 Ethernet controller
- 88W8618 audio controller
- Wolfson WM8750 mixer chip (volume control and mute only)
- 128?64 display with brightness control
- all input buttons
[..snip..]
hw/musicpal.c line 86
Quoted 7 lines
hw/musicpal.c new file +7
-0+static void sound_fill_mixer_buffer (mv88w8618_sound_state *s,unsigned int length) +{ +unsigned int pos; +double val; + +if (s->mute) { +memset (s->mixer_buffer,0 , length);
Zero is not correct "mute" value for signed formats.
hw/musicpal.c line 95
Quoted 9 lines
hw/musicpal.c new file +9
-0+return ; + } + +if (s->playback_mode &1 ) +for (pos =0 ; pos < length; pos +=2 ) { + val = *(int16_t *)(s->target_buffer + s->play_pos + pos); + val = val *pow (10.0 , s->volume/20.0 ); + *(int16_t *)(s->mixer_buffer + pos) = val; + }
On the surface it's sounds like endianess problems are ahead.
hw/musicpal.c line 102
Quoted 7 lines
hw/musicpal.c new file +7
-0+else +for (pos =0 ; pos < length; pos +=1 ) { + val = *(int8_t *)(s->target_buffer + s->play_pos + pos); + val = val *pow (10.0 , s->volume/20.0 ); + *(int8_t *)(s->mixer_buffer + pos) = val; + } +}
There's actually a generic framework for volume manipulation in QEMUs audio, unfortunatelly it's unconditionally disabled (define NOVOL in audio/mixeng.c) for the reasons i can't recall now.
Also adlib emulation is only conditionally available due to usage of floating point arithmetics, maybe rules have changed now though, but "fixing" this particular issue in this particular case seems easy enough.
hw/musicpal.c line 113
Quoted 10 lines
hw/musicpal.c new file +10
-0+static void sound_callback (void *opaque,int free) +{ +mv88w8618_sound_state *s = opaque; +uint32_t old_status = s->status; +int64_t now =qemu_get_clock (rt_clock); +unsigned int n; + +if (now - s->last_callback < (1000 * s->threshold/2 ) / (44100 *2 )) +return ; + s->last_callback = now;
Here two clocks are being used for things - the audio one which influences `free' paramter and rt_clock, even if there are reasons why free could not be used choice of rt_clock over vm_clock seems wrong.
Quoted 10 lines
hw/musicpal.c new file +10
-0+while (free >0 ) { + n =audio_MIN (s->threshold - s->play_pos, (unsigned int )free); +sound_fill_mixer_buffer (s, n); + n =AUD_write (s->voice, s->mixer_buffer, n); +if (!n) +break ; + + s->play_pos += n; + free -= n; +
hw/musicpal.c line 134
Quoted 10 lines
hw/musicpal.c new file +10
-0+if (s->play_pos >= s->threshold/2 ) + s->status |=1 <<6 ; +if (s->play_pos == s->threshold) { + s->status |=1 <<7 ; + s->play_pos =0 ; +break ; + } + } +if ((s->status ^ old_status) & s->irq_enable) +qemu_irq_raise (s->irq);
While might be perfectly fine the above checking when to fire IRQ looks suspicious.
[..snip..]
hw/musicpal.c line 178
Quoted 11 lines
hw/musicpal.c new file +11
-0+static void mv88w8618_sound_write (void *opaque,target_phys_addr_t offset, +uint32_t value) +{ +mv88w8618_sound_state *s = opaque; + + offset -= s->base; +switch (offset) { +case 0x00 :/* Playback Mode */ + s->playback_mode = value; +if (value & (1 <<7 )) { +audsettings_t as = {0 ,2 ,0 ,0 };
Endianess set to 0 while the actual volume manipulations imply host byte order..
Quoted 38 lines
hw/musicpal.c new file +37
-0+if (value & (1 <<9 )) + as.freq = (24576000 /64 ) / (s->clock_div +1 ); +else + as.freq = (11289600 /64 ) / (s->clock_div +1 ); +if (value &1 ) + as.fmt =AUD_FMT_S16 ; +else + as.fmt =AUD_FMT_S8 ; + + s->voice =AUD_open_out (&s->card, s->voice, sound_name, + s, sound_callback, &as); +if (s->voice) +AUD_set_active_out (s->voice,1 ); +else +AUD_log (sound_name,"Could not open voice\n" ); + s->last_callback =0 ; + s->status =0 ; + }else if (s->voice) +AUD_set_active_out (s->voice,0 ); +break ; + +case 0x18 :/* Clock Divider */ + s->clock_div = (value >>8 ) &0xFF ; +break ; + +case 0x20 :/* Interrupt Status */ + s->status &= ~value; +break ; + +case 0x24 :/* Interrupt Enable */ + s->irq_enable = value; +if (s->status & s->irq_enable) +qemu_irq_raise (s->irq); +break ; + +case 0x28 :/* Tx Start Low */ + s->phys_buf = (s->phys_buf &0xFFFF0000 ) | (value &0xFFFF );
[..snip..]
Signature · 1 line
mailto:[email protected]
Quoted 13 lines · malc
Quoted 9 lines · Jan Kiszka
This is the board emulation for Freecom's MusicPal, featuring
- rudimentary PIT and PIC
- up to 2 UARTs
- 88W8xx8 Ethernet controller
- 88W8618 audio controller
- Wolfson WM8750 mixer chip (volume control and mute only)
- 128?64 display with brightness control
- all input buttons[..snip..]
Quoted 10 lines
hw/musicpal.c line 86
Quoted 8 lines
+static void
sound_fill_mixer_buffer(mv88w8618_sound_state *s, unsigned int length)hw/musicpal.c new file +6
-0+{ +unsigned int pos; +double val; + +if (s->mute) { +memset (s->mixer_buffer,0 , length);Zero is not correct "mute" value for signed formats.
Hmm, maybe it's still too early for me - but what else??
Quoted 11 lines
hw/musicpal.c line 95
Quoted 10 lines
hw/musicpal.c new file +9
-0+return ; + } + +if (s->playback_mode &1 ) +for (pos =0 ; pos < length; pos +=2 ) { + val = *(int16_t *)(s->target_buffer + s->play_pos + pos); + val = val *pow (10.0 , s->volume/20.0 ); + *(int16_t *)(s->mixer_buffer + pos) = val; + }
Quoted 1 line
On the surface it's sounds like endianess problems are ahead.
Oh, yes.
Quoted 9 lines
hw/musicpal.c line 102
Quoted 8 lines
hw/musicpal.c new file +7
-0+else +for (pos =0 ; pos < length; pos +=1 ) { + val = *(int8_t *)(s->target_buffer + s->play_pos + pos); + val = val *pow (10.0 , s->volume/20.0 ); + *(int8_t *)(s->mixer_buffer + pos) = val; + } +}
Quoted 3 lines
There's actually a generic framework for volume manipulation in QEMUs audio, unfortunatelly it's unconditionally disabled (define NOVOL in
audio/mixeng.c) for the reasons i can't recall now.
Yeah, looked around a bit before hacking those loops but indeed found nothing ready to use.
Quoted 5 lines
Also adlib emulation is only conditionally available due to usage of floating point arithmetics, maybe rules have changed now though, but "fixing" this particular issue in this particular case seems easy enough.
Sorry, didn't get what you mean here.
Quoted 15 lines
hw/musicpal.c line 113
Quoted 11 lines
hw/musicpal.c new file +10
-0+static void sound_callback (void *opaque,int free) +{ +mv88w8618_sound_state *s = opaque; +uint32_t old_status = s->status; +int64_t now =qemu_get_clock (rt_clock); +unsigned int n; + +if (now - s->last_callback < (1000 * s->threshold/2 ) / (44100 *2 )) +return ; + s->last_callback = now;Here two clocks are being used for things - the audio one which influences `free' paramter and
rt_clock, even if there are reasons why free could not be used choice ofrt_clockovervm_clockseems wrong.
Yes, using the monotonic vm_clock is better. Will change.
Starring at the condition above, I realized that it is also bogus, in several ways. It should still cause far more interrupts than on the real device...
Quoted 16 lines
Quoted 16 lines
hw/musicpal.c new file +15
-0+while (free >0 ) { + n =audio_MIN (s->threshold - s->play_pos, (unsigned int )free); +sound_fill_mixer_buffer (s, n); + n =AUD_write (s->voice, s->mixer_buffer, n); +if (!n) +break ; + + s->play_pos += n; + free -= n; + +if (s->play_pos >= s->threshold/2 ) + s->status |=1 <<6 ; +if (s->play_pos == s->threshold) { + s->status |=1 <<7 ; + s->play_pos =0 ;
Quoted 8 lines
hw/musicpal.c line 134
Quoted 5 lines
hw/musicpal.c new file +5
-0+break ; + } + } +if ((s->status ^ old_status) & s->irq_enable) +qemu_irq_raise (s->irq);While might be perfectly fine the above checking when to fire IRQ looks suspicious.
Both status and irq_enable use the same bit semantics, so playing with bit operations is indeed OK here.
Quoted 18 lines
[..snip..]
hw/musicpal.c line 178
Quoted 12 lines
+static void mv88w8618_sound_write(void *opaque,
target_phys_addr_toffset,hw/musicpal.c new file +10
-0+ uint32_t value) +{ +mv88w8618_sound_state *s = opaque; + + offset -= s->base; +switch (offset) { +case 0x00 :/* Playback Mode */ + s->playback_mode = value; +if (value & (1 <<7 )) { +audsettings_t as = {0 ,2 ,0 ,0 };Endianess set to 0 while the actual volume manipulations imply host byte order..
Yes, will "officially" switch to host order.
Quoted 35 lines
Quoted 34 lines
hw/musicpal.c new file +33
-0+if (value & (1 <<9 )) + as.freq = (24576000 /64 ) / (s->clock_div +1 ); +else + as.freq = (11289600 /64 ) / (s->clock_div +1 ); +if (value &1 ) + as.fmt =AUD_FMT_S16 ; +else + as.fmt =AUD_FMT_S8 ; + + s->voice =AUD_open_out (&s->card, s->voice, sound_name, + s, sound_callback, &as); +if (s->voice) +AUD_set_active_out (s->voice,1 ); +else +AUD_log (sound_name,"Could not open voice\n" ); + s->last_callback =0 ; + s->status =0 ; + }else if (s->voice) +AUD_set_active_out (s->voice,0 ); +break ; + +case 0x18 :/* Clock Divider */ + s->clock_div = (value >>8 ) &0xFF ; +break ; + +case 0x20 :/* Interrupt Status */ + s->status &= ~value; +break ; + +case 0x24 :/* Interrupt Enable */ + s->irq_enable = value; +if (s->status & s->irq_enable) +qemu_irq_raise (s->irq);
Quoted 7 lines
Quoted 4 lines
hw/musicpal.c new file +4
-0+break ; + +case 0x28 :/* Tx Start Low */ + s->phys_buf = (s->phys_buf &0xFFFF0000 ) | (value &0xFFFF );[..snip..]
Thanks a lot for your review!
Jan
Quoted 4 lines · Jan Kiszka
Quoted 1 line · malc
Zero is not correct "mute" value for signed formats.
Hmm, maybe it's still too early for me - but what else??
Well, zero is not correct for unsigned formats, but I cannot see any such formats in use in your emulation...
BTW, there are quite a few magic constants that it would probably be better to #define.
Cheers,
Signature · 1 line
Stuart Brady
Quoted 7 lines · Stuart Brady
Quoted 3 lines · Jan Kiszka
Quoted 1 line · malc
Zero is not correct "mute" value for signed formats.
Hmm, maybe it's still too early for me - but what else??
Well, zero is not correct for unsigned formats, but I cannot see any such formats in use in your emulation...
/me too.
Quoted 2 lines
BTW, there are quite a few magic constants that it would probably be better to #define.
I know. Comes a bit from the initial approach: "Hey, let's see how far this hack will get."
Will try to fix those spots where possible.
Jan
Quoted 2 lines · Jan Kiszka
Quoted 1 line · malc
On Sun, 13 Apr 2008, Jan Kiszka wrote:
[..snip..]
Quoted 11 lines
Quoted 10 lines
hw/musicpal.c line 86
Quoted 8 lines
+static void
sound_fill_mixer_buffer(mv88w8618_sound_state *s, unsigned int length)hw/musicpal.c new file +6
-0+{ +unsigned int pos; +double val; + +if (s->mute) { +memset (s->mixer_buffer,0 , length);Zero is not correct "mute" value for signed formats.
Quoted 1 line
Hmm, maybe it's still too early for me - but what else??
Well.. 0x80 for S8 and 0x8000 for S16 (audio_pcm_info_clear_buf in audio.c)
Quoted 24 lines
Quoted 11 lines
hw/musicpal.c line 95
Quoted 10 lines
hw/musicpal.c new file +9
-0+return ; + } + +if (s->playback_mode &1 ) +for (pos =0 ; pos < length; pos +=2 ) { + val = *(int16_t *)(s->target_buffer + s->play_pos + pos); + val = val *pow (10.0 , s->volume/20.0 ); + *(int16_t *)(s->mixer_buffer + pos) = val; + }Quoted 1 line
On the surface it's sounds like endianess problems are ahead.
Oh, yes.
Quoted 9 lines
Quoted 8 lines
hw/musicpal.c new file +7
-0+else +for (pos =0 ; pos < length; pos +=1 ) { + val = *(int8_t *)(s->target_buffer + s->play_pos + pos); + val = val *pow (10.0 , s->volume/20.0 ); + *(int8_t *)(s->mixer_buffer + pos) = val; + } +}
Quoted 6 lines
Quoted 3 lines
There's actually a generic framework for volume manipulation in QEMUs audio, unfortunatelly it's unconditionally disabled (define NOVOL in
audio/mixeng.c) for the reasons i can't recall now.Yeah, looked around a bit before hacking those loops but indeed found nothing ready to use.
It's just a question of adding(and using) AUD_set_volume[_in/out], which will set vol field of respective soft voice and removing uncoditional #define NOVOL from mixeng.c.
Quoted 7 lines
Quoted 5 lines
Also adlib emulation is only conditionally available due to usage of floating point arithmetics, maybe rules have changed now though, but "fixing" this particular issue in this particular case seems easy enough.
Sorry, didn't get what you mean here.
Adlib is not built by default, requires explicit --enable-adlib switch to configure, because fmopl.c uses FPU and Fabrice didn't like the idea. Fixing it here is just a matter of switching to fixed point.
Then again if volume manipulation in main audio proper are brought up to date this all can be scrapped and it will also "magically" solve the endianness issue.
Quoted 16 lines
Quoted 15 lines
hw/musicpal.c line 113
Quoted 11 lines
hw/musicpal.c new file +10
-0+static void sound_callback (void *opaque,int free) +{ +mv88w8618_sound_state *s = opaque; +uint32_t old_status = s->status; +int64_t now =qemu_get_clock (rt_clock); +unsigned int n; + +if (now - s->last_callback < (1000 * s->threshold/2 ) / (44100 *2 )) +return ; + s->last_callback = now;Here two clocks are being used for things - the audio one which influences `free' paramter and
rt_clock, even if there are reasons why free could not be used choice ofrt_clockovervm_clockseems wrong.
Quoted 1 line
Yes, using the monotonic
vm_clockis better. Will change.
Why is it needed at all? `free' already supplies you with an audio clock source.
[..snip..]
Quoted 2 lines
Thanks a lot for your review!
You are welcome.
Signature · 1 line
mailto:[email protected]
Quoted 18 lines · malc
Quoted 2 lines · Jan Kiszka
Quoted 1 line · malc
On Sun, 13 Apr 2008, Jan Kiszka wrote:
[..snip..]
Quoted 11 lines
Quoted 10 lines
hw/musicpal.c line 86
Quoted 8 lines
+static void
sound_fill_mixer_buffer(mv88w8618_sound_state *s, unsigned int length)hw/musicpal.c new file +6
-0+{ +unsigned int pos; +double val; + +if (s->mute) { +memset (s->mixer_buffer,0 , length);Zero is not correct "mute" value for signed formats.
Quoted 4 lines
Quoted 1 line
Hmm, maybe it's still too early for me - but what else??
Well..
0x80for S8 and0x8000for S16 (audio_pcm_info_clear_bufinaudio.c)
void audio_pcm_info_clear_buf (struct audio_pcm_info *info, ... {
...
if (info->sign) {
memset (buf, 0x00, len << info->shift);
}Quoted 28 lines
Quoted 28 lines
Quoted 11 lines
hw/musicpal.c line 95
Quoted 10 lines
hw/musicpal.c new file +9
-0+return ; + } + +if (s->playback_mode &1 ) +for (pos =0 ; pos < length; pos +=2 ) { + val = *(int16_t *)(s->target_buffer + s->play_pos + pos); + val = val *pow (10.0 , s->volume/20.0 ); + *(int16_t *)(s->mixer_buffer + pos) = val; + }Quoted 1 line
On the surface it's sounds like endianess problems are ahead.
Oh, yes.
Quoted 12 lines
hw/musicpal.c line 102
Quoted 8 lines
hw/musicpal.c new file +7
-0+else +for (pos =0 ; pos < length; pos +=1 ) { + val = *(int8_t *)(s->target_buffer + s->play_pos + pos); + val = val *pow (10.0 , s->volume/20.0 ); + *(int8_t *)(s->mixer_buffer + pos) = val; + } +}There's actually a generic framework for volume manipulation in QEMUs audio, unfortunatelly it's unconditionally disabled (define NOVOL in
audio/mixeng.c) for the reasons i can't recall now.
Quoted 6 lines
Quoted 2 lines
Yeah, looked around a bit before hacking those loops but indeed found nothing ready to use.
It's just a question of adding(and using) AUD_set_volume[_in/out], which will set vol field of respective soft voice and removing uncoditional #define NOVOL from
mixeng.c.
Well, I was (and still am) deterred by the plain fact that there are some dead AUD_set_volume spots in ac97.c. If it is that simple, why do we still lack an implementation? My feeling is that you are far deeper into this than I am at this point. So maybe you would like me to test some AUD_set_volume-providing patch of yours? :->
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
Also adlib emulation is only conditionally available due to usage of floating point arithmetics, maybe rules have changed now though, but "fixing" this particular issue in this particular case seems easy enough.
Sorry, didn't get what you mean here.
Quoted 7 lines
Adlib is not built by default, requires explicit --enable-adlib switch to configure, because
fmopl.cuses FPU and Fabrice didn't like the idea. Fixing it here is just a matter of switching to fixed point.Then again if volume manipulation in main audio proper are brought up to date this all can be scrapped and it will also "magically" solve the endianness issue.
I do agree that such stuff should be solved generically, but for now adding a simple le16_to_cpu does not compare to fixing QEMU's soft-volume management for me (given limited time).
Quoted 18 lines
Quoted 18 lines
Quoted 17 lines
hw/musicpal.c line 113
Quoted 11 lines
hw/musicpal.c new file +10
-0+static void sound_callback (void *opaque,int free) +{ +mv88w8618_sound_state *s = opaque; +uint32_t old_status = s->status; +int64_t now =qemu_get_clock (rt_clock); +unsigned int n; + +if (now - s->last_callback < (1000 * s->threshold/2 ) / (44100 *2 )) +return ; + s->last_callback = now;Here two clocks are being used for things - the audio one which influences
`free' paramter andrt_clock, even if there are reasons why free could not
be used choice ofrt_clockovervm_clockseems wrong.
Quoted 4 lines
Quoted 1 line
Yes, using the monotonic
vm_clockis better. Will change.Why is it needed at all? `free' already supplies you with an audio clock source.
The callback mechanism fires far more often than the emulated hardware expects (and can handle), I need throttling.
The sad truth is that my original version was still off my an order of magnitude, causing overload for the guest while decoding obviously more complex 128kbit-WMA streams. Now it runs smoothly. :)
Maybe there is a way to customize the callback rate, I haven't looked that close, but I'm open for suggestions.
Thanks again,
Jan
Quoted 24 lines · Jan Kiszka
Quoted 18 lines · malc
Quoted 2 lines · Jan Kiszka
Quoted 1 line · malc
On Sun, 13 Apr 2008, Jan Kiszka wrote:
[..snip..]
Quoted 11 lines
Quoted 10 lines
hw/musicpal.c line 86
Quoted 8 lines
+static void
sound_fill_mixer_buffer(mv88w8618_sound_state *s, unsigned int length)hw/musicpal.c new file +6
-0+{ +unsigned int pos; +double val; + +if (s->mute) { +memset (s->mixer_buffer,0 , length);Zero is not correct "mute" value for signed formats.
Quoted 4 lines
Quoted 1 line
Hmm, maybe it's still too early for me - but what else??
Well..
0x80for S8 and0x8000for S16 (audio_pcm_info_clear_bufinaudio.c)
Quoted 6 lines
void
audio_pcm_info_clear_buf(structaudio_pcm_info*info, ... {
...if (info->sign) { memset (buf, 0x00, len << info->shift);}
Apparently i should get some sleep..
[..snip..]
Quoted 10 lines
Quoted 4 lines
It's just a question of adding(and using) AUD_set_volume[_in/out], which will set vol field of respective soft voice and removing uncoditional #define NOVOL from
mixeng.c.Well, I was (and still am) deterred by the plain fact that there are some dead
AUD_set_volumespots inac97.c. If it is that simple, why do we still lack an implementation? My feeling is that you are far deeper into this than I am at this point. So maybe you would like me to test some AUD_set_volume-providing patch of yours? :->
Heh. The reason why it's in this state is probably that i has only one (disabled) user, namely ac97, and this one is relative newcomer. But this are indeed that simple, one must provide this AUD_set_volume_out thing that would fill in the vol structure of the soft voice, there are 3 fields to set (left, right and mute) two of them being 32.32 fixed point (if my memory serves me). The above blunder, however, shows that i'm in no shape of doing that, at least not untill i get some sleep.
Quoted 12 lines
Quoted 12 lines
Quoted 7 lines
Quoted 5 lines
Also adlib emulation is only conditionally available due to usage of floating point arithmetics, maybe rules have changed now though, but "fixing" this particular issue in this particular case seems easy enough.
Sorry, didn't get what you mean here.
Adlib is not built by default, requires explicit --enable-adlib switch to configure, because
fmopl.cuses FPU and Fabrice didn't like the idea. Fixing it here is just a matter of switching to fixed point.
Quoted 7 lines
Quoted 3 lines
Then again if volume manipulation in main audio proper are brought up to date this all can be scrapped and it will also "magically" solve the endianness issue.
I do agree that such stuff should be solved generically, but for now adding a simple
le16_to_cpudoes not compare to fixing QEMU's soft-volume management for me (given limited time).
And not forgetting to set aud_settings endiannes to AUDIO_HOST_ENDIANESS
Quoted 18 lines
Quoted 18 lines
Quoted 18 lines
Quoted 17 lines
hw/musicpal.c line 113
Quoted 11 lines
hw/musicpal.c new file +10
-0+static void sound_callback (void *opaque,int free) +{ +mv88w8618_sound_state *s = opaque; +uint32_t old_status = s->status; +int64_t now =qemu_get_clock (rt_clock); +unsigned int n; + +if (now - s->last_callback < (1000 * s->threshold/2 ) / (44100 *2 )) +return ; + s->last_callback = now;Here two clocks are being used for things - the audio one which influences
`free' paramter andrt_clock, even if there are reasons why free could not
be used choice ofrt_clockovervm_clockseems wrong.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Yes, using the monotonic
vm_clockis better. Will change.Why is it needed at all? `free' already supplies you with an audio clock source.
The callback mechanism fires far more often than the emulated hardware expects (and can handle), I need throttling.
So just leave the callback until free get's big enough, you will get dropouts either way, it's just that. I guess you might also use the AUD_init_time_stamp_out/AUD_get_elapsed_usec_out mechanism (implemenation is right at the end of audio/audio_template.h), but i still think that just going with free is saner approach.
Quoted 3 lines
The sad truth is that my original version was still off my an order of magnitude, causing overload for the guest while decoding obviously more complex 128kbit-WMA streams. Now it runs smoothly. :)
Great.
Quoted 2 lines
Maybe there is a way to customize the callback rate, I haven't looked that close, but I'm open for suggestions.
No, you can't customize that.
Signature · 1 line
mailto:[email protected]
Quoted 5 lines · malc
So just leave the callback until free get's big enough, you will get dropouts either way, it's just that. I guess you might also use the AUD_init_time_stamp_out/AUD_get_elapsed_usec_out mechanism (implemenation is right at the end of
audio/audio_template.h), but i still think that just going with free is saner approach.
Well, took some iterations to switch my mind completely to the new model, but now this timestamp-free version runs as smooth as the previous one:
static void audio_callback(void *opaque, int free)
{
mv88w8618_audio_state *s = opaque;
unsigned int block_size, written;
block_size = s->threshold/2 - s->block_written;
if (free - s->last_free < block_size)
return;
audio_fill_mixer_buffer(s, block_size);
written = AUD_write(s->voice, s->mixer_buffer, block_size);
s->last_free = free - written;
if (written < block_size) {
s->block_written += written;
return;
}
s->block_written = 0;
if (s->play_pos == 0) {
s->status |= MP_AUDIO_TX_HALF;
s->play_pos = s->threshold/2;
} else {
s->status |= MP_AUDIO_TX_FULL;
s->play_pos = 0;
}
if (s->status & s->irq_enable)
qemu_irq_raise(s->irq);
}[ This would be even simpler if I could assume written == block_size ||
written == 0. But who knows... ]
Jan
Quoted 10 lines · Jan Kiszka
Quoted 5 lines · malc
So just leave the callback until free get's big enough, you will get dropouts either way, it's just that. I guess you might also use the AUD_init_time_stamp_out/AUD_get_elapsed_usec_out mechanism (implemenation is right at the end of
audio/audio_template.h), but i still think that just going with free is saner approach.Well, took some iterations to switch my mind completely to the new model, but now this timestamp-free version runs as smooth as the previous one:
Glad to hear that.
Quoted 32 lines
static void
audio_callback(void *opaque, int free){ mv88w8618_audio_state *s = opaque; unsigned int block_size, written; block_size = s->threshold/2 - s->block_written; if (free - s->last_free < block_size) return; audio_fill_mixer_buffer(s, block_size); written = AUD_write(s->voice, s->mixer_buffer, block_size); s->last_free = free - written; if (written < block_size) { s->block_written += written; return; } s->block_written = 0; if (s->play_pos == 0) { s->status |= MP_AUDIO_TX_HALF; s->play_pos = s->threshold/2; } else { s->status |= MP_AUDIO_TX_FULL; s->play_pos = 0; } if (s->status & s->irq_enable) qemu_irq_raise(s->irq); }
Quoted 2 lines
[ This would be even simpler if I could assume written ==
block_size||
written == 0. But who knows... ]
The API has no way to enforce this, therefore the only sane option is to assume it will happen. I confess i don't quite get the logic behind the last_free, but as you said, it works smoothly, so be it.
Signature · 1 line
mailto:[email protected]
Quoted 48 lines · malc
Quoted 11 lines · Jan Kiszka
Quoted 6 lines · malc
So just leave the callback until free get's big enough, you will get dropouts either way, it's just that. I guess you might also use the AUD_init_time_stamp_out/AUD_get_elapsed_usec_out mechanism (implemenation
is right at the end ofaudio/audio_template.h), but i still think that just going with free is saner approach.Well, took some iterations to switch my mind completely to the new model, but now this timestamp-free version runs as smooth as the previous one:
Glad to hear that.
Quoted 32 lines
static void
audio_callback(void *opaque, int free){ mv88w8618_audio_state *s = opaque; unsigned int block_size, written; block_size = s->threshold/2 - s->block_written; if (free - s->last_free < block_size) return; audio_fill_mixer_buffer(s, block_size); written = AUD_write(s->voice, s->mixer_buffer, block_size); s->last_free = free - written; if (written < block_size) { s->block_written += written; return; } s->block_written = 0; if (s->play_pos == 0) { s->status |= MP_AUDIO_TX_HALF; s->play_pos = s->threshold/2; } else { s->status |= MP_AUDIO_TX_FULL; s->play_pos = 0; } if (s->status & s->irq_enable) qemu_irq_raise(s->irq); }
Quoted 6 lines
Quoted 2 lines
[ This would be even simpler if I could assume written ==
block_size||
written == 0. But who knows... ]The API has no way to enforce this, therefore the only sane option is to assume it will happen. I confess i don't quite get the logic behind the
last_free, but as you said, it works smoothly, so be it.
It's kind of a beautification: If the host buffer is larger than s->threshold (the buffer size the guest wishes to use), there would be an initial IRQ burst otherwise until the host buffer is consumed.
Jan
Quoted 21 lines · Jan Kiszka
This is the board emulation for Freecom's MusicPal, featuring
- rudimentary PIT and PIC
- up to 2 UARTs
- 88W8xx8 Ethernet controller
- 88W8618 audio controller
- Wolfson WM8750 mixer chip (volume control and mute only)
- 128×64 display with brightness control
- all input buttonsUsing up to 32 MB flash, I hit a limit /wrt
phys_ram_size. I worked around this for now by extendingMAX_BIOS_SIZEto 32 MB, surely not a nice solution.The emulation suffers a bit from limited time, specifically to implement details of hardware setup/shutdown. Another problem - not only for the emulation, but also for native Linux support - is that Marvell yet hides their specs from the community, and existing Linux code [1] is not that clear in every required detail. In case the specs remain Marvell secret, maybe the emulation can help a bit to reverse-engineer the remaining bits (I'm thinking of the binary-blobbed WLAN driver e.g.).
Quoted 1 line
Here is an update, fixing specifically the TX IRQ rate of the audio emulation (which now allows me to play WMA 128kbit streams like "delta radio" smoothly) and incorporating many of the comments the reviewers provided (but not all: still home-brewed volume control, still many unnamed magics).
--- Makefile.target | 1 hw/boards.h | 3 hw/musicpal.c | 1286 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ sysemu.h | 2 vl.c | 1 5 files changed, 1292 insertions(+), 1 deletion(-)
Makefile.target +1 -0
@@ -612,6 +612,7 @@ OBJS+= spitz.o ide.o serial.o nand.o ecc OBJS+= omap1.o omap_lcdc.o omap_dma.o omap_clk.o omap_mmc.o omap_i2c.o OBJS+= palm.o tsc210x.o OBJS+= mst_fpga.o mainstone.o +OBJS+= musicpal.o pflash_cfi02.o CPPFLAGS += -DHAS_AUDIO endif ifeq ($(TARGET_BASE_ARCH), sh4)
hw/boards.h +3 -0
@@ -101,4 +101,7 @@ extern QEMUMachine dummy_m68k_machine;/* mainstone.c */ extern QEMUMachine mainstone2_machine; +/* musicpal.c */ +extern QEMUMachine musicpal_machine; +#endif
vl.c +1 -0
@@ -8056,6 +8056,7 @@ static void register_machines(void)qemu_register_machine (&connex_machine);qemu_register_machine (&verdex_machine);qemu_register_machine (&mainstone2_machine); +qemu_register_machine (&musicpal_machine);#elif defined(TARGET_SH4 )qemu_register_machine (&shix_machine);qemu_register_machine (&r2d_machine);
sysemu.h +1 -1
@@ -109,7 +109,7 @@ extern unsigned int nb_prom_envs;#endif /* XXX: make it dynamic */ -#define MAX_BIOS_SIZE (4 * 1024 * 1024) +#define MAX_BIOS_SIZE (32 * 1024 * 1024)#if defined (TARGET_PPC )#define BIOS_SIZE (1024 * 1024)#elif defined (TARGET_SPARC64 )
hw/musicpal.c new file +1286 -0
1,286 lines not on this page · raw message
Quoted 6 lines · Jan Kiszka
hw/musicpal.c new file +6
-0+static void audio_fill_mixer_buffer (mv88w8618_audio_state *s,unsigned int length) +{ +unsigned int pos; +double val; + +if (s->mute) {
hw/musicpal.c line 115
Quoted 10 lines
hw/musicpal.c new file +10
-0+memset (s->mixer_buffer,0 , length); +return ; + } + +if (s->playback_mode &1 ) +for (pos =0 ; pos < length; pos +=2 ) { + val = *(int16_t *)(s->target_buffer + s->play_pos + pos); + val =le16_to_cpu (val) *pow (10.0 , s->volume/20.0 ); + *(int16_t *)(s->mixer_buffer + pos) = val; + }
This variant "sounds" better:
for (pos = 0; pos < length; pos += 2) {
uint16_t tmp = *(uint16_t *)
(s->target_buffer + s->play_pos + pos);
val = (int16_t)le16_to_cpu(tmp);
val = val * pow(10.0, s->volume/20.0);
*(int16_t *)(s->mixer_buffer + pos) = val;
}Find latest patches at http://home.arcor.de/jan.kiszka/patches/QEMU/
Jan
Hi,
Quoted 6 lines · Jan Kiszka
This is the board emulation for Freecom's MusicPal, featuring
- rudimentary PIT and PIC
- up to 2 UARTs
- 88W8xx8 Ethernet controller
- 88W8618 audio controller
- Wolfson WM8750 mixer chip (volume control and mute only)
Are you sure that hw/wm8750.c is not reusable? It's probably better to extend it with volume control, and audio data transmission through i2c, instead of having two implementations in QEMU.
Regarding volume control, since there are only 0x7f possible values it's nicer to use a const table than pull in math.h. Actually I wouldn't worry about that at all because the layout of volume levels on the host is not guaranteed to be linear (I think?), it may very well be already exponential. For example the Linux WM8750 driver doesn't do the reverse transformation which would mean that on a host whose ALSA output device is a WM8750, the Qemu vm would have a wrong volume scale.
Quoted 6 lines
- 128×64 display with brightness control
- all input buttonsUsing up to 32 MB flash, I hit a limit /wrt
phys_ram_size. I worked around this for now by extendingMAX_BIOS_SIZEto 32 MB, surely not a nice solution.
You can use -m 150 or similar.
Please also format the code similarly to rest of Qemu.
Signature · 2 lines
Please do not print this email unless absolutely necessary. Spread environmental awareness.
Quoted 12 lines · andrzej zaborowski
Quoted 6 lines · Jan Kiszka
This is the board emulation for Freecom's MusicPal, featuring
- rudimentary PIT and PIC
- up to 2 UARTs
- 88W8xx8 Ethernet controller
- 88W8618 audio controller
- Wolfson WM8750 mixer chip (volume control and mute only)Are you sure that
hw/wm8750.cis not reusable? It's probably better
to extend it with volume control, and audio data transmission through i2c, instead of having two implementations in QEMU.
Quoted 8 lines
Regarding volume control, since there are only
0x7fpossible values it's nicer to use a const table than pull inmath.h. Actually I wouldn't worry about that at all because the layout of volume levels on the host is not guaranteed to be linear (I think?), it may very well be already exponential. For example the Linux WM8750 driver doesn't do the reverse transformation which would mean that on a host whose ALSA output device is a WM8750, the Qemu vm would have a wrong volume scale.
Ah, we're multiplying the samples, rather than setting volume on host.
Please ignore the above comment, but it would still be good to avoidpow() (especially if return value is invariant across the loop and perhaps across many buffer fills).
Signature · 2 lines
Please do not print this email unless absolutely necessary. Spread environmental awareness.
Quoted 10 lines · andrzej zaborowski
Hi,
Quoted 6 lines · Jan Kiszka
This is the board emulation for Freecom's MusicPal, featuring
- rudimentary PIT and PIC
- up to 2 UARTs
- 88W8xx8 Ethernet controller
- 88W8618 audio controller
- Wolfson WM8750 mixer chip (volume control and mute only)
Quoted 3 lines
Are you sure that
hw/wm8750.cis not reusable? It's probably better to extend it with volume control, and audio data transmission through i2c, instead of having two implementations in QEMU.
Will check again, but I don't think it is helpful, at least at this point. The thing is that the MusicPal uses the on-chip DAC, not the one of the Wolfson. The latter seems to be responsible for analogous mixing only.
Quoted 9 lines
Regarding volume control, since there are only
0x7fpossible values it's nicer to use a const table than pull inmath.h. Actually I wouldn't worry about that at all because the layout of volume levels on the host is not guaranteed to be linear (I think?), it may very well be already exponential. For example the Linux WM8750 driver doesn't do the reverse transformation which would mean that on a host whose ALSA output device is a WM8750, the Qemu vm would have a wrong volume scale.
[ Ignoring the above as advised in the follow-up, but will think about avoiding or reducing the usage of pow(). ]
Quoted 11 lines
Quoted 7 lines
- 128×64 display with brightness control
- all input buttonsUsing up to 32 MB flash, I hit a limit /wrt
phys_ram_size. I worked around this for now by extendingMAX_BIOS_SIZEto 32 MB, surely not a nice solution.You can use -m 150 or similar.
Please also format the code similarly to rest of Qemu.
That would just increase ram_size, thus won't help as I need memory beyond it (here for the pflash in R/W mode).
Jan
Quoted 10 lines · Jan Kiszka
Quoted 9 lines · andrzej zaborowski
Hi,
Quoted 6 lines · Jan Kiszka
This is the board emulation for Freecom's MusicPal, featuring
- rudimentary PIT and PIC
- up to 2 UARTs
- 88W8xx8 Ethernet controller
- 88W8618 audio controller
- Wolfson WM8750 mixer chip (volume control and mute only)
Quoted 8 lines
Quoted 3 lines
Are you sure that
hw/wm8750.cis not reusable? It's probably better to extend it with volume control, and audio data transmission through i2c, instead of having two implementations in QEMU.Will check again, but I don't think it is helpful, at least at this point. The thing is that the MusicPal uses the on-chip DAC, not the one of the Wolfson. The latter seems to be responsible for analogous mixing only.
That was nonsense: The audio architecture of the MusicPal appears to be much like the one of the Spitz. Redirecting the audio stream to the Wolfson should be feasible. However, lacking support for volume control and muting currently prevents this.
Andrzej, as you have written the wm8750, do you already know where which volume level would have to be applied (open-coded or via some AUD_set_volume)? I'm currently only using LOUT2VOL, and I'm a bit lazy to study the datasheet /wrt all the mixer details.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
- 128×64 display with brightness control
- all input buttonsUsing up to 32 MB flash, I hit a limit /wrt
phys_ram_size. I worked around this for now by extendingMAX_BIOS_SIZEto 32 MB, surely not a nice solution.
Quoted 6 lines
Quoted 3 lines
You can use -m 150 or similar.
Please also format the code similarly to rest of Qemu.
That would just increase
ram_size, thus won't help as I need memory beyond it (here for the pflash in R/W mode).
OK, I see what you mean after looking at your N800 patches: You apply a fixed RAM size, leaving the rest of what the user provided via -m to SRAM and flash. Not optimal IMHO, you may sometimes also want to play with the RAM size even if the real devices has a fixed amount. And it is far from being intuitive as well.
The only true solution I see right now is moving qemu_vmalloc into the machine initialization code. Is there anything between current qemu_vmalloc and machine->init that relies on phys_ram_base being valid (and which can't be moved after the machine init) and thus prevents this?
Jan
Quoted 4 lines · Jan Kiszka
Andrzej, as you have written the wm8750, do you already know where which volume level would have to be applied (open-coded or via some
AUD_set_volume)? I'm currently only using LOUT2VOL, and I'm a bit lazy to study the datasheet /wrt all the mixer details.
My idea was to open
http://www.wolfsonmicro.com/uploads/documents/en/WM8750.pdf and on the first page every Wolfson datasheet has its diagram of all audio paths (of which there are always too many) and then trace with my finger the path between the source (the I2C or I2S interfaces) and the sink (the analog output), and then multiply all the volume values that are applied there (both analog and digital) and pass that to host mixer through some functions in audio/ for the given SWVoice - but we don't have any such functions and I'm ok with using the host mixer manually.
(VirtualBox has them implemented iirc) So yes, maybe it makes sense
to multiply the samples for the moment and use only LOUTnVOL / ROUTnVOL values as these are used by the guests we're interested in.
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
- 128×64 display with brightness control
- all input buttonsUsing up to 32 MB flash, I hit a limit /wrt
phys_ram_size. I worked around this for now by extendingMAX_BIOS_SIZEto 32 MB, surely not a nice solution.
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
You can use -m 150 or similar.
Please also format the code similarly to rest of Qemu.
That would just increase
ram_size, thus won't help as I need memory beyond it (here for the pflash in R/W mode).
Yes, I had not looked at how ram_size was used in the musicpal board initialisation, sorry.
Quoted 7 lines
OK, I see what you mean after looking at your N800 patches: You apply a
fixed RAM size, leaving the rest of what the user provided via -m to SRAM and flash. Not optimal IMHO, you may sometimes also want to play with the RAM size even if the real devices has a fixed amount. And it is far from being intuitive as well.
Yes, although you allow the user to set also a smaller RAM than what the virtual machine expects.
Quoted 5 lines
The only true solution I see right now is moving
qemu_vmallocinto the machine initialization code. Is there anything between currentqemu_vmallocandmachine->initthat relies onphys_ram_basebeing valid (and which can't be moved after the machine init) and thus prevents this?
I had a different idea: add a field ram_constraint in struct QEMUMachine, which would hold the amount of RAM the machine always needs (e.g. bios and video RAM), and the low bit could hold a flag RAM_SIZE_FIXED for machines that have only such RAM (basically the criteria should be whether it's possible for the guest to detect the memory size there is on board - on machines like Spitz there's no way) and for such machines the -m parameter would be invalid. I'll try to come up with a patch.
Signature · 2 lines
Please do not print this email unless absolutely necessary. Spread environmental awareness.
Quoted 6 lines · andrzej zaborowski
Quoted 4 lines · Jan Kiszka
Andrzej, as you have written the wm8750, do you already know where which volume level would have to be applied (open-coded or via some
AUD_set_volume)? I'm currently only using LOUT2VOL, and I'm a bit lazy to study the datasheet /wrt all the mixer details.
Quoted 12 lines
My idea was to open
http://www.wolfsonmicro.com/uploads/documents/en/WM8750.pdf and on the first page every Wolfson datasheet has its diagram of all audio paths (of which there are always too many) and then trace with my finger the path between the source (the I2C or I2S interfaces) and the sink (the analog output), and then multiply all the volume values that are applied there (both analog and digital) and pass that to host mixer through some functions in audio/ for the given SWVoice - but we don't have any such functions and I'm ok with using the host mixer manually.
(VirtualBox has them implemented iirc) So yes, maybe it makes sense
to multiply the samples for the moment and use only LOUTnVOL / ROUTnVOL values as these are used by the guests we're interested in.
Done, and it finally works. One of the two quirks I found in wm8750 made the switch a bit hairy. Patches will follow.
Quoted 19 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
- 128×64 display with brightness control
- all input buttonsUsing up to 32 MB flash, I hit a limit /wrt
phys_ram_size. I worked around this for now by extendingMAX_BIOS_SIZEto 32 MB, surely not a nice solution.Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
You can use -m 150 or similar.
Please also format the code similarly to rest of Qemu.
That would just increase
ram_size, thus won't help as I need memory beyond it (here for the pflash in R/W mode).Yes, I had not looked at how
ram_sizewas used in the musicpal board initialisation, sorry.Quoted 1 line
Quoted 8 lines
Quoted 5 lines
OK, I see what you mean after looking at your N800 patches: You apply a
fixed RAM size, leaving the rest of what the user provided via -m to SRAM and flash. Not optimal IMHO, you may sometimes also want to play with the RAM size even if the real devices has a fixed amount. And it is far from being intuitive as well.Yes, although you allow the user to set also a smaller RAM than what the virtual machine expects.
That's indeed something the machine should take of (if there are such hard limits).
Quoted 6 lines
Quoted 5 lines
The only true solution I see right now is moving
qemu_vmallocinto the machine initialization code. Is there anything between currentqemu_vmallocandmachine->initthat relies onphys_ram_basebeing valid (and which can't be moved after the machine init) and thus prevents this?
Quoted 6 lines
I had a different idea: add a field
ram_constraintin struct QEMUMachine, which would hold the amount of RAM the machine always needs (e.g. bios and video RAM), and the low bit could hold a flagRAM_SIZE_FIXEDfor machines that have only such RAM (basically the criteria should be whether it's possible for the guest to detect the memory size there is on board - on machines like Spitz there's no way)
IIRC, embedded boards let the boot loader "detect" this. I see valid scenarios where one wants to play with different sizes and may therefore patch U-Boot - or load the kernel directly which should make QEMU set the related ATAG field appropriately, no?
Quoted 2 lines
and for such machines the -m parameter would be invalid. I'll try to come up with a patch.
I originally had the same idea but I dropped it because it would still overload -m with semantics that don't belong there. IMHO -m should only describe the main RAM size, not any additionally by QEMU required memory for establishing fixed SRAM or even for backing up flash devices. That's at least what I would expect from this switch and what the documentation suggests as well so far.
Thus, back to square #1: Can't we move qemu_vmalloc into the machine-specific init handlers?
Jan
Quoted 21 lines · Jan Kiszka
Quoted 6 lines · andrzej zaborowski
Quoted 4 lines · Jan Kiszka
Andrzej, as you have written the wm8750, do you already know where which volume level would have to be applied (open-coded or via some
AUD_set_volume)? I'm currently only using LOUT2VOL, and I'm a bit lazy to study the datasheet /wrt all the mixer details.Quoted 12 lines
My idea was to open
http://www.wolfsonmicro.com/uploads/documents/en/WM8750.pdf and on the first page every Wolfson datasheet has its diagram of all audio paths (of which there are always too many) and then trace with my finger the path between the source (the I2C or I2S interfaces) and the sink (the analog output), and then multiply all the volume values that are applied there (both analog and digital) and pass that to host mixer through some functions in audio/ for the given SWVoice - but we don't have any such functions and I'm ok with using the host mixer manually.
(VirtualBox has them implemented iirc) So yes, maybe it makes sense
to multiply the samples for the moment and use only LOUTnVOL / ROUTnVOL values as these are used by the guests we're interested in.
Quoted 2 lines
Done, and it finally works. One of the two quirks I found in wm8750 made
the switch a bit hairy. Patches will follow.
Thanks. I pushed the patch with fixes. Regarding the wm8750_fini patch, I'll #if 0 it because it's possible that a board will have this chip on something hotpluggable and will need to create and destroy it various times and it's easy to miss something in the clean-up. Regarding the volume patch, I'll make a look-up table at one point, and then merge. Also, if we have 16-bit data and 7-bit volume scale maybe we're fine with scalling only the most-significant-byte and avoiding endianness headaches (or maybe not). Nevertheless the MusicPal emulator should be bootable without that.
Quoted 49 lines
Quoted 21 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
- 128×64 display with brightness control
- all input buttonsUsing up to 32 MB flash, I hit a limit /wrt
phys_ram_size. I worked around this for now by extendingMAX_BIOS_SIZEto 32 MB, surely not a nice solution.Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
You can use -m 150 or similar.
Please also format the code similarly to rest of Qemu.
That would just increase
ram_size, thus won't help as I need memory beyond it (here for the pflash in R/W mode).Yes, I had not looked at how
ram_sizewas used in the musicpal board initialisation, sorry.Quoted 1 line
Quoted 8 lines
Quoted 5 lines
OK, I see what you mean after looking at your N800 patches: You apply a
fixed RAM size, leaving the rest of what the user provided via -m to SRAM and flash. Not optimal IMHO, you may sometimes also want to play with the RAM size even if the real devices has a fixed amount. And it is far from being intuitive as well.Yes, although you allow the user to set also a smaller RAM than what the virtual machine expects.
That's indeed something the machine should take of (if there are such
hard limits).Quoted 6 lines
Quoted 5 lines
The only true solution I see right now is moving
qemu_vmallocinto the machine initialization code. Is there anything between currentqemu_vmallocandmachine->initthat relies onphys_ram_basebeing valid (and which can't be moved after the machine init) and thus prevents this?Quoted 6 lines
I had a different idea: add a field
ram_constraintin struct QEMUMachine, which would hold the amount of RAM the machine always needs (e.g. bios and video RAM), and the low bit could hold a flagRAM_SIZE_FIXEDfor machines that have only such RAM (basically the criteria should be whether it's possible for the guest to detect the memory size there is on board - on machines like Spitz there's no way)
Quoted 4 lines
IIRC, embedded boards let the boot loader "detect" this. I see valid
scenarios where one wants to play with different sizes and may therefore patch U-Boot - or load the kernel directly which should make QEMU set the related ATAG field appropriately, no?
Yes, in case of a standard firmware like Linux or U-boot - but we probably don't need to provide options for everything one may want to play with unless it's a valid hardware configuration (like in the PC case where you can add and take away RAM sticks), at some point the user needs to edit the source either way.
Anyway almost half of the boards in qemu ignored ram_size until now and risked the provided size being too low and segfaulting, so with the patch I sent in another mail at least there's a check, and the check is only done once for all boards so it can be removed from the few boards that did it.
Quoted 6 lines
Quoted 4 lines
and for such machines the -m parameter would be invalid. I'll try to come up with a patch.
Quoted 6 lines
I originally had the same idea but I dropped it because it would still
overload -m with semantics that don't belong there. IMHO -m should only describe the main RAM size, not any additionally by QEMU required memory for establishing fixed SRAM or even for backing up flash devices. That's at least what I would expect from this switch and what the documentation suggests as well so far.
This property is not changed by the patch (I hope).
Quoted 3 lines
Thus, back to square #1: Can't we move
qemu_vmallocinto the machine-specific init handlers?
I think it would work (but I wouldn't do that).
Cheers
Signature · 2 lines
Please do not print this email unless absolutely necessary. Spread environmental awareness.
Quoted 22 lines · andrzej zaborowski
Quoted 21 lines · Jan Kiszka
Quoted 18 lines · andrzej zaborowski
Quoted 4 lines · Jan Kiszka
Andrzej, as you have written the wm8750, do you already know where which volume level would have to be applied (open-coded or via some
AUD_set_volume)? I'm currently only using LOUT2VOL, and I'm a bit lazy to study the datasheet /wrt all the mixer details.My idea was to open
http://www.wolfsonmicro.com/uploads/documents/en/WM8750.pdf and on the first page every Wolfson datasheet has its diagram of all audio paths (of which there are always too many) and then trace with my finger the path between the source (the I2C or I2S interfaces) and the sink (the analog output), and then multiply all the volume values that are applied there (both analog and digital) and pass that to host mixer through some functions in audio/ for the given SWVoice - but we don't have any such functions and I'm ok with using the host mixer manually.
(VirtualBox has them implemented iirc) So yes, maybe it makes sense
to multiply the samples for the moment and use only LOUTnVOL / ROUTnVOL values as these are used by the guests we're interested in.
Quoted 8 lines
Quoted 2 lines
Done, and it finally works. One of the two quirks I found in wm8750 made
the switch a bit hairy. Patches will follow.Thanks. I pushed the patch with fixes. Regarding the
wm8750_finipatch, I'll #if 0 it because it's possible that a board will have this chip on something hotpluggable and will need to create and destroy it various times and it's easy to miss something in the clean-up. Regarding the volume patch, I'll make a look-up table at one point,
Don't understand yet why (are you afraid of pow, libm, or float in general?), but if it helps to get things merged... ;)
Quoted 2 lines
and then merge. Also, if we have 16-bit data and 7-bit volume scale maybe we're fine with scalling only the most-significant-byte and
Hmm, wasn't endianness about finding out which byte is most-significant and which not? :->
Quoted 73 lines
avoiding endianness headaches (or maybe not). Nevertheless the MusicPal emulator should be bootable without that.
Quoted 48 lines
Quoted 20 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
- 128×64 display with brightness control
- all input buttonsUsing up to 32 MB flash, I hit a limit /wrt
phys_ram_size. I worked around this for now by extendingMAX_BIOS_SIZEto 32 MB, surely not a nice solution.Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
You can use -m 150 or similar.
Please also format the code similarly to rest of Qemu.
That would just increase
ram_size, thus won't help as I need memory beyond it (here for the pflash in R/W mode).Yes, I had not looked at how
ram_sizewas used in the musicpal board initialisation, sorry.Quoted 1 line
Quoted 8 lines
Quoted 5 lines
OK, I see what you mean after looking at your N800 patches: You apply a
fixed RAM size, leaving the rest of what the user provided via -m to SRAM and flash. Not optimal IMHO, you may sometimes also want to play with the RAM size even if the real devices has a fixed amount. And it is far from being intuitive as well.Yes, although you allow the user to set also a smaller RAM than what the virtual machine expects.
That's indeed something the machine should take of (if there are such
hard limits).Quoted 6 lines
Quoted 5 lines
The only true solution I see right now is moving
qemu_vmallocinto the machine initialization code. Is there anything between currentqemu_vmallocandmachine->initthat relies onphys_ram_basebeing valid (and which can't be moved after the machine init) and thus prevents this?Quoted 6 lines
I had a different idea: add a field
ram_constraintin struct QEMUMachine, which would hold the amount of RAM the machine always needs (e.g. bios and video RAM), and the low bit could hold a flagRAM_SIZE_FIXEDfor machines that have only such RAM (basically the criteria should be whether it's possible for the guest to detect the memory size there is on board - on machines like Spitz there's no way)Quoted 4 lines
IIRC, embedded boards let the boot loader "detect" this. I see valid
scenarios where one wants to play with different sizes and may therefore patch U-Boot - or load the kernel directly which should make QEMU set the related ATAG field appropriately, no?Yes, in case of a standard firmware like Linux or U-boot - but we probably don't need to provide options for everything one may want to play with unless it's a valid hardware configuration (like in the PC case where you can add and take away RAM sticks), at some point the user needs to edit the source either way.
Anyway almost half of the boards in qemu ignored
ram_sizeuntil now and risked the provided size being too low and segfaulting, so with the patch I sent in another mail at least there's a check, and the check is only done once for all boards so it can be removed from the few boards that did it.Quoted 5 lines
Quoted 3 lines
and for such machines the -m parameter would be invalid. I'll try to come up with a patch.
Quoted 8 lines
Quoted 6 lines
I originally had the same idea but I dropped it because it would still
overload -m with semantics that don't belong there. IMHO -m should only describe the main RAM size, not any additionally by QEMU required memory for establishing fixed SRAM or even for backing up flash devices. That's at least what I would expect from this switch and what the documentation suggests as well so far.This property is not changed by the patch (I hope).
Yes, it restores the original semantic, at least as long as RAMSIZE_FIXED is not set. That case is still a bit suboptimal as you have to provide pessimistic values, e.g. the maximum flash size that can be used. But I can live with it I guess.
Jan
Quoted 26 lines · Jan Kiszka
Quoted 25 lines · andrzej zaborowski
Quoted 23 lines · Jan Kiszka
Quoted 6 lines · andrzej zaborowski
Quoted 4 lines · Jan Kiszka
Andrzej, as you have written the wm8750, do you already know where which volume level would have to be applied (open-coded or via some
AUD_set_volume)? I'm currently only using LOUT2VOL, and I'm a bit lazy to study the datasheet /wrt all the mixer details.Quoted 12 lines
My idea was to open
http://www.wolfsonmicro.com/uploads/documents/en/WM8750.pdf and on the first page every Wolfson datasheet has its diagram of all audio paths (of which there are always too many) and then trace with my finger the path between the source (the I2C or I2S interfaces) and the sink (the analog output), and then multiply all the volume values that are applied there (both analog and digital) and pass that to host mixer through some functions in audio/ for the given SWVoice - but we don't have any such functions and I'm ok with using the host mixer manually.
(VirtualBox has them implemented iirc) So yes, maybe it makes sense
to multiply the samples for the moment and use only LOUTnVOL / ROUTnVOL values as these are used by the guests we're interested in.Done, and it finally works. One of the two quirks I found in wm8750 made
the switch a bit hairy. Patches will follow.
Quoted 8 lines
Quoted 5 lines
Thanks. I pushed the patch with fixes. Regarding the
wm8750_finipatch, I'll #if 0 it because it's possible that a board will have this chip on something hotpluggable and will need to create and destroy it various times and it's easy to miss something in the clean-up. Regarding the volume patch, I'll make a look-up table at one point,Don't understand yet why (are you afraid of pow, libm, or float in
general?), but if it helps to get things merged... ;)
I think the idea (not mine) was to not depend on it if not necessary, like in this case, pow() is a rather heavy tool for the simple issue.
Quoted 8 lines
Quoted 4 lines
and then merge. Also, if we have 16-bit data and 7-bit volume scale maybe we're fine with scalling only the most-significant-byte and
Hmm, wasn't endianness about finding out which byte is most-significant
and which not? :->
Yes, the thing is that WM8750's input endianness is known and the MSB is always the same one. The host's endianness varies and to do the multiplication in C we need the samples to be host-endian (but audio functions do accept guest-endian).
Quoted 82 lines
Quoted 82 lines
avoiding endianness headaches (or maybe not). Nevertheless the MusicPal emulator should be bootable without that.
Quoted 48 lines
Quoted 20 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
- 128×64 display with brightness control
- all input buttonsUsing up to 32 MB flash, I hit a limit /wrt
phys_ram_size. I worked around this for now by extendingMAX_BIOS_SIZEto 32 MB, surely not a nice solution.Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
You can use -m 150 or similar.
Please also format the code similarly to rest of Qemu.
That would just increase
ram_size, thus won't help as I need memory beyond it (here for the pflash in R/W mode).Yes, I had not looked at how
ram_sizewas used in the musicpal board initialisation, sorry.Quoted 1 line
Quoted 8 lines
Quoted 5 lines
OK, I see what you mean after looking at your N800 patches: You apply a
fixed RAM size, leaving the rest of what the user provided via -m to SRAM and flash. Not optimal IMHO, you may sometimes also want to play with the RAM size even if the real devices has a fixed amount. And it is far from being intuitive as well.Yes, although you allow the user to set also a smaller RAM than what the virtual machine expects.
That's indeed something the machine should take of (if there are such
hard limits).Quoted 6 lines
Quoted 5 lines
The only true solution I see right now is moving
qemu_vmallocinto the machine initialization code. Is there anything between currentqemu_vmallocandmachine->initthat relies onphys_ram_basebeing valid (and which can't be moved after the machine init) and thus prevents this?Quoted 6 lines
I had a different idea: add a field
ram_constraintin struct QEMUMachine, which would hold the amount of RAM the machine always needs (e.g. bios and video RAM), and the low bit could hold a flagRAM_SIZE_FIXEDfor machines that have only such RAM (basically the criteria should be whether it's possible for the guest to detect the memory size there is on board - on machines like Spitz there's no way)Quoted 4 lines
IIRC, embedded boards let the boot loader "detect" this. I see valid
scenarios where one wants to play with different sizes and may therefore patch U-Boot - or load the kernel directly which should make QEMU set the related ATAG field appropriately, no?Yes, in case of a standard firmware like Linux or U-boot - but we probably don't need to provide options for everything one may want to play with unless it's a valid hardware configuration (like in the PC case where you can add and take away RAM sticks), at some point the user needs to edit the source either way.
Anyway almost half of the boards in qemu ignored
ram_sizeuntil now and risked the provided size being too low and segfaulting, so with the patch I sent in another mail at least there's a check, and the check is only done once for all boards so it can be removed from the few boards that did it.Quoted 11 lines
Quoted 3 lines
and for such machines the -m parameter would be invalid. I'll try to come up with a patch.
I originally had the same idea but I dropped it because it would still
overload -m with semantics that don't belong there. IMHO -m should only describe the main RAM size, not any additionally by QEMU required memory for establishing fixed SRAM or even for backing up flash devices. That's at least what I would expect from this switch and what the documentation suggests as well so far.
Quoted 6 lines
Quoted 1 line
This property is not changed by the patch (I hope).
Yes, it restores the original semantic, at least as long as
RAMSIZE_FIXEDis not set. That case is still a bit suboptimal as you have to provide pessimistic values, e.g. the maximum flash size that can be used. But I can live with it I guess.
You absolutely don't have to set RAMSIZE_FIXED if you want to use -m. In the board init you can for example decide to have fixed ram size and flash size decided by ram_size if that's what you need. As long as there's only one -m allowed one of the sizes has to be hardcoded.
Thanks for the rework.
Signature · 2 lines
Please do not print this email unless absolutely necessary. Spread environmental awareness.
PATCH 1/1 Add support for Marvell 88w8618 / MusicPal
In reply to Jan KiszkaHere comes a polished and further improved version of the MusicPal emulation. Changes since last posting:
- Exploit existing wm8750 model (enhanced version)
- Fix IRQ re-entrance due to wrong PIC status
- (For now) fixed RAM size, using recently proposed per-machine ram_require
- Code cleanup, all magics pushed into #defines as well
- Reformatting according to QEMU coding style
This patch depends on
- SVN #4225
- Andrzej's per-machine memory size patch
- wm8750 volume patch
--- Makefile.target | 1 hw/boards.h | 3 hw/musicpal.c | 1456 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ vl.c | 1 4 files changed, 1461 insertions(+)
Makefile.target +1 -0
@@ -614,6 +614,7 @@ OBJS+= omap2.o omap_dss.o OBJS+= palm.o tsc210x.o OBJS+= nseries.o blizzard.o onenand.o vga.o cbus.o OBJS+= mst_fpga.o mainstone.o +OBJS+= musicpal.o pflash_cfi02.o CPPFLAGS += -DHAS_AUDIO endif ifeq ($(TARGET_BASE_ARCH), sh4)
hw/boards.h +3 -0
@@ -107,4 +107,7 @@ extern QEMUMachine dummy_m68k_machine;/* mainstone.c */ extern QEMUMachine mainstone2_machine; +/* musicpal.c */ +extern QEMUMachine musicpal_machine; +#endif
vl.c +1 -0
@@ -8057,6 +8057,7 @@ static void register_machines(void)qemu_register_machine (&connex_machine);qemu_register_machine (&verdex_machine);qemu_register_machine (&mainstone2_machine); +qemu_register_machine (&musicpal_machine);#elif defined(TARGET_SH4 )qemu_register_machine (&shix_machine);qemu_register_machine (&r2d_machine);
hw/musicpal.c new file +1456 -0
1,456 lines not on this page · raw message