On Tue, 3 Feb 2026, Chad Jablonski wrote:
Are you sure dst_offset calculation this way is correct? Also if
s->regs.crtc_offset is not 0 as that seems to be added to dst_bits for
Rage128Pro. This may be replaced with correct value from ctx later but
then you could just keep the original here to make sure it's not broken.
Good catch on this. I totally overlooked the R128-specific update to it.
I'll keep the original here. It does eventually bite us again in patch
11. This was an attempt to avoid adding it to the ctx while also allowing us
to pass only VGACommonState into ati_set_dirty instead of the full
ATIVGAState. But it may have been a bit ambitious.
The alternative here is to either continue to pass ATIVGAState to
ati_set_dirty or add dst_offset to the ATI2DCtx. It feels a little out of
place to me (like vram_end) on ATI2DCtx but I think that's a better tradeoff
than continuing to pass the full ATIVGAState struct. I'll add it to the ctx
in v8 unless you have objections or a cleaner approach.
I can't tell without looking at the new patch version but I think
ati_vga_set_dirty needs to determine if the update is visible so it would
need two values for that: start of visible screen (offset from vram start)
and length of it so maybe these two values are needed in Ctx (or maybe
multiple of these as there could be more screens but we only emulate one
now) but I don't mind either if we keep passing ATIVGAState to all these
functions.
Originally I thought Ctx would only include values that override what are
in regs such as expanded bits and current src and dest coords that aren't
updated in regs so host data can set and update these and not include a
copy of every reg which could still be get from ATIVGAState. That way we
don't need a separate ATIHostDataState struct either just include an
ATI2DCtx in ATIVGAState and the blt function can use that instead of some
regs while can keep using regs for other values. But if this would be too
much reverting then I'm also OK with current approach and we can refine
it later.
Regards,
BALATON Zoltan
