[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