| Message ID | 20220112210730.292775-6-deller@gmx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes and updates for hppa target | expand |
+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. > > 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 > >
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. >> 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 >>
Sven Schnelle <svens@stackframe.org> writes: > 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. I think my initial thought that the register 118000 is the buffer access mode for the color map is just wrong. I think it's setting the SRC & DST buffer access mode at the same time because on the Visualize FX cards we have similar registers: #define B2_FBC_BABoth 0x00920804 /* DBA & SBA (reads return DBA) (RW) */ #define B2_FBC_DBA 0x00920808 /* Destination Bitmap Access Register (RW) */ #define B2_FBC_SBA 0x0092080C /* Source Bitmap Access Register (RW) */ Looking at Artist, we have: CMAP_BM_ACCESS = 0x118000 DST_BM_ACCESS = 0x118004 SRC_BM_ACCESS = 0x118008 Given that artist and visualize fx are very similar when it comes to 2D acceleration, i think CMAP_BM_ACCESS is just changing both registers, completely unrelated to the color map. I tried changing the code, but of course that breaks a lot of things. Let me see whether i can make that work. /Sven
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; }
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. I haven't seen any negative side effects when running HP-UX. 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(-) -- 2.31.1