Re: [PATCH 1/2] fb: add support for foreign endianness

Re: [PATCH 1/2] fb: add support for foreign endianness

[Andrew Morton]

On Tue, 5 Feb 2008 18:44:32 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> This patch adds support for the framebuffers with non-native
> endianness. This is done via FBINFO_FOREIGN_ENDIAN flag that will
> be used by the drivers. Depending on the host endianness this flag
> will be overwritten by FBINFO_BE_MATH internal flag, or cleared.
> 
> Tested to work on MPC8360E-RDK (BE) + Fujitsu MINT framebuffer (LE).

That's a pretty large patch to fbdev core, and Tony appears to have gone
offline again and you didn't cc the fbdev mailing list.

I fixed the third problem.  Could the other fbdev developers please review
and comment on this?

Thanks.



Actually...  should CONFIG_FB_FOREIGN_ENDIAN exist, or should this feature
be permanently enabled?

I'd like to at least queue a patch in -mm so that CONFIG_FB_FOREIGN_ENDIAN
is forced-on, so the code gets some runtime testing.  Will that break
anything?



From: Anton Vorontsov <avorontsov@ru.mvista.com>

Add support for the framebuffers with non-native endianness.  This is done via
FBINFO_FOREIGN_ENDIAN flag that will be used by the drivers.  Depending on the
host endianness this flag will be overwritten by FBINFO_BE_MATH internal flag,
or cleared.

Tested to work on MPC8360E-RDK (BE) + Fujitsu MINT framebuffer (LE).

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Cc: "Antonino A. Daplas" <adaplas@pol.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/video/Kconfig       |    9 +++++
 drivers/video/cfbcopyarea.c |   23 ++++++++-------
 drivers/video/cfbfillrect.c |   48 +++++++++++++++++--------------
 drivers/video/cfbimgblt.c   |   52 +++++++++++++++++-----------------
 drivers/video/fb_draw.h     |   31 +++++++++++---------
 drivers/video/fbmem.c       |    8 +++++
 drivers/video/syscopyarea.c |   20 ++++++-------
 drivers/video/sysfillrect.c |   49 ++++++++++++++++----------------
 drivers/video/sysimgblt.c   |   47 ++++++++++++++----------------
 include/linux/fb.h          |   36 +++++++++++++++++------
 10 files changed, 183 insertions(+), 140 deletions(-)

diff -puN drivers/video/Kconfig~fb-add-support-for-foreign-endianness drivers/video/Kconfig
--- a/drivers/video/Kconfig~fb-add-support-for-foreign-endianness
+++ a/drivers/video/Kconfig
@@ -139,6 +139,15 @@ config FB_SYS_IMAGEBLIT
 	  blitting. This is used by drivers that don't provide their own
 	  (accelerated) version and the framebuffer is in system RAM.
 
+config FB_FOREIGN_ENDIAN
+	bool "Enable support for foreign endianness"
+	depends on FB
+	---help---
+	  This option enables support for the framebuffers with non-native
+	  endianness (e.g. Little-Endian framebuffer on a Big-Endian machine).
+	  You probably don't have such hardware, so in most cases it's safe to
+	  say "n" here.
+
 config FB_SYS_FOPS
        tristate
        depends on FB
diff -puN drivers/video/cfbcopyarea.c~fb-add-support-for-foreign-endianness drivers/video/cfbcopyarea.c
--- a/drivers/video/cfbcopyarea.c~fb-add-support-for-foreign-endianness
+++ a/drivers/video/cfbcopyarea.c
@@ -44,15 +44,16 @@
      */
 
 static void
-bitcpy(unsigned long __iomem *dst, int dst_idx, const unsigned long __iomem *src,
-	int src_idx, int bits, unsigned n, u32 bswapmask)
+bitcpy(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
+		const unsigned long __iomem *src, int src_idx, int bits,
+		unsigned n, u32 bswapmask)
 {
 	unsigned long first, last;
 	int const shift = dst_idx-src_idx;
 	int left, right;
 
-	first = fb_shifted_pixels_mask_long(dst_idx, bswapmask);
-	last = ~fb_shifted_pixels_mask_long((dst_idx+n) % bits, bswapmask);
+	first = fb_shifted_pixels_mask_long(p, dst_idx, bswapmask);
+	last = ~fb_shifted_pixels_mask_long(p, (dst_idx+n) % bits, bswapmask);
 
 	if (!shift) {
 		// Same alignment for source and dest
@@ -202,8 +203,9 @@ bitcpy(unsigned long __iomem *dst, int d
      */
 
 static void
-bitcpy_rev(unsigned long __iomem *dst, int dst_idx, const unsigned long __iomem *src,
-		int src_idx, int bits, unsigned n, u32 bswapmask)
+bitcpy_rev(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
+		const unsigned long __iomem *src, int src_idx, int bits,
+		unsigned n, u32 bswapmask)
 {
 	unsigned long first, last;
 	int shift;
@@ -221,8 +223,9 @@ bitcpy_rev(unsigned long __iomem *dst, i
 
 	shift = dst_idx-src_idx;
 
-	first = fb_shifted_pixels_mask_long(bits - 1 - dst_idx, bswapmask);
-	last = ~fb_shifted_pixels_mask_long(bits - 1 - ((dst_idx-n) % bits), bswapmask);
+	first = fb_shifted_pixels_mask_long(p, bits - 1 - dst_idx, bswapmask);
+	last = ~fb_shifted_pixels_mask_long(p, bits - 1 - ((dst_idx-n) % bits),
+					    bswapmask);
 
 	if (!shift) {
 		// Same alignment for source and dest
@@ -404,7 +407,7 @@ void cfb_copyarea(struct fb_info *p, con
 			dst_idx &= (bytes - 1);
 			src += src_idx >> (ffs(bits) - 1);
 			src_idx &= (bytes - 1);
-			bitcpy_rev(dst, dst_idx, src, src_idx, bits,
+			bitcpy_rev(p, dst, dst_idx, src, src_idx, bits,
 				width*p->var.bits_per_pixel, bswapmask);
 		}
 	} else {
@@ -413,7 +416,7 @@ void cfb_copyarea(struct fb_info *p, con
 			dst_idx &= (bytes - 1);
 			src += src_idx >> (ffs(bits) - 1);
 			src_idx &= (bytes - 1);
-			bitcpy(dst, dst_idx, src, src_idx, bits,
+			bitcpy(p, dst, dst_idx, src, src_idx, bits,
 				width*p->var.bits_per_pixel, bswapmask);
 			dst_idx += bits_per_line;
 			src_idx += bits_per_line;
diff -puN drivers/video/cfbfillrect.c~fb-add-support-for-foreign-endianness drivers/video/cfbfillrect.c
--- a/drivers/video/cfbfillrect.c~fb-add-support-for-foreign-endianness
+++ a/drivers/video/cfbfillrect.c
@@ -36,16 +36,16 @@
      */
 
 static void
-bitfill_aligned(unsigned long __iomem *dst, int dst_idx, unsigned long pat,
-		unsigned n, int bits, u32 bswapmask)
+bitfill_aligned(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
+		unsigned long pat, unsigned n, int bits, u32 bswapmask)
 {
 	unsigned long first, last;
 
 	if (!n)
 		return;
 
-	first = fb_shifted_pixels_mask_long(dst_idx, bswapmask);
-	last = ~fb_shifted_pixels_mask_long((dst_idx+n) % bits, bswapmask);
+	first = fb_shifted_pixels_mask_long(p, dst_idx, bswapmask);
+	last = ~fb_shifted_pixels_mask_long(p, (dst_idx+n) % bits, bswapmask);
 
 	if (dst_idx+n <= bits) {
 		// Single word
@@ -93,16 +93,16 @@ bitfill_aligned(unsigned long __iomem *d
      */
 
 static void
-bitfill_unaligned(unsigned long __iomem *dst, int dst_idx, unsigned long pat,
-			int left, int right, unsigned n, int bits)
+bitfill_unaligned(struct fb_info *p, unsigned long __iomem *dst, int dst_idx,
+		  unsigned long pat, int left, int right, unsigned n, int bits)
 {
 	unsigned long first, last;
 
 	if (!n)
 		return;
 
-	first = FB_SHIFT_HIGH(~0UL, dst_idx);
-	last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits));
+	first = FB_SHIFT_HIGH(p, ~0UL, dst_idx);
+	last = ~(FB_SHIFT_HIGH(p, ~0UL, (dst_idx+n) % bits));
 
 	if (dst_idx+n <= bits) {
 		// Single word
@@ -147,8 +147,9 @@ bitfill_unaligned(unsigned long __iomem 
      *  Aligned pattern invert using 32/64-bit memory accesses
      */
 static void
-bitfill_aligned_rev(unsigned long __iomem *dst, int dst_idx, unsigned long pat,
-		unsigned n, int bits, u32 bswapmask)
+bitfill_aligned_rev(struct fb_info *p, unsigned long __iomem *dst,
+		    int dst_idx, unsigned long pat, unsigned n, int bits,
+		    u32 bswapmask)
 {
 	unsigned long val = pat, dat;
 	unsigned long first, last;
@@ -156,8 +157,8 @@ bitfill_aligned_rev(unsigned long __iome
 	if (!n)
 		return;
 
-	first = fb_shifted_pixels_mask_long(dst_idx, bswapmask);
-	last = ~fb_shifted_pixels_mask_long((dst_idx+n) % bits, bswapmask);
+	first = fb_shifted_pixels_mask_long(p, dst_idx, bswapmask);
+	last = ~fb_shifted_pixels_mask_long(p, (dst_idx+n) % bits, bswapmask);
 
 	if (dst_idx+n <= bits) {
 		// Single word
@@ -217,16 +218,17 @@ bitfill_aligned_rev(unsigned long __iome
      */
 
 static void
-bitfill_unaligned_rev(unsigned long __iomem *dst, int dst_idx, unsigned long pat,
-			int left, int right, unsigned n, int bits)
+bitfill_unaligned_rev(struct fb_info *p, unsigned long __iomem *dst,
+		      int dst_idx, unsigned long pat, int left, int right,
+		      unsigned n, int bits)
 {
 	unsigned long first, last, dat;
 
 	if (!n)
 		return;
 
-	first = FB_SHIFT_HIGH(~0UL, dst_idx);
-	last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits));
+	first = FB_SHIFT_HIGH(p, ~0UL, dst_idx);
+	last = ~(FB_SHIFT_HIGH(p, ~0UL, (dst_idx+n) % bits));
 
 	if (dst_idx+n <= bits) {
 		// Single word
@@ -306,7 +308,8 @@ void cfb_fillrect(struct fb_info *p, con
 		p->fbops->fb_sync(p);
 	if (!left) {
 		u32 bswapmask = fb_compute_bswapmask(p);
-		void (*fill_op32)(unsigned long __iomem *dst, int dst_idx,
+		void (*fill_op32)(struct fb_info *p,
+				  unsigned long __iomem *dst, int dst_idx,
 		                  unsigned long pat, unsigned n, int bits,
 				  u32 bswapmask) = NULL;
 
@@ -325,16 +328,17 @@ void cfb_fillrect(struct fb_info *p, con
 		while (height--) {
 			dst += dst_idx >> (ffs(bits) - 1);
 			dst_idx &= (bits - 1);
-			fill_op32(dst, dst_idx, pat, width*bpp, bits, bswapmask);
+			fill_op32(p, dst, dst_idx, pat, width*bpp, bits,
+				  bswapmask);
 			dst_idx += p->fix.line_length*8;
 		}
 	} else {
 		int right;
 		int r;
 		int rot = (left-dst_idx) % bpp;
-		void (*fill_op)(unsigned long __iomem *dst, int dst_idx,
-		                unsigned long pat, int left, int right,
-		                unsigned n, int bits) = NULL;
+		void (*fill_op)(struct fb_info *p, unsigned long __iomem *dst,
+				int dst_idx, unsigned long pat, int left,
+				int right, unsigned n, int bits) = NULL;
 
 		/* rotate pattern to correct start position */
 		pat = pat << rot | pat >> (bpp-rot);
@@ -355,7 +359,7 @@ void cfb_fillrect(struct fb_info *p, con
 		while (height--) {
 			dst += dst_idx >> (ffs(bits) - 1);
 			dst_idx &= (bits - 1);
-			fill_op(dst, dst_idx, pat, left, right,
+			fill_op(p, dst, dst_idx, pat, left, right,
 				width*bpp, bits);
 			r = (p->fix.line_length*8) % bpp;
 			pat = pat << (bpp-r) | pat >> r;
diff -puN drivers/video/cfbimgblt.c~fb-add-support-for-foreign-endianness drivers/video/cfbimgblt.c
--- a/drivers/video/cfbimgblt.c~fb-add-support-for-foreign-endianness
+++ a/drivers/video/cfbimgblt.c
@@ -43,30 +43,26 @@
 #define DPRINTK(fmt, args...)
 #endif
 
-static const u32 cfb_tab8[] = {
-#if defined(__BIG_ENDIAN)
+static const u32 cfb_tab8_be[] = {
     0x00000000,0x000000ff,0x0000ff00,0x0000ffff,
     0x00ff0000,0x00ff00ff,0x00ffff00,0x00ffffff,
     0xff000000,0xff0000ff,0xff00ff00,0xff00ffff,
     0xffff0000,0xffff00ff,0xffffff00,0xffffffff
-#elif defined(__LITTLE_ENDIAN)
+};
+
+static const u32 cfb_tab8_le[] = {
     0x00000000,0xff000000,0x00ff0000,0xffff0000,
     0x0000ff00,0xff00ff00,0x00ffff00,0xffffff00,
     0x000000ff,0xff0000ff,0x00ff00ff,0xffff00ff,
     0x0000ffff,0xff00ffff,0x00ffffff,0xffffffff
-#else
-#error FIXME: No endianness??
-#endif
 };
 
-static const u32 cfb_tab16[] = {
-#if defined(__BIG_ENDIAN)
+static const u32 cfb_tab16_be[] = {
     0x00000000, 0x0000ffff, 0xffff0000, 0xffffffff
-#elif defined(__LITTLE_ENDIAN)
+};
+
+static const u32 cfb_tab16_le[] = {
     0x00000000, 0xffff0000, 0x0000ffff, 0xffffffff
-#else
-#error FIXME: No endianness??
-#endif
 };
 
 static const u32 cfb_tab32[] = {
@@ -98,7 +94,8 @@ static inline void color_imageblit(const
 		val = 0;
 		
 		if (start_index) {
-			u32 start_mask = ~fb_shifted_pixels_mask_u32(start_index, bswapmask);
+			u32 start_mask = ~fb_shifted_pixels_mask_u32(p,
+						start_index, bswapmask);
 			val = FB_READL(dst) & start_mask;
 			shift = start_index;
 		}
@@ -108,20 +105,21 @@ static inline void color_imageblit(const
 				color = palette[*src];
 			else
 				color = *src;
-			color <<= FB_LEFT_POS(bpp);
-			val |= FB_SHIFT_HIGH(color, shift ^ bswapmask);
+			color <<= FB_LEFT_POS(p, bpp);
+			val |= FB_SHIFT_HIGH(p, color, shift ^ bswapmask);
 			if (shift >= null_bits) {
 				FB_WRITEL(val, dst++);
 	
 				val = (shift == null_bits) ? 0 : 
-					FB_SHIFT_LOW(color, 32 - shift);
+					FB_SHIFT_LOW(p, color, 32 - shift);
 			}
 			shift += bpp;
 			shift &= (32 - 1);
 			src++;
 		}
 		if (shift) {
-			u32 end_mask = fb_shifted_pixels_mask_u32(shift, bswapmask);
+			u32 end_mask = fb_shifted_pixels_mask_u32(p, shift,
+						bswapmask);
 
 			FB_WRITEL((FB_READL(dst) & end_mask) | val, dst);
 		}
@@ -152,8 +150,8 @@ static inline void slow_imageblit(const 
 	u32 bswapmask = fb_compute_bswapmask(p);
 
 	dst2 = (u32 __iomem *) dst1;
-	fgcolor <<= FB_LEFT_POS(bpp);
-	bgcolor <<= FB_LEFT_POS(bpp);
+	fgcolor <<= FB_LEFT_POS(p, bpp);
+	bgcolor <<= FB_LEFT_POS(p, bpp);
 
 	for (i = image->height; i--; ) {
 		shift = val = 0;
@@ -164,7 +162,8 @@ static inline void slow_imageblit(const 
 
 		/* write leading bits */
 		if (start_index) {
-			u32 start_mask = ~fb_shifted_pixels_mask_u32(start_index, bswapmask);
+			u32 start_mask = ~fb_shifted_pixels_mask_u32(p,
+						start_index, bswapmask);
 			val = FB_READL(dst) & start_mask;
 			shift = start_index;
 		}
@@ -172,13 +171,13 @@ static inline void slow_imageblit(const 
 		while (j--) {
 			l--;
 			color = (*s & (1 << l)) ? fgcolor : bgcolor;
-			val |= FB_SHIFT_HIGH(color, shift ^ bswapmask);
+			val |= FB_SHIFT_HIGH(p, color, shift ^ bswapmask);
 			
 			/* Did the bitshift spill bits to the next long? */
 			if (shift >= null_bits) {
 				FB_WRITEL(val, dst++);
 				val = (shift == null_bits) ? 0 :
-					FB_SHIFT_LOW(color,32 - shift);
+					FB_SHIFT_LOW(p, color, 32 - shift);
 			}
 			shift += bpp;
 			shift &= (32 - 1);
@@ -187,7 +186,8 @@ static inline void slow_imageblit(const 
 
 		/* write trailing bits */
  		if (shift) {
-			u32 end_mask = fb_shifted_pixels_mask_u32(shift, bswapmask);
+			u32 end_mask = fb_shifted_pixels_mask_u32(p, shift,
+						bswapmask);
 
 			FB_WRITEL((FB_READL(dst) & end_mask) | val, dst);
 		}
@@ -223,13 +223,13 @@ static inline void fast_imageblit(const 
 	u32 __iomem *dst;
 	const u32 *tab = NULL;
 	int i, j, k;
-		
+
 	switch (bpp) {
 	case 8:
-		tab = cfb_tab8;
+		tab = fb_be_math(p) ? cfb_tab8_be : cfb_tab8_le;
 		break;
 	case 16:
-		tab = cfb_tab16;
+		tab = fb_be_math(p) ? cfb_tab16_be : cfb_tab16_le;
 		break;
 	case 32:
 	default:
diff -puN drivers/video/fb_draw.h~fb-add-support-for-foreign-endianness drivers/video/fb_draw.h
--- a/drivers/video/fb_draw.h~fb-add-support-for-foreign-endianness
+++ a/drivers/video/fb_draw.h
@@ -94,41 +94,44 @@ static inline unsigned long fb_rev_pixel
 	return val;
 }
 
-static inline u32 fb_shifted_pixels_mask_u32(u32 index, u32 bswapmask)
+static inline u32 fb_shifted_pixels_mask_u32(struct fb_info *p, u32 index,
+					     u32 bswapmask)
 {
 	u32 mask;
 
 	if (!bswapmask) {
-		mask = FB_SHIFT_HIGH(~(u32)0, index);
+		mask = FB_SHIFT_HIGH(p, ~(u32)0, index);
 	} else {
-		mask = 0xff << FB_LEFT_POS(8);
-		mask = FB_SHIFT_LOW(mask, index & (bswapmask)) & mask;
-		mask = FB_SHIFT_HIGH(mask, index & ~(bswapmask));
+		mask = 0xff << FB_LEFT_POS(p, 8);
+		mask = FB_SHIFT_LOW(p, mask, index & (bswapmask)) & mask;
+		mask = FB_SHIFT_HIGH(p, mask, index & ~(bswapmask));
 #if defined(__i386__) || defined(__x86_64__)
 		/* Shift argument is limited to 0 - 31 on x86 based CPU's */
 		if(index + bswapmask < 32)
 #endif
-			mask |= FB_SHIFT_HIGH(~(u32)0,
+			mask |= FB_SHIFT_HIGH(p, ~(u32)0,
 					(index + bswapmask) & ~(bswapmask));
 	}
 	return mask;
 }
 
-static inline unsigned long fb_shifted_pixels_mask_long(u32 index, u32 bswapmask)
+static inline unsigned long fb_shifted_pixels_mask_long(struct fb_info *p,
+							u32 index,
+							u32 bswapmask)
 {
 	unsigned long mask;
 
 	if (!bswapmask) {
-		mask = FB_SHIFT_HIGH(~0UL, index);
+		mask = FB_SHIFT_HIGH(p, ~0UL, index);
 	} else {
-		mask = 0xff << FB_LEFT_POS(8);
-		mask = FB_SHIFT_LOW(mask, index & (bswapmask)) & mask;
-		mask = FB_SHIFT_HIGH(mask, index & ~(bswapmask));
+		mask = 0xff << FB_LEFT_POS(p, 8);
+		mask = FB_SHIFT_LOW(p, mask, index & (bswapmask)) & mask;
+		mask = FB_SHIFT_HIGH(p, mask, index & ~(bswapmask));
 #if defined(__i386__) || defined(__x86_64__)
 		/* Shift argument is limited to 0 - 31 on x86 based CPU's */
 		if(index + bswapmask < BITS_PER_LONG)
 #endif
-			mask |= FB_SHIFT_HIGH(~0UL,
+			mask |= FB_SHIFT_HIGH(p, ~0UL,
 					(index + bswapmask) & ~(bswapmask));
 	}
 	return mask;
@@ -158,8 +161,8 @@ static inline unsigned long fb_rev_pixel
 	return val;
 }
 
-#define fb_shifted_pixels_mask_u32(i, b) FB_SHIFT_HIGH(~(u32)0, (i))
-#define fb_shifted_pixels_mask_long(i, b) FB_SHIFT_HIGH(~0UL, (i))
+#define fb_shifted_pixels_mask_u32(p, i, b) FB_SHIFT_HIGH((p), ~(u32)0, (i))
+#define fb_shifted_pixels_mask_long(p, i, b) FB_SHIFT_HIGH((p), ~0UL, (i))
 #define fb_compute_bswapmask(...) 0
 
 #endif  /* CONFIG_FB_CFB_REV_PIXELS_IN_BYTE */
diff -puN drivers/video/fbmem.c~fb-add-support-for-foreign-endianness drivers/video/fbmem.c
--- a/drivers/video/fbmem.c~fb-add-support-for-foreign-endianness
+++ a/drivers/video/fbmem.c
@@ -1368,6 +1368,7 @@ register_framebuffer(struct fb_info *fb_
 	int i;
 	struct fb_event event;
 	struct fb_videomode mode;
+	const bool foreign_endian = fb_info->flags & FBINFO_FOREIGN_ENDIAN;
 
 	if (num_registered_fb == FB_MAX)
 		return -ENXIO;
@@ -1404,6 +1405,13 @@ register_framebuffer(struct fb_info *fb_
 	if (!fb_info->pixmap.blit_y)
 		fb_info->pixmap.blit_y = ~(u32)0;
 
+	fb_info->flags &= ~FBINFO_FOREIGN_ENDIAN;
+#ifdef __BIG_ENDIAN
+	fb_info->flags |= foreign_endian ? 0 : FBINFO_BE_MATH;
+#else
+	fb_info->flags |= foreign_endian ? FBINFO_BE_MATH : 0;
+#endif
+
 	if (!fb_info->modelist.prev || !fb_info->modelist.next)
 		INIT_LIST_HEAD(&fb_info->modelist);
 
diff -puN drivers/video/syscopyarea.c~fb-add-support-for-foreign-endianness drivers/video/syscopyarea.c
--- a/drivers/video/syscopyarea.c~fb-add-support-for-foreign-endianness
+++ a/drivers/video/syscopyarea.c
@@ -26,15 +26,15 @@
      */
 
 static void
-bitcpy(unsigned long *dst, int dst_idx, const unsigned long *src,
-	int src_idx, int bits, unsigned n)
+bitcpy(struct fb_info *p, unsigned long *dst, int dst_idx,
+		const unsigned long *src, int src_idx, int bits, unsigned n)
 {
 	unsigned long first, last;
 	int const shift = dst_idx-src_idx;
 	int left, right;
 
-	first = FB_SHIFT_HIGH(~0UL, dst_idx);
-	last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits));
+	first = FB_SHIFT_HIGH(p, ~0UL, dst_idx);
+	last = ~(FB_SHIFT_HIGH(p, ~0UL, (dst_idx+n) % bits));
 
 	if (!shift) {
 		/* Same alignment for source and dest */
@@ -167,8 +167,8 @@ bitcpy(unsigned long *dst, int dst_idx, 
      */
 
 static void
-bitcpy_rev(unsigned long *dst, int dst_idx, const unsigned long *src,
-	   int src_idx, int bits, unsigned n)
+bitcpy_rev(struct fb_info *p, unsigned long *dst, int dst_idx,
+		const unsigned long *src, int src_idx, int bits, unsigned n)
 {
 	unsigned long first, last;
 	int shift;
@@ -186,8 +186,8 @@ bitcpy_rev(unsigned long *dst, int dst_i
 
 	shift = dst_idx-src_idx;
 
-	first = FB_SHIFT_LOW(~0UL, bits - 1 - dst_idx);
-	last = ~(FB_SHIFT_LOW(~0UL, bits - 1 - ((dst_idx-n) % bits)));
+	first = FB_SHIFT_LOW(p, ~0UL, bits - 1 - dst_idx);
+	last = ~(FB_SHIFT_LOW(p, ~0UL, bits - 1 - ((dst_idx-n) % bits)));
 
 	if (!shift) {
 		/* Same alignment for source and dest */
@@ -353,7 +353,7 @@ void sys_copyarea(struct fb_info *p, con
 			dst_idx &= (bytes - 1);
 			src += src_idx >> (ffs(bits) - 1);
 			src_idx &= (bytes - 1);
-			bitcpy_rev(dst, dst_idx, src, src_idx, bits,
+			bitcpy_rev(p, dst, dst_idx, src, src_idx, bits,
 				width*p->var.bits_per_pixel);
 		}
 	} else {
@@ -362,7 +362,7 @@ void sys_copyarea(struct fb_info *p, con
 			dst_idx &= (bytes - 1);
 			src += src_idx >> (ffs(bits) - 1);
 			src_idx &= (bytes - 1);
-			bitcpy(dst, dst_idx, src, src_idx, bits,
+			bitcpy(p, dst, dst_idx, src, src_idx, bits,
 				width*p->var.bits_per_pixel);
 			dst_idx += bits_per_line;
 			src_idx += bits_per_line;
diff -puN drivers/video/sysfillrect.c~fb-add-support-for-foreign-endianness drivers/video/sysfillrect.c
--- a/drivers/video/sysfillrect.c~fb-add-support-for-foreign-endianness
+++ a/drivers/video/sysfillrect.c
@@ -22,16 +22,16 @@
      */
 
 static void
-bitfill_aligned(unsigned long *dst, int dst_idx, unsigned long pat,
-		unsigned n, int bits)
+bitfill_aligned(struct fb_info *p, unsigned long *dst, int dst_idx,
+		unsigned long pat, unsigned n, int bits)
 {
 	unsigned long first, last;
 
 	if (!n)
 		return;
 
-	first = FB_SHIFT_HIGH(~0UL, dst_idx);
-	last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits));
+	first = FB_SHIFT_HIGH(p, ~0UL, dst_idx);
+	last = ~(FB_SHIFT_HIGH(p, ~0UL, (dst_idx+n) % bits));
 
 	if (dst_idx+n <= bits) {
 		/* Single word */
@@ -78,16 +78,16 @@ bitfill_aligned(unsigned long *dst, int 
      */
 
 static void
-bitfill_unaligned(unsigned long *dst, int dst_idx, unsigned long pat,
-		  int left, int right, unsigned n, int bits)
+bitfill_unaligned(struct fb_info *p, unsigned long *dst, int dst_idx,
+		  unsigned long pat, int left, int right, unsigned n, int bits)
 {
 	unsigned long first, last;
 
 	if (!n)
 		return;
 
-	first = FB_SHIFT_HIGH(~0UL, dst_idx);
-	last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits));
+	first = FB_SHIFT_HIGH(p, ~0UL, dst_idx);
+	last = ~(FB_SHIFT_HIGH(p, ~0UL, (dst_idx+n) % bits));
 
 	if (dst_idx+n <= bits) {
 		/* Single word */
@@ -132,8 +132,8 @@ bitfill_unaligned(unsigned long *dst, in
      *  Aligned pattern invert using 32/64-bit memory accesses
      */
 static void
-bitfill_aligned_rev(unsigned long *dst, int dst_idx, unsigned long pat,
-		    unsigned n, int bits)
+bitfill_aligned_rev(struct fb_info *p, unsigned long *dst, int dst_idx,
+		    unsigned long pat, unsigned n, int bits)
 {
 	unsigned long val = pat;
 	unsigned long first, last;
@@ -141,8 +141,8 @@ bitfill_aligned_rev(unsigned long *dst, 
 	if (!n)
 		return;
 
-	first = FB_SHIFT_HIGH(~0UL, dst_idx);
-	last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits));
+	first = FB_SHIFT_HIGH(p, ~0UL, dst_idx);
+	last = ~(FB_SHIFT_HIGH(p, ~0UL, (dst_idx+n) % bits));
 
 	if (dst_idx+n <= bits) {
 		/* Single word */
@@ -188,16 +188,17 @@ bitfill_aligned_rev(unsigned long *dst, 
      */
 
 static void
-bitfill_unaligned_rev(unsigned long *dst, int dst_idx, unsigned long pat,
-			int left, int right, unsigned n, int bits)
+bitfill_unaligned_rev(struct fb_info *p, unsigned long *dst, int dst_idx,
+		      unsigned long pat, int left, int right, unsigned n,
+		      int bits)
 {
 	unsigned long first, last;
 
 	if (!n)
 		return;
 
-	first = FB_SHIFT_HIGH(~0UL, dst_idx);
-	last = ~(FB_SHIFT_HIGH(~0UL, (dst_idx+n) % bits));
+	first = FB_SHIFT_HIGH(p, ~0UL, dst_idx);
+	last = ~(FB_SHIFT_HIGH(p, ~0UL, (dst_idx+n) % bits));
 
 	if (dst_idx+n <= bits) {
 		/* Single word */
@@ -267,9 +268,9 @@ void sys_fillrect(struct fb_info *p, con
 	if (p->fbops->fb_sync)
 		p->fbops->fb_sync(p);
 	if (!left) {
-		void (*fill_op32)(unsigned long *dst, int dst_idx,
-		                  unsigned long pat, unsigned n, int bits) =
-			NULL;
+		void (*fill_op32)(struct fb_info *p, unsigned long *dst,
+				  int dst_idx, unsigned long pat, unsigned n,
+				  int bits) = NULL;
 
 		switch (rect->rop) {
 		case ROP_XOR:
@@ -287,16 +288,16 @@ void sys_fillrect(struct fb_info *p, con
 		while (height--) {
 			dst += dst_idx >> (ffs(bits) - 1);
 			dst_idx &= (bits - 1);
-			fill_op32(dst, dst_idx, pat, width*bpp, bits);
+			fill_op32(p, dst, dst_idx, pat, width*bpp, bits);
 			dst_idx += p->fix.line_length*8;
 		}
 	} else {
 		int right;
 		int r;
 		int rot = (left-dst_idx) % bpp;
-		void (*fill_op)(unsigned long *dst, int dst_idx,
-		                unsigned long pat, int left, int right,
-		                unsigned n, int bits) = NULL;
+		void (*fill_op)(struct fb_info *p, unsigned long *dst,
+				int dst_idx, unsigned long pat, int left,
+				int right, unsigned n, int bits) = NULL;
 
 		/* rotate pattern to correct start position */
 		pat = pat << rot | pat >> (bpp-rot);
@@ -318,7 +319,7 @@ void sys_fillrect(struct fb_info *p, con
 		while (height--) {
 			dst += dst_idx >> (ffs(bits) - 1);
 			dst_idx &= (bits - 1);
-			fill_op(dst, dst_idx, pat, left, right,
+			fill_op(p, dst, dst_idx, pat, left, right,
 				width*bpp, bits);
 			r = (p->fix.line_length*8) % bpp;
 			pat = pat << (bpp-r) | pat >> r;
diff -puN drivers/video/sysimgblt.c~fb-add-support-for-foreign-endianness drivers/video/sysimgblt.c
--- a/drivers/video/sysimgblt.c~fb-add-support-for-foreign-endianness
+++ a/drivers/video/sysimgblt.c
@@ -23,30 +23,26 @@
 #define DPRINTK(fmt, args...)
 #endif
 
-static const u32 cfb_tab8[] = {
-#if defined(__BIG_ENDIAN)
+static const u32 cfb_tab8_be[] = {
     0x00000000,0x000000ff,0x0000ff00,0x0000ffff,
     0x00ff0000,0x00ff00ff,0x00ffff00,0x00ffffff,
     0xff000000,0xff0000ff,0xff00ff00,0xff00ffff,
     0xffff0000,0xffff00ff,0xffffff00,0xffffffff
-#elif defined(__LITTLE_ENDIAN)
+};
+
+static const u32 cfb_tab8_le[] = {
     0x00000000,0xff000000,0x00ff0000,0xffff0000,
     0x0000ff00,0xff00ff00,0x00ffff00,0xffffff00,
     0x000000ff,0xff0000ff,0x00ff00ff,0xffff00ff,
     0x0000ffff,0xff00ffff,0x00ffffff,0xffffffff
-#else
-#error FIXME: No endianness??
-#endif
 };
 
-static const u32 cfb_tab16[] = {
-#if defined(__BIG_ENDIAN)
+static const u32 cfb_tab16_be[] = {
     0x00000000, 0x0000ffff, 0xffff0000, 0xffffffff
-#elif defined(__LITTLE_ENDIAN)
+};
+
+static const u32 cfb_tab16_le[] = {
     0x00000000, 0xffff0000, 0x0000ffff, 0xffffffff
-#else
-#error FIXME: No endianness??
-#endif
 };
 
 static const u32 cfb_tab32[] = {
@@ -72,7 +68,7 @@ static void color_imageblit(const struct
 		val = 0;
 
 		if (start_index) {
-			u32 start_mask = ~(FB_SHIFT_HIGH(~(u32)0,
+			u32 start_mask = ~(FB_SHIFT_HIGH(p, ~(u32)0,
 							 start_index));
 			val = *dst & start_mask;
 			shift = start_index;
@@ -83,20 +79,20 @@ static void color_imageblit(const struct
 				color = palette[*src];
 			else
 				color = *src;
-			color <<= FB_LEFT_POS(bpp);
-			val |= FB_SHIFT_HIGH(color, shift);
+			color <<= FB_LEFT_POS(p, bpp);
+			val |= FB_SHIFT_HIGH(p, color, shift);
 			if (shift >= null_bits) {
 				*dst++ = val;
 
 				val = (shift == null_bits) ? 0 :
-					FB_SHIFT_LOW(color, 32 - shift);
+					FB_SHIFT_LOW(p, color, 32 - shift);
 			}
 			shift += bpp;
 			shift &= (32 - 1);
 			src++;
 		}
 		if (shift) {
-			u32 end_mask = FB_SHIFT_HIGH(~(u32)0, shift);
+			u32 end_mask = FB_SHIFT_HIGH(p, ~(u32)0, shift);
 
 			*dst &= end_mask;
 			*dst |= val;
@@ -125,8 +121,8 @@ static void slow_imageblit(const struct 
 	u32 i, j, l;
 
 	dst2 = dst1;
-	fgcolor <<= FB_LEFT_POS(bpp);
-	bgcolor <<= FB_LEFT_POS(bpp);
+	fgcolor <<= FB_LEFT_POS(p, bpp);
+	bgcolor <<= FB_LEFT_POS(p, bpp);
 
 	for (i = image->height; i--; ) {
 		shift = val = 0;
@@ -137,7 +133,8 @@ static void slow_imageblit(const struct 
 
 		/* write leading bits */
 		if (start_index) {
-			u32 start_mask = ~(FB_SHIFT_HIGH(~(u32)0,start_index));
+			u32 start_mask = ~(FB_SHIFT_HIGH(p, ~(u32)0,
+							 start_index));
 			val = *dst & start_mask;
 			shift = start_index;
 		}
@@ -145,13 +142,13 @@ static void slow_imageblit(const struct 
 		while (j--) {
 			l--;
 			color = (*s & (1 << l)) ? fgcolor : bgcolor;
-			val |= FB_SHIFT_HIGH(color, shift);
+			val |= FB_SHIFT_HIGH(p, color, shift);
 
 			/* Did the bitshift spill bits to the next long? */
 			if (shift >= null_bits) {
 				*dst++ = val;
 				val = (shift == null_bits) ? 0 :
-					FB_SHIFT_LOW(color,32 - shift);
+					FB_SHIFT_LOW(p, color, 32 - shift);
 			}
 			shift += bpp;
 			shift &= (32 - 1);
@@ -160,7 +157,7 @@ static void slow_imageblit(const struct 
 
 		/* write trailing bits */
  		if (shift) {
-			u32 end_mask = FB_SHIFT_HIGH(~(u32)0, shift);
+			u32 end_mask = FB_SHIFT_HIGH(p, ~(u32)0, shift);
 
 			*dst &= end_mask;
 			*dst |= val;
@@ -199,10 +196,10 @@ static void fast_imageblit(const struct 
 
 	switch (bpp) {
 	case 8:
-		tab = cfb_tab8;
+		tab = fb_be_math(p) ? cfb_tab8_be : cfb_tab8_le;
 		break;
 	case 16:
-		tab = cfb_tab16;
+		tab = fb_be_math(p) ? cfb_tab16_be : cfb_tab16_le;
 		break;
 	case 32:
 	default:
diff -puN include/linux/fb.h~fb-add-support-for-foreign-endianness include/linux/fb.h
--- a/include/linux/fb.h~fb-add-support-for-foreign-endianness
+++ a/include/linux/fb.h
@@ -791,6 +791,17 @@ struct fb_tile_ops {
  */
 #define FBINFO_MISC_ALWAYS_SETPAR   0x40000
 
+/*
+ * Host and GPU endianness differ.
+ */
+#define FBINFO_FOREIGN_ENDIAN	0x100000
+/*
+ * Big endian math. This is the same flags as above, but with different
+ * meaning, it is set by the fb subsystem depending FOREIGN_ENDIAN flag
+ * and host endianness. Drivers should not use this flag.
+ */
+#define FBINFO_BE_MATH  0x100000
+
 struct fb_info {
 	int node;
 	int flags;
@@ -899,15 +910,11 @@ struct fb_info {
 
 #endif
 
-#if defined (__BIG_ENDIAN)
-#define FB_LEFT_POS(bpp)          (32 - bpp)
-#define FB_SHIFT_HIGH(val, bits)  ((val) >> (bits))
-#define FB_SHIFT_LOW(val, bits)   ((val) << (bits))
-#else
-#define FB_LEFT_POS(bpp)          (0)
-#define FB_SHIFT_HIGH(val, bits)  ((val) << (bits))
-#define FB_SHIFT_LOW(val, bits)   ((val) >> (bits))
-#endif
+#define FB_LEFT_POS(p, bpp)          (fb_be_math(p) ? (32 - (bpp)) : 0)
+#define FB_SHIFT_HIGH(p, val, bits)  (fb_be_math(p) ? (val) >> (bits) : \
+						      (val) << (bits))
+#define FB_SHIFT_LOW(p, val, bits)   (fb_be_math(p) ? (val) << (bits) : \
+						      (val) >> (bits))
 
     /*
      *  `Generic' versions of the frame buffer device operations
@@ -970,6 +977,17 @@ extern void fb_deferred_io_cleanup(struc
 extern int fb_deferred_io_fsync(struct file *file, struct dentry *dentry,
 				int datasync);
 
+static inline bool fb_be_math(struct fb_info *info)
+{
+#if defined(CONFIG_FB_FOREIGN_ENDIAN)
+	return info->flags & FBINFO_BE_MATH;
+#elif defined(__BIG_ENDIAN)
+	return true;
+#else
+	return false;
+#endif
+}
+
 /* drivers/video/fbsysfs.c */
 extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
 extern void framebuffer_release(struct fb_info *info);
_


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/2] fb: add support for foreign endianness

[Anton Vorontsov]

On Thu, Feb 14, 2008 at 10:49:42PM -0800, Andrew Morton wrote:
> On Tue, 5 Feb 2008 18:44:32 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> 
> > This patch adds support for the framebuffers with non-native
> > endianness. This is done via FBINFO_FOREIGN_ENDIAN flag that will
> > be used by the drivers. Depending on the host endianness this flag
> > will be overwritten by FBINFO_BE_MATH internal flag, or cleared.
> > 
> > Tested to work on MPC8360E-RDK (BE) + Fujitsu MINT framebuffer (LE).
> 
> That's a pretty large patch to fbdev core,

Yes, but changes are mostly trivial. No insurance of typos and thinkos
though. OTOH, it survived my tests.

> and Tony appears to have gone
> offline again and you didn't cc the fbdev mailing list.

FRAMEBUFFER LAYER
P:      Antonino Daplas
M:      adaplas@gmail.com
L:      linux-fbdev-devel@lists.sourceforge.net (subscribers-only)

I'm lazy to subscribe on occasional patches. Yup, this is hardly
an excuse...

> I fixed the third problem.  Could the other fbdev developers please review
> and comment on this?
> 
> Thanks.
> 
> 
> 
> Actually...  should CONFIG_FB_FOREIGN_ENDIAN exist, or should this feature
> be permanently enabled?

It should not. Because with CONFIG_FB_FOREIGN_ENDIAN enabled, drawing
might be a little bit slower, because of external checks for endianness
(I didn't noticed any slowdown, but I guess it's still measurable if we
find some fb benchmark).

Thinking about it a little bit more, I believe we want to add a choice,
which exactly foreign endianness we want to compile-in. Then, if we're
pretty confident that particular BE machine uses only LE framebuffer,
gcc can optimize away endianness checks.

Patch on top of previous inlined here. With that patch, slowdown is
possible with CONFIG_FB_BOTH_ENDIAN option only, which is still
default if FOREIGN_ENDIAN is selected.

> I'd like to at least queue a patch in -mm so that CONFIG_FB_FOREIGN_ENDIAN
> is forced-on, so the code gets some runtime testing.  Will that break
> anything?

Nope, it should not break anything, just possible fbconsole/tux drawing
slowdown.


Thanks,

- - - -
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Subject: fb: add support for choice foreign endianness

With this patch FB_FOREIGN_ENDIAN converted to menuconfig with the
choice inside: which type of foreign endianness we want to compile-in.

As a bonus, now fb subsystem will refuse to register framebuffer with
unsupported endianness, and will suggest a way to solve the problem.

[ on top of fb-add-support-for-foreign-endianness.patch ]

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/video/Kconfig |   27 +++++++++++++++++++++------
 drivers/video/fbmem.c |   38 ++++++++++++++++++++++++++++++--------
 include/linux/fb.h    |   14 +++++++++++---
 3 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 6b3940d..9e0b6ea 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -139,14 +139,29 @@ config FB_SYS_IMAGEBLIT
 	  blitting. This is used by drivers that don't provide their own
 	  (accelerated) version and the framebuffer is in system RAM.
 
-config FB_FOREIGN_ENDIAN
-	bool "Enable support for foreign endianness"
+menuconfig FB_FOREIGN_ENDIAN
+	bool "Framebuffer foreign endianness support"
 	depends on FB
 	---help---
-	  This option enables support for the framebuffers with non-native
-	  endianness (e.g. Little-Endian framebuffer on a Big-Endian machine).
-	  You probably don't have such hardware, so in most cases it's safe to
-	  say "n" here.
+	  This menu will let you enable support for the framebuffers with
+	  non-native endianness (e.g. Little-Endian framebuffer on a
+	  Big-Endian machine). Most probably you don't have such hardware,
+	  so it's safe to say "n" here.
+
+choice
+	prompt "Choice endianness support"
+	depends on FB_FOREIGN_ENDIAN
+
+config FB_BOTH_ENDIAN
+	bool "Support for Big- and Little-Endian framebuffers"
+
+config FB_BIG_ENDIAN
+	bool "Support for Big-Endian framebuffers only"
+
+config FB_LITTLE_ENDIAN
+	bool "Support for Little-Endian framebuffers only"
+
+endchoice
 
 config FB_SYS_FOPS
        tristate
diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index c086004..11d92fd 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1352,6 +1352,32 @@ static const struct file_operations fb_fops = {
 
 struct class *fb_class;
 EXPORT_SYMBOL(fb_class);
+
+static int fb_check_foreignness(struct fb_info *fi)
+{
+	const bool foreign_endian = fi->flags & FBINFO_FOREIGN_ENDIAN;
+
+	fi->flags &= ~FBINFO_FOREIGN_ENDIAN;
+
+#ifdef __BIG_ENDIAN
+	fi->flags |= foreign_endian ? 0 : FBINFO_BE_MATH;
+#else
+	fi->flags |= foreign_endian ? FBINFO_BE_MATH : 0;
+#endif /* __BIG_ENDIAN */
+
+	if (fi->flags & FBINFO_BE_MATH && !fb_be_math(fi)) {
+		pr_err("%s: enable CONFIG_FB_BIG_ENDIAN to "
+		       "support this framebuffer\n", fi->fix.id);
+		return -ENOSYS;
+	} else if (!(fi->flags & FBINFO_BE_MATH) && fb_be_math(fi)) {
+		pr_err("%s: enable CONFIG_FB_LITTLE_ENDIAN to "
+		       "support this framebuffer\n", fi->fix.id);
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
 /**
  *	register_framebuffer - registers a frame buffer device
  *	@fb_info: frame buffer info structure
@@ -1368,10 +1394,13 @@ register_framebuffer(struct fb_info *fb_info)
 	int i;
 	struct fb_event event;
 	struct fb_videomode mode;
-	const bool foreign_endian = fb_info->flags & FBINFO_FOREIGN_ENDIAN;
 
 	if (num_registered_fb == FB_MAX)
 		return -ENXIO;
+
+	if (fb_check_foreignness(fb_info))
+		return -ENOSYS;
+
 	num_registered_fb++;
 	for (i = 0 ; i < FB_MAX; i++)
 		if (!registered_fb[i])
@@ -1405,13 +1434,6 @@ register_framebuffer(struct fb_info *fb_info)
 	if (!fb_info->pixmap.blit_y)
 		fb_info->pixmap.blit_y = ~(u32)0;
 
-	fb_info->flags &= ~FBINFO_FOREIGN_ENDIAN;
-#ifdef __BIG_ENDIAN
-	fb_info->flags |= foreign_endian ? 0 : FBINFO_BE_MATH;
-#else
-	fb_info->flags |= foreign_endian ? FBINFO_BE_MATH : 0;
-#endif
-
 	if (!fb_info->modelist.prev || !fb_info->modelist.next)
 		INIT_LIST_HEAD(&fb_info->modelist);
 
diff --git a/include/linux/fb.h b/include/linux/fb.h
index a067ed3..72295b0 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -979,13 +979,21 @@ extern int fb_deferred_io_fsync(struct file *file, struct dentry *dentry,
 
 static inline bool fb_be_math(struct fb_info *info)
 {
-#if defined(CONFIG_FB_FOREIGN_ENDIAN)
+#ifdef CONFIG_FB_FOREIGN_ENDIAN
+#if defined(CONFIG_FB_BOTH_ENDIAN)
 	return info->flags & FBINFO_BE_MATH;
-#elif defined(__BIG_ENDIAN)
+#elif defined(CONFIG_FB_BIG_ENDIAN)
+	return true;
+#elif defined(CONFIG_FB_LITTLE_ENDIAN)
+	return false;
+#endif /* CONFIG_FB_BOTH_ENDIAN */
+#else
+#ifdef __BIG_ENDIAN
 	return true;
 #else
 	return false;
-#endif
+#endif /* __BIG_ENDIAN */
+#endif /* CONFIG_FB_FOREIGN_ENDIAN */
 }
 
 /* drivers/video/fbsysfs.c */
-- 
1.5.2.2

Re: [PATCH 1/2] fb: add support for foreign endianness

[Geert Uytterhoeven]

On Fri, 15 Feb 2008, Anton Vorontsov wrote:
> On Thu, Feb 14, 2008 at 10:49:42PM -0800, Andrew Morton wrote:
> > On Tue, 5 Feb 2008 18:44:32 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> > > This patch adds support for the framebuffers with non-native
> > > endianness. This is done via FBINFO_FOREIGN_ENDIAN flag that will
> > > be used by the drivers. Depending on the host endianness this flag
> > > will be overwritten by FBINFO_BE_MATH internal flag, or cleared.
> > > 
> > > Tested to work on MPC8360E-RDK (BE) + Fujitsu MINT framebuffer (LE).
> > 
> > That's a pretty large patch to fbdev core,
> 
> Yes, but changes are mostly trivial. No insurance of typos and thinkos
> though. OTOH, it survived my tests.
> 
> > and Tony appears to have gone
> > offline again and you didn't cc the fbdev mailing list.
> 
> FRAMEBUFFER LAYER
> P:      Antonino Daplas
> M:      adaplas@gmail.com
> L:      linux-fbdev-devel@lists.sourceforge.net (subscribers-only)
> 
> I'm lazy to subscribe on occasional patches. Yup, this is hardly
> an excuse...

While linux-fbdev is subscribers-only, non-subscribers are not plainly
rejected, but moderated, so the casual patch/comment/question comes
through.

I'll cook up a patch for MAINTAINERS to clarify.

> > Actually...  should CONFIG_FB_FOREIGN_ENDIAN exist, or should this feature
> > be permanently enabled?
> 
> It should not. Because with CONFIG_FB_FOREIGN_ENDIAN enabled, drawing
> might be a little bit slower, because of external checks for endianness
> (I didn't noticed any slowdown, but I guess it's still measurable if we
> find some fb benchmark).

> > I'd like to at least queue a patch in -mm so that CONFIG_FB_FOREIGN_ENDIAN
> > is forced-on, so the code gets some runtime testing.  Will that break
> > anything?
> 
> Nope, it should not break anything, just possible fbconsole/tux drawing
> slowdown.

Tux drawing speed doesn't really matter, as it's done once only.
Console text drawing is different.

`time cat MAINTAINERS' is a good test. Of course, is most time is spent
in scrolling, you won't see much difference.

So probably we need a test file that contains lines of text interspersed
with `HOME' escape sequences, so we don't scroll?

> Thinking about it a little bit more, I believe we want to add a choice,
> which exactly foreign endianness we want to compile-in. Then, if we're
> pretty confident that particular BE machine uses only LE framebuffer,
> gcc can optimize away endianness checks.
> 
> Patch on top of previous inlined here. With that patch, slowdown is
> possible with CONFIG_FB_BOTH_ENDIAN option only, which is still
> default if FOREIGN_ENDIAN is selected.
> With this patch FB_FOREIGN_ENDIAN converted to menuconfig with the
> choice inside: which type of foreign endianness we want to compile-in.
> 

The notion of `FOREIGN_ENDIAN' is relative, as it depends on the
architecture you're compiling for.

Suppose you have a PCI graphics card with a frame buffer that's always
big endian. When compiling for a big endian platform, the driver won't
depend on FB_FOREIGN_ENDIAN. When compiling for a little endian
platform, it will.

Shouldn't we add LITTLE_ENDIAN and BIG_ENDIAN Kconfig vars first, just
like we have 64BIT?

> As a bonus, now fb subsystem will refuse to register framebuffer with
> unsupported endianness, and will suggest a way to solve the problem.

I'd like to handle this in Kconfig (cfr. above).

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/2] fb: add support for foreign endianness

[Krzysztof Helt]

On Sun, 17 Feb 2008 10:44:32 +0100 (CET)
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> On Fri, 15 Feb 2008, Anton Vorontsov wrote:
> > On Thu, Feb 14, 2008 at 10:49:42PM -0800, Andrew Morton wrote:
> > > On Tue, 5 Feb 2008 18:44:32 +0300 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> > > Actually...  should CONFIG_FB_FOREIGN_ENDIAN exist, or should this feature
> > > be permanently enabled?
> > 
(...)
> 
> The notion of `FOREIGN_ENDIAN' is relative, as it depends on the
> architecture you're compiling for.
> 
> Suppose you have a PCI graphics card with a frame buffer that's always
> big endian. When compiling for a big endian platform, the driver won't
> depend on FB_FOREIGN_ENDIAN. When compiling for a little endian
> platform, it will.
> 
> Shouldn't we add LITTLE_ENDIAN and BIG_ENDIAN Kconfig vars first, just
> like we have 64BIT?
> 

I disagree here. The FOREIGN_ENDIAN is enough. It is determined only by
graphics chip endianess and CPU (arch) endianess.
I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
Both resolve endianess at driver level. Actually, both handle it by setting special
bits so the graphics chip itself reorder bytes to transform foreign endianess. 
I understand that this patch is for chips which cannot reorder bytes by themselves.

So the FOREIGN_ENDIANESS flag should be set by the driver if it is needed
(if the graphics chip is BE and CPU is LE a simple #ifdef will add the flag).

> 
> I'd like to handle this in Kconfig (cfr. above).
> 

Again, it is possible. It is enough to put one rule which enables
the FOREIGN_ENDIAN if the architecture endianess is "foreign"
for the driver. The advantage here is that it  can be set only
for drivers which need it (as some driver can handle it without
this code). It should be hidden option set only internally if needed
(no user selectable).

I tested this patch on the s3c2410fb with disabled byte order
corrections by the graphics chip itself. It worked for 8-bit depth
but not for 16-bit depth (pixel position seemed ok but wrong tux'
colors). I will investigate. The s3c2410fb is BE and the kernel
was arm LE.

I would like to extend this patch to fb depths below 8-bit. The
s3c2410fb cannot handle this correctly with LE kernel.

Kind regards,
Krzysztof

----------------------------------------------------------------------
Masz ostatnia szanse !
Sprawdz >>> http://link.interia.pl/f1d02  


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH 1/2] fb: add support for foreign endianness

[Valdis.Kletnieks]

[-- Attachment #1.1: Type: text/plain, Size: 522 bytes --]

On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said:
> I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
> Both resolve endianess at driver level. Actually, both handle it by setting special
> bits so the graphics chip itself reorder bytes to transform foreign endianess. 
> I understand that this patch is for chips which cannot reorder bytes by themselves.

Does anybody know of such a chip that's actually available in the wild?  Or are
we writing drivers for speculative possible chips?

[-- Attachment #1.2: Type: application/pgp-signature, Size: 226 bytes --]

[-- Attachment #2: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[-- Attachment #3: Type: text/plain, Size: 182 bytes --]

_______________________________________________
Linux-fbdev-devel mailing list
Linux-fbdev-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

Re: [PATCH 1/2] fb: add support for foreign endianness

[Anton Vorontsov]

On Mon, Feb 18, 2008 at 12:30:11PM -0500, Valdis.Kletnieks@vt.edu wrote:
> On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said:
> > I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
> > Both resolve endianess at driver level. Actually, both handle it by setting special
> > bits so the graphics chip itself reorder bytes to transform foreign endianess. 
> > I understand that this patch is for chips which cannot reorder bytes by themselves.
> 
> Does anybody know of such a chip that's actually available in the wild?

LE Fujitsu mb86277 (MINT) on the BE MPC8360E.

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness

[Clemens Koller]

Valdis.Kletnieks@vt.edu schrieb:
> On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said:
>> I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
>> Both resolve endianess at driver level. Actually, both handle it by setting special
>> bits so the graphics chip itself reorder bytes to transform foreign endianess. 
>> I understand that this patch is for chips which cannot reorder bytes by themselves.
> 
> Does anybody know of such a chip that's actually available in the wild?  Or are
> we writing drivers for speculative possible chips?
> 

I had troubles with the Silicon Motion SM501/SM502 endianess on PowerPC PCI vs. LocalBus.
The chip also has a register to swap endianess, but that seems to only affect some
LocalBus modes.
The current fb and X drivers are working, but when it comes to font
aliasing and hw-acceleration, the problems start to rise again...

Regards,

Clemens

Re: [PATCH 1/2] fb: add support for foreign endianness

[Benjamin Herrenschmidt]

On Tue, 2008-02-19 at 00:35 +0100, Clemens Koller wrote:
> Valdis.Kletnieks@vt.edu schrieb:
> > On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said:
> >> I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
> >> Both resolve endianess at driver level. Actually, both handle it by setting special
> >> bits so the graphics chip itself reorder bytes to transform foreign endianess. 
> >> I understand that this patch is for chips which cannot reorder bytes by themselves.
> > 
> > Does anybody know of such a chip that's actually available in the wild?  Or are
> > we writing drivers for speculative possible chips?
> > 
> 
> I had troubles with the Silicon Motion SM501/SM502 endianess on PowerPC PCI vs. LocalBus.
> The chip also has a register to swap endianess, but that seems to only affect some
> LocalBus modes.
> The current fb and X drivers are working, but when it comes to font
> aliasing and hw-acceleration, the problems start to rise again...

Most "sane" gfx chips nowadays provide configurable surfaces that allow
to perform the swap when writing/reading from regions of the
framebuffer, with the ability to set a different swapper setting (based
on bit depth) per region.

Then there is also the risk that your PCI<->Localbus has been wired
improperly :-)

Ben.



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness

[Clemens Koller]

Benjamin Herrenschmidt schrieb:
> On Tue, 2008-02-19 at 00:35 +0100, Clemens Koller wrote:
>> Valdis.Kletnieks@vt.edu schrieb:
>>> On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said:
>>>> I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
>>>> Both resolve endianess at driver level. Actually, both handle it by setting special
>>>> bits so the graphics chip itself reorder bytes to transform foreign endianess. 
>>>> I understand that this patch is for chips which cannot reorder bytes by themselves.
>>> Does anybody know of such a chip that's actually available in the wild?  Or are
>>> we writing drivers for speculative possible chips?
>>>
>> I had troubles with the Silicon Motion SM501/SM502 endianess on PowerPC PCI vs. LocalBus.
>> The chip also has a register to swap endianess, but that seems to only affect some
>> LocalBus modes.
>> The current fb and X drivers are working, but when it comes to font
>> aliasing and hw-acceleration, the problems start to rise again...
> 
> Most "sane" gfx chips nowadays provide configurable surfaces that allow
> to perform the swap when writing/reading from regions of the
> framebuffer, with the ability to set a different swapper setting (based
> on bit depth) per region.

Most! But not the SM50x. I still hope I would be wrong here. :-(

> Then there is also the risk that your PCI<->Localbus has been wired
> improperly :-)

That's not an issue in my case. The SM50x can be connected to
either an PCI or some Local/CPU-whateverbus IF.
I.e. on the MPC85xx PowerPC, PCI and LocalBus are separate bussses.
If the sm501 is attached to the MPC85xx' PCI like any other video card,
the PCI config-space is can be accessed as usual, whereas the framebuffer
memory area is byte-swapped compared to other common video cards.

So, to get back on topic:
I would welcome endianess swapping in SW. Some architectures (PowerPC)
should also be able to do swapped-endian mmapping. I just haven't
had time for a closer look but it looks also interesting way to do it
that way.

Regards,

Clemens

Re: [PATCH 1/2] fb: add support for foreign endianness

[Andrew Morton]

On Tue, 19 Feb 2008 12:27:54 +0100 Clemens Koller <clemens.koller@anagramm.de> wrote:

> Benjamin Herrenschmidt schrieb:
> > On Tue, 2008-02-19 at 00:35 +0100, Clemens Koller wrote:
> >> Valdis.Kletnieks@vt.edu schrieb:
> >>> On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said:
> >>>> I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
> >>>> Both resolve endianess at driver level. Actually, both handle it by setting special
> >>>> bits so the graphics chip itself reorder bytes to transform foreign endianess. 
> >>>> I understand that this patch is for chips which cannot reorder bytes by themselves.
> >>> Does anybody know of such a chip that's actually available in the wild?  Or are
> >>> we writing drivers for speculative possible chips?
> >>>
> >> I had troubles with the Silicon Motion SM501/SM502 endianess on PowerPC PCI vs. LocalBus.
> >> The chip also has a register to swap endianess, but that seems to only affect some
> >> LocalBus modes.
> >> The current fb and X drivers are working, but when it comes to font
> >> aliasing and hw-acceleration, the problems start to rise again...
> > 
> > Most "sane" gfx chips nowadays provide configurable surfaces that allow
> > to perform the swap when writing/reading from regions of the
> > framebuffer, with the ability to set a different swapper setting (based
> > on bit depth) per region.
> 
> Most! But not the SM50x. I still hope I would be wrong here. :-(
> 
> > Then there is also the risk that your PCI<->Localbus has been wired
> > improperly :-)
> 
> That's not an issue in my case. The SM50x can be connected to
> either an PCI or some Local/CPU-whateverbus IF.
> I.e. on the MPC85xx PowerPC, PCI and LocalBus are separate bussses.
> If the sm501 is attached to the MPC85xx' PCI like any other video card,
> the PCI config-space is can be accessed as usual, whereas the framebuffer
> memory area is byte-swapped compared to other common video cards.
> 
> So, to get back on topic:
> I would welcome endianess swapping in SW. Some architectures (PowerPC)
> should also be able to do swapped-endian mmapping. I just haven't
> had time for a closer look but it looks also interesting way to do it
> that way.

Bizarrely, the original author of the patch (Anton) has fallen off the cc. 
Could whoever did that please thwap himself?

Anyway, my head is now officially spinning.  Did anyone actually have a
reason why we shouldn't proceed with Anton's patch?

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness

[Clemens Koller]

Andrew Morton schrieb:
> On Tue, 19 Feb 2008 12:27:54 +0100 Clemens Koller <clemens.koller@anagramm.de> wrote:
>> Benjamin Herrenschmidt schrieb:
>>> On Tue, 2008-02-19 at 00:35 +0100, Clemens Koller wrote:
>>>> Valdis.Kletnieks@vt.edu schrieb:
>>>>> On Mon, 18 Feb 2008 08:18:47 +0100, Krzysztof Helt said:
>>>>>> I know two fb drivers which use endianess information (pm2fb and s3c2410fb).
>>>>>> Both resolve endianess at driver level. Actually, both handle it by setting special
>>>>>> bits so the graphics chip itself reorder bytes to transform foreign endianess. 
>>>>>> I understand that this patch is for chips which cannot reorder bytes by themselves.
>>>>> Does anybody know of such a chip that's actually available in the wild?  Or are
>>>>> we writing drivers for speculative possible chips?
>>>>>
>>>> I had troubles with the Silicon Motion SM501/SM502 endianess on PowerPC PCI vs. LocalBus.
>>>> The chip also has a register to swap endianess, but that seems to only affect some
>>>> LocalBus modes.
>>>> The current fb and X drivers are working, but when it comes to font
>>>> aliasing and hw-acceleration, the problems start to rise again...
>>> Most "sane" gfx chips nowadays provide configurable surfaces that allow
>>> to perform the swap when writing/reading from regions of the
>>> framebuffer, with the ability to set a different swapper setting (based
>>> on bit depth) per region.
>> Most! But not the SM50x. I still hope I would be wrong here. :-(
>>
>>> Then there is also the risk that your PCI<->Localbus has been wired
>>> improperly :-)
>> That's not an issue in my case. The SM50x can be connected to
>> either an PCI or some Local/CPU-whateverbus IF.
>> I.e. on the MPC85xx PowerPC, PCI and LocalBus are separate bussses.
>> If the sm501 is attached to the MPC85xx' PCI like any other video card,
>> the PCI config-space is can be accessed as usual, whereas the framebuffer
>> memory area is byte-swapped compared to other common video cards.
>>
>> So, to get back on topic:
>> I would welcome endianess swapping in SW. Some architectures (PowerPC)
>> should also be able to do swapped-endian mmapping. I just haven't
>> had time for a closer look but it looks also interesting way to do it
>> that way.
> 
> Bizarrely, the original author of the patch (Anton) has fallen off the cc. 
> Could whoever did that please thwap himself?

Propably my bad, being subscribed to several CCed lists...

> Anyway, my head is now officially spinning.  Did anyone actually have a
> reason why we shouldn't proceed with Anton's patch?

Since it seem that there are some odd chips out in the wild, I guess
Valdis (also readded to CC:) has no more objections to give it a try. :-)

Regards,

Clemens

Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Tue, 19 Feb 2008 04:05:30 PST, Andrew Morton said:
> On Tue, 19 Feb 2008 12:27:54 +0100 Clemens Koller <clemens.koller@anagramm.de
> wrote:
> > That's not an issue in my case. The SM50x can be connected to
> > either an PCI or some Local/CPU-whateverbus IF.
> > I.e. on the MPC85xx PowerPC, PCI and LocalBus are separate bussses.
> > If the sm501 is attached to the MPC85xx' PCI like any other video card,
> > the PCI config-space is can be accessed as usual, whereas the framebuffer
> > memory area is byte-swapped compared to other common video cards.

> Anyway, my head is now officially spinning.  Did anyone actually have a
> reason why we shouldn't proceed with Anton's patch?

Clemens answered my question regarding the real-life existence of hardware
that would benefit.  I'd say if Anton's patch works on Clemens' hardware and
otherwise passes review, we should proceed...


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness

[Benjamin Herrenschmidt]

On Tue, 2008-02-19 at 19:47 -0500, Valdis.Kletnieks@vt.edu wrote:
> On Tue, 19 Feb 2008 04:05:30 PST, Andrew Morton said:
> > On Tue, 19 Feb 2008 12:27:54 +0100 Clemens Koller <clemens.koller@anagramm.de
> > wrote:
> > > That's not an issue in my case. The SM50x can be connected to
> > > either an PCI or some Local/CPU-whateverbus IF.
> > > I.e. on the MPC85xx PowerPC, PCI and LocalBus are separate bussses.
> > > If the sm501 is attached to the MPC85xx' PCI like any other video card,
> > > the PCI config-space is can be accessed as usual, whereas the framebuffer
> > > memory area is byte-swapped compared to other common video cards.
> 
> > Anyway, my head is now officially spinning.  Did anyone actually have a
> > reason why we shouldn't proceed with Anton's patch?
> 
> Clemens answered my question regarding the real-life existence of hardware
> that would benefit.  I'd say if Anton's patch works on Clemens' hardware and
> otherwise passes review, we should proceed...

No objection here neither.

Ben.

Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness

[Paul Mackerras]

Andrew Morton writes:

> Bizarrely, the original author of the patch (Anton) has fallen off the cc. 
> Could whoever did that please thwap himself?
> 
> Anyway, my head is now officially spinning.  Did anyone actually have a
> reason why we shouldn't proceed with Anton's patch?

I was wondering if it would be sufficient to provide alternative
versions of fb_readl, fb_writel etc. that do byte-swapping.  That
would mean that all framebuffers would have to have the same
endianness, but that would suffice for embedded systems such as
Anton's and would end up a lot simpler IMHO.

Paul.

Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness

[Anton Vorontsov]

On Wed, Feb 20, 2008 at 11:56:35AM +1100, Paul Mackerras wrote:
> Andrew Morton writes:
> 
> > Bizarrely, the original author of the patch (Anton) has fallen off the cc. 
> > Could whoever did that please thwap himself?
> > 
> > Anyway, my head is now officially spinning.  Did anyone actually have a
> > reason why we shouldn't proceed with Anton's patch?
> 
> I was wondering if it would be sufficient to provide alternative
> versions of fb_readl, fb_writel etc. that do byte-swapping.

This is of course viable alternative. And I was considering this, but
later I abandoned the idea: that way we'll end up doing math in the
native endianness and then converting it to the foreign. This feels
ugly in contrast when we can do the right math in the first place, per
framebuffer.

> That
> would mean that all framebuffers would have to have the same
> endianness,

Yup, another downside of changing the code to fix some narrow
problem. Plus, this means things will break if/when we'll attach
PCI video card into the MPC8360E-RDK.


Thanks,

-- 
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2

Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness

[Benjamin Herrenschmidt]

On Wed, 2008-02-20 at 15:18 +0300, Anton Vorontsov wrote:
> On Wed, Feb 20, 2008 at 11:56:35AM +1100, Paul Mackerras wrote:
> > Andrew Morton writes:
> > 
> > > Bizarrely, the original author of the patch (Anton) has fallen off the cc. 
> > > Could whoever did that please thwap himself?
> > > 
> > > Anyway, my head is now officially spinning.  Did anyone actually have a
> > > reason why we shouldn't proceed with Anton's patch?
> > 
> > I was wondering if it would be sufficient to provide alternative
> > versions of fb_readl, fb_writel etc. that do byte-swapping.
> 
> This is of course viable alternative. And I was considering this, but
> later I abandoned the idea: that way we'll end up doing math in the
> native endianness and then converting it to the foreign. This feels
> ugly in contrast when we can do the right math in the first place, per
> framebuffer.

Also, the type of swap to do in fb_readl/writel would have to depend
on the bit depth which is kind of ugly.

> > That
> > would mean that all framebuffers would have to have the same
> > endianness,
> 
> Yup, another downside of changing the code to fix some narrow
> problem. Plus, this means things will break if/when we'll attach
> PCI video card into the MPC8360E-RDK.

Right.

Ben.

Re: [Linux-fbdev-devel] [PATCH 1/2] fb: add support for foreign endianness

[Paul Mackerras]

Anton Vorontsov writes:

> > I was wondering if it would be sufficient to provide alternative
> > versions of fb_readl, fb_writel etc. that do byte-swapping.
> 
> This is of course viable alternative. And I was considering this, but
> later I abandoned the idea: that way we'll end up doing math in the
> native endianness and then converting it to the foreign. This feels
> ugly in contrast when we can do the right math in the first place, per
> framebuffer.

OK.  I guess I'm convinced then.  However, your patch description
needs to be a lot better.  It should describe things like why you
want to make the change and why the change you are proposing is a good
idea and is better than other alternatives.  If you'd done that
originally we might not have needed to have all this discussion. :)

Paul.

Re: [PATCH 1/2] fb: add support for foreign endianness

[Clemens Koller]

Paul Mackerras schrieb:
> Andrew Morton writes:
> 
>> Bizarrely, the original author of the patch (Anton) has fallen off the cc. 
>> Could whoever did that please thwap himself?
>>
>> Anyway, my head is now officially spinning.  Did anyone actually have a
>> reason why we shouldn't proceed with Anton's patch?
> 
> I was wondering if it would be sufficient to provide alternative
> versions of fb_readl, fb_writel etc. that do byte-swapping.  That
> would mean that all framebuffers would have to have the same
> endianness, but that would suffice for embedded systems such as
> Anton's and would end up a lot simpler IMHO.

Yes. At least the affected PowerPC platforms can do byte-swapping on
the fly, so this should be sufficient and without performance
penalty, AFAICS.

Regards,

Clemens

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/