Re: [U-Boot] [PATCH v5 04/12] samsung: misc: move display logo function to misc.c file.
Hi Minkyu, On 01/14/2014 09:55 AM, Minkyu Kang wrote: Dear Przemyslaw Marczak, On 14/01/14 17:02, Przemyslaw Marczak wrote: Hello Jaehoon, On 01/14/2014 04:41 AM, Jaehoon Chung wrote: Dear Przemyslaw, On 01/10/2014 11:31 PM, Przemyslaw Marczak wrote: board/samsung/common/misc.c: - move draw_logo() function from exynos_fb.c - add get_tizen_logo_info() function call removed from board files boards: - update board files - add CONFIG_MISC_INIT_R to Universal, Trats and Trats2 Signed-off-by: Przemyslaw Marczak Tested-by: Hyungwon Hwang --- changes v2: - configs cleanup - add check logo address before display Changes v3: - none Changes v4: - none Changes v5: - none board/samsung/common/misc.c | 42 ++ board/samsung/trats/trats.c |3 --- board/samsung/trats2/trats2.c|4 --- board/samsung/universal_c210/universal.c |4 --- drivers/video/exynos_fb.c| 28 include/configs/s5pc210_universal.h |3 +++ include/configs/trats.h |3 +++ include/configs/trats2.h |3 +++ 8 files changed, 51 insertions(+), 39 deletions(-) diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c index 3764d12..6188e29 100644 --- a/board/samsung/common/misc.c +++ b/board/samsung/common/misc.c @@ -6,9 +6,51 @@ */ #include +#include +#include + +#ifdef CONFIG_CMD_BMP +static void draw_logo(void) +{ +int x, y; +ulong addr; + +#ifdef CONFIG_TIZEN +get_tizen_logo_info(&panel_info); +#else +return; if CONFIG_TINZE didn't set, draw_logo should be just return, right? Then I think this point could be changed more readable. #ifdef CONFIG_TIZEN int x, y; ulong addr; get_tizen_logo_info(...); add = panel_info.logo_addr; ... bmp_display(addr, x, y); #endif how about? Best Regards, Jaehoon Chung You know, this file is common for all Samsung platforms,and I think this function should not depends only on Tizen logo. In this case user can choose other logo just by adding function like "get_logo" instead of the return statement. I also think that there is need for some common function which allows set proper logo by board config, but maybe not in this patch set? I agreed with you. You said that "there is need for some common function which allows set proper logo by board config". At previous version of board files, you already have common function (init_panel_info - even though this function is not for the logo but I think, it's enough). @@ -484,10 +484,6 @@ void init_panel_info(vidinfo_t *vid) vid->resolution = HD_RESOLUTION; vid->rgb_mode= MODE_RGB_P; -#ifdef CONFIG_TIZEN - get_tizen_logo_info(vid); -#endif - /* for LD9040. */ vid->pclk_name = 1; /* MPLL */ vid->sclk_div = 1; If you remove this change at each boards, then you don't have add ifdef CONFIG_TIZEN at draw_logo function. It can be split common stuffs and board specific stuffs efficiently. How you think? Thanks, Minkyu Kang. Actually I had something else on my mind but this is more simply, so I will change this back. Thank you, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 04/12] samsung: misc: move display logo function to misc.c file.
Dear Przemyslaw Marczak, On 14/01/14 17:02, Przemyslaw Marczak wrote: > Hello Jaehoon, > > On 01/14/2014 04:41 AM, Jaehoon Chung wrote: >> Dear Przemyslaw, >> >> On 01/10/2014 11:31 PM, Przemyslaw Marczak wrote: >>> board/samsung/common/misc.c: >>> - move draw_logo() function from exynos_fb.c >>> - add get_tizen_logo_info() function call removed from board files >>> >>> boards: >>> - update board files >>> - add CONFIG_MISC_INIT_R to Universal, Trats and Trats2 >>> >>> Signed-off-by: Przemyslaw Marczak >>> Tested-by: Hyungwon Hwang >>> --- >>> changes v2: >>> - configs cleanup >>> - add check logo address before display >>> >>> Changes v3: >>> - none >>> >>> Changes v4: >>> - none >>> >>> Changes v5: >>> - none >>> >>> board/samsung/common/misc.c | 42 >>> ++ >>> board/samsung/trats/trats.c |3 --- >>> board/samsung/trats2/trats2.c|4 --- >>> board/samsung/universal_c210/universal.c |4 --- >>> drivers/video/exynos_fb.c| 28 >>> include/configs/s5pc210_universal.h |3 +++ >>> include/configs/trats.h |3 +++ >>> include/configs/trats2.h |3 +++ >>> 8 files changed, 51 insertions(+), 39 deletions(-) >>> >>> diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c >>> index 3764d12..6188e29 100644 >>> --- a/board/samsung/common/misc.c >>> +++ b/board/samsung/common/misc.c >>> @@ -6,9 +6,51 @@ >>>*/ >>> >>> #include >>> +#include >>> +#include >>> + >>> +#ifdef CONFIG_CMD_BMP >>> +static void draw_logo(void) >>> +{ >>> +int x, y; >>> +ulong addr; >>> + >>> +#ifdef CONFIG_TIZEN >>> +get_tizen_logo_info(&panel_info); >>> +#else >>> +return; >> if CONFIG_TINZE didn't set, draw_logo should be just return, right? >> Then I think this point could be changed more readable. >> >> #ifdef CONFIG_TIZEN >> int x, y; >> ulong addr; >> >> get_tizen_logo_info(...); >> >> add = panel_info.logo_addr; >> ... >> bmp_display(addr, x, y); >> #endif >> >> how about? >> >> Best Regards, >> Jaehoon Chung >> >> > > You know, this file is common for all Samsung platforms,and I think this > function should not depends only on Tizen logo. In this case user can choose > other logo just by adding function like "get_logo" instead of the return > statement. I also think that there is need for some common function which > allows set proper logo by board config, but maybe not in this patch set? I agreed with you. You said that "there is need for some common function which allows set proper logo by board config". At previous version of board files, you already have common function (init_panel_info - even though this function is not for the logo but I think, it's enough). @@ -484,10 +484,6 @@ void init_panel_info(vidinfo_t *vid) vid->resolution = HD_RESOLUTION; vid->rgb_mode = MODE_RGB_P; -#ifdef CONFIG_TIZEN - get_tizen_logo_info(vid); -#endif - /* for LD9040. */ vid->pclk_name = 1; /* MPLL */ vid->sclk_div = 1; If you remove this change at each boards, then you don't have add ifdef CONFIG_TIZEN at draw_logo function. It can be split common stuffs and board specific stuffs efficiently. How you think? Thanks, Minkyu Kang. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 04/12] samsung: misc: move display logo function to misc.c file.
Hello Jaehoon, On 01/14/2014 04:41 AM, Jaehoon Chung wrote: Dear Przemyslaw, On 01/10/2014 11:31 PM, Przemyslaw Marczak wrote: board/samsung/common/misc.c: - move draw_logo() function from exynos_fb.c - add get_tizen_logo_info() function call removed from board files boards: - update board files - add CONFIG_MISC_INIT_R to Universal, Trats and Trats2 Signed-off-by: Przemyslaw Marczak Tested-by: Hyungwon Hwang --- changes v2: - configs cleanup - add check logo address before display Changes v3: - none Changes v4: - none Changes v5: - none board/samsung/common/misc.c | 42 ++ board/samsung/trats/trats.c |3 --- board/samsung/trats2/trats2.c|4 --- board/samsung/universal_c210/universal.c |4 --- drivers/video/exynos_fb.c| 28 include/configs/s5pc210_universal.h |3 +++ include/configs/trats.h |3 +++ include/configs/trats2.h |3 +++ 8 files changed, 51 insertions(+), 39 deletions(-) diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c index 3764d12..6188e29 100644 --- a/board/samsung/common/misc.c +++ b/board/samsung/common/misc.c @@ -6,9 +6,51 @@ */ #include +#include +#include + +#ifdef CONFIG_CMD_BMP +static void draw_logo(void) +{ + int x, y; + ulong addr; + +#ifdef CONFIG_TIZEN + get_tizen_logo_info(&panel_info); +#else + return; if CONFIG_TINZE didn't set, draw_logo should be just return, right? Then I think this point could be changed more readable. #ifdef CONFIG_TIZEN int x, y; ulong addr; get_tizen_logo_info(...); add = panel_info.logo_addr; ... bmp_display(addr, x, y); #endif how about? Best Regards, Jaehoon Chung You know, this file is common for all Samsung platforms, and I think this function should not depends only on Tizen logo. In this case user can choose other logo just by adding function like "get_logo" instead of the return statement. I also think that there is need for some common function which allows set proper logo by board config, but maybe not in this patch set? Thank you, -- Przemyslaw Marczak Samsung R&D Institute Poland Samsung Electronics p.marc...@samsung.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 04/12] samsung: misc: move display logo function to misc.c file.
Dear Przemyslaw, On 01/10/2014 11:31 PM, Przemyslaw Marczak wrote: > board/samsung/common/misc.c: > - move draw_logo() function from exynos_fb.c > - add get_tizen_logo_info() function call removed from board files > > boards: > - update board files > - add CONFIG_MISC_INIT_R to Universal, Trats and Trats2 > > Signed-off-by: Przemyslaw Marczak > Tested-by: Hyungwon Hwang > --- > changes v2: > - configs cleanup > - add check logo address before display > > Changes v3: > - none > > Changes v4: > - none > > Changes v5: > - none > > board/samsung/common/misc.c | 42 > ++ > board/samsung/trats/trats.c |3 --- > board/samsung/trats2/trats2.c|4 --- > board/samsung/universal_c210/universal.c |4 --- > drivers/video/exynos_fb.c| 28 > include/configs/s5pc210_universal.h |3 +++ > include/configs/trats.h |3 +++ > include/configs/trats2.h |3 +++ > 8 files changed, 51 insertions(+), 39 deletions(-) > > diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c > index 3764d12..6188e29 100644 > --- a/board/samsung/common/misc.c > +++ b/board/samsung/common/misc.c > @@ -6,9 +6,51 @@ > */ > > #include > +#include > +#include > + > +#ifdef CONFIG_CMD_BMP > +static void draw_logo(void) > +{ > + int x, y; > + ulong addr; > + > +#ifdef CONFIG_TIZEN > + get_tizen_logo_info(&panel_info); > +#else > + return; if CONFIG_TINZE didn't set, draw_logo should be just return, right? Then I think this point could be changed more readable. #ifdef CONFIG_TIZEN int x, y; ulong addr; get_tizen_logo_info(...); add = panel_info.logo_addr; ... bmp_display(addr, x, y); #endif how about? Best Regards, Jaehoon Chung > +#endif > + > + addr = panel_info.logo_addr; > + if (!addr) { > + error("There is no logo data."); > + return; > + } > + > + if (panel_info.vl_width >= panel_info.logo_width) { > + x = ((panel_info.vl_width - panel_info.logo_width) >> 1); > + } else { > + x = 0; > + printf("Warning: image width is bigger than display width\n"); > + } > + > + if (panel_info.vl_height >= panel_info.logo_height) { > + y = ((panel_info.vl_height - panel_info.logo_height) >> 1); > + } else { > + y = 0; > + printf("Warning: image height is bigger than display height\n"); > + } > + > + bmp_display(addr, x, y); > +} > +#endif /* CONFIG_CMD_BMP */ > > /* Common for Samsung boards */ > int misc_init_r(void) > { > +#ifdef CONFIG_CMD_BMP > + if (panel_info.logo_on) > + draw_logo(); > +#endif > return 0; > } > diff --git a/board/samsung/trats/trats.c b/board/samsung/trats/trats.c > index 640a193..cd27f18 100644 > --- a/board/samsung/trats/trats.c > +++ b/board/samsung/trats/trats.c > @@ -770,9 +770,6 @@ void init_panel_info(vidinfo_t *vid) > vid->resolution = HD_RESOLUTION, > vid->rgb_mode = MODE_RGB_P, > > -#ifdef CONFIG_TIZEN > - get_tizen_logo_info(vid); > -#endif > mipi_lcd_device.reverse_panel = 1; > > strcpy(s6e8ax0_platform_data.lcd_panel_name, mipi_lcd_device.name); > diff --git a/board/samsung/trats2/trats2.c b/board/samsung/trats2/trats2.c > index b4ccf51..9a2c212 100644 > --- a/board/samsung/trats2/trats2.c > +++ b/board/samsung/trats2/trats2.c > @@ -596,10 +596,6 @@ void init_panel_info(vidinfo_t *vid) > > mipi_lcd_device.reverse_panel = 1; > > -#ifdef CONFIG_TIZEN > - get_tizen_logo_info(vid); > -#endif > - > strcpy(dsim_platform_data.lcd_panel_name, mipi_lcd_device.name); > dsim_platform_data.mipi_power = mipi_power; > dsim_platform_data.phy_enable = set_mipi_phy_ctrl; > diff --git a/board/samsung/universal_c210/universal.c > b/board/samsung/universal_c210/universal.c > index 54d0e1e..166d5ee 100644 > --- a/board/samsung/universal_c210/universal.c > +++ b/board/samsung/universal_c210/universal.c > @@ -484,10 +484,6 @@ void init_panel_info(vidinfo_t *vid) > vid->resolution = HD_RESOLUTION; > vid->rgb_mode = MODE_RGB_P; > > -#ifdef CONFIG_TIZEN > - get_tizen_logo_info(vid); > -#endif > - > /* for LD9040. */ > vid->pclk_name = 1; /* MPLL */ > vid->sclk_div = 1; > diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c > index 7d4c6e0..00a0a11 100644 > --- a/drivers/video/exynos_fb.c > +++ b/drivers/video/exynos_fb.c > @@ -62,31 +62,6 @@ static void exynos_lcd_init(vidinfo_t *vid) > lcd_set_flush_dcache(1); > } > > -#ifdef CONFIG_CMD_BMP > -static void draw_logo(void) > -{ > - int x, y; > - ulong addr; > - > - if (panel_width >= panel_info.logo_width) { > - x = ((panel_width - panel_info.logo_width) >> 1); > - } else { > - x = 0; > - pr