* [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
@ 2026-03-03 2:47 Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 01/17] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes Chad Jablonski
` (17 more replies)
0 siblings, 18 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
This series implements HOST_DATA as a blit source enabling text rendering in
xterm under X.org with 2D acceleration.
The series builds up functionality incrementally:
* Patches 1-6: Bug fixes and register implementations
* Patches 7-14: Refactor of ati_2d_blt to decouple from ATIVGAState
* Patch 15: Scissor clipping implementation
* Patches 16-17: HOST_DATA register writes, color expansion, and
accumulator flush
Tested with xterm on X.org using the r128 driver built with
--disable-dri (MMIO mode).
Hardware tests can be found at:
https://codeberg.org/cjab/ati-tests/src/commit/32ed42625cb3f094d4a3d3ce3ad26cfa5f872ac2/tests/common/host_data.c
Changes from v10:
- Rename ati_{flush,finish}_host_data to ati_host_data_{flush,finish}
- Use sizeof accumulator when flushing SRC_COLOR datatype
- Simplify ati_host_data_finish by flushing and immediately ending the blit
- Combine HOST_DATA* and HOST_DATA_LAST handlers
- Reset host_data.next within ati_host_data_flush
Changes from v9:
- Remove unneeded FIXME comment in ati_2d_do_blt
- ati_set_dirty is no longer called when the blit exits early
- Drop patch 15 moving src bounds validation to ati_2d_blt
- Check src vram bounds using clipped values, only when host_data is inactive
- Return to 128-bit accumulator for simplicity
- Small tweak to color expansion to avoid a multiply
Changes from v8:
- Rename tmp_stride to tmp_stride_words
- Small formatting tweaks
- Add clipping debug logging
- Replace extract8 with BIT masking in expansion hot path
- Add SRC_COLOR support
- Fix SRC_COLOR value (was 0x20000, should have been 0x30000)
- Add partial SRC_MONO_FRGD support
- Src bound validation now only done on ROP3_SRCCOPY blits
- Other improved log messages
- Host data accumulator expanded from 128-bits to 256-bits based on hardware
testing. Real hardware flushes after 128-bits but early HOST_DATA_LAST writes
make use of the entire 256-bits to finish the blit.
- Added ati_finish_host_data to match hardware in the early HOST_DATA_LAST case
- Added active flag for host_data blits ignoring HOST_DATA writes
when no blit is active.
- Removed ati_host_data_reset. This is now handled either when flushing or a new
blit is started.
Changes from v7:
- Avoid potential issue with dst_offset calculation (found by Zoltan)
- Remove unnecessary newlines and other small formatting fixes
- Squash patch 15 into patches 13 and 14
- Note: Zoltan's suggestion to embed ATI2DCtx in ATIVGAState will be
revisited in a follow-up series
Changes from v6:
- Reduce churn by adding ctx_ struct and pointer
- Moved extraction of ati_set_dirty helper earlier in the series
- Moved register logging into setup_2d_blt_ctx
- Remove unnecessary comments
Changes from v5:
- Added previously standalone aperture size patch to series
- Introduced ATI2DCtx to hold all blit state (suggested by Zoltan)
- Made refactor patches more granular
- Simplified DP_GUI_MASTER_CNTL bit shifting
- Sorted clipping regs by offsets
- Removed dst_x/y update after blit (confirmed by hardware testing)
Changes from v4:
- Refactored ati_2d_blt to accept src and dst state
- Implemented host_data blits on top of the refactored ati_2d_blt
(suggested by Zoltan)
- Dropped separate field storage for DP_DATATYPE/DP_MIX (per Zoltan)
Changes from v3:
- New patch 1 fixing DP_DATATYPE/MIX register aliasing, this supersedes old patch 5
- Fix MSB/LSB HOST_DATA bit ordering, it is per-byte rather than per-word
- Fixed a missing break in register write handler
- Squashed and reorganized the dst/scissor helper patches (per Zoltan)
- Simplified register field constants and increment logic (per Zoltan)
- Tested with MorphOS and TVPaint (opaque bitmap fonts now work)
Changes from v2:
- Verified with hardware that clipping default bit latches
- Verified with hardware that pitch/offset default bits latch
(supersedes Zoltan's "ati-vga: Separate default control bit for
source")
- A new approach to HOST_DATA using a 128-bit accumulator instead of a
large buffer
- Fixed a longstanding bug in (DST/SRC)_PITCH reads that zeroed pitch value.
- Removed mask from the DP_GUI_MASTER_CNTL write, all fields are
important
- Created helpers for code shared between ati_2d_blt and
ati_host_data_flush
Changes from v1:
- Split monolithic patch into 7 logical patches as requested
- Integrate HOST_DATA blit into existing ati_2d_blt()
- Add fallback implementation for non-pixman builds
- Remove unnecessary initialization in realize (kept in reset only)
- Fixed DP_GUI_MASTER_CNTL mask to preserve GMC_SRC_SOURCE field
- Implement scissor clipping
Chad Jablonski (17):
ati-vga: Fix framebuffer mapping by using hardware-correct aperture
sizes
ati-vga: Fix DST_PITCH and SRC_PITCH reads
ati-vga: Read aliased values from DP_GUI_MASTER_CNTL
ati-vga: Latch src and dst pitch and offset on master_cntl default
ati-vga: Implement foreground and background color register writes
ati-vga: Add scissor clipping register support
ati-vga: Remove dst_x/y updates after blit
ati-vga: Consolidate dirty region tracking in ati_2d_blt
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
hw/display/ati.c | 137 ++++++++++++-
hw/display/ati_2d.c | 462 +++++++++++++++++++++++++++++-------------
hw/display/ati_dbg.c | 9 +
hw/display/ati_int.h | 26 ++-
hw/display/ati_regs.h | 20 ++
5 files changed, 509 insertions(+), 145 deletions(-)
--
2.52.0
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v11 01/17] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 02/17] ati-vga: Fix DST_PITCH and SRC_PITCH reads Chad Jablonski
` (16 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
Rage 128 cards always request 64MB for their linear (framebuffer)
aperture and R100 cards always request 128MB. This is regardless
of the amount of physical VRAM on the board. The following are results
from real hardware tests:
Card VRAM PCI BAR0 CONFIG_MEMSIZE CONFIG_APER_SIZE AGP_APER_OFFSET
----------------------- ---- -------- -------------- ---------------- ---------------
Rage 128 Pro Ultra TF 32MB 64MB 0x02000000 0x02000000 0x02000000
Rage 128 RF/SG AGP 16MB 64MB 0x01000000 0x02000000 0x02000000
Radeon R100 QD [Radeon 7200] 64MB 128MB 0x04000000 0x04000000 N/A
Radeon RV100 QY [Radeon 7000/VE] 32MB 128MB 0x02000000 0x04000000 N/A
Previously the linear aperture (BAR0) would match the VRAM size.
This discrepancy caused issues with the X.org and XFree86 r128 drivers.
These drivers apply a mask of 0xfc000000 (2^26 = 64MB) to the linear
aperture address. If that address is not on a 64MB boundary the
framebuffer points to an incorrect memory location.
Testing shows that the Radeon R100 also has a BAR0 larger than VRAM
(128MB in this case) and the X.org radeon driver also masks to 64MB.
For Rage 128, CONFIG_APER_SIZE also differs from the previous value and
the behavior stated in the documentation. The Rage 128 register guide
states that it should contain the size of the VRAM + AGP memory. The cards
tested above show that this isn't the case. These tests also included
enabling/disabling AGP with 8MB of memory. It didn't change the
contents of CONFIG_APER_SIZE.
For both Rage 128 and R100 the CONFIG_APER_SIZE is half of the PCI BAR0 size.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati.c | 16 ++++++++++++++--
hw/display/ati_int.h | 5 +++++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index e9c3ad2cd1..8438a77de0 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -361,7 +361,7 @@ 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 / 2;
+ val = memory_region_size(&s->linear_aper) / 2;
break;
case CONFIG_REG_1_BASE:
val = pci_default_read_config(&s->dev,
@@ -952,6 +952,7 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp)
{
ATIVGAState *s = ATI_VGA(dev);
VGACommonState *vga = &s->vga;
+ uint64_t aper_size;
#ifndef CONFIG_PIXMAN
if (s->use_pixman != 0) {
@@ -1011,7 +1012,18 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp)
/* io space is alias to beginning of mmregs */
memory_region_init_alias(&s->io, OBJECT(s), "ati.io", &s->mm, 0, 0x100);
- pci_register_bar(dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &vga->vram);
+ /*
+ * The framebuffer is at the beginning of the linear aperture. For
+ * Rage128 the upper half of the aperture is reserved for an AGP
+ * window (which we do not emulate.)
+ */
+ aper_size = s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF ?
+ ATI_RAGE128_LINEAR_APER_SIZE : ATI_R100_LINEAR_APER_SIZE;
+ memory_region_init(&s->linear_aper, OBJECT(dev), "ati-linear-aperture0",
+ aper_size);
+ memory_region_add_subregion(&s->linear_aper, 0, &vga->vram);
+
+ pci_register_bar(dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->linear_aper);
pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mm);
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index f5a47b82b0..708cc1dd3a 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -10,6 +10,7 @@
#define ATI_INT_H
#include "qemu/timer.h"
+#include "qemu/units.h"
#include "hw/pci/pci_device.h"
#include "hw/i2c/bitbang_i2c.h"
#include "vga_int.h"
@@ -29,6 +30,9 @@
/* Radeon RV100 (VE) */
#define PCI_DEVICE_ID_ATI_RADEON_QY 0x5159
+#define ATI_RAGE128_LINEAR_APER_SIZE (64 * MiB)
+#define ATI_R100_LINEAR_APER_SIZE (128 * MiB)
+
#define TYPE_ATI_VGA "ati-vga"
OBJECT_DECLARE_SIMPLE_TYPE(ATIVGAState, ATI_VGA)
@@ -97,6 +101,7 @@ struct ATIVGAState {
QEMUCursor *cursor;
QEMUTimer vblank_timer;
bitbang_i2c_interface bbi2c;
+ MemoryRegion linear_aper;
MemoryRegion io;
MemoryRegion mm;
ATIVGARegs regs;
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 02/17] ati-vga: Fix DST_PITCH and SRC_PITCH reads
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 01/17] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 03/17] ati-vga: Read aliased values from DP_GUI_MASTER_CNTL Chad Jablonski
` (15 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
Reading DST_PITCH and SRC_PITCH on the Rage 128 is broken. The read
handlers attempt to construct the value from pitch and tile bits in
the register state but mistakenly AND them instead of ORing them. This
means the pitch is always zero on read.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index 8438a77de0..777a6b0a2e 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -438,7 +438,7 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
case DST_PITCH:
val = s->regs.dst_pitch;
if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
- val &= s->regs.dst_tile << 16;
+ val |= s->regs.dst_tile << 16;
}
break;
case DST_WIDTH:
@@ -468,7 +468,7 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
case SRC_PITCH:
val = s->regs.src_pitch;
if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
- val &= s->regs.src_tile << 16;
+ val |= s->regs.src_tile << 16;
}
break;
case DP_BRUSH_BKGD_CLR:
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 03/17] ati-vga: Read aliased values from DP_GUI_MASTER_CNTL
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 01/17] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 02/17] ati-vga: Fix DST_PITCH and SRC_PITCH reads Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 04/17] ati-vga: Latch src and dst pitch and offset on master_cntl default Chad Jablonski
` (14 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
DP_GUI_MASTER_CNTL aliases several fields from DP_DATATYPE and DP_MIX.
These were being written correctly but not returned on a read of
DP_GUI_MASTER_CNTL.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati.c | 8 +++++++-
hw/display/ati_regs.h | 5 +++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index 777a6b0a2e..028efd13e1 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -460,7 +460,13 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
val = s->regs.dst_y;
break;
case DP_GUI_MASTER_CNTL:
- val = s->regs.dp_gui_master_cntl;
+ /* DP_GUI_MASTER_CNTL aliases fields from DP_MIX and DP_DATATYPE */
+ val = s->regs.dp_gui_master_cntl |
+ ((s->regs.dp_datatype & DP_BRUSH_DATATYPE) >> 4) |
+ ((s->regs.dp_datatype & DP_DST_DATATYPE) << 8) |
+ ((s->regs.dp_datatype & DP_SRC_DATATYPE) >> 4) |
+ (s->regs.dp_mix & DP_ROP3) |
+ ((s->regs.dp_mix & DP_SRC_SOURCE) << 16);
break;
case SRC_OFFSET:
val = s->regs.src_offset;
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index d7127748ff..0a0825db04 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -386,6 +386,9 @@
#define DST_16BPP 0x00000004
#define DST_24BPP 0x00000005
#define DST_32BPP 0x00000006
+#define DP_DST_DATATYPE 0x0000000f
+#define DP_BRUSH_DATATYPE 0x00000f00
+#define DP_SRC_DATATYPE 0x00030000
#define BRUSH_SOLIDCOLOR 0x00000d00
@@ -437,6 +440,8 @@
#define DP_SRC_RECT 0x00000200
#define DP_SRC_HOST 0x00000300
#define DP_SRC_HOST_BYTEALIGN 0x00000400
+#define DP_SRC_SOURCE 0x00000700
+#define DP_ROP3 0x00ff0000
/* LVDS_GEN_CNTL constants */
#define LVDS_BL_MOD_LEVEL_MASK 0x0000ff00
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 04/17] ati-vga: Latch src and dst pitch and offset on master_cntl default
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (2 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 03/17] ati-vga: Read aliased values from DP_GUI_MASTER_CNTL Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 05/17] ati-vga: Implement foreground and background color register writes Chad Jablonski
` (13 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
Hardware testing on the Rage 128 confirms that (SRC/DST)_OFFSET,
and (SRC/DST)_PITCH are latched when (SRC/DST)_PITCH_OFFSET_CNTL bits
in DP_GUI_MASTER_CNTL are set to "default".
The earlier approach looked at the state of the (SRC/DST)_PITCH_OFFSET_CNTL
bits when offset and pitch registers were used. This meant that when
(SRC/DST)_PITCH_OFFSET_CNTL was reset to "leave alone" the old values
stored in the registers would return. This is not how the real hardware
works.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati.c | 9 +++++++++
hw/display/ati_2d.c | 13 ++++---------
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index 028efd13e1..ce23e5e48b 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -868,6 +868,15 @@ static void ati_mm_write(void *opaque, hwaddr addr,
s->regs.dp_datatype = (data & 0x0f00) >> 8 | (data & 0x30f0) << 4 |
(data & 0x4000) << 16;
s->regs.dp_mix = (data & GMC_ROP3_MASK) | (data & 0x7000000) >> 16;
+
+ if (!(data & GMC_SRC_PITCH_OFFSET_CNTL)) {
+ s->regs.src_offset = s->regs.default_offset;
+ s->regs.src_pitch = s->regs.default_pitch;
+ }
+ if (!(data & GMC_DST_PITCH_OFFSET_CNTL)) {
+ s->regs.dst_offset = s->regs.default_offset;
+ s->regs.dst_pitch = s->regs.default_pitch;
+ }
break;
case DST_WIDTH_X:
s->regs.dst_x = data & 0x3fff;
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 309bb5ccb6..a8c4c534b9 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -43,8 +43,6 @@ static int ati_bpp_from_datatype(ATIVGAState *s)
}
}
-#define DEFAULT_CNTL (s->regs.dp_gui_master_cntl & GMC_DST_PITCH_OFFSET_CNTL)
-
void ati_2d_blt(ATIVGAState *s)
{
/* FIXME it is probably more complex than this and may need to be */
@@ -63,13 +61,12 @@ void ati_2d_blt(ATIVGAState *s)
qemu_log_mask(LOG_GUEST_ERROR, "Invalid bpp\n");
return;
}
- int dst_stride = DEFAULT_CNTL ? s->regs.dst_pitch : s->regs.default_pitch;
+ int dst_stride = s->regs.dst_pitch;
if (!dst_stride) {
qemu_log_mask(LOG_GUEST_ERROR, "Zero dest pitch\n");
return;
}
- uint8_t *dst_bits = s->vga.vram_ptr + (DEFAULT_CNTL ?
- s->regs.dst_offset : s->regs.default_offset);
+ uint8_t *dst_bits = s->vga.vram_ptr + s->regs.dst_offset;
if (s->dev_id == PCI_DEVICE_ID_ATI_RAGE128_PF) {
dst_bits += s->regs.crtc_offset & 0x07ffffff;
@@ -97,14 +94,12 @@ void ati_2d_blt(ATIVGAState *s)
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);
- int src_stride = DEFAULT_CNTL ?
- s->regs.src_pitch : s->regs.default_pitch;
+ int src_stride = s->regs.src_pitch;
if (!src_stride) {
qemu_log_mask(LOG_GUEST_ERROR, "Zero source pitch\n");
return;
}
- uint8_t *src_bits = s->vga.vram_ptr + (DEFAULT_CNTL ?
- s->regs.src_offset : s->regs.default_offset);
+ uint8_t *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;
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 05/17] ati-vga: Implement foreground and background color register writes
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (3 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 04/17] ati-vga: Latch src and dst pitch and offset on master_cntl default Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 06/17] ati-vga: Add scissor clipping register support Chad Jablonski
` (12 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
These are straightforward 32-bit register write handlers. They're
necessary for a future patch which will use them for color expansion
from monochrome host data transfers.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index ce23e5e48b..26fc74b19b 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -924,6 +924,12 @@ static void ati_mm_write(void *opaque, hwaddr addr,
case DP_CNTL:
s->regs.dp_cntl = data;
break;
+ case DP_SRC_FRGD_CLR:
+ s->regs.dp_src_frgd_clr = data;
+ break;
+ case DP_SRC_BKGD_CLR:
+ s->regs.dp_src_bkgd_clr = data;
+ break;
case DP_DATATYPE:
s->regs.dp_datatype = data & 0xe0070f0f;
break;
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 06/17] ati-vga: Add scissor clipping register support
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (4 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 05/17] ati-vga: Implement foreground and background color register writes Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 07/17] ati-vga: Remove dst_x/y updates after blit Chad Jablonski
` (11 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
Implement read and write operations on SC_TOP_LEFT, SC_BOTTOM_RIGHT,
and SRC_SC_BOTTOM_RIGHT registers. These registers are also updated
when the src and/or dst clipping fields on DP_GUI_MASTER_CNTL are set
to default clipping.
Scissor clipping is used when rendering text in X.org. The r128 driver
sends host data much wider than is necessary to draw a glyph and cuts it
down to size using clipping before rendering. The actual clipping
implementation follows in a future patch.
This also includes a very minor refactor of the combined
default_sc_bottom_right field in the registers struct to
default_sc_bottom and default_sc_right. This was done to
stay consistent with the other scissor registers and prevent repeated
masking and extraction.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
v5: Removed unused default_sc_bottom_right field
---
hw/display/ati.c | 70 +++++++++++++++++++++++++++++++++++++++++--
hw/display/ati_int.h | 9 +++++-
hw/display/ati_regs.h | 2 ++
3 files changed, 78 insertions(+), 3 deletions(-)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index 26fc74b19b..6cf243bcf9 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -514,7 +514,32 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, unsigned int size)
val |= s->regs.default_tile << 16;
break;
case DEFAULT_SC_BOTTOM_RIGHT:
- val = s->regs.default_sc_bottom_right;
+ val = (s->regs.default_sc_bottom << 16) |
+ s->regs.default_sc_right;
+ break;
+ case SC_TOP:
+ val = s->regs.sc_top;
+ break;
+ case SC_LEFT:
+ val = s->regs.sc_left;
+ break;
+ case SC_BOTTOM:
+ val = s->regs.sc_bottom;
+ break;
+ case SC_RIGHT:
+ val = s->regs.sc_right;
+ break;
+ case SRC_SC_BOTTOM:
+ val = s->regs.src_sc_bottom;
+ break;
+ case SRC_SC_RIGHT:
+ val = s->regs.src_sc_right;
+ break;
+ case SC_TOP_LEFT:
+ case SC_BOTTOM_RIGHT:
+ case SRC_SC_BOTTOM_RIGHT:
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "Read from write-only register 0x%x\n", (unsigned)addr);
break;
default:
break;
@@ -877,6 +902,16 @@ static void ati_mm_write(void *opaque, hwaddr addr,
s->regs.dst_offset = s->regs.default_offset;
s->regs.dst_pitch = s->regs.default_pitch;
}
+ if (!(data & GMC_SRC_CLIPPING)) {
+ s->regs.src_sc_right = s->regs.default_sc_right;
+ s->regs.src_sc_bottom = s->regs.default_sc_bottom;
+ }
+ if (!(data & GMC_DST_CLIPPING)) {
+ s->regs.sc_top = 0;
+ s->regs.sc_left = 0;
+ s->regs.sc_right = s->regs.default_sc_right;
+ s->regs.sc_bottom = s->regs.default_sc_bottom;
+ }
break;
case DST_WIDTH_X:
s->regs.dst_x = data & 0x3fff;
@@ -956,7 +991,38 @@ static void ati_mm_write(void *opaque, hwaddr addr,
}
break;
case DEFAULT_SC_BOTTOM_RIGHT:
- s->regs.default_sc_bottom_right = data & 0x3fff3fff;
+ s->regs.default_sc_right = data & 0x3fff;
+ s->regs.default_sc_bottom = (data >> 16) & 0x3fff;
+ break;
+ case SC_TOP_LEFT:
+ s->regs.sc_left = data & 0x3fff;
+ s->regs.sc_top = (data >> 16) & 0x3fff;
+ break;
+ case SC_LEFT:
+ s->regs.sc_left = data & 0x3fff;
+ break;
+ case SC_TOP:
+ s->regs.sc_top = data & 0x3fff;
+ break;
+ case SC_BOTTOM_RIGHT:
+ s->regs.sc_right = data & 0x3fff;
+ s->regs.sc_bottom = (data >> 16) & 0x3fff;
+ break;
+ case SC_RIGHT:
+ s->regs.sc_right = data & 0x3fff;
+ break;
+ case SC_BOTTOM:
+ s->regs.sc_bottom = data & 0x3fff;
+ break;
+ case SRC_SC_BOTTOM_RIGHT:
+ s->regs.src_sc_right = data & 0x3fff;
+ s->regs.src_sc_bottom = (data >> 16) & 0x3fff;
+ break;
+ case SRC_SC_RIGHT:
+ s->regs.src_sc_right = data & 0x3fff;
+ break;
+ case SRC_SC_BOTTOM:
+ s->regs.src_sc_bottom = data & 0x3fff;
break;
default:
break;
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index 708cc1dd3a..98f57ca5fa 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -78,14 +78,21 @@ typedef struct ATIVGARegs {
uint32_t dp_brush_frgd_clr;
uint32_t dp_src_frgd_clr;
uint32_t dp_src_bkgd_clr;
+ uint16_t sc_top;
+ uint16_t sc_left;
+ uint16_t sc_bottom;
+ uint16_t sc_right;
+ uint16_t src_sc_bottom;
+ uint16_t src_sc_right;
uint32_t dp_cntl;
uint32_t dp_datatype;
uint32_t dp_mix;
uint32_t dp_write_mask;
uint32_t default_offset;
uint32_t default_pitch;
+ uint16_t default_sc_bottom;
+ uint16_t default_sc_right;
uint32_t default_tile;
- uint32_t default_sc_bottom_right;
} ATIVGARegs;
struct ATIVGAState {
diff --git a/hw/display/ati_regs.h b/hw/display/ati_regs.h
index 0a0825db04..3999edb9b7 100644
--- a/hw/display/ati_regs.h
+++ b/hw/display/ati_regs.h
@@ -397,6 +397,8 @@
#define GMC_DST_PITCH_OFFSET_CNTL 0x00000002
#define GMC_SRC_CLIP_DEFAULT 0x00000000
#define GMC_DST_CLIP_DEFAULT 0x00000000
+#define GMC_SRC_CLIPPING 0x00000004
+#define GMC_DST_CLIPPING 0x00000008
#define GMC_BRUSH_SOLIDCOLOR 0x000000d0
#define GMC_SRC_DSTCOLOR 0x00003000
#define GMC_BYTE_ORDER_MSB_TO_LSB 0x00000000
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 07/17] ati-vga: Remove dst_x/y updates after blit
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (5 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 06/17] ati-vga: Add scissor clipping register support Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 08/17] ati-vga: Consolidate dirty region tracking in ati_2d_blt Chad Jablonski
` (10 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
The Mobility M6 register reference (DST_HEIGHT_WIDTH) states that dst_y is
updated after a blit but this appears to not be the case.
Hardware testing revealed that both the R128 and R100 do not update
dst_x or dst_y after a blit, regardless of the source. This removes
the update.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati_2d.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index a8c4c534b9..cfc7bf9789 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -174,10 +174,6 @@ void ati_2d_blt(ATIVGAState *s)
dst_y * surface_stride(ds),
s->regs.dst_height * surface_stride(ds));
}
- s->regs.dst_x = (s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT ?
- dst_x + s->regs.dst_width : dst_x);
- s->regs.dst_y = (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ?
- dst_y + s->regs.dst_height : dst_y);
break;
}
case ROP3_PATCOPY:
@@ -228,8 +224,6 @@ void ati_2d_blt(ATIVGAState *s)
dst_y * surface_stride(ds),
s->regs.dst_height * surface_stride(ds));
}
- s->regs.dst_y = (s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM ?
- dst_y + s->regs.dst_height : dst_y);
break;
}
default:
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 08/17] ati-vga: Consolidate dirty region tracking in ati_2d_blt
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (6 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 07/17] ati-vga: Remove dst_x/y updates after blit Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 09/17] ati-vga: Remove src and dst stride mutation " Chad Jablonski
` (9 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
Both supported ROPs follow the same memory set dirty logic.
This consolidates that logic to remove the duplication.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati_2d.c | 42 +++++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index cfc7bf9789..980cdd6ac0 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -43,15 +43,28 @@ static int ati_bpp_from_datatype(ATIVGAState *s)
}
}
+static void ati_set_dirty(ATIVGAState *s,
+ const uint8_t *dst_bits, unsigned dst_y)
+{
+ 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 +
+ 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));
+ }
+}
+
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);
- DPRINTF("%p %u ds: %p %d %d rop: %x\n", s->vga.vram_ptr,
- s->vga.vbe_start_addr, surface_data(ds), surface_stride(ds),
- surface_bits_per_pixel(ds),
- (s->regs.dp_mix & GMC_ROP3_MASK) >> 16);
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 ?
@@ -166,14 +179,6 @@ void ati_2d_blt(ATIVGAState *s)
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 +
- s->vga.vbe_regs[VBE_DISPI_INDEX_YRES] * s->vga.vbe_line_offset) {
- memory_region_set_dirty(&s->vga.vram, s->vga.vbe_start_addr +
- s->regs.dst_offset +
- dst_y * surface_stride(ds),
- s->regs.dst_height * surface_stride(ds));
- }
break;
}
case ROP3_PATCOPY:
@@ -216,18 +221,13 @@ void ati_2d_blt(ATIVGAState *s)
}
}
}
- 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) {
- memory_region_set_dirty(&s->vga.vram, s->vga.vbe_start_addr +
- s->regs.dst_offset +
- dst_y * surface_stride(ds),
- s->regs.dst_height * surface_stride(ds));
- }
break;
}
default:
qemu_log_mask(LOG_UNIMP, "Unimplemented ati_2d blt op %x\n",
(s->regs.dp_mix & GMC_ROP3_MASK) >> 16);
+ return;
}
+
+ ati_set_dirty(s, dst_bits, dst_y);
}
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 09/17] ati-vga: Remove src and dst stride mutation in ati_2d_blt
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (7 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 08/17] ati-vga: Consolidate dirty region tracking in ati_2d_blt Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 10/17] ati-vga: Use local variables for register values " Chad Jablonski
` (8 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
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>
---
hw/display/ati_2d.c | 41 ++++++++++++++++++-----------------------
1 file changed, 18 insertions(+), 23 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 980cdd6ac0..e366774835 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -85,6 +85,7 @@ void ati_2d_blt(ATIVGAState *s)
dst_bits += s->regs.crtc_offset & 0x07ffffff;
dst_stride *= bpp;
}
+ int dst_stride_words = dst_stride / sizeof(uint32_t);
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
@@ -118,6 +119,7 @@ void ati_2d_blt(ATIVGAState *s)
src_bits += s->regs.crtc_offset & 0x07ffffff;
src_stride *= bpp;
}
+ int src_stride_words = src_stride / sizeof(uint32_t);
if (src_x > 0x3fff || src_y > 0x3fff || src_bits >= end
|| src_bits + src_x
+ (src_y + s->regs.dst_height) * src_stride >= end) {
@@ -125,34 +127,32 @@ 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,
+ src_bits, 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);
#ifdef CONFIG_PIXMAN
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,19 @@ 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,
+ dst_bits, dst_stride_words, 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_words, 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.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 10/17] ati-vga: Use local variables for register values in ati_2d_blt
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (8 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 09/17] ati-vga: Remove src and dst stride mutation " Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 11/17] ati-vga: Introduce ATI2DCtx struct for 2D blit context Chad Jablonski
` (7 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
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>
---
hw/display/ati_2d.c | 97 ++++++++++++++++++++++++---------------------
1 file changed, 52 insertions(+), 45 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index e366774835..689b07b4d0 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -65,10 +65,20 @@ 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);
+ bool use_pixman_fill = s->use_pixman & BIT(0);
+ bool use_pixman_blt = s->use_pixman & BIT(1);
+ 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 +89,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;
@@ -88,26 +98,25 @@ void ati_2d_blt(ATIVGAState *s)
int dst_stride_words = dst_stride / sizeof(uint32_t);
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");
@@ -122,7 +131,7 @@ void ati_2d_blt(ATIVGAState *s)
int src_stride_words = src_stride / sizeof(uint32_t);
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;
}
@@ -130,30 +139,28 @@ void ati_2d_blt(ATIVGAState *s)
DPRINTF("pixman_blt(%p, %p, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d)\n",
src_bits, 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);
#ifdef CONFIG_PIXMAN
- 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 (use_pixman_blt && 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);
- } else if (s->use_pixman & BIT(1)) {
+ dst_width, dst_height);
+ } else if (use_pixman_blt) {
/* 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 +170,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,34 +191,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, %d, %d, %d, %d, %d, %d, %x)\n",
dst_bits, dst_stride_words, 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)) ||
+ if (!use_pixman_fill ||
!pixman_fill((uint32_t *)dst_bits, dst_stride_words, bpp, dst_x,
- dst_y, s->regs.dst_width, s->regs.dst_height, filler))
+ 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);
}
}
@@ -220,7 +227,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.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 11/17] ati-vga: Introduce ATI2DCtx struct for 2D blit context
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (9 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 10/17] ati-vga: Use local variables for register values " Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 12/17] ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt Chad Jablonski
` (6 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
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>
---
hw/display/ati_2d.c | 221 +++++++++++++++++++++++++-------------------
1 file changed, 126 insertions(+), 95 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 689b07b4d0..d43268ba5f 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));
}
}
@@ -67,100 +86,106 @@ void ati_2d_blt(ATIVGAState *s)
/* rewritten but for now as a start just to get some output: */
bool use_pixman_fill = s->use_pixman & BIT(0);
bool use_pixman_blt = s->use_pixman & BIT(1);
- 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;
}
- int dst_stride_words = dst_stride / sizeof(uint32_t);
- 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) {
+ int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
+ 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;
}
- int src_stride_words = src_stride / sizeof(uint32_t);
- if (src_x > 0x3fff || src_y > 0x3fff || src_bits >= end
- || src_bits + src_x
- + (src_y + dst_height) * src_stride >= end) {
+ int src_stride_words = ctx->src_stride / sizeof(uint32_t);
+ 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, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d)\n",
- src_bits, dst_bits, src_stride_words, dst_stride_words,
- bpp, bpp, src_x, src_y, dst_x, dst_y,
- dst_width, dst_height);
+ ctx->src_bits, 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);
#ifdef CONFIG_PIXMAN
- if (use_pixman_blt && 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);
+ if (use_pixman_blt && 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 (use_pixman_blt) {
/* 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
@@ -169,18 +194,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;
@@ -191,35 +219,38 @@ 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, %d, %d, %d, %d, %d, %d, %x)\n",
- dst_bits, dst_stride_words, bpp, dst_x, dst_y,
- dst_width, dst_height, filler);
+ ctx->dst_bits, dst_stride_words, ctx->bpp, ctx->dst.x,
+ ctx->dst.y, ctx->dst.width, ctx->dst.height, filler);
#ifdef CONFIG_PIXMAN
if (!use_pixman_fill ||
- !pixman_fill((uint32_t *)dst_bits, dst_stride_words, bpp, dst_x,
- dst_y, dst_width, dst_height, filler))
+ !pixman_fill((uint32_t *)ctx->dst_bits, dst_stride_words, 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);
}
}
}
@@ -227,9 +258,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.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 12/17] ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (10 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 11/17] ati-vga: Introduce ATI2DCtx struct for 2D blit context Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 13/17] ati-vga: Split ati_2d_do_blt " Chad Jablonski
` (5 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
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>
---
hw/display/ati_2d.c | 73 +++++++++++++++++++++++----------------------
1 file changed, 38 insertions(+), 35 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index d43268ba5f..f1536e5425 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,76 +80,79 @@ 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: */
- bool use_pixman_fill = s->use_pixman & BIT(0);
- bool use_pixman_blt = s->use_pixman & BIT(1);
- 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)
+{
+ bool use_pixman_fill = s->use_pixman & BIT(0);
+ bool use_pixman_blt = s->use_pixman & BIT(1);
+ 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;
- }
int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
- 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;
- }
int src_stride_words = ctx->src_stride / sizeof(uint32_t);
if (ctx->src.x > 0x3fff || ctx->src.y > 0x3fff
|| ctx->src_bits >= ctx->vram_end
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 13/17] ati-vga: Split ati_2d_do_blt from ati_2d_blt
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (11 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 12/17] ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 14/17] ati-vga: Remove ATIVGAState param from ati_2d_do_blt Chad Jablonski
` (4 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
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>
---
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 f1536e5425..df8975d9d7 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -123,27 +123,24 @@ 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)
{
bool use_pixman_fill = s->use_pixman & BIT(0);
bool use_pixman_blt = s->use_pixman & BIT(1);
- 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;
}
int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
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:
@@ -151,7 +148,7 @@ 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;
}
int src_stride_words = ctx->src_stride / sizeof(uint32_t);
if (ctx->src.x > 0x3fff || ctx->src.y > 0x3fff
@@ -159,7 +156,7 @@ void ati_2d_blt(ATIVGAState *s)
|| 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, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d)\n",
@@ -262,8 +259,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.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 14/17] ati-vga: Remove ATIVGAState param from ati_2d_do_blt
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (12 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 13/17] ati-vga: Split ati_2d_do_blt " Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 15/17] ati-vga: Implement scissor rectangle clipping for 2D operations Chad Jablonski
` (3 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
This completes the decoupling from the ATIVGAState struct.
Signed-off-by: Chad Jablonski <chad@jablonski.xyz>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/display/ati_2d.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index df8975d9d7..4fa28f7beb 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -123,10 +123,10 @@ 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)
{
- bool use_pixman_fill = s->use_pixman & BIT(0);
- bool use_pixman_blt = s->use_pixman & BIT(1);
+ bool use_pixman_fill = use_pixman & BIT(0);
+ bool use_pixman_blt = use_pixman & BIT(1);
if (!ctx->bpp) {
qemu_log_mask(LOG_GUEST_ERROR, "Invalid bpp\n");
return false;
@@ -269,7 +269,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.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 15/17] ati-vga: Implement scissor rectangle clipping for 2D operations
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (13 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 14/17] ati-vga: Remove ATIVGAState param from ati_2d_do_blt Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 16/17] ati-vga: Implement HOST_DATA register writes Chad Jablonski
` (2 subsequent siblings)
17 siblings, 0 replies; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
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>
---
hw/display/ati_2d.c | 84 ++++++++++++++++++++++++++++++---------------
1 file changed, 57 insertions(+), 27 deletions(-)
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 4fa28f7beb..e240093f12 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 ?
@@ -127,6 +133,7 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
{
bool use_pixman_fill = use_pixman & BIT(0);
bool use_pixman_blt = use_pixman & BIT(1);
+ QemuRect vis_src, vis_dst;
if (!ctx->bpp) {
qemu_log_mask(LOG_GUEST_ERROR, "Invalid bpp\n");
return false;
@@ -142,6 +149,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->src.width, ctx->src.height,
+ vis_src.x, vis_src.y, vis_src.width, vis_src.height);
+
switch (ctx->rop3) {
case ROP3_SRCCOPY:
{
@@ -151,9 +181,9 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
return false;
}
int src_stride_words = ctx->src_stride / sizeof(uint32_t);
- if (ctx->src.x > 0x3fff || ctx->src.y > 0x3fff
+ if (vis_src.x > 0x3fff || vis_src.y > 0x3fff
|| ctx->src_bits >= ctx->vram_end
- || ctx->src_bits + ctx->src.x + (ctx->src.y + ctx->dst.height)
+ || 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;
@@ -161,31 +191,31 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
DPRINTF("pixman_blt(%p, %p, %d, %d, %d, %d, %d, %d, %d, %d, %d, %d)\n",
ctx->src_bits, 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);
+ dst_stride_words, 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
if (use_pixman_blt && 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);
+ vis_src.x, vis_src.y, vis_dst.x, vis_dst.y,
+ vis_dst.width, vis_dst.height);
} else if (use_pixman_blt) {
/* 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
@@ -195,20 +225,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;
@@ -236,20 +266,20 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
}
DPRINTF("pixman_fill(%p, %d, %d, %d, %d, %d, %d, %x)\n",
- ctx->dst_bits, dst_stride_words, ctx->bpp, ctx->dst.x,
- ctx->dst.y, ctx->dst.width, ctx->dst.height, filler);
+ ctx->dst_bits, dst_stride_words, ctx->bpp, vis_dst.x,
+ vis_dst.y, vis_dst.width, vis_dst.height, filler);
#ifdef CONFIG_PIXMAN
if (!use_pixman_fill ||
!pixman_fill((uint32_t *)ctx->dst_bits, dst_stride_words, 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.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 16/17] ati-vga: Implement HOST_DATA register writes
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (14 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 15/17] ati-vga: Implement scissor rectangle clipping for 2D operations Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-04 0:32 ` BALATON Zoltan
2026-03-03 2:47 ` [PATCH v11 17/17] ati-vga: Implement HOST_DATA flush to VRAM Chad Jablonski
2026-03-08 22:21 ` [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Philippe Mathieu-Daudé
17 siblings, 1 reply; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
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>
---
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 6cf243bcf9..fa31401ba6 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -1024,6 +1024,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;
}
@@ -1129,6 +1150,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 98f57ca5fa..baa264215c 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -95,6 +95,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;
@@ -112,6 +120,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.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v11 17/17] ati-vga: Implement HOST_DATA flush to VRAM
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (15 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 16/17] ati-vga: Implement HOST_DATA register writes Chad Jablonski
@ 2026-03-03 2:47 ` Chad Jablonski
2026-03-04 0:38 ` BALATON Zoltan
2026-03-08 22:21 ` [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Philippe Mathieu-Daudé
17 siblings, 1 reply; 25+ messages in thread
From: Chad Jablonski @ 2026-03-03 2:47 UTC (permalink / raw)
To: qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau,
Chad Jablonski
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>
---
hw/display/ati.c | 6 +-
hw/display/ati_2d.c | 131 +++++++++++++++++++++++++++++++++++++++++-
hw/display/ati_int.h | 3 +
hw/display/ati_regs.h | 4 ++
4 files changed, 138 insertions(+), 6 deletions(-)
diff --git a/hw/display/ati.c b/hw/display/ati.c
index fa31401ba6..c93ef64525 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -1038,11 +1038,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 e240093f12..549a85dd3c 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;
@@ -181,10 +183,10 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
return false;
}
int src_stride_words = ctx->src_stride / sizeof(uint32_t);
- 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) {
+ * ctx->src_stride >= ctx->vram_end)) {
qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
return false;
}
@@ -298,8 +300,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 baa264215c..28f4e9d977 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -32,6 +32,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)
@@ -126,5 +127,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.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v11 16/17] ati-vga: Implement HOST_DATA register writes
2026-03-03 2:47 ` [PATCH v11 16/17] ati-vga: Implement HOST_DATA register writes Chad Jablonski
@ 2026-03-04 0:32 ` BALATON Zoltan
0 siblings, 0 replies; 25+ messages in thread
From: BALATON Zoltan @ 2026-03-04 0:32 UTC (permalink / raw)
To: Chad Jablonski; +Cc: qemu-devel, Gerd Hoffmann, Marc-André Lureau
On Mon, 2 Mar 2026, Chad Jablonski wrote:
> 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>
> ---
> 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 6cf243bcf9..fa31401ba6 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -1024,6 +1024,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;
> }
> @@ -1129,6 +1150,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 98f57ca5fa..baa264215c 100644
> --- a/hw/display/ati_int.h
> +++ b/hw/display/ati_int.h
> @@ -95,6 +95,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;
> @@ -112,6 +120,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
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 17/17] ati-vga: Implement HOST_DATA flush to VRAM
2026-03-03 2:47 ` [PATCH v11 17/17] ati-vga: Implement HOST_DATA flush to VRAM Chad Jablonski
@ 2026-03-04 0:38 ` BALATON Zoltan
2026-03-04 1:26 ` Chad Jablonski
0 siblings, 1 reply; 25+ messages in thread
From: BALATON Zoltan @ 2026-03-04 0:38 UTC (permalink / raw)
To: Chad Jablonski; +Cc: qemu-devel, Gerd Hoffmann, Marc-André Lureau
On Mon, 2 Mar 2026, Chad Jablonski wrote:
> 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>
Also tested that it works with MorphOS that was known to need it for some
font rendering so this fixes Xorg and MorphOS. It is now fully reviewed,
but I can't send pull requests so is there anybody collecting patches for
hw/display and will send a pull before the freeze? I may have one more
patch to send before the freeze but otherwise this series is all we would
like to include for ati-vga in the coming release.
Thank you,
BALATON Zoltan
> ---
> hw/display/ati.c | 6 +-
> hw/display/ati_2d.c | 131 +++++++++++++++++++++++++++++++++++++++++-
> hw/display/ati_int.h | 3 +
> hw/display/ati_regs.h | 4 ++
> 4 files changed, 138 insertions(+), 6 deletions(-)
>
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index fa31401ba6..c93ef64525 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -1038,11 +1038,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 e240093f12..549a85dd3c 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;
> @@ -181,10 +183,10 @@ static bool ati_2d_do_blt(ATI2DCtx *ctx, uint8_t use_pixman)
> return false;
> }
> int src_stride_words = ctx->src_stride / sizeof(uint32_t);
> - 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) {
> + * ctx->src_stride >= ctx->vram_end)) {
> qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
> return false;
> }
> @@ -298,8 +300,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 baa264215c..28f4e9d977 100644
> --- a/hw/display/ati_int.h
> +++ b/hw/display/ati_int.h
> @@ -32,6 +32,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)
> @@ -126,5 +127,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
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 17/17] ati-vga: Implement HOST_DATA flush to VRAM
2026-03-04 0:38 ` BALATON Zoltan
@ 2026-03-04 1:26 ` Chad Jablonski
2026-03-04 1:36 ` BALATON Zoltan
0 siblings, 1 reply; 25+ messages in thread
From: Chad Jablonski @ 2026-03-04 1:26 UTC (permalink / raw)
To: BALATON Zoltan, Chad Jablonski
Cc: qemu-devel, Gerd Hoffmann, Marc-André Lureau
>
> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
>
> Also tested that it works with MorphOS that was known to need it for some
> font rendering so this fixes Xorg and MorphOS. It is now fully reviewed,
This is great! Thank you Zoltan for all of your time and patience
reviewing.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 17/17] ati-vga: Implement HOST_DATA flush to VRAM
2026-03-04 1:26 ` Chad Jablonski
@ 2026-03-04 1:36 ` BALATON Zoltan
0 siblings, 0 replies; 25+ messages in thread
From: BALATON Zoltan @ 2026-03-04 1:36 UTC (permalink / raw)
To: Chad Jablonski; +Cc: qemu-devel, Gerd Hoffmann, Marc-André Lureau
On Tue, 3 Mar 2026, Chad Jablonski wrote:
>> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
>>
>> Also tested that it works with MorphOS that was known to need it for some
>> font rendering so this fixes Xorg and MorphOS. It is now fully reviewed,
>
> This is great! Thank you Zoltan for all of your time and patience
> reviewing.
Thank you for taking up this work and not giving up and doing an excellent
work testing real cards and modelling it. Now we just need somebody to
pick up the patches so it can be merged. Hopefully somebody is still
reading this thread.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
` (16 preceding siblings ...)
2026-03-03 2:47 ` [PATCH v11 17/17] ati-vga: Implement HOST_DATA flush to VRAM Chad Jablonski
@ 2026-03-08 22:21 ` Philippe Mathieu-Daudé
2026-03-09 0:12 ` BALATON Zoltan
17 siblings, 1 reply; 25+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-03-08 22:21 UTC (permalink / raw)
To: Chad Jablonski, qemu-devel
Cc: BALATON Zoltan, Gerd Hoffmann, Marc-André Lureau
Hi Chad,
On 3/3/26 03:47, Chad Jablonski wrote:
> This series implements HOST_DATA as a blit source enabling text rendering in
> xterm under X.org with 2D acceleration.
>
> The series builds up functionality incrementally:
>
> * Patches 1-6: Bug fixes and register implementations
> * Patches 7-14: Refactor of ati_2d_blt to decouple from ATIVGAState
> * Patch 15: Scissor clipping implementation
> * Patches 16-17: HOST_DATA register writes, color expansion, and
> accumulator flush
> Chad Jablonski (17):
> ati-vga: Fix framebuffer mapping by using hardware-correct aperture
> sizes
> ati-vga: Fix DST_PITCH and SRC_PITCH reads
> ati-vga: Read aliased values from DP_GUI_MASTER_CNTL
> ati-vga: Latch src and dst pitch and offset on master_cntl default
> ati-vga: Implement foreground and background color register writes
> ati-vga: Add scissor clipping register support
> ati-vga: Remove dst_x/y updates after blit
> ati-vga: Consolidate dirty region tracking in ati_2d_blt
> 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
Zoltan asked me to look at your series [*] but unfortunately I'm
getting a build failure when building without PIXMAN starting
with patch #9:
../hw/display/ati_2d.c: In function ‘ati_2d_do_blt’:
../hw/display/ati_2d.c:185:13: error: unused variable ‘src_stride_words’
[-Werror=unused-variable]
185 | int src_stride_words = ctx->src_stride / sizeof(uint32_t);
| ^~~~~~~~~~~~~~~~
../hw/display/ati_2d.c:147:9: error: unused variable ‘dst_stride_words’
[-Werror=unused-variable]
147 | int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
| ^~~~~~~~~~~~~~~~
../hw/display/ati_2d.c:137:10: error: unused variable ‘use_pixman_blt’
[-Werror=unused-variable]
137 | bool use_pixman_blt = use_pixman & BIT(1);
| ^~~~~~~~~~~~~~
../hw/display/ati_2d.c:136:10: error: unused variable ‘use_pixman_fill’
[-Werror=unused-variable]
136 | bool use_pixman_fill = use_pixman & BIT(0);
| ^~~~~~~~~~~~~~~
I'm queueing the first 8 patches hoping it helps.
Also, please fix the checkpatch.pl coding style errors
before re-posting.
Regards,
Phil.
[*]
https://lore.kernel.org/qemu-devel/7f1df452-05c2-efa3-fdce-a5d987cfdcc8@eik.bme.hu/
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
2026-03-08 22:21 ` [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Philippe Mathieu-Daudé
@ 2026-03-09 0:12 ` BALATON Zoltan
2026-03-09 1:55 ` BALATON Zoltan
0 siblings, 1 reply; 25+ messages in thread
From: BALATON Zoltan @ 2026-03-09 0:12 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Chad Jablonski, qemu-devel, Gerd Hoffmann, Marc-André Lureau
[-- Attachment #1: Type: text/plain, Size: 3042 bytes --]
On Sun, 8 Mar 2026, Philippe Mathieu-Daudé wrote:
> Hi Chad,
>
> On 3/3/26 03:47, Chad Jablonski wrote:
>> This series implements HOST_DATA as a blit source enabling text rendering
>> in
>> xterm under X.org with 2D acceleration.
>>
>> The series builds up functionality incrementally:
>>
>> * Patches 1-6: Bug fixes and register implementations
>> * Patches 7-14: Refactor of ati_2d_blt to decouple from ATIVGAState
>> * Patch 15: Scissor clipping implementation
>> * Patches 16-17: HOST_DATA register writes, color expansion, and
>> accumulator flush
>
>
>> Chad Jablonski (17):
>> ati-vga: Fix framebuffer mapping by using hardware-correct aperture
>> sizes
>> ati-vga: Fix DST_PITCH and SRC_PITCH reads
>> ati-vga: Read aliased values from DP_GUI_MASTER_CNTL
>> ati-vga: Latch src and dst pitch and offset on master_cntl default
>> ati-vga: Implement foreground and background color register writes
>> ati-vga: Add scissor clipping register support
>> ati-vga: Remove dst_x/y updates after blit
>> ati-vga: Consolidate dirty region tracking in ati_2d_blt
>> 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
>
> Zoltan asked me to look at your series [*] but unfortunately I'm
> getting a build failure when building without PIXMAN starting
> with patch #9:
>
> ../hw/display/ati_2d.c: In function ‘ati_2d_do_blt’:
> ../hw/display/ati_2d.c:185:13: error: unused variable ‘src_stride_words’
> [-Werror=unused-variable]
> 185 | int src_stride_words = ctx->src_stride / sizeof(uint32_t);
> | ^~~~~~~~~~~~~~~~
> ../hw/display/ati_2d.c:147:9: error: unused variable ‘dst_stride_words’
> [-Werror=unused-variable]
> 147 | int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
> | ^~~~~~~~~~~~~~~~
> ../hw/display/ati_2d.c:137:10: error: unused variable ‘use_pixman_blt’
> [-Werror=unused-variable]
> 137 | bool use_pixman_blt = use_pixman & BIT(1);
> | ^~~~~~~~~~~~~~
> ../hw/display/ati_2d.c:136:10: error: unused variable ‘use_pixman_fill’
> [-Werror=unused-variable]
> 136 | bool use_pixman_fill = use_pixman & BIT(0);
> | ^~~~~~~~~~~~~~~
>
> I'm queueing the first 8 patches hoping it helps.
>
> Also, please fix the checkpatch.pl coding style errors
> before re-posting.
Sorry, my mistake I did not notice this during review and testing. I'm
fixing it up and will send updated version of the last part of the series
not yet queued. Thanks a lot for your help.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering
2026-03-09 0:12 ` BALATON Zoltan
@ 2026-03-09 1:55 ` BALATON Zoltan
0 siblings, 0 replies; 25+ messages in thread
From: BALATON Zoltan @ 2026-03-09 1:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Chad Jablonski, qemu-devel, Gerd Hoffmann, Marc-André Lureau
[-- Attachment #1: Type: text/plain, Size: 3339 bytes --]
On Mon, 9 Mar 2026, BALATON Zoltan wrote:
> On Sun, 8 Mar 2026, Philippe Mathieu-Daudé wrote:
>> Hi Chad,
>>
>> On 3/3/26 03:47, Chad Jablonski wrote:
>>> This series implements HOST_DATA as a blit source enabling text rendering
>>> in
>>> xterm under X.org with 2D acceleration.
>>>
>>> The series builds up functionality incrementally:
>>>
>>> * Patches 1-6: Bug fixes and register implementations
>>> * Patches 7-14: Refactor of ati_2d_blt to decouple from ATIVGAState
>>> * Patch 15: Scissor clipping implementation
>>> * Patches 16-17: HOST_DATA register writes, color expansion, and
>>> accumulator flush
>>
>>
>>> Chad Jablonski (17):
>>> ati-vga: Fix framebuffer mapping by using hardware-correct aperture
>>> sizes
>>> ati-vga: Fix DST_PITCH and SRC_PITCH reads
>>> ati-vga: Read aliased values from DP_GUI_MASTER_CNTL
>>> ati-vga: Latch src and dst pitch and offset on master_cntl default
>>> ati-vga: Implement foreground and background color register writes
>>> ati-vga: Add scissor clipping register support
>>> ati-vga: Remove dst_x/y updates after blit
>>> ati-vga: Consolidate dirty region tracking in ati_2d_blt
>>> 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
>>
>> Zoltan asked me to look at your series [*] but unfortunately I'm
>> getting a build failure when building without PIXMAN starting
>> with patch #9:
>>
>> ../hw/display/ati_2d.c: In function ‘ati_2d_do_blt’:
>> ../hw/display/ati_2d.c:185:13: error: unused variable ‘src_stride_words’
>> [-Werror=unused-variable]
>> 185 | int src_stride_words = ctx->src_stride / sizeof(uint32_t);
>> | ^~~~~~~~~~~~~~~~
>> ../hw/display/ati_2d.c:147:9: error: unused variable ‘dst_stride_words’
>> [-Werror=unused-variable]
>> 147 | int dst_stride_words = ctx->dst_stride / sizeof(uint32_t);
>> | ^~~~~~~~~~~~~~~~
>> ../hw/display/ati_2d.c:137:10: error: unused variable ‘use_pixman_blt’
>> [-Werror=unused-variable]
>> 137 | bool use_pixman_blt = use_pixman & BIT(1);
>> | ^~~~~~~~~~~~~~
>> ../hw/display/ati_2d.c:136:10: error: unused variable ‘use_pixman_fill’
>> [-Werror=unused-variable]
>> 136 | bool use_pixman_fill = use_pixman & BIT(0);
>> | ^~~~~~~~~~~~~~~
>>
>> I'm queueing the first 8 patches hoping it helps.
>>
>> Also, please fix the checkpatch.pl coding style errors
>> before re-posting.
>
> Sorry, my mistake I did not notice this during review and testing. I'm fixing
> it up and will send updated version of the last part of the series not yet
> queued. Thanks a lot for your help.
I sent updated v12 fixing the above. Before that I've also sent this small
fix independent of this series:
https://patchew.org/QEMU/cover.1773009887.git.balaton@eik.bme.hu/
Thank you,
BALATON Zoltan
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2026-03-09 1:55 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-03 2:47 [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 01/17] ati-vga: Fix framebuffer mapping by using hardware-correct aperture sizes Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 02/17] ati-vga: Fix DST_PITCH and SRC_PITCH reads Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 03/17] ati-vga: Read aliased values from DP_GUI_MASTER_CNTL Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 04/17] ati-vga: Latch src and dst pitch and offset on master_cntl default Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 05/17] ati-vga: Implement foreground and background color register writes Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 06/17] ati-vga: Add scissor clipping register support Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 07/17] ati-vga: Remove dst_x/y updates after blit Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 08/17] ati-vga: Consolidate dirty region tracking in ati_2d_blt Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 09/17] ati-vga: Remove src and dst stride mutation " Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 10/17] ati-vga: Use local variables for register values " Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 11/17] ati-vga: Introduce ATI2DCtx struct for 2D blit context Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 12/17] ati-vga: Extract setup_2d_blt_ctx from ati_2d_blt Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 13/17] ati-vga: Split ati_2d_do_blt " Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 14/17] ati-vga: Remove ATIVGAState param from ati_2d_do_blt Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 15/17] ati-vga: Implement scissor rectangle clipping for 2D operations Chad Jablonski
2026-03-03 2:47 ` [PATCH v11 16/17] ati-vga: Implement HOST_DATA register writes Chad Jablonski
2026-03-04 0:32 ` BALATON Zoltan
2026-03-03 2:47 ` [PATCH v11 17/17] ati-vga: Implement HOST_DATA flush to VRAM Chad Jablonski
2026-03-04 0:38 ` BALATON Zoltan
2026-03-04 1:26 ` Chad Jablonski
2026-03-04 1:36 ` BALATON Zoltan
2026-03-08 22:21 ` [PATCH v11 00/17] ati-vga: Implement HOST_DATA transfers to enable X.org text rendering Philippe Mathieu-Daudé
2026-03-09 0:12 ` BALATON Zoltan
2026-03-09 1:55 ` BALATON Zoltan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.