[PATCH v5 08/12] ati-vga: Consolidate set dirty and dst update

Chad Jablonski posted 12 patches 3 weeks, 1 day ago
There is a newer version of this series
[PATCH v5 08/12] ati-vga: Consolidate set dirty and dst update
Posted by Chad Jablonski 3 weeks, 1 day ago
Both supported ROPs follow the same memory set dirty logic.
This consolidates that logic to remove the duplication.

Hardware testing revealed that the Rage 128 does not update dst_x or dst_y
after a blit, regardless of the source. This removes the update for the
Rage 128 device.

Note that there is a behavior change here in that previously the "fill"
ROPs updated only dst_y and SRC_COPY updated dst_y and dst_x. The
Mobility M6 register reference (DST_HEIGHT_WIDTH) states that dst_y is
updated after a blit but doesn't mention dst_x.

Signed-off-by: Chad Jablonski <chad@jablonski.xyz>

---

I plan to validate the above Radeon behavior in the future but I don't
have the best test environment set up for that card at the moment.
Zoltan if you've seen the dst_x behavior is required then we can modify
this but otherwise it felt safe to me to follow the docs for now.
---
 hw/display/ati_2d.c | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 91fd3b7827..38390f2da8 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -207,18 +207,6 @@ void ati_2d_blt(ATIVGAState *s)
                 memmove(&dst->bits[i], &src->bits[j], dst->rect.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->rect.y * surface_stride(ds),
-                                    dst->rect.height * surface_stride(ds));
-        }
-        s->regs.dst_x = (dst->left_to_right ?
-                         dst->rect.x + dst->rect.width : dst->rect.x);
-        s->regs.dst_y = (dst->top_to_bottom ?
-                         dst->rect.y + dst->rect.height : dst->rect.y);
         break;
     }
     case ROP3_PATCOPY:
@@ -260,20 +248,29 @@ 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->rect.y * surface_stride(ds),
-                                    dst->rect.height * surface_stride(ds));
-        }
-        s->regs.dst_y = (dst->top_to_bottom ?
-                         dst->rect.y + dst->rect.height : dst->rect.y);
         break;
     }
     default:
         qemu_log_mask(LOG_UNIMP, "Unimplemented ati_2d blt op %x\n",
                       (s->regs.dp_mix & GMC_ROP3_MASK) >> 16);
     }
+
+    if (s->dev_id != PCI_DEVICE_ID_ATI_RAGE128_PF) {
+        /*
+         * Hardware testing shows that dst is _not_ updated for Rage 128.
+         * The M6 (R100/Radeon) docs state however that dst_y is updated.
+         * This has not yet been validated on R100 hardware.
+         */
+        s->regs.dst_y = (dst->top_to_bottom ?
+                        dst->rect.y + dst->rect.height : dst->rect.y);
+    }
+
+    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->rect.y * surface_stride(ds),
+                                dst->rect.height * surface_stride(ds));
+    }
 }
-- 
2.51.2
Re: [PATCH v5 08/12] ati-vga: Consolidate set dirty and dst update
Posted by Chad Jablonski 2 weeks, 3 days ago
>
> I plan to validate the above Radeon behavior in the future but I don't
> have the best test environment set up for that card at the moment.
> Zoltan if you've seen the dst_x behavior is required then we can modify
> this but otherwise it felt safe to me to follow the docs for now.

Just an update on Radeon (R100) behavior. I've confirmed in my testing that it
also does _not_ update dst_x and dst_y after vram and host data sourced blits.
This contradicts the m6 register guide.
Re: [PATCH v5 08/12] ati-vga: Consolidate set dirty and dst update
Posted by BALATON Zoltan 2 weeks, 3 days ago
On Tue, 20 Jan 2026, Chad Jablonski wrote:
>> I plan to validate the above Radeon behavior in the future but I don't
>> have the best test environment set up for that card at the moment.
>> Zoltan if you've seen the dst_x behavior is required then we can modify
>> this but otherwise it felt safe to me to follow the docs for now.
>
> Just an update on Radeon (R100) behavior. I've confirmed in my testing that it
> also does _not_ update dst_x and dst_y after vram and host data sourced blits.
> This contradicts the m6 register guide.

Interesting. I think I've added the update because I've found that some 
guest needed that but I don't remember which. I mostly tested with MorphOS 
and Linux. But it could be I've seen it in the docs and that's why I 
implemented it, I don't remember. If it does not break MorphOS it could be 
removed to match what hardware seem to do. I think most drivers will set 
these on every blit anyway.

Regards,
BALATON Zoltan