| Message ID | cover.1530047900.git.balaton@eik.bme.hu |
|---|---|
| Headers | show |
| Series | Misc sm501 improvements | expand |
On Tue, 26 Jun 2018, BALATON Zoltan wrote: > Version 2 of the sm501 changes with fixes that are needed to get > AmigaOS 4.1FE to boot and able to produce graphics. > > The strange blue-white colors that first appear are actually correct > and because of AmigaOS selecting a low resolution PAL mode by default > instead of a board specific mode. To work around this one can select > the last option to boot the live CD and then select a better board > specific mode from ScreenMode Prefs. It takes a while for the > ScreenMode window to appear and graphics operations are slow which > could use some improvement but at least it seems to work correctly now > apart from some unimplemented drawing modes for compositing. > > If this could be merged before the freeze with the sam460ex patches > and Sebastian's ehci patch then QEMU 3.0 could be the first version > that can boot AmigaOS. Ping? Sorry to push it but it would be great if this could get in for 3.0. Peter, this may need your attention like last time as it's not clear whose tree it should go through and David did not seem to be confident taking it via the ppc tree. Thank you, BALATON Zoltan > BALATON Zoltan (3): > sm501: Implement i2c part for reading monitor EDID > sm501: Fix support for non-zero frame buffer start address > sm501: Set updated region dirty after 2D operation > > Sebastian Bauer (4): > sm501: Perform a full update after palette change > sm501: Use values from the pitch register for 2D operations > sm501: Implement negated destination raster operation mode > sm501: Log unimplemented raster operation modes > > default-configs/ppc-softmmu.mak | 1 + > default-configs/ppcemb-softmmu.mak | 1 + > default-configs/sh4-softmmu.mak | 2 + > default-configs/sh4eb-softmmu.mak | 2 + > hw/display/sm501.c | 229 +++++++++++++++++++++++++++++++++++-- > 5 files changed, 223 insertions(+), 12 deletions(-) > >
On 30 June 2018 at 21:34, BALATON Zoltan <balaton@eik.bme.hu> wrote: > On Tue, 26 Jun 2018, BALATON Zoltan wrote: >> >> Version 2 of the sm501 changes with fixes that are needed to get >> AmigaOS 4.1FE to boot and able to produce graphics. >> >> The strange blue-white colors that first appear are actually correct >> and because of AmigaOS selecting a low resolution PAL mode by default >> instead of a board specific mode. To work around this one can select >> the last option to boot the live CD and then select a better board >> specific mode from ScreenMode Prefs. It takes a while for the >> ScreenMode window to appear and graphics operations are slow which >> could use some improvement but at least it seems to work correctly now >> apart from some unimplemented drawing modes for compositing. >> >> If this could be merged before the freeze with the sam460ex patches >> and Sebastian's ehci patch then QEMU 3.0 could be the first version >> that can boot AmigaOS. > > > Ping? Sorry to push it but it would be great if this could get in for 3.0. > Peter, this may need your attention like last time as it's not clear whose > tree it should go through and David did not seem to be confident taking it > via the ppc tree. I can have a look, but really I think these should go via the ppc tree -- the device is used in a ppc machine and an sh4 one, and the ppc tree is much more active than sh4. thanks -- PMM
On Mon, 2 Jul 2018, Peter Maydell wrote: > On 30 June 2018 at 21:34, BALATON Zoltan <balaton@eik.bme.hu> wrote: >> On Tue, 26 Jun 2018, BALATON Zoltan wrote: >>> >>> Version 2 of the sm501 changes with fixes that are needed to get >>> AmigaOS 4.1FE to boot and able to produce graphics. >>> >>> The strange blue-white colors that first appear are actually correct >>> and because of AmigaOS selecting a low resolution PAL mode by default >>> instead of a board specific mode. To work around this one can select >>> the last option to boot the live CD and then select a better board >>> specific mode from ScreenMode Prefs. It takes a while for the >>> ScreenMode window to appear and graphics operations are slow which >>> could use some improvement but at least it seems to work correctly now >>> apart from some unimplemented drawing modes for compositing. >>> >>> If this could be merged before the freeze with the sam460ex patches >>> and Sebastian's ehci patch then QEMU 3.0 could be the first version >>> that can boot AmigaOS. >> >> >> Ping? Sorry to push it but it would be great if this could get in for 3.0. >> Peter, this may need your attention like last time as it's not clear whose >> tree it should go through and David did not seem to be confident taking it >> via the ppc tree. > > I can have a look, but really I think these should go via the > ppc tree -- the device is used in a ppc machine and an sh4 one, > and the ppc tree is much more active than sh4. Thank you. Please agree on what should be the needed action here. It would be a pity if this missed 3.0 because of this. Could it be merged before the freeze if no obvious problems are found with that it can be removed later if it causes any trouble (I think that's unlikely as it's only touches rarely used parts). Regards, BALATON Zoltan
On 2 July 2018 at 11:33, BALATON Zoltan <balaton@eik.bme.hu> wrote: > Thank you. Please agree on what should be the needed action here. It would > be a pity if this missed 3.0 because of this. Could it be merged before the > freeze if no obvious problems are found with that it can be removed later if > it causes any trouble (I think that's unlikely as it's only touches rarely > used parts). These are bug fixes, so there is no strict requirement for them to get in for the softfreeze deadline. I agree that we should get these in for 3.0. thanks -- PMM
On Mon, 2 Jul 2018, Peter Maydell wrote: > On 2 July 2018 at 11:33, BALATON Zoltan <balaton@eik.bme.hu> wrote: >> Thank you. Please agree on what should be the needed action here. It would >> be a pity if this missed 3.0 because of this. Could it be merged before the >> freeze if no obvious problems are found with that it can be removed later if >> it causes any trouble (I think that's unlikely as it's only touches rarely >> used parts). > > These are bug fixes, so there is no strict requirement for them > to get in for the softfreeze deadline. I agree that we should > get these in for 3.0. Some of them do implement new features of sm501 previously not emulated so it depends on your judgement but if we get a green card from you that these can get in as bugfixes before hard freeze then I'm not worried that much. :-) Thank you, BALATON Zoltan
On Mon, Jul 02, 2018 at 10:42:07AM +0100, Peter Maydell wrote: > On 30 June 2018 at 21:34, BALATON Zoltan <balaton@eik.bme.hu> wrote: > > On Tue, 26 Jun 2018, BALATON Zoltan wrote: > >> > >> Version 2 of the sm501 changes with fixes that are needed to get > >> AmigaOS 4.1FE to boot and able to produce graphics. > >> > >> The strange blue-white colors that first appear are actually correct > >> and because of AmigaOS selecting a low resolution PAL mode by default > >> instead of a board specific mode. To work around this one can select > >> the last option to boot the live CD and then select a better board > >> specific mode from ScreenMode Prefs. It takes a while for the > >> ScreenMode window to appear and graphics operations are slow which > >> could use some improvement but at least it seems to work correctly now > >> apart from some unimplemented drawing modes for compositing. > >> > >> If this could be merged before the freeze with the sam460ex patches > >> and Sebastian's ehci patch then QEMU 3.0 could be the first version > >> that can boot AmigaOS. > > > > > > Ping? Sorry to push it but it would be great if this could get in for 3.0. > > Peter, this may need your attention like last time as it's not clear whose > > tree it should go through and David did not seem to be confident taking it > > via the ppc tree. > > I can have a look, but really I think these should go via the > ppc tree -- the device is used in a ppc machine and an sh4 one, > and the ppc tree is much more active than sh4. Hm, well, if you like. Although the gradual creep of my maintainership scope into things I know less and less about makes me rather nervous. I tried to convince Balaton Zoltan to become sm501 maintainer, but he didn't bite. Zoltan, I'm sorry I haven't been following this series closely since I asked you to direct it to Peter. If you resend to me, with whatever accumulated reviews you have, I'll merge it via the ppc tree. It won't make the soft freeze, but seems we have a quorum to consider it close enough to a bug fix, so we should be able to squeeze it in before the hard freeze.
On 3 July 2018 at 01:28, David Gibson <david@gibson.dropbear.id.au> wrote: > On Mon, Jul 02, 2018 at 10:42:07AM +0100, Peter Maydell wrote: >> I can have a look, but really I think these should go via the >> ppc tree -- the device is used in a ppc machine and an sh4 one, >> and the ppc tree is much more active than sh4. > > Hm, well, if you like. Although the gradual creep of my > maintainership scope into things I know less and less about makes me > rather nervous. I tried to convince Balaton Zoltan to become sm501 > maintainer, but he didn't bite. It's not like I know anything more about the sm501 than you do :-) The arm parts of the tree have a lot of board and device code I'm not familiar with either. Generally the approach I use is: * eyeball the code to see if it's doing anything that looks "obviously wrong", failing to use correct QEMU APIs, etc * anything that touches 'core' code or code common to multiple machines gets closer scrutiny * I might check the data sheet if I'm feeling enthusastic or if the code looks like it's doing weird things, but often not, especially if the code has had review from somebody else * if I have a test image to hand I'll do a smoke test, but often I don't have a test image Basically I think the important thing is to make sure the codebase stays maintainable and generally the quality bar in terms of using the right APIs and design approaches tends to ratchet up rather than down. If our implementation of an obscure device isn't actually right, that doesn't really affect very many users, so I worry less about that side of things. thanks -- PMM
Hi David, I'll reply to you using Peter mail :) On 07/03/2018 06:13 AM, Peter Maydell wrote: > On 3 July 2018 at 01:28, David Gibson <david@gibson.dropbear.id.au> wrote: >> On Mon, Jul 02, 2018 at 10:42:07AM +0100, Peter Maydell wrote: >>> I can have a look, but really I think these should go via the >>> ppc tree -- the device is used in a ppc machine and an sh4 one, >>> and the ppc tree is much more active than sh4. >> >> Hm, well, if you like. Although the gradual creep of my >> maintainership scope into things I know less and less about makes me >> rather nervous. I tried to convince Balaton Zoltan to become sm501 >> maintainer, but he didn't bite. > > It's not like I know anything more about the sm501 than you do :-) > > The arm parts of the tree have a lot of board and device code > I'm not familiar with either. Generally the approach I use is: > * eyeball the code to see if it's doing anything that looks > "obviously wrong", failing to use correct QEMU APIs, etc Gerd Hoffmann did a review for the graphic code, then commented "all look sane": http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg06493.html I don't have a serious understanding of QEMU graphic internals, but looked at those patches and didn't find anything "obviously wrong". > * anything that touches 'core' code or code common to multiple > machines gets closer scrutiny The only SH4 tests I run are using Aurelien Debian: https://people.debian.org/~aurel32/qemu/sh4/ > * I might check the data sheet if I'm feeling enthusastic or > if the code looks like it's doing weird things, but often > not, especially if the code has had review from somebody else I did a review of the I2C/DDC patch with the specs at hand. > * if I have a test image to hand I'll do a smoke test, but often > I don't have a test image Neither do I. > Basically I think the important thing is to make sure the > codebase stays maintainable and generally the quality bar > in terms of using the right APIs and design approaches > tends to ratchet up rather than down. If our implementation > of an obscure device isn't actually right, that doesn't > really affect very many users, so I worry less about that > side of things. Zoltan used few deprecated API but said updating "could be done in separate clean up", which I agree :) http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg06911.html FWIW and using Peter's criteria I think this series is good to go. Regards, Phil.
On Wed, 4 Jul 2018, Philippe Mathieu-Daudé wrote: > Hi David, I'll reply to you using Peter mail :) Thanks a lot for this summary and your review spotting an error. I forgot about Gerd's OK as he did not send formal Reviewed or Acked but you're right that's also counts to show that the series should be sane. I did not want to become maintainer of sm501 for at least two reasons: 1. Because it was part of SH and thus it should already have a maintainer or if it doesn't any more then I can't take that either as I don't know SH 2. I'm not familiar with the tasks a maintainer has to do and don't have enough time now to do that But if it's only a question of responsibility I don't ask you to take that, only ask David or Peter to help in merging this. I take responsibility for problems it causes and will try to fix it or I'm OK to have it removed if I fail to do that. Hope that's acceptable at least for now, then we can discuss about what to do with the sam460ex and related parts for the future. Thank you, BALATON Zoltan > On 07/03/2018 06:13 AM, Peter Maydell wrote: >> On 3 July 2018 at 01:28, David Gibson <david@gibson.dropbear.id.au> wrote: >>> On Mon, Jul 02, 2018 at 10:42:07AM +0100, Peter Maydell wrote: >>>> I can have a look, but really I think these should go via the >>>> ppc tree -- the device is used in a ppc machine and an sh4 one, >>>> and the ppc tree is much more active than sh4. >>> >>> Hm, well, if you like. Although the gradual creep of my >>> maintainership scope into things I know less and less about makes me >>> rather nervous. I tried to convince Balaton Zoltan to become sm501 >>> maintainer, but he didn't bite. >> >> It's not like I know anything more about the sm501 than you do :-) >> >> The arm parts of the tree have a lot of board and device code >> I'm not familiar with either. Generally the approach I use is: >> * eyeball the code to see if it's doing anything that looks >> "obviously wrong", failing to use correct QEMU APIs, etc > > Gerd Hoffmann did a review for the graphic code, > then commented "all look sane": > http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg06493.html > > I don't have a serious understanding of QEMU graphic internals, but > looked at those patches and didn't find anything "obviously wrong". > >> * anything that touches 'core' code or code common to multiple >> machines gets closer scrutiny > > The only SH4 tests I run are using Aurelien Debian: > https://people.debian.org/~aurel32/qemu/sh4/ > >> * I might check the data sheet if I'm feeling enthusastic or >> if the code looks like it's doing weird things, but often >> not, especially if the code has had review from somebody else > > I did a review of the I2C/DDC patch with the specs at hand. > >> * if I have a test image to hand I'll do a smoke test, but often >> I don't have a test image > > Neither do I. > >> Basically I think the important thing is to make sure the >> codebase stays maintainable and generally the quality bar >> in terms of using the right APIs and design approaches >> tends to ratchet up rather than down. If our implementation >> of an obscure device isn't actually right, that doesn't >> really affect very many users, so I worry less about that >> side of things. > > Zoltan used few deprecated API but said updating "could be done in > separate clean up", which I agree :) > http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg06911.html > > FWIW and using Peter's criteria I think this series is good to go. > > Regards, > > Phil. > >