Re: [Linux-fbdev-users] [PATCH] cfbimgblt: Allow generic BitBLT functions to work with swapped pixel order in bytes.

[Pavel P. <pi...@cm...>]

Hello all,

I have found that I forgot to update prototypes for swapping bypass function.
I have corrected one case for copy-area function still not covered by previous
code. This is the case, where horizontal difference of source and destination
is not multiple of pixels in byte. This is relatively complicated case and I
am not sure, if it can appear during normal operation. I have not found
solution other, than to swap pixels in long after reads and before writes.
These changes should not lead to different operation for non swapped compilation,
but because more reorganization of code has been done I would submit this as
different patch.

Actual version of patches is there
  http://rtime.felk.cvut.cz/repos/ppisa-linux-devel/kernel-patches/current/cfb-pixels-in-byte-swapped.patch
  http://rtime.felk.cvut.cz/repos/ppisa-linux-devel/kernel-patches/current/cfb-copyarea-swapping.patch

The changes in original patch are correction of bypass functions

===============================================================================
+#else /* CONFIG_FB_CFB_REV_PIXELS_IN_BYTE */
+
+static inline unsigned long fb_rev_pixels_in_long(unsigned long val, u32 bswapmask)
+{
+	return val;
+}
+
+static inline u32 fb_shifted_pixels_mask_u32(u32 index, u32 bswapmask)
+{
+	return FB_SHIFT_HIGH(~(u32)0, index);
+}
+
+static inline unsigned long fb_shifted_pixels_mask_long(u32 index, u32 bswapmask)
+{
+	return FB_SHIFT_HIGH(~0UL, index);
+}
+
+static inline u32 fb_compute_bswapmask(struct fb_info *info)
+{
+	return 0;
+}
+
+#endif  /* CONFIG_FB_CFB_REV_PIXELS_IN_BYTE */
+
===============================================================================

and addition of new function for full pixels swapping (aghr)

===============================================================================
+
+#ifdef CONFIG_FB_CFB_REV_PIXELS_IN_BYTE
+
+#if BITS_PER_LONG == 64
+#define REV_PIXELS_MASK1 0x5555555555555555ul
+#define REV_PIXELS_MASK2 0x3333333333333333ul
+#define REV_PIXELS_MASK4 0x0f0f0f0f0f0f0f0ful
+#else
+#define REV_PIXELS_MASK1 0x55555555ul
+#define REV_PIXELS_MASK2 0x33333333ul
+#define REV_PIXELS_MASK4 0x0f0f0f0ful
+#endif
+
+static inline unsigned long fb_rev_pixels_in_long(unsigned long val, u32 bswapmask)
+{
+	if(bswapmask & 1)
+		val = comp(val >> 1, val << 1, REV_PIXELS_MASK1);
+	if(bswapmask & 2)
+		val = comp(val >> 2, val << 2, REV_PIXELS_MASK2);
+	if(bswapmask & 3)
+		val = comp(val >> 4, val << 4, REV_PIXELS_MASK4);
+}
===============================================================================

The additional changes in copy area are in the following patch

Best wishes

            Pavel

===============================================================================
Subject: Framebuffer copyarea function taught to fully support swapped pixel order in bytes
From: Pavel Pisa <pi...@cm...>

This correct case, when source and destination X coordinates difference is not
multiple of pixels in byte. This is probably rare case, but this case should be
supported for completeness.

Reorganization of FB_READL and FB_WRITEL calls results in code size decrease
for normal build without swapping support and size with support enabled
is reasonable too.

Signed-off-by: Pavel Pisa <pi...@cm...>

 drivers/video/cfbcopyarea.c |   84 ++++++++++++++++++++++++++++++--------------
 1 file changed, 59 insertions(+), 25 deletions(-)

Index: linux-2.6.23-git/drivers/video/cfbcopyarea.c
===================================================================
--- linux-2.6.23-git.orig/drivers/video/cfbcopyarea.c
+++ linux-2.6.23-git/drivers/video/cfbcopyarea.c
@@ -94,29 +94,34 @@ bitcpy(unsigned long __iomem *dst, int d
 				FB_WRITEL( comp( FB_READL(src), FB_READL(dst), last), dst);
 		}
 	} else {
+		// Different alignment for source and dest
 		unsigned long d0, d1;
 		int m;
-		// Different alignment for source and dest
 
 		right = shift & (bits - 1);
 		left = -shift & (bits - 1);
+		bswapmask &= shift;
 
 		if (dst_idx+n <= bits) {
 			// Single destination word
 			if (last)
 				first &= last;
+			d0 = FB_READL(src);
+			d0 = fb_rev_pixels_in_long(d0, bswapmask);
 			if (shift > 0) {
 				// Single source word
-				FB_WRITEL( comp( FB_READL(src) >> right, FB_READL(dst), first), dst);
+				d0 >>= right;
 			} else if (src_idx+n <= bits) {
 				// Single source word
-				FB_WRITEL( comp(FB_READL(src) << left, FB_READL(dst), first), dst);
+				d0 <<= left;;
 			} else {
 				// 2 source words
-				d0 = FB_READL(src++);
-				d1 = FB_READL(src);
-				FB_WRITEL( comp(d0<<left | d1>>right, FB_READL(dst), first), dst);
+				d1 = FB_READL(src + 1);
+				d1 = fb_rev_pixels_in_long(d1, bswapmask);
+				d0 = d0<<left | d1>>right;
 			}
+			d0 = fb_rev_pixels_in_long(d0, bswapmask);
+			FB_WRITEL( comp(d0, FB_READL(dst), first), dst);
 		} else {
 			// Multiple destination words
 			/** We must always remember the last value read, because in case
@@ -125,25 +130,31 @@ bitcpy(unsigned long __iomem *dst, int d
 			overlap with the current long from SRC. We store this value in
 			'd0'. */
 			d0 = FB_READL(src++);
+			d0 = fb_rev_pixels_in_long(d0, bswapmask);
 			// Leading bits
 			if (shift > 0) {
 				// Single source word
-				FB_WRITEL( comp(d0 >> right, FB_READL(dst), first), dst);
+				d1 = d0;
+				d0 >>= right;
 				dst++;
 				n -= bits - dst_idx;
 			} else {
 				// 2 source words
 				d1 = FB_READL(src++);
-				FB_WRITEL( comp(d0<<left | d1>>right, FB_READL(dst), first), dst);
-				d0 = d1;
+				d1 = fb_rev_pixels_in_long(d1, bswapmask);
+
+				d0 = d0<<left | d1>>right;
 				dst++;
 				n -= bits - dst_idx;
 			}
+			d0 = fb_rev_pixels_in_long(d0, bswapmask);
+			FB_WRITEL( comp(d0, FB_READL(dst), first), dst);
+			d0 = d1;
 
 			// Main chunk
 			m = n % bits;
 			n /= bits;
-			while (n >= 4) {
+			while ((n >= 4) && !bswapmask) {
 				d1 = FB_READL(src++);
 				FB_WRITEL(d0 << left | d1 >> right, dst++);
 				d0 = d1;
@@ -160,7 +171,10 @@ bitcpy(unsigned long __iomem *dst, int d
 			}
 			while (n--) {
 				d1 = FB_READL(src++);
-				FB_WRITEL(d0 << left | d1 >> right, dst++);
+				d1 = fb_rev_pixels_in_long(d1, bswapmask);
+				d0 = d0 << left | d1 >> right;
+				d0 = fb_rev_pixels_in_long(d0, bswapmask);
+				FB_WRITEL(d0, dst++);
 				d0 = d1;
 			}
 
@@ -168,12 +182,15 @@ bitcpy(unsigned long __iomem *dst, int d
 			if (last) {
 				if (m <= right) {
 					// Single source word
-					FB_WRITEL( comp(d0 << left, FB_READL(dst), last), dst);
+					d0 <<= left;
 				} else {
 					// 2 source words
 					d1 = FB_READL(src);
-					FB_WRITEL( comp(d0<<left | d1>>right, FB_READL(dst), last), dst);
+					d1 = fb_rev_pixels_in_long(d1, bswapmask);
+					d0 = d0<<left | d1>>right;
 				}
+				d0 = fb_rev_pixels_in_long(d0, bswapmask);
+				FB_WRITEL( comp(d0, FB_READL(dst), last), dst);
 			}
 		}
 	}
@@ -247,24 +264,32 @@ bitcpy_rev(unsigned long __iomem *dst, i
 		}
 	} else {
 		// Different alignment for source and dest
+		unsigned long d0, d1;
+		int m;
 
 		int const left = -shift & (bits-1);
 		int const right = shift & (bits-1);
+		bswapmask &= shift;
 
 		if ((unsigned long)dst_idx+1 >= n) {
 			// Single destination word
 			if (last)
 				first &= last;
+			d0 = FB_READL(src);
 			if (shift < 0) {
 				// Single source word
-				FB_WRITEL( comp( FB_READL(src)<<left, FB_READL(dst), first), dst);
+				d0 <<= left;
 			} else if (1+(unsigned long)src_idx >= n) {
 				// Single source word
-				FB_WRITEL( comp( FB_READL(src)>>right, FB_READL(dst), first), dst);
+				d0 >>= right;
 			} else {
 				// 2 source words
-				FB_WRITEL( comp( (FB_READL(src)>>right | FB_READL(src-1)<<left), FB_READL(dst), first), dst);
+				d1 = FB_READL(src - 1);
+				d1 = fb_rev_pixels_in_long(d1, bswapmask);
+				d0 = d0>>right | d1<<left;
 			}
+			d0 = fb_rev_pixels_in_long(d0, bswapmask);
+			FB_WRITEL( comp(d0, FB_READL(dst), first), dst);
 		} else {
 			// Multiple destination words
 			/** We must always remember the last value read, because in case
@@ -272,27 +297,30 @@ bitcpy_rev(unsigned long __iomem *dst, i
 			1bpp), we always collect one full long for DST and that might
 			overlap with the current long from SRC. We store this value in
 			'd0'. */
-			unsigned long d0, d1;
-			int m;
 
 			d0 = FB_READL(src--);
+			d0 = fb_rev_pixels_in_long(d0, bswapmask);
 			// Leading bits
 			if (shift < 0) {
 				// Single source word
-				FB_WRITEL( comp( (d0 << left), FB_READL(dst), first), dst);
+				d1 = d0;
+				d0 <<= left;
 			} else {
 				// 2 source words
 				d1 = FB_READL(src--);
-				FB_WRITEL( comp( (d0>>right | d1<<left), FB_READL(dst), first), dst);
-				d0 = d1;
+				d1 = fb_rev_pixels_in_long(d1, bswapmask);
+				d0 = d0>>right | d1<<left;
 			}
+			d0 = fb_rev_pixels_in_long(d0, bswapmask);
+			FB_WRITEL( comp(d0, FB_READL(dst), first), dst);
+			d0 = d1;
 			dst--;
 			n -= dst_idx+1;
 
 			// Main chunk
 			m = n % bits;
 			n /= bits;
-			while (n >= 4) {
+			while ((n >= 4) && !bswapmask) {
 				d1 = FB_READL(src--);
 				FB_WRITEL(d0 >> right | d1 << left, dst--);
 				d0 = d1;
@@ -309,7 +337,10 @@ bitcpy_rev(unsigned long __iomem *dst, i
 			}
 			while (n--) {
 				d1 = FB_READL(src--);
-				FB_WRITEL(d0 >> right | d1 << left, dst--);
+				d1 = fb_rev_pixels_in_long(d1, bswapmask);
+				d0 = d0 >> right | d1 << left;
+				d0 = fb_rev_pixels_in_long(d0, bswapmask);
+				FB_WRITEL(d0, dst--);
 				d0 = d1;
 			}
 
@@ -317,12 +348,15 @@ bitcpy_rev(unsigned long __iomem *dst, i
 			if (last) {
 				if (m <= left) {
 					// Single source word
-					FB_WRITEL( comp(d0 >> right, FB_READL(dst), last), dst);
+					d0 >>= right;
 				} else {
 					// 2 source words
 					d1 = FB_READL(src);
-					FB_WRITEL( comp(d0>>right | d1<<left, FB_READL(dst), last), dst);
+					d1 = fb_rev_pixels_in_long(d1, bswapmask);
+					d0 = d0>>right | d1<<left;
 				}
+				d0 = fb_rev_pixels_in_long(d0, bswapmask);
+				FB_WRITEL( comp(d0, FB_READL(dst), last), dst);
 			}
 		}
 	}