Hi On Tue, Oct 10, 2023 at 1:53 PM BALATON Zoltan <bala...@eik.bme.hu> wrote: > > On Tue, 10 Oct 2023, Marc-André Lureau wrote: > > Hi Zoltan > > > > On Mon, Sep 18, 2023 at 9:59 PM BALATON Zoltan <bala...@eik.bme.hu> wrote: > >> > >> On Mon, 18 Sep 2023, marcandre.lur...@redhat.com wrote: > >>> From: Marc-André Lureau <marcandre.lur...@redhat.com> > >>> > >>> Drop the "x-pixman" property and use fallback path in such case. > >>> > >>> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > >>> --- > >>> hw/display/sm501.c | 19 ++++++++++++++++--- > >>> 1 file changed, 16 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/hw/display/sm501.c b/hw/display/sm501.c > >>> index 0eecd00701..a897c82f04 100644 > >>> --- a/hw/display/sm501.c > >>> +++ b/hw/display/sm501.c > >>> @@ -730,7 +730,6 @@ static void sm501_2d_operation(SM501State *s) > >>> switch (cmd) { > >>> case 0: /* BitBlt */ > >>> { > >>> - static uint32_t tmp_buf[16384]; > >>> unsigned int src_x = (s->twoD_source >> 16) & 0x01FFF; > >>> unsigned int src_y = s->twoD_source & 0xFFFF; > >>> uint32_t src_base = s->twoD_source_base & 0x03FFFFFF; > >>> @@ -828,9 +827,11 @@ static void sm501_2d_operation(SM501State *s) > >>> de = db + (width + (height - 1) * dst_pitch) * bypp; > >>> overlap = (db < se && sb < de); > >>> } > >>> +#ifdef CONFIG_PIXMAN > >>> if (overlap && (s->use_pixman & BIT(2))) { > >>> /* pixman can't do reverse blit: copy via temporary */ > >>> int tmp_stride = DIV_ROUND_UP(width * bypp, > >>> sizeof(uint32_t)); > >>> + static uint32_t tmp_buf[16384]; > >>> uint32_t *tmp = tmp_buf; > >>> > >>> if (tmp_stride * sizeof(uint32_t) * height > > >>> sizeof(tmp_buf)) { > >>> @@ -860,9 +861,12 @@ static void sm501_2d_operation(SM501State *s) > >>> dst_pitch * bypp / > >>> sizeof(uint32_t), > >>> 8 * bypp, 8 * bypp, src_x, src_y, > >>> dst_x, dst_y, width, height); > >>> - } else { > >>> + } else > >>> +#else > >>> + { > >>> fallback = true; > >>> } > >>> +#endif > >>> if (fallback) { > >>> uint8_t *sp = s->local_mem + src_base; > >>> uint8_t *d = s->local_mem + dst_base; > >>> @@ -894,10 +898,13 @@ static void sm501_2d_operation(SM501State *s) > >>> color = cpu_to_le16(color); > >>> } > >>> > >>> +#ifdef CONFIG_PIXMAN > >>> if (!(s->use_pixman & BIT(0)) || (width == 1 && height == 1) || > >>> !pixman_fill((uint32_t *)&s->local_mem[dst_base], > >>> dst_pitch * bypp / sizeof(uint32_t), 8 * bypp, > >>> - dst_x, dst_y, width, height, color)) { > >>> + dst_x, dst_y, width, height, color)) > >>> +#endif > >>> + { > >>> /* fallback when pixman failed or we don't want to call it */ > >>> uint8_t *d = s->local_mem + dst_base; > >>> unsigned int x, y, i; > >>> @@ -2038,7 +2045,9 @@ static void sm501_realize_sysbus(DeviceState *dev, > >>> Error **errp) > >>> > >>> static Property sm501_sysbus_properties[] = { > >>> DEFINE_PROP_UINT32("vram-size", SM501SysBusState, vram_size, 0), > >>> +#ifdef CONFIG_PIXMAN > >>> DEFINE_PROP_UINT8("x-pixman", SM501SysBusState, state.use_pixman, 7), > >>> +#endif > >> > >> Do we want to omit the property when compiled without pixman? I think we > >> could leave it there and it would just be ignored without pixman but the > >> same command line would still work and need less ifdefs in code. > > > > That's a reasonable idea to me. At least, it can handle x-pixman=0 > > fine when !CONFIG_PIXMAN then. > > > > Btw, looking at it, it seems it should be DEFINE_PROP_BIT instead. I > > will add a TODO :) > > Erm, a bit can be 1 or 0 but the default value of it is 7. It's not a > single but but a bitmask the enable/disable pisman for different > operations separately so I think it can't be _BIT.
Sure, but we could have more explicitly different BIT properties ("x-pixman-fill", "x-pixman-blit", "x-pixman-overlap-blit"). thanks -- Marc-André Lureau