[PATCH v4 8/9] ati-vga: Implement HOST_DATA register writes

Chad Jablonski posted 9 patches 1 month ago
There is a newer version of this series
[PATCH v4 8/9] ati-vga: Implement HOST_DATA register writes
Posted by Chad Jablonski 1 month ago
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      | 32 ++++++++++++++++++++++++++++++++
 hw/display/ati_dbg.c  |  9 +++++++++
 hw/display/ati_int.h  |  9 +++++++++
 hw/display/ati_regs.h |  9 +++++++++
 4 files changed, 59 insertions(+)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index 04f1c3c790..88d30bf532 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -567,6 +567,13 @@ static inline void ati_reg_write_offs(uint32_t *reg, int offs,
     }
 }
 
+static void ati_host_data_reset(ATIHostDataState *hd)
+{
+    hd->next = 0;
+    hd->row = 0;
+    hd->col = 0;
+}
+
 static void ati_mm_write(void *opaque, hwaddr addr,
                            uint64_t data, unsigned int size)
 {
@@ -842,6 +849,7 @@ static void ati_mm_write(void *opaque, hwaddr addr,
         break;
     case DST_WIDTH:
         s->regs.dst_width = data & 0x3fff;
+        ati_host_data_reset(&s->host_data);
         ati_2d_blt(s);
         break;
     case DST_HEIGHT:
@@ -892,6 +900,7 @@ static void ati_mm_write(void *opaque, hwaddr addr,
     case DST_HEIGHT_WIDTH:
         s->regs.dst_width = data & 0x3fff;
         s->regs.dst_height = (data >> 16) & 0x3fff;
+        ati_host_data_reset(&s->host_data);
         ati_2d_blt(s);
         break;
     case DP_GUI_MASTER_CNTL:
@@ -929,6 +938,7 @@ static void ati_mm_write(void *opaque, hwaddr addr,
     case DST_WIDTH_X:
         s->regs.dst_x = data & 0x3fff;
         s->regs.dst_width = (data >> 16) & 0x3fff;
+        ati_host_data_reset(&s->host_data);
         ati_2d_blt(s);
         break;
     case SRC_X_Y:
@@ -942,6 +952,7 @@ static void ati_mm_write(void *opaque, hwaddr addr,
     case DST_WIDTH_HEIGHT:
         s->regs.dst_height = data & 0x3fff;
         s->regs.dst_width = (data >> 16) & 0x3fff;
+        ati_host_data_reset(&s->host_data);
         ati_2d_blt(s);
         break;
     case DST_HEIGHT_Y:
@@ -1043,6 +1054,25 @@ 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:
+        s->host_data.acc[s->host_data.next++] = data;
+        if (s->host_data.next >= 4) {
+            qemu_log_mask(LOG_UNIMP, "HOST_DATA flush not yet implemented\n");
+            s->host_data.next = 0;
+        }
+        break;
+    case HOST_DATA_LAST:
+        s->host_data.acc[s->host_data.next] = data;
+        qemu_log_mask(LOG_UNIMP, "HOST_DATA flush not yet implemented\n");
+        ati_host_data_reset(&s->host_data);
+        break;
     default:
         break;
     }
@@ -1136,6 +1166,8 @@ static void ati_vga_reset(DeviceState *dev)
     /* reset vga */
     vga_common_reset(&s->vga);
     s->mode = VGA_MODE;
+
+    ati_host_data_reset(&s->host_data);
 }
 
 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 d9ac8ee135..3029dc7e3c 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -14,6 +14,7 @@
 #include "hw/i2c/bitbang_i2c.h"
 #include "vga_int.h"
 #include "qom/object.h"
+#include "qemu/units.h"
 
 /*#define DEBUG_ATI*/
 
@@ -97,6 +98,13 @@ typedef struct ATIVGARegs {
     uint16_t src_sc_right;
 } ATIVGARegs;
 
+typedef struct ATIHostDataState {
+    uint32_t row;
+    uint32_t col;
+    uint32_t next;
+    uint32_t acc[4];
+} ATIHostDataState;
+
 struct ATIVGAState {
     PCIDevice dev;
     VGACommonState vga;
@@ -113,6 +121,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 9c638314f0..c8bbafe1c6 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.51.2
Re: [PATCH v4 8/9] ati-vga: Implement HOST_DATA register writes
Posted by BALATON Zoltan 1 month ago
On Tue, 6 Jan 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>
> ---
> hw/display/ati.c      | 32 ++++++++++++++++++++++++++++++++
> hw/display/ati_dbg.c  |  9 +++++++++
> hw/display/ati_int.h  |  9 +++++++++
> hw/display/ati_regs.h |  9 +++++++++
> 4 files changed, 59 insertions(+)
>
[...]
> diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
> index d9ac8ee135..3029dc7e3c 100644
> --- a/hw/display/ati_int.h
> +++ b/hw/display/ati_int.h
> @@ -14,6 +14,7 @@
> #include "hw/i2c/bitbang_i2c.h"
> #include "vga_int.h"
> #include "qom/object.h"
> +#include "qemu/units.h"

This include seems to be unneeded.

Regards,
BALATON Zoltan

> /*#define DEBUG_ATI*/
>
> @@ -97,6 +98,13 @@ typedef struct ATIVGARegs {
>     uint16_t src_sc_right;
> } ATIVGARegs;
>
> +typedef struct ATIHostDataState {
> +    uint32_t row;
> +    uint32_t col;
> +    uint32_t next;
> +    uint32_t acc[4];
> +} ATIHostDataState;
> +
> struct ATIVGAState {
>     PCIDevice dev;
>     VGACommonState vga;
> @@ -113,6 +121,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 9c638314f0..c8bbafe1c6 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
>
Re: [PATCH v4 8/9] ati-vga: Implement HOST_DATA register writes
Posted by BALATON Zoltan 1 month ago
On Tue, 6 Jan 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>
> ---
> hw/display/ati.c      | 32 ++++++++++++++++++++++++++++++++
> hw/display/ati_dbg.c  |  9 +++++++++
> hw/display/ati_int.h  |  9 +++++++++
> hw/display/ati_regs.h |  9 +++++++++
> 4 files changed, 59 insertions(+)
>
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index 04f1c3c790..88d30bf532 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -567,6 +567,13 @@ static inline void ati_reg_write_offs(uint32_t *reg, int offs,
>     }
> }
>
> +static void ati_host_data_reset(ATIHostDataState *hd)
> +{
> +    hd->next = 0;
> +    hd->row = 0;
> +    hd->col = 0;
> +}
> +
> static void ati_mm_write(void *opaque, hwaddr addr,
>                            uint64_t data, unsigned int size)
> {
> @@ -842,6 +849,7 @@ static void ati_mm_write(void *opaque, hwaddr addr,
>         break;
>     case DST_WIDTH:
>         s->regs.dst_width = data & 0x3fff;
> +        ati_host_data_reset(&s->host_data);
>         ati_2d_blt(s);
>         break;
>     case DST_HEIGHT:
> @@ -892,6 +900,7 @@ static void ati_mm_write(void *opaque, hwaddr addr,
>     case DST_HEIGHT_WIDTH:
>         s->regs.dst_width = data & 0x3fff;
>         s->regs.dst_height = (data >> 16) & 0x3fff;
> +        ati_host_data_reset(&s->host_data);
>         ati_2d_blt(s);
>         break;
>     case DP_GUI_MASTER_CNTL:
> @@ -929,6 +938,7 @@ static void ati_mm_write(void *opaque, hwaddr addr,
>     case DST_WIDTH_X:
>         s->regs.dst_x = data & 0x3fff;
>         s->regs.dst_width = (data >> 16) & 0x3fff;
> +        ati_host_data_reset(&s->host_data);
>         ati_2d_blt(s);
>         break;
>     case SRC_X_Y:
> @@ -942,6 +952,7 @@ static void ati_mm_write(void *opaque, hwaddr addr,
>     case DST_WIDTH_HEIGHT:
>         s->regs.dst_height = data & 0x3fff;
>         s->regs.dst_width = (data >> 16) & 0x3fff;
> +        ati_host_data_reset(&s->host_data);

Is it documented somewhere that writing these registers reset the host 
data or is it from experiments? Are there any other registers that may 
need to reset or are these the only place (apart from HOST_DATA_LAST) that 
terminate host data?

Regards,
BALATON Zoltan

>         ati_2d_blt(s);
>         break;
>     case DST_HEIGHT_Y:
> @@ -1043,6 +1054,25 @@ 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:
> +        s->host_data.acc[s->host_data.next++] = data;
> +        if (s->host_data.next >= 4) {
> +            qemu_log_mask(LOG_UNIMP, "HOST_DATA flush not yet implemented\n");
> +            s->host_data.next = 0;
> +        }
> +        break;
> +    case HOST_DATA_LAST:
> +        s->host_data.acc[s->host_data.next] = data;
> +        qemu_log_mask(LOG_UNIMP, "HOST_DATA flush not yet implemented\n");
> +        ati_host_data_reset(&s->host_data);
> +        break;
>     default:
>         break;
>     }
> @@ -1136,6 +1166,8 @@ static void ati_vga_reset(DeviceState *dev)
>     /* reset vga */
>     vga_common_reset(&s->vga);
>     s->mode = VGA_MODE;
> +
> +    ati_host_data_reset(&s->host_data);
> }
>
> 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 d9ac8ee135..3029dc7e3c 100644
> --- a/hw/display/ati_int.h
> +++ b/hw/display/ati_int.h
> @@ -14,6 +14,7 @@
> #include "hw/i2c/bitbang_i2c.h"
> #include "vga_int.h"
> #include "qom/object.h"
> +#include "qemu/units.h"
>
> /*#define DEBUG_ATI*/
>
> @@ -97,6 +98,13 @@ typedef struct ATIVGARegs {
>     uint16_t src_sc_right;
> } ATIVGARegs;
>
> +typedef struct ATIHostDataState {
> +    uint32_t row;
> +    uint32_t col;
> +    uint32_t next;
> +    uint32_t acc[4];
> +} ATIHostDataState;
> +
> struct ATIVGAState {
>     PCIDevice dev;
>     VGACommonState vga;
> @@ -113,6 +121,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 9c638314f0..c8bbafe1c6 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
>
Re: [PATCH v4 8/9] ati-vga: Implement HOST_DATA register writes
Posted by Chad Jablonski 1 month ago
>
> Is it documented somewhere that writing these registers reset the host 
> data or is it from experiments? Are there any other registers that may 
> need to reset or are these the only place (apart from HOST_DATA_LAST) that 
> terminate host data?
>

I haven't seen documentation on this or done extensive testing on it yet. My
reasoning for placing them here was that these writes kick off blits and so
it seemed like a safe place to do this. I agree though, it would be much better
to know how this actually behaves. I suspect at the very least there are more
register writes that do this. I'll see what else I can learn here
through testing.
Re: [PATCH v4 8/9] ati-vga: Implement HOST_DATA register writes
Posted by BALATON Zoltan 1 month ago
On Tue, 6 Jan 2026, Chad Jablonski wrote:
>> Is it documented somewhere that writing these registers reset the host
>> data or is it from experiments? Are there any other registers that may
>> need to reset or are these the only place (apart from HOST_DATA_LAST) that
>> terminate host data?
>
> I haven't seen documentation on this or done extensive testing on it yet. My
> reasoning for placing them here was that these writes kick off blits and so
> it seemed like a safe place to do this. I agree though, it would be much better
> to know how this actually behaves. I suspect at the very least there are more
> register writes that do this. I'll see what else I can learn here
> through testing.

It's OK for now, we can fix it up later if new info is found I was just 
asking if this was something known or a guess. If you can do testing with 
real card that's better but not needed if it would take too much time.

Regards,
BALATON Zoltan