[PATCH v2] fb: fix fast-path detection

Daniel Kurtz djkurtz at chromium.org
Mon Mar 24 19:50:31 PDT 2014


The width parameter is used to disable the blit fast-path (memcpy) when
source and destination rows overlap in memory. This check was added in [0].

Unfortunately, the calculation to determine if source and destination
lines overlapped was incorrect:
  (1) it converts width from pixels to bytes, but width is actually in
      bits, not pixels.
  (2) it adds this byte offset to dst/srcLine, which implicitly converts
      the offset from bytes to sizeof(FbBits).

Fix both of these by converting width from bits to FbBits, and let the
pointer arithmetic convert to byte addresses.

For example:
A 32-bpp 1366 pixel-wide row will have
  width = 1366 * 32 = 43712 bits
  bpp = 32
  (bpp >> 3) = 4
  width * (bpp >> 3) = 174848 FbBits
  (FbBits *)width => 699392 bytes

So, "careful" was true if the destination line was within 699392 bytes,
instead of just within its 1366 * 4 = 5464 byte row.

This bug causes us to take the slow path for large non-overlapping rows
that are "close" in memory.  As a data point, XGetImage(1366x768) on my
ARM chromebook was taking ~140 ms, but with this fixed, it now takes
about 60 ms.
  XGetImage() -> exaGetImage() -> fbGetImage -> fbBlt()

[0] commit e32cc0b4c85c78cd8743a6e1680dcc79054b57ce
Author: Adam Jackson <ajax at redhat.com>
Date:   Thu Apr 21 16:37:11 2011 -0400

    fb: Fix memcpy abuse

    The memcpy fast path implicitly assumes that the copy walks
    left-to-right.  That's not something memcpy guarantees, and newer glibc
    on some processors will indeed break that assumption.  Since we walk a
    line at a time, check the source and destination against the width of
    the blit to determine whether we can be sloppy enough to allow memcpy.
    (Having done this, we can remove the check for !reverse as well.)


Signed-off-by: Daniel Kurtz <djkurtz at chromium.org>
---
v2: Convert width to FbBits using FB_SHIFT, per Keith Packard 

 fb/fbblt.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fb/fbblt.c b/fb/fbblt.c
index 72a05f6..f85409e 100644
--- a/fb/fbblt.c
+++ b/fb/fbblt.c
@@ -41,12 +41,12 @@
 
 void
 fbBlt(FbBits * srcLine,
-      FbStride srcStride,
-      int srcX,
+      FbStride srcStride,  /* FbBits */
+      int srcX,            /* bits */
       FbBits * dstLine,
-      FbStride dstStride,
-      int dstX,
-      int width,
+      FbStride dstStride,  /* FbBits */
+      int dstX,            /* bits */
+      int width,           /* bits */
       int height, int alu, FbBits pm, int bpp, Bool reverse, Bool upsidedown)
 {
     FbBits *src, *dst;
@@ -66,8 +66,8 @@ fbBlt(FbBits * srcLine,
         return;
     }
 
-    careful = !((srcLine < dstLine && srcLine + width * (bpp >> 3) > dstLine) ||
-                (dstLine < srcLine && dstLine + width * (bpp >> 3) > srcLine))
+    careful = ((srcLine < dstLine && &srcLine[width >> FB_SHIFT] > dstLine) ||
+               (dstLine < srcLine && &dstLine[width >> FB_SHIFT] > srcLine))
         || (bpp & 7);
 
     if (alu == GXcopy && pm == FB_ALLONES && !careful &&
-- 
1.9.1.423.g4596e3a



More information about the xorg-devel mailing list