qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 6/9] vt82c686: Support machine-default audiodev with fallback


From: BALATON Zoltan
Subject: Re: [PATCH 6/9] vt82c686: Support machine-default audiodev with fallback
Date: Sat, 23 Sep 2023 01:54:50 +0200 (CEST)

On Fri, 22 Sep 2023, Paolo Bonzini wrote:
On Fri, Sep 22, 2023 at 2:17 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("460exb");
    mc->default_ram_size = 512 * MiB;
    mc->default_ram_id = "ppc4xx.sdram";
+
+    machine_add_audiodev_property(mc);

This hunk has nothing to do with vt82686 so probably should be in
previoius patch. Also sam460ex embedded sound part is not emulated and can
only use PCI sound cards. What this line is needed for?

No, this line shouldn't be there.

If every machine
now needs this call, can it be added in some generic machine init func in
hw/core/machine.c instead?

It is not needed by every machine, only by every machine that has
embedded sound.

I'm not sure about this whole series. Looks like it gets rid of 71 lines
but adding 158 and makes the users' life harder by requireing them to
specify drivers they may not even know about. What's the point? Is there
still a default to use the normally used audiodev for the platform or
people will now get errors and no sound unless they change their command
lines?

I think you're right, I should have sent this series without the last
two patches.

The first seven add more functionality, because they let you use
-audiodev for configuration of embedded boards. This is why they add
some lines of code.

The last two patches instead are the ones that make -audio or
-audiodev mandatory. They should be separate and they may not be a
good idea without something like "-audio default". And if no "-audio
default" is added, there is more code that can go (for example the
--audio-drv-list option to configure and CONFIG_AUDIO_DRIVERS).

I still don't see the point, because it already works without these changes. With current master one can specify -audiodev for -M paegasos2 and it gives a warning but does the right thing and sets the audiodev for via-ac97. I think the warning can be avoided by using -global to set the via-ac97 audiodev property but since it picks up the -audiodev there's no need to. Apart from the warning this is convenient for the user, what's proposed in this series seems less so. What is the issue this series tries to solve?

Regards,
BALATON Zoltan

reply via email to

[Prev in Thread] Current Thread [Next in Thread]