Re: [PATCH 0/5] Pegasos2 fixes and audio output support

[Bernhard Beschow]

Am 22. Februar 2023 19:25:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Wed, 22 Feb 2023, Bernhard Beschow wrote:
>> On Wed, Feb 22, 2023 at 4:38 PM Bernhard Beschow <shentey@gmail.com> wrote:
>>> On Tue, Feb 21, 2023 at 7:44 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>>> This series fixes PCI interrupts on the ppc/pegasos2 machine and adds
>>>> partial implementation of the via-ac97 sound part enough to get audio
>>>> output. I'd like this to be merged for QEMU 8.0.
>>>>
>>>> Regards,
>>>> BALATON Zoltan
>>>>
>>>> BALATON Zoltan (5):
>>>>   hw/isa/vt82c686: Implement interrupt routing in via_isa_set_irq
>>>>   hw/isa/vt82c686: Implement PIRQ pins
>>>>   hw/ppc/pegasos2: Fix PCI interrupt routing
>>>>   hw/audio/ac97: Split off some definitions to a header
>>>>   hw/audio/via-ac97: Basic implementation of audio playback
>>>>
>>>>  hw/audio/ac97.c            |  43 +---
>>>>  hw/audio/ac97.h            |  65 ++++++
>>>>  hw/audio/trace-events      |   6 +
>>>>  hw/audio/via-ac97.c        | 436 ++++++++++++++++++++++++++++++++++++-
>>>>  hw/ide/via.c               |   2 +-
>>>>  hw/isa/vt82c686.c          |  61 +++++-
>>>>  hw/pci-host/mv64361.c      |   4 -
>>>>  hw/ppc/pegasos2.c          |  26 ++-
>>>>  hw/usb/vt82c686-uhci-pci.c |   5 +-
>>>>  include/hw/isa/vt82c686.h  |  39 +++-
>>>>  10 files changed, 626 insertions(+), 61 deletions(-)
>>>>  create mode 100644 hw/audio/ac97.h
>>>>
>>>> --
>>>> 2.30.7
>>>>
>>>>
>>> Wow, the MorphOS people paid attention to sound design. Thanks for
>>> presenting it to us, Zoltan!
>>>
>>> I've had a closer look at your series and I think it can be simplified:
>>> Patch 2 can be implemented quite straight-forward like I proposed in a
>>> private mail: https://github.com/shentok/qemu/commit/via-priq-routing.
>>> Then, in order to make patch 3 "hw/ppc/pegasos2: Fix PCI interrupt routing"
>>> working, one can expose the PCI interrupts with a single line like you do
>>> in patch 2. With this, patch 1 "hw/isa/vt82c686: Implement interrupt
>>> routing in via_isa_set_irq" isn't needed any longer and can be omitted.
>>>
>>> In via-ac97, rather than using via_isa_set_irq(), pci_set_irq() can be
>>> used instead. pci_set_irq() internally takes care of all the ISA interrupt
>>> level tracking patch 1 attempted to address.
>>>
>>
>> Here is a proof of concept branch to demonstrate that the simplification
>> actually works: https://github.com/shentok/qemu/commits/pegasos2 (Tested
>> with MorphOS with and without pegasos2.rom).
>
>Does this only work because both the via-ac97 and the PCI interrupts are 
>mapped to the same ISA IRQ and you've only tested sound? The guest could 
>configure each device to use a different IRQ, also mapping them so they share 
>one ISA interrupt. What happens if multiple devices are mapped to IRQ 9 (which 
>is the case on pegasos2 where PCI cards, ac97 and USB all share this IRQ) and 
>more than one such device wants to raise an interrupt at the same time? If you 
>ack the ac97 interrupt but a PCI network card or the USB part still wants to 
>get the CPUs attention the ISA IRQ should remain raised until all devices are 
>serviced.

pci_bus_get_irq_level(), used in via_isa_set_pci_irq(), should handle
exactly that case very well.

>I don't see a way to track the status of all devices in a single qemu_irq 
>which can only be up or down so we need something to store the state of each 
>source.

pci_set_irq() causes pci_bus_change_irq_level() to be called.
pci_bus_change_irq_level() tracks the sum of all irq levels of all
devices attached to a particular pin in irq_count. Have a look at
pci_bus_change_irq_level() and you will understand better.

>My patch adds a state register to each ISA IRQ line for all possible sources 
>which could probably be stored once but then for each change of ISA IRQ status 
>all the mapped devices should be checked and combined so it's easier to store 
>them for each IRQ. Does your approach still work if you play sound, and copy 
>something from network to a USB device at the same time? (I'm not sure mine 
>does not have remaining bugs but I don't think this can be simplified that way 
>but if you can prove it would work I don't mind taking an alternative version 
>but I'm not convinced yet.)

Well, I can't prove that my approach works but unfortunately I can
prove that both our approaches cause a freeze :/ Try:
1. Start `qemu-system-ppc -M pegasos2 -bios pegasos2.rom -rtc
base=localtime -device ati-vga,guest_hwcursor=true,romfile="" -cdrom
morphos-3.17.iso -device usb-mouse -device usb-kbd`
2. Move the mouse while sound is playing
-> Observe the VM to freeze

So there must be an issue somewhere else...

Best regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>>> I might have further comments but I think it's enough for now.
>>>
>>> Thanks again for making via-ac97 work!
>>>
>>> Best regards,
>>> Bernhard
>>>
>>