Thread (11 messages) 11 messages, 4 authors, 2014-06-17

Re: [PATCH] offb: Fix little-endian support

From: Cedric Le Goater <hidden>
Date: 2014-05-14 17:04:19
Also in: linux-fbdev

On 05/14/2014 04:24 PM, Takashi Iwai wrote:
At Wed, 14 May 2014 16:01:17 +0200,
Cedric Le Goater wrote:
quoted
Hi Iwai-san,

On 05/14/2014 03:21 PM, Takashi Iwai wrote:
quoted
Although the color palette was corrected for little endian by the
commit  [e1edf18b: offb: Add palette hack for little endian], the
graphics mode is still shown in psychedelic colors.  
Are you referring to the linux logo colors ? If so, could you please
try the patch below, it should be a fix.
Not only penguin logo but the whole X graphics got strange colors,
too, according to the bug report.  I put the original reporter/tester
(Dinar Valeev) to Cc.
I'm merely a person who tries to fix this mess ;)

BTW, did you try to run X on fbdev?
Not at the time, I was working on the console only, BE and LE. 
I just tried fbdev and indeed this is a psychedelic mess :)

Your fix has also issues on BE and console and the patch of mine 
below is of no use for fbdev. Damn, this is a nightmare.

C. 
quoted
quoted
For fixing this
properly, we rather need to correct the RGB offsets depending on
endianess.

Since the RGB base offsets are corrected, we don't need the hack for
pallette color entries.  This patch reverts that, too.
Are you testing using qemu -vga std -vnc :x ? If so, did you try changing 
the depth to 8,15,16,32 ?
Yes, it was with qemu -vga std -vnc :x.
About different color depths, Dinar can test / clarify better, I
suppose.
quoted
 I think the patch might be breaking big endian 
too.
Big endian should work as is because my patch uses the original
offsets when fb_be_math() is true.  It corrects the RGB offsets if
!fb_be_math().

But, I'm also entirely not sure whether this is 100% correct, either.
Namely, if the RGB offsets were correct for some little endian
machines with offb, my patch would break it, of course.  But, then
your previous fix must have already broken it as well, so I took the
same fb_be_math() check.


thanks,

Takashi
quoted
Now, I am far from being an expert on frame buffers. It would be glad 
to have some insights on that topic. 

Thanks,

C. 


[PATCH] fb: fix logo palette entries for little endian

The offb_cmap_byteswap() routine helps byteswapping the color map
entries when required. This patch externalizes and renames the helper 
routine to adjust the pseudo palette of the logo when running on 
little endian.

Signed-off-by: Cédric Le Goater <redacted>
---
 drivers/video/fbmem.c |    6 ++++--
 drivers/video/offb.c  |   11 +----------
 include/linux/fb.h    |    8 ++++++++
 3 files changed, 13 insertions(+), 12 deletions(-)

Index: linux.git/drivers/video/fbmem.c
===================================================================
--- linux.git.orig/drivers/video/fbmem.c
+++ linux.git/drivers/video/fbmem.c
@@ -252,7 +252,8 @@ static void  fb_set_logo_truepalette(str
 	blueshift  = info->var.blue.offset  - (8 - info->var.blue.length);
 
 	for ( i = 0; i < logo->clutsize; i++) {
-		palette[i+32] = (safe_shift((clut[0] & redmask), redshift) |
+		palette[i+32] = fb_cmap_byteswap(info,
+				 safe_shift((clut[0] & redmask), redshift) |
 				 safe_shift((clut[1] & greenmask), greenshift) |
 				 safe_shift((clut[2] & bluemask), blueshift));
 		clut += 3;
@@ -271,7 +272,8 @@ static void fb_set_logo_directpalette(st
 	blueshift = info->var.blue.offset;
 
 	for (i = 32; i < 32 + logo->clutsize; i++)
-		palette[i] = i << redshift | i << greenshift | i << blueshift;
+		palette[i] = fb_cmap_byteswap(info, i << redshift |
+					i << greenshift | i << blueshift);
 }
 
 static void fb_set_logo(struct fb_info *info,
Index: linux.git/drivers/video/offb.c
===================================================================
--- linux.git.orig/drivers/video/offb.c
+++ linux.git/drivers/video/offb.c
@@ -91,15 +91,6 @@ extern boot_infos_t *boot_infos;
 #define AVIVO_DC_LUTB_WHITE_OFFSET_GREEN        0x6cd4
 #define AVIVO_DC_LUTB_WHITE_OFFSET_RED          0x6cd8
 
-#define FB_RIGHT_POS(p, bpp)         (fb_be_math(p) ? 0 : (32 - (bpp)))
-
-static inline u32 offb_cmap_byteswap(struct fb_info *info, u32 value)
-{
-	u32 bpp = info->var.bits_per_pixel;
-
-	return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
-}
-
     /*
      *  Set a single color register. The values supplied are already
      *  rounded down to the hardware's capabilities (according to the
@@ -129,7 +120,7 @@ static int offb_setcolreg(u_int regno, u
 			mask <<= info->var.transp.offset;
 			value |= mask;
 		}
-		pal[regno] = offb_cmap_byteswap(info, value);
+		pal[regno] = fb_cmap_byteswap(info, value);
 		return 0;
 	}
 
Index: linux.git/include/linux/fb.h
===================================================================
--- linux.git.orig/include/linux/fb.h
+++ linux.git/include/linux/fb.h
@@ -582,6 +582,7 @@ static inline struct apertures_struct *a
 
 #endif
 
+#define FB_RIGHT_POS(p, bpp)         (fb_be_math(p) ? 0 : (32 - (bpp)))
 #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))
@@ -681,6 +682,13 @@ static inline bool fb_be_math(struct fb_
 #endif /* CONFIG_FB_FOREIGN_ENDIAN */
 }
 
+static inline u32 fb_cmap_byteswap(struct fb_info *info, u32 value)
+{
+	u32 bpp = info->var.bits_per_pixel;
+
+	return cpu_to_be32(value) >> FB_RIGHT_POS(info, bpp);
+}
+
 /* drivers/video/fbsysfs.c */
 extern struct fb_info *framebuffer_alloc(size_t size, struct device *dev);
 extern void framebuffer_release(struct fb_info *info);
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help