qemu-devel
[Top][All Lists]
Advanced

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

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


From: BALATON Zoltan
Subject: Re: [PATCH v4 0/7] Pegasos2 fixes and audio output support
Date: Wed, 1 Mar 2023 19:17:38 +0100 (CET)

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

reply via email to

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