[Qemu-devel] Possible bug in cirrus_vga.c vs. HDR handling

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

Hi folks !

I was debugging a problem with 16bpp support, when I found out that
my attempts at writing to the Hidden DAC Register were not working.

The reason was that I (well, cirrusdrmfb really) was doing the sequence
READ, READ, READ, READ, WRITE (to the DAC mask register), which should
have worked, except that the code in qemu for reads does:

static int cirrus_read_hidden_dac(CirrusVGAState * s)
{
    if (++s->cirrus_hidden_dac_lockindex == 5) {
        s->cirrus_hidden_dac_lockindex = 0;
        return s->cirrus_hidden_dac_data;
    }
    return 0xff;
}

(Note the *pre*increment)

And s->cirrus_hidden_dac_lockindex is initialized in  cirrus_reset() to:

 s->cirrus_hidden_dac_lockindex = 5;

So the first 4 reads cause it to go to 6,7,8,9... and never
actually wrap back, so the write doesn't work as it checks for
the value being exactly 4.

Now, that -might- mimmic the real HW (I've added a workaround to drmfb
that does a dummy write first), but it looks dubious to me.

Shouldn't we initialize the lockindex to 0 in reset ? or to 4 ?

Cheers,
Ben.