Patchwork [U-Boot,1/6] exynos_fb: Remove usage of static defines

login
register
mail settings
Submitter Ajay Kumar
Date Sept. 30, 2013, 11:20 a.m.
Message ID <1380540055-469-2-git-send-email-ajaykumar.rs@samsung.com>
Download mbox | patch
Permalink /patch/279106/
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Comments

Ajay Kumar - Sept. 30, 2013, 11:20 a.m.
Previously, we used to statically assign values for vl_col, vl_row and
vl_bpix using #defines like LCD_XRES, LCD_YRES and LCD_COLOR16.

Introducing the function exynos_lcd_early_init() would take care of this
assignment on the fly by parsing FIMD DT properties, thereby allowing us
to remove LCD_XRES and LCD_YRES from the main config file.

Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
---
 arch/arm/include/asm/arch-exynos/system.h |  1 +
 board/samsung/common/board.c              | 15 +++++++++++++++
 drivers/video/exynos_fb.c                 | 20 ++++++--------------
 include/configs/exynos5250-dt.h           |  2 --
 4 files changed, 22 insertions(+), 16 deletions(-)
Ajay kumar - Oct. 15, 2013, 6:33 a.m.
ping.


On Mon, Sep 30, 2013 at 4:50 PM, Ajay Kumar <ajaykumar.rs@samsung.com>wrote:

> Previously, we used to statically assign values for vl_col, vl_row and
> vl_bpix using #defines like LCD_XRES, LCD_YRES and LCD_COLOR16.
>
> Introducing the function exynos_lcd_early_init() would take care of this
> assignment on the fly by parsing FIMD DT properties, thereby allowing us
> to remove LCD_XRES and LCD_YRES from the main config file.
>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> ---
>  arch/arm/include/asm/arch-exynos/system.h |  1 +
>  board/samsung/common/board.c              | 15 +++++++++++++++
>  drivers/video/exynos_fb.c                 | 20 ++++++--------------
>  include/configs/exynos5250-dt.h           |  2 --
>  4 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-exynos/system.h
> b/arch/arm/include/asm/arch-exynos/system.h
> index 7e2057c..4968d3d 100644
> --- a/arch/arm/include/asm/arch-exynos/system.h
> +++ b/arch/arm/include/asm/arch-exynos/system.h
> @@ -39,5 +39,6 @@ struct exynos5_sysreg {
>
>  void set_usbhost_mode(unsigned int mode);
>  void set_system_display_ctrl(void);
> +int exynos_lcd_early_init(const void *blob);
>
>  #endif /* _EXYNOS4_SYSTEM_H */
> diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c
> index ce85ddb..ae89c94 100644
> --- a/board/samsung/common/board.c
> +++ b/board/samsung/common/board.c
> @@ -17,6 +17,7 @@
>  #include <asm/arch/gpio.h>
>  #include <asm/arch/pinmux.h>
>  #include <asm/arch/power.h>
> +#include <asm/arch/system.h>
>  #include <power/pmic.h>
>  #include <power/max77686_pmic.h>
>
> @@ -130,6 +131,20 @@ int board_early_init_f(void)
>  #ifdef CONFIG_SYS_I2C_INIT_BOARD
>         board_i2c_init(gd->fdt_blob);
>  #endif
> +
> +#if defined(CONFIG_OF_CONTROL) && defined(CONFIG_EXYNOS_FB)
> +/*
> + * board_init_f(arch/arm/lib/board.c) calls lcd_setmem() which needs
> + * panel_info.vl_col, panel_info.vl_row and panel_info.vl_bpix, to reserve
> + * FB memory at a very early stage. So, we need to fill panel_info.vl_col,
> + * panel_info.vl_row and panel_info.vl_bpix before lcd_setmem() is called.
> + */
> +       err = exynos_lcd_early_init(gd->fdt_blob);
> +       if (err) {
> +               debug("LCD early init failed\n");
> +               return err;
> +       }
> +#endif
>         return err;
>  }
>  #endif
> diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c
> index 5c7ec91..686870f 100644
> --- a/drivers/video/exynos_fb.c
> +++ b/drivers/video/exynos_fb.c
> @@ -27,17 +27,12 @@ DECLARE_GLOBAL_DATA_PTR;
>
>  static unsigned int panel_width, panel_height;
>
> -/*
> - * board_init_f(arch/arm/lib/board.c) calls lcd_setmem() which needs
> - * panel_info.vl_col, panel_info.vl_row and panel_info.vl_bpix to reserve
> - * FB memory at a very early stage, i.e even before exynos_fimd_parse_dt()
> - * is called. So, we are forced to statically assign it.
> - */
>  #ifdef CONFIG_OF_CONTROL
>  vidinfo_t panel_info  = {
> -       .vl_col = LCD_XRES,
> -       .vl_row = LCD_YRES,
> -       .vl_bpix = LCD_COLOR16,
> +       /* Insert a value here so that we don't end up in the BSS
> +        * Reference: drivers/video/tegra.c
> +        */
> +       .vl_col = -1,
>  };
>  #endif
>
> @@ -159,7 +154,7 @@ static void lcd_panel_on(vidinfo_t *vid)
>  }
>
>  #ifdef CONFIG_OF_CONTROL
> -int exynos_fimd_parse_dt(const void *blob)
> +int exynos_lcd_early_init(const void *blob)
>  {
>         unsigned int node;
>         node = fdtdec_next_compatible(blob, 0, COMPAT_SAMSUNG_EXYNOS_FIMD);
> @@ -303,10 +298,7 @@ void lcd_ctrl_init(void *lcdbase)
>         set_system_display_ctrl();
>         set_lcd_clk();
>
> -#ifdef CONFIG_OF_CONTROL
> -       if (exynos_fimd_parse_dt(gd->fdt_blob))
> -               debug("Can't get proper panel info\n");
> -#else
> +#ifndef CONFIG_OF_CONTROL
>         /* initialize parameters which is specific to panel. */
>         init_panel_info(&panel_info);
>  #endif
> diff --git a/include/configs/exynos5250-dt.h
> b/include/configs/exynos5250-dt.h
> index 689919d..508962c 100644
> --- a/include/configs/exynos5250-dt.h
> +++ b/include/configs/exynos5250-dt.h
> @@ -50,8 +50,6 @@
>  #ifdef CONFIG_LCD
>  #define CONFIG_EXYNOS_FB
>  #define CONFIG_EXYNOS_DP
> -#define LCD_XRES                       2560
> -#define LCD_YRES                       1600
>  #define LCD_BPP                        LCD_COLOR16
>  #endif
>  #endif  /* __CONFIG_5250_H */
> --
> 1.7.12.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
Simon Glass - Oct. 16, 2013, 4:34 p.m.
Hi Ajay,

[once more from the right address]

On Mon, Sep 30, 2013 at 5:20 AM, Ajay Kumar <ajaykumar.rs@samsung.com>wrote:

> Previously, we used to statically assign values for vl_col, vl_row and
> vl_bpix using #defines like LCD_XRES, LCD_YRES and LCD_COLOR16.
>
> Introducing the function exynos_lcd_early_init() would take care of this
> assignment on the fly by parsing FIMD DT properties, thereby allowing us
> to remove LCD_XRES and LCD_YRES from the main config file.
>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>

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

I can't test this on Pit at present - any chance of a series at some point
to enable that?

I pushed my branch to u-boot-x86.git branch try-5420b.

As a general comment, it would be nice to follow up with a series to fully
enable device tree for the GPIOs also. At the moment these are hard-coded.

Regards,
Simon
Ajay kumar - Oct. 23, 2013, 8:28 a.m.
Hi Simon,


On Wed, Oct 16, 2013 at 10:04 PM, Simon Glass <sjg@chromium.org> wrote:

> Hi Ajay,
>
> [once more from the right address]
>
> On Mon, Sep 30, 2013 at 5:20 AM, Ajay Kumar <ajaykumar.rs@samsung.com
> >wrote:
>
> > Previously, we used to statically assign values for vl_col, vl_row and
> > vl_bpix using #defines like LCD_XRES, LCD_YRES and LCD_COLOR16.
> >
> > Introducing the function exynos_lcd_early_init() would take care of this
> > assignment on the fly by parsing FIMD DT properties, thereby allowing us
> > to remove LCD_XRES and LCD_YRES from the main config file.
> >
> > Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
> >
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
> I can't test this on Pit at present - any chance of a series at some point
> to enable that?
>
Should wait till Rajeshwari sends a patch for peach-pit dts.

 I pushed my branch to u-boot-x86.git branch try-5420b.
>
> As a general comment, it would be nice to follow up with a series to fully
> enable device tree for the GPIOs also. At the moment these are hard-coded.
>
Ok. I will do this for exynos video driver.

 Regards,
> Simon
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>

Thanks and regards,
Ajay Kumar
Simon Glass - Oct. 23, 2013, 12:38 p.m.
On Oct 23, 2013 9:28 AM, "Ajay kumar" <ajaynumb@gmail.com> wrote:
>
> Hi Simon,
>
>
> On Wed, Oct 16, 2013 at 10:04 PM, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Ajay,
>>
>> [once more from the right address]
>>
>> On Mon, Sep 30, 2013 at 5:20 AM, Ajay Kumar <ajaykumar.rs@samsung.com
>wrote:
>>
>> > Previously, we used to statically assign values for vl_col, vl_row and
>> > vl_bpix using #defines like LCD_XRES, LCD_YRES and LCD_COLOR16.
>> >
>> > Introducing the function exynos_lcd_early_init() would take care of
this
>> > assignment on the fly by parsing FIMD DT properties, thereby allowing
us
>> > to remove LCD_XRES and LCD_YRES from the main config file.
>> >
>> > Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>> >
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>>
>> I can't test this on Pit at present - any chance of a series at some
point
>> to enable that?
>
> Should wait till Rajeshwari sends a patch for peach-pit dts.
>
>> I pushed my branch to u-boot-x86.git branch try-5420b.
>>
>> As a general comment, it would be nice to follow up with a series to
fully
>> enable device tree for the GPIOs also. At the moment these are
hard-coded.
>
> Ok. I will do this for exynos video driver.

Sounds good on both counts, thank you.

>
>> Regards,
>> Simon
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>
>
> Thanks and regards,
> Ajay Kumar
>

Patch

diff --git a/arch/arm/include/asm/arch-exynos/system.h b/arch/arm/include/asm/arch-exynos/system.h
index 7e2057c..4968d3d 100644
--- a/arch/arm/include/asm/arch-exynos/system.h
+++ b/arch/arm/include/asm/arch-exynos/system.h
@@ -39,5 +39,6 @@  struct exynos5_sysreg {
 
 void set_usbhost_mode(unsigned int mode);
 void set_system_display_ctrl(void);
+int exynos_lcd_early_init(const void *blob);
 
 #endif	/* _EXYNOS4_SYSTEM_H */
diff --git a/board/samsung/common/board.c b/board/samsung/common/board.c
index ce85ddb..ae89c94 100644
--- a/board/samsung/common/board.c
+++ b/board/samsung/common/board.c
@@ -17,6 +17,7 @@ 
 #include <asm/arch/gpio.h>
 #include <asm/arch/pinmux.h>
 #include <asm/arch/power.h>
+#include <asm/arch/system.h>
 #include <power/pmic.h>
 #include <power/max77686_pmic.h>
 
@@ -130,6 +131,20 @@  int board_early_init_f(void)
 #ifdef CONFIG_SYS_I2C_INIT_BOARD
 	board_i2c_init(gd->fdt_blob);
 #endif
+
+#if defined(CONFIG_OF_CONTROL) && defined(CONFIG_EXYNOS_FB)
+/*
+ * board_init_f(arch/arm/lib/board.c) calls lcd_setmem() which needs
+ * panel_info.vl_col, panel_info.vl_row and panel_info.vl_bpix, to reserve
+ * FB memory at a very early stage. So, we need to fill panel_info.vl_col,
+ * panel_info.vl_row and panel_info.vl_bpix before lcd_setmem() is called.
+ */
+	err = exynos_lcd_early_init(gd->fdt_blob);
+	if (err) {
+		debug("LCD early init failed\n");
+		return err;
+	}
+#endif
 	return err;
 }
 #endif
diff --git a/drivers/video/exynos_fb.c b/drivers/video/exynos_fb.c
index 5c7ec91..686870f 100644
--- a/drivers/video/exynos_fb.c
+++ b/drivers/video/exynos_fb.c
@@ -27,17 +27,12 @@  DECLARE_GLOBAL_DATA_PTR;
 
 static unsigned int panel_width, panel_height;
 
-/*
- * board_init_f(arch/arm/lib/board.c) calls lcd_setmem() which needs
- * panel_info.vl_col, panel_info.vl_row and panel_info.vl_bpix to reserve
- * FB memory at a very early stage, i.e even before exynos_fimd_parse_dt()
- * is called. So, we are forced to statically assign it.
- */
 #ifdef CONFIG_OF_CONTROL
 vidinfo_t panel_info  = {
-	.vl_col = LCD_XRES,
-	.vl_row = LCD_YRES,
-	.vl_bpix = LCD_COLOR16,
+	/* Insert a value here so that we don't end up in the BSS
+	 * Reference: drivers/video/tegra.c
+	 */
+	.vl_col = -1,
 };
 #endif
 
@@ -159,7 +154,7 @@  static void lcd_panel_on(vidinfo_t *vid)
 }
 
 #ifdef CONFIG_OF_CONTROL
-int exynos_fimd_parse_dt(const void *blob)
+int exynos_lcd_early_init(const void *blob)
 {
 	unsigned int node;
 	node = fdtdec_next_compatible(blob, 0, COMPAT_SAMSUNG_EXYNOS_FIMD);
@@ -303,10 +298,7 @@  void lcd_ctrl_init(void *lcdbase)
 	set_system_display_ctrl();
 	set_lcd_clk();
 
-#ifdef CONFIG_OF_CONTROL
-	if (exynos_fimd_parse_dt(gd->fdt_blob))
-		debug("Can't get proper panel info\n");
-#else
+#ifndef CONFIG_OF_CONTROL
 	/* initialize parameters which is specific to panel. */
 	init_panel_info(&panel_info);
 #endif
diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h
index 689919d..508962c 100644
--- a/include/configs/exynos5250-dt.h
+++ b/include/configs/exynos5250-dt.h
@@ -50,8 +50,6 @@ 
 #ifdef CONFIG_LCD
 #define CONFIG_EXYNOS_FB
 #define CONFIG_EXYNOS_DP
-#define LCD_XRES			2560
-#define LCD_YRES			1600
 #define LCD_BPP			LCD_COLOR16
 #endif
 #endif  /* __CONFIG_5250_H */