Re: [PATCH 5/5] hw/display/artist: Fix framebuffer access for Linux

[Sven Schnelle <svens@stackframe.org>]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> +Sven
>
> On 12/1/22 22:07, Helge Deller wrote:
>> This patch fixes two problems which prevented Linux to access the
>> artist graphics framebuffer:
>> The check if the framebuffer or the color map should be accessed was
>> incomplete. By using the vram_read/write_bufidx() functions we now check
>> correctly if ARTIST_BUFFER_CMAP should be accessed.
>> The second fix is to correctly calculate the X- and Y-coordinates
>> and
>> check against the graphics resolution.
>> With this fix in place, the Linux stifb driver now works correctly,
>> shows the penguins at bootup and uses the stifb as graphics console.
>
> Cool, could you add a test similar to these?
>
> $ git grep Tux tests/avocado/
> tests/avocado/machine_arm_integratorcp.py:69:        Boot Linux and
> verify the Tux logo is displayed on the framebuffer.
> tests/avocado/machine_mips_malta.py:44:        Boot Linux kernel and
> check Tux logo is displayed on the framebuffer.
>
>> I haven't seen any negative side effects when running HP-UX.

Hmm, the patch below  breaks hp-ux 10.20 for me, please see the attached screenshot.

[-- Attachment #2: broken.png --]
[-- Type: image/png, Size: 26638 bytes --]

[-- Attachment #3: Type: text/plain, Size: 2077 bytes --]


>> Signed-off-by: Helge Deller <deller@gmx.de>
>> Cc: qemu-stable@nongnu.org
>> ---
>>   hw/display/artist.c | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>> diff --git a/hw/display/artist.c b/hw/display/artist.c
>> index 6384076c60..fbf5525334 100644
>> --- a/hw/display/artist.c
>> +++ b/hw/display/artist.c
>> @@ -1186,7 +1186,7 @@ static void artist_vram_write(void *opaque, hwaddr addr, uint64_t val,
>>       unsigned int offset;
>>       trace_artist_vram_write(size, addr, val);
>> -    if (s->cmap_bm_access) {
>> +    if (vram_write_bufidx(s) == ARTIST_BUFFER_CMAP) {
>>           buf = &s->vram_buffer[ARTIST_BUFFER_CMAP];
>>           if (addr + 3 < buf->size) {
>>               *(uint32_t *)(buf->data + addr) = val;
>> @@ -1195,14 +1195,14 @@ static void artist_vram_write(void *opaque, hwaddr addr, uint64_t val,
>>       }
>>       buf = vram_write_buffer(s);
>> -    posy = ADDR_TO_Y(addr >> 2);
>> -    posx = ADDR_TO_X(addr >> 2);
>> +    posy = ADDR_TO_Y(addr);
>> +    posx = ADDR_TO_X(addr);
>>       if (!buf->size) {
>>           return;
>>       }
>> -    if (posy > buf->height || posx > buf->width) {
>> +    if (posy >= buf->height || posx >= buf->width) {
>>           return;
>>       }
>> @@ -1242,7 +1242,7 @@ static uint64_t artist_vram_read(void *opaque,
>> hwaddr addr, unsigned size)
>>       uint64_t val;
>>       unsigned int posy, posx;
>> -    if (s->cmap_bm_access) {
>> +    if (vram_read_bufidx(s) == ARTIST_BUFFER_CMAP) {
>>           buf = &s->vram_buffer[ARTIST_BUFFER_CMAP];
>>           val = 0;
>>           if (addr < buf->size && addr + 3 < buf->size) {
>> @@ -1257,10 +1257,10 @@ static uint64_t artist_vram_read(void *opaque, hwaddr addr, unsigned size)
>>           return 0;
>>       }
>> -    posy = ADDR_TO_Y(addr >> 2);
>> -    posx = ADDR_TO_X(addr >> 2);
>> +    posy = ADDR_TO_Y(addr);
>> +    posx = ADDR_TO_X(addr);
>> -    if (posy > buf->height || posx > buf->width) {
>> +    if (posy >= buf->height || posx >= buf->width) {
>>           return 0;
>>       }
>> --
>> 2.31.1
>>