Re: [PATCH v6 7/8] ati-vga: Simplify pointer image handling

[BALATON Zoltan <balaton@eik.bme.hu>]

On Tue, 24 Mar 2026, Chad Jablonski wrote:
>>      /* FIXME handle cur_hv_offs correctly */
>> +    srcoff = s->regs.cur_offset - (s->regs.cur_hv_offs >> 16) -
>> +             (s->regs.cur_hv_offs & 0xffff) * 16;
>
> This is pre-existing (and a FIXME!) but do we want to be subtracting the
> horizontal cursor offset (s->regs.cur_hv_offs >> 16) here? I think when
> it's non-zero it would mean that the first row of the cursor would be offset
> and rows would contain bytes from the next row at the end. The way I read the
> register reference is that the horizontal offset is the number of columns the
> cursor is shifted. So I don't think this would affect the source, just how it's drawn.

As you say it's preexisting and I did not try to fix that and left the 
FIXME comment there. I haven't seen anything using hv_offset so far except 
the Linux test image at the very left and top of the screen but that also 
has other problems with the mouse pointer so unless it's first confirmed 
on real hardware that it's supposed to work I won't try to fix that 
because I tested MacOS that also uses big endian frame buffer and that 
works now so it might be the Linux driver is buggy or there's some other 
swap bit somewhere we should take into account. As nothing else seems to 
use non-0 hv_offset this should not matter for most drivers so until we 
see something that needs it and we can test with I leave it a FIXME.

>> +    for (int i = 0; i < 64; i++, srcoff += 16) {
>> +        data[i] = ldq_le_p(&s->vga.vram_ptr[srcoff]);
>> +        data[i + 64] = ldq_le_p(&s->vga.vram_ptr[srcoff + 8]);
>
> Is this ldq_le_p correct here? Wouldn't this cause issues on BE hosts?

I'm not sure as I noted in commit message. According to the docs I've seen 
hw cursor data should always be little endian and if we pass these on as 
host endian data here then le should be correct but I could not test on 
big endian host so can't say for sure.

Regards,
BALATON Zoltan