[U-Boot] [PATCH 2/2] EXYNOS: draw 32bpp bitmap TIZEN logo

[Donghwa Lee <dh09.lee@samsung.com>]

Thank you for your comment.

On 23 April 2012 18:00, Minkyu Kang wrote:

> On 23 April 2012 10:47, Donghwa Lee <dh09.lee@samsung.com> wrote:
>> This patches support drawing 32bpp bitmap TIZEN logo in exynos fb.
>> asm/arch/trats_logo.h data is compressed by zip and decomressed at
>> the exynos fb driver.
>>
>> And vl_bpix of vidinfo_t is changed proper value for u-boot.
>>
>> Signed-off-by: Donghwa Lee <dh09.lee@samsung.com>
>> Signed-off-by: Kyungmin park <kyungmin.park@samsung.com>
>> ---
>>  arch/arm/include/asm/arch-exynos/trats_logo.h | 5070 +++++++++++++++++++++++++
>>  board/samsung/trats/trats.c                   |    2 +-
>>  drivers/video/exynos_fb.c                     |   40 +-
>>  drivers/video/exynos_fimd.c                   |    6 +-
>>  include/configs/trats.h                       |    4 +-
>>  5 files changed, 5116 insertions(+), 6 deletions(-)
>>  create mode 100644 arch/arm/include/asm/arch-exynos/trats_logo.h
>>
>> diff --git a/arch/arm/include/asm/arch-exynos/trats_logo.h b/arch/arm/include/asm/arch-exynos/trats_logo.h
>> new file mode 100644
>> index 0000000..a595abd
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-exynos/trats_logo.h
>> @@ -0,0 +1,5070 @@
>> +/*
>> + * (C) Copyright 2012 Samsung Electronics
>> + * Donghwa Lee <dh09.lee@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * aint with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + *
>> + */
>> +
>> +#define        TRATS_LOGO_WIDTH                520
>> +#define        TRATS_LOGO_HEIGHT               120
>> +#define        TRATS_LOGO_BPP                  32
>> +
>> +unsigned char trats_logo[]={
>> +};
> 
> trats logo?
> I feel, it's a TIZEN logo maybe.
> 

> Also, Is it a board specific? then it should be moved to boards/samsung/trats/
> 

Ok, I will change to TIZEN logo and move it to board/samsung/trats.

>> diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c
>> index f9894b1..d067bb4 100644
>> --- a/board/samsung/trats/trats.c
>> +++ b/board/samsung/trats/trats.c
>> @@ -467,7 +467,7 @@ void init_panel_info(vidinfo_t *vid)
>>        vid->vl_vsp     = CONFIG_SYS_LOW;
>>        vid->vl_dp      = CONFIG_SYS_LOW;
>>
>> -       vid->vl_bpix    = 32;
>> +       vid->vl_bpix    = 5;
>>        vid->dual_lcd_enabled = 0;
>>
>>        /* s6e8ax0 Panel */
>> diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c
>> index 0eb7f58..c111855 100644
>> --- a/drivers/video/exynos_fb.c
>> +++ b/drivers/video/exynos_fb.c
>> @@ -22,6 +22,7 @@
>>
>>  #include <config.h>
>>  #include <common.h>
>> +#include <malloc.h>
>>  #include <lcd.h>
>>  #include <asm/io.h>
>>  #include <asm/arch/cpu.h>
>> @@ -29,6 +30,7 @@
>>  #include <asm/arch/clk.h>
>>  #include <asm/arch/mipi_dsim.h>
>>  #include <asm/arch/system.h>
>> +#include <asm/arch/trats_logo.h>
>>
>>  #include "exynos_fb.h"
>>
>> @@ -52,7 +54,7 @@ static void exynos_lcd_init_mem(void *lcdbase, vidinfo_t *vid)
>>        unsigned long palette_size;
>>        unsigned int fb_size;
>>
>> -       fb_size = vid->vl_row * vid->vl_col * (vid->vl_bpix >> 3);
>> +       fb_size = vid->vl_row * vid->vl_col * (NBITS(vid->vl_bpix) >> 3);
>>
>>        lcd_base = lcdbase;
>>
>> @@ -67,6 +69,38 @@ static void exynos_lcd_init(vidinfo_t *vid)
>>        exynos_fimd_lcd_init(vid);
>>  }
>>
>> +static void draw_logo(void *lcdbase)
>> +{
>> +       int x, y;
>> +       unsigned int in_len, width, height;
>> +       unsigned long out_len =
>> +               (ARRAY_SIZE(trats_logo) * sizeof(*trats_logo)) + 1;
>> +       void *dst = NULL;
>> +
>> +       width = TRATS_LOGO_WIDTH;
>> +       height = TRATS_LOGO_HEIGHT;
> 
> NAK.
> trats_logo and TRATS_* are board specific.
> 

I will add logo size variable in the vidinfo_t structure.

>> +       x = ((panel_width - width) >> 1);
>> +       y = ((panel_height - height) >> 1) - 5;
>> +
>> +       in_len = width * height * 8;
>> +       dst = malloc(in_len);
>> +       if (dst == NULL) {
>> +               printf("Error: malloc in gunzip failed!\n");
>> +               return;
>> +       }
> 
> need space here.
> 

>> +       if (gunzip(dst, in_len, (uchar *)trats_logo, &out_len) != 0) {
>> +               free(dst);
>> +               return;
>> +       }
> 
> need space here.
> 

Ok, I will fix it.

>> +       if (out_len == CONFIG_SYS_VIDEO_LOGO_MAX_SIZE)
> 
> need brace at this if statement.
> And maybe it should be "if (out_len >= CONFIG_SYS_VIDEO_LOGO_MAX_SIZE) " ?
> 
>> +               printf("Image could be truncated"
>> +                               " (increase CONFIG_SYS_VIDEO_LOGO_MAX_SIZE)!\n");
>> +
>> +       lcd_display_bitmap((ulong)dst, x, y);
>> +
>> +       free(dst);
>> +}
>> +
>>  static void lcd_panel_on(vidinfo_t *vid)
>>  {
>>        udelay(vid->init_delay);
>> @@ -113,6 +147,10 @@ void lcd_ctrl_init(void *lcdbase)
>>
>>        exynos_lcd_init_mem(lcdbase, &panel_info);
>>
>> +       memset(lcdbase, 0, panel_width * panel_height *
>> +                       (NBITS(panel_info.vl_bpix) >> 3));
>> +       draw_logo(lcdbase);
> 
> Always draw the logo?
> 

umh... How about your opinion? When board start, I think boot logo has to turn on.

>> +
>>        exynos_lcd_init(&panel_info);
>>  }
>>
>> diff --git a/drivers/video/exynos_fimd.c b/drivers/video/exynos_fimd.c
>> index cd2b1b6..8f2d667 100644
>> --- a/drivers/video/exynos_fimd.c
>> +++ b/drivers/video/exynos_fimd.c
>> @@ -110,7 +110,7 @@ static void exynos_fimd_set_buffer_address(unsigned int win_id)
>>                (struct exynos4_fb *)samsung_get_base_fimd();
>>
>>        start_addr = (unsigned long)lcd_base_addr;
>> -       end_addr = start_addr + ((pvid->vl_col * (pvid->vl_bpix / 8)) *
>> +       end_addr = start_addr + ((pvid->vl_col * (NBITS(pvid->vl_bpix) / 8)) *
> 
> Is it related change?
> I think this is another bug fix.
> If so, please fix them to another patch.
> 

in u-boot mainline vl_bpix is usually used like this, so I had changed. Before sending
this patch, exynos framebuffer didn't drawing logo. At that time I didn't know above macro.
But if it is not used like above, drawing logo feature is not worked.

If you want to separate another patch, I will do that.

>>                                pvid->vl_row);
>>
>>        writel(start_addr, (unsigned int)&fimd_ctrl->vidw00add0b0 +
>> @@ -331,7 +331,7 @@ void exynos_fimd_lcd_init(vidinfo_t *vid)
>>        exynos_fimd_set_buffer_address(pvid->win_id);
>>
>>        /* set buffer size */
>> -       cfg = EXYNOS_VIDADDR_PAGEWIDTH(pvid->vl_col * pvid->vl_bpix / 8);
>> +       cfg = EXYNOS_VIDADDR_PAGEWIDTH(pvid->vl_col * NBITS(pvid->vl_bpix) / 8);
> 
> ditto.
> 
>>        writel(cfg, (unsigned int)&fimd_ctrl->vidw00add2 +
>>                                        EXYNOS_BUFFER_SIZE(pvid->win_id));
>>
>> @@ -350,5 +350,5 @@ void exynos_fimd_lcd_init(vidinfo_t *vid)
>>
>>  unsigned long exynos_fimd_calc_fbsize(void)
>>  {
>> -       return pvid->vl_col * pvid->vl_row * (pvid->vl_bpix / 8);
>> +       return pvid->vl_col * pvid->vl_row * (NBITS(pvid->vl_bpix) / 8);
> 
> ditto.
> 
>>  }
>> diff --git a/include/configs/trats.h b/include/configs/trats.h
>> index 5f913ca..b326035 100644
>> --- a/include/configs/trats.h
>> +++ b/include/configs/trats.h
>> @@ -216,9 +216,11 @@
>>  /* LCD */
>>  #define CONFIG_EXYNOS_FB
>>  #define CONFIG_LCD
>> +#define CONFIG_CMD_BMP
>>  #define CONFIG_FB_ADDR         0x52504000
>>  #define CONFIG_S6E8AX0
>>  #define CONFIG_EXYNOS_MIPI_DSIM
>> -#define CONFIG_SYS_VIDEO_LOGO_MAX_SIZE (1280 * 720 * 4)
>> +#define CONFIG_VIDEO_BMP_GZIP
>> +#define CONFIG_SYS_VIDEO_LOGO_MAX_SIZE (500 * 120 * 4)
> 
> I think.. this is duplicated configuration.
> You need these defines to getting max size of logo.
> 

Yes, above configuration can be removed, I will remove it.

> +#define        TRATS_LOGO_WIDTH                520
> +#define        TRATS_LOGO_HEIGHT               120
> +#define        TRATS_LOGO_BPP                  32
> 
> Thanks.
> Minkyu Kang.


I will send next version patches soon.

Thank you,
Donghwa Lee