qemu-ppc
[Top][All Lists]
Advanced

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

Re: [PATCH] hw/display/sm501: Add alternatives to pixman routines


From: BALATON Zoltan
Subject: Re: [PATCH] hw/display/sm501: Add alternatives to pixman routines
Date: Fri, 24 Feb 2023 13:36:14 +0100 (CET)

On Fri, 24 Feb 2023, Peter Maydell wrote:
On Fri, 24 Feb 2023 at 00:18, BALATON Zoltan <balaton@eik.bme.hu> wrote:

Pixman can sometimes return false so add fallbacks for such cases and
also add a property to disable pixman and always use the fallbacks
which can be useful on platforms where pixman is broken or for testing
different drawing methods.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
Also ping for the other sm501 patch I've sent a week ago:
20230216144043.D632874634B@zero.eik.bme.hu/">https://patchew.org/QEMU/20230216144043.D632874634B@zero.eik.bme.hu/
These two patches are needed to fix graphics issues with AmigaOS so
I'd like them to be merged for 8.0

@@ -2010,6 +2035,7 @@ static void sm501_realize_sysbus(DeviceState *dev, Error 
**errp)
 static Property sm501_sysbus_properties[] = {
     DEFINE_PROP_UINT32("vram-size", SM501SysBusState, vram_size, 0),
     DEFINE_PROP_UINT32("base", SM501SysBusState, base, 0),
+    DEFINE_PROP_BOOL("x-pixman", SM501SysBusState, state.use_pixman, true),
     DEFINE_PROP_END_OF_LIST(),
 };

@@ -2093,6 +2119,7 @@ static void sm501_realize_pci(PCIDevice *dev, Error 
**errp)

 static Property sm501_pci_properties[] = {
     DEFINE_PROP_UINT32("vram-size", SM501PCIState, vram_size, 64 * MiB),
+    DEFINE_PROP_BOOL("x-pixman", SM501PCIState, state.use_pixman, true),
     DEFINE_PROP_END_OF_LIST(),
 };

I don't think this should be a user-facing property on a single
graphics device. Either pixman works, or it doesn't (in which
case we might need to do configure time checks and have a
fallback), but we shouldn't make the user have to set an
undocumented property on the device to get it to work.

This is mostly for testing now, that's why I've also called it with an x- prefix and not intended as a final solution or something that users would need to use. I'd decide later if we want to keep pixman or replace it with something else (as the problems we found with recently makes me think it might not be such a good idea after all) but for now I need a way to control it and do testing with or without pixman to get some data to help to decide what to do with it. This patch should handle tha cases where pixman returns false (like missing implementation on macOS aarch64) so no need to set this option normally. It's only when I want to easily test if a problem is pixman related, so I can easily set it to fallback or ask users who report a problem to try that. If you think this should be clarified in the commit message can you please suggest a consise sentence to describe this in a way that makes sense in English?

Regards,
BALATON Zoltan



reply via email to

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