Re: [PATCH 5/5] hw/audio/via-ac97: Basic implementation of audio playback

[BALATON Zoltan]

On Fri, 24 Feb 2023, Bernhard Beschow wrote:
> Am 23. Februar 2023 19:40:03 UTC schrieb "Volker Rümelin" <vr_qemu@t-online.de>:
> > Am 21.02.23 um 19:44 schrieb BALATON Zoltan:
> > > This adds basic implementation of the AC'97 sound part used in VIA
> > > south bridge chips. Not all features of the device is emulated, only
> > > one playback channel is supported but this is enough to get sound
> > > output from some guests running on machines using this device such as
> > > pegasos2.
> > >
> > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> > > ---
> > >   hw/audio/trace-events     |   6 +
> > >   hw/audio/via-ac97.c       | 436 +++++++++++++++++++++++++++++++++++++-
> > >   hw/isa/vt82c686.c         |   3 +-
> > >   include/hw/isa/vt82c686.h |  26 +++
> > >   4 files changed, 466 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/hw/audio/trace-events b/hw/audio/trace-events
> > > index e0e71cd9b1..6eccdaa4b5 100644
> > > --- a/hw/audio/trace-events
> > > +++ b/hw/audio/trace-events
> > > @@ -11,3 +11,9 @@ hda_audio_running(const char *stream, int nr, bool running) 
> > > "st %s, nr %d, run %
> > >   hda_audio_format(const char *stream, int chan, const char *fmt, int freq) "st %s, 
> > > %d x %s @ %d Hz"
> > >   hda_audio_adjust(const char *stream, int pos) "st %s, pos %d"
> > >   hda_audio_overrun(const char *stream) "st %s"
> > > +
> > > +#via-ac97.c
> > > +via_ac97_codec_write(uint8_t addr, uint16_t val) "0x%x <- 0x%x"
> > > +via_ac97_sgd_fetch(uint32_t addr, char stop, char eol, char flag, uint32_t len) 
> > > "addr=0x%x %c%c%c len=%d"
> > > +via_ac97_sgd_read(uint64_t addr, unsigned size, uint64_t val) "0x%"PRIx64" %d -> 
> > > 0x%"PRIx64
> > > +via_ac97_sgd_write(uint64_t addr, unsigned size, uint64_t val) "0x%"PRIx64" %d <- 
> > > 0x%"PRIx64
> > > diff --git a/hw/audio/via-ac97.c b/hw/audio/via-ac97.c
> > > index d1a856f63d..cdac5bc14b 100644
> > > --- a/hw/audio/via-ac97.c
> > > +++ b/hw/audio/via-ac97.c
> > > @@ -1,39 +1,467 @@
> > >   /*
> > >    * VIA south bridges sound support
> > >    *
> > > + * Copyright (c) 2022-2023 BALATON Zoltan
> > > + *
> > >    * This work is licensed under the GNU GPL license version 2 or later.
> > >    */
> > >     /*
> > > - * TODO: This is entirely boiler plate just registering empty PCI devices
> > > - * with the right ID guests expect, functionality should be added here.
> > > + * TODO: This is only a basic implementation of one audio playback channel
> > > + *       more functionality should be added here.
> > >    */
> > >     #include "qemu/osdep.h"
> > > +#include "qemu/log.h"
> > >   #include "hw/isa/vt82c686.h"
> > > -#include "hw/pci/pci_device.h"
> > > +#include "ac97.h"
> > > +#include "trace.h"
> > > +
> > > +#define CLEN_IS_EOL(x)  ((x)->clen & BIT(31))
> > > +#define CLEN_IS_FLAG(x) ((x)->clen & BIT(30))
> > > +#define CLEN_IS_STOP(x) ((x)->clen & BIT(29))
> > > +#define CLEN_LEN(x)     ((x)->clen & 0xfff)
> >
> > Hi,
> >
> > the CLEN_LEN mask is 0xffffff instead of 0xfff. The SGD Table Base Count has 24 
> > bits.
>
> This fixes sound for a x86 Linux guest on my pc-via branch.

Thanks, will be in next version. Not sure where have I lost that, maybe 
accidenatlly deleted part of the constant on a rebase and did not notice.
Bernhard does that mean you give a Tested-by for this patch?

Regards,
BALATON Zoltan