On behalve of Zoltan BALATON:

Hello,

This is marked v3 to avoid confusion with previously separate patches
that already had v2, even if the series had no v2 yet.

This series now includes all patches needed to get AmigaOS 4.1 run
well on pegasos2 and add audio output to this board. It has 3 parts:
patches 1-3 improve hw/display/sm501 model to avoid graphics problems
that were present with AmigaOS; patches 4-6 fix PCI interrupt routing
in VIA VT8231 model and in pegasos2 board that fixes PCI cards such as
network or sound card not working before; finally patches 7-8 add
basic implementation of the via-ac97 audio part of VT8231 (also used
on VT82C686B) for output that is the default audio device on pegasos2.

This version was re-tested by Rene Engel with latest AmigaOS version
and runs as well as my original series did (posted a video with that
before). This works now on an M1 MacStudio on macOS where it was
unusable before. I've also tested it on Linux x86_64 with older
AmigaOS version that also boots and makes sound and verified MorphOS
still boots and also has sound now.

One known problem with this version that includes Berhard's
alternative vt82c686 patches is with MorphOS which uses level
sensitive mode of the i8259 PIC that QEMU does not support so it hangs
when multiple devices try to raise a shared IRQ. I could work around
that in my otiginal series (see here:
https://lists.nongnu.org/archive/html/qemu-ppc/2023-02/msg00403.html )
where this works and was also tested, that version is available here:
https://osdn.net/projects/qmiga/scm/git/qemu/tree/pegasos2/
but I could not convince Bernhard so I now expect him to provide a
work around for that. This isn't a blocker though as MorphOS already
runs on mac99 and sam460ex and only available as a time limited demo
(they only sell licenses for real hardware) so not really usable apart
from testing anyway so getting it running on pegasos2 would be nice
but not a prioriey, more important is that AmigaOS runs for which this
is the only viable machine as sam460ex version runs much slower. So
I'd like this to be merged for 8.0 as it is now or only minor chnages
(or alternatively we can return to my series which was also tested the
same way and apart from different VIA IRQ router modelling contains
the same patches).

Please review and let me know who will take care of merging this for 8.0.

Regards,
BALATON Zoltan

v4:
* Rebased onto "[PATCH v3 0/3] VT82xx PCI IRQ routing fixes"

Based-on: <20230227123316.18719-1-shentey@gmail.com>
          "[PATCH v3 0/3] VT82xx PCI IRQ routing fixes"

BALATON Zoltan (7):
  hw/display/sm501: Implement more 2D raster operations
  hw/display/sm501: Add fallbacks to pixman routines
  hw/display/sm501: Add debug property to control pixman usage
  hw/isa/vt82c686: Declare gpio inputs so it can be connected in board
    code
  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.h           |  65 ++++++
 include/hw/isa/vt82c686.h |  25 +++
 hw/audio/ac97.c           |  43 +---
 hw/audio/via-ac97.c       | 455 +++++++++++++++++++++++++++++++++++++-
 hw/display/sm501.c        | 119 +++++++---
 hw/isa/vt82c686.c         |   3 +-
 hw/pci-host/mv64361.c     |   4 -
 hw/ppc/pegasos2.c         |  20 +-
 hw/audio/trace-events     |   6 +
 hw/isa/trace-events       |   1 +
 10 files changed, 662 insertions(+), 79 deletions(-)
 create mode 100644 hw/audio/ac97.h

-- 
2.39.2

On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> On behalve of Zoltan BALATON:

You don't have to do that and in fact please don't. I'll handle this 
series just reply to the one patch that needs update with only the changes 
that were asked by review.

Regards,
BALATON Zoltan

On Mon, Feb 27, 2023 at 2:00 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:

> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> > On behalve of Zoltan BALATON:
>
> You don't have to do that and in fact please don't. I'll handle this
> series just reply to the one patch that needs update with only the changes
> that were asked by review.
>
> Regards,
> BALATON Zoltan
>

Google seems to agree with me by not letting me send your patches :/

  Sent [PATCH v4 0/7] Pegasos2 fixes and audio output support
  Sent [PATCH v4 1/7] hw/display/sm501: Implement more 2D raster operations
  Sent [PATCH v4 2/7] hw/display/sm501: Add fallbacks to pixman routines
  Sent [PATCH v4 3/7] hw/display/sm501: Add debug property to control
pixman usage
  4.3.0 Mail server temporarily rejected message.
bk4-20020a170906b0c400b008d7a8083dffsm3186414ejb.222 - gsmtp

As said before I don't want to iterate on the changes of this series. I
can't test them and from my POV they seem unnecessary to me since all the
test cases I can perform still work without the IRQ changes.

Looking at the schematics I get the impression that the PCI IRQs actually
work the other way around: Instead of the INTx lines of the 2nd PCI bus
triggering both the north and the south bridge IRQ controllers, the two PCI
buses of the north bridge both trigger the VT82xx PCI IRQ router. I'm not a
hardware engineer so I could be totally off here. That's why I rather leave
my hands off of this stuff.

Best regards,
Bernhard

On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> On Mon, Feb 27, 2023 at 2:00 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>> On behalve of Zoltan BALATON:
>>
>> You don't have to do that and in fact please don't. I'll handle this
>> series just reply to the one patch that needs update with only the changes
>> that were asked by review.
>>
>> Regards,
>> BALATON Zoltan
>>
>
> Google seems to agree with me by not letting me send your patches :/
>
>  Sent [PATCH v4 0/7] Pegasos2 fixes and audio output support
>  Sent [PATCH v4 1/7] hw/display/sm501: Implement more 2D raster operations
>  Sent [PATCH v4 2/7] hw/display/sm501: Add fallbacks to pixman routines
>  Sent [PATCH v4 3/7] hw/display/sm501: Add debug property to control
> pixman usage
>  4.3.0 Mail server temporarily rejected message.
> bk4-20020a170906b0c400b008d7a8083dffsm3186414ejb.222 - gsmtp
>
> As said before I don't want to iterate on the changes of this series. I
> can't test them and from my POV they seem unnecessary to me since all the
> test cases I can perform still work without the IRQ changes.

Then why do you make me track your series and asking me to check if you 
broke anything in my patches during your rebase that I've asked you not 
to do? The series I've submitted (both my original and the one with your 
changes) were tested and made sure it worked with AmigaOS that you say you 
can't test.

> Looking at the schematics I get the impression that the PCI IRQs actually
> work the other way around: Instead of the INTx lines of the 2nd PCI bus
> triggering both the north and the south bridge IRQ controllers, the two PCI
> buses of the north bridge both trigger the VT82xx PCI IRQ router. I'm not a
> hardware engineer so I could be totally off here. That's why I rather leave
> my hands off of this stuff.

You don't seem to leave your hands off and got involved voluntarily so now 
don't run away :-)

I'm no hardware engineer either but in any case pci_set_irq cannot be used 
from a PCIDevice as I explained in the other message so your approach with 
that is clearly wrong and we need gpios that something else connects to 
the PCI bus. You could only do that if the VIA chip was a north bridge 
that had a PCI bus but it doesn't. In pegasos2 the north bridge is the 
MV64361 but the PCI interrupt lines are connected to its GPP (General 
purpose or multi purpose pins in docs that are just gpio lines, which are 
programmable inputs or outputs). These can generate an interrupt in the 
MV64361 but the VT8231 also has an ability to route PCI IRQs to ISA 
interrupts which some guests use. So I think the way I've modeled it is 
correct by connecting the PCI bus interrupt pins to both of these chips 
and since they are independent models the only place you can do it cleanly 
is the board code. Other boards may connect the VIA PIRQ pins differently 
but this model allows for that now. What is that you still don't like 
about it?

Regards,
BALATON Zoltan

On Mon, Feb 27, 2023 at 2:45 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:

> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> > On Mon, Feb 27, 2023 at 2:00 PM BALATON Zoltan <balaton@eik.bme.hu>
> wrote:
> >
> >> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> >>> On behalve of Zoltan BALATON:
> >>
> >> You don't have to do that and in fact please don't. I'll handle this
> >> series just reply to the one patch that needs update with only the
> changes
> >> that were asked by review.
> >>
> >> Regards,
> >> BALATON Zoltan
> >>
> >
> > Google seems to agree with me by not letting me send your patches :/
> >
> >  Sent [PATCH v4 0/7] Pegasos2 fixes and audio output support
> >  Sent [PATCH v4 1/7] hw/display/sm501: Implement more 2D raster
> operations
> >  Sent [PATCH v4 2/7] hw/display/sm501: Add fallbacks to pixman routines
> >  Sent [PATCH v4 3/7] hw/display/sm501: Add debug property to control
> > pixman usage
> >  4.3.0 Mail server temporarily rejected message.
> > bk4-20020a170906b0c400b008d7a8083dffsm3186414ejb.222 - gsmtp
> >
> > As said before I don't want to iterate on the changes of this series. I
> > can't test them and from my POV they seem unnecessary to me since all the
> > test cases I can perform still work without the IRQ changes.
>
> Then why do you make me track your series and asking me to check if you
> broke anything in my patches during your rebase that I've asked you not
> to do?


Because I couldn't convince you about the PCI IRQ router changes ;) I've
asked how to proceed and got the suggestion to post an alternative series.


> The series I've submitted (both my original and the one with your
> changes) were tested and made sure it worked with AmigaOS that you say you
> can't test.
>

Unfortunately my patches had changes merged in. This now makes it hard to
show what really changed (spoiler: nothing that affects behavior).

As you probably noticed in the "resend" version of this iteration I split
off a patch introducing the priq properties. It belongs to the sub series
of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want
to show up in `git blame` as the author of any of these changes. I
attributed it to you because this was really your change which I'm not even
sure is legal.

Let's avoid such complications by keeping our series separate.


> > Looking at the schematics I get the impression that the PCI IRQs actually
> > work the other way around: Instead of the INTx lines of the 2nd PCI bus
> > triggering both the north and the south bridge IRQ controllers, the two
> PCI
> > buses of the north bridge both trigger the VT82xx PCI IRQ router. I'm
> not a
> > hardware engineer so I could be totally off here. That's why I rather
> leave
> > my hands off of this stuff.
>
> You don't seem to leave your hands off and got involved voluntarily so now
> don't run away :-)
>

I'm happy to comment on it but please don't make me change anything there
for now. Feature freeze is approaching soon after all which in turn raises
the temperature for development.


> I'm no hardware engineer either but in any case pci_set_irq cannot be used
> from a PCIDevice as I explained in the other message so your approach with
> that is clearly wrong and we need gpios that something else connects to
> the PCI bus. You could only do that if the VIA chip was a north bridge
> that had a PCI bus but it doesn't. In pegasos2 the north bridge is the
> MV64361 but the PCI interrupt lines are connected to its GPP (General
> purpose or multi purpose pins in docs that are just gpio lines, which are
> programmable inputs or outputs). These can generate an interrupt in the
> MV64361 but the VT8231 also has an ability to route PCI IRQs to ISA
> interrupts which some guests use. So I think the way I've modeled it is
> correct by connecting the PCI bus interrupt pins to both of these chips
> and since they are independent models the only place you can do it cleanly
> is the board code. Other boards may connect the VIA PIRQ pins differently
> but this model allows for that now. What is that you still don't like
> about it?
>

On page 13 there is a note saying "Southbridge is INT controller". Another
note says "AGP uses A as first Int for none shared operation". These notes
describe hardware, so should apply to all guests.

Furthermore, I couldn't find any remotely useful documentation for the
MV64361 chip. This turns any discussion into guesswork.

Best regards,
Bernhard

>
> Regards,
> BALATON Zoltan

On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> Unfortunately my patches had changes merged in. This now makes it hard to
> show what really changed (spoiler: nothing that affects behavior).
>
> As you probably noticed in the "resend" version of this iteration I split
> off a patch introducing the priq properties. It belongs to the sub series
> of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want
> to show up in `git blame` as the author of any of these changes. I
> attributed it to you because this was really your change which I'm not even
> sure is legal.
>
> Let's avoid such complications by keeping our series separate.

Let's cool down a bit. Philippe took some of the sm501 patches in his 
giant pull request (and a lot of your patches too) now so I'll wait until 
that lands and hope to get some review for the remaining patches too. Once 
that pull req is merged I'll rebase the remaining patches and resubmit the 
series also adding changes for reasonable review comments I get by then.

I plan to include your patches as before and competely ignore your 
alternative series as that's just complicating things now. I also would 
like to add another patch to implement the same workaround for MorphOS 
that I had in my original series if I can figure thet out. This could be a 
separate patch though so it's easy to revert when the i8259 model is fixed 
in the future (if ever). If you don't want to be author of your patch with 
only changing pci_bus_irqs for gpio_named then I can add Suggested-by 
instead which would still show it was your idea but would not get 
attributed in git blame for it.

You can't convince me that using pci_bus_irqs is the right way for the 
reasons I've explained before. It's also only used by pci-hosts and boards 
everywhere else except in piix which you've added (and those may also be 
wrong then). It may still work if I fix that up in pegasos2 but still not 
the right way. I mean now:

mv64361 connects PCI interrupts to its gpp 12-15 pins within its model, 
this lacks the connections to vt8231 so ISA IRQs aren't raised for PCI 
devices such as -device rtl8139

after my changes:

pegasos2 board connects PCI interrupts to both gpp 12-15 of mv64371 and 
PINTA-D pins of VT8231 after it instantiates both which matches what the 
schematics say and also how QDev prefers to model chip pins.

with your series:

mv64361 connects it as above then VT8132 replaces it with its own. Then 
you want me to add another patch to fix that all up to arrive at the same 
result as above? That's just clearly wrong and unnecessary. I don't see 
why you don't accept the above solution?

Regards,
BALATON Zoltan

On 27/2/23 18:47, BALATON Zoltan wrote:
> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>> Unfortunately my patches had changes merged in. This now makes it hard to
>> show what really changed (spoiler: nothing that affects behavior).
>>
>> As you probably noticed in the "resend" version of this iteration I split
>> off a patch introducing the priq properties. It belongs to the sub series
>> of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want
>> to show up in `git blame` as the author of any of these changes. I
>> attributed it to you because this was really your change which I'm not 
>> even
>> sure is legal.
>>
>> Let's avoid such complications by keeping our series separate.
> 
> Let's cool down a bit. Philippe took some of the sm501 patches in his 
> giant pull request (and a lot of your patches too) now so I'll wait 
> until that lands and hope to get some review for the remaining patches 
> too. Once that pull req is merged I'll rebase the remaining patches and 
> resubmit the series also adding changes for reasonable review comments I 
> get by then.

I'm sorry it took me so long, I was expecting these patches to be picked
up by other maintainers but everybody is very busy. I know you'll need
to rebase and it might be painful. But I did that believing it is the
best I could give to the project with my current capacity. Also I don't
want to overlap (too much) into other (sub)maintained areas.
If you are stuck with a rebase, I can try to help.
We'll get to the end of PIIX, if this isn't this release, it will be
the next one. I've been waiting for these cleanups since 4 years
already, before Hervé Poussineau pushed toward that direction during
3 years. We always hit problems due to PC world inheritance which,
as a production system, can not regress.

Also I don't want to compete with you guys, I'd really love to work
together as team, but I have other responsibilities and sometime I
can't spend the time I'd like here.

What blocks the PIIX changes is the "q35/ich9/-device piix3" broken
config. I'll explain elsewhere why it is broken. The problem is I
don't know how to suggest alternative.

Bernhard sometimes it is easier to share visions in a 30min call,
rather than on a such thread. If you want I'm open for one.

On Mon, 27 Feb 2023, Philippe Mathieu-Daudé wrote:
> On 27/2/23 18:47, BALATON Zoltan wrote:
>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>> Unfortunately my patches had changes merged in. This now makes it hard to
>>> show what really changed (spoiler: nothing that affects behavior).
>>> 
>>> As you probably noticed in the "resend" version of this iteration I split
>>> off a patch introducing the priq properties. It belongs to the sub series
>>> of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want
>>> to show up in `git blame` as the author of any of these changes. I
>>> attributed it to you because this was really your change which I'm not 
>>> even
>>> sure is legal.
>>> 
>>> Let's avoid such complications by keeping our series separate.
>> 
>> Let's cool down a bit. Philippe took some of the sm501 patches in his giant 
>> pull request (and a lot of your patches too) now so I'll wait until that 
>> lands and hope to get some review for the remaining patches too. Once that 
>> pull req is merged I'll rebase the remaining patches and resubmit the 
>> series also adding changes for reasonable review comments I get by then.
>
> I'm sorry it took me so long, I was expecting these patches to be picked
> up by other maintainers but everybody is very busy. I know you'll need

You have no reason to apologise really, you did a great job merging all 
the patches. I was thinking that because as you say every maintainer is 
very busy now and we also had CI outage for a few weeks should we consider 
extending the date until the freeze by one or two weeks? That would allow 
people to relax a bit and be able to consolidate and merge all still 
pending patches. Postponing the 8.0 release one or two weeks is probably 
better than missing a lot of changes until the next release in September. 
We'd still aim for the original freeze date but if we fail to meet that it 
would be more convenient to know there could be a possibility for 
extending it. But make it clear that this is only for this one time 
because of CI outage and additional maintainer load caused by that so not 
something that should be done regularly but under current circumstances I 
would consider it.

Regards,
BALATON Zoltan

On Tue, Feb 28, 2023 at 04:05:30PM +0100, BALATON Zoltan wrote:
> On Mon, 27 Feb 2023, Philippe Mathieu-Daudé wrote:
> > On 27/2/23 18:47, BALATON Zoltan wrote:
> > > On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> > > > Unfortunately my patches had changes merged in. This now makes it hard to
> > > > show what really changed (spoiler: nothing that affects behavior).
> > > > 
> > > > As you probably noticed in the "resend" version of this iteration I split
> > > > off a patch introducing the priq properties. It belongs to the sub series
> > > > of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want
> > > > to show up in `git blame` as the author of any of these changes. I
> > > > attributed it to you because this was really your change which
> > > > I'm not even
> > > > sure is legal.
> > > > 
> > > > Let's avoid such complications by keeping our series separate.
> > > 
> > > Let's cool down a bit. Philippe took some of the sm501 patches in
> > > his giant pull request (and a lot of your patches too) now so I'll
> > > wait until that lands and hope to get some review for the remaining
> > > patches too. Once that pull req is merged I'll rebase the remaining
> > > patches and resubmit the series also adding changes for reasonable
> > > review comments I get by then.
> > 
> > I'm sorry it took me so long, I was expecting these patches to be picked
> > up by other maintainers but everybody is very busy. I know you'll need
> 
> You have no reason to apologise really, you did a great job merging all the
> patches. I was thinking that because as you say every maintainer is very
> busy now and we also had CI outage for a few weeks should we consider
> extending the date until the freeze by one or two weeks? That would allow
> people to relax a bit and be able to consolidate and merge all still pending
> patches. Postponing the 8.0 release one or two weeks is probably better than
> missing a lot of changes until the next release in September. We'd still aim
> for the original freeze date but if we fail to meet that it would be more
> convenient to know there could be a possibility for extending it. But make
> it clear that this is only for this one time because of CI outage and
> additional maintainer load caused by that so not something that should be
> done regularly but under current circumstances I would consider it.

There's no need to change the release schedule IMHO. Subsystem maintainers
should continue to send pull requests as normal. Peter is still processing
PRs, albeit at a lower rate with adhoc CI. From the soft freeze POV what
matters is just that the PRs are posted on the mailing list before the
deadline. If they're posted in time, they're still valid for inclusion in
the release. Our CI allowance is reset at the end of today anyway.

With 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 :|

On Tue, 28 Feb 2023, Daniel P. Berrangé wrote:
> On Tue, Feb 28, 2023 at 04:05:30PM +0100, BALATON Zoltan wrote:
>> On Mon, 27 Feb 2023, Philippe Mathieu-Daudé wrote:
>>> On 27/2/23 18:47, BALATON Zoltan wrote:
>>>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>>>> Unfortunately my patches had changes merged in. This now makes it hard to
>>>>> show what really changed (spoiler: nothing that affects behavior).
>>>>>
>>>>> As you probably noticed in the "resend" version of this iteration I split
>>>>> off a patch introducing the priq properties. It belongs to the sub series
>>>>> of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want
>>>>> to show up in `git blame` as the author of any of these changes. I
>>>>> attributed it to you because this was really your change which
>>>>> I'm not even
>>>>> sure is legal.
>>>>>
>>>>> Let's avoid such complications by keeping our series separate.
>>>>
>>>> Let's cool down a bit. Philippe took some of the sm501 patches in
>>>> his giant pull request (and a lot of your patches too) now so I'll
>>>> wait until that lands and hope to get some review for the remaining
>>>> patches too. Once that pull req is merged I'll rebase the remaining
>>>> patches and resubmit the series also adding changes for reasonable
>>>> review comments I get by then.
>>>
>>> I'm sorry it took me so long, I was expecting these patches to be picked
>>> up by other maintainers but everybody is very busy. I know you'll need
>>
>> You have no reason to apologise really, you did a great job merging all the
>> patches. I was thinking that because as you say every maintainer is very
>> busy now and we also had CI outage for a few weeks should we consider
>> extending the date until the freeze by one or two weeks? That would allow
>> people to relax a bit and be able to consolidate and merge all still pending
>> patches. Postponing the 8.0 release one or two weeks is probably better than
>> missing a lot of changes until the next release in September. We'd still aim
>> for the original freeze date but if we fail to meet that it would be more
>> convenient to know there could be a possibility for extending it. But make
>> it clear that this is only for this one time because of CI outage and
>> additional maintainer load caused by that so not something that should be
>> done regularly but under current circumstances I would consider it.
>
> There's no need to change the release schedule IMHO. Subsystem maintainers
> should continue to send pull requests as normal. Peter is still processing
> PRs, albeit at a lower rate with adhoc CI. From the soft freeze POV what
> matters is just that the PRs are posted on the mailing list before the
> deadline. If they're posted in time, they're still valid for inclusion in
> the release. Our CI allowance is reset at the end of today anyway.

Problem is that some patches will need to be rebased on still pending pull 
request and also may need the maintainer's attention to review them and 
send the final pull despite series have been on the list few weeks before 
the freeze that should be in time. So this will mean we might still have 
some pending patches end of this week and if somebody asks for last minute 
changes then it will be hard to meet the deadline. So at least one week 
extension seems to resolve the pressure this causes and also give 
maintainers some time to catch up. It's better than just saying "Sorry it 
was out fault but you've still missed the release, see you next time."

Regards,
BALATON Zoltan

Am 27. Februar 2023 22:12:46 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 27/2/23 18:47, BALATON Zoltan wrote:
>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>> Unfortunately my patches had changes merged in. This now makes it hard to
>>> show what really changed (spoiler: nothing that affects behavior).
>>> 
>>> As you probably noticed in the "resend" version of this iteration I split
>>> off a patch introducing the priq properties. It belongs to the sub series
>>> of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want
>>> to show up in `git blame` as the author of any of these changes. I
>>> attributed it to you because this was really your change which I'm not even
>>> sure is legal.
>>> 
>>> Let's avoid such complications by keeping our series separate.
>> 
>> Let's cool down a bit. Philippe took some of the sm501 patches in his giant pull request (and a lot of your patches too) now so I'll wait until that lands and hope to get some review for the remaining patches too. Once that pull req is merged I'll rebase the remaining patches and resubmit the series also adding changes for reasonable review comments I get by then.
>
>I'm sorry it took me so long, I was expecting these patches to be picked
>up by other maintainers but everybody is very busy. I know you'll need
>to rebase and it might be painful. But I did that believing it is the
>best I could give to the project with my current capacity. Also I don't
>want to overlap (too much) into other (sub)maintained areas.
>If you are stuck with a rebase, I can try to help.
>We'll get to the end of PIIX, if this isn't this release, it will be
>the next one. I've been waiting for these cleanups since 4 years
>already, before Hervé Poussineau pushed toward that direction during
>3 years. We always hit problems due to PC world inheritance which,
>as a production system, can not regress.

Did you intend to reply in the PIIX consolidation or global ISA bus renoval series? This is a VT82xx thread...

Thanks for picking up the ICH9 changes!

>
>Also I don't want to compete with you guys, I'd really love to work
>together as team, but I have other responsibilities and sometime I
>can't spend the time I'd like here.
>
>What blocks the PIIX changes is the "q35/ich9/-device piix3" broken
>config. I'll explain elsewhere why it is broken. The problem is I
>don't know how to suggest alternative.
>
>Bernhard sometimes it is easier to share visions in a 30min call,
>rather than on a such thread. If you want I'm open for one.
>

Sure, I'm open for that. I'll let you know when I'll have some time.

Best regards,
Bernhard

On Mon, 27 Feb 2023, Bernhard Beschow wrote:
> On Mon, Feb 27, 2023 at 2:45 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>> On Mon, Feb 27, 2023 at 2:00 PM BALATON Zoltan <balaton@eik.bme.hu>
>> wrote:
>>>
>>>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>>>> On behalve of Zoltan BALATON:
>>>>
>>>> You don't have to do that and in fact please don't. I'll handle this
>>>> series just reply to the one patch that needs update with only the
>> changes
>>>> that were asked by review.
>>>>
>>>> Regards,
>>>> BALATON Zoltan
>>>>
>>>
>>> Google seems to agree with me by not letting me send your patches :/
>>>
>>>  Sent [PATCH v4 0/7] Pegasos2 fixes and audio output support
>>>  Sent [PATCH v4 1/7] hw/display/sm501: Implement more 2D raster
>> operations
>>>  Sent [PATCH v4 2/7] hw/display/sm501: Add fallbacks to pixman routines
>>>  Sent [PATCH v4 3/7] hw/display/sm501: Add debug property to control
>>> pixman usage
>>>  4.3.0 Mail server temporarily rejected message.
>>> bk4-20020a170906b0c400b008d7a8083dffsm3186414ejb.222 - gsmtp
>>>
>>> As said before I don't want to iterate on the changes of this series. I
>>> can't test them and from my POV they seem unnecessary to me since all the
>>> test cases I can perform still work without the IRQ changes.
>>
>> Then why do you make me track your series and asking me to check if you
>> broke anything in my patches during your rebase that I've asked you not
>> to do?
>
>
> Because I couldn't convince you about the PCI IRQ router changes ;) I've
> asked how to proceed and got the suggestion to post an alternative series.

That's fine and you did that and I got your changes incorporated in my 
series and had that tested again and then submitted as one series as asked 
by the maintainer to keep all this togetner. Then you come back willing to 
split this series again, reposting some untested changes that I have no 
idea what did you change. This isn't very friendly before a freeze so 
please stop doing that and keep this in one series otherwise we get lost 
between all the changes.

>> The series I've submitted (both my original and the one with your
>> changes) were tested and made sure it worked with AmigaOS that you say you
>> can't test.
>>
>
> Unfortunately my patches had changes merged in. This now makes it hard to
> show what really changed (spoiler: nothing that affects behavior).

The two changes I've made and noted in the commit message were:

1. removing *empty* lines from a switch that only made it half as long and 
easier to read without any change in content.

2. instead of changing the interrupts of the PCI bus this device is 
connected to with pci_bus_irqs(), I export these as gpio pins to model the 
real chip which is then connected in the board as in real hadware.

> As you probably noticed in the "resend" version of this iteration I split
> off a patch introducing the priq properties. It belongs to the sub series
> of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want
> to show up in `git blame` as the author of any of these changes. I
> attributed it to you because this was really your change which I'm not even
> sure is legal.
>
> Let's avoid such complications by keeping our series separate.

Yout series is wrong because of the pci_bus_irqs (did you check PCI cards 
such as adding one with -device still work on fuloong2e with your patch?) 
I've corrected in my series so that it also fits in with my changes. If 
you don't like this change then we can drop your series and go back to 
mine instead or make the patch Suggested-by you and I take the From: or 
just keep out of this but please decide what you want and dont make it 
unnecessarily difficult to review and merge this series.

>>> Looking at the schematics I get the impression that the PCI IRQs actually
>>> work the other way around: Instead of the INTx lines of the 2nd PCI bus
>>> triggering both the north and the south bridge IRQ controllers, the two
>> PCI
>>> buses of the north bridge both trigger the VT82xx PCI IRQ router. I'm
>> not a
>>> hardware engineer so I could be totally off here. That's why I rather
>> leave
>>> my hands off of this stuff.
>>
>> You don't seem to leave your hands off and got involved voluntarily so now
>> don't run away :-)
>>
>
> I'm happy to comment on it but please don't make me change anything there
> for now. Feature freeze is approaching soon after all which in turn raises
> the temperature for development.

I can just say the same to you. This was my series that you changed so 
don't ask me to not change it. I've changed it as you proposed despite I'm 
not buting that's the right way and had it re-tested but it's still not 
good for you?

>> I'm no hardware engineer either but in any case pci_set_irq cannot be used
>> from a PCIDevice as I explained in the other message so your approach with
>> that is clearly wrong and we need gpios that something else connects to
>> the PCI bus. You could only do that if the VIA chip was a north bridge
>> that had a PCI bus but it doesn't. In pegasos2 the north bridge is the
>> MV64361 but the PCI interrupt lines are connected to its GPP (General
>> purpose or multi purpose pins in docs that are just gpio lines, which are
>> programmable inputs or outputs). These can generate an interrupt in the
>> MV64361 but the VT8231 also has an ability to route PCI IRQs to ISA
>> interrupts which some guests use. So I think the way I've modeled it is
>> correct by connecting the PCI bus interrupt pins to both of these chips
>> and since they are independent models the only place you can do it cleanly
>> is the board code. Other boards may connect the VIA PIRQ pins differently
>> but this model allows for that now. What is that you still don't like
>> about it?
>>
>
> On page 13 there is a note saying "Southbridge is INT controller". Another
> note says "AGP uses A as first Int for none shared operation". These notes
> describe hardware, so should apply to all guests.
>
> Furthermore, I couldn't find any remotely useful documentation for the
> MV64361 chip. This turns any discussion into guesswork.

Maybe check here: qmiga.osdn.io there are some hints how to find some 
docs. Can't link then due to copyright reasons.

Regards,
BALATON Zoltan

On 27/02/2023 16:58, BALATON Zoltan wrote:

> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>> On Mon, Feb 27, 2023 at 2:45 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>>> On Mon, Feb 27, 2023 at 2:00 PM BALATON Zoltan <balaton@eik.bme.hu>
>>> wrote:
>>>>
>>>>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>>>>> On behalve of Zoltan BALATON:
>>>>>
>>>>> You don't have to do that and in fact please don't. I'll handle this
>>>>> series just reply to the one patch that needs update with only the
>>> changes
>>>>> that were asked by review.
>>>>>
>>>>> Regards,
>>>>> BALATON Zoltan
>>>>>
>>>>
>>>> Google seems to agree with me by not letting me send your patches :/
>>>>
>>>>  Sent [PATCH v4 0/7] Pegasos2 fixes and audio output support
>>>>  Sent [PATCH v4 1/7] hw/display/sm501: Implement more 2D raster
>>> operations
>>>>  Sent [PATCH v4 2/7] hw/display/sm501: Add fallbacks to pixman routines
>>>>  Sent [PATCH v4 3/7] hw/display/sm501: Add debug property to control
>>>> pixman usage
>>>>  4.3.0 Mail server temporarily rejected message.
>>>> bk4-20020a170906b0c400b008d7a8083dffsm3186414ejb.222 - gsmtp
>>>>
>>>> As said before I don't want to iterate on the changes of this series. I
>>>> can't test them and from my POV they seem unnecessary to me since all the
>>>> test cases I can perform still work without the IRQ changes.
>>>
>>> Then why do you make me track your series and asking me to check if you
>>> broke anything in my patches during your rebase that I've asked you not
>>> to do?
>>
>>
>> Because I couldn't convince you about the PCI IRQ router changes ;) I've
>> asked how to proceed and got the suggestion to post an alternative series.
> 
> That's fine and you did that and I got your changes incorporated in my series and had 
> that tested again and then submitted as one series as asked by the maintainer to keep 
> all this togetner. Then you come back willing to split this series again, reposting 
> some untested changes that I have no idea what did you change. This isn't very 
> friendly before a freeze so please stop doing that and keep this in one series 
> otherwise we get lost between all the changes.
> 
>>> The series I've submitted (both my original and the one with your
>>> changes) were tested and made sure it worked with AmigaOS that you say you
>>> can't test.
>>>
>>
>> Unfortunately my patches had changes merged in. This now makes it hard to
>> show what really changed (spoiler: nothing that affects behavior).
> 
> The two changes I've made and noted in the commit message were:
> 
> 1. removing *empty* lines from a switch that only made it half as long and easier to 
> read without any change in content.
> 
> 2. instead of changing the interrupts of the PCI bus this device is connected to with 
> pci_bus_irqs(), I export these as gpio pins to model the real chip which is then 
> connected in the board as in real hadware.
> 
>> As you probably noticed in the "resend" version of this iteration I split
>> off a patch introducing the priq properties. It belongs to the sub series
>> of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want
>> to show up in `git blame` as the author of any of these changes. I
>> attributed it to you because this was really your change which I'm not even
>> sure is legal.
>>
>> Let's avoid such complications by keeping our series separate.
> 
> Yout series is wrong because of the pci_bus_irqs (did you check PCI cards such as 
> adding one with -device still work on fuloong2e with your patch?) I've corrected in 
> my series so that it also fits in with my changes. If you don't like this change then 
> we can drop your series and go back to mine instead or make the patch Suggested-by 
> you and I take the From: or just keep out of this but please decide what you want and 
> dont make it unnecessarily difficult to review and merge this series.
> 
>>>> Looking at the schematics I get the impression that the PCI IRQs actually
>>>> work the other way around: Instead of the INTx lines of the 2nd PCI bus
>>>> triggering both the north and the south bridge IRQ controllers, the two
>>> PCI
>>>> buses of the north bridge both trigger the VT82xx PCI IRQ router. I'm
>>> not a
>>>> hardware engineer so I could be totally off here. That's why I rather
>>> leave
>>>> my hands off of this stuff.
>>>
>>> You don't seem to leave your hands off and got involved voluntarily so now
>>> don't run away :-)
>>>
>>
>> I'm happy to comment on it but please don't make me change anything there
>> for now. Feature freeze is approaching soon after all which in turn raises
>> the temperature for development.
> 
> I can just say the same to you. This was my series that you changed so don't ask me 
> to not change it. I've changed it as you proposed despite I'm not buting that's the 
> right way and had it re-tested but it's still not good for you?
> 
>>> I'm no hardware engineer either but in any case pci_set_irq cannot be used
>>> from a PCIDevice as I explained in the other message so your approach with
>>> that is clearly wrong and we need gpios that something else connects to
>>> the PCI bus. You could only do that if the VIA chip was a north bridge
>>> that had a PCI bus but it doesn't. In pegasos2 the north bridge is the
>>> MV64361 but the PCI interrupt lines are connected to its GPP (General
>>> purpose or multi purpose pins in docs that are just gpio lines, which are
>>> programmable inputs or outputs). These can generate an interrupt in the
>>> MV64361 but the VT8231 also has an ability to route PCI IRQs to ISA
>>> interrupts which some guests use. So I think the way I've modeled it is
>>> correct by connecting the PCI bus interrupt pins to both of these chips
>>> and since they are independent models the only place you can do it cleanly
>>> is the board code. Other boards may connect the VIA PIRQ pins differently
>>> but this model allows for that now. What is that you still don't like
>>> about it?
>>>
>>
>> On page 13 there is a note saying "Southbridge is INT controller". Another
>> note says "AGP uses A as first Int for none shared operation". These notes
>> describe hardware, so should apply to all guests.
>>
>> Furthermore, I couldn't find any remotely useful documentation for the
>> MV64361 chip. This turns any discussion into guesswork.
> 
> Maybe check here: qmiga.osdn.io there are some hints how to find some docs. Can't 
> link then due to copyright reasons.

I tried using the recommended searches, however all I can find are PDFs of the 
product summary. Can you provide some more hints, such as a filename for example?


ATB,

Mark.

On Wed, 1 Mar 2023, Mark Cave-Ayland wrote:
> On 27/02/2023 16:58, BALATON Zoltan wrote:
>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>> On Mon, Feb 27, 2023 at 2:45 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>>>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>>>> On Mon, Feb 27, 2023 at 2:00 PM BALATON Zoltan <balaton@eik.bme.hu>
>>>> wrote:
>>>>> 
>>>>>> On Mon, 27 Feb 2023, Bernhard Beschow wrote:
>>>>>>> On behalve of Zoltan BALATON:
>>>>>> 
>>>>>> You don't have to do that and in fact please don't. I'll handle this
>>>>>> series just reply to the one patch that needs update with only the
>>>> changes
>>>>>> that were asked by review.
>>>>>> 
>>>>>> Regards,
>>>>>> BALATON Zoltan
>>>>>> 
>>>>> 
>>>>> Google seems to agree with me by not letting me send your patches :/
>>>>> 
>>>>>  Sent [PATCH v4 0/7] Pegasos2 fixes and audio output support
>>>>>  Sent [PATCH v4 1/7] hw/display/sm501: Implement more 2D raster
>>>> operations
>>>>>  Sent [PATCH v4 2/7] hw/display/sm501: Add fallbacks to pixman routines
>>>>>  Sent [PATCH v4 3/7] hw/display/sm501: Add debug property to control
>>>>> pixman usage
>>>>>  4.3.0 Mail server temporarily rejected message.
>>>>> bk4-20020a170906b0c400b008d7a8083dffsm3186414ejb.222 - gsmtp
>>>>> 
>>>>> As said before I don't want to iterate on the changes of this series. I
>>>>> can't test them and from my POV they seem unnecessary to me since all 
>>>>> the
>>>>> test cases I can perform still work without the IRQ changes.
>>>> 
>>>> Then why do you make me track your series and asking me to check if you
>>>> broke anything in my patches during your rebase that I've asked you not
>>>> to do?
>>> 
>>> 
>>> Because I couldn't convince you about the PCI IRQ router changes ;) I've
>>> asked how to proceed and got the suggestion to post an alternative series.
>> 
>> That's fine and you did that and I got your changes incorporated in my 
>> series and had that tested again and then submitted as one series as asked 
>> by the maintainer to keep all this togetner. Then you come back willing to 
>> split this series again, reposting some untested changes that I have no 
>> idea what did you change. This isn't very friendly before a freeze so 
>> please stop doing that and keep this in one series otherwise we get lost 
>> between all the changes.
>> 
>>>> The series I've submitted (both my original and the one with your
>>>> changes) were tested and made sure it worked with AmigaOS that you say 
>>>> you
>>>> can't test.
>>>> 
>>> 
>>> Unfortunately my patches had changes merged in. This now makes it hard to
>>> show what really changed (spoiler: nothing that affects behavior).
>> 
>> The two changes I've made and noted in the commit message were:
>> 
>> 1. removing *empty* lines from a switch that only made it half as long and 
>> easier to read without any change in content.
>> 
>> 2. instead of changing the interrupts of the PCI bus this device is 
>> connected to with pci_bus_irqs(), I export these as gpio pins to model the 
>> real chip which is then connected in the board as in real hadware.
>> 
>>> As you probably noticed in the "resend" version of this iteration I split
>>> off a patch introducing the priq properties. It belongs to the sub series
>>> of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want
>>> to show up in `git blame` as the author of any of these changes. I
>>> attributed it to you because this was really your change which I'm not 
>>> even
>>> sure is legal.
>>> 
>>> Let's avoid such complications by keeping our series separate.
>> 
>> Yout series is wrong because of the pci_bus_irqs (did you check PCI cards 
>> such as adding one with -device still work on fuloong2e with your patch?) 
>> I've corrected in my series so that it also fits in with my changes. If you 
>> don't like this change then we can drop your series and go back to mine 
>> instead or make the patch Suggested-by you and I take the From: or just 
>> keep out of this but please decide what you want and dont make it 
>> unnecessarily difficult to review and merge this series.
>> 
>>>>> Looking at the schematics I get the impression that the PCI IRQs 
>>>>> actually
>>>>> work the other way around: Instead of the INTx lines of the 2nd PCI bus
>>>>> triggering both the north and the south bridge IRQ controllers, the two
>>>> PCI
>>>>> buses of the north bridge both trigger the VT82xx PCI IRQ router. I'm
>>>> not a
>>>>> hardware engineer so I could be totally off here. That's why I rather
>>>> leave
>>>>> my hands off of this stuff.
>>>> 
>>>> You don't seem to leave your hands off and got involved voluntarily so 
>>>> now
>>>> don't run away :-)
>>>> 
>>> 
>>> I'm happy to comment on it but please don't make me change anything there
>>> for now. Feature freeze is approaching soon after all which in turn raises
>>> the temperature for development.
>> 
>> I can just say the same to you. This was my series that you changed so 
>> don't ask me to not change it. I've changed it as you proposed despite I'm 
>> not buting that's the right way and had it re-tested but it's still not 
>> good for you?
>> 
>>>> I'm no hardware engineer either but in any case pci_set_irq cannot be 
>>>> used
>>>> from a PCIDevice as I explained in the other message so your approach 
>>>> with
>>>> that is clearly wrong and we need gpios that something else connects to
>>>> the PCI bus. You could only do that if the VIA chip was a north bridge
>>>> that had a PCI bus but it doesn't. In pegasos2 the north bridge is the
>>>> MV64361 but the PCI interrupt lines are connected to its GPP (General
>>>> purpose or multi purpose pins in docs that are just gpio lines, which are
>>>> programmable inputs or outputs). These can generate an interrupt in the
>>>> MV64361 but the VT8231 also has an ability to route PCI IRQs to ISA
>>>> interrupts which some guests use. So I think the way I've modeled it is
>>>> correct by connecting the PCI bus interrupt pins to both of these chips
>>>> and since they are independent models the only place you can do it 
>>>> cleanly
>>>> is the board code. Other boards may connect the VIA PIRQ pins differently
>>>> but this model allows for that now. What is that you still don't like
>>>> about it?
>>>> 
>>> 
>>> On page 13 there is a note saying "Southbridge is INT controller". Another
>>> note says "AGP uses A as first Int for none shared operation". These notes
>>> describe hardware, so should apply to all guests.
>>> 
>>> Furthermore, I couldn't find any remotely useful documentation for the
>>> MV64361 chip. This turns any discussion into guesswork.
>> 
>> Maybe check here: qmiga.osdn.io there are some hints how to find some docs. 
>> Can't link then due to copyright reasons.
>
> I tried using the recommended searches, however all I can find are PDFs of 
> the product summary. Can you provide some more hints, such as a filename for 
> example?

Maybe these are not available any more but it's a family of chips called 
MV64360, MB64361 and MV64362 (or Marvell Discovery II) with the different 
numbers are the same chip with different options such as number of PCI 
buses or ethernet controllers so the datasheet refers to them as 
MV64360/1/2 but maybe it's under MV64360. Hope this helps.

Regards,
BALATON Zoltan