diff mbox

[U-Boot,V3,3/4] video: Modify exynos_fimd driver to support LCD console

Message ID 1356006906-31510-4-git-send-email-ajaykumar.rs@samsung.com
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Commit Message

Ajay Kumar Dec. 20, 2012, 12:35 p.m. UTC 
Currently, exynos FIMD driver is being used to support only TIZEN LOGOs.
In order to get LCD console, we need to enable half word swap feature
of FIMD and use 16 BPP.
LCD console and proprietary Logo cannot be used simultaneously.
You should define CONFIG_EXYNOS_LOGO for proprietary Logo, and if
CONFIG_EXYNOS_LOGO is not defined you get output console on LCD.
CONFIG_EXYNOS_LOGO is added to Trats configuration to keep
existing logo feature intact in Trats.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
---
 drivers/video/exynos_fb.c   |    5 ++++-
 drivers/video/exynos_fimd.c |   10 ++++++++--
 include/configs/trats.h     |    1 +
 3 files changed, 13 insertions(+), 3 deletions(-)

Comments

Simon Glass Dec. 20, 2012, 8:46 p.m. UTC | #1 
Hi,

On Thu, Dec 20, 2012 at 4:35 AM, Ajay Kumar <ajaykumar.rs@samsung.com>wrote:

> Currently, exynos FIMD driver is being used to support only TIZEN LOGOs.
> In order to get LCD console, we need to enable half word swap feature
> of FIMD and use 16 BPP.
> LCD console and proprietary Logo cannot be used simultaneously.
> You should define CONFIG_EXYNOS_LOGO for proprietary Logo, and if
> CONFIG_EXYNOS_LOGO is not defined you get output console on LCD.
> CONFIG_EXYNOS_LOGO is added to Trats configuration to keep
> existing logo feature intact in Trats.
>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>

There is nothing in the README, but I suppose this is a chip-specific
patch, so fair enough. But see below.

Acked-by: Simon Glass <sjg@chomium.org>



> ---
>  drivers/video/exynos_fb.c   |    5 ++++-
>  drivers/video/exynos_fimd.c |   10 ++++++++--
>  include/configs/trats.h     |    1 +
>  3 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c
> index d9a3f9a..c111a09 100644
> --- a/drivers/video/exynos_fb.c
> +++ b/drivers/video/exynos_fb.c
> @@ -65,6 +65,7 @@ static void exynos_lcd_init(vidinfo_t *vid)
>         exynos_fimd_lcd_init(vid);
>  }
>
> +#ifdef CONFIG_EXYNOS_LOGO
>

Do you actually need this #ifdef, and the one below it? You already have
panel_info.logo_on. I suppose you could do, at the top of the file:

+#ifdef CONFIG_EXYNOS_LOGO
#define panal_exynos_logo()  panel_info.logo_on
#else
#define panal_exynos_logo() 0
#endif



>  static void draw_logo(void)
>  {
>         int x, y;
> @@ -87,6 +88,7 @@ static void draw_logo(void)
>         addr = panel_info.logo_addr;
>         bmp_display(addr, x, y);
>  }
> +#endif
>
>  static void lcd_panel_on(vidinfo_t *vid)
>  {
> @@ -142,12 +144,13 @@ void lcd_ctrl_init(void *lcdbase)
>
>  void lcd_enable(void)
>  {
> +#ifdef CONFIG_EXYNOS_LOGO
>         if (panel_info.logo_on) {
>

Then could do 'if (panal_exynos_logo())' here and rermove the #ifdefs. Same
below. #ifdefs are considered bad because they create new code paths that
might not always be compiled.

                memset(lcd_base, 0, panel_width * panel_height *
>                                 (NBITS(panel_info.vl_bpix) >> 3));
>                 draw_logo();
>         }
> -
> +#endif
>         lcd_panel_on(&panel_info);
>  }
>
> diff --git a/drivers/video/exynos_fimd.c b/drivers/video/exynos_fimd.c
> index 06eae2e..f2e4c27 100644
> --- a/drivers/video/exynos_fimd.c
> +++ b/drivers/video/exynos_fimd.c
> @@ -88,14 +88,20 @@ static void exynos_fimd_set_par(unsigned int win_id)
>         /* DATAPATH is DMA */
>         cfg |= EXYNOS_WINCON_DATAPATH_DMA;
>
> -       /* bpp is 32 */
> +#ifdef CONFIG_EXYNOS_LOGO /* To get proprietary LOGO */
>         cfg |= EXYNOS_WINCON_WSWP_ENABLE;
> +#else  /* To get output console on LCD */
> +       cfg |= EXYNOS_WINCON_HAWSWP_ENABLE;
> +#endif
>
>         /* dma burst is 16 */
>         cfg |= EXYNOS_WINCON_BURSTLEN_16WORD;
>
> -       /* pixel format is unpacked RGB888 */
> +#ifdef CONFIG_EXYNOS_LOGO /* To get proprietary LOGO */
>         cfg |= EXYNOS_WINCON_BPPMODE_24BPP_888;
> +#else  /* To get output console on LCD */
> +       cfg |= EXYNOS_WINCON_BPPMODE_16BPP_565;
> +#endif
>
>         writel(cfg, (unsigned int)&fimd_ctrl->wincon0 +
>                         EXYNOS_WINCON(win_id));
> diff --git a/include/configs/trats.h b/include/configs/trats.h
> index a24e945..1573573 100644
> --- a/include/configs/trats.h
> +++ b/include/configs/trats.h
> @@ -252,6 +252,7 @@
>  #define CONFIG_EXYNOS_FB
>  #define CONFIG_LCD
>  #define CONFIG_CMD_BMP
> +#define CONFIG_EXYNOS_LOGO
>  #define CONFIG_BMP_32BPP
>  #define CONFIG_FB_ADDR         0x52504000
>  #define CONFIG_S6E8AX0
> --
> 1.7.1
>
>
Donghwa Lee Dec. 21, 2012, 1:53 a.m. UTC | #2 
On 2012년 12월 20일 21:35, Ajay Kumar wrote:
> Currently, exynos FIMD driver is being used to support only TIZEN LOGOs.
> In order to get LCD console, we need to enable half word swap feature
> of FIMD and use 16 BPP.
> LCD console and proprietary Logo cannot be used simultaneously.
> You should define CONFIG_EXYNOS_LOGO for proprietary Logo, and if
> CONFIG_EXYNOS_LOGO is not defined you get output console on LCD.
> CONFIG_EXYNOS_LOGO is added to Trats configuration to keep
> existing logo feature intact in Trats.
>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> ---
>   drivers/video/exynos_fb.c   |    5 ++++-
>   drivers/video/exynos_fimd.c |   10 ++++++++--
>   include/configs/trats.h     |    1 +
>   3 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c
> index d9a3f9a..c111a09 100644
> --- a/drivers/video/exynos_fb.c
> +++ b/drivers/video/exynos_fb.c
> @@ -65,6 +65,7 @@ static void exynos_lcd_init(vidinfo_t *vid)
>   	exynos_fimd_lcd_init(vid);
>   }
>   
> +#ifdef CONFIG_EXYNOS_LOGO
>   static void draw_logo(void)
>   {
>   	int x, y;
> @@ -87,6 +88,7 @@ static void draw_logo(void)
>   	addr = panel_info.logo_addr;
>   	bmp_display(addr, x, y);
>   }
> +#endif
>   
>   static void lcd_panel_on(vidinfo_t *vid)
>   {
> @@ -142,12 +144,13 @@ void lcd_ctrl_init(void *lcdbase)
>   
>   void lcd_enable(void)
>   {
> +#ifdef CONFIG_EXYNOS_LOGO
>   	if (panel_info.logo_on) {
>   		memset(lcd_base, 0, panel_width * panel_height *
>   				(NBITS(panel_info.vl_bpix) >> 3));
>   		draw_logo();
>   	}
> -
> +#endif
>   	lcd_panel_on(&panel_info);
>   }
>   
> diff --git a/drivers/video/exynos_fimd.c b/drivers/video/exynos_fimd.c
> index 06eae2e..f2e4c27 100644
> --- a/drivers/video/exynos_fimd.c
> +++ b/drivers/video/exynos_fimd.c
> @@ -88,14 +88,20 @@ static void exynos_fimd_set_par(unsigned int win_id)
>   	/* DATAPATH is DMA */
>   	cfg |= EXYNOS_WINCON_DATAPATH_DMA;
>   
> -	/* bpp is 32 */
> +#ifdef CONFIG_EXYNOS_LOGO /* To get proprietary LOGO */
>   	cfg |= EXYNOS_WINCON_WSWP_ENABLE;
> +#else	/* To get output console on LCD */
> +	cfg |= EXYNOS_WINCON_HAWSWP_ENABLE;
> +#endif
>   
>   	/* dma burst is 16 */
>   	cfg |= EXYNOS_WINCON_BURSTLEN_16WORD;
>   
> -	/* pixel format is unpacked RGB888 */
> +#ifdef CONFIG_EXYNOS_LOGO /* To get proprietary LOGO */
>   	cfg |= EXYNOS_WINCON_BPPMODE_24BPP_888;
> +#else	/* To get output console on LCD */
> +	cfg |= EXYNOS_WINCON_BPPMODE_16BPP_565;
> +#endif
>   
>   	writel(cfg, (unsigned int)&fimd_ctrl->wincon0 +
>   			EXYNOS_WINCON(win_id));
> diff --git a/include/configs/trats.h b/include/configs/trats.h
> index a24e945..1573573 100644
> --- a/include/configs/trats.h
> +++ b/include/configs/trats.h
> @@ -252,6 +252,7 @@
>   #define CONFIG_EXYNOS_FB
>   #define CONFIG_LCD
>   #define CONFIG_CMD_BMP
> +#define CONFIG_EXYNOS_LOGO
>   #define CONFIG_BMP_32BPP
>   #define CONFIG_FB_ADDR		0x52504000
>   #define CONFIG_S6E8AX0
Hi,

How about use 'if (vid->logo_on)' instead of #ifdef CONFIG_EXYNOS_LOGO?
In the vidinfo_t structure, 'logo_on' flag already exist.

Thank you,
Donghwa Lee
Minkyu Kang Dec. 21, 2012, 2:59 a.m. UTC | #3 
Dear Ajay,

On 21/12/12 10:53, Donghwa Lee wrote:
> On 2012년 12월 20일 21:35, Ajay Kumar wrote:
>> Currently, exynos FIMD driver is being used to support only TIZEN LOGOs.
>> In order to get LCD console, we need to enable half word swap feature
>> of FIMD and use 16 BPP.
>> LCD console and proprietary Logo cannot be used simultaneously.
>> You should define CONFIG_EXYNOS_LOGO for proprietary Logo, and if
>> CONFIG_EXYNOS_LOGO is not defined you get output console on LCD.
>> CONFIG_EXYNOS_LOGO is added to Trats configuration to keep
>> existing logo feature intact in Trats.
>>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>> ---
>>   drivers/video/exynos_fb.c   |    5 ++++-
>>   drivers/video/exynos_fimd.c |   10 ++++++++--
>>   include/configs/trats.h     |    1 +
>>   3 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c
>> index d9a3f9a..c111a09 100644
>> --- a/drivers/video/exynos_fb.c
>> +++ b/drivers/video/exynos_fb.c
>> @@ -65,6 +65,7 @@ static void exynos_lcd_init(vidinfo_t *vid)
>>       exynos_fimd_lcd_init(vid);
>>   }
>>   +#ifdef CONFIG_EXYNOS_LOGO
>>   static void draw_logo(void)
>>   {
>>       int x, y;
>> @@ -87,6 +88,7 @@ static void draw_logo(void)
>>       addr = panel_info.logo_addr;
>>       bmp_display(addr, x, y);
>>   }
>> +#endif
>>     static void lcd_panel_on(vidinfo_t *vid)
>>   {
>> @@ -142,12 +144,13 @@ void lcd_ctrl_init(void *lcdbase)
>>     void lcd_enable(void)
>>   {
>> +#ifdef CONFIG_EXYNOS_LOGO
>>       if (panel_info.logo_on) {
>>           memset(lcd_base, 0, panel_width * panel_height *
>>                   (NBITS(panel_info.vl_bpix) >> 3));
>>           draw_logo();
>>       }
>> -
>> +#endif
>>       lcd_panel_on(&panel_info);
>>   }
>>   diff --git a/drivers/video/exynos_fimd.c b/drivers/video/exynos_fimd.c
>> index 06eae2e..f2e4c27 100644
>> --- a/drivers/video/exynos_fimd.c
>> +++ b/drivers/video/exynos_fimd.c
>> @@ -88,14 +88,20 @@ static void exynos_fimd_set_par(unsigned int win_id)
>>       /* DATAPATH is DMA */
>>       cfg |= EXYNOS_WINCON_DATAPATH_DMA;
>>   -    /* bpp is 32 */
>> +#ifdef CONFIG_EXYNOS_LOGO /* To get proprietary LOGO */
>>       cfg |= EXYNOS_WINCON_WSWP_ENABLE;
>> +#else    /* To get output console on LCD */
>> +    cfg |= EXYNOS_WINCON_HAWSWP_ENABLE;
>> +#endif
>>         /* dma burst is 16 */
>>       cfg |= EXYNOS_WINCON_BURSTLEN_16WORD;
>>   -    /* pixel format is unpacked RGB888 */
>> +#ifdef CONFIG_EXYNOS_LOGO /* To get proprietary LOGO */
>>       cfg |= EXYNOS_WINCON_BPPMODE_24BPP_888;
>> +#else    /* To get output console on LCD */
>> +    cfg |= EXYNOS_WINCON_BPPMODE_16BPP_565;
>> +#endif
>>         writel(cfg, (unsigned int)&fimd_ctrl->wincon0 +
>>               EXYNOS_WINCON(win_id));
>> diff --git a/include/configs/trats.h b/include/configs/trats.h
>> index a24e945..1573573 100644
>> --- a/include/configs/trats.h
>> +++ b/include/configs/trats.h
>> @@ -252,6 +252,7 @@
>>   #define CONFIG_EXYNOS_FB
>>   #define CONFIG_LCD
>>   #define CONFIG_CMD_BMP
>> +#define CONFIG_EXYNOS_LOGO
>>   #define CONFIG_BMP_32BPP
>>   #define CONFIG_FB_ADDR        0x52504000
>>   #define CONFIG_S6E8AX0
> Hi,
> 
> How about use 'if (vid->logo_on)' instead of #ifdef CONFIG_EXYNOS_LOGO?
> In the vidinfo_t structure, 'logo_on' flag already exist.

I agreed with Donghwa.
Ajay, please check it.

> 
> Thank you,
> Donghwa Lee
> 

Thanks.
Minkyu Kang.
diff mbox

Patch

diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c
index d9a3f9a..c111a09 100644
--- a/drivers/video/exynos_fb.c
+++ b/drivers/video/exynos_fb.c
@@ -65,6 +65,7 @@  static void exynos_lcd_init(vidinfo_t *vid)
 	exynos_fimd_lcd_init(vid);
 }
 
+#ifdef CONFIG_EXYNOS_LOGO
 static void draw_logo(void)
 {
 	int x, y;
@@ -87,6 +88,7 @@  static void draw_logo(void)
 	addr = panel_info.logo_addr;
 	bmp_display(addr, x, y);
 }
+#endif
 
 static void lcd_panel_on(vidinfo_t *vid)
 {
@@ -142,12 +144,13 @@  void lcd_ctrl_init(void *lcdbase)
 
 void lcd_enable(void)
 {
+#ifdef CONFIG_EXYNOS_LOGO
 	if (panel_info.logo_on) {
 		memset(lcd_base, 0, panel_width * panel_height *
 				(NBITS(panel_info.vl_bpix) >> 3));
 		draw_logo();
 	}
-
+#endif
 	lcd_panel_on(&panel_info);
 }
 
diff --git a/drivers/video/exynos_fimd.c b/drivers/video/exynos_fimd.c
index 06eae2e..f2e4c27 100644
--- a/drivers/video/exynos_fimd.c
+++ b/drivers/video/exynos_fimd.c
@@ -88,14 +88,20 @@  static void exynos_fimd_set_par(unsigned int win_id)
 	/* DATAPATH is DMA */
 	cfg |= EXYNOS_WINCON_DATAPATH_DMA;
 
-	/* bpp is 32 */
+#ifdef CONFIG_EXYNOS_LOGO /* To get proprietary LOGO */
 	cfg |= EXYNOS_WINCON_WSWP_ENABLE;
+#else	/* To get output console on LCD */
+	cfg |= EXYNOS_WINCON_HAWSWP_ENABLE;
+#endif
 
 	/* dma burst is 16 */
 	cfg |= EXYNOS_WINCON_BURSTLEN_16WORD;
 
-	/* pixel format is unpacked RGB888 */
+#ifdef CONFIG_EXYNOS_LOGO /* To get proprietary LOGO */
 	cfg |= EXYNOS_WINCON_BPPMODE_24BPP_888;
+#else	/* To get output console on LCD */
+	cfg |= EXYNOS_WINCON_BPPMODE_16BPP_565;
+#endif
 
 	writel(cfg, (unsigned int)&fimd_ctrl->wincon0 +
 			EXYNOS_WINCON(win_id));
diff --git a/include/configs/trats.h b/include/configs/trats.h
index a24e945..1573573 100644
--- a/include/configs/trats.h
+++ b/include/configs/trats.h
@@ -252,6 +252,7 @@ 
 #define CONFIG_EXYNOS_FB
 #define CONFIG_LCD
 #define CONFIG_CMD_BMP
+#define CONFIG_EXYNOS_LOGO
 #define CONFIG_BMP_32BPP
 #define CONFIG_FB_ADDR		0x52504000
 #define CONFIG_S6E8AX0