| Message ID | 3e987e7acfdfb20485bbf9e9ee5f1a5394408f98.1389352945.git.p.marczak@samsung.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Minkyu Kang |
| Headers | show |
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 <p.marczak@samsung.com> > Tested-by: Hyungwon Hwang <human.hwang@samsung.com> > --- > 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 <common.h> > +#include <lcd.h> > +#include <libtizen.h> > + > +#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; > - printf("Warning: image width is bigger than display width\n"); > - } > - > - if (panel_height >= panel_info.logo_height) { > - y = ((panel_height - panel_info.logo_height) >> 1) - 4; > - } else { > - y = 0; > - printf("Warning: image height is bigger than display height\n"); > - } > - > - addr = panel_info.logo_addr; > - bmp_display(addr, x, y); > -} > -#endif > - > void __exynos_cfg_lcd_gpio(void) > { > } > @@ -323,9 +298,6 @@ void lcd_enable(void) > if (panel_info.logo_on) { > memset((void *) gd->fb_base, 0, panel_width * panel_height * > (NBITS(panel_info.vl_bpix) >> 3)); > -#ifdef CONFIG_CMD_BMP > - draw_logo(); > -#endif > } > > lcd_panel_on(&panel_info); > diff --git a/include/configs/s5pc210_universal.h b/include/configs/s5pc210_universal.h > index 02a1c99..8a749f2 100644 > --- a/include/configs/s5pc210_universal.h > +++ b/include/configs/s5pc210_universal.h > @@ -269,6 +269,9 @@ void universal_spi_sda(int bit); > int universal_spi_read(void); > #endif > > +/* Common misc for Samsung */ > +#define CONFIG_MISC_INIT_R > + > /* > * LCD Settings > */ > diff --git a/include/configs/trats.h b/include/configs/trats.h > index 9727f7a..1a75bb3 100644 > --- a/include/configs/trats.h > +++ b/include/configs/trats.h > @@ -301,6 +301,9 @@ > #define CONFIG_USB_GADGET_VBUS_DRAW 2 > #define CONFIG_USB_CABLE_CHECK > > +/* Common misc for Samsung */ > +#define CONFIG_MISC_INIT_R > + > /* LCD */ > #define CONFIG_EXYNOS_FB > #define CONFIG_LCD > diff --git a/include/configs/trats2.h b/include/configs/trats2.h > index 7dfbe98..e54fe52 100644 > --- a/include/configs/trats2.h > +++ b/include/configs/trats2.h > @@ -312,6 +312,9 @@ int get_soft_i2c_sda_pin(void); > #define CONFIG_USB_GADGET_VBUS_DRAW 2 > #define CONFIG_USB_CABLE_CHECK > > +/* Common misc for Samsung */ > +#define CONFIG_MISC_INIT_R > + > /* LCD */ > #define CONFIG_EXYNOS_FB > #define CONFIG_LCD >
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 <p.marczak@samsung.com> >> Tested-by: Hyungwon Hwang <human.hwang@samsung.com> >> --- >> 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 <common.h> >> +#include <lcd.h> >> +#include <libtizen.h> >> + >> +#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,
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 <p.marczak@samsung.com> >>> Tested-by: Hyungwon Hwang <human.hwang@samsung.com> >>> --- >>> 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 <common.h> >>> +#include <lcd.h> >>> +#include <libtizen.h> >>> + >>> +#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.
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 <p.marczak@samsung.com> >>>> Tested-by: Hyungwon Hwang <human.hwang@samsung.com> >>>> --- >>>> 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 <common.h> >>>> +#include <lcd.h> >>>> +#include <libtizen.h> >>>> + >>>> +#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,
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 <common.h> +#include <lcd.h> +#include <libtizen.h> + +#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; +#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; - printf("Warning: image width is bigger than display width\n"); - } - - if (panel_height >= panel_info.logo_height) { - y = ((panel_height - panel_info.logo_height) >> 1) - 4; - } else { - y = 0; - printf("Warning: image height is bigger than display height\n"); - } - - addr = panel_info.logo_addr; - bmp_display(addr, x, y); -} -#endif - void __exynos_cfg_lcd_gpio(void) { } @@ -323,9 +298,6 @@ void lcd_enable(void) if (panel_info.logo_on) { memset((void *) gd->fb_base, 0, panel_width * panel_height * (NBITS(panel_info.vl_bpix) >> 3)); -#ifdef CONFIG_CMD_BMP - draw_logo(); -#endif } lcd_panel_on(&panel_info); diff --git a/include/configs/s5pc210_universal.h b/include/configs/s5pc210_universal.h index 02a1c99..8a749f2 100644 --- a/include/configs/s5pc210_universal.h +++ b/include/configs/s5pc210_universal.h @@ -269,6 +269,9 @@ void universal_spi_sda(int bit); int universal_spi_read(void); #endif +/* Common misc for Samsung */ +#define CONFIG_MISC_INIT_R + /* * LCD Settings */ diff --git a/include/configs/trats.h b/include/configs/trats.h index 9727f7a..1a75bb3 100644 --- a/include/configs/trats.h +++ b/include/configs/trats.h @@ -301,6 +301,9 @@ #define CONFIG_USB_GADGET_VBUS_DRAW 2 #define CONFIG_USB_CABLE_CHECK +/* Common misc for Samsung */ +#define CONFIG_MISC_INIT_R + /* LCD */ #define CONFIG_EXYNOS_FB #define CONFIG_LCD diff --git a/include/configs/trats2.h b/include/configs/trats2.h index 7dfbe98..e54fe52 100644 --- a/include/configs/trats2.h +++ b/include/configs/trats2.h @@ -312,6 +312,9 @@ int get_soft_i2c_sda_pin(void); #define CONFIG_USB_GADGET_VBUS_DRAW 2 #define CONFIG_USB_CABLE_CHECK +/* Common misc for Samsung */ +#define CONFIG_MISC_INIT_R + /* LCD */ #define CONFIG_EXYNOS_FB #define CONFIG_LCD