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

[Antonino A. D. <ad...@gm...>]

On Thu, 2007-08-30 at 19:28 +0200, Pavel Pisa wrote:
> Hello Antonino and others,
> 
> the generic BLT functions binds unconditionally byte endian with
> pixels in byte order. Unfortunately there are more devices which
> can work only in unsupported pixels layout. One of them is
> Freescale MX1 found on PiMX1 board when used in 4 bpp monochromatic
> mode. Curiously Microwindows and Qt-embedded take this layout
> as default one and works without problems. Linux console output
> is funny on the other hand.
> 
> I would like to know, if suggested solution is at least remotely
> acceptable for mainline in future. It would be probably good
> to add some flag to struct fb_info.var which would enable swapping.
> This would allow code to be generic even if
>   CONFIG_FB_CFB_REV_PIXELS_IN_BYTE
> is selected. If option is disabled, then there should be
> zero overhead, all abundant code should be removed by compiller.
> 
> If this is acceptable, I try to add support for rectangle
> and copy in future, but this do not seems to be critical for
> console. Normal fonts widths are multiples of 8 so problems
> are not seen.
> 
> Best wishes
> 
>              Pavel Pisa
> 
> ===================================================================
> 
> The next patch allows generic frame-buffer code to correctly write texts
> and blit images for 1, 2 and 4 bit per pixel frame-buffer organizations
> when pixels in bytes are organized to in opposite order than bytes
> in long type.
> 
> Overhead should be reasonable. If option is not selected, than compiler
> should eliminate completely all overhead.
> 
> Signed-off-by: Pavel Pisa <pi...@cm...>
> 
>  drivers/video/Kconfig     |   10 +++++++++
>  drivers/video/cfbimgblt.c |   48 ++++++++++++++++++++++++++++++++++++++++------
>  2 files changed, 52 insertions(+), 6 deletions(-)
> 
> Index: linux-2.6.23-git/drivers/video/cfbimgblt.c
> ===================================================================
> --- linux-2.6.23-git.orig/drivers/video/cfbimgblt.c
> +++ linux-2.6.23-git/drivers/video/cfbimgblt.c
> @@ -75,6 +75,20 @@ static const u32 cfb_tab32[] = {
>  #define FB_WRITEL fb_writel
>  #define FB_READL  fb_readl
>  
> +#define FB_SHIFTED_PIXELS_MASK(index, bswapmask) ({ \
> +		u32 _m; \
> +		u32 _indx = (index); \
> +		if (!bswapmask) { \
> +			_m = FB_SHIFT_HIGH(~(u32)0, _indx); \
> +		} else { \
> +			_m = 0xff << FB_LEFT_POS(8); \
> +			_m = FB_SHIFT_LOW(_m, _indx & (bswapmask)) & _m; \
> +			_m = FB_SHIFT_HIGH(_m, _indx & ~(bswapmask)); \
> +			_m |= FB_SHIFT_HIGH(~(u32)0, (_indx + bswapmask) & ~(bswapmask)); \
> +		} \
> +		_m; \
> +	})
> +

If you can convert the above to to an inline function instead of a
macro, that would be best.

> +
> +#ifdef CONFIG_FB_CFB_REV_PIXELS_IN_BYTE
> +	if (bpp < 8) {
> +		/*
> +		 * Reversed order of pixel layout in bytes
> +		 * works only for 1, 2 and 4 bpp
> +		 */
> +		bswapmask = 7 - bpp + 1;
> +	}
> +#endif

And this too, if you can create a small inline function and move out the
#ifdef out of the function, that would be nice also.

Tony