* [PATCH v2 0/4] Misc ati-vga patches
@ 2023-11-01 20:45 BALATON Zoltan
2023-11-01 20:45 ` [PATCH v2 1/4] ati-vga: Fix aperture sizes BALATON Zoltan
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: BALATON Zoltan @ 2023-11-01 20:45 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, marcandre.lureau
Changes in v2:
- Add HOST_PATH_CNTL reg in patch 1 to match Linux vram size calculation
- Add a new patch to implement pixman fallbacks for ati-vga that
should help with the series that make pixman optional.
Some misc patches I had laying around that could be upstreamed just to
clean up my tree a bit.
BALATON Zoltan (4):
ati-vga: Fix aperture sizes
ati-vga: Support unaligned access to GPIO DDC registers
ati-vga: Add 30 bit palette access register
ati-vga: Implement fallback for pixman routines
hw/display/ati.c | 61 +++++++++++++++++++++++++----------
hw/display/ati_2d.c | 75 ++++++++++++++++++++++++++++++-------------
hw/display/ati_dbg.c | 2 ++
hw/display/ati_int.h | 2 ++
hw/display/ati_regs.h | 2 ++
5 files changed, 103 insertions(+), 39 deletions(-)
--
2.30.9
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2 1/4] ati-vga: Fix aperture sizes 2023-11-01 20:45 [PATCH v2 0/4] Misc ati-vga patches BALATON Zoltan @ 2023-11-01 20:45 ` BALATON Zoltan 2023-11-07 14:51 ` Michael Tokarev 2023-11-01 20:45 ` [PATCH v2 2/4] ati-vga: Support unaligned access to GPIO DDC registers BALATON Zoltan ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: BALATON Zoltan @ 2023-11-01 20:45 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, marcandre.lureau Apparently these should be half the memory region sizes confirmed at least by Radeon FCocde ROM while Rage 128 Pro ROMs don't seem to use these. Linux r100 DRM driver also checks for a bit in HOST_PATH_CNTL so we also add that even though the FCode ROM does not seem to set it. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/display/ati.c | 7 +++++-- hw/display/ati_dbg.c | 1 + hw/display/ati_regs.h | 1 + 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/display/ati.c b/hw/display/ati.c index c36282c343..ea7ab89a19 100644 --- a/hw/display/ati.c +++ b/hw/display/ati.c @@ -349,14 +349,17 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size) PCI_BASE_ADDRESS_0, size) & 0xfffffff0; break; case CONFIG_APER_SIZE: - val = s->vga.vram_size; + val = s->vga.vram_size / 2; break; case CONFIG_REG_1_BASE: val = pci_default_read_config(&s->dev, PCI_BASE_ADDRESS_2, size) & 0xfffffff0; break; case CONFIG_REG_APER_SIZE: - val = memory_region_size(&s->mm); + val = memory_region_size(&s->mm) / 2; + break; + case HOST_PATH_CNTL: + val = BIT(23); /* Radeon HDP_APER_CNTL */ break; case MC_STATUS: val = 5; diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c index bd0ecd48c7..4aec1c383a 100644 --- a/hw/display/ati_dbg.c +++ b/hw/display/ati_dbg.c @@ -38,6 +38,7 @@ static struct ati_regdesc ati_reg_names[] = { {"CONFIG_APER_SIZE", 0x0108}, {"CONFIG_REG_1_BASE", 0x010c}, {"CONFIG_REG_APER_SIZE", 0x0110}, + {"HOST_PATH_CNTL", 0x0130}, {"MEM_CNTL", 0x0140}, {"MC_FB_LOCATION", 0x0148}, {"MC_AGP_LOCATION", 0x014C}, diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h index d6282b2ef2..c697b328da 100644 --- a/hw/display/ati_regs.h +++ b/hw/display/ati_regs.h @@ -56,6 +56,7 @@ #define CONFIG_APER_SIZE 0x0108 #define CONFIG_REG_1_BASE 0x010c #define CONFIG_REG_APER_SIZE 0x0110 +#define HOST_PATH_CNTL 0x0130 #define MEM_CNTL 0x0140 #define MC_FB_LOCATION 0x0148 #define MC_AGP_LOCATION 0x014C -- 2.30.9 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] ati-vga: Fix aperture sizes 2023-11-01 20:45 ` [PATCH v2 1/4] ati-vga: Fix aperture sizes BALATON Zoltan @ 2023-11-07 14:51 ` Michael Tokarev 2023-11-07 15:33 ` BALATON Zoltan 0 siblings, 1 reply; 13+ messages in thread From: Michael Tokarev @ 2023-11-07 14:51 UTC (permalink / raw) To: BALATON Zoltan, qemu-devel; +Cc: Gerd Hoffmann, marcandre.lureau 01.11.2023 23:45, BALATON Zoltan: > Apparently these should be half the memory region sizes confirmed at > least by Radeon FCocde ROM while Rage 128 Pro ROMs don't seem to use > these. Linux r100 DRM driver also checks for a bit in HOST_PATH_CNTL > so we also add that even though the FCode ROM does not seem to set it. Is it stable-worthy? Thanks, /mjt ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] ati-vga: Fix aperture sizes 2023-11-07 14:51 ` Michael Tokarev @ 2023-11-07 15:33 ` BALATON Zoltan 2023-11-07 17:27 ` Michael Tokarev 0 siblings, 1 reply; 13+ messages in thread From: BALATON Zoltan @ 2023-11-07 15:33 UTC (permalink / raw) To: Michael Tokarev; +Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau On Tue, 7 Nov 2023, Michael Tokarev wrote: > 01.11.2023 23:45, BALATON Zoltan: >> Apparently these should be half the memory region sizes confirmed at >> least by Radeon FCocde ROM while Rage 128 Pro ROMs don't seem to use >> these. Linux r100 DRM driver also checks for a bit in HOST_PATH_CNTL >> so we also add that even though the FCode ROM does not seem to set it. > > Is it stable-worthy? Not really beacause this is only needed by RV100 drivers but that GPU is not emulated enough yet to work so this won't help them. However the last patch adding pixman fallbacks to ati_2d.c fixes graphics issues on Apple silicon Macs where pixman does not work that happens also with the default rage128p emulation so that patch may be useful in stable. It should be independent of the other patches in the series so should apply without the other patches. Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] ati-vga: Fix aperture sizes 2023-11-07 15:33 ` BALATON Zoltan @ 2023-11-07 17:27 ` Michael Tokarev 0 siblings, 0 replies; 13+ messages in thread From: Michael Tokarev @ 2023-11-07 17:27 UTC (permalink / raw) To: BALATON Zoltan; +Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau 07.11.2023 18:33, BALATON Zoltan: .. >> Is it stable-worthy? > > Not really beacause this is only needed by RV100 drivers but that GPU is not emulated enough yet to work so this won't help them. However the last > patch adding pixman fallbacks to ati_2d.c fixes graphics issues on Apple silicon Macs where pixman does not work that happens also with the default > rage128p emulation so that patch may be useful in stable. It should be independent of the other patches in the series so should apply without the > other patches. Heh. Interesting. Thank you for the clarification, I'm picking up 08730ee0cc01 "ati-vga: Implement fallback for pixman routines" instead of aperture sizes fix. /mjt ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/4] ati-vga: Support unaligned access to GPIO DDC registers 2023-11-01 20:45 [PATCH v2 0/4] Misc ati-vga patches BALATON Zoltan 2023-11-01 20:45 ` [PATCH v2 1/4] ati-vga: Fix aperture sizes BALATON Zoltan @ 2023-11-01 20:45 ` BALATON Zoltan 2023-11-01 20:45 ` [PATCH v2 3/4] ati-vga: Add 30 bit palette access register BALATON Zoltan 2023-11-01 20:45 ` [PATCH v2 4/4] ati-vga: Implement fallback for pixman routines BALATON Zoltan 3 siblings, 0 replies; 13+ messages in thread From: BALATON Zoltan @ 2023-11-01 20:45 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, marcandre.lureau [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain, Size: 3743 bytes --] The GPIO_VGA_DDC and GPIO_DVI_DDC registers are used on Radeon for DDC access. Some drivers like the PPC Mac FCode ROM uses unaligned writes to these registers so implement this the same way as already done for GPIO_MONID which is used the same way for the Rage 128 Pro. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- hw/display/ati.c | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/hw/display/ati.c b/hw/display/ati.c index ea7ab89a19..b56dabaccb 100644 --- a/hw/display/ati.c +++ b/hw/display/ati.c @@ -319,11 +319,13 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size) case DAC_CNTL: val = s->regs.dac_cntl; break; - case GPIO_VGA_DDC: - val = s->regs.gpio_vga_ddc; + case GPIO_VGA_DDC ... GPIO_VGA_DDC + 3: + val = ati_reg_read_offs(s->regs.gpio_vga_ddc, + addr - GPIO_VGA_DDC, size); break; - case GPIO_DVI_DDC: - val = s->regs.gpio_dvi_ddc; + case GPIO_DVI_DDC ... GPIO_DVI_DDC + 3: + val = ati_reg_read_offs(s->regs.gpio_dvi_ddc, + addr - GPIO_DVI_DDC, size); break; case GPIO_MONID ... GPIO_MONID + 3: val = ati_reg_read_offs(s->regs.gpio_monid, @@ -629,29 +631,34 @@ static void ati_mm_write(void *opaque, hwaddr addr, s->regs.dac_cntl = data & 0xffffe3ff; s->vga.dac_8bit = !!(data & DAC_8BIT_EN); break; - case GPIO_VGA_DDC: + /* + * GPIO regs for DDC access. Because some drivers access these via + * multiple byte writes we have to be careful when we send bits to + * avoid spurious changes in bitbang_i2c state. Only do it when either + * the enable bits are changed or output bits changed while enabled. + */ + case GPIO_VGA_DDC ... GPIO_VGA_DDC + 3: if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) { /* FIXME: Maybe add a property to select VGA or DVI port? */ } break; - case GPIO_DVI_DDC: + case GPIO_DVI_DDC ... GPIO_DVI_DDC + 3: if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) { - s->regs.gpio_dvi_ddc = ati_i2c(&s->bbi2c, data, 0); + ati_reg_write_offs(&s->regs.gpio_dvi_ddc, + addr - GPIO_DVI_DDC, data, size); + if ((addr <= GPIO_DVI_DDC + 2 && addr + size > GPIO_DVI_DDC + 2) || + (addr == GPIO_DVI_DDC && (s->regs.gpio_dvi_ddc & 0x30000))) { + s->regs.gpio_dvi_ddc = ati_i2c(&s->bbi2c, + s->regs.gpio_dvi_ddc, 0); + } } break; case GPIO_MONID ... GPIO_MONID + 3: /* FIXME What does Radeon have here? */ if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) { + /* Rage128p accesses DDC via MONID(1-2) with additional mask bit */ ati_reg_write_offs(&s->regs.gpio_monid, addr - GPIO_MONID, data, size); - /* - * Rage128p accesses DDC used to get EDID via these bits. - * Because some drivers access this via multiple byte writes - * we have to be careful when we send bits to avoid spurious - * changes in bitbang_i2c state. So only do it when mask is set - * and either the enable bits are changed or output bits changed - * while enabled. - */ if ((s->regs.gpio_monid & BIT(25)) && ((addr <= GPIO_MONID + 2 && addr + size > GPIO_MONID + 2) || (addr == GPIO_MONID && (s->regs.gpio_monid & 0x60000)))) { -- 2.30.9 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/4] ati-vga: Add 30 bit palette access register 2023-11-01 20:45 [PATCH v2 0/4] Misc ati-vga patches BALATON Zoltan 2023-11-01 20:45 ` [PATCH v2 1/4] ati-vga: Fix aperture sizes BALATON Zoltan 2023-11-01 20:45 ` [PATCH v2 2/4] ati-vga: Support unaligned access to GPIO DDC registers BALATON Zoltan @ 2023-11-01 20:45 ` BALATON Zoltan 2023-11-01 20:45 ` [PATCH v2 4/4] ati-vga: Implement fallback for pixman routines BALATON Zoltan 3 siblings, 0 replies; 13+ messages in thread From: BALATON Zoltan @ 2023-11-01 20:45 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, marcandre.lureau Radeon cards have a 30 bit DAC and corresponding palette register to access it. We only use 8 bits but let the guests use 10 bit color values for those that access it through this register. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/display/ati.c | 9 +++++++++ hw/display/ati_dbg.c | 1 + hw/display/ati_int.h | 1 + hw/display/ati_regs.h | 1 + 4 files changed, 12 insertions(+) diff --git a/hw/display/ati.c b/hw/display/ati.c index b56dabaccb..5e38d2c3de 100644 --- a/hw/display/ati.c +++ b/hw/display/ati.c @@ -339,6 +339,9 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size) case PALETTE_DATA: val = vga_ioport_read(&s->vga, VGA_PEL_D); break; + case PALETTE_30_DATA: + val = s->regs.palette[vga_ioport_read(&s->vga, VGA_PEL_IR)]; + break; case CNFG_CNTL: val = s->regs.config_cntl; break; @@ -687,6 +690,12 @@ static void ati_mm_write(void *opaque, hwaddr addr, data >>= 8; vga_ioport_write(&s->vga, VGA_PEL_D, data & 0xff); break; + case PALETTE_30_DATA: + s->regs.palette[vga_ioport_read(&s->vga, VGA_PEL_IW)] = data; + vga_ioport_write(&s->vga, VGA_PEL_D, (data >> 22) & 0xff); + vga_ioport_write(&s->vga, VGA_PEL_D, (data >> 12) & 0xff); + vga_ioport_write(&s->vga, VGA_PEL_D, (data >> 2) & 0xff); + break; case CNFG_CNTL: s->regs.config_cntl = data; break; diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c index 4aec1c383a..3ffa7f35df 100644 --- a/hw/display/ati_dbg.c +++ b/hw/display/ati_dbg.c @@ -30,6 +30,7 @@ static struct ati_regdesc ati_reg_names[] = { {"AMCGPIO_EN_MIR", 0x00a8}, {"PALETTE_INDEX", 0x00b0}, {"PALETTE_DATA", 0x00b4}, + {"PALETTE_30_DATA", 0x00b8}, {"CNFG_CNTL", 0x00e0}, {"GEN_RESET_CNTL", 0x00f0}, {"CNFG_MEMSIZE", 0x00f8}, diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h index e8d3c7af75..8abb873f01 100644 --- a/hw/display/ati_int.h +++ b/hw/display/ati_int.h @@ -44,6 +44,7 @@ typedef struct ATIVGARegs { uint32_t gpio_dvi_ddc; uint32_t gpio_monid; uint32_t config_cntl; + uint32_t palette[256]; uint32_t crtc_h_total_disp; uint32_t crtc_h_sync_strt_wid; uint32_t crtc_v_total_disp; diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h index c697b328da..d7127748ff 100644 --- a/hw/display/ati_regs.h +++ b/hw/display/ati_regs.h @@ -48,6 +48,7 @@ #define AMCGPIO_EN_MIR 0x00a8 #define PALETTE_INDEX 0x00b0 #define PALETTE_DATA 0x00b4 +#define PALETTE_30_DATA 0x00b8 #define CNFG_CNTL 0x00e0 #define GEN_RESET_CNTL 0x00f0 #define CNFG_MEMSIZE 0x00f8 -- 2.30.9 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/4] ati-vga: Implement fallback for pixman routines 2023-11-01 20:45 [PATCH v2 0/4] Misc ati-vga patches BALATON Zoltan ` (2 preceding siblings ...) 2023-11-01 20:45 ` [PATCH v2 3/4] ati-vga: Add 30 bit palette access register BALATON Zoltan @ 2023-11-01 20:45 ` BALATON Zoltan 2023-11-06 10:41 ` Marc-André Lureau 3 siblings, 1 reply; 13+ messages in thread From: BALATON Zoltan @ 2023-11-01 20:45 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, marcandre.lureau Pixman routines can fail if no implementation is available and it will become optional soon so add fallbacks when pixman does not work. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/display/ati.c | 8 +++++ hw/display/ati_2d.c | 75 +++++++++++++++++++++++++++++++------------- hw/display/ati_int.h | 1 + 3 files changed, 62 insertions(+), 22 deletions(-) diff --git a/hw/display/ati.c b/hw/display/ati.c index 5e38d2c3de..f911fbc327 100644 --- a/hw/display/ati.c +++ b/hw/display/ati.c @@ -1047,6 +1047,7 @@ static Property ati_vga_properties[] = { DEFINE_PROP_UINT16("x-device-id", ATIVGAState, dev_id, PCI_DEVICE_ID_ATI_RAGE128_PF), DEFINE_PROP_BOOL("guest_hwcursor", ATIVGAState, cursor_guest_mode, false), + DEFINE_PROP_UINT8("x-pixman", ATIVGAState, use_pixman, 3), DEFINE_PROP_END_OF_LIST() }; @@ -1068,11 +1069,18 @@ static void ati_vga_class_init(ObjectClass *klass, void *data) k->exit = ati_vga_exit; } +static void ati_vga_init(Object *o) +{ + object_property_set_description(o, "x-pixman", "Use pixman for: " + "1: fill, 2: blit"); +} + static const TypeInfo ati_vga_info = { .name = TYPE_ATI_VGA, .parent = TYPE_PCI_DEVICE, .instance_size = sizeof(ATIVGAState), .class_init = ati_vga_class_init, + .instance_init = ati_vga_init, .interfaces = (InterfaceInfo[]) { { INTERFACE_CONVENTIONAL_PCI_DEVICE }, { }, diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c index 7d786653e8..0e6b8e4367 100644 --- a/hw/display/ati_2d.c +++ b/hw/display/ati_2d.c @@ -92,6 +92,7 @@ void ati_2d_blt(ATIVGAState *s) switch (s->regs.dp_mix & GMC_ROP3_MASK) { case ROP3_SRCCOPY: { + bool fallback = false; unsigned src_x = (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT ? s->regs.src_x : s->regs.src_x + 1 - s->regs.dst_width); unsigned src_y = (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ? @@ -122,27 +123,50 @@ void ati_2d_blt(ATIVGAState *s) src_bits, dst_bits, src_stride, dst_stride, bpp, bpp, src_x, src_y, dst_x, dst_y, s->regs.dst_width, s->regs.dst_height); - if (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && + if ((s->use_pixman & BIT(1)) && + s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { - pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, - src_stride, dst_stride, bpp, bpp, - src_x, src_y, dst_x, dst_y, - s->regs.dst_width, s->regs.dst_height); - } else { + fallback = !pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, + src_stride, dst_stride, bpp, bpp, + src_x, src_y, dst_x, dst_y, + s->regs.dst_width, s->regs.dst_height); + } else if (s->use_pixman & BIT(1)) { /* FIXME: We only really need a temporary if src and dst overlap */ int llb = s->regs.dst_width * (bpp / 8); int tmp_stride = DIV_ROUND_UP(llb, sizeof(uint32_t)); uint32_t *tmp = g_malloc(tmp_stride * sizeof(uint32_t) * s->regs.dst_height); - pixman_blt((uint32_t *)src_bits, tmp, - src_stride, tmp_stride, bpp, bpp, - src_x, src_y, 0, 0, - s->regs.dst_width, s->regs.dst_height); - pixman_blt(tmp, (uint32_t *)dst_bits, - tmp_stride, dst_stride, bpp, bpp, - 0, 0, dst_x, dst_y, - s->regs.dst_width, s->regs.dst_height); + fallback = !pixman_blt((uint32_t *)src_bits, tmp, + src_stride, tmp_stride, bpp, bpp, + src_x, src_y, 0, 0, + s->regs.dst_width, s->regs.dst_height); + if (!fallback) { + fallback = !pixman_blt(tmp, (uint32_t *)dst_bits, + tmp_stride, dst_stride, bpp, bpp, + 0, 0, dst_x, dst_y, + s->regs.dst_width, s->regs.dst_height); + } g_free(tmp); + } else { + fallback = true; + } + if (fallback) { + unsigned int y, i, j, bypp = bpp / 8; + unsigned int src_pitch = src_stride * sizeof(uint32_t); + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); + + for (y = 0; y < s->regs.dst_height; y++) { + i = dst_x * bypp; + j = src_x * bypp; + if (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { + i += (dst_y + y) * dst_pitch; + j += (src_y + y) * src_pitch; + } else { + i += (dst_y + s->regs.dst_height - 1 - y) * dst_pitch; + j += (src_y + s->regs.dst_height - 1 - y) * src_pitch; + } + memmove(&dst_bits[i], &src_bits[j], s->regs.dst_width * bypp); + } } if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + @@ -180,14 +204,21 @@ void ati_2d_blt(ATIVGAState *s) dst_stride /= sizeof(uint32_t); DPRINTF("pixman_fill(%p, %d, %d, %d, %d, %d, %d, %x)\n", - dst_bits, dst_stride, bpp, - dst_x, dst_y, - s->regs.dst_width, s->regs.dst_height, - filler); - pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, - dst_x, dst_y, - s->regs.dst_width, s->regs.dst_height, - filler); + dst_bits, dst_stride, bpp, dst_x, dst_y, + s->regs.dst_width, s->regs.dst_height, filler); + if (!(s->use_pixman & BIT(0)) || + !pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, dst_x, dst_y, + s->regs.dst_width, s->regs.dst_height, filler)) { + /* fallback when pixman failed or we don't want to call it */ + unsigned int x, y, i, bypp = bpp / 8; + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); + for (y = 0; y < s->regs.dst_height; y++) { + i = dst_x * bypp + (dst_y + y) * dst_pitch; + for (x = 0; x < s->regs.dst_width; x++, i += bypp) { + stn_he_p(&dst_bits[i], bypp, filler); + } + } + } if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + s->vga.vbe_regs[VBE_DISPI_INDEX_YRES] * s->vga.vbe_line_offset) { diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h index 8abb873f01..f5a47b82b0 100644 --- a/hw/display/ati_int.h +++ b/hw/display/ati_int.h @@ -90,6 +90,7 @@ struct ATIVGAState { char *model; uint16_t dev_id; uint8_t mode; + uint8_t use_pixman; bool cursor_guest_mode; uint16_t cursor_size; uint32_t cursor_offset; -- 2.30.9 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] ati-vga: Implement fallback for pixman routines 2023-11-01 20:45 ` [PATCH v2 4/4] ati-vga: Implement fallback for pixman routines BALATON Zoltan @ 2023-11-06 10:41 ` Marc-André Lureau 2023-11-06 11:02 ` BALATON Zoltan 0 siblings, 1 reply; 13+ messages in thread From: Marc-André Lureau @ 2023-11-06 10:41 UTC (permalink / raw) To: BALATON Zoltan; +Cc: qemu-devel, Gerd Hoffmann Hi On Thu, Nov 2, 2023 at 12:46 AM BALATON Zoltan <balaton@eik.bme.hu> wrote: > > Pixman routines can fail if no implementation is available and it will > become optional soon so add fallbacks when pixman does not work. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/display/ati.c | 8 +++++ > hw/display/ati_2d.c | 75 +++++++++++++++++++++++++++++++------------- > hw/display/ati_int.h | 1 + > 3 files changed, 62 insertions(+), 22 deletions(-) > > diff --git a/hw/display/ati.c b/hw/display/ati.c > index 5e38d2c3de..f911fbc327 100644 > --- a/hw/display/ati.c > +++ b/hw/display/ati.c > @@ -1047,6 +1047,7 @@ static Property ati_vga_properties[] = { > DEFINE_PROP_UINT16("x-device-id", ATIVGAState, dev_id, > PCI_DEVICE_ID_ATI_RAGE128_PF), > DEFINE_PROP_BOOL("guest_hwcursor", ATIVGAState, cursor_guest_mode, false), > + DEFINE_PROP_UINT8("x-pixman", ATIVGAState, use_pixman, 3), > DEFINE_PROP_END_OF_LIST() > }; > > @@ -1068,11 +1069,18 @@ static void ati_vga_class_init(ObjectClass *klass, void *data) > k->exit = ati_vga_exit; > } > > +static void ati_vga_init(Object *o) > +{ > + object_property_set_description(o, "x-pixman", "Use pixman for: " > + "1: fill, 2: blit"); > +} > + > static const TypeInfo ati_vga_info = { > .name = TYPE_ATI_VGA, > .parent = TYPE_PCI_DEVICE, > .instance_size = sizeof(ATIVGAState), > .class_init = ati_vga_class_init, > + .instance_init = ati_vga_init, > .interfaces = (InterfaceInfo[]) { > { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > { }, > diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c > index 7d786653e8..0e6b8e4367 100644 > --- a/hw/display/ati_2d.c > +++ b/hw/display/ati_2d.c > @@ -92,6 +92,7 @@ void ati_2d_blt(ATIVGAState *s) > switch (s->regs.dp_mix & GMC_ROP3_MASK) { > case ROP3_SRCCOPY: > { > + bool fallback = false; > unsigned src_x = (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT ? > s->regs.src_x : s->regs.src_x + 1 - s->regs.dst_width); > unsigned src_y = (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ? > @@ -122,27 +123,50 @@ void ati_2d_blt(ATIVGAState *s) > src_bits, dst_bits, src_stride, dst_stride, bpp, bpp, > src_x, src_y, dst_x, dst_y, > s->regs.dst_width, s->regs.dst_height); > - if (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && > + if ((s->use_pixman & BIT(1)) && > + s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && > s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { > - pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, > - src_stride, dst_stride, bpp, bpp, > - src_x, src_y, dst_x, dst_y, > - s->regs.dst_width, s->regs.dst_height); > - } else { > + fallback = !pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, > + src_stride, dst_stride, bpp, bpp, > + src_x, src_y, dst_x, dst_y, > + s->regs.dst_width, s->regs.dst_height); > + } else if (s->use_pixman & BIT(1)) { > /* FIXME: We only really need a temporary if src and dst overlap */ > int llb = s->regs.dst_width * (bpp / 8); > int tmp_stride = DIV_ROUND_UP(llb, sizeof(uint32_t)); > uint32_t *tmp = g_malloc(tmp_stride * sizeof(uint32_t) * > s->regs.dst_height); > - pixman_blt((uint32_t *)src_bits, tmp, > - src_stride, tmp_stride, bpp, bpp, > - src_x, src_y, 0, 0, > - s->regs.dst_width, s->regs.dst_height); > - pixman_blt(tmp, (uint32_t *)dst_bits, > - tmp_stride, dst_stride, bpp, bpp, > - 0, 0, dst_x, dst_y, > - s->regs.dst_width, s->regs.dst_height); > + fallback = !pixman_blt((uint32_t *)src_bits, tmp, > + src_stride, tmp_stride, bpp, bpp, > + src_x, src_y, 0, 0, > + s->regs.dst_width, s->regs.dst_height); > + if (!fallback) { > + fallback = !pixman_blt(tmp, (uint32_t *)dst_bits, > + tmp_stride, dst_stride, bpp, bpp, > + 0, 0, dst_x, dst_y, > + s->regs.dst_width, s->regs.dst_height); > + } > g_free(tmp); > + } else { > + fallback = true; > + } > + if (fallback) { > + unsigned int y, i, j, bypp = bpp / 8; > + unsigned int src_pitch = src_stride * sizeof(uint32_t); > + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); > + > + for (y = 0; y < s->regs.dst_height; y++) { > + i = dst_x * bypp; > + j = src_x * bypp; > + if (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { > + i += (dst_y + y) * dst_pitch; > + j += (src_y + y) * src_pitch; > + } else { > + i += (dst_y + s->regs.dst_height - 1 - y) * dst_pitch; > + j += (src_y + s->regs.dst_height - 1 - y) * src_pitch; > + } > + memmove(&dst_bits[i], &src_bits[j], s->regs.dst_width * bypp); This doesn't seem to handle overlapping regions the same as the pixman-version. Or am I missing something? > + } > } > if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && > dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + > @@ -180,14 +204,21 @@ void ati_2d_blt(ATIVGAState *s) > > dst_stride /= sizeof(uint32_t); > DPRINTF("pixman_fill(%p, %d, %d, %d, %d, %d, %d, %x)\n", > - dst_bits, dst_stride, bpp, > - dst_x, dst_y, > - s->regs.dst_width, s->regs.dst_height, > - filler); > - pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, > - dst_x, dst_y, > - s->regs.dst_width, s->regs.dst_height, > - filler); > + dst_bits, dst_stride, bpp, dst_x, dst_y, > + s->regs.dst_width, s->regs.dst_height, filler); > + if (!(s->use_pixman & BIT(0)) || > + !pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, dst_x, dst_y, > + s->regs.dst_width, s->regs.dst_height, filler)) { > + /* fallback when pixman failed or we don't want to call it */ > + unsigned int x, y, i, bypp = bpp / 8; > + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); > + for (y = 0; y < s->regs.dst_height; y++) { > + i = dst_x * bypp + (dst_y + y) * dst_pitch; > + for (x = 0; x < s->regs.dst_width; x++, i += bypp) { > + stn_he_p(&dst_bits[i], bypp, filler); > + } > + } > + } > if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && > dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + > s->vga.vbe_regs[VBE_DISPI_INDEX_YRES] * s->vga.vbe_line_offset) { > diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h > index 8abb873f01..f5a47b82b0 100644 > --- a/hw/display/ati_int.h > +++ b/hw/display/ati_int.h > @@ -90,6 +90,7 @@ struct ATIVGAState { > char *model; > uint16_t dev_id; > uint8_t mode; > + uint8_t use_pixman; > bool cursor_guest_mode; > uint16_t cursor_size; > uint32_t cursor_offset; > -- > 2.30.9 > > -- Marc-André Lureau ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] ati-vga: Implement fallback for pixman routines 2023-11-06 10:41 ` Marc-André Lureau @ 2023-11-06 11:02 ` BALATON Zoltan 2023-11-06 11:10 ` Marc-André Lureau 0 siblings, 1 reply; 13+ messages in thread From: BALATON Zoltan @ 2023-11-06 11:02 UTC (permalink / raw) To: Marc-André Lureau; +Cc: qemu-devel, Gerd Hoffmann [-- Attachment #1: Type: text/plain, Size: 8233 bytes --] On Mon, 6 Nov 2023, Marc-André Lureau wrote: > On Thu, Nov 2, 2023 at 12:46 AM BALATON Zoltan <balaton@eik.bme.hu> wrote: >> >> Pixman routines can fail if no implementation is available and it will >> become optional soon so add fallbacks when pixman does not work. >> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> hw/display/ati.c | 8 +++++ >> hw/display/ati_2d.c | 75 +++++++++++++++++++++++++++++++------------- >> hw/display/ati_int.h | 1 + >> 3 files changed, 62 insertions(+), 22 deletions(-) >> >> diff --git a/hw/display/ati.c b/hw/display/ati.c >> index 5e38d2c3de..f911fbc327 100644 >> --- a/hw/display/ati.c >> +++ b/hw/display/ati.c >> @@ -1047,6 +1047,7 @@ static Property ati_vga_properties[] = { >> DEFINE_PROP_UINT16("x-device-id", ATIVGAState, dev_id, >> PCI_DEVICE_ID_ATI_RAGE128_PF), >> DEFINE_PROP_BOOL("guest_hwcursor", ATIVGAState, cursor_guest_mode, false), >> + DEFINE_PROP_UINT8("x-pixman", ATIVGAState, use_pixman, 3), >> DEFINE_PROP_END_OF_LIST() >> }; >> >> @@ -1068,11 +1069,18 @@ static void ati_vga_class_init(ObjectClass *klass, void *data) >> k->exit = ati_vga_exit; >> } >> >> +static void ati_vga_init(Object *o) >> +{ >> + object_property_set_description(o, "x-pixman", "Use pixman for: " >> + "1: fill, 2: blit"); >> +} >> + >> static const TypeInfo ati_vga_info = { >> .name = TYPE_ATI_VGA, >> .parent = TYPE_PCI_DEVICE, >> .instance_size = sizeof(ATIVGAState), >> .class_init = ati_vga_class_init, >> + .instance_init = ati_vga_init, >> .interfaces = (InterfaceInfo[]) { >> { INTERFACE_CONVENTIONAL_PCI_DEVICE }, >> { }, >> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c >> index 7d786653e8..0e6b8e4367 100644 >> --- a/hw/display/ati_2d.c >> +++ b/hw/display/ati_2d.c >> @@ -92,6 +92,7 @@ void ati_2d_blt(ATIVGAState *s) >> switch (s->regs.dp_mix & GMC_ROP3_MASK) { >> case ROP3_SRCCOPY: >> { >> + bool fallback = false; >> unsigned src_x = (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT ? >> s->regs.src_x : s->regs.src_x + 1 - s->regs.dst_width); >> unsigned src_y = (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ? >> @@ -122,27 +123,50 @@ void ati_2d_blt(ATIVGAState *s) >> src_bits, dst_bits, src_stride, dst_stride, bpp, bpp, >> src_x, src_y, dst_x, dst_y, >> s->regs.dst_width, s->regs.dst_height); >> - if (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && >> + if ((s->use_pixman & BIT(1)) && >> + s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && >> s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { >> - pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, >> - src_stride, dst_stride, bpp, bpp, >> - src_x, src_y, dst_x, dst_y, >> - s->regs.dst_width, s->regs.dst_height); >> - } else { >> + fallback = !pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, >> + src_stride, dst_stride, bpp, bpp, >> + src_x, src_y, dst_x, dst_y, >> + s->regs.dst_width, s->regs.dst_height); >> + } else if (s->use_pixman & BIT(1)) { >> /* FIXME: We only really need a temporary if src and dst overlap */ >> int llb = s->regs.dst_width * (bpp / 8); >> int tmp_stride = DIV_ROUND_UP(llb, sizeof(uint32_t)); >> uint32_t *tmp = g_malloc(tmp_stride * sizeof(uint32_t) * >> s->regs.dst_height); >> - pixman_blt((uint32_t *)src_bits, tmp, >> - src_stride, tmp_stride, bpp, bpp, >> - src_x, src_y, 0, 0, >> - s->regs.dst_width, s->regs.dst_height); >> - pixman_blt(tmp, (uint32_t *)dst_bits, >> - tmp_stride, dst_stride, bpp, bpp, >> - 0, 0, dst_x, dst_y, >> - s->regs.dst_width, s->regs.dst_height); >> + fallback = !pixman_blt((uint32_t *)src_bits, tmp, >> + src_stride, tmp_stride, bpp, bpp, >> + src_x, src_y, 0, 0, >> + s->regs.dst_width, s->regs.dst_height); >> + if (!fallback) { >> + fallback = !pixman_blt(tmp, (uint32_t *)dst_bits, >> + tmp_stride, dst_stride, bpp, bpp, >> + 0, 0, dst_x, dst_y, >> + s->regs.dst_width, s->regs.dst_height); >> + } >> g_free(tmp); >> + } else { >> + fallback = true; >> + } >> + if (fallback) { >> + unsigned int y, i, j, bypp = bpp / 8; >> + unsigned int src_pitch = src_stride * sizeof(uint32_t); >> + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); >> + >> + for (y = 0; y < s->regs.dst_height; y++) { >> + i = dst_x * bypp; >> + j = src_x * bypp; >> + if (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { >> + i += (dst_y + y) * dst_pitch; >> + j += (src_y + y) * src_pitch; >> + } else { >> + i += (dst_y + s->regs.dst_height - 1 - y) * dst_pitch; >> + j += (src_y + s->regs.dst_height - 1 - y) * src_pitch; >> + } >> + memmove(&dst_bits[i], &src_bits[j], s->regs.dst_width * bypp); > > This doesn't seem to handle overlapping regions the same as the > pixman-version. Or am I missing something? memmove (as opposed to memcpy) allows overlapping regions and handles them correctly so no temporary needed for this. I've tested it with MorphOS and still got correct picture. Regards, BALATON Zoltan >> + } >> } >> if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && >> dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + >> @@ -180,14 +204,21 @@ void ati_2d_blt(ATIVGAState *s) >> >> dst_stride /= sizeof(uint32_t); >> DPRINTF("pixman_fill(%p, %d, %d, %d, %d, %d, %d, %x)\n", >> - dst_bits, dst_stride, bpp, >> - dst_x, dst_y, >> - s->regs.dst_width, s->regs.dst_height, >> - filler); >> - pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, >> - dst_x, dst_y, >> - s->regs.dst_width, s->regs.dst_height, >> - filler); >> + dst_bits, dst_stride, bpp, dst_x, dst_y, >> + s->regs.dst_width, s->regs.dst_height, filler); >> + if (!(s->use_pixman & BIT(0)) || >> + !pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, dst_x, dst_y, >> + s->regs.dst_width, s->regs.dst_height, filler)) { >> + /* fallback when pixman failed or we don't want to call it */ >> + unsigned int x, y, i, bypp = bpp / 8; >> + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); >> + for (y = 0; y < s->regs.dst_height; y++) { >> + i = dst_x * bypp + (dst_y + y) * dst_pitch; >> + for (x = 0; x < s->regs.dst_width; x++, i += bypp) { >> + stn_he_p(&dst_bits[i], bypp, filler); >> + } >> + } >> + } >> if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && >> dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + >> s->vga.vbe_regs[VBE_DISPI_INDEX_YRES] * s->vga.vbe_line_offset) { >> diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h >> index 8abb873f01..f5a47b82b0 100644 >> --- a/hw/display/ati_int.h >> +++ b/hw/display/ati_int.h >> @@ -90,6 +90,7 @@ struct ATIVGAState { >> char *model; >> uint16_t dev_id; >> uint8_t mode; >> + uint8_t use_pixman; >> bool cursor_guest_mode; >> uint16_t cursor_size; >> uint32_t cursor_offset; >> -- >> 2.30.9 >> >> > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] ati-vga: Implement fallback for pixman routines 2023-11-06 11:02 ` BALATON Zoltan @ 2023-11-06 11:10 ` Marc-André Lureau 2023-11-06 11:17 ` BALATON Zoltan 0 siblings, 1 reply; 13+ messages in thread From: Marc-André Lureau @ 2023-11-06 11:10 UTC (permalink / raw) To: BALATON Zoltan; +Cc: qemu-devel, Gerd Hoffmann Hi On Mon, Nov 6, 2023 at 3:02 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: > > On Mon, 6 Nov 2023, Marc-André Lureau wrote: > > On Thu, Nov 2, 2023 at 12:46 AM BALATON Zoltan <balaton@eik.bme.hu> wrote: > >> > >> Pixman routines can fail if no implementation is available and it will > >> become optional soon so add fallbacks when pixman does not work. > >> > >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > >> --- > >> hw/display/ati.c | 8 +++++ > >> hw/display/ati_2d.c | 75 +++++++++++++++++++++++++++++++------------- > >> hw/display/ati_int.h | 1 + > >> 3 files changed, 62 insertions(+), 22 deletions(-) > >> > >> diff --git a/hw/display/ati.c b/hw/display/ati.c > >> index 5e38d2c3de..f911fbc327 100644 > >> --- a/hw/display/ati.c > >> +++ b/hw/display/ati.c > >> @@ -1047,6 +1047,7 @@ static Property ati_vga_properties[] = { > >> DEFINE_PROP_UINT16("x-device-id", ATIVGAState, dev_id, > >> PCI_DEVICE_ID_ATI_RAGE128_PF), > >> DEFINE_PROP_BOOL("guest_hwcursor", ATIVGAState, cursor_guest_mode, false), > >> + DEFINE_PROP_UINT8("x-pixman", ATIVGAState, use_pixman, 3), > >> DEFINE_PROP_END_OF_LIST() > >> }; > >> > >> @@ -1068,11 +1069,18 @@ static void ati_vga_class_init(ObjectClass *klass, void *data) > >> k->exit = ati_vga_exit; > >> } > >> > >> +static void ati_vga_init(Object *o) > >> +{ > >> + object_property_set_description(o, "x-pixman", "Use pixman for: " > >> + "1: fill, 2: blit"); > >> +} > >> + > >> static const TypeInfo ati_vga_info = { > >> .name = TYPE_ATI_VGA, > >> .parent = TYPE_PCI_DEVICE, > >> .instance_size = sizeof(ATIVGAState), > >> .class_init = ati_vga_class_init, > >> + .instance_init = ati_vga_init, > >> .interfaces = (InterfaceInfo[]) { > >> { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > >> { }, > >> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c > >> index 7d786653e8..0e6b8e4367 100644 > >> --- a/hw/display/ati_2d.c > >> +++ b/hw/display/ati_2d.c > >> @@ -92,6 +92,7 @@ void ati_2d_blt(ATIVGAState *s) > >> switch (s->regs.dp_mix & GMC_ROP3_MASK) { > >> case ROP3_SRCCOPY: > >> { > >> + bool fallback = false; > >> unsigned src_x = (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT ? > >> s->regs.src_x : s->regs.src_x + 1 - s->regs.dst_width); > >> unsigned src_y = (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ? > >> @@ -122,27 +123,50 @@ void ati_2d_blt(ATIVGAState *s) > >> src_bits, dst_bits, src_stride, dst_stride, bpp, bpp, > >> src_x, src_y, dst_x, dst_y, > >> s->regs.dst_width, s->regs.dst_height); > >> - if (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && > >> + if ((s->use_pixman & BIT(1)) && > >> + s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && > >> s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { > >> - pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, > >> - src_stride, dst_stride, bpp, bpp, > >> - src_x, src_y, dst_x, dst_y, > >> - s->regs.dst_width, s->regs.dst_height); > >> - } else { > >> + fallback = !pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, > >> + src_stride, dst_stride, bpp, bpp, > >> + src_x, src_y, dst_x, dst_y, > >> + s->regs.dst_width, s->regs.dst_height); > >> + } else if (s->use_pixman & BIT(1)) { > >> /* FIXME: We only really need a temporary if src and dst overlap */ > >> int llb = s->regs.dst_width * (bpp / 8); > >> int tmp_stride = DIV_ROUND_UP(llb, sizeof(uint32_t)); > >> uint32_t *tmp = g_malloc(tmp_stride * sizeof(uint32_t) * > >> s->regs.dst_height); > >> - pixman_blt((uint32_t *)src_bits, tmp, > >> - src_stride, tmp_stride, bpp, bpp, > >> - src_x, src_y, 0, 0, > >> - s->regs.dst_width, s->regs.dst_height); > >> - pixman_blt(tmp, (uint32_t *)dst_bits, > >> - tmp_stride, dst_stride, bpp, bpp, > >> - 0, 0, dst_x, dst_y, > >> - s->regs.dst_width, s->regs.dst_height); > >> + fallback = !pixman_blt((uint32_t *)src_bits, tmp, > >> + src_stride, tmp_stride, bpp, bpp, > >> + src_x, src_y, 0, 0, > >> + s->regs.dst_width, s->regs.dst_height); > >> + if (!fallback) { > >> + fallback = !pixman_blt(tmp, (uint32_t *)dst_bits, > >> + tmp_stride, dst_stride, bpp, bpp, > >> + 0, 0, dst_x, dst_y, > >> + s->regs.dst_width, s->regs.dst_height); > >> + } > >> g_free(tmp); > >> + } else { > >> + fallback = true; > >> + } > >> + if (fallback) { > >> + unsigned int y, i, j, bypp = bpp / 8; > >> + unsigned int src_pitch = src_stride * sizeof(uint32_t); > >> + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); > >> + > >> + for (y = 0; y < s->regs.dst_height; y++) { > >> + i = dst_x * bypp; > >> + j = src_x * bypp; > >> + if (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { > >> + i += (dst_y + y) * dst_pitch; > >> + j += (src_y + y) * src_pitch; > >> + } else { > >> + i += (dst_y + s->regs.dst_height - 1 - y) * dst_pitch; > >> + j += (src_y + s->regs.dst_height - 1 - y) * src_pitch; > >> + } > >> + memmove(&dst_bits[i], &src_bits[j], s->regs.dst_width * bypp); > > > > This doesn't seem to handle overlapping regions the same as the > > pixman-version. Or am I missing something? > > memmove (as opposed to memcpy) allows overlapping regions and handles them > correctly so no temporary needed for this. I've tested it with MorphOS and > still got correct picture. But it is calling memmove() for each line, you may have overlapping rectangles. Having a temporary like above should solve this issue, assuming it's the correct behaviour. > > Regards, > BALATON Zoltan > > >> + } > >> } > >> if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && > >> dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + > >> @@ -180,14 +204,21 @@ void ati_2d_blt(ATIVGAState *s) > >> > >> dst_stride /= sizeof(uint32_t); > >> DPRINTF("pixman_fill(%p, %d, %d, %d, %d, %d, %d, %x)\n", > >> - dst_bits, dst_stride, bpp, > >> - dst_x, dst_y, > >> - s->regs.dst_width, s->regs.dst_height, > >> - filler); > >> - pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, > >> - dst_x, dst_y, > >> - s->regs.dst_width, s->regs.dst_height, > >> - filler); > >> + dst_bits, dst_stride, bpp, dst_x, dst_y, > >> + s->regs.dst_width, s->regs.dst_height, filler); > >> + if (!(s->use_pixman & BIT(0)) || > >> + !pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, dst_x, dst_y, > >> + s->regs.dst_width, s->regs.dst_height, filler)) { > >> + /* fallback when pixman failed or we don't want to call it */ > >> + unsigned int x, y, i, bypp = bpp / 8; > >> + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); > >> + for (y = 0; y < s->regs.dst_height; y++) { > >> + i = dst_x * bypp + (dst_y + y) * dst_pitch; > >> + for (x = 0; x < s->regs.dst_width; x++, i += bypp) { > >> + stn_he_p(&dst_bits[i], bypp, filler); > >> + } > >> + } > >> + } > >> if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && > >> dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + > >> s->vga.vbe_regs[VBE_DISPI_INDEX_YRES] * s->vga.vbe_line_offset) { > >> diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h > >> index 8abb873f01..f5a47b82b0 100644 > >> --- a/hw/display/ati_int.h > >> +++ b/hw/display/ati_int.h > >> @@ -90,6 +90,7 @@ struct ATIVGAState { > >> char *model; > >> uint16_t dev_id; > >> uint8_t mode; > >> + uint8_t use_pixman; > >> bool cursor_guest_mode; > >> uint16_t cursor_size; > >> uint32_t cursor_offset; > >> -- > >> 2.30.9 > >> > >> > > > > > > -- Marc-André Lureau ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] ati-vga: Implement fallback for pixman routines 2023-11-06 11:10 ` Marc-André Lureau @ 2023-11-06 11:17 ` BALATON Zoltan 2023-11-06 11:32 ` Marc-André Lureau 0 siblings, 1 reply; 13+ messages in thread From: BALATON Zoltan @ 2023-11-06 11:17 UTC (permalink / raw) To: Marc-André Lureau; +Cc: qemu-devel, Gerd Hoffmann [-- Attachment #1: Type: text/plain, Size: 9439 bytes --] On Mon, 6 Nov 2023, Marc-André Lureau wrote: > On Mon, Nov 6, 2023 at 3:02 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: >> >> On Mon, 6 Nov 2023, Marc-André Lureau wrote: >>> On Thu, Nov 2, 2023 at 12:46 AM BALATON Zoltan <balaton@eik.bme.hu> wrote: >>>> >>>> Pixman routines can fail if no implementation is available and it will >>>> become optional soon so add fallbacks when pixman does not work. >>>> >>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >>>> --- >>>> hw/display/ati.c | 8 +++++ >>>> hw/display/ati_2d.c | 75 +++++++++++++++++++++++++++++++------------- >>>> hw/display/ati_int.h | 1 + >>>> 3 files changed, 62 insertions(+), 22 deletions(-) >>>> >>>> diff --git a/hw/display/ati.c b/hw/display/ati.c >>>> index 5e38d2c3de..f911fbc327 100644 >>>> --- a/hw/display/ati.c >>>> +++ b/hw/display/ati.c >>>> @@ -1047,6 +1047,7 @@ static Property ati_vga_properties[] = { >>>> DEFINE_PROP_UINT16("x-device-id", ATIVGAState, dev_id, >>>> PCI_DEVICE_ID_ATI_RAGE128_PF), >>>> DEFINE_PROP_BOOL("guest_hwcursor", ATIVGAState, cursor_guest_mode, false), >>>> + DEFINE_PROP_UINT8("x-pixman", ATIVGAState, use_pixman, 3), >>>> DEFINE_PROP_END_OF_LIST() >>>> }; >>>> >>>> @@ -1068,11 +1069,18 @@ static void ati_vga_class_init(ObjectClass *klass, void *data) >>>> k->exit = ati_vga_exit; >>>> } >>>> >>>> +static void ati_vga_init(Object *o) >>>> +{ >>>> + object_property_set_description(o, "x-pixman", "Use pixman for: " >>>> + "1: fill, 2: blit"); >>>> +} >>>> + >>>> static const TypeInfo ati_vga_info = { >>>> .name = TYPE_ATI_VGA, >>>> .parent = TYPE_PCI_DEVICE, >>>> .instance_size = sizeof(ATIVGAState), >>>> .class_init = ati_vga_class_init, >>>> + .instance_init = ati_vga_init, >>>> .interfaces = (InterfaceInfo[]) { >>>> { INTERFACE_CONVENTIONAL_PCI_DEVICE }, >>>> { }, >>>> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c >>>> index 7d786653e8..0e6b8e4367 100644 >>>> --- a/hw/display/ati_2d.c >>>> +++ b/hw/display/ati_2d.c >>>> @@ -92,6 +92,7 @@ void ati_2d_blt(ATIVGAState *s) >>>> switch (s->regs.dp_mix & GMC_ROP3_MASK) { >>>> case ROP3_SRCCOPY: >>>> { >>>> + bool fallback = false; >>>> unsigned src_x = (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT ? >>>> s->regs.src_x : s->regs.src_x + 1 - s->regs.dst_width); >>>> unsigned src_y = (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ? >>>> @@ -122,27 +123,50 @@ void ati_2d_blt(ATIVGAState *s) >>>> src_bits, dst_bits, src_stride, dst_stride, bpp, bpp, >>>> src_x, src_y, dst_x, dst_y, >>>> s->regs.dst_width, s->regs.dst_height); >>>> - if (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && >>>> + if ((s->use_pixman & BIT(1)) && >>>> + s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && >>>> s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { >>>> - pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, >>>> - src_stride, dst_stride, bpp, bpp, >>>> - src_x, src_y, dst_x, dst_y, >>>> - s->regs.dst_width, s->regs.dst_height); >>>> - } else { >>>> + fallback = !pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, >>>> + src_stride, dst_stride, bpp, bpp, >>>> + src_x, src_y, dst_x, dst_y, >>>> + s->regs.dst_width, s->regs.dst_height); >>>> + } else if (s->use_pixman & BIT(1)) { >>>> /* FIXME: We only really need a temporary if src and dst overlap */ >>>> int llb = s->regs.dst_width * (bpp / 8); >>>> int tmp_stride = DIV_ROUND_UP(llb, sizeof(uint32_t)); >>>> uint32_t *tmp = g_malloc(tmp_stride * sizeof(uint32_t) * >>>> s->regs.dst_height); >>>> - pixman_blt((uint32_t *)src_bits, tmp, >>>> - src_stride, tmp_stride, bpp, bpp, >>>> - src_x, src_y, 0, 0, >>>> - s->regs.dst_width, s->regs.dst_height); >>>> - pixman_blt(tmp, (uint32_t *)dst_bits, >>>> - tmp_stride, dst_stride, bpp, bpp, >>>> - 0, 0, dst_x, dst_y, >>>> - s->regs.dst_width, s->regs.dst_height); >>>> + fallback = !pixman_blt((uint32_t *)src_bits, tmp, >>>> + src_stride, tmp_stride, bpp, bpp, >>>> + src_x, src_y, 0, 0, >>>> + s->regs.dst_width, s->regs.dst_height); >>>> + if (!fallback) { >>>> + fallback = !pixman_blt(tmp, (uint32_t *)dst_bits, >>>> + tmp_stride, dst_stride, bpp, bpp, >>>> + 0, 0, dst_x, dst_y, >>>> + s->regs.dst_width, s->regs.dst_height); >>>> + } >>>> g_free(tmp); >>>> + } else { >>>> + fallback = true; >>>> + } >>>> + if (fallback) { >>>> + unsigned int y, i, j, bypp = bpp / 8; >>>> + unsigned int src_pitch = src_stride * sizeof(uint32_t); >>>> + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); >>>> + >>>> + for (y = 0; y < s->regs.dst_height; y++) { >>>> + i = dst_x * bypp; >>>> + j = src_x * bypp; >>>> + if (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { >>>> + i += (dst_y + y) * dst_pitch; >>>> + j += (src_y + y) * src_pitch; >>>> + } else { >>>> + i += (dst_y + s->regs.dst_height - 1 - y) * dst_pitch; >>>> + j += (src_y + s->regs.dst_height - 1 - y) * src_pitch; >>>> + } >>>> + memmove(&dst_bits[i], &src_bits[j], s->regs.dst_width * bypp); >>> >>> This doesn't seem to handle overlapping regions the same as the >>> pixman-version. Or am I missing something? >> >> memmove (as opposed to memcpy) allows overlapping regions and handles them >> correctly so no temporary needed for this. I've tested it with MorphOS and >> still got correct picture. > > But it is calling memmove() for each line, you may have overlapping > rectangles. Having a temporary like above should solve this issue, > assuming it's the correct behaviour. Lines are handled by DST_Y_TOP_TO_BOTTOM so they are processed either top down or bottom up to avoid overlap. We only need temporary for pixman because the only does top down left to right but here vertical direction is handled by going through lines in the right order and horisontal overlap is handled by memmove. No need for temporary in this case. This is doing the same as sm501 so if you accept that works this is exactly the same only the coords calculation is a bit different due to ati-vga storing values differently. Regards, BALATON Zoltan >>>> + } >>>> } >>>> if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && >>>> dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + >>>> @@ -180,14 +204,21 @@ void ati_2d_blt(ATIVGAState *s) >>>> >>>> dst_stride /= sizeof(uint32_t); >>>> DPRINTF("pixman_fill(%p, %d, %d, %d, %d, %d, %d, %x)\n", >>>> - dst_bits, dst_stride, bpp, >>>> - dst_x, dst_y, >>>> - s->regs.dst_width, s->regs.dst_height, >>>> - filler); >>>> - pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, >>>> - dst_x, dst_y, >>>> - s->regs.dst_width, s->regs.dst_height, >>>> - filler); >>>> + dst_bits, dst_stride, bpp, dst_x, dst_y, >>>> + s->regs.dst_width, s->regs.dst_height, filler); >>>> + if (!(s->use_pixman & BIT(0)) || >>>> + !pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, dst_x, dst_y, >>>> + s->regs.dst_width, s->regs.dst_height, filler)) { >>>> + /* fallback when pixman failed or we don't want to call it */ >>>> + unsigned int x, y, i, bypp = bpp / 8; >>>> + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); >>>> + for (y = 0; y < s->regs.dst_height; y++) { >>>> + i = dst_x * bypp + (dst_y + y) * dst_pitch; >>>> + for (x = 0; x < s->regs.dst_width; x++, i += bypp) { >>>> + stn_he_p(&dst_bits[i], bypp, filler); >>>> + } >>>> + } >>>> + } >>>> if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && >>>> dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + >>>> s->vga.vbe_regs[VBE_DISPI_INDEX_YRES] * s->vga.vbe_line_offset) { >>>> diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h >>>> index 8abb873f01..f5a47b82b0 100644 >>>> --- a/hw/display/ati_int.h >>>> +++ b/hw/display/ati_int.h >>>> @@ -90,6 +90,7 @@ struct ATIVGAState { >>>> char *model; >>>> uint16_t dev_id; >>>> uint8_t mode; >>>> + uint8_t use_pixman; >>>> bool cursor_guest_mode; >>>> uint16_t cursor_size; >>>> uint32_t cursor_offset; >>>> -- >>>> 2.30.9 >>>> >>>> >>> >>> >>> > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] ati-vga: Implement fallback for pixman routines 2023-11-06 11:17 ` BALATON Zoltan @ 2023-11-06 11:32 ` Marc-André Lureau 0 siblings, 0 replies; 13+ messages in thread From: Marc-André Lureau @ 2023-11-06 11:32 UTC (permalink / raw) To: BALATON Zoltan; +Cc: qemu-devel, Gerd Hoffmann Hi On Mon, Nov 6, 2023 at 3:17 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: > > On Mon, 6 Nov 2023, Marc-André Lureau wrote: > > On Mon, Nov 6, 2023 at 3:02 PM BALATON Zoltan <balaton@eik.bme.hu> wrote: > >> > >> On Mon, 6 Nov 2023, Marc-André Lureau wrote: > >>> On Thu, Nov 2, 2023 at 12:46 AM BALATON Zoltan <balaton@eik.bme.hu> wrote: > >>>> > >>>> Pixman routines can fail if no implementation is available and it will > >>>> become optional soon so add fallbacks when pixman does not work. > >>>> > >>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > >>>> --- > >>>> hw/display/ati.c | 8 +++++ > >>>> hw/display/ati_2d.c | 75 +++++++++++++++++++++++++++++++------------- > >>>> hw/display/ati_int.h | 1 + > >>>> 3 files changed, 62 insertions(+), 22 deletions(-) > >>>> > >>>> diff --git a/hw/display/ati.c b/hw/display/ati.c > >>>> index 5e38d2c3de..f911fbc327 100644 > >>>> --- a/hw/display/ati.c > >>>> +++ b/hw/display/ati.c > >>>> @@ -1047,6 +1047,7 @@ static Property ati_vga_properties[] = { > >>>> DEFINE_PROP_UINT16("x-device-id", ATIVGAState, dev_id, > >>>> PCI_DEVICE_ID_ATI_RAGE128_PF), > >>>> DEFINE_PROP_BOOL("guest_hwcursor", ATIVGAState, cursor_guest_mode, false), > >>>> + DEFINE_PROP_UINT8("x-pixman", ATIVGAState, use_pixman, 3), > >>>> DEFINE_PROP_END_OF_LIST() > >>>> }; > >>>> > >>>> @@ -1068,11 +1069,18 @@ static void ati_vga_class_init(ObjectClass *klass, void *data) > >>>> k->exit = ati_vga_exit; > >>>> } > >>>> > >>>> +static void ati_vga_init(Object *o) > >>>> +{ > >>>> + object_property_set_description(o, "x-pixman", "Use pixman for: " > >>>> + "1: fill, 2: blit"); > >>>> +} > >>>> + > >>>> static const TypeInfo ati_vga_info = { > >>>> .name = TYPE_ATI_VGA, > >>>> .parent = TYPE_PCI_DEVICE, > >>>> .instance_size = sizeof(ATIVGAState), > >>>> .class_init = ati_vga_class_init, > >>>> + .instance_init = ati_vga_init, > >>>> .interfaces = (InterfaceInfo[]) { > >>>> { INTERFACE_CONVENTIONAL_PCI_DEVICE }, > >>>> { }, > >>>> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c > >>>> index 7d786653e8..0e6b8e4367 100644 > >>>> --- a/hw/display/ati_2d.c > >>>> +++ b/hw/display/ati_2d.c > >>>> @@ -92,6 +92,7 @@ void ati_2d_blt(ATIVGAState *s) > >>>> switch (s->regs.dp_mix & GMC_ROP3_MASK) { > >>>> case ROP3_SRCCOPY: > >>>> { > >>>> + bool fallback = false; > >>>> unsigned src_x = (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT ? > >>>> s->regs.src_x : s->regs.src_x + 1 - s->regs.dst_width); > >>>> unsigned src_y = (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ? > >>>> @@ -122,27 +123,50 @@ void ati_2d_blt(ATIVGAState *s) > >>>> src_bits, dst_bits, src_stride, dst_stride, bpp, bpp, > >>>> src_x, src_y, dst_x, dst_y, > >>>> s->regs.dst_width, s->regs.dst_height); > >>>> - if (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && > >>>> + if ((s->use_pixman & BIT(1)) && > >>>> + s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT && > >>>> s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { > >>>> - pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, > >>>> - src_stride, dst_stride, bpp, bpp, > >>>> - src_x, src_y, dst_x, dst_y, > >>>> - s->regs.dst_width, s->regs.dst_height); > >>>> - } else { > >>>> + fallback = !pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits, > >>>> + src_stride, dst_stride, bpp, bpp, > >>>> + src_x, src_y, dst_x, dst_y, > >>>> + s->regs.dst_width, s->regs.dst_height); > >>>> + } else if (s->use_pixman & BIT(1)) { > >>>> /* FIXME: We only really need a temporary if src and dst overlap */ > >>>> int llb = s->regs.dst_width * (bpp / 8); > >>>> int tmp_stride = DIV_ROUND_UP(llb, sizeof(uint32_t)); > >>>> uint32_t *tmp = g_malloc(tmp_stride * sizeof(uint32_t) * > >>>> s->regs.dst_height); > >>>> - pixman_blt((uint32_t *)src_bits, tmp, > >>>> - src_stride, tmp_stride, bpp, bpp, > >>>> - src_x, src_y, 0, 0, > >>>> - s->regs.dst_width, s->regs.dst_height); > >>>> - pixman_blt(tmp, (uint32_t *)dst_bits, > >>>> - tmp_stride, dst_stride, bpp, bpp, > >>>> - 0, 0, dst_x, dst_y, > >>>> - s->regs.dst_width, s->regs.dst_height); > >>>> + fallback = !pixman_blt((uint32_t *)src_bits, tmp, > >>>> + src_stride, tmp_stride, bpp, bpp, > >>>> + src_x, src_y, 0, 0, > >>>> + s->regs.dst_width, s->regs.dst_height); > >>>> + if (!fallback) { > >>>> + fallback = !pixman_blt(tmp, (uint32_t *)dst_bits, > >>>> + tmp_stride, dst_stride, bpp, bpp, > >>>> + 0, 0, dst_x, dst_y, > >>>> + s->regs.dst_width, s->regs.dst_height); > >>>> + } > >>>> g_free(tmp); > >>>> + } else { > >>>> + fallback = true; > >>>> + } > >>>> + if (fallback) { > >>>> + unsigned int y, i, j, bypp = bpp / 8; > >>>> + unsigned int src_pitch = src_stride * sizeof(uint32_t); > >>>> + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); > >>>> + > >>>> + for (y = 0; y < s->regs.dst_height; y++) { > >>>> + i = dst_x * bypp; > >>>> + j = src_x * bypp; > >>>> + if (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) { > >>>> + i += (dst_y + y) * dst_pitch; > >>>> + j += (src_y + y) * src_pitch; > >>>> + } else { > >>>> + i += (dst_y + s->regs.dst_height - 1 - y) * dst_pitch; > >>>> + j += (src_y + s->regs.dst_height - 1 - y) * src_pitch; > >>>> + } > >>>> + memmove(&dst_bits[i], &src_bits[j], s->regs.dst_width * bypp); > >>> > >>> This doesn't seem to handle overlapping regions the same as the > >>> pixman-version. Or am I missing something? > >> > >> memmove (as opposed to memcpy) allows overlapping regions and handles them > >> correctly so no temporary needed for this. I've tested it with MorphOS and > >> still got correct picture. > > > > But it is calling memmove() for each line, you may have overlapping > > rectangles. Having a temporary like above should solve this issue, > > assuming it's the correct behaviour. > > Lines are handled by DST_Y_TOP_TO_BOTTOM so they are processed either top > down or bottom up to avoid overlap. We only need temporary for pixman > because the only does top down left to right but here vertical direction > is handled by going through lines in the right order and horisontal > overlap is handled by memmove. No need for temporary in this case. This is > doing the same as sm501 so if you accept that works this is exactly the > same only the coords calculation is a bit different due to ati-vga storing > values differently. ok, thanks Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Regards, > BALATON Zoltan > > >>>> + } > >>>> } > >>>> if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && > >>>> dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + > >>>> @@ -180,14 +204,21 @@ void ati_2d_blt(ATIVGAState *s) > >>>> > >>>> dst_stride /= sizeof(uint32_t); > >>>> DPRINTF("pixman_fill(%p, %d, %d, %d, %d, %d, %d, %x)\n", > >>>> - dst_bits, dst_stride, bpp, > >>>> - dst_x, dst_y, > >>>> - s->regs.dst_width, s->regs.dst_height, > >>>> - filler); > >>>> - pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, > >>>> - dst_x, dst_y, > >>>> - s->regs.dst_width, s->regs.dst_height, > >>>> - filler); > >>>> + dst_bits, dst_stride, bpp, dst_x, dst_y, > >>>> + s->regs.dst_width, s->regs.dst_height, filler); > >>>> + if (!(s->use_pixman & BIT(0)) || > >>>> + !pixman_fill((uint32_t *)dst_bits, dst_stride, bpp, dst_x, dst_y, > >>>> + s->regs.dst_width, s->regs.dst_height, filler)) { > >>>> + /* fallback when pixman failed or we don't want to call it */ > >>>> + unsigned int x, y, i, bypp = bpp / 8; > >>>> + unsigned int dst_pitch = dst_stride * sizeof(uint32_t); > >>>> + for (y = 0; y < s->regs.dst_height; y++) { > >>>> + i = dst_x * bypp + (dst_y + y) * dst_pitch; > >>>> + for (x = 0; x < s->regs.dst_width; x++, i += bypp) { > >>>> + stn_he_p(&dst_bits[i], bypp, filler); > >>>> + } > >>>> + } > >>>> + } > >>>> if (dst_bits >= s->vga.vram_ptr + s->vga.vbe_start_addr && > >>>> dst_bits < s->vga.vram_ptr + s->vga.vbe_start_addr + > >>>> s->vga.vbe_regs[VBE_DISPI_INDEX_YRES] * s->vga.vbe_line_offset) { > >>>> diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h > >>>> index 8abb873f01..f5a47b82b0 100644 > >>>> --- a/hw/display/ati_int.h > >>>> +++ b/hw/display/ati_int.h > >>>> @@ -90,6 +90,7 @@ struct ATIVGAState { > >>>> char *model; > >>>> uint16_t dev_id; > >>>> uint8_t mode; > >>>> + uint8_t use_pixman; > >>>> bool cursor_guest_mode; > >>>> uint16_t cursor_size; > >>>> uint32_t cursor_offset; > >>>> -- > >>>> 2.30.9 > >>>> > >>>> > >>> > >>> > >>> > > > > > > > > -- Marc-André Lureau ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-11-07 17:29 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-01 20:45 [PATCH v2 0/4] Misc ati-vga patches BALATON Zoltan 2023-11-01 20:45 ` [PATCH v2 1/4] ati-vga: Fix aperture sizes BALATON Zoltan 2023-11-07 14:51 ` Michael Tokarev 2023-11-07 15:33 ` BALATON Zoltan 2023-11-07 17:27 ` Michael Tokarev 2023-11-01 20:45 ` [PATCH v2 2/4] ati-vga: Support unaligned access to GPIO DDC registers BALATON Zoltan 2023-11-01 20:45 ` [PATCH v2 3/4] ati-vga: Add 30 bit palette access register BALATON Zoltan 2023-11-01 20:45 ` [PATCH v2 4/4] ati-vga: Implement fallback for pixman routines BALATON Zoltan 2023-11-06 10:41 ` Marc-André Lureau 2023-11-06 11:02 ` BALATON Zoltan 2023-11-06 11:10 ` Marc-André Lureau 2023-11-06 11:17 ` BALATON Zoltan 2023-11-06 11:32 ` Marc-André Lureau
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).