Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk
On 18/05/20 15:27, Gerd Hoffmann wrote: > Ah, cool, that shows that I'm on the right path with my idea ;) > Sneak preview: > https://git.kraxel.org/cgit/qemu/log/?h=sirius/soundhw > > Suggestions for a good name? I've used "pc.pcspk" for now, > but maybe "pc.audiodev0" or "pc.sound0" is better? Something like "-M pc,pcspk-audiodev=..."? Paolo
Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk
Hi, > >> Initialization order looks tricky though. I'd have to create pcspk > >> early, simliar to flash, in pc_machine_initfn(). Problem is I don't > >> have a isa bus yet at that point (flash is sysbus and doesn't have this > >> problem). I'm open to suggestions hiow do deal with that best. > > > > Seems I've found a way to deal with that: "ISADevice *pcspk = > > object_new(TYPE_PC_SPEAKER);" can be done before the isa bus exists > > & we can fixup things later using qdev_set_parent_bus(). > > You'll want to watch out for the series I hope to post shortly: it'll be > dev = qdev_new(TYPE_PC_SPEAKER); qdev_realize(dev, bus, errp) then. No > need for qdev_set_parent_bus(). Ah, cool, that shows that I'm on the right path with my idea ;) Sneak preview: https://git.kraxel.org/cgit/qemu/log/?h=sirius/soundhw Suggestions for a good name? I've used "pc.pcspk" for now, but maybe "pc.audiodev0" or "pc.sound0" is better? take care, Gerd
Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk
Gerd Hoffmann writes: > Hi, > >> Initialization order looks tricky though. I'd have to create pcspk >> early, simliar to flash, in pc_machine_initfn(). Problem is I don't >> have a isa bus yet at that point (flash is sysbus and doesn't have this >> problem). I'm open to suggestions hiow do deal with that best. > > Seems I've found a way to deal with that: "ISADevice *pcspk = > object_new(TYPE_PC_SPEAKER);" can be done before the isa bus exists > & we can fixup things later using qdev_set_parent_bus(). You'll want to watch out for the series I hope to post shortly: it'll be dev = qdev_new(TYPE_PC_SPEAKER); qdev_realize(dev, bus, errp) then. No need for qdev_set_parent_bus().
Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk
Hi, > Initialization order looks tricky though. I'd have to create pcspk > early, simliar to flash, in pc_machine_initfn(). Problem is I don't > have a isa bus yet at that point (flash is sysbus and doesn't have this > problem). I'm open to suggestions hiow do deal with that best. Seems I've found a way to deal with that: "ISADevice *pcspk = object_new(TYPE_PC_SPEAKER);" can be done before the isa bus exists & we can fixup things later using qdev_set_parent_bus(). cheers, Gerd
Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk
On Mon, May 18, 2020 at 11:26:50AM +0100, Daniel P. Berrangé wrote:
> On Mon, May 18, 2020 at 12:16:28PM +0200, Gerd Hoffmann wrote:
> > On Fri, May 15, 2020 at 05:08:23PM +0200, Ján Tomko wrote:
> > > On a Friday in 2020, Gerd Hoffmann wrote:
> > > > Add deprecation message to the audio init function.
> > > >
> > > > Factor out audio initialization and call that from
> > > > both audio init and realize, so setting audiodev via
> > > > -global is enough to properly initialize pcspk.
> > > >
> > > > Signed-off-by: Gerd Hoffmann
> > > > ---
> > > > hw/audio/pcspk.c | 24 +---
> > > > 1 file changed, 21 insertions(+), 3 deletions(-)
> > > >
> > > > @@ -236,9 +245,18 @@ static const TypeInfo pcspk_info = {
> > > > .class_init = pcspk_class_initfn,
> > > > };
> > > >
> > > > +static int pcspk_audio_init_soundhw(ISABus *bus)
> > > > +{
> > > > +PCSpkState *s = pcspk_state;
> > > > +
> > > > +warn_report("'-soundhw pcspk' is deprecated, "
> > > > +"please set a backend using '-global
> > > > isa-pcspk.audiodev=' instead");
> > > > +return pcspk_audio_init(s);
> > >
> > > -soundhw pcspk is the only soundhw device present in libvirt git.
> > >
> > > Is there a way to probe for this change via QMP?
> >
> > Oops. I'm surprised libvirt actually supports pcspk.
> >
> > There is no way to see that in qmp, and I can't think of an easy way
> > to add that. Does libvirt check for command line switches still?
> > So it could see -soundhw going away if that happens?
>
> IIUC, instead of probing for whether -soundhw is deprecated, it should
> be suffiicent for us to probe if "isa-pcspk.audiodev" exists. Assuming
> we always use isa-pcspk.audiodev if it exists, then we'll trivially
> avoid using the -soundhw arg.
It's not that easy unfortunately. We have .audiodev for a few releases
already. But just setting that isn't enough to initialize pcspk in
current qemu, "-soundhw pcspk" is still needed ...
I'm looking at how to initialize onboard audio devices currently, maybe
the best way to handle that is to do it flash-style with machine
properties (i.e have a pc.pcslk alias for pcspk.audiodev, simliar to
pc.flash0 being an alias for pflash.drive). That'll be better that
-global and it'll also be visible in QOM.
Initialization order looks tricky though. I'd have to create pcspk
early, simliar to flash, in pc_machine_initfn(). Problem is I don't
have a isa bus yet at that point (flash is sysbus and doesn't have this
problem). I'm open to suggestions hiow do deal with that best.
take care,
Gerd
Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk
On a Monday in 2020, Daniel P. Berrangé wrote:
On Mon, May 18, 2020 at 12:16:28PM +0200, Gerd Hoffmann wrote:
On Fri, May 15, 2020 at 05:08:23PM +0200, Ján Tomko wrote:
> On a Friday in 2020, Gerd Hoffmann wrote:
> > Add deprecation message to the audio init function.
> >
> > Factor out audio initialization and call that from
> > both audio init and realize, so setting audiodev via
> > -global is enough to properly initialize pcspk.
> >
> > Signed-off-by: Gerd Hoffmann
> > ---
> > hw/audio/pcspk.c | 24 +---
> > 1 file changed, 21 insertions(+), 3 deletions(-)
> >
> > @@ -236,9 +245,18 @@ static const TypeInfo pcspk_info = {
> > .class_init = pcspk_class_initfn,
> > };
> >
> > +static int pcspk_audio_init_soundhw(ISABus *bus)
> > +{
> > +PCSpkState *s = pcspk_state;
> > +
> > +warn_report("'-soundhw pcspk' is deprecated, "
> > +"please set a backend using '-global isa-pcspk.audiodev='
instead");
> > +return pcspk_audio_init(s);
>
> -soundhw pcspk is the only soundhw device present in libvirt git.
>
> Is there a way to probe for this change via QMP?
Oops. I'm surprised libvirt actually supports pcspk.
There is no way to see that in qmp, and I can't think of an easy way
to add that. Does libvirt check for command line switches still?
So it could see -soundhw going away if that happens?
IIUC, instead of probing for whether -soundhw is deprecated, it should
be suffiicent for us to probe if "isa-pcspk.audiodev" exists. Assuming
we always use isa-pcspk.audiodev if it exists, then we'll trivially
avoid using the -soundhw arg.
Yes, we can probe for that, but the phrasing in the commit message makes
it look like setting the property via -global will only be effective
after this commit.
Jano
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
signature.asc
Description: PGP signature
Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk
On Mon, May 18, 2020 at 12:16:28PM +0200, Gerd Hoffmann wrote:
> On Fri, May 15, 2020 at 05:08:23PM +0200, Ján Tomko wrote:
> > On a Friday in 2020, Gerd Hoffmann wrote:
> > > Add deprecation message to the audio init function.
> > >
> > > Factor out audio initialization and call that from
> > > both audio init and realize, so setting audiodev via
> > > -global is enough to properly initialize pcspk.
> > >
> > > Signed-off-by: Gerd Hoffmann
> > > ---
> > > hw/audio/pcspk.c | 24 +---
> > > 1 file changed, 21 insertions(+), 3 deletions(-)
> > >
> > > @@ -236,9 +245,18 @@ static const TypeInfo pcspk_info = {
> > > .class_init = pcspk_class_initfn,
> > > };
> > >
> > > +static int pcspk_audio_init_soundhw(ISABus *bus)
> > > +{
> > > +PCSpkState *s = pcspk_state;
> > > +
> > > +warn_report("'-soundhw pcspk' is deprecated, "
> > > +"please set a backend using '-global
> > > isa-pcspk.audiodev=' instead");
> > > +return pcspk_audio_init(s);
> >
> > -soundhw pcspk is the only soundhw device present in libvirt git.
> >
> > Is there a way to probe for this change via QMP?
>
> Oops. I'm surprised libvirt actually supports pcspk.
>
> There is no way to see that in qmp, and I can't think of an easy way
> to add that. Does libvirt check for command line switches still?
> So it could see -soundhw going away if that happens?
IIUC, instead of probing for whether -soundhw is deprecated, it should
be suffiicent for us to probe if "isa-pcspk.audiodev" exists. Assuming
we always use isa-pcspk.audiodev if it exists, then we'll trivially
avoid using the -soundhw arg.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk
On Fri, May 15, 2020 at 05:08:23PM +0200, Ján Tomko wrote:
> On a Friday in 2020, Gerd Hoffmann wrote:
> > Add deprecation message to the audio init function.
> >
> > Factor out audio initialization and call that from
> > both audio init and realize, so setting audiodev via
> > -global is enough to properly initialize pcspk.
> >
> > Signed-off-by: Gerd Hoffmann
> > ---
> > hw/audio/pcspk.c | 24 +---
> > 1 file changed, 21 insertions(+), 3 deletions(-)
> >
> > @@ -236,9 +245,18 @@ static const TypeInfo pcspk_info = {
> > .class_init = pcspk_class_initfn,
> > };
> >
> > +static int pcspk_audio_init_soundhw(ISABus *bus)
> > +{
> > +PCSpkState *s = pcspk_state;
> > +
> > +warn_report("'-soundhw pcspk' is deprecated, "
> > +"please set a backend using '-global
> > isa-pcspk.audiodev=' instead");
> > +return pcspk_audio_init(s);
>
> -soundhw pcspk is the only soundhw device present in libvirt git.
>
> Is there a way to probe for this change via QMP?
Oops. I'm surprised libvirt actually supports pcspk.
There is no way to see that in qmp, and I can't think of an easy way
to add that. Does libvirt check for command line switches still?
So it could see -soundhw going away if that happens?
take care,
Gerd
Re: [PATCH v2 11/13] audio: deprecate -soundhw pcspk
On a Friday in 2020, Gerd Hoffmann wrote:
Add deprecation message to the audio init function.
Factor out audio initialization and call that from
both audio init and realize, so setting audiodev via
-global is enough to properly initialize pcspk.
Signed-off-by: Gerd Hoffmann
---
hw/audio/pcspk.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
@@ -236,9 +245,18 @@ static const TypeInfo pcspk_info = {
.class_init = pcspk_class_initfn,
};
+static int pcspk_audio_init_soundhw(ISABus *bus)
+{
+PCSpkState *s = pcspk_state;
+
+warn_report("'-soundhw pcspk' is deprecated, "
+"please set a backend using '-global isa-pcspk.audiodev='
instead");
+return pcspk_audio_init(s);
-soundhw pcspk is the only soundhw device present in libvirt git.
Is there a way to probe for this change via QMP?
Jano
+}
+
static void pcspk_register(void)
{
type_register_static(_info);
-isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init);
+isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
}
type_init(pcspk_register)
--
2.18.4
signature.asc
Description: PGP signature
[PATCH v2 11/13] audio: deprecate -soundhw pcspk
Add deprecation message to the audio init function.
Factor out audio initialization and call that from
both audio init and realize, so setting audiodev via
-global is enough to properly initialize pcspk.
Signed-off-by: Gerd Hoffmann
---
hw/audio/pcspk.c | 24 +---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index c37a3878612e..ab290e686783 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -28,6 +28,7 @@
#include "audio/audio.h"
#include "qemu/module.h"
#include "qemu/timer.h"
+#include "qemu/error-report.h"
#include "hw/timer/i8254.h"
#include "migration/vmstate.h"
#include "hw/audio/pcspk.h"
@@ -112,11 +113,15 @@ static void pcspk_callback(void *opaque, int free)
}
}
-static int pcspk_audio_init(ISABus *bus)
+static int pcspk_audio_init(PCSpkState *s)
{
-PCSpkState *s = pcspk_state;
struct audsettings as = {PCSPK_SAMPLE_RATE, 1, AUDIO_FORMAT_U8, 0};
+if (s->voice) {
+/* already initialized */
+return 0;
+}
+
AUD_register_card(s_spk, >card);
s->voice = AUD_open_out(>card, s->voice, s_spk, s, pcspk_callback, );
@@ -185,6 +190,10 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp)
isa_register_ioport(isadev, >ioport, s->iobase);
+if (s->card.state) {
+pcspk_audio_init(s);
+}
+
pcspk_state = s;
}
@@ -236,9 +245,18 @@ static const TypeInfo pcspk_info = {
.class_init = pcspk_class_initfn,
};
+static int pcspk_audio_init_soundhw(ISABus *bus)
+{
+PCSpkState *s = pcspk_state;
+
+warn_report("'-soundhw pcspk' is deprecated, "
+"please set a backend using '-global
isa-pcspk.audiodev=' instead");
+return pcspk_audio_init(s);
+}
+
static void pcspk_register(void)
{
type_register_static(_info);
-isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init);
+isa_register_soundhw("pcspk", "PC speaker", pcspk_audio_init_soundhw);
}
type_init(pcspk_register)
--
2.18.4
