[PATCH] fb: fix fast-path detection

Daniel Kurtz djkurtz at chromium.org
Mon Mar 24 00:29:16 PDT 2014


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

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

Fix both of these by converting width from bits to pixels, 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 pixels
  (FbBits *)width => 699392 bytes

So, "careful" would be true if the destination line is withing 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>
---
 fb/fbblt.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/fb/fbblt.c b/fb/fbblt.c
index 72a05f6..804137c 100644
--- a/fb/fbblt.c
+++ b/fb/fbblt.c
@@ -40,13 +40,13 @@
 }
 
 void
-fbBlt(FbBits * srcLine,
-      FbStride srcStride,
-      int srcX,
-      FbBits * dstLine,
-      FbStride dstStride,
-      int dstX,
-      int width,
+fbBlt(FbBits * srcLine,    /* pixels */
+      FbStride srcStride,  /* pixels */
+      int srcX,            /* bits */
+      FbBits * dstLine,    /* pixels */
+      FbStride dstStride,  /* pixels */
+      int dstX,            /* bits */
+      int width,           /* bits */
       int height, int alu, FbBits pm, int bpp, Bool reverse, Bool upsidedown)
 {
     FbBits *src, *dst;
@@ -66,8 +66,9 @@ fbBlt(FbBits * srcLine,
         return;
     }
 
-    careful = !((srcLine < dstLine && srcLine + width * (bpp >> 3) > dstLine) ||
-                (dstLine < srcLine && dstLine + width * (bpp >> 3) > srcLine))
+    /* We must be careful if src and dst regions overlap */
+    careful = ((srcLine < dstLine && srcLine + (width / bpp) > dstLine) ||
+                (dstLine < srcLine && dstLine + (width / bpp) > srcLine))
         || (bpp & 7);
 
     if (alu == GXcopy && pm == FB_ALLONES && !careful &&
-- 
1.9.1.423.g4596e3a



More information about the xorg-devel mailing list