ARM: EXYNOS: add data structure for EXYNOS display driver
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v3 2/4 ARM: EXYNOS: add data structure for EXYNOS display driver
add vidinfo data structure for EXYNOS display driver
--- include/lcd.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 63 insertions(+), 0 deletions(-)
include/lcd.h +63 -0
@@ -56,6 +56,11 @@ extern void lcd_initcolregs (void);/* gunzip_bmp used if CONFIG_VIDEO_BMP_GZIP */ extern struct bmp_image *gunzip_bmp (unsigned long addr,unsigned long *lenp); +enum { +FIMD_RGB_INTERFACE =1 , +FIMD_CPU_INTERFACE =2 , +}; +#if definedCONFIG_MPC823 /* * LCD controller stucture for MPC823 CPU
@@ -183,6 +188,63 @@ typedef struct vidinfo { u_long mmio; /* Memory mapped registers */ } vidinfo_t; +#elif defined(CONFIG_EXYNOS_FB) + +typedef struct vidinfo { + ushort vl_col; /* Number of columns (i.e. 640) */ + ushort vl_row; /* Number of rows (i.e. 480) */ + ushort vl_width; /* Width of display area in millimeters */ + ushort vl_height; /* Height of display area in millimeters */ + + /* LCD configuration register */ + u_char vl_freq; /* Frequency */ + u_char vl_clkp; /* Clock polarity */ + u_char vl_oep; /* Output Enable polarity */ + u_char vl_hsp; /* Horizontal Sync polarity */ + u_char vl_vsp; /* Vertical Sync polarity */ + u_char vl_dp; /* Data polarity */ + u_char vl_bpix; /* Bits per pixel, 0 = 1, 1 = 2, 2 = 4, 3 = 8, 4 = 16 */ + + /* Horizontal control register. Timing from data sheet */ + u_char vl_hspw; /* Horz sync pulse width */ + u_char vl_hfpd; /* Wait before of line */ + u_char vl_hbpd; /* Wait end of line */ + + /* Vertical control register. */ + u_char vl_vspw; /* Vertical sync pulse width */ + u_char vl_vfpd; /* Wait before of frame */ + u_char vl_vbpd; /* Wait end of frame */ + u_char vl_cmd_allow_len; /* Wait end of frame */ + + void (*cfg_gpio)(void); + void (*backlight_on)(unsigned int onoff); + void (*reset_lcd)(void); + void (*lcd_power_on)(void); + void (*cfg_ldo)(void); + void (*enable_ldo)(unsigned int onoff); + void (*mipi_power)(void); + void (*backlight_reset)(void); + + unsigned int win_id; + unsigned int init_delay; + unsigned int power_on_delay; + unsigned int reset_delay; + unsigned int interface_mode; + unsigned int cs_setup; + unsigned int wr_setup; + unsigned int wr_act; + unsigned int wr_hold; + + /* parent clock name(MPLL, EPLL or VPLL) */ + unsigned int pclk_name; + /* ratio value for source clock from parent clock. */ + unsigned int sclk_div; + + unsigned int lcd_rotate; + unsigned int dual_lcd_enabled; + +} vidinfo_t; + #else typedef struct vidinfo {
@@ -213,6 +275,7 @@ void lcd_puts (const char *s);void lcd_printf (const char *fmt, ...);void lcd_clear (void );int lcd_display_bitmap (ulong bmp_image,int x,int y); +void init_panel_info (vidinfo_t *vid);/* Allow boards to customize the information displayed */ void lcd_show_board_info (void );
Signature · 1 line
1.7.4.1
PATCH v3 2/4 ARM: EXYNOS: add data structure for EXYNOS display driver
Hi!
Sorry for not looking at this earlier. The patch looks pretty good, but there are some style issues, please see some comments below.
Also the patch subject should reflect on which subsystem the patch is doing the changes, so I would suggest using the patch subject like "LCD: add data structure for EXYNOS display driver"
Quoted 14 lines · On Mon, 02 Apr 2012 17:39:24 +0900 Donghwa Lee <[email protected]> wrote:
add vidinfo data structure for EXYNOS display driver
Signed-off-by Donghwa Lee <[email protected]>Signed-off-by Inki Dae <[email protected]>Signed-off-by Kyungmin Park <[email protected]>--- include/lcd.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 63 insertions(+), 0 deletions(-)include/lcd.h +0
-0
include/lcd.h extern void lcd_initcolregs (void); · line 62
Quoted 7 lines
include/lcd.h +4
-0@@ -56,6 +56,11 @@ extern void lcd_initcolregs (void);/* gunzip_bmp used if CONFIG_VIDEO_BMP_GZIP */ extern struct bmp_image *gunzip_bmp (unsigned long addr,unsigned long *lenp); +enum { +FIMD_RGB_INTERFACE =1 , +FIMD_CPU_INTERFACE =2 , +};
This is EXYNOS specific so please move it under
#elif defined(CONFIG_EXYNOS_FB) line.
...
include/lcd.h typedef struct vidinfo { · line 206
Quoted 8 lines
include/lcd.h +8
-0+/* LCD configuration register */ +u_char vl_freq;/* Frequency */ +u_char vl_clkp;/* Clock polarity */ +u_char vl_oep;/* Output Enable polarity */ +u_char vl_hsp;/* Horizontal Sync polarity */ +u_char vl_vsp;/* Vertical Sync polarity */ +u_char vl_dp;/* Data polarity */ +u_char vl_bpix;/* Bits per pixel, 0 = 1, 1 = 2, 2 = 4, 3 = 8, 4 = 16 */
The above line is too long, we have 80 chars/line limit. I know there are other bad examples in this file, but please change to
u_char vl_bpix; /* Bits per pixel, bpp = pow(2, vl_bpix) */
...
include/lcd.h void lcd_puts (const char *s); · line 278
Quoted 5 lines
include/lcd.h +1
-0@@ -213,6 +275,7 @@ void lcd_puts (const char *s);void lcd_printf (const char *fmt, ...);void lcd_clear (void );int lcd_display_bitmap (ulong bmp_image,int x,int y); +void init_panel_info (vidinfo_t *vid);
Please remove space between function name and open parenthesis '('.
There are other examples with spaces between function names and '(' in this file, I'll fix them later.
Btw, where is init_panel_info() defined? I only see that it is used in drivers/video/exynos_fb.c, but do not see the function code. So the driver cannot be build at all. How did you test it?
Also try to check your patches with 'tools/checkpatch.pl'. It will warn you about such style issues. Some of the warnings will be Linux style specific, these can be disabled using "--ignore=" option, e.g. using --ignore=NEW_TYPEDEFS would be appropriate for checking this patch.
Thanks,
Anatolij
PATCH v3 2/4 ARM: EXYNOS: add data structure for EXYNOS display driver
Hi,
Thank you for your comments.
Quoted 9 lines · Anatolij Gustschin
Hi!
Sorry for not looking at this earlier. The patch looks pretty good, but there are some style issues, please see some comments below.
Also the patch subject should reflect on which subsystem the patch is doing the changes, so I would suggest using the patch subject like "LCD: add data structure for EXYNOS display driver"
Ok, I will change the title.
Quoted 29 lines
Quoted 14 lines · On Mon, 02 Apr 2012 17:39:24 +0900 Donghwa Lee <[email protected]> wrote:
add vidinfo data structure for EXYNOS display driver
Signed-off-by Donghwa Lee <[email protected]>Signed-off-by Inki Dae <[email protected]>Signed-off-by Kyungmin Park <[email protected]>--- include/lcd.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 63 insertions(+), 0 deletions(-)include/lcd.h +0
-0include/lcd.h extern void lcd_initcolregs (void); · line 62
Quoted 7 lines
include/lcd.h +4
-0@@ -56,6 +56,11 @@ extern void lcd_initcolregs (void);/* gunzip_bmp used if CONFIG_VIDEO_BMP_GZIP */ extern struct bmp_image *gunzip_bmp (unsigned long addr,unsigned long *lenp); +enum { +FIMD_RGB_INTERFACE =1 , +FIMD_CPU_INTERFACE =2 , +};This is EXYNOS specific so please move it under
#elifdefined(CONFIG_EXYNOS_FB)line.
Quoted 1 line
...
Ok, I will move it.
Quoted 30 lines
include/lcd.h typedef struct vidinfo { · line 206
Quoted 8 lines
include/lcd.h +8
-0+/* LCD configuration register */ +u_char vl_freq;/* Frequency */ +u_char vl_clkp;/* Clock polarity */ +u_char vl_oep;/* Output Enable polarity */ +u_char vl_hsp;/* Horizontal Sync polarity */ +u_char vl_vsp;/* Vertical Sync polarity */ +u_char vl_dp;/* Data polarity */ +u_char vl_bpix;/* Bits per pixel, 0 = 1, 1 = 2, 2 = 4, 3 = 8, 4 = 16 */ The above line is too long, we have 80 chars/line limit. I know there are other bad examples in this file, but please change to
u_char vl_bpix; /* Bits per pixel, bpp = pow(2, vl_bpix) */...
include/lcd.h void lcd_puts (const char *s); · line 278
Quoted 5 lines
include/lcd.h +1
-0@@ -213,6 +275,7 @@ void lcd_puts (const char *s);void lcd_printf (const char *fmt, ...);void lcd_clear (void );int lcd_display_bitmap (ulong bmp_image,int x,int y); +void init_panel_info (vidinfo_t *vid);Please remove space between function name and open parenthesis '('.
There are other examples with spaces between function names and '(' in this file, I'll fix them later.
Btw, where is
init_panel_info()defined? I only see that it is used indrivers/video/exynos_fb.c, but do not see the function code. So the driver cannot be build at all. How did you test it?
Quoted 6 lines
Also try to check your patches with '
tools/checkpatch.pl'. It will warn you about such style issues. Some of the warnings will be Linux style specific, these can be disabled using "--ignore=" option, e.g. using --ignore=NEW_TYPEDEFS would be appropriate for checking this patch.
This function is used in TRATS board file to get vidinfo data that sets up in board file, Although I had already check by using checkpatch.pl, maybe some file was missed. I will send patch next version.
Thank you,
Donghwa Lee