common lcd refactor
Subsystems (4)
cover common lcd refactor
This series is part of my ongoing efforts to cleanup common/lcd code (and at some point merge it with the CONFIG_VIDEO code).
This series focuses on eliminating platform specific #defines from lcd.c code, and then adds a few platform independent code refactors and cleanups on top of that. A few notes on the series:
- The patch "lcd: split splash code into its own function" depends on Bo Shen's patch "lcd: fix console address is not initialized"
(https://patchwork.ozlabs.org/patch/433746/).
- The patch "lcd: split configuration_get_cmap" turned out to be a little
problematic: the goal was to move all platform specific code to the appropriate lcd/fb drivers, and keep the generic case in lcd.c as a weak function. This was not possible to do for configuration_get_cmap(), because the weak version still has to compile even if it is overridden, and unfortunately the generic case references panel_info, which is a struct that varies depending on the configuration, and does not always have a cmap field. This demonstrated that the visibility of panel_info should be reviewed, and that there might be a need for some kind of lcd_generic driver.
Since this may warrant a series of its own, I opted to tentatively implement the generic configuration_get_cmap() function in lcd.h as static inline, and plan to revisit it in the next series.
The end result was compile tested on arm and powerpc, and tested on cm_t35 (splash screen feature).
Nikita Kiryanov (13):
lcd: move platform specific structs to their own headers
lcd: split configuration_get_cmap
lcd: atmel: move atmel specific fb_put_word to atmel_lcdfb
lcd: mpc8xx: move mpc823 specific fb_put_byte to mpc8xx_lcd.c
lcd: atmel: introduce lcd_logo_set_cmap
lcd: mpc823: move mpc823 specific lcd_logo_set_cmap code to
mpc8xx_lcd.c
lcd: logo: move generic cmap setting to lcd_logo_set_cmap()
lcd: introduce lcd_set_cmap
lcd: remove unused includes
lcd: various cleanups
lcd: rename bitmap_plot to better represent its functionality
lcd: dt: extract simplefb support
lcd: split splash code into its own function
board/raspberrypi/rpi/rpi.c | 1 + common/Makefile | 1 + common/lcd.c | 358 ++++++++++--------------------------------- common/lcd_simplefb.c | 59 +++++++ common/splash.c | 16 ++ drivers/video/atmel_hlcdfb.c | 13 ++ drivers/video/atmel_lcdfb.c | 51 ++++++ drivers/video/exynos_fb.c | 9 ++ drivers/video/mpc8xx_lcd.c | 29 ++++ drivers/video/pxa_lcd.c | 6 + include/atmel_lcd.h | 38 +++++ include/exynos_lcd.h | 81 ++++++++++ include/fdt_simplefb.h | 14 ++ include/lcd.h | 313 ++++++------------------------------- include/mpc823_lcd.h | 43 ++++++ include/pxa_lcd.h | 80 ++++++++++ include/splash.h | 11 +- 17 files changed, 576 insertions(+), 547 deletions(-) create mode 100644 common/lcd_simplefb.c create mode 100644 include/atmel_lcd.h create mode 100644 include/exynos_lcd.h create mode 100644 include/fdt_simplefb.h create mode 100644 include/mpc823_lcd.h create mode 100644 include/pxa_lcd.h
Signature · 1 line
1.9.1
PATCH 1/13 lcd: move platform specific structs to their own headers
common/lcd code is full of platform specific code and definitions, which ideally should reside with the respective driver code. Take a step towards that goal by moving platform specific structs from lcd.h to their own header files.
The structs for the generic case (the #else for all the platform specific cases) is retained in lcd.h as the default case.
--- include/atmel_lcd.h | 38 ++++++++++ include/exynos_lcd.h | 81 +++++++++++++++++++++ include/lcd.h | 201 ++------------------------------------------------- include/mpc823_lcd.h | 43 +++++++++++ include/pxa_lcd.h | 80 ++++++++++++++++++++ 5 files changed, 247 insertions(+), 196 deletions(-) create mode 100644 include/atmel_lcd.h create mode 100644 include/exynos_lcd.h create mode 100644 include/mpc823_lcd.h create mode 100644 include/pxa_lcd.h
include/atmel_lcd.h new file +38 -0
@@ -0,0 +1,38 @@ +/* +* atmel_lcd.h - Atmel LCD Controller structures +* +* (C) Copyright 2001 +* Wolfgang Denk, DENX Software Engineering, wd at denx.de. +* +* SPDX-License-Identifier: GPL-2.0+ +*/ + +#ifndef _ATMEL_LCD_H_ +#define _ATMEL_LCD_H_ + +typedef struct vidinfo { +ushort vl_col;/* Number of columns (i.e. 640) */ +ushort vl_row;/* Number of rows (i.e. 480) */ +u_long vl_clk;/* pixel clock in ps */ + +/* LCD configuration register */ +u_long vl_sync;/* Horizontal / vertical sync */ +u_long vl_bpix;/* Bits per pixel, 0 = 1, 1 = 2, 2 = 4, 3 = 8, 4 = 16 */ +u_long vl_tft;/* 0 = passive, 1 = TFT */ +u_long vl_cont_pol_low;/* contrast polarity is low */ +u_long vl_clk_pol;/* clock polarity */ + +/* Horizontal control register. */ +u_long vl_hsync_len;/* Length of horizontal sync */ +u_long vl_left_margin;/* Time from sync to picture */ +u_long vl_right_margin;/* Time from picture to sync */ + +/* Vertical control register. */ +u_long vl_vsync_len;/* Length of vertical sync */ +u_long vl_upper_margin;/* Time from sync to picture */ +u_long vl_lower_margin;/* Time from picture to sync */ + +u_long mmio;/* Memory mapped registers */ +}vidinfo_t ; + +#endif
include/exynos_lcd.h new file +81 -0
81 lines not on this page · raw message
include/lcd.h +5 -196
218 lines not on this page · raw message
include/mpc823_lcd.h new file +43 -0
43 lines not on this page · raw message
include/pxa_lcd.h new file +80 -0
80 lines not on this page · raw message
Signature · 1 line
1.9.1
PATCH 1/13 lcd: move platform specific structs to their own headers
Hi Nikita,
Quoted 6 lines · Nikita Kiryanov
common/lcd code is full of platform specific code and definitions, which ideally should reside with the respective driver code. Take a step towards that goal by moving platform specific structs from
lcd.hto their own header files.The structs for the generic case (the #else for all the platform specific cases) is retained in
lcd.has the default case.
platform-specific (i.e. add a hypen)
Quoted 5 lines
Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>
Quoted 11 lines
--- include/atmel_lcd.h | 38 ++++++++++ include/exynos_lcd.h | 81 +++++++++++++++++++++ include/lcd.h | 201 ++------------------------------------------------- include/mpc823_lcd.h | 43 +++++++++++ include/pxa_lcd.h | 80 ++++++++++++++++++++ 5 files changed, 247 insertions(+), 196 deletions(-) create mode 100644 include/atmel_lcd.h create mode 100644 include/exynos_lcd.h create mode 100644 include/mpc823_lcd.h create mode 100644 include/pxa_lcd.h
PATCH 2/13 lcd: split configuration_get_cmap
In reply to Nikita Kiryanovconfiguration_get_cmap() is multiple platform specific functions stuffed into one function. Split it into multiple versions, and move each version to the appropriate driver to reduce the #ifdef complexity.
--- common/lcd.c | 19 ------------------- drivers/video/atmel_hlcdfb.c | 13 +++++++++++++ drivers/video/atmel_lcdfb.c | 5 +++++ drivers/video/exynos_fb.c | 9 +++++++++ drivers/video/mpc8xx_lcd.c | 7 +++++++ drivers/video/pxa_lcd.c | 6 ++++++ include/lcd.h | 9 +++++++++ 7 files changed, 49 insertions(+), 19 deletions(-)
common/lcd.c +0 -19
@@ -383,25 +383,6 @@ int lcd_getbgcolor(void)/************************************************************************/ /* ** Chipset depending Bitmap / Logo stuff... */ /************************************************************************/ -static inline ushort *configuration_get_cmap (void ) -{ -#if definedCONFIG_CPU_PXA -struct pxafb_info *fbi = &panel_info.pxa; -return (ushort *)fbi->palette; -#elif defined(CONFIG_MPC823 ) -immap_t *immr = (immap_t *)CONFIG_SYS_IMMR ; -cpm8xx_t *cp = &(immr->im_cpm); -return (ushort *)&(cp->lcd_cmap[255 *sizeof (ushort)]); -#elif defined(CONFIG_ATMEL_LCD ) -return (ushort *)(panel_info.mmio +ATMEL_LCDC_LUT (0 )); -#elif !defined(CONFIG_ATMEL_HLCD ) && !defined(CONFIG_EXYNOS_FB ) -return panel_info.cmap; -#elif defined(CONFIG_LCD_LOGO ) -return bmp_logo_palette; -#else -return NULL ; -#endif -}#ifdef CONFIG_LCD_LOGO void bitmap_plot (int x,int y)
drivers/video/atmel_hlcdfb.c +13 -0
@@ -13,6 +13,10 @@#include <lcd.h> #include <atmel_hlcdc.h> +#if defined(CONFIG_LCD_LOGO ) +#include <bmp_logo.h> +#endif +/* configurable parameters */ #define ATMEL_LCDC_CVAL_DEFAULT 0xc8#define ATMEL_LCDC_DMA_BURST_LEN 8
@@ -37,6 +41,15 @@ void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue) panel_info.mmio + ATMEL_LCDC_LUT(regno)); } +ushort *configuration_get_cmap (void ) +{ +#if defined(CONFIG_LCD_LOGO ) +return bmp_logo_palette; +#else +return NULL ; +#endif +} +void lcd_ctrl_init (void *lcdbase) {unsigned long value;
drivers/video/atmel_lcdfb.c +5 -0
@@ -29,6 +29,11 @@#define lcdc_readl (mmio, reg) __raw_readl((mmio)+(reg))#define lcdc_writel (mmio, reg, val) __raw_writel((val), (mmio)+(reg)) +ushort *configuration_get_cmap (void ) +{ +return (ushort *)(panel_info.mmio +ATMEL_LCDC_LUT (0 )); +} +void lcd_setcolreg (ushort regno,ushort red,ushort green,ushort blue) {#if defined(CONFIG_ATMEL_LCD_BGR555 )
drivers/video/exynos_fb.c +9 -0
@@ -37,6 +37,15 @@ vidinfo_t panel_info = { };#endif +ushort *configuration_get_cmap (void ) +{ +#if defined(CONFIG_LCD_LOGO ) +return bmp_logo_palette; +#else +return NULL ; +#endif +} +static void exynos_lcd_init_mem (void *lcdbase,vidinfo_t *vid) {unsigned long palette_size;
drivers/video/mpc8xx_lcd.c +7 -0
@@ -357,6 +357,13 @@ lcd_setcolreg (ushort regno, ushort red, ushort green, ushort blue)/*----------------------------------------------------------------------*/ +ushort *configuration_get_cmap (void ) +{ +immap_t *immr = (immap_t *)CONFIG_SYS_IMMR ; +cpm8xx_t *cp = &(immr->im_cpm); +return (ushort *)&(cp->lcd_cmap[255 *sizeof (ushort)]); +} +void lcd_enable (void ) {volatile immap_t *immr = (immap_t *)CONFIG_SYS_IMMR ;
drivers/video/pxa_lcd.c +6 -0
@@ -342,6 +342,12 @@ static int pxafb_init (vidinfo_t *vid);/* --------------- PXA chipset specific functions ------------------- */ /************************************************************************/ +ushort *configuration_get_cmap (void ) +{ +struct pxafb_info *fbi = &panel_info.pxa; +return (ushort *)fbi->palette; +} +void lcd_ctrl_init (void *lcdbase) {pxafb_init_mem (lcdbase, &panel_info);
include/lcd.h +9 -0
@@ -42,13 +42,17 @@ void lcd_set_flush_dcache(int flush); #if defined CONFIG_MPC823 #include <mpc823_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_CPU_PXA25X) || defined(CONFIG_CPU_PXA27X) || \ defined CONFIG_CPU_MONAHANS #include <pxa_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_ATMEL_LCD) || defined(CONFIG_ATMEL_HLCD) #include <atmel_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_EXYNOS_FB) #include <exynos_lcd.h> +ushort *configuration_get_cmap(void); #else typedef struct vidinfo { ushort vl_col; /* Number of columns (i.e. 160) */
@@ -60,6 +64,11 @@ typedef struct vidinfo { void *priv; /* Pointer to driver-specific data */ } vidinfo_t; + +static inline ushort *configuration_get_cmap (void ) +{ +return panel_info.cmap; +}#endif extern vidinfo_t panel_info;
Signature · 1 line
1.9.1
PATCH 2/13 lcd: split configuration_get_cmap
Hi Nikita,
Quoted 8 lines · Nikita Kiryanov
configuration_get_cmap()is multiple platform specific functions stuffed into one function. Split it into multiple versions, and move each version to the appropriate driver to reduce the #ifdef complexity.Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>
Quoted 9 lines
--- common/lcd.c | 19 ------------------- drivers/video/atmel_hlcdfb.c | 13 +++++++++++++ drivers/video/atmel_lcdfb.c | 5 +++++ drivers/video/exynos_fb.c | 9 +++++++++ drivers/video/mpc8xx_lcd.c | 7 +++++++ drivers/video/pxa_lcd.c | 6 ++++++ include/lcd.h | 9 +++++++++ 7 files changed, 49 insertions(+), 19 deletions(-)
See suggestion below.
[snip]
Quoted 23 lines
include/lcd.h +4
-0@@ -42,13 +42,17 @@ void lcd_set_flush_dcache(int flush); #if defined CONFIG_MPC823 #include <mpc823_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_CPU_PXA25X) || defined(CONFIG_CPU_PXA27X) || \ defined CONFIG_CPU_MONAHANS #include <pxa_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_ATMEL_LCD) || defined(CONFIG_ATMEL_HLCD) #include <atmel_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_EXYNOS_FB) #include <exynos_lcd.h> +ushort *configuration_get_cmap(void); #else typedef struct vidinfo { ushort vl_col; /* Number of columns (i.e. 160) */
include/lcd.h typedef struct vidinfo { · line 72
Quoted 9 lines
include/lcd.h +5
-0@@ -60,6 +64,11 @@ typedef struct vidinfo { void *priv; /* Pointer to driver-specific data */ } vidinfo_t; + +static inline ushort *configuration_get_cmap (void ) +{ +return panel_info.cmap; +}#endif
I'd argue for dropping the inline version and just having the same prototype in each case.
Regards,
Simon
PATCH 2/13 lcd: split configuration_get_cmap
Hi Simon,
Quoted 50 lines · Simon Glass
Hi Nikita,
Quoted 8 lines · Nikita Kiryanov
configuration_get_cmap()is multiple platform specific functions stuffed into one function. Split it into multiple versions, and move each version to the appropriate driver to reduce the #ifdef complexity.Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>Quoted 9 lines
--- common/lcd.c | 19 ------------------- drivers/video/atmel_hlcdfb.c | 13 +++++++++++++ drivers/video/atmel_lcdfb.c | 5 +++++ drivers/video/exynos_fb.c | 9 +++++++++ drivers/video/mpc8xx_lcd.c | 7 +++++++ drivers/video/pxa_lcd.c | 6 ++++++ include/lcd.h | 9 +++++++++ 7 files changed, 49 insertions(+), 19 deletions(-)Reviewed-by Simon Glass <[email protected]>See suggestion below.
[snip]
Quoted 24 lines
include/lcd.h +4
-0@@ -42,13 +42,17 @@ void lcd_set_flush_dcache(int flush); #if defined CONFIG_MPC823 #include <mpc823_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_CPU_PXA25X) || defined(CONFIG_CPU_PXA27X) || \ defined CONFIG_CPU_MONAHANS #include <pxa_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_ATMEL_LCD) || defined(CONFIG_ATMEL_HLCD) #include <atmel_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_EXYNOS_FB) #include <exynos_lcd.h> +ushort *configuration_get_cmap(void); #else typedef struct vidinfo { ushort vl_col; /* Number of columns (i.e. 160) */@@ -60,6 +64,11 @@ typedef struct vidinfo {
Quoted 11 lines
include/lcd.h typedef struct vidinfo { · line 72
Quoted 8 lines
include/lcd.h +5
-0void *priv; /* Pointer to driver-specific data */ } vidinfo_t; + +static inline ushort *configuration_get_cmap (void ) +{ +return panel_info.cmap; +}#endif I'd argue for dropping the inline version and just having the same prototype in each case.
Are you suggesting something along the lines of:
static ushort *configuration_get_cmap(void)
{
return panel_info.cmap;
}#endif
ushort *configuration_get_cmap(void);
?
This is something I considered while preparing this, but I was under the impression that this isn't well defined by the standard. However, I actually have confirmation now that it is in fact defined behavior[1], so I will submit a V2 for this (even though I expect all of the above to disappear from lcd.h in the next refactor series).
Quoted 4 lines
Regards,
Simon
Signature · 2 lines
Regards, Nikita Kiryanov
PATCH 2/13 lcd: split configuration_get_cmap
Hi Nikita,
Quoted 96 lines · Nikita Kiryanov
Hi Simon,
Quoted 57 lines · Simon Glass
Hi Nikita,
Quoted 11 lines · Nikita Kiryanov
configuration_get_cmap()is multiple platform specific functions stuffed into
one function. Split it into multiple versions, and move each version to the
appropriate driver to reduce the #ifdef complexity.Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>Quoted 9 lines
--- common/lcd.c | 19 ------------------- drivers/video/atmel_hlcdfb.c | 13 +++++++++++++ drivers/video/atmel_lcdfb.c | 5 +++++ drivers/video/exynos_fb.c | 9 +++++++++ drivers/video/mpc8xx_lcd.c | 7 +++++++ drivers/video/pxa_lcd.c | 6 ++++++ include/lcd.h | 9 +++++++++ 7 files changed, 49 insertions(+), 19 deletions(-)Reviewed-by Simon Glass <[email protected]>See suggestion below.
[snip]
Quoted 25 lines
include/lcd.h +4
-0@@ -42,13 +42,17 @@ void lcd_set_flush_dcache(int flush); #if defined CONFIG_MPC823 #include <mpc823_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_CPU_PXA25X) || defined(CONFIG_CPU_PXA27X) || \ defined CONFIG_CPU_MONAHANS #include <pxa_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_ATMEL_LCD) || defined(CONFIG_ATMEL_HLCD) #include <atmel_lcd.h> +ushort *configuration_get_cmap(void); #elif defined(CONFIG_EXYNOS_FB) #include <exynos_lcd.h> +ushort *configuration_get_cmap(void); #else typedef struct vidinfo { ushort vl_col; /* Number of columns (i.e. 160) */@@ -60,6 +64,11 @@ typedef struct vidinfo {Quoted 12 lines
include/lcd.h typedef struct vidinfo { · line 72
Quoted 8 lines
include/lcd.h +5
-0void *priv; /* Pointer to driver-specific data */ } vidinfo_t; + +static inline ushort *configuration_get_cmap (void ) +{ +return panel_info.cmap; +}#endif I'd argue for dropping the inline version and just having the same prototype in each case.
Are you suggesting something along the lines of:
static ushort
*configuration_get_cmap(void){ return panel_info.cmap; }#endif
ushort
*configuration_get_cmap(void);?
This is something I considered while preparing this, but I was under the impression
that this isn't well defined by the standard. However, I actually have confirmation
now that it is in fact defined behavior[1], so I will submit a V2 for this (even though
I expect all of the above to disappear fromlcd.hin the next refactor series).
Quoted 2 lines
OK, although if you are planning to get rid of this code in the next refactor, it's fine just as you have it in v1.
Regards,
Simon
PATCH 3/13 lcd: atmel: move atmel specific fb_put_word to atmel_lcdfb
In reply to Nikita KiryanovReduce the amount of platform dependant code in common/lcd.c by moving Atmel specific implementation of fb_put_word() to atmel_lcdfb.c. Since we must also have a default implementation for everybody else, make the remainder of the code into a weak function.
--- common/lcd.c | 11 +---------- drivers/video/atmel_lcdfb.c | 9 +++++++++ 2 files changed, 10 insertions(+), 10 deletions(-)
common/lcd.c +1 -10
@@ -642,20 +642,11 @@ static void lcd_display_rle8_bitmap(bmp_image_t *bmp, ushort *cmap, uchar *fb,#endif #if defined(CONFIG_BMP_16BPP ) -#if defined(CONFIG_ATMEL_LCD_BGR555 ) -static inline void fb_put_word (uchar **fb,uchar **from) -{ - *(*fb)++ = (((*from)[0 ] &0x1f ) <<2 ) | ((*from)[1 ] &0x03 ); - *(*fb)++ = ((*from)[0 ] &0xe0 ) | (((*from)[1 ] &0x7c ) >>2 ); - *from +=2 ; -} -#else -static inline void fb_put_word (uchar **fb,uchar **from) +__weak void fb_put_word (uchar **fb,uchar **from) { *(*fb)++ = *(*from)++; *(*fb)++ = *(*from)++; } -#endif #endif /* CONFIG_BMP_16BPP */ int lcd_display_bitmap (ulong bmp_image,int x,int y)
drivers/video/atmel_lcdfb.c +9 -0
@@ -34,6 +34,15 @@ ushort *configuration_get_cmap(void)return (ushort *)(panel_info.mmio +ATMEL_LCDC_LUT (0 )); } +#if defined(CONFIG_BMP_16BPP ) && defined(CONFIG_ATMEL_LCD_BGR555 ) +void fb_put_word (uchar **fb,uchar **from) +{ + *(*fb)++ = (((*from)[0 ] &0x1f ) <<2 ) | ((*from)[1 ] &0x03 ); + *(*fb)++ = ((*from)[0 ] &0xe0 ) | (((*from)[1 ] &0x7c ) >>2 ); + *from +=2 ; +} +#endif +void lcd_setcolreg (ushort regno,ushort red,ushort green,ushort blue) {#if defined(CONFIG_ATMEL_LCD_BGR555 )
Signature · 1 line
1.9.1
PATCH 3/13 lcd: atmel: move atmel specific fb_put_word to atmel_lcdfb
Quoted 5 lines · Nikita Kiryanov
Reduce the amount of platform dependant code in
common/lcd.cby moving Atmel specific implementation offb_put_word()toatmel_lcdfb.c. Since we must also have a default implementation for everybody else, make the remainder of the code into a weak function.
Quoted 8 lines
Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 11 +---------- drivers/video/atmel_lcdfb.c | 9 +++++++++ 2 files changed, 10 insertions(+), 10 deletions(-)
PATCH 4/13 lcd: mpc8xx: move mpc823 specific fb_put_byte to mpc8xx_lcd.c
In reply to Nikita KiryanovReduce the amount of platform dependant code in common/lcd.c by moving MPC823 specific implementation of fb_put_byte() to mpc8xx_lcd.c. Since we must also have a default implementation for everybody else, make the remainder of the code into a weak function.
--- common/lcd.c | 11 +++++------ drivers/video/mpc8xx_lcd.c | 7 +++++++ 2 files changed, 12 insertions(+), 6 deletions(-)
common/lcd.c +5 -6
@@ -635,11 +635,10 @@ static void lcd_display_rle8_bitmap(bmp_image_t *bmp, ushort *cmap, uchar *fb, }#endif -#if defined(CONFIG_MPC823 ) -#define FB_PUT_BYTE (fb, from) *(fb)++ = (255 - *(from)++) -#else -#define FB_PUT_BYTE (fb, from) *(fb)++ = *(from)++ -#endif +__weak void fb_put_byte (uchar **fb,uchar **from) +{ + *(*fb)++ = *(*from)++; +}#if defined(CONFIG_BMP_16BPP )__weak void fb_put_word (uchar **fb,uchar **from)
@@ -764,7 +763,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)WATCHDOG_RESET ();for (j =0 ; j < width; j++) {if (bpix !=16 ) { -FB_PUT_BYTE (fb, bmap); +fb_put_byte (&fb, &bmap); }else { *(uint16_t *)fb = cmap_base[*(bmap++)]; fb +=sizeof (uint16_t ) /sizeof (*fb);
drivers/video/mpc8xx_lcd.c +7 -0
@@ -364,6 +364,13 @@ ushort *configuration_get_cmap(void)return (ushort *)&(cp->lcd_cmap[255 *sizeof (ushort)]); } +#if defined(CONFIG_MPC823 ) +void fb_put_byte (uchar **fb,uchar **from) +{ + *(*fb)++ = (255 - *(*from)++); +} +#endif +void lcd_enable (void ) {volatile immap_t *immr = (immap_t *)CONFIG_SYS_IMMR ;
Signature · 1 line
1.9.1
PATCH 4/13 lcd: mpc8xx: move mpc823 specific fb_put_byte to mpc8xx_lcd.c
Quoted 5 lines · Nikita Kiryanov
Reduce the amount of platform dependant code in
common/lcd.cby moving MPC823 specific implementation offb_put_byte()tompc8xx_lcd.c. Since we must also have a default implementation for everybody else, make the remainder of the code into a weak function.
Quoted 7 lines
Signed-off-by Nikita Kiryanov <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 11 +++++------ drivers/video/mpc8xx_lcd.c | 7 +++++++ 2 files changed, 12 insertions(+), 6 deletions(-)
PATCH 5/13 lcd: atmel: introduce lcd_logo_set_cmap
In reply to Nikita KiryanovReduce the bitmap_plot #ifdef complexity by extracting Atmel specific code for setting cmap into a new function lcd_logo_set_cmap(), which is implemented in atmel_lcdfb driver and defined as part of common/lcd.c api with a weak dummy version. In the Atmel implementation, ARRAY_SIZE(bmp_logo_palette) is switched for BMP_LOGO_COLORS to avoid having to include bmp_logo_data.h, which would cause a compilation error because the logo data and palette arrays would be defined twice.
This is a step towards cleaning bitmap_plot() of platform specific code.
--- common/lcd.c | 29 ++++++++--------------------- drivers/video/atmel_lcdfb.c | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 21 deletions(-)
common/lcd.c +8 -21
@@ -385,13 +385,13 @@ int lcd_getbgcolor(void)/************************************************************************/ #ifdef CONFIG_LCD_LOGO +__weak void lcd_logo_set_cmap (void ) +{ +} +void bitmap_plot (int x,int y) { -#ifdef CONFIG_ATMEL_LCD -uint *cmap = (uint *)bmp_logo_palette; -#else ushort *cmap = (ushort *)bmp_logo_palette; -#endif ushort i, j;uchar *bmap;uchar *fb;
@@ -417,8 +417,6 @@ void bitmap_plot(int x, int y)*/ #if defined(CONFIG_MPC823 ) cmap = (ushort *) &(cp->lcd_cmap[BMP_LOGO_OFFSET *sizeof (ushort)]); -#elif defined(CONFIG_ATMEL_LCD ) - cmap = (uint *)configuration_get_cmap ();#else cmap =configuration_get_cmap ();#endif
@@ -426,25 +424,14 @@ void bitmap_plot(int x, int y)WATCHDOG_RESET ();/* Set color map */ +#ifdef CONFIG_ATMEL_LCD +lcd_logo_set_cmap (); +#else for (i =0 ; i <ARRAY_SIZE (bmp_logo_palette); ++i) {ushort colreg = bmp_logo_palette[i]; -#ifdef CONFIG_ATMEL_LCD -uint lut_entry; -#ifdef CONFIG_ATMEL_LCD_BGR555 - lut_entry = ((colreg &0x000F ) <<11 ) | - ((colreg &0x00F0 ) <<2 ) | - ((colreg &0x0F00 ) >>7 ); -#else /* CONFIG_ATMEL_LCD_RGB565 */ - lut_entry = ((colreg &0x000F ) <<1 ) | - ((colreg &0x00F0 ) <<3 ) | - ((colreg &0x0F00 ) <<4 ); -#endif - *(cmap +BMP_LOGO_OFFSET ) = lut_entry; - cmap++; -#else /* !CONFIG_ATMEL_LCD */ *cmap++ = colreg; -#endif /* CONFIG_ATMEL_LCD */ } +#endif WATCHDOG_RESET ();
drivers/video/atmel_lcdfb.c +26 -0
@@ -43,6 +43,32 @@ void fb_put_word(uchar **fb, uchar **from) }#endif +#ifdef CONFIG_LCD_LOGO +#include <bmp_logo.h> +void lcd_logo_set_cmap (void ) +{ +int i; +uint lut_entry; +ushort colreg; +uint *cmap = (uint *)configuration_get_cmap (); + +for (i =0 ; i <BMP_LOGO_COLORS ; ++i) { + colreg = bmp_logo_palette[i]; +#ifdef CONFIG_ATMEL_LCD_BGR555 + lut_entry = ((colreg &0x000F ) <<11 ) | + ((colreg &0x00F0 ) <<2 ) | + ((colreg &0x0F00 ) >>7 ); +#else + lut_entry = ((colreg &0x000F ) <<1 ) | + ((colreg &0x00F0 ) <<3 ) | + ((colreg &0x0F00 ) <<4 ); +#endif + *(cmap +BMP_LOGO_OFFSET ) = lut_entry; + cmap++; + } +} +#endif +void lcd_setcolreg (ushort regno,ushort red,ushort green,ushort blue) {#if defined(CONFIG_ATMEL_LCD_BGR555 )
Signature · 1 line
1.9.1
PATCH 5/13 lcd: atmel: introduce lcd_logo_set_cmap
Quoted 10 lines · Nikita Kiryanov
Reduce the
bitmap_plot#ifdef complexity by extracting Atmel specific code for setting cmap into a new functionlcd_logo_set_cmap(), which is implemented inatmel_lcdfbdriver and defined as part ofcommon/lcd.capi with a weak dummy version. In the Atmel implementation,ARRAY_SIZE(bmp_logo_palette)is switched forBMP_LOGO_COLORSto avoid having to includebmp_logo_data.h, which would cause a compilation error because the logo data and palette arrays would be defined twice.This is a step towards cleaning
bitmap_plot()of platform specific code.
Quoted 8 lines
Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 29 ++++++++--------------------- drivers/video/atmel_lcdfb.c | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 21 deletions(-)
PATCH 6/13 lcd: mpc823: move mpc823 specific lcd_logo_set_cmap code to mpc8xx_lcd.c
In reply to Nikita KiryanovReduce the bitmap_plot #ifdef complexity by extracting MPC823 specific code for setting cmap into its own implementation of lcd_logo_set_cmap(), implemented in mpc8xx_lcd.c. In the MPC823 implementation, ARRAY_SIZE(bmp_logo_palette) is switched for BMP_LOGO_COLORS to avoid having to include bmp_logo_data.h, which would cause a compilation error because the logo data and palette arrays would be defined twice.
--- common/lcd.c | 10 +--------- drivers/video/mpc8xx_lcd.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 9 deletions(-)
common/lcd.c +1 -9
@@ -396,10 +396,6 @@ void bitmap_plot(int x, int y)uchar *bmap;uchar *fb;ushort *fb16; -#if defined(CONFIG_MPC823 ) -immap_t *immr = (immap_t *)CONFIG_SYS_IMMR ; -cpm8xx_t *cp = &(immr->im_cpm); -#endif unsigned bpix =NBITS (panel_info.vl_bpix); debug("Logo: width %d height %d colors %d cmap %d\n",
@@ -415,16 +411,12 @@ void bitmap_plot(int x, int y)* cmap was set to the source palette, so no change is done. * This avoids even more ifdefs in the next stanza */ -#if defined(CONFIG_MPC823 ) - cmap = (ushort *) &(cp->lcd_cmap[BMP_LOGO_OFFSET *sizeof (ushort)]); -#else cmap =configuration_get_cmap (); -#endif WATCHDOG_RESET ();/* Set color map */ -#ifdef CONFIG_ATMEL_LCD +#if defined(CONFIG_ATMEL_LCD ) || defined(CONFIG_MPC823 )lcd_logo_set_cmap ();#else for (i =0 ; i <ARRAY_SIZE (bmp_logo_palette); ++i) {
drivers/video/mpc8xx_lcd.c +15 -0
@@ -371,6 +371,21 @@ void fb_put_byte(uchar **fb, uchar **from) }#endif +#ifdef CONFIG_LCD_LOGO +#include <bmp_logo.h> +void lcd_logo_set_cmap (void ) +{ +int i; +ushort *cmap; +immap_t *immr = (immap_t *)CONFIG_SYS_IMMR ; +cpm8xx_t *cp = &(immr->im_cpm); + cmap = (ushort *)&(cp->lcd_cmap[BMP_LOGO_OFFSET *sizeof (ushort)]); + +for (i =0 ; i <BMP_LOGO_COLORS ; ++i) + *cmap++ = bmp_logo_palette[i]; +} +#endif +void lcd_enable (void ) {volatile immap_t *immr = (immap_t *)CONFIG_SYS_IMMR ;
Signature · 1 line
1.9.1
PATCH 6/13 lcd: mpc823: move mpc823 specific lcd_logo_set_cmap code to mpc8xx_lcd.c
Quoted 7 lines · Nikita Kiryanov
Reduce the
bitmap_plot#ifdef complexity by extracting MPC823 specific code for setting cmap into its own implementation oflcd_logo_set_cmap(), implemented inmpc8xx_lcd.c. In the MPC823 implementation,ARRAY_SIZE(bmp_logo_palette)is switched forBMP_LOGO_COLORSto avoid having to includebmp_logo_data.h, which would cause a compilation error because the logo data and palette arrays would be defined twice.
Quoted 7 lines
Signed-off-by Nikita Kiryanov <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 10 +--------- drivers/video/mpc8xx_lcd.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 9 deletions(-)
PATCH 7/13 lcd: logo: move generic cmap setting to lcd_logo_set_cmap()
In reply to Nikita KiryanovGet rid of platform specific #ifdefs in bitmap_plot() by moving the generic case of setting cmap into the weak lcd_logo_set_cmap().
--- common/lcd.c | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-)
common/lcd.c +7 -21
@@ -387,44 +387,30 @@ int lcd_getbgcolor(void)#ifdef CONFIG_LCD_LOGO __weak void lcd_logo_set_cmap (void ) { +int i; +ushort *cmap =configuration_get_cmap (); + +for (i =0 ; i <ARRAY_SIZE (bmp_logo_palette); ++i) + *cmap++ = bmp_logo_palette[i]; }void bitmap_plot (int x,int y) { -ushort *cmap = (ushort *)bmp_logo_palette;ushort i, j;uchar *bmap;uchar *fb;ushort *fb16;unsigned bpix =NBITS (panel_info.vl_bpix); -debug ("Logo: width %d height %d colors %d cmap %d\n" , -BMP_LOGO_WIDTH ,BMP_LOGO_HEIGHT ,BMP_LOGO_COLORS , -ARRAY_SIZE (bmp_logo_palette)); +debug ("Logo: width %d height %d colors %d\n" , +BMP_LOGO_WIDTH ,BMP_LOGO_HEIGHT ,BMP_LOGO_COLORS ); bmap = &bmp_logo_bitmap[0 ]; fb = (uchar *)(lcd_base + y * lcd_line_length + x * bpix /8 );if (bpix <12 ) { -/* Leave room for default color map -* default case: generic system with no cmap (most likely 16bpp) -* cmap was set to the source palette, so no change is done. -* This avoids even more ifdefs in the next stanza -*/ - cmap =configuration_get_cmap (); -WATCHDOG_RESET (); - -/* Set color map */ -#if defined(CONFIG_ATMEL_LCD ) || defined(CONFIG_MPC823 )lcd_logo_set_cmap (); -#else -for (i =0 ; i <ARRAY_SIZE (bmp_logo_palette); ++i) { -ushort colreg = bmp_logo_palette[i]; - *cmap++ = colreg; - } -#endif -WATCHDOG_RESET ();for (i =0 ; i <BMP_LOGO_HEIGHT ; ++i) {
Signature · 1 line
1.9.1
PATCH 7/13 lcd: logo: move generic cmap setting to lcd_logo_set_cmap()
Quoted 10 lines · Nikita Kiryanov
Get rid of platform specific #ifdefs in
bitmap_plot()by moving the generic case of setting cmap into the weaklcd_logo_set_cmap().Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-)
PATCH 8/13 lcd: introduce lcd_set_cmap
In reply to Nikita KiryanovReduce the lcd_display_bitmap #ifdef complexity by extracting Atmel specific code for setting cmap for bitmap images into a new function lcd_set_cmap(). A default version is implemented with the remainder of the code.
--- common/lcd.c | 46 ++++++++++++++++++++++----------------------- drivers/video/atmel_lcdfb.c | 11 +++++++++++ 2 files changed, 33 insertions(+), 24 deletions(-)
common/lcd.c +22 -24
@@ -613,9 +613,27 @@ __weak void fb_put_word(uchar **fb, uchar **from) }#endif /* CONFIG_BMP_16BPP */ +__weak void lcd_set_cmap (bmp_image_t *bmp,unsigned colors) +{ +int i; +bmp_color_table_entry_t cte; +ushort *cmap =configuration_get_cmap (); + +for (i =0 ; i < colors; ++i) { + cte = bmp->color_table[i]; + *cmap = (((cte.red) <<8 ) &0xf800 ) | + (((cte.green) <<3 ) &0x07e0 ) | + (((cte.blue) >>3 ) &0x001f ); +#if defined(CONFIG_MPC823 ) + cmap--; +#else + cmap++; +#endif + } +} +int lcd_display_bitmap (ulong bmp_image,int x,int y) { -ushort *cmap =NULL ;ushort *cmap_base =NULL ;ushort i, j;uchar *fb;
@@ -663,29 +681,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)debug ("Display-bmp: %d x %d with %d colors\n" , (int )width, (int )height, (int )colors); -if (bmp_bpix ==8 ) { - cmap =configuration_get_cmap (); - cmap_base = cmap; - -/* Set color map */ -for (i =0 ; i < colors; ++i) { -bmp_color_table_entry_t cte = bmp->color_table[i]; -#if !defined(CONFIG_ATMEL_LCD ) -ushort colreg = - ( ((cte.red) <<8 ) &0xf800 ) | - ( ((cte.green) <<3 ) &0x07e0 ) | - ( ((cte.blue) >>3 ) &0x001f ) ; - *cmap = colreg; -#if defined(CONFIG_MPC823 ) - cmap--; -#else - cmap++; -#endif -#else /* CONFIG_ATMEL_LCD */ -lcd_setcolreg (i, cte.red, cte.green, cte.blue); -#endif - } - } +if (bmp_bpix ==8 ) +lcd_set_cmap (bmp, colors); padded_width = (width &0x3 ? (width & ~0x3 ) +4 : width);
@@ -706,6 +703,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y)switch (bmp_bpix) {case 1 :/* pass through */ case 8 : { + cmap_base =configuration_get_cmap ();#ifdef CONFIG_LCD_BMP_RLE8 u32 compression =get_unaligned_le32 (&bmp->header.compression);if (compression ==BMP_BI_RLE8 ) {
drivers/video/atmel_lcdfb.c +11 -0
@@ -11,6 +11,7 @@#include <asm/arch/gpio.h> #include <asm/arch/clk.h> #include <lcd.h> +#include <bmp_layout.h> #include <atmel_lcdc.h> /* configurable parameters */
@@ -80,6 +81,16 @@ void lcd_setcolreg(ushort regno, ushort red, ushort green, ushort blue)#endif } +void lcd_set_cmap (bmp_image_t *bmp,unsigned colors) +{ +int i; + +for (i =0 ; i < colors; ++i) { +bmp_color_table_entry_t cte = bmp->color_table[i]; +lcd_setcolreg (i, cte.red, cte.green, cte.blue); + } +} +void lcd_ctrl_init (void *lcdbase) {unsigned long value;
Signature · 1 line
1.9.1
PATCH 8/13 lcd: introduce lcd_set_cmap
Quoted 4 lines · Nikita Kiryanov
Reduce the
lcd_display_bitmap#ifdef complexity by extracting Atmel specific code for setting cmap for bitmap images into a new functionlcd_set_cmap(). A default version is implemented with the remainder of the code.
Quoted 8 lines
Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 46 ++++++++++++++++++++++----------------------- drivers/video/atmel_lcdfb.c | 11 +++++++++++ 2 files changed, 33 insertions(+), 24 deletions(-)
I *think* I understand this...
PATCH 9/13 lcd: remove unused includes
In reply to Nikita KiryanovRemove unused includes.
--- common/lcd.c | 18 ------------------ 1 file changed, 18 deletions(-)
common/lcd.c +0 -18
@@ -16,14 +16,9 @@#include <config.h> #include <common.h> #include <command.h> -#include <stdarg.h> -#include <search.h> #include <env_callback.h> #include <linux/types.h> #include <stdio_dev.h> -#if defined(CONFIG_POST ) -#include <post.h> -#endif #include <lcd.h> #include <watchdog.h> #include <asm/unaligned.h>
@@ -32,19 +27,6 @@#include <asm/unaligned.h> #include <fdt_support.h> -#if defined(CONFIG_CPU_PXA25X ) || defined(CONFIG_CPU_PXA27X ) || \ - defined(CONFIG_CPU_MONAHANS ) -#include <asm/byteorder.h> -#endif - -#if defined(CONFIG_MPC823 ) -#include <lcdvideo.h> -#endif - -#if defined(CONFIG_ATMEL_LCD ) -#include <atmel_lcdc.h> -#endif -#if defined(CONFIG_LCD_DT_SIMPLEFB )#include <libfdt.h> #endif
Signature · 1 line
1.9.1
PATCH 9/13 lcd: remove unused includes
Quoted 9 lines · Nikita Kiryanov
Remove unused includes.
Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 18 ------------------ 1 file changed, 18 deletions(-)
PATCH 10/13 lcd: various cleanups
In reply to Nikita KiryanovThis cleanup mostly focuses on removing unnecessary whitespace and comments which are superfluous and/or do not conform to the coding style.
--- common/lcd.c | 90 +++++++++------------------------------------------ include/lcd.h | 102 ++++++++++++++++++++-------------------------------------- 2 files changed, 49 insertions(+), 143 deletions(-)
common/lcd.c +15 -75
@@ -1,5 +1,5 @@/* -* Common LCD routines for supported CPUs +* Common LCD routines * * (C) Copyright 2001-2002 * Wolfgang Denk, DENX Software Engineering -- wd at denx.de
@@ -7,12 +7,7 @@* SPDX-License-Identifier: GPL-2.0+ */ -/************************************************************************/ -/* ** HEADER FILES */ -/************************************************************************/ -/* #define DEBUG */ -#include <config.h> #include <common.h> #include <command.h>
@@ -26,25 +21,18 @@#include <asm/io.h> #include <asm/unaligned.h> #include <fdt_support.h> +#include <video_font.h> #if defined(CONFIG_LCD_DT_SIMPLEFB )#include <libfdt.h> #endif -/************************************************************************/ -/* ** FONT DATA */ -/************************************************************************/ -#include <video_font.h> /* Get font data, width and height */ - -/************************************************************************/ -/* ** LOGO DATA */ -/************************************************************************/ #ifdef CONFIG_LCD_LOGO -# include <bmp_logo.h> /* Get logo data, width and height */ -# include <bmp_logo_data.h> -# if (CONSOLE_COLOR_WHITE >=BMP_LOGO_OFFSET ) && (LCD_BPP !=LCD_COLOR16 ) -# error Default Color Map overlaps with Logo Color Map -# endif +#include <bmp_logo.h> +#include <bmp_logo_data.h> +#if (CONSOLE_COLOR_WHITE >=BMP_LOGO_OFFSET ) && (LCD_BPP !=LCD_COLOR16 ) +#error Default Color Map overlaps with Logo Color Map +#endif #endif #ifdef CONFIG_SANDBOX
@@ -57,30 +45,23 @@#if (LCD_BPP !=LCD_COLOR8 ) && (LCD_BPP !=LCD_COLOR16 ) && \ (LCD_BPP !=LCD_COLOR32 ) -# error Unsupported LCD BPP. +#error Unsupported LCD BPP.#endif DECLARE_GLOBAL_DATA_PTR ;static int lcd_init (void *lcdbase); -static void *lcd_logo (void ); -static void lcd_setfgcolor (int color);static void lcd_setbgcolor (int color);static int lcd_color_fg;static int lcd_color_bg;int lcd_line_length; -char lcd_is_enabled =0 ; -static void *lcd_base;/* Start of framebuffer memory */ -static char lcd_flush_dcache;/* 1 to flush dcache after each lcd update */ -/************************************************************************/ -/* Flush LCD activity to the caches */ void lcd_sync (void ) {
@@ -110,8 +91,6 @@ void lcd_set_flush_dcache(int flush) lcd_flush_dcache = (flush !=0 ); } -/*----------------------------------------------------------------------*/ -static void lcd_stub_putc (struct stdio_dev *dev,const char c) {lcd_putc (c);
@@ -122,9 +101,7 @@ static void lcd_stub_puts(struct stdio_dev *dev, const char *s)lcd_puts (s); } -/************************************************************************/ -/** Small utility to check that you got the colours right */ -/************************************************************************/ +/* Small utility to check that you got the colours right */ #ifdef LCD_TEST_PATTERN #define N_BLK_VERT 2
@@ -158,10 +135,6 @@ static void test_pattern(void) }#endif /* LCD_TEST_PATTERN */ - -/************************************************************************/ -/* ** GENERIC Initialization Routines */ -/************************************************************************/ /* * With most lcd drivers the line length is set up * by calculating it from panel_info parameters. Some
@@ -181,7 +154,7 @@ int drv_lcd_init(void) lcd_base =map_sysmem (gd->fb_base,0 ); -lcd_init (lcd_base);/* LCD initialization */ +lcd_init (lcd_base);/* Device initialization */ memset (&lcddev,0 ,sizeof (lcddev));
@@ -197,7 +170,6 @@ int drv_lcd_init(void)return (rc ==0 ) ?1 : rc; } -/*----------------------------------------------------------------------*/ void lcd_clear (void ) {short console_rows, console_cols;
@@ -261,20 +233,11 @@ static int do_lcd_clear(cmd_tbl_t *cmdtp, int flag, int argc,lcd_clear ();return 0 ; } - -U_BOOT_CMD ( - cls,1 ,1 , do_lcd_clear, -"clear screen" , -"" -); - -/*----------------------------------------------------------------------*/ +U_BOOT_CMD (cls,1 ,1 , do_lcd_clear,"clear screen" ,"" );static int lcd_init (void *lcdbase) { -/* Initialize the lcd controller */ debug ("[LCD] Initializing LCD frambuffer at %p\n" , lcdbase); -lcd_ctrl_init (lcdbase);/*
@@ -304,10 +267,6 @@ static int lcd_init(void *lcdbase)return 0 ; } - -/************************************************************************/ -/* ** ROM capable initialization part - needed to reserve FB memory */ -/************************************************************************/ /* * This is called early in the system initialization to grab memory * for the LCD controller.
@@ -338,8 +297,6 @@ ulong lcd_setmem(ulong addr)return addr; } -/*----------------------------------------------------------------------*/ -static void lcd_setfgcolor (int color) { lcd_color_fg = color;
@@ -350,8 +307,6 @@ int lcd_getfgcolor(void)return lcd_color_fg; } -/*----------------------------------------------------------------------*/ -static void lcd_setbgcolor (int color) { lcd_color_bg = color;
@@ -362,10 +317,6 @@ int lcd_getbgcolor(void)return lcd_color_bg; } -/************************************************************************/ -/* ** Chipset depending Bitmap / Logo stuff... */ -/************************************************************************/ -#ifdef CONFIG_LCD_LOGO __weak void lcd_logo_set_cmap (void ) {
@@ -379,17 +330,14 @@ __weak void lcd_logo_set_cmap(void)void bitmap_plot (int x,int y) {ushort i, j; -uchar *bmap; -uchar *fb; -ushort *fb16; +uchar *bmap = &bmp_logo_bitmap[0 ];unsigned bpix =NBITS (panel_info.vl_bpix); +uchar *fb = (uchar *)(lcd_base + y * lcd_line_length + x * bpix /8 ); +ushort *fb16;debug ("Logo: width %d height %d colors %d\n" ,BMP_LOGO_WIDTH ,BMP_LOGO_HEIGHT ,BMP_LOGO_COLORS ); - bmap = &bmp_logo_bitmap[0 ]; - fb = (uchar *)(lcd_base + y * lcd_line_length + x * bpix /8 ); -if (bpix <12 ) {WATCHDOG_RESET ();lcd_logo_set_cmap ();
@@ -424,13 +372,7 @@ void bitmap_plot(int x, int y)static inline void bitmap_plot (int x,int y) {}#endif /* CONFIG_LCD_LOGO */ -/*----------------------------------------------------------------------*/ #if defined(CONFIG_CMD_BMP ) || defined(CONFIG_SPLASH_SCREEN ) -/* -* Display the BMP file located@address bmp_image. -* Only uncompressed. -*/ -#ifdef CONFIG_SPLASH_SCREEN_ALIGN #define BMP_ALIGN_CENTER 0x7FFF
@@ -451,9 +393,7 @@ static void splash_align_axis(int *axis, unsigned long panel_size, }#endif -#ifdef CONFIG_LCD_BMP_RLE8 -#define BMP_RLE8_ESCAPE 0#define BMP_RLE8_EOL 0#define BMP_RLE8_EOBMP 1
@@ -683,7 +623,7 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y) (y + height - 1) * lcd_line_length + x * bpix / 8);switch (bmp_bpix) { -case 1 :/* pass through */ +case 1 :case 8 : { cmap_base =configuration_get_cmap ();#ifdef CONFIG_LCD_BMP_RLE8
include/lcd.h +34 -68
169 lines not on this page · raw message
Signature · 1 line
1.9.1
PATCH 10/13 lcd: various cleanups
Quoted 10 lines · Nikita Kiryanov
This cleanup mostly focuses on removing unnecessary whitespace and comments which are superfluous and/or do not conform to the coding style.
Signed-off-by Nikita Kiryanov <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 90 +++++++++------------------------------------------ include/lcd.h | 102 ++++++++++++++++++++-------------------------------------- 2 files changed, 49 insertions(+), 143 deletions(-)
Optional comment below.
Quoted 7 lines
common/lcd.c +0
-0
common/lcd.c __weak void lcd_logo_set_cmap(void) · line 335
Quoted 9 lines
common/lcd.c +2
-3@@ -379,17 +330,14 @@ __weak void lcd_logo_set_cmap(void)void bitmap_plot (int x,int y) {ushort i, j; -uchar *bmap; -uchar *fb; -ushort *fb16; +uchar *bmap = &bmp_logo_bitmap[0 ];unsigned bpix =NBITS (panel_info.vl_bpix); +uchar *fb = (uchar *)(lcd_base + y * lcd_line_length + x * bpix /8 );
I don't thing it's better to attach these to the declaration. Personal preference though, it's up to you.
common/lcd.c __weak void lcd_logo_set_cmap(void) · line 343
Quoted 11 lines
common/lcd.c +1
-3+ushort *fb16;debug ("Logo: width %d height %d colors %d\n" ,BMP_LOGO_WIDTH ,BMP_LOGO_HEIGHT ,BMP_LOGO_COLORS ); - bmap = &bmp_logo_bitmap[0 ]; - fb = (uchar *)(lcd_base + y * lcd_line_length + x * bpix /8 ); -if (bpix <12 ) {WATCHDOG_RESET ();lcd_logo_set_cmap ();
Regards,
Simon
PATCH 11/13 lcd: rename bitmap_plot to better represent its functionality
In reply to Nikita KiryanovThe name "bitmap_plot" is misleading because it implies that this is a generic function capable of dealing with any bitmap, but its implementation only works with the logo data.
Rename the function to better reflect this.
--- common/lcd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
common/lcd.c +3 -3
@@ -327,7 +327,7 @@ __weak void lcd_logo_set_cmap(void) *cmap++ = bmp_logo_palette[i]; } -void bitmap_plot (int x,int y) +void lcd_logo_plot (int x,int y) {ushort i, j;uchar *bmap = &bmp_logo_bitmap[0 ];
@@ -369,7 +369,7 @@ void bitmap_plot(int x, int y) lcd_sync(); }#else -static inline void bitmap_plot (int x,int y) {} +static inline void lcd_logo_plot (int x,int y) {}#endif /* CONFIG_LCD_LOGO */ #if defined(CONFIG_CMD_BMP ) || defined(CONFIG_SPLASH_SCREEN )
@@ -729,7 +729,7 @@ static void *lcd_logo(void) }#endif /* CONFIG_SPLASH_SCREEN */ -bitmap_plot (0 ,0 ); +lcd_logo_plot (0 ,0 );#ifdef CONFIG_LCD_INFO lcd_set_col (LCD_INFO_X /VIDEO_FONT_WIDTH );
Signature · 1 line
1.9.1
PATCH 11/13 lcd: rename bitmap_plot to better represent its functionality
Quoted 4 lines · Nikita Kiryanov
The name "
bitmap_plot" is misleading because it implies that this is a generic function capable of dealing with any bitmap, but its implementation only works with the logo data.
Quoted 8 lines
Rename the function to better reflect this.
Signed-off-by Nikita Kiryanov <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
PATCH 12/13 lcd: dt: extract simplefb support
In reply to Nikita KiryanovWe now have api functions that can support compiling simplefb code as its own module. Since this code is not part of the display functionality, extract it to its own file.
Raspberry Pi config file is updated to compile the new file.
--- board/raspberrypi/rpi/rpi.c | 1 + common/Makefile | 1 + common/lcd.c | 50 -------------------------------------- common/lcd_simplefb.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ include/fdt_simplefb.h | 14 +++++++++++ include/lcd.h | 3 --- 6 files changed, 75 insertions(+), 53 deletions(-) create mode 100644 common/lcd_simplefb.c create mode 100644 include/fdt_simplefb.h
board/raspberrypi/rpi/rpi.c +1 -0
@@ -18,6 +18,7 @@#include <config.h> #include <dm.h> #include <fdt_support.h> +#include <fdt_simplefb.h> #include <lcd.h> #include <mmc.h> #include <asm/gpio.h>
common/Makefile +1 -0
@@ -197,6 +197,7 @@ obj-$(CONFIG_I2C_EDID) += edid.o obj-$(CONFIG_KALLSYMS) += kallsyms.o obj-y += splash.o obj-$(CONFIG_LCD) += lcd.o lcd_console.o +obj-$(CONFIG_LCD_DT_SIMPLEFB) += lcd_simplefb.o obj-$(CONFIG_LYNXKDI) += lynxkdi.o obj-$(CONFIG_MENU) += menu.o obj-$(CONFIG_MODEM_SUPPORT) += modem.o
common/lcd.c +0 -50
61 lines not on this page · raw message
common/lcd_simplefb.c new file +59 -0
59 lines not on this page · raw message
include/fdt_simplefb.h new file +14 -0
@@ -0,0 +1,14 @@ +/* +* Simplefb device tree support +* +* (C) Copyright 2015 +* Stephen Warren <[email protected]> +* +* SPDX-License-Identifier: GPL-2.0+ +*/ + +#ifndef _FDT_SIMPLEFB_H_ +#define _FDT_SIMPLEFB_H_ +int lcd_dt_simplefb_add_node (void *blob); +int lcd_dt_simplefb_enable_existing_node (void *blob); +#endif
include/lcd.h +0 -3
@@ -130,9 +130,6 @@ void lcd_show_board_info(void);/* Return the size of the LCD frame buffer, and the line length */ int lcd_get_size (int *line_length); -int lcd_dt_simplefb_add_node (void *blob); -int lcd_dt_simplefb_enable_existing_node (void *blob); -/* Update the LCD / flush the cache */ void lcd_sync (void );
Signature · 1 line
1.9.1
PATCH 12/13 lcd: dt: extract simplefb support
Quoted 5 lines · Nikita Kiryanov
We now have api functions that can support compiling simplefb code as its own module. Since this code is not part of the display functionality, extract it to its own file.
Raspberry Pi config file is updated to compile the new file.
I don't see any config file updates. I think one isn't needed, since the config file must already have CONFIG_LCD_DT_SIMPLEFB enabled, since the existing code is under that ifdef.
PATCH 12/13 lcd: dt: extract simplefb support
Hi Stephen,
Quoted 8 lines · Stephen Warren
Quoted 5 lines · Nikita Kiryanov
We now have api functions that can support compiling simplefb code as its own module. Since this code is not part of the display functionality, extract it to its own file.
Raspberry Pi config file is updated to compile the new file.
I don't see any config file updates. I think one isn't needed, since the config file must already have
CONFIG_LCD_DT_SIMPLEFBenabled, since the existing code is under that ifdef.
Yes that was a poor choice of words on my part. Should have been something like "...is updated to accommodate the changes". I'll fix it in a V2.
Quoted 3 lines
Acked-by Stephen Warren <[email protected]>
Signature · 2 lines
Regards, Nikita Kiryanov
PATCH 12/13 lcd: dt: extract simplefb support
In reply to Nikita KiryanovQuoted 10 lines · Nikita Kiryanov
We now have api functions that can support compiling simplefb code as its own module. Since this code is not part of the display functionality, extract it to its own file.
Raspberry Pi config file is updated to compile the new file.
Signed-off-by Nikita Kiryanov <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>Cc Stephen Warren <[email protected]>
Could even go in fdtdec.c if you like.
Quoted 10 lines
--- board/raspberrypi/rpi/rpi.c | 1 + common/Makefile | 1 + common/lcd.c | 50 -------------------------------------- common/lcd_simplefb.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ include/fdt_simplefb.h | 14 +++++++++++ include/lcd.h | 3 --- 6 files changed, 75 insertions(+), 53 deletions(-) create mode 100644 common/lcd_simplefb.c create mode 100644 include/fdt_simplefb.h
PATCH 12/13 lcd: dt: extract simplefb support
Hi Simon,
Quoted 7 lines · Simon Glass
Quoted 6 lines · Nikita Kiryanov
We now have api functions that can support compiling simplefb code as its own module. Since this code is not part of the display functionality, extract it to its own file.
Raspberry Pi config file is updated to compile the new file.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Nikita Kiryanov <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>Cc Stephen Warren <[email protected]>Reviewed-by Simon Glass <[email protected]>Could even go in
fdtdec.cif you like.
Hmm... I'm a little torn, but in the long term I think it's better to keep subsystem-specific fdt code in separate files, because otherwise fdtdec.c will eventually grow to enormous proportions.
Quoted 11 lines
Quoted 11 lines
--- board/raspberrypi/rpi/rpi.c | 1 + common/Makefile | 1 + common/lcd.c | 50 -------------------------------------- common/lcd_simplefb.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ include/fdt_simplefb.h | 14 +++++++++++ include/lcd.h | 3 --- 6 files changed, 75 insertions(+), 53 deletions(-)create mode 100644
common/lcd_simplefb.c
create mode 100644include/fdt_simplefb.h
Signature · 2 lines
Regards, Nikita Kiryanov
PATCH 13/13 lcd: split splash code into its own function
In reply to Nikita Kiryanovlcd_logo() currently performs tasks well beyond just displaying the logo. It has code which displays splash image, it has logic which determines when the different display features are displayed, and it is coupled with the lcd console because it holds the responsibility of returning the lcd console base address.
Make lcd_logo() just about the logo by:
* Moving splash image display code into a dedicated function
* Moving the logic regarding when various features are displayed to lcd_clear() (which is arguably not the correct name for housing such code either, but it is currently the most fitting location code wise)
* Move the responsibility of setting the console base address to lcd_clear() too.
--- common/lcd.c | 52 ++++++++++++++++++++++------------------------------ common/splash.c | 16 ++++++++++++++++ include/splash.h | 11 ++++++++++- 3 files changed, 48 insertions(+), 31 deletions(-)
common/lcd.c +22 -30
@@ -46,7 +46,7 @@DECLARE_GLOBAL_DATA_PTR ;static int lcd_init (void *lcdbase); -static void *lcd_logo (void ); +static void lcd_logo (void );static void lcd_setfgcolor (int color);static void lcd_setbgcolor (int color);
@@ -169,6 +169,9 @@ void lcd_clear(void) {short console_rows, console_cols;int bg_color; +char *s; +ulong addr; +static int do_splash =1 ;#if LCD_BPP ==LCD_COLOR8 /* Setting the palette */ lcd_setcolreg (CONSOLE_COLOR_BLACK ,0 ,0 ,0 );
@@ -218,7 +221,23 @@ void lcd_clear(void)#endif console_cols = panel_info.vl_col /VIDEO_FONT_WIDTH ;lcd_init_console (lcd_base, console_rows, console_cols); -lcd_init_console (lcd_logo (), console_rows, console_cols); +if (do_splash) { + s =getenv ("splashimage" ); +if (s) { + do_splash =0 ; + addr =simple_strtoul (s,NULL ,16 ); +if (lcd_splash (addr) ==0 ) { +lcd_sync (); +return ; + } + } + } + +lcd_logo (); +#if defined(CONFIG_LCD_LOGO ) && !defined(CONFIG_LCD_INFO_BELOW_LOGO ) + addr = (ulong )lcd_base +BMP_LOGO_HEIGHT * lcd_line_length; +lcd_init_console ((void *)addr, console_rows, console_cols); +#endif lcd_sync (); }
@@ -701,29 +720,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y) }#endif -static void *lcd_logo (void ) +static void lcd_logo (void ) { -#ifdef CONFIG_SPLASH_SCREEN -char *s; -ulong addr; -static int do_splash =1 ; - -if (do_splash && (s =getenv ("splashimage" )) !=NULL ) { -int x =0 , y =0 ; - do_splash =0 ; - -if (splash_screen_prepare ()) -return (void *)lcd_base; - - addr =simple_strtoul (s,NULL ,16 ); - -splash_get_pos (&x, &y); - -if (bmp_display (addr, x, y) ==0 ) -return (void *)lcd_base; - } -#endif /* CONFIG_SPLASH_SCREEN */ -lcd_logo_plot (0 ,0 );#ifdef CONFIG_LCD_INFO
@@ -731,12 +729,6 @@ static void *lcd_logo(void)lcd_set_row (LCD_INFO_Y /VIDEO_FONT_HEIGHT );lcd_show_board_info ();#endif /* CONFIG_LCD_INFO */ - -#if defined(CONFIG_LCD_LOGO ) && !defined(CONFIG_LCD_INFO_BELOW_LOGO ) -return (void *)((ulong )lcd_base +BMP_LOGO_HEIGHT * lcd_line_length); -#else -return (void *)lcd_base; -#endif /* CONFIG_LCD_LOGO && !defined(CONFIG_LCD_INFO_BELOW_LOGO) */ }#ifdef CONFIG_SPLASHIMAGE_GUARD
common/splash.c +16 -0
@@ -22,6 +22,7 @@#include <common.h> #include <splash.h> +#include <lcd.h> __weak int splash_screen_prepare (void ) {
@@ -50,3 +51,18 @@ void splash_get_pos(int *x, int *y) } }#endif /* CONFIG_SPLASH_SCREEN_ALIGN */ + +#if defined(CONFIG_SPLASH_SCREEN ) && defined(CONFIG_LCD ) +int lcd_splash (ulong addr) +{ +int x =0 , y =0 , ret; + + ret =splash_screen_prepare (); +if (ret) +return ret; + +splash_get_pos (&x, &y); + +return bmp_display (addr, x, y); +} +#endif
include/splash.h +10 -1
@@ -21,7 +21,7 @@#ifndef _SPLASH_H_#define _SPLASH_H_ - +#include <errno.h> int splash_screen_prepare (void );
@@ -31,6 +31,15 @@ void splash_get_pos(int *x, int *y);static inline void splash_get_pos (int *x,int *y) { }#endif +#if defined(CONFIG_SPLASH_SCREEN ) && defined(CONFIG_LCD ) +int lcd_splash (ulong addr); +#else +static inline int lcd_splash (ulong addr) +{ +return -ENOSYS ; +} +#endif +#define BMP_ALIGN_CENTER 0x7FFF#endif
Signature · 1 line
1.9.1
PATCH 13/13 lcd: split splash code into its own function
Hi Nikita,
Quoted 58 lines · Nikita Kiryanov
lcd_logo()currently performs tasks well beyond just displaying the logo. It has code which displays splash image, it has logic which determines when the different display features are displayed, and it is coupled with the lcd console because it holds the responsibility of returning the lcd console base address.Make
lcd_logo()just about the logo by:
* Moving splash image display code into a dedicated function
* Moving the logic regarding when various features are displayed tolcd_clear()(which is arguably not the correct name for housing such code either, but it is currently the most fitting location code wise)
* Move the responsibility of setting the console base address tolcd_clear()too.Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 52 ++++++++++++++++++++++------------------------------ common/splash.c | 16 ++++++++++++++++ include/splash.h | 11 ++++++++++- 3 files changed, 48 insertions(+), 31 deletions(-)common/lcd.c +10
-2@@ -46,7 +46,7 @@DECLARE_GLOBAL_DATA_PTR ;static int lcd_init (void *lcdbase); -static void *lcd_logo (void ); +static void lcd_logo (void );static void lcd_setfgcolor (int color);static void lcd_setbgcolor (int color);@@ -169,6 +169,9 @@ void lcd_clear(void) {short console_rows, console_cols;int bg_color; +char *s; +ulong addr; +static int do_splash =1 ;#if LCD_BPP ==LCD_COLOR8 /* Setting the palette */ lcd_setcolreg (CONSOLE_COLOR_BLACK ,0 ,0 ,0 );@@ -218,7 +221,23 @@ void lcd_clear(void)#endif console_cols = panel_info.vl_col /VIDEO_FONT_WIDTH ;lcd_init_console (lcd_base, console_rows, console_cols); -lcd_init_console (lcd_logo (), console_rows, console_cols); +if (do_splash) { + s =getenv ("splashimage" ); +if (s) { + do_splash =0 ; + addr =simple_strtoul (s,NULL ,16 ); +if (lcd_splash (addr) ==0 ) {
common/lcd.c void lcd_clear(void) · line 239
Quoted 10 lines
common/lcd.c +10
-0+lcd_sync (); +return ; + } + } + } + +lcd_logo (); +#if defined(CONFIG_LCD_LOGO ) && !defined(CONFIG_LCD_INFO_BELOW_LOGO ) + addr = (ulong )lcd_base +BMP_LOGO_HEIGHT * lcd_line_length; +lcd_init_console ((void *)addr, console_rows, console_cols);
I'm just a bit unsure about this - before this function was always called. I have some idea but can you explain why it is now in an #ifidef?
Quoted 109 lines
common/lcd.c +2
-28+#endif lcd_sync (); }@@ -701,29 +720,8 @@ int lcd_display_bitmap(ulong bmp_image, int x, int y) }#endif -static void *lcd_logo (void ) +static void lcd_logo (void ) { -#ifdef CONFIG_SPLASH_SCREEN -char *s; -ulong addr; -static int do_splash =1 ; - -if (do_splash && (s =getenv ("splashimage" )) !=NULL ) { -int x =0 , y =0 ; - do_splash =0 ; - -if (splash_screen_prepare ()) -return (void *)lcd_base; - - addr =simple_strtoul (s,NULL ,16 ); - -splash_get_pos (&x, &y); - -if (bmp_display (addr, x, y) ==0 ) -return (void *)lcd_base; - } -#endif /* CONFIG_SPLASH_SCREEN */ -lcd_logo_plot (0 ,0 );#ifdef CONFIG_LCD_INFO @@ -731,12 +729,6 @@ static void *lcd_logo(void)lcd_set_row (LCD_INFO_Y /VIDEO_FONT_HEIGHT );lcd_show_board_info ();#endif /* CONFIG_LCD_INFO */ - -#if defined(CONFIG_LCD_LOGO ) && !defined(CONFIG_LCD_INFO_BELOW_LOGO ) -return (void *)((ulong )lcd_base +BMP_LOGO_HEIGHT * lcd_line_length); -#else -return (void *)lcd_base; -#endif /* CONFIG_LCD_LOGO && !defined(CONFIG_LCD_INFO_BELOW_LOGO) */ }#ifdef CONFIG_SPLASHIMAGE_GUARD common/splash.c +16
-0@@ -22,6 +22,7 @@#include <common.h> #include <splash.h> +#include <lcd.h> __weak int splash_screen_prepare (void ) {@@ -50,3 +51,18 @@ void splash_get_pos(int *x, int *y) } }#endif /* CONFIG_SPLASH_SCREEN_ALIGN */ + +#if defined(CONFIG_SPLASH_SCREEN ) && defined(CONFIG_LCD ) +int lcd_splash (ulong addr) +{ +int x =0 , y =0 , ret; + + ret =splash_screen_prepare (); +if (ret) +return ret; + +splash_get_pos (&x, &y); + +return bmp_display (addr, x, y); +} +#endif include/splash.h +10
-2@@ -21,7 +21,7 @@#ifndef _SPLASH_H_#define _SPLASH_H_ - +#include <errno.h> int splash_screen_prepare (void );@@ -31,6 +31,15 @@ void splash_get_pos(int *x, int *y);static inline void splash_get_pos (int *x,int *y) { }#endif +#if defined(CONFIG_SPLASH_SCREEN ) && defined(CONFIG_LCD ) +int lcd_splash (ulong addr); +#else +static inline int lcd_splash (ulong addr) +{ +return -ENOSYS ; +} +#endif +#define BMP_ALIGN_CENTER 0x7FFF#endif --
Quoted 2 lines
1.9.1
Regards,
SImon
PATCH 13/13 lcd: split splash code into its own function
Hi Simon,
Quoted 67 lines · Simon Glass
Hi Nikita,
Quoted 64 lines · Nikita Kiryanov
lcd_logo()currently performs tasks well beyond just displaying the logo. It has code which displays splash image, it has logic which determines when the different display features are displayed, and it is coupled with the lcd console because it holds the responsibility of returning the lcd console base address.Make
lcd_logo()just about the logo by:
* Moving splash image display code into a dedicated function
* Moving the logic regarding when various features are displayed tolcd_clear()(which is arguably not the correct name for housing such code either, but it is currently the most fitting location code wise)
* Move the responsibility of setting the console base address tolcd_clear()too.Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 52 ++++++++++++++++++++++------------------------------ common/splash.c | 16 ++++++++++++++++ include/splash.h | 11 ++++++++++- 3 files changed, 48 insertions(+), 31 deletions(-)common/lcd.c +16
-2@@ -46,7 +46,7 @@DECLARE_GLOBAL_DATA_PTR ;static int lcd_init (void *lcdbase); -static void *lcd_logo (void ); +static void lcd_logo (void );static void lcd_setfgcolor (int color);static void lcd_setbgcolor (int color);@@ -169,6 +169,9 @@ void lcd_clear(void) {short console_rows, console_cols;int bg_color; +char *s; +ulong addr; +static int do_splash =1 ;#if LCD_BPP ==LCD_COLOR8 /* Setting the palette */ lcd_setcolreg (CONSOLE_COLOR_BLACK ,0 ,0 ,0 );@@ -218,7 +221,23 @@ void lcd_clear(void)#endif console_cols = panel_info.vl_col /VIDEO_FONT_WIDTH ;lcd_init_console (lcd_base, console_rows, console_cols); -lcd_init_console (lcd_logo (), console_rows, console_cols); +if (do_splash) { + s =getenv ("splashimage" ); +if (s) { + do_splash =0 ; + addr =simple_strtoul (s,NULL ,16 ); +if (lcd_splash (addr) ==0 ) { +lcd_sync (); +return ; + } + } + } +
Quoted 8 lines
common/lcd.c void lcd_clear(void) · line 239
Quoted 4 lines
common/lcd.c +4
-0+lcd_logo (); +#if defined(CONFIG_LCD_LOGO ) && !defined(CONFIG_LCD_INFO_BELOW_LOGO ) + addr = (ulong )lcd_base +BMP_LOGO_HEIGHT * lcd_line_length; +lcd_init_console ((void *)addr, console_rows, console_cols);I'm just a bit unsure about this - before this function was always called. I have some idea but can you explain why it is now in an #ifidef?
The only time when the base address of the console is defined as anything other than lcd_base, is when we get to the very end of the original lcd_logo() and the above #defines apply. In all other cases, the address of lcd_base is used, and that is set as default at the start of lcd_clear().
Signature · 2 lines
Regards, Nikita Kiryanov
PATCH 13/13 lcd: split splash code into its own function
Hi NIkita,
Quoted 86 lines · Nikita Kiryanov
Hi Simon,
Quoted 72 lines · Simon Glass
Hi Nikita,
Quoted 67 lines · Nikita Kiryanov
lcd_logo()currently performs tasks well beyond just displaying the logo. It has code which displays splash image, it has logic which determines when the different display features are displayed, and it is coupled with the lcd console because it holds the responsibility of returning the lcd console base address.Make
lcd_logo()just about the logo by:
* Moving splash image display code into a dedicated function
* Moving the logic regarding when various features are displayed tolcd_clear()(which is arguably not the correct name for housing such code either, but it is currently the most fitting location code wise)
* Move the responsibility of setting the console base address tolcd_clear()too.Signed-off-by Nikita Kiryanov <[email protected]>Cc Bo Shen <[email protected]>Cc Simon Glass <[email protected]>Cc Anatolij Gustschin <[email protected]>--- common/lcd.c | 52 ++++++++++++++++++++++------------------------------ common/splash.c | 16 ++++++++++++++++ include/splash.h | 11 ++++++++++- 3 files changed, 48 insertions(+), 31 deletions(-)common/lcd.c +17
-2@@ -46,7 +46,7 @@DECLARE_GLOBAL_DATA_PTR ;static int lcd_init (void *lcdbase); -static void *lcd_logo (void ); +static void lcd_logo (void );static void lcd_setfgcolor (int color);static void lcd_setbgcolor (int color);@@ -169,6 +169,9 @@ void lcd_clear(void) {short console_rows, console_cols;int bg_color; +char *s; +ulong addr; +static int do_splash =1 ;#if LCD_BPP ==LCD_COLOR8 /* Setting the palette */ lcd_setcolreg (CONSOLE_COLOR_BLACK ,0 ,0 ,0 );@@ -218,7 +221,23 @@ void lcd_clear(void)#endif console_cols = panel_info.vl_col /VIDEO_FONT_WIDTH ;lcd_init_console (lcd_base, console_rows, console_cols); -lcd_init_console (lcd_logo (), console_rows, console_cols); +if (do_splash) { + s =getenv ("splashimage" ); +if (s) { + do_splash =0 ; + addr =simple_strtoul (s,NULL ,16 ); +if (lcd_splash (addr) ==0 ) { +lcd_sync (); +return ; + } + } + } + +lcd_logo ();Quoted 8 lines
common/lcd.c void lcd_clear(void) · line 239
Quoted 3 lines
common/lcd.c +3
-0+#if defined(CONFIG_LCD_LOGO ) && !defined(CONFIG_LCD_INFO_BELOW_LOGO ) + addr = (ulong )lcd_base +BMP_LOGO_HEIGHT * lcd_line_length; +lcd_init_console ((void *)addr, console_rows, console_cols);I'm just a bit unsure about this - before this function was always called. I have some idea but can you explain why it is now in an #ifidef?
Quoted 6 lines
The only time when the base address of the console is defined as anything other
thanlcd_base, is when we get to the very end of the originallcd_logo()and the above #defines apply. In all other cases, the address oflcd_baseis used,
and that is set as default at the start oflcd_clear().
OK I see, thanks.
Regards,
Simon