On Thu, 15 Jan 2026, Chad Jablonski wrote:
A large amount of the common setup involved in a blit deals with the
destination. This moves that setup to a helper function which
initializes a struct (ATIBltDst) holding all of the dst state. This setup
will be shared between blits from memory as well as from HOST_DATA.
Otherwise this is a pure refactor of the ati_2d_blt function to make use
of the setup_2d_blt_dst helper and the struct it initializes. There should
be no change in behavior in this patch.
Signed-off-by: Chad Jablonski <[email protected]>
---
hw/display/ati_2d.c | 195 +++++++++++++++++++++++++-------------------
1 file changed, 112 insertions(+), 83 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index a8c4c534b9..75bd38e9b0 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:
@@ -24,7 +25,16 @@
* possible.
*/
-static int ati_bpp_from_datatype(ATIVGAState *s)
+typedef struct {
+ QemuRect rect;
+ int bpp;
+ int stride;
+ bool top_to_bottom;
+ bool left_to_right;
+ uint8_t *bits;
+} ATIBltDst;
I'll need to look at this and further patches in more detail but I wonder
if we need separate types for BltDst and BltSrc or could we use a single
type for both maybe not using some fields for Src. But if that would not
simplify things then these can be separate too.
+
+static int ati_bpp_from_datatype(const ATIVGAState *s)
{
switch (s->regs.dp_datatype & 0xf) {
case 2:
@@ -43,57 +53,76 @@ static int ati_bpp_from_datatype(ATIVGAState *s)
}
}
+static void setup_2d_blt_dst(const ATIVGAState *s, ATIBltDst *dst)
+{
+ unsigned dst_x, dst_y;
+ dst->bpp = ati_bpp_from_datatype(s);
+ dst->stride = s->regs.dst_pitch;
+ dst->left_to_right = s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT;
+ dst->top_to_bottom = s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM;
+ dst->bits = s->vga.vram_ptr + s->regs.dst_offset;
+ dst_x = (dst->left_to_right ?
+ s->regs.dst_x : s->regs.dst_x + 1 - s->regs.dst_width);
+ dst_y = (dst->top_to_bottom ?
+ s->regs.dst_y : s->regs.dst_y + 1 - s->regs.dst_height);
+ qemu_rect_init(&dst->rect, dst_x, dst_y,
+ s->regs.dst_width, s->regs.dst_height);
+ if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
+ dst->bits += s->regs.crtc_offset & 0x07ffffff;
+ dst->stride *= dst->bpp;
+ }
+}
+
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: */
DisplaySurface *ds = qemu_console_surface(s->vga.con);
+ uint8_t *end = s->vga.vram_ptr + s->vga.vram_size;
+ int dst_stride_words;
+ ATIBltDst _dst; /* TEMP: avoid churn in future patches */
+ ATIBltDst *dst = &_dst;
These will be removed but names beginning with _ are reserved and should
not be used so maybe this (and _src in next patch) should be dst_ or
something else? Can the churn be avoided by reorganising changes or moving
converting to pointer in a separate patch? This patch is quite long and
mixes pointer conversion with other refactoring so separating those may
help in review.
Regards,
BALATON Zoltan
