qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stip


From: Mark Cave-Ayland
Subject: Re: [PATCH v3] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter
Date: Sun, 25 Oct 2020 10:55:30 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1

On 24/10/2020 21:51, Philippe Mathieu-Daudé wrote:

The S24/TCX datasheet is listed as "Unable to locate" on [1].

However the NetBSD revision 1.32 of the driver introduced
64-bit accesses to the stippler and blitter [2]. It is safe
to assume these memory regions are 64-bit accessible.
QEMU implementation is 32-bit, so fill the 'impl' fields.

Michael Lorenz (author of the NetBSD code [2]) provided us with more
information in [3]:

IIRC the real hardware *requires* 64bit accesses for stipple and
blitter operations to work. For stipples you write a 64bit word into
STIP space, the address defines where in the framebuffer you want to
draw, the data contain a 32bit bitmask, foreground colour and a ROP.
BLIT space works similarly, the 64bit word contains an offset were to
read pixels from, and how many you want to copy.

One more thing since there seems to be some confusion - 64bit accesses
on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
even though its node says it is.
S24 is a card that plugs into a special slot on the SS5 mainboard,
which is shared with an SBus slot and looks a lot like a horizontal
UPA slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's
AFX bus which is 64bit wide and intended for graphics.
Early FFB docs even mentioned connecting to both AFX and UPA,
no idea if that was ever realized in hardware though.

[1] 
http://web.archive.org/web/20111209011516/http://wikis.sun.com/display/FOSSdocs/Home
[2] 
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/sbus/tcx.c.diff?r1=1.31&r2=1.32
[3] https://www.mail-archive.com/qemu-devel@nongnu.org/msg734928.html

Reported-by: Andreas Gustafsson <gson@gson.org>
Buglink: https://bugs.launchpad.net/bugs/1892540
Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Andreas Gustafsson <gson@gson.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Since v2:
- added Michael's memories
- added R-b/T-b tags

Since v1:
- added missing uncommitted staged changes... (tcx_blit_ops)
---
  hw/display/tcx.c | 18 +++++++++++++++---
  1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index c9d5e45cd1f..878ecc8c506 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -549,20 +549,28 @@ static const MemoryRegionOps tcx_stip_ops = {
      .read = tcx_stip_readl,
      .write = tcx_stip_writel,
      .endianness = DEVICE_NATIVE_ENDIAN,
-    .valid = {
+    .impl = {
          .min_access_size = 4,
          .max_access_size = 4,
      },
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 8,
+    },
  };
static const MemoryRegionOps tcx_rstip_ops = {
      .read = tcx_stip_readl,
      .write = tcx_rstip_writel,
      .endianness = DEVICE_NATIVE_ENDIAN,
-    .valid = {
+    .impl = {
          .min_access_size = 4,
          .max_access_size = 4,
      },
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 8,
+    },
  };
static uint64_t tcx_blit_readl(void *opaque, hwaddr addr,
@@ -651,10 +659,14 @@ static const MemoryRegionOps tcx_rblit_ops = {
      .read = tcx_blit_readl,
      .write = tcx_rblit_writel,
      .endianness = DEVICE_NATIVE_ENDIAN,
-    .valid = {
+    .impl = {
          .min_access_size = 4,
          .max_access_size = 4,
      },
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 8,
+    },
  };
static void tcx_invalidate_cursor_position(TCXState *s)

I'd already queued v2 of this patch (see my earlier email) with the intent to send a PR today, however I'll replace it with this v3 instead.


ATB,

Mark.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]