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

[Peter Maydell]

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.

thanks
-- PMM