Re: [U-Boot] [PATCH v5 04/12] samsung: misc: move display logo function to misc.c file.

2014-01-15 Thread Przemyslaw Marczak 

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.marc...@samsung.com
Tested-by: Hyungwon Hwang human.hw...@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,
--
Przemyslaw Marczak
Samsung RD 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.

2014-01-14 Thread Przemyslaw Marczak 

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.marc...@samsung.com
Tested-by: Hyungwon Hwang human.hw...@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,
--
Przemyslaw Marczak
Samsung RD 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.

2014-01-14 Thread Minkyu Kang 
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.marc...@samsung.com
 Tested-by: Hyungwon Hwang human.hw...@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.
___
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.

2014-01-13 Thread Jaehoon Chung 
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.marc...@samsung.com
 Tested-by: Hyungwon Hwang human.hw...@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 =