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 <[email protected]>

---

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


Reply via email to