* [PATCH v12 1/9] ati-vga: Remove src and dst stride mutation in ati_2d_blt
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
@ 2026-03-09 1:47 ` BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 2/9] ati-vga: Use local variables for register values " BALATON Zoltan
` (9 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: BALATON Zoltan @ 2026-03-09 1:47 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski,
Philippe Mathieu-Daudé
From: Chad Jablonski <chad@jablonski.xyz>
Pixman requires stride in words. So over the course of the ati_2d_blt
function both src and dst stride were mutated before being passed to
pixman and then back afterwards.
This creates local variables holding src and dst stride in words
avoiding the potentially confusing mutation.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
[balaton: Fix build without pixman]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati_2d.c | 48 +++++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 980cdd6ac0..8a7dcdf6e9 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -125,34 +125,34 @@ void ati_2d_blt(ATIVGAState *s)
return;
}
- src_stride /= sizeof(uint32_t);
- dst_stride /= sizeof(uint32_t);
- DPRINTF("pixman_blt(%p, %p, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d)\n",
- 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);
+ DPRINTF("pixman_blt(%p, %p, %ld, %ld, %d, %d, %d, %d, %d, %d, %d, %d)\n",
+ src_bits, dst_bits, src_stride / sizeof(uint32_t),
+ dst_stride / sizeof(uint32_t), bpp, bpp, src_x, src_y, dst_x,
+ dst_y, s->regs.dst_width, s->regs.dst_height);
#ifdef CONFIG_PIXMAN
+ int src_stride_words = src_stride / sizeof(uint32_t);
+ int dst_stride_words = dst_stride / sizeof(uint32_t);
if ((s->use_pixman & BIT(1)) &&
s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT &&
s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) {
fallback = !pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits,
- src_stride, dst_stride, bpp, bpp,
+ src_stride_words, dst_stride_words, 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) *
+ int tmp_stride_words = DIV_ROUND_UP(llb, sizeof(uint32_t));
+ uint32_t *tmp = g_malloc(tmp_stride_words * sizeof(uint32_t) *
s->regs.dst_height);
fallback = !pixman_blt((uint32_t *)src_bits, tmp,
- src_stride, tmp_stride, bpp, bpp,
+ src_stride_words, tmp_stride_words, 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,
+ tmp_stride_words, dst_stride_words,
+ bpp, bpp, 0, 0, dst_x, dst_y,
s->regs.dst_width, s->regs.dst_height);
}
g_free(tmp);
@@ -163,18 +163,15 @@ void ati_2d_blt(ATIVGAState *s)
}
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;
+ i += (dst_y + y) * dst_stride;
+ j += (src_y + y) * src_stride;
} else {
- i += (dst_y + s->regs.dst_height - 1 - y) * dst_pitch;
- j += (src_y + s->regs.dst_height - 1 - y) * src_pitch;
+ i += (dst_y + s->regs.dst_height - 1 - y) * dst_stride;
+ j += (src_y + s->regs.dst_height - 1 - y) * src_stride;
}
memmove(&dst_bits[i], &src_bits[j], s->regs.dst_width * bypp);
}
@@ -201,21 +198,20 @@ void ati_2d_blt(ATIVGAState *s)
break;
}
- 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,
+ DPRINTF("pixman_fill(%p, %ld, %d, %d, %d, %d, %d, %x)\n",
+ dst_bits, dst_stride / sizeof(uint32_t), bpp, dst_x, dst_y,
s->regs.dst_width, s->regs.dst_height, filler);
#ifdef CONFIG_PIXMAN
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))
+ !pixman_fill((uint32_t *)dst_bits, dst_stride / sizeof(uint32_t),
+ bpp, dst_x, dst_y, s->regs.dst_width,
+ s->regs.dst_height, filler))
#endif
{
/* 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;
+ i = dst_x * bypp + (dst_y + y) * dst_stride;
for (x = 0; x < s->regs.dst_width; x++, i += bypp) {
stn_he_p(&dst_bits[i], bypp, filler);
}
--
2.41.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v12 2/9] ati-vga: Use local variables for register values in ati_2d_blt
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 1/9] ati-vga: Remove src and dst stride mutation in ati_2d_blt BALATON Zoltan
@ 2026-03-09 1:47 ` BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 3/9] ati-vga: Introduce ATI2DCtx struct for 2D blit context BALATON Zoltan
` (8 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: BALATON Zoltan @ 2026-03-09 1:47 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski,
Philippe Mathieu-Daudé
From: Chad Jablonski <chad@jablonski.xyz>
ati_2d_blt uses a mixture of locals and direct register access of needed
state. This assigns all values derived from register state to local
variables. It prepares the function for a larger refactor that removes
the dependency on the full device and direct register access entirely.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
[balaton: Fix build without pixman]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati_2d.c | 92 +++++++++++++++++++++++----------------------
1 file changed, 48 insertions(+), 44 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 8a7dcdf6e9..b9cd766130 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -65,10 +65,18 @@ void ati_2d_blt(ATIVGAState *s)
{
/* FIXME it is probably more complex than this and may need to be */
/* rewritten but for now as a start just to get some output: */
- unsigned dst_x = (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT ?
- s->regs.dst_x : s->regs.dst_x + 1 - s->regs.dst_width);
- unsigned dst_y = (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ?
- s->regs.dst_y : s->regs.dst_y + 1 - s->regs.dst_height);
+ uint32_t rop3 = s->regs.dp_mix & GMC_ROP3_MASK;
+ bool left_to_right = s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT;
+ bool top_to_bottom = s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM;
+ uint32_t frgd_clr = s->regs.dp_brush_frgd_clr;
+ uint8_t *palette = s->vga.palette;
+ unsigned dst_offset = s->regs.dst_offset;
+ unsigned dst_width = s->regs.dst_width;
+ unsigned dst_height = s->regs.dst_height;
+ unsigned dst_x = (left_to_right ?
+ s->regs.dst_x : s->regs.dst_x + 1 - dst_width);
+ unsigned dst_y = (top_to_bottom ?
+ s->regs.dst_y : s->regs.dst_y + 1 - dst_height);
int bpp = ati_bpp_from_datatype(s);
if (!bpp) {
qemu_log_mask(LOG_GUEST_ERROR, "Invalid bpp\n");
@@ -79,7 +87,7 @@ void ati_2d_blt(ATIVGAState *s)
qemu_log_mask(LOG_GUEST_ERROR, "Zero dest pitch\n");
return;
}
- uint8_t *dst_bits = s->vga.vram_ptr + s->regs.dst_offset;
+ uint8_t *dst_bits = s->vga.vram_ptr + dst_offset;
if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
dst_bits += s->regs.crtc_offset & 0x07ffffff;
@@ -87,26 +95,25 @@ void ati_2d_blt(ATIVGAState *s)
}
uint8_t *end = s->vga.vram_ptr + s->vga.vram_size;
if (dst_x > 0x3fff || dst_y > 0x3fff || dst_bits >= end
- || dst_bits + dst_x
- + (dst_y + s->regs.dst_height) * dst_stride >= end) {
+ || dst_bits + dst_x + (dst_y + dst_height) * dst_stride >= end) {
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
return;
}
DPRINTF("%d %d %d, %d %d %d, (%d,%d) -> (%d,%d) %dx%d %c %c\n",
- s->regs.src_offset, s->regs.dst_offset, s->regs.default_offset,
- s->regs.src_pitch, s->regs.dst_pitch, s->regs.default_pitch,
+ s->regs.src_offset, dst_offset, s->regs.default_offset,
+ s->regs.src_pitch, dst_stride, s->regs.default_pitch,
s->regs.src_x, s->regs.src_y, dst_x, dst_y,
- s->regs.dst_width, s->regs.dst_height,
- (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT ? '>' : '<'),
- (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ? 'v' : '^'));
- switch (s->regs.dp_mix & GMC_ROP3_MASK) {
+ dst_width, dst_height,
+ (left_to_right ? '>' : '<'),
+ (top_to_bottom ? 'v' : '^'));
+ switch (rop3) {
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 ?
- s->regs.src_y : s->regs.src_y + 1 - s->regs.dst_height);
+ unsigned src_x = (left_to_right ?
+ s->regs.src_x : s->regs.src_x + 1 - dst_width);
+ unsigned src_y = (top_to_bottom ?
+ s->regs.src_y : s->regs.src_y + 1 - dst_height);
int src_stride = s->regs.src_pitch;
if (!src_stride) {
qemu_log_mask(LOG_GUEST_ERROR, "Zero source pitch\n");
@@ -120,7 +127,7 @@ void ati_2d_blt(ATIVGAState *s)
}
if (src_x > 0x3fff || src_y > 0x3fff || src_bits >= end
|| src_bits + src_x
- + (src_y + s->regs.dst_height) * src_stride >= end) {
+ + (src_y + dst_height) * src_stride >= end) {
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
return;
}
@@ -128,32 +135,30 @@ void ati_2d_blt(ATIVGAState *s)
DPRINTF("pixman_blt(%p, %p, %ld, %ld, %d, %d, %d, %d, %d, %d, %d, %d)\n",
src_bits, dst_bits, src_stride / sizeof(uint32_t),
dst_stride / sizeof(uint32_t), bpp, bpp, src_x, src_y, dst_x,
- dst_y, s->regs.dst_width, s->regs.dst_height);
+ dst_y, dst_width, dst_height);
#ifdef CONFIG_PIXMAN
int src_stride_words = src_stride / sizeof(uint32_t);
int dst_stride_words = dst_stride / sizeof(uint32_t);
- if ((s->use_pixman & BIT(1)) &&
- s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT &&
- s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) {
+ if ((s->use_pixman & BIT(1)) && left_to_right && top_to_bottom) {
fallback = !pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits,
src_stride_words, dst_stride_words, bpp, bpp,
src_x, src_y, dst_x, dst_y,
- s->regs.dst_width, s->regs.dst_height);
+ dst_width, 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 llb = dst_width * (bpp / 8);
int tmp_stride_words = DIV_ROUND_UP(llb, sizeof(uint32_t));
uint32_t *tmp = g_malloc(tmp_stride_words * sizeof(uint32_t) *
- s->regs.dst_height);
+ dst_height);
fallback = !pixman_blt((uint32_t *)src_bits, tmp,
src_stride_words, tmp_stride_words, bpp, bpp,
src_x, src_y, 0, 0,
- s->regs.dst_width, s->regs.dst_height);
+ dst_width, dst_height);
if (!fallback) {
fallback = !pixman_blt(tmp, (uint32_t *)dst_bits,
tmp_stride_words, dst_stride_words,
bpp, bpp, 0, 0, dst_x, dst_y,
- s->regs.dst_width, s->regs.dst_height);
+ dst_width, dst_height);
}
g_free(tmp);
} else
@@ -163,17 +168,17 @@ void ati_2d_blt(ATIVGAState *s)
}
if (fallback) {
unsigned int y, i, j, bypp = bpp / 8;
- for (y = 0; y < s->regs.dst_height; y++) {
+ for (y = 0; y < dst_height; y++) {
i = dst_x * bypp;
j = src_x * bypp;
- if (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM) {
+ if (top_to_bottom) {
i += (dst_y + y) * dst_stride;
j += (src_y + y) * src_stride;
} else {
- i += (dst_y + s->regs.dst_height - 1 - y) * dst_stride;
- j += (src_y + s->regs.dst_height - 1 - y) * src_stride;
+ i += (dst_y + dst_height - 1 - y) * dst_stride;
+ j += (src_y + dst_height - 1 - y) * src_stride;
}
- memmove(&dst_bits[i], &src_bits[j], s->regs.dst_width * bypp);
+ memmove(&dst_bits[i], &src_bits[j], dst_width * bypp);
}
}
break;
@@ -184,35 +189,34 @@ void ati_2d_blt(ATIVGAState *s)
{
uint32_t filler = 0;
- switch (s->regs.dp_mix & GMC_ROP3_MASK) {
+ switch (rop3) {
case ROP3_PATCOPY:
- filler = s->regs.dp_brush_frgd_clr;
+ filler = frgd_clr;
break;
case ROP3_BLACKNESS:
- filler = 0xffUL << 24 | rgb_to_pixel32(s->vga.palette[0],
- s->vga.palette[1], s->vga.palette[2]);
+ filler = 0xffUL << 24 | rgb_to_pixel32(palette[0], palette[1],
+ palette[2]);
break;
case ROP3_WHITENESS:
- filler = 0xffUL << 24 | rgb_to_pixel32(s->vga.palette[3],
- s->vga.palette[4], s->vga.palette[5]);
+ filler = 0xffUL << 24 | rgb_to_pixel32(palette[3], palette[4],
+ palette[5]);
break;
}
DPRINTF("pixman_fill(%p, %ld, %d, %d, %d, %d, %d, %x)\n",
dst_bits, dst_stride / sizeof(uint32_t), bpp, dst_x, dst_y,
- s->regs.dst_width, s->regs.dst_height, filler);
+ dst_width, dst_height, filler);
#ifdef CONFIG_PIXMAN
if (!(s->use_pixman & BIT(0)) ||
!pixman_fill((uint32_t *)dst_bits, dst_stride / sizeof(uint32_t),
- bpp, dst_x, dst_y, s->regs.dst_width,
- s->regs.dst_height, filler))
+ bpp, dst_x, dst_y, dst_width, dst_height, filler))
#endif
{
/* fallback when pixman failed or we don't want to call it */
unsigned int x, y, i, bypp = bpp / 8;
- for (y = 0; y < s->regs.dst_height; y++) {
+ for (y = 0; y < dst_height; y++) {
i = dst_x * bypp + (dst_y + y) * dst_stride;
- for (x = 0; x < s->regs.dst_width; x++, i += bypp) {
+ for (x = 0; x < dst_width; x++, i += bypp) {
stn_he_p(&dst_bits[i], bypp, filler);
}
}
@@ -221,7 +225,7 @@ void ati_2d_blt(ATIVGAState *s)
}
default:
qemu_log_mask(LOG_UNIMP, "Unimplemented ati_2d blt op %x\n",
- (s->regs.dp_mix & GMC_ROP3_MASK) >> 16);
+ rop3 >> 16);
return;
}
--
2.41.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v12 3/9] ati-vga: Introduce ATI2DCtx struct for 2D blit context
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 1/9] ati-vga: Remove src and dst stride mutation in ati_2d_blt BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 2/9] ati-vga: Use local variables for register values " BALATON Zoltan
@ 2026-03-09 1:47 ` BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 4/9] ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt BALATON Zoltan
` (7 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: BALATON Zoltan @ 2026-03-09 1:47 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski,
Philippe Mathieu-Daudé
From: Chad Jablonski <chad@jablonski.xyz>
Previously all state derived from registers was moved to locals. Now we
can mechanically replace those locals with fields on the new ATI2DCtx
struct.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
[balaton: Fix build without pixman]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati_2d.c | 226 +++++++++++++++++++++++++-------------------
1 file changed, 131 insertions(+), 95 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index b9cd766130..9e0115c49d 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -13,6 +13,7 @@
#include "qemu/log.h"
#include "ui/pixel_ops.h"
#include "ui/console.h"
+#include "ui/rect.h"
/*
* NOTE:
@@ -43,21 +44,39 @@ static int ati_bpp_from_datatype(ATIVGAState *s)
}
}
-static void ati_set_dirty(ATIVGAState *s,
- const uint8_t *dst_bits, unsigned dst_y)
+typedef struct {
+ int bpp;
+ uint32_t rop3;
+ bool left_to_right;
+ bool top_to_bottom;
+ uint32_t frgd_clr;
+ const uint8_t *palette;
+ const uint8_t *vram_end;
+
+ QemuRect dst;
+ int dst_stride;
+ uint8_t *dst_bits;
+ uint32_t dst_offset;
+
+ QemuRect src;
+ int src_stride;
+ const uint8_t *src_bits;
+} ATI2DCtx;
+
+static void ati_set_dirty(VGACommonState *vga, const ATI2DCtx *ctx)
{
- VGACommonState *vga = &s->vga;
DisplaySurface *ds = qemu_console_surface(vga->con);
DPRINTF("%p %u ds: %p %d %d rop: %x\n", vga->vram_ptr, vga->vbe_start_addr,
surface_data(ds), surface_stride(ds), surface_bits_per_pixel(ds),
- (s->regs.dp_mix & GMC_ROP3_MASK) >> 16);
- if (dst_bits >= vga->vram_ptr + vga->vbe_start_addr &&
- dst_bits < vga->vram_ptr + vga->vbe_start_addr +
+ ctx->rop3 >> 16);
+ if (ctx->dst_bits >= vga->vram_ptr + vga->vbe_start_addr &&
+ ctx->dst_bits < vga->vram_ptr + vga->vbe_start_addr +
vga->vbe_regs[VBE_DISPI_INDEX_YRES] * vga->vbe_line_offset) {
memory_region_set_dirty(&vga->vram, vga->vbe_start_addr +
- s->regs.dst_offset + dst_y * surface_stride(ds),
- s->regs.dst_height * surface_stride(ds));
+ ctx->dst_offset + ctx->dst.y
+ * surface_stride(ds),
+ ctx->dst.height * surface_stride(ds));
}
}
@@ -65,100 +84,109 @@ void ati_2d_blt(ATIVGAState *s)
{
/* FIXME it is probably more complex than this and may need to be */
/* rewritten but for now as a start just to get some output: */
- uint32_t rop3 = s->regs.dp_mix & GMC_ROP3_MASK;
- bool left_to_right = s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT;
- bool top_to_bottom = s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM;
- uint32_t frgd_clr = s->regs.dp_brush_frgd_clr;
- uint8_t *palette = s->vga.palette;
- unsigned dst_offset = s->regs.dst_offset;
- unsigned dst_width = s->regs.dst_width;
- unsigned dst_height = s->regs.dst_height;
- unsigned dst_x = (left_to_right ?
- s->regs.dst_x : s->regs.dst_x + 1 - dst_width);
- unsigned dst_y = (top_to_bottom ?
- s->regs.dst_y : s->regs.dst_y + 1 - dst_height);
- int bpp = ati_bpp_from_datatype(s);
- if (!bpp) {
+ ATI2DCtx ctx_;
+ ATI2DCtx *ctx = &ctx_;
+ ctx->rop3 = s->regs.dp_mix & GMC_ROP3_MASK;
+ ctx->left_to_right = s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT;
+ ctx->top_to_bottom = s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM;
+ ctx->frgd_clr = s->regs.dp_brush_frgd_clr;
+ ctx->palette = s->vga.palette;
+ ctx->dst_offset = s->regs.dst_offset;
+ ctx->dst.width = s->regs.dst_width;
+ ctx->dst.height = s->regs.dst_height;
+ ctx->dst.x = (ctx->left_to_right ?
+ s->regs.dst_x : s->regs.dst_x + 1 - ctx->dst.width);
+ ctx->dst.y = (ctx->top_to_bottom ?
+ s->regs.dst_y : s->regs.dst_y + 1 - ctx->dst.height);
+ ctx->bpp = ati_bpp_from_datatype(s);
+ if (!ctx->bpp) {
qemu_log_mask(LOG_GUEST_ERROR, "Invalid bpp\n");
return;
}
- int dst_stride = s->regs.dst_pitch;
- if (!dst_stride) {
+ ctx->dst_stride = s->regs.dst_pitch;
+ if (!ctx->dst_stride) {
qemu_log_mask(LOG_GUEST_ERROR, "Zero dest pitch\n");
return;
}
- uint8_t *dst_bits = s->vga.vram_ptr + dst_offset;
+ ctx->dst_bits = s->vga.vram_ptr + ctx->dst_offset;
if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
- dst_bits += s->regs.crtc_offset & 0x07ffffff;
- dst_stride *= bpp;
+ ctx->dst_bits += s->regs.crtc_offset & 0x07ffffff;
+ ctx->dst_stride *= ctx->bpp;
}
- uint8_t *end = s->vga.vram_ptr + s->vga.vram_size;
- if (dst_x > 0x3fff || dst_y > 0x3fff || dst_bits >= end
- || dst_bits + dst_x + (dst_y + dst_height) * dst_stride >= end) {
+ ctx->vram_end = s->vga.vram_ptr + s->vga.vram_size;
+ if (ctx->dst.x > 0x3fff || ctx->dst.y > 0x3fff ||
+ ctx->dst_bits >= ctx->vram_end || ctx->dst_bits + ctx->dst.x +
+ (ctx->dst.y + ctx->dst.height) * ctx->dst_stride >= ctx->vram_end) {
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
return;
}
DPRINTF("%d %d %d, %d %d %d, (%d,%d) -> (%d,%d) %dx%d %c %c\n",
- s->regs.src_offset, dst_offset, s->regs.default_offset,
- s->regs.src_pitch, dst_stride, s->regs.default_pitch,
- s->regs.src_x, s->regs.src_y, dst_x, dst_y,
- dst_width, dst_height,
- (left_to_right ? '>' : '<'),
- (top_to_bottom ? 'v' : '^'));
- switch (rop3) {
+ s->regs.src_offset, ctx->dst_offset, s->regs.default_offset,
+ ctx->src_stride, ctx->dst_stride, s->regs.default_pitch,
+ ctx->src.x, ctx->src.y, ctx->dst.x, ctx->dst.y,
+ ctx->dst.width, ctx->dst.height,
+ (ctx->left_to_right ? '>' : '<'),
+ (ctx->top_to_bottom ? 'v' : '^'));
+ switch (ctx->rop3) {
case ROP3_SRCCOPY:
{
bool fallback = false;
- unsigned src_x = (left_to_right ?
- s->regs.src_x : s->regs.src_x + 1 - dst_width);
- unsigned src_y = (top_to_bottom ?
- s->regs.src_y : s->regs.src_y + 1 - dst_height);
- int src_stride = s->regs.src_pitch;
- if (!src_stride) {
+ ctx->src.x = (ctx->left_to_right ?
+ s->regs.src_x : s->regs.src_x + 1 - ctx->dst.width);
+ ctx->src.y = (ctx->top_to_bottom ?
+ s->regs.src_y : s->regs.src_y + 1 - ctx->dst.height);
+ ctx->src_stride = s->regs.src_pitch;
+ if (!ctx->src_stride) {
qemu_log_mask(LOG_GUEST_ERROR, "Zero source pitch\n");
return;
}
- uint8_t *src_bits = s->vga.vram_ptr + s->regs.src_offset;
+ ctx->src_bits = s->vga.vram_ptr + s->regs.src_offset;
if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
- src_bits += s->regs.crtc_offset & 0x07ffffff;
- src_stride *= bpp;
+ ctx->src_bits += s->regs.crtc_offset & 0x07ffffff;
+ ctx->src_stride *= ctx->bpp;
}
- if (src_x > 0x3fff || src_y > 0x3fff || src_bits >= end
- || src_bits + src_x
- + (src_y + dst_height) * src_stride >= end) {
+ if (ctx->src.x > 0x3fff || ctx->src.y > 0x3fff ||
+ ctx->src_bits >= ctx->vram_end ||
+ ctx->src_bits + ctx->src.x + (ctx->src.y + ctx->dst.height) *
+ ctx->src_stride >= ctx->vram_end) {
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
return;
}
DPRINTF("pixman_blt(%p, %p, %ld, %ld, %d, %d, %d, %d, %d, %d, %d, %d)\n",
- src_bits, dst_bits, src_stride / sizeof(uint32_t),
- dst_stride / sizeof(uint32_t), bpp, bpp, src_x, src_y, dst_x,
- dst_y, dst_width, dst_height);
+ ctx->src_bits, ctx->dst_bits,
+ ctx->src_stride / sizeof(uint32_t),
+ ctx->dst_stride / sizeof(uint32_t),
+ ctx->bpp, ctx->bpp, ctx->src.x, ctx->src.y, ctx->dst.x,
+ ctx->dst.y, ctx->dst.width, ctx->dst.height);
#ifdef CONFIG_PIXMAN
- int src_stride_words = src_stride / sizeof(uint32_t);
- int dst_stride_words = dst_stride / sizeof(uint32_t);
- if ((s->use_pixman & BIT(1)) && left_to_right && top_to_bottom) {
- fallback = !pixman_blt((uint32_t *)src_bits, (uint32_t *)dst_bits,
- src_stride_words, dst_stride_words, bpp, bpp,
- src_x, src_y, dst_x, dst_y,
- dst_width, dst_height);
+ int src_stride_words = ctx->src_stride / sizeof(uint32_t);
+ int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
+ if ((s->use_pixman & BIT(1)) &&
+ ctx->left_to_right && ctx->top_to_bottom) {
+ fallback = !pixman_blt((uint32_t *)ctx->src_bits,
+ (uint32_t *)ctx->dst_bits, src_stride_words,
+ dst_stride_words, ctx->bpp, ctx->bpp,
+ ctx->src.x, ctx->src.y, ctx->dst.x,
+ ctx->dst.y, ctx->dst.width, ctx->dst.height);
} else if (s->use_pixman & BIT(1)) {
/* FIXME: We only really need a temporary if src and dst overlap */
- int llb = dst_width * (bpp / 8);
+ int llb = ctx->dst.width * (ctx->bpp / 8);
int tmp_stride_words = DIV_ROUND_UP(llb, sizeof(uint32_t));
uint32_t *tmp = g_malloc(tmp_stride_words * sizeof(uint32_t) *
- dst_height);
- fallback = !pixman_blt((uint32_t *)src_bits, tmp,
- src_stride_words, tmp_stride_words, bpp, bpp,
- src_x, src_y, 0, 0,
- dst_width, dst_height);
+ ctx->dst.height);
+ fallback = !pixman_blt((uint32_t *)ctx->src_bits, tmp,
+ src_stride_words, tmp_stride_words, ctx->bpp,
+ ctx->bpp, ctx->src.x, ctx->src.y, 0, 0,
+ ctx->dst.width, ctx->dst.height);
if (!fallback) {
- fallback = !pixman_blt(tmp, (uint32_t *)dst_bits,
+ fallback = !pixman_blt(tmp, (uint32_t *)ctx->dst_bits,
tmp_stride_words, dst_stride_words,
- bpp, bpp, 0, 0, dst_x, dst_y,
- dst_width, dst_height);
+ ctx->bpp, ctx->bpp, 0, 0,
+ ctx->dst.x, ctx->dst.y,
+ ctx->dst.width, ctx->dst.height);
}
g_free(tmp);
} else
@@ -167,18 +195,21 @@ void ati_2d_blt(ATIVGAState *s)
fallback = true;
}
if (fallback) {
- unsigned int y, i, j, bypp = bpp / 8;
- for (y = 0; y < dst_height; y++) {
- i = dst_x * bypp;
- j = src_x * bypp;
- if (top_to_bottom) {
- i += (dst_y + y) * dst_stride;
- j += (src_y + y) * src_stride;
+ unsigned int y, i, j, bypp = ctx->bpp / 8;
+ for (y = 0; y < ctx->dst.height; y++) {
+ i = ctx->dst.x * bypp;
+ j = ctx->src.x * bypp;
+ if (ctx->top_to_bottom) {
+ i += (ctx->dst.y + y) * ctx->dst_stride;
+ j += (ctx->src.y + y) * ctx->src_stride;
} else {
- i += (dst_y + dst_height - 1 - y) * dst_stride;
- j += (src_y + dst_height - 1 - y) * src_stride;
+ i += (ctx->dst.y + ctx->dst.height - 1 - y)
+ * ctx->dst_stride;
+ j += (ctx->src.y + ctx->dst.height - 1 - y)
+ * ctx->src_stride;
}
- memmove(&dst_bits[i], &src_bits[j], dst_width * bypp);
+ memmove(&ctx->dst_bits[i], &ctx->src_bits[j],
+ ctx->dst.width * bypp);
}
}
break;
@@ -189,35 +220,40 @@ void ati_2d_blt(ATIVGAState *s)
{
uint32_t filler = 0;
- switch (rop3) {
+ switch (ctx->rop3) {
case ROP3_PATCOPY:
- filler = frgd_clr;
+ filler = ctx->frgd_clr;
break;
case ROP3_BLACKNESS:
- filler = 0xffUL << 24 | rgb_to_pixel32(palette[0], palette[1],
- palette[2]);
+ filler = 0xffUL << 24 | rgb_to_pixel32(ctx->palette[0],
+ ctx->palette[1],
+ ctx->palette[2]);
break;
case ROP3_WHITENESS:
- filler = 0xffUL << 24 | rgb_to_pixel32(palette[3], palette[4],
- palette[5]);
+ filler = 0xffUL << 24 | rgb_to_pixel32(ctx->palette[3],
+ ctx->palette[4],
+ ctx->palette[5]);
break;
}
DPRINTF("pixman_fill(%p, %ld, %d, %d, %d, %d, %d, %x)\n",
- dst_bits, dst_stride / sizeof(uint32_t), bpp, dst_x, dst_y,
- dst_width, dst_height, filler);
+ ctx->dst_bits, ctx->dst_stride / sizeof(uint32_t), ctx->bpp,
+ ctx->dst.x, ctx->dst.y, ctx->dst.width, ctx->dst.height,
+ filler);
#ifdef CONFIG_PIXMAN
if (!(s->use_pixman & BIT(0)) ||
- !pixman_fill((uint32_t *)dst_bits, dst_stride / sizeof(uint32_t),
- bpp, dst_x, dst_y, dst_width, dst_height, filler))
+ !pixman_fill((uint32_t *)ctx->dst_bits,
+ ctx->dst_stride / sizeof(uint32_t), ctx->bpp,
+ ctx->dst.x, ctx->dst.y,
+ ctx->dst.width, ctx->dst.height, filler))
#endif
{
/* fallback when pixman failed or we don't want to call it */
- unsigned int x, y, i, bypp = bpp / 8;
- for (y = 0; y < dst_height; y++) {
- i = dst_x * bypp + (dst_y + y) * dst_stride;
- for (x = 0; x < dst_width; x++, i += bypp) {
- stn_he_p(&dst_bits[i], bypp, filler);
+ unsigned int x, y, i, bypp = ctx->bpp / 8;
+ for (y = 0; y < ctx->dst.height; y++) {
+ i = ctx->dst.x * bypp + (ctx->dst.y + y) * ctx->dst_stride;
+ for (x = 0; x < ctx->dst.width; x++, i += bypp) {
+ stn_he_p(&ctx->dst_bits[i], bypp, filler);
}
}
}
@@ -225,9 +261,9 @@ void ati_2d_blt(ATIVGAState *s)
}
default:
qemu_log_mask(LOG_UNIMP, "Unimplemented ati_2d blt op %x\n",
- rop3 >> 16);
+ ctx->rop3 >> 16);
return;
}
- ati_set_dirty(s, dst_bits, dst_y);
+ ati_set_dirty(&s->vga, ctx);
}
--
2.41.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v12 4/9] ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
` (2 preceding siblings ...)
2026-03-09 1:47 ` [PATCH v12 3/9] ati-vga: Introduce ATI2DCtx struct for 2D blit context BALATON Zoltan
@ 2026-03-09 1:47 ` BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 5/9] ati-vga: Split ati_2d_do_blt " BALATON Zoltan
` (6 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: BALATON Zoltan @ 2026-03-09 1:47 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski,
Philippe Mathieu-Daudé
From: Chad Jablonski <chad@jablonski.xyz>
setup_2d_blt_ctx is responsible for knowing how to retrieve the state
needed by ati_2d_blt from the registers and assigning it to the ATI2DCtx.
This will be useful in a future patch when HOST_DATA needs to make small
modifications to the ctx.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
[balaton: Fix build without pixman]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati_2d.c | 69 +++++++++++++++++++++++----------------------
1 file changed, 36 insertions(+), 33 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 9e0115c49d..a83cf14a4a 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -25,7 +25,7 @@
* possible.
*/
-static int ati_bpp_from_datatype(ATIVGAState *s)
+static int ati_bpp_from_datatype(const ATIVGAState *s)
{
switch (s->regs.dp_datatype & 0xf) {
case 2:
@@ -80,73 +80,76 @@ static void ati_set_dirty(VGACommonState *vga, const ATI2DCtx *ctx)
}
}
-void ati_2d_blt(ATIVGAState *s)
+static void setup_2d_blt_ctx(const ATIVGAState *s, ATI2DCtx *ctx)
{
- /* FIXME it is probably more complex than this and may need to be */
- /* rewritten but for now as a start just to get some output: */
- ATI2DCtx ctx_;
- ATI2DCtx *ctx = &ctx_;
+ ctx->bpp = ati_bpp_from_datatype(s);
ctx->rop3 = s->regs.dp_mix & GMC_ROP3_MASK;
ctx->left_to_right = s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT;
ctx->top_to_bottom = s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM;
ctx->frgd_clr = s->regs.dp_brush_frgd_clr;
ctx->palette = s->vga.palette;
ctx->dst_offset = s->regs.dst_offset;
+ ctx->vram_end = s->vga.vram_ptr + s->vga.vram_size;
+
ctx->dst.width = s->regs.dst_width;
ctx->dst.height = s->regs.dst_height;
ctx->dst.x = (ctx->left_to_right ?
s->regs.dst_x : s->regs.dst_x + 1 - ctx->dst.width);
ctx->dst.y = (ctx->top_to_bottom ?
s->regs.dst_y : s->regs.dst_y + 1 - ctx->dst.height);
- ctx->bpp = ati_bpp_from_datatype(s);
+ ctx->dst_stride = s->regs.dst_pitch;
+ ctx->dst_bits = s->vga.vram_ptr + s->regs.dst_offset;
+ if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
+ ctx->dst_bits += s->regs.crtc_offset & 0x07ffffff;
+ ctx->dst_stride *= ctx->bpp;
+ }
+
+ ctx->src.x = (ctx->left_to_right ?
+ s->regs.src_x : s->regs.src_x + 1 - ctx->dst.width);
+ ctx->src.y = (ctx->top_to_bottom ?
+ s->regs.src_y : s->regs.src_y + 1 - ctx->dst.height);
+ ctx->src_stride = s->regs.src_pitch;
+ ctx->src_bits = s->vga.vram_ptr + s->regs.src_offset;
+ if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
+ ctx->src_bits += s->regs.crtc_offset & 0x07ffffff;
+ ctx->src_stride *= ctx->bpp;
+ }
+ DPRINTF("%d %d %d, %d %d %d, (%d,%d) -> (%d,%d) %dx%d %c %c\n",
+ s->regs.src_offset, s->regs.dst_offset, s->regs.default_offset,
+ ctx->src_stride, ctx->dst_stride, s->regs.default_pitch,
+ ctx->src.x, ctx->src.y, ctx->dst.x, ctx->dst.y,
+ ctx->dst.width, ctx->dst.height,
+ (ctx->left_to_right ? '>' : '<'),
+ (ctx->top_to_bottom ? 'v' : '^'));
+}
+
+void ati_2d_blt(ATIVGAState *s)
+{
+ ATI2DCtx ctx_;
+ ATI2DCtx *ctx = &ctx_;
+ setup_2d_blt_ctx(s, ctx);
if (!ctx->bpp) {
qemu_log_mask(LOG_GUEST_ERROR, "Invalid bpp\n");
return;
}
- ctx->dst_stride = s->regs.dst_pitch;
if (!ctx->dst_stride) {
qemu_log_mask(LOG_GUEST_ERROR, "Zero dest pitch\n");
return;
}
- ctx->dst_bits = s->vga.vram_ptr + ctx->dst_offset;
-
- if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
- ctx->dst_bits += s->regs.crtc_offset & 0x07ffffff;
- ctx->dst_stride *= ctx->bpp;
- }
- ctx->vram_end = s->vga.vram_ptr + s->vga.vram_size;
if (ctx->dst.x > 0x3fff || ctx->dst.y > 0x3fff ||
ctx->dst_bits >= ctx->vram_end || ctx->dst_bits + ctx->dst.x +
(ctx->dst.y + ctx->dst.height) * ctx->dst_stride >= ctx->vram_end) {
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
return;
}
- DPRINTF("%d %d %d, %d %d %d, (%d,%d) -> (%d,%d) %dx%d %c %c\n",
- s->regs.src_offset, ctx->dst_offset, s->regs.default_offset,
- ctx->src_stride, ctx->dst_stride, s->regs.default_pitch,
- ctx->src.x, ctx->src.y, ctx->dst.x, ctx->dst.y,
- ctx->dst.width, ctx->dst.height,
- (ctx->left_to_right ? '>' : '<'),
- (ctx->top_to_bottom ? 'v' : '^'));
switch (ctx->rop3) {
case ROP3_SRCCOPY:
{
bool fallback = false;
- ctx->src.x = (ctx->left_to_right ?
- s->regs.src_x : s->regs.src_x + 1 - ctx->dst.width);
- ctx->src.y = (ctx->top_to_bottom ?
- s->regs.src_y : s->regs.src_y + 1 - ctx->dst.height);
- ctx->src_stride = s->regs.src_pitch;
if (!ctx->src_stride) {
qemu_log_mask(LOG_GUEST_ERROR, "Zero source pitch\n");
return;
}
- ctx->src_bits = s->vga.vram_ptr + s->regs.src_offset;
-
- if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
- ctx->src_bits += s->regs.crtc_offset & 0x07ffffff;
- ctx->src_stride *= ctx->bpp;
- }
if (ctx->src.x > 0x3fff || ctx->src.y > 0x3fff ||
ctx->src_bits >= ctx->vram_end ||
ctx->src_bits + ctx->src.x + (ctx->src.y + ctx->dst.height) *
--
2.41.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v12 5/9] ati-vga: Split ati_2d_do_blt from ati_2d_blt
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
` (3 preceding siblings ...)
2026-03-09 1:47 ` [PATCH v12 4/9] ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt BALATON Zoltan
@ 2026-03-09 1:47 ` BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 6/9] ati-vga: Remove ATIVGAState param from ati_2d_do_blt BALATON Zoltan
` (5 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: BALATON Zoltan @ 2026-03-09 1:47 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski,
Philippe Mathieu-Daudé
From: Chad Jablonski <chad@jablonski.xyz>
ati_2d_blt remains the public interface to the blitter but the bulk of
the implementation is moved down into ati_2d_do_blt which is passed an
ATI2DCtx.
ati_2d_do_blt returns a bool that is true when the blit succeeded, which
means that a screen region will need to be set dirty. Otherwise false is
returned.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
[balaton: Fix build without pixman]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati_2d.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index a83cf14a4a..0475fcb585 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -123,24 +123,21 @@ static void setup_2d_blt_ctx(const ATIVGAState *s, ATI2DCtx *ctx)
(ctx->top_to_bottom ? 'v' : '^'));
}
-void ati_2d_blt(ATIVGAState *s)
+static bool ati_2d_do_blt(ATIVGAState *s, ATI2DCtx *ctx)
{
- ATI2DCtx ctx_;
- ATI2DCtx *ctx = &ctx_;
- setup_2d_blt_ctx(s, ctx);
if (!ctx->bpp) {
qemu_log_mask(LOG_GUEST_ERROR, "Invalid bpp\n");
- return;
+ return false;
}
if (!ctx->dst_stride) {
qemu_log_mask(LOG_GUEST_ERROR, "Zero dest pitch\n");
- return;
+ return false;
}
if (ctx->dst.x > 0x3fff || ctx->dst.y > 0x3fff ||
ctx->dst_bits >= ctx->vram_end || ctx->dst_bits + ctx->dst.x +
(ctx->dst.y + ctx->dst.height) * ctx->dst_stride >= ctx->vram_end) {
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
- return;
+ return false;
}
switch (ctx->rop3) {
case ROP3_SRCCOPY:
@@ -148,14 +145,14 @@ void ati_2d_blt(ATIVGAState *s)
bool fallback = false;
if (!ctx->src_stride) {
qemu_log_mask(LOG_GUEST_ERROR, "Zero source pitch\n");
- return;
+ return false;
}
if (ctx->src.x > 0x3fff || ctx->src.y > 0x3fff ||
ctx->src_bits >= ctx->vram_end ||
ctx->src_bits + ctx->src.x + (ctx->src.y + ctx->dst.height) *
ctx->src_stride >= ctx->vram_end) {
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
- return;
+ return false;
}
DPRINTF("pixman_blt(%p, %p, %ld, %ld, %d, %d, %d, %d, %d, %d, %d, %d)\n",
@@ -265,8 +262,17 @@ void ati_2d_blt(ATIVGAState *s)
default:
qemu_log_mask(LOG_UNIMP, "Unimplemented ati_2d blt op %x\n",
ctx->rop3 >> 16);
- return;
+ return false;
}
- ati_set_dirty(&s->vga, ctx);
+ return true;
+}
+
+void ati_2d_blt(ATIVGAState *s)
+{
+ ATI2DCtx ctx;
+ setup_2d_blt_ctx(s, &ctx);
+ if (ati_2d_do_blt(s, &ctx)) {
+ ati_set_dirty(&s->vga, &ctx);
+ }
}
--
2.41.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v12 6/9] ati-vga: Remove ATIVGAState param from ati_2d_do_blt
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
` (4 preceding siblings ...)
2026-03-09 1:47 ` [PATCH v12 5/9] ati-vga: Split ati_2d_do_blt " BALATON Zoltan
@ 2026-03-09 1:47 ` BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 7/9] ati-vga: Implement scissor rectangle clipping for 2D operations BALATON Zoltan
` (4 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: BALATON Zoltan @ 2026-03-09 1:47 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski,
Philippe Mathieu-Daudé
From: Chad Jablonski <chad@jablonski.xyz>
This completes the decoupling from the ATIVGAState struct.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
[balaton: Fix build without pixman]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati_2d.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 0475fcb585..1dfad730ba 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -123,7 +123,7 @@ static void setup_2d_blt_ctx(const ATIVGAState *s, ATI2DCtx *ctx)
(ctx->top_to_bottom ? 'v' : '^'));
}
-static bool ati_2d_do_blt(ATIVGAState *s, ATI2DCtx *ctx)
+static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
{
if (!ctx->bpp) {
qemu_log_mask(LOG_GUEST_ERROR, "Invalid bpp\n");
@@ -164,14 +164,14 @@ static bool ati_2d_do_blt(ATIVGAState *s, ATI2DCtx *ctx)
#ifdef CONFIG_PIXMAN
int src_stride_words = ctx->src_stride / sizeof(uint32_t);
int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
- if ((s->use_pixman & BIT(1)) &&
+ if ((use_pixman & BIT(1)) &&
ctx->left_to_right && ctx->top_to_bottom) {
fallback = !pixman_blt((uint32_t *)ctx->src_bits,
(uint32_t *)ctx->dst_bits, src_stride_words,
dst_stride_words, ctx->bpp, ctx->bpp,
ctx->src.x, ctx->src.y, ctx->dst.x,
ctx->dst.y, ctx->dst.width, ctx->dst.height);
- } else if (s->use_pixman & BIT(1)) {
+ } else if (use_pixman & BIT(1)) {
/* FIXME: We only really need a temporary if src and dst overlap */
int llb = ctx->dst.width * (ctx->bpp / 8);
int tmp_stride_words = DIV_ROUND_UP(llb, sizeof(uint32_t));
@@ -241,7 +241,7 @@ static bool ati_2d_do_blt(ATIVGAState *s, ATI2DCtx *ctx)
ctx->dst.x, ctx->dst.y, ctx->dst.width, ctx->dst.height,
filler);
#ifdef CONFIG_PIXMAN
- if (!(s->use_pixman & BIT(0)) ||
+ if (!(use_pixman & BIT(0)) ||
!pixman_fill((uint32_t *)ctx->dst_bits,
ctx->dst_stride / sizeof(uint32_t), ctx->bpp,
ctx->dst.x, ctx->dst.y,
@@ -272,7 +272,7 @@ void ati_2d_blt(ATIVGAState *s)
{
ATI2DCtx ctx;
setup_2d_blt_ctx(s, &ctx);
- if (ati_2d_do_blt(s, &ctx)) {
+ if (ati_2d_do_blt(&ctx, s->use_pixman)) {
ati_set_dirty(&s->vga, &ctx);
}
}
--
2.41.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v12 7/9] ati-vga: Implement scissor rectangle clipping for 2D operations
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
` (5 preceding siblings ...)
2026-03-09 1:47 ` [PATCH v12 6/9] ati-vga: Remove ATIVGAState param from ati_2d_do_blt BALATON Zoltan
@ 2026-03-09 1:47 ` BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 8/9] ati-vga: Implement HOST_DATA register writes BALATON Zoltan
` (3 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: BALATON Zoltan @ 2026-03-09 1:47 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski,
Philippe Mathieu-Daudé
From: Chad Jablonski <chad@jablonski.xyz>
Use scissor registers to clip blit operations. This is required
for text rendering in X using the r128 driver. Without it overly-wide
glyphs are drawn and create all sorts of chaos.
The visible destination rectangle (vis_dst) is the intersection of the
scissor rectangle and the destination rectangle (dst).
The src also needs to be offset if clipped on the top and/or
left sides to ensure that src data is read correctly and appears
clipped when drawn rather than shifted.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
[balaton: Fix build without pixman]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati_2d.c | 87 ++++++++++++++++++++++++++++++---------------
1 file changed, 58 insertions(+), 29 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 1dfad730ba..d62ad849f3 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -52,6 +52,7 @@ typedef struct {
uint32_t frgd_clr;
const uint8_t *palette;
const uint8_t *vram_end;
+ QemuRect scissor;
QemuRect dst;
int dst_stride;
@@ -91,6 +92,11 @@ static void setup_2d_blt_ctx(const ATIVGAState *s, ATI2DCtx *ctx)
ctx->dst_offset = s->regs.dst_offset;
ctx->vram_end = s->vga.vram_ptr + s->vga.vram_size;
+ ctx->scissor.width = s->regs.sc_right - s->regs.sc_left + 1;
+ ctx->scissor.height = s->regs.sc_bottom - s->regs.sc_top + 1;
+ ctx->scissor.x = s->regs.sc_left;
+ ctx->scissor.y = s->regs.sc_top;
+
ctx->dst.width = s->regs.dst_width;
ctx->dst.height = s->regs.dst_height;
ctx->dst.x = (ctx->left_to_right ?
@@ -125,6 +131,8 @@ static void setup_2d_blt_ctx(const ATIVGAState *s, ATI2DCtx *ctx)
static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
{
+ QemuRect vis_src, vis_dst;
+
if (!ctx->bpp) {
qemu_log_mask(LOG_GUEST_ERROR, "Invalid bpp\n");
return false;
@@ -139,6 +147,29 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
return false;
}
+ qemu_rect_intersect(&ctx->dst, &ctx->scissor, &vis_dst);
+ if (!vis_dst.height || !vis_dst.width) {
+ /* Nothing is visible, completely clipped */
+ return false;
+ }
+ /*
+ * The src must be offset if clipping is applied to the dst.
+ * This is so that when the source is blit to a dst clipped
+ * on the top or left the src image is not shifted into the
+ * clipped region but actually clipped.
+ */
+ vis_src.x = ctx->src.x + (vis_dst.x - ctx->dst.x);
+ vis_src.y = ctx->src.y + (vis_dst.y - ctx->dst.y);
+ vis_src.width = vis_dst.width;
+ vis_src.height = vis_dst.height;
+
+ DPRINTF("dst: (%d,%d) %dx%d -> vis_dst: (%d,%d) %dx%d\n",
+ ctx->dst.x, ctx->dst.y, ctx->dst.width, ctx->dst.height,
+ vis_dst.x, vis_dst.y, vis_dst.width, vis_dst.height);
+ DPRINTF("src: (%d,%d) %dx%d -> vis_src: (%d,%d) %dx%d\n",
+ ctx->src.x, ctx->src.y, ctx->dst.width, ctx->dst.height,
+ vis_src.x, vis_src.y, vis_src.width, vis_src.height);
+
switch (ctx->rop3) {
case ROP3_SRCCOPY:
{
@@ -147,10 +178,9 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
qemu_log_mask(LOG_GUEST_ERROR, "Zero source pitch\n");
return false;
}
- if (ctx->src.x > 0x3fff || ctx->src.y > 0x3fff ||
- ctx->src_bits >= ctx->vram_end ||
- ctx->src_bits + ctx->src.x + (ctx->src.y + ctx->dst.height) *
- ctx->src_stride >= ctx->vram_end) {
+ if (vis_src.x > 0x3fff || vis_src.y > 0x3fff ||
+ ctx->src_bits >= ctx->vram_end || ctx->src_bits + vis_src.x +
+ (vis_src.y + vis_dst.height) * ctx->src_stride >= ctx->vram_end) {
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
return false;
}
@@ -159,8 +189,8 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
ctx->src_bits, ctx->dst_bits,
ctx->src_stride / sizeof(uint32_t),
ctx->dst_stride / sizeof(uint32_t),
- ctx->bpp, ctx->bpp, ctx->src.x, ctx->src.y, ctx->dst.x,
- ctx->dst.y, ctx->dst.width, ctx->dst.height);
+ ctx->bpp, ctx->bpp, vis_src.x, vis_src.y, vis_dst.x, vis_dst.y,
+ vis_dst.width, vis_dst.height);
#ifdef CONFIG_PIXMAN
int src_stride_words = ctx->src_stride / sizeof(uint32_t);
int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
@@ -169,24 +199,24 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
fallback = !pixman_blt((uint32_t *)ctx->src_bits,
(uint32_t *)ctx->dst_bits, src_stride_words,
dst_stride_words, ctx->bpp, ctx->bpp,
- ctx->src.x, ctx->src.y, ctx->dst.x,
- ctx->dst.y, ctx->dst.width, ctx->dst.height);
+ vis_src.x, vis_src.y, vis_dst.x, vis_dst.y,
+ vis_dst.width, vis_dst.height);
} else if (use_pixman & BIT(1)) {
/* FIXME: We only really need a temporary if src and dst overlap */
- int llb = ctx->dst.width * (ctx->bpp / 8);
+ int llb = vis_dst.width * (ctx->bpp / 8);
int tmp_stride_words = DIV_ROUND_UP(llb, sizeof(uint32_t));
uint32_t *tmp = g_malloc(tmp_stride_words * sizeof(uint32_t) *
- ctx->dst.height);
+ vis_dst.height);
fallback = !pixman_blt((uint32_t *)ctx->src_bits, tmp,
src_stride_words, tmp_stride_words, ctx->bpp,
- ctx->bpp, ctx->src.x, ctx->src.y, 0, 0,
- ctx->dst.width, ctx->dst.height);
+ ctx->bpp, vis_src.x, vis_src.y, 0, 0,
+ vis_dst.width, vis_dst.height);
if (!fallback) {
fallback = !pixman_blt(tmp, (uint32_t *)ctx->dst_bits,
tmp_stride_words, dst_stride_words,
ctx->bpp, ctx->bpp, 0, 0,
- ctx->dst.x, ctx->dst.y,
- ctx->dst.width, ctx->dst.height);
+ vis_dst.x, vis_dst.y,
+ vis_dst.width, vis_dst.height);
}
g_free(tmp);
} else
@@ -196,20 +226,20 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
}
if (fallback) {
unsigned int y, i, j, bypp = ctx->bpp / 8;
- for (y = 0; y < ctx->dst.height; y++) {
- i = ctx->dst.x * bypp;
- j = ctx->src.x * bypp;
+ for (y = 0; y < vis_dst.height; y++) {
+ i = vis_dst.x * bypp;
+ j = vis_src.x * bypp;
if (ctx->top_to_bottom) {
- i += (ctx->dst.y + y) * ctx->dst_stride;
- j += (ctx->src.y + y) * ctx->src_stride;
+ i += (vis_dst.y + y) * ctx->dst_stride;
+ j += (vis_src.y + y) * ctx->src_stride;
} else {
- i += (ctx->dst.y + ctx->dst.height - 1 - y)
+ i += (vis_dst.y + vis_dst.height - 1 - y)
* ctx->dst_stride;
- j += (ctx->src.y + ctx->dst.height - 1 - y)
+ j += (vis_src.y + vis_dst.height - 1 - y)
* ctx->src_stride;
}
memmove(&ctx->dst_bits[i], &ctx->src_bits[j],
- ctx->dst.width * bypp);
+ vis_dst.width * bypp);
}
}
break;
@@ -238,21 +268,20 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
DPRINTF("pixman_fill(%p, %ld, %d, %d, %d, %d, %d, %x)\n",
ctx->dst_bits, ctx->dst_stride / sizeof(uint32_t), ctx->bpp,
- ctx->dst.x, ctx->dst.y, ctx->dst.width, ctx->dst.height,
- filler);
+ vis_dst.x, vis_dst.y, vis_dst.width, vis_dst.height, filler);
#ifdef CONFIG_PIXMAN
if (!(use_pixman & BIT(0)) ||
!pixman_fill((uint32_t *)ctx->dst_bits,
ctx->dst_stride / sizeof(uint32_t), ctx->bpp,
- ctx->dst.x, ctx->dst.y,
- ctx->dst.width, ctx->dst.height, filler))
+ vis_dst.x, vis_dst.y, vis_dst.width, vis_dst.height,
+ filler))
#endif
{
/* fallback when pixman failed or we don't want to call it */
unsigned int x, y, i, bypp = ctx->bpp / 8;
- for (y = 0; y < ctx->dst.height; y++) {
- i = ctx->dst.x * bypp + (ctx->dst.y + y) * ctx->dst_stride;
- for (x = 0; x < ctx->dst.width; x++, i += bypp) {
+ for (y = 0; y < vis_dst.height; y++) {
+ i = vis_dst.x * bypp + (vis_dst.y + y) * ctx->dst_stride;
+ for (x = 0; x < vis_dst.width; x++, i += bypp) {
stn_he_p(&ctx->dst_bits[i], bypp, filler);
}
}
--
2.41.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v12 8/9] ati-vga: Implement HOST_DATA register writes
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
` (6 preceding siblings ...)
2026-03-09 1:47 ` [PATCH v12 7/9] ati-vga: Implement scissor rectangle clipping for 2D operations BALATON Zoltan
@ 2026-03-09 1:47 ` BALATON Zoltan
2026-03-09 1:47 ` [PATCH v12 9/9] ati-vga: Implement HOST_DATA flush to VRAM BALATON Zoltan
` (2 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: BALATON Zoltan @ 2026-03-09 1:47 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski,
Philippe Mathieu-Daudé
From: Chad Jablonski <chad@jablonski.xyz>
Writing to any of the HOST_DATA0-7 registers pushes the written data
into a 128-bit accumulator. When the accumulator is full a flush is
triggered to copy it to the framebuffer. A final write to HOST_DATA_LAST
will also initiate a flush. The flush itself is left for the next patch.
Unaligned HOST_DATA* writes result in, from what I can tell, undefined
behavior on real hardware. A well-behaved driver shouldn't be doing this
anyway. For that reason they are not handled here at all.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati.c | 26 ++++++++++++++++++++++++++
hw/display/ati_dbg.c | 9 +++++++++
hw/display/ati_int.h | 9 +++++++++
hw/display/ati_regs.h | 9 +++++++++
4 files changed, 53 insertions(+)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index 9a7c5703b0..16dbb743e1 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -1023,6 +1023,27 @@ static void ati_mm_write(void *opaque, hwaddr addr,
case SRC_SC_BOTTOM:
s->regs.src_sc_bottom = data & 0x3fff;
break;
+ case HOST_DATA0:
+ case HOST_DATA1:
+ case HOST_DATA2:
+ case HOST_DATA3:
+ case HOST_DATA4:
+ case HOST_DATA5:
+ case HOST_DATA6:
+ case HOST_DATA7:
+ case HOST_DATA_LAST:
+ if (!s->host_data.active) {
+ break;
+ }
+ s->host_data.acc[s->host_data.next++] = data;
+ if (addr == HOST_DATA_LAST) {
+ qemu_log_mask(LOG_UNIMP, "HOST_DATA finish not yet implemented\n");
+ s->host_data.next = 0;
+ } else if (s->host_data.next >= 4) {
+ qemu_log_mask(LOG_UNIMP, "HOST_DATA flush not yet implemented\n");
+ s->host_data.next = 0;
+ }
+ break;
default:
break;
}
@@ -1128,6 +1149,11 @@ static void ati_vga_reset(DeviceState *dev)
/* reset vga */
vga_common_reset(&s->vga);
s->mode = VGA_MODE;
+
+ s->host_data.active = false;
+ s->host_data.next = 0;
+ s->host_data.row = 0;
+ s->host_data.col = 0;
}
static void ati_vga_exit(PCIDevice *dev)
diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c
index 3ffa7f35df..5c799d540a 100644
--- a/hw/display/ati_dbg.c
+++ b/hw/display/ati_dbg.c
@@ -252,6 +252,15 @@ static struct ati_regdesc ati_reg_names[] = {
{"MC_SRC1_CNTL", 0x19D8},
{"TEX_CNTL", 0x1800},
{"RAGE128_MPP_TB_CONFIG", 0x01c0},
+ {"HOST_DATA0", 0x17c0},
+ {"HOST_DATA1", 0x17c4},
+ {"HOST_DATA2", 0x17c8},
+ {"HOST_DATA3", 0x17cc},
+ {"HOST_DATA4", 0x17d0},
+ {"HOST_DATA5", 0x17d4},
+ {"HOST_DATA6", 0x17d8},
+ {"HOST_DATA7", 0x17dc},
+ {"HOST_DATA_LAST", 0x17e0},
{NULL, -1}
};
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index 874c63eb54..063efc7bba 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -96,6 +96,14 @@ typedef struct ATIVGARegs {
uint32_t default_tile;
} ATIVGARegs;
+typedef struct ATIHostDataState {
+ bool active;
+ uint32_t row;
+ uint32_t col;
+ uint32_t next;
+ uint32_t acc[4];
+} ATIHostDataState;
+
struct ATIVGAState {
PCIDevice dev;
VGACommonState vga;
@@ -114,6 +122,7 @@ struct ATIVGAState {
MemoryRegion io;
MemoryRegion mm;
ATIVGARegs regs;
+ ATIHostDataState host_data;
};
const char *ati_reg_name(int num);
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index 3999edb9b7..48f15e9b1d 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -252,6 +252,15 @@
#define DP_T12_CNTL 0x178c
#define DST_BRES_T1_LNTH 0x1790
#define DST_BRES_T2_LNTH 0x1794
+#define HOST_DATA0 0x17c0
+#define HOST_DATA1 0x17c4
+#define HOST_DATA2 0x17c8
+#define HOST_DATA3 0x17cc
+#define HOST_DATA4 0x17d0
+#define HOST_DATA5 0x17d4
+#define HOST_DATA6 0x17d8
+#define HOST_DATA7 0x17dc
+#define HOST_DATA_LAST 0x17e0
#define SCALE_SRC_HEIGHT_WIDTH 0x1994
#define SCALE_OFFSET_0 0x1998
#define SCALE_PITCH 0x199c
--
2.41.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v12 9/9] ati-vga: Implement HOST_DATA flush to VRAM
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
` (7 preceding siblings ...)
2026-03-09 1:47 ` [PATCH v12 8/9] ati-vga: Implement HOST_DATA register writes BALATON Zoltan
@ 2026-03-09 1:47 ` BALATON Zoltan
2026-03-09 2:00 ` [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
2026-03-09 13:03 ` Philippe Mathieu-Daudé
10 siblings, 0 replies; 14+ messages in thread
From: BALATON Zoltan @ 2026-03-09 1:47 UTC (permalink / raw)
To: qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski,
Philippe Mathieu-Daudé
From: Chad Jablonski <chad@jablonski.xyz>
Implement flushing the 128-bit HOST_DATA accumulator to VRAM to enable
text rendering in X. Supports all datatypes (monochrome frgd/bkgd,
monochrome frgd, and color), however monochrome frgd support is
partial and does not properly handle transparency/leave-alone.
The flush is broken up into two steps. First, if necessary, expansion of the
monochrome bits to the destination color depth. Then the expanded pixels
are sent to the ati_2d_do_blt one scanline at a time. ati_2d_do_blt then
clips and performs the blit.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
[balaton: Fix build without pixman]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati.c | 6 +-
hw/display/ati_2d.c | 132 +++++++++++++++++++++++++++++++++++++++++-
hw/display/ati_int.h | 3 +
hw/display/ati_regs.h | 4 ++
4 files changed, 139 insertions(+), 6 deletions(-)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index 16dbb743e1..bb9e0bba28 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -1037,11 +1037,9 @@ static void ati_mm_write(void *opaque, hwaddr addr,
}
s->host_data.acc[s->host_data.next++] = data;
if (addr == HOST_DATA_LAST) {
- qemu_log_mask(LOG_UNIMP, "HOST_DATA finish not yet implemented\n");
- s->host_data.next = 0;
+ ati_host_data_finish(s);
} else if (s->host_data.next >= 4) {
- qemu_log_mask(LOG_UNIMP, "HOST_DATA flush not yet implemented\n");
- s->host_data.next = 0;
+ ati_host_data_flush(s);
}
break;
default:
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index d62ad849f3..26ae2f1825 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -47,6 +47,7 @@ static int ati_bpp_from_datatype(const ATIVGAState *s)
typedef struct {
int bpp;
uint32_t rop3;
+ bool host_data_active;
bool left_to_right;
bool top_to_bottom;
uint32_t frgd_clr;
@@ -85,6 +86,7 @@ static void setup_2d_blt_ctx(const ATIVGAState *s, ATI2DCtx *ctx)
{
ctx->bpp = ati_bpp_from_datatype(s);
ctx->rop3 = s->regs.dp_mix & GMC_ROP3_MASK;
+ ctx->host_data_active = s->host_data.active;
ctx->left_to_right = s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT;
ctx->top_to_bottom = s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM;
ctx->frgd_clr = s->regs.dp_brush_frgd_clr;
@@ -178,9 +180,10 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
qemu_log_mask(LOG_GUEST_ERROR, "Zero source pitch\n");
return false;
}
- if (vis_src.x > 0x3fff || vis_src.y > 0x3fff ||
+ if (!ctx->host_data_active &&
+ (vis_src.x > 0x3fff || vis_src.y > 0x3fff ||
ctx->src_bits >= ctx->vram_end || ctx->src_bits + vis_src.x +
- (vis_src.y + vis_dst.height) * ctx->src_stride >= ctx->vram_end) {
+ (vis_src.y + vis_dst.height) * ctx->src_stride >= ctx->vram_end)) {
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
return false;
}
@@ -300,8 +303,133 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
void ati_2d_blt(ATIVGAState *s)
{
ATI2DCtx ctx;
+ uint32_t src_source = s->regs.dp_mix & DP_SRC_SOURCE;
+
+ /* Finish any active HOST_DATA blits before starting a new blit */
+ ati_host_data_finish(s);
+
+ if (src_source == DP_SRC_HOST || src_source == DP_SRC_HOST_BYTEALIGN) {
+ /* Begin a HOST_DATA blit */
+ s->host_data.active = true;
+ s->host_data.next = 0;
+ s->host_data.col = 0;
+ s->host_data.row = 0;
+ return;
+ }
setup_2d_blt_ctx(s, &ctx);
if (ati_2d_do_blt(&ctx, s->use_pixman)) {
ati_set_dirty(&s->vga, &ctx);
}
}
+
+bool ati_host_data_flush(ATIVGAState *s)
+{
+ ATI2DCtx ctx, chunk;
+ uint32_t fg = s->regs.dp_src_frgd_clr;
+ uint32_t bg = s->regs.dp_src_bkgd_clr;
+ unsigned bypp, pix_count, row, col, idx;
+ uint8_t pix_buf[ATI_HOST_DATA_ACC_BITS * sizeof(uint32_t)];
+ uint32_t byte_pix_order = s->regs.dp_datatype & DP_BYTE_PIX_ORDER;
+ uint32_t src_source = s->regs.dp_mix & DP_SRC_SOURCE;
+ uint32_t src_datatype = s->regs.dp_datatype & DP_SRC_DATATYPE;
+
+ if (!s->host_data.active) {
+ return false;
+ }
+ if (src_source != DP_SRC_HOST) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "host_data_blt: unsupported src_source %x\n", src_source);
+ return false;
+ }
+ if (src_datatype != SRC_MONO_FRGD_BKGD && src_datatype != SRC_MONO_FRGD &&
+ src_datatype != SRC_COLOR) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "host_data_blt: undefined src_datatype %x\n",
+ src_datatype);
+ return false;
+ }
+
+ setup_2d_blt_ctx(s, &ctx);
+
+ if (!ctx.left_to_right || !ctx.top_to_bottom) {
+ qemu_log_mask(LOG_UNIMP,
+ "host_data_blt: unsupported blit direction %c%c\n",
+ ctx.left_to_right ? '>' : '<',
+ ctx.top_to_bottom ? 'v' : '^');
+ return false;
+ }
+
+ bypp = ctx.bpp / 8;
+
+ if (src_datatype == SRC_COLOR) {
+ pix_count = ATI_HOST_DATA_ACC_BITS / ctx.bpp;
+ memcpy(pix_buf, &s->host_data.acc[0], sizeof(s->host_data.acc));
+ } else {
+ pix_count = ATI_HOST_DATA_ACC_BITS;
+ /* Expand monochrome bits to color pixels */
+ idx = 0;
+ for (int word = 0; word < 4; word++) {
+ for (int byte = 0; byte < 4; byte++) {
+ uint8_t byte_val = s->host_data.acc[word] >> (byte * 8);
+ for (int i = 0; i < 8; i++) {
+ bool is_fg = byte_val & BIT(byte_pix_order ? i : 7 - i);
+ uint32_t color = is_fg ? fg : bg;
+ stn_he_p(&pix_buf[idx], bypp, color);
+ idx += bypp;
+ }
+ }
+ }
+ }
+
+ /* Copy and then modify blit ctx for use in a chunked blit */
+ chunk = ctx;
+ chunk.src_bits = pix_buf;
+ chunk.src.y = 0;
+ chunk.src_stride = ATI_HOST_DATA_ACC_BITS * bypp;
+
+ /* Blit one scanline chunk at a time */
+ row = s->host_data.row;
+ col = s->host_data.col;
+ idx = 0;
+ DPRINTF("blt %dpx @ row: %d, col: %d\n", pix_count, row, col);
+ while (idx < pix_count && row < ctx.dst.height) {
+ unsigned pix_in_scanline = MIN(pix_count - idx,
+ ctx.dst.width - col);
+ chunk.src.x = idx;
+ /* Build a rect for this scanline chunk */
+ chunk.dst.x = ctx.dst.x + col;
+ chunk.dst.y = ctx.dst.y + row;
+ chunk.dst.width = pix_in_scanline;
+ chunk.dst.height = 1;
+ DPRINTF("blt %dpx span @ row: %d, col: %d to dst (%d,%d)\n",
+ pix_in_scanline, row, col, chunk.dst.x, chunk.dst.y);
+ if (ati_2d_do_blt(&chunk, s->use_pixman)) {
+ ati_set_dirty(&s->vga, &chunk);
+ }
+ idx += pix_in_scanline;
+ col += pix_in_scanline;
+ if (col >= ctx.dst.width) {
+ col = 0;
+ row += 1;
+ }
+ }
+
+ /* Track state of the overall blit for use by the next flush */
+ s->host_data.next = 0;
+ s->host_data.row = row;
+ s->host_data.col = col;
+ if (s->host_data.row >= ctx.dst.height) {
+ s->host_data.active = false;
+ }
+
+ return s->host_data.active;
+}
+
+void ati_host_data_finish(ATIVGAState *s)
+{
+ if (ati_host_data_flush(s)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "HOST_DATA blit ended before all data was written\n");
+ }
+ s->host_data.active = false;
+}
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index 063efc7bba..21b74511e0 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -33,6 +33,7 @@
#define ATI_RAGE128_LINEAR_APER_SIZE (64 * MiB)
#define ATI_R100_LINEAR_APER_SIZE (128 * MiB)
+#define ATI_HOST_DATA_ACC_BITS 128
#define TYPE_ATI_VGA "ati-vga"
OBJECT_DECLARE_SIMPLE_TYPE(ATIVGAState, ATI_VGA)
@@ -128,5 +129,7 @@ struct ATIVGAState {
const char *ati_reg_name(int num);
void ati_2d_blt(ATIVGAState *s);
+bool ati_host_data_flush(ATIVGAState *s);
+void ati_host_data_finish(ATIVGAState *s);
#endif /* ATI_INT_H */
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index 48f15e9b1d..b813fa119e 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -397,7 +397,11 @@
#define DST_32BPP 0x00000006
#define DP_DST_DATATYPE 0x0000000f
#define DP_BRUSH_DATATYPE 0x00000f00
+#define SRC_MONO_FRGD_BKGD 0x00000000
+#define SRC_MONO_FRGD 0x00010000
+#define SRC_COLOR 0x00030000
#define DP_SRC_DATATYPE 0x00030000
+#define DP_BYTE_PIX_ORDER 0x40000000
#define BRUSH_SOLIDCOLOR 0x00000d00
--
2.41.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
` (8 preceding siblings ...)
2026-03-09 1:47 ` [PATCH v12 9/9] ati-vga: Implement HOST_DATA flush to VRAM BALATON Zoltan
@ 2026-03-09 2:00 ` Chad Jablonski
2026-03-09 13:03 ` Philippe Mathieu-Daudé
10 siblings, 0 replies; 14+ messages in thread
From: Chad Jablonski @ 2026-03-09 2:00 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski,
Philippe Mathieu-Daudé
Thanks Zoltan! You beat me to it. This fix looks good to me. And thank
you Philippe for picking this up.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
2026-03-09 1:47 [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering BALATON Zoltan
` (9 preceding siblings ...)
2026-03-09 2:00 ` [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
@ 2026-03-09 13:03 ` Philippe Mathieu-Daudé
2026-03-09 13:13 ` BALATON Zoltan
10 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-03-09 13:03 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel
Cc: Gerd Hoffmann, marcandre.lureau, Chad Jablonski
On 9/3/26 02:47, BALATON Zoltan wrote:
> Changes from v11:
> - Drop already merged patches in [PULL 00/49] Misc HW patches for 2026-03-08
> Based-on: 20260308223433.25503-1-philmd@linaro.org
This didn't apply on this tag:
Applying: ati-vga: Remove src and dst stride mutation in ati_2d_blt
Applying: ati-vga: Use local variables for register values in ati_2d_blt
Applying: ati-vga: Introduce ATI2DCtx struct for 2D blit context
Patch failed at 0003 ati-vga: Introduce ATI2DCtx struct for 2D blit context
error: patch failed: hw/display/ati_2d.c:43
error: hw/display/ati_2d.c: patch does not apply
But I figured it out (minor checkpatch change).
Series queued, thanks.
> - Fix build without pixman
>
> Chad Jablonski (9):
> ati-vga: Remove src and dst stride mutation in ati_2d_blt
> ati-vga: Use local variables for register values in ati_2d_blt
> ati-vga: Introduce ATI2DCtx struct for 2D blit context
> ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt
> ati-vga: Split ati_2d_do_blt from ati_2d_blt
> ati-vga: Remove ATIVGAState param from ati_2d_do_blt
> ati-vga: Implement scissor rectangle clipping for 2D operations
> ati-vga: Implement HOST_DATA register writes
> ati-vga: Implement HOST_DATA flush to VRAM
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
2026-03-09 13:03 ` Philippe Mathieu-Daudé
@ 2026-03-09 13:13 ` BALATON Zoltan
2026-03-09 13:57 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 14+ messages in thread
From: BALATON Zoltan @ 2026-03-09 13:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau, Chad Jablonski
[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]
On Mon, 9 Mar 2026, Philippe Mathieu-Daudé wrote:
> On 9/3/26 02:47, BALATON Zoltan wrote:
>> Changes from v11:
>> - Drop already merged patches in [PULL 00/49] Misc HW patches for
>> 2026-03-08
>> Based-on: 20260308223433.25503-1-philmd@linaro.org
>
> This didn't apply on this tag:
>
> Applying: ati-vga: Remove src and dst stride mutation in ati_2d_blt
> Applying: ati-vga: Use local variables for register values in ati_2d_blt
> Applying: ati-vga: Introduce ATI2DCtx struct for 2D blit context
> Patch failed at 0003 ati-vga: Introduce ATI2DCtx struct for 2D blit context
> error: patch failed: hw/display/ati_2d.c:43
> error: hw/display/ati_2d.c: patch does not apply
>
> But I figured it out (minor checkpatch change).
Checkpatch did not complain to me before but there seem to have been a
white space change in you pull request. I noticed that now and sent v13
for that patch but looks like you were faster. Thanks a lot again for your
help.
> Series queued, thanks.
>
>> - Fix build without pixman
>>
>> Chad Jablonski (9):
>> ati-vga: Remove src and dst stride mutation in ati_2d_blt
>> ati-vga: Use local variables for register values in ati_2d_blt
>> ati-vga: Introduce ATI2DCtx struct for 2D blit context
>> ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt
>> ati-vga: Split ati_2d_do_blt from ati_2d_blt
>> ati-vga: Remove ATIVGAState param from ati_2d_do_blt
>> ati-vga: Implement scissor rectangle clipping for 2D operations
>> ati-vga: Implement HOST_DATA register writes
>> ati-vga: Implement HOST_DATA flush to VRAM
There was the other two patch series to fix guest_hwcursor will you queue
that too?
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v12 0/9] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
2026-03-09 13:13 ` BALATON Zoltan
@ 2026-03-09 13:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-03-09 13:57 UTC (permalink / raw)
To: BALATON Zoltan
Cc: qemu-devel, Gerd Hoffmann, marcandre.lureau, Chad Jablonski
On 9/3/26 14:13, BALATON Zoltan wrote:
> On Mon, 9 Mar 2026, Philippe Mathieu-Daudé wrote:
>> On 9/3/26 02:47, BALATON Zoltan wrote:
>>> Changes from v11:
>>> - Drop already merged patches in [PULL 00/49] Misc HW patches for
>>> 2026-03-08
>>> Based-on: 20260308223433.25503-1-philmd@linaro.org
>>
>> This didn't apply on this tag:
>>
>> Applying: ati-vga: Remove src and dst stride mutation in ati_2d_blt
>> Applying: ati-vga: Use local variables for register values in ati_2d_blt
>> Applying: ati-vga: Introduce ATI2DCtx struct for 2D blit context
>> Patch failed at 0003 ati-vga: Introduce ATI2DCtx struct for 2D blit
>> context
>> error: patch failed: hw/display/ati_2d.c:43
>> error: hw/display/ati_2d.c: patch does not apply
>>
>> But I figured it out (minor checkpatch change).
>
> Checkpatch did not complain to me before but there seem to have been a
> white space change in you pull request. I noticed that now and sent v13
> for that patch but looks like you were faster. Thanks a lot again for
> your help.
The errors were:
46/57 Checking commit 7ebf5e5ac2ad (ati-vga: Consolidate dirty region
tracking in ati_2d_blt)
ERROR: "foo * bar" should be "foo *bar"
#38: FILE: hw/display/ati_2d.c:59:
+ s->regs.dst_offset + dst_y *
surface_stride(ds),
48/57 Checking commit 360cf823d841 (ati-vga: Use local variables for
register values in ati_2d_blt)
ERROR: suspect code indent for conditional statements (8, 8)
#196: FILE: hw/display/ati_2d.c:212:
+ if (!use_pixman_fill ||
[...]
{
I marked the first one as spurious, and am going to ignore the
second (some refactor could avoid the #ifdef'ry, but no time for
that change before the soft-freeze).
>
>> Series queued, thanks.
>>
>>> - Fix build without pixman
>>>
>>> Chad Jablonski (9):
>>> ati-vga: Remove src and dst stride mutation in ati_2d_blt
>>> ati-vga: Use local variables for register values in ati_2d_blt
>>> ati-vga: Introduce ATI2DCtx struct for 2D blit context
>>> ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt
>>> ati-vga: Split ati_2d_do_blt from ati_2d_blt
>>> ati-vga: Remove ATIVGAState param from ati_2d_do_blt
>>> ati-vga: Implement scissor rectangle clipping for 2D operations
>>> ati-vga: Implement HOST_DATA register writes
>>> ati-vga: Implement HOST_DATA flush to VRAM
>
> There was the other two patch series to fix guest_hwcursor will you
> queue that too?
Yep!
^ permalink raw reply [flat|nested] 14+ messages in thread