From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Nelson Date: Tue, 18 Sep 2012 06:28:52 -0700 Subject: [U-Boot] [PATCH V2 2/2] i.MX6: mx6qsabrelite: Add splash screen support In-Reply-To: <505826F9.5070103@denx.de> References: <1347913251-1096-1-git-send-email-eric.nelson@boundarydevices.com> <1347923652-1915-1-git-send-email-eric.nelson@boundarydevices.com> <1347923652-1915-3-git-send-email-eric.nelson@boundarydevices.com> <505826F9.5070103@denx.de> Message-ID: <50587714.3040209@boundarydevices.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Thanks for the review, Stefano. On 09/18/2012 12:47 AM, Stefano Babic wrote: > On 18/09/2012 01:14, Eric Nelson wrote: > > Hi Eric, > >> Adds support for the Hannstar 1024 x 768 LVDS panel (Freescale part >> number MCIMX-LVDS1) to SABRE-Lite board. >> >> This commit is a rebase Fabio Estevan's patch from 5/31 to >> u-boot-video/master: >> http://patchwork.ozlabs.org/patch/162206/ >> >> Modifications include: >> removal of i2c setup (unneeded) >> cleanup of lcd_iomux to use struct mxc_ccm_reg and anatop_regs >> and associated constants >> > > All this stuff should not be part of the commit message, because it is > more or less a changelog. Should you also include Fabio's signed-off ? > Okay. I'll take it out of V3. I didn't include Fabio's sign off because he hadn't seen this yet and I changed a fair amount of code. >> Signed-off-by: Eric Nelson >> --- > >> arch/arm/include/asm/arch-mx6/crm_regs.h | 4 + >> board/freescale/mx6qsabrelite/mx6qsabrelite.c | 90 +++++++++++++++++++++++++ >> include/configs/mx6qsabrelite.h | 14 ++++- >> 3 files changed, 107 insertions(+), 1 deletions(-) >> >> diff --git a/arch/arm/include/asm/arch-mx6/crm_regs.h b/arch/arm/include/asm/arch-mx6/crm_regs.h >> index 8388e38..cffc0a1 100644 >> --- a/arch/arm/include/asm/arch-mx6/crm_regs.h >> +++ b/arch/arm/include/asm/arch-mx6/crm_regs.h >> @@ -294,6 +294,10 @@ struct mxc_ccm_reg { >> #define MXC_CCM_CHSCCDR_IPU1_DI0_CLK_SEL_MASK (0x7) >> #define MXC_CCM_CHSCCDR_IPU1_DI0_CLK_SEL_OFFSET 0 >> >> +#define CHSCCDR_CLK_SEL_LDB_DI0 3 >> +#define CHSCCDR_PODF_DIVIDE_BY_3 2 >> +#define CHSCCDR_IPU_PRE_CLK_540M_PFD 5 >> + >> /* Define the bits in register CSCDR2 */ >> #define MXC_CCM_CSCDR2_ECSPI_CLK_PODF_MASK (0x3F<< 19) >> #define MXC_CCM_CSCDR2_ECSPI_CLK_PODF_OFFSET 19 >> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c >> index 4b4e89b..1632e7b 100644 >> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c >> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c >> @@ -36,6 +36,9 @@ >> #include >> #include >> #include >> +#include >> +#include >> +#include >> DECLARE_GLOBAL_DATA_PTR; >> >> #define UART_PAD_CTRL (PAD_CTL_PKE | PAD_CTL_PUE | \ >> @@ -195,6 +198,10 @@ static iomux_v3_cfg_t button_pads[] = { >> MX6Q_PAD_GPIO_18__GPIO_7_13 | MUX_PAD_CTRL(BUTTON_PAD_CTRL), >> }; >> >> +iomux_v3_cfg_t lcd_gpio[] = { >> + MX6Q_PAD_SD1_CMD__GPIO_1_18 | MUX_PAD_CTRL(NO_PAD_CTRL), >> +}; >> + >> static void setup_iomux_enet(void) >> { >> gpio_direction_output(IMX_GPIO_NR(3, 23), 0); >> @@ -372,10 +379,84 @@ int setup_sata(void) >> } >> #endif >> >> +static struct fb_videomode lvds_xga = { >> + .name = "Hannstar-XGA", >> + .refresh = 60, >> + .xres = 1024, >> + .yres = 768, >> + .pixclock = 15385, >> + .left_margin = 220, >> + .right_margin = 40, >> + .upper_margin = 21, >> + .lower_margin = 7, >> + .hsync_len = 60, >> + .vsync_len = 10, >> + .sync = FB_SYNC_EXT, >> + .vmode = FB_VMODE_NONINTERLACED >> +}; >> + >> +void lcd_iomux(void) >> +{ >> + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; >> + struct anatop_regs *anatop = (struct anatop_regs *)ANATOP_BASE_ADDR; >> + >> + int reg; >> + /* Turn on GPIO backlight */ >> + imx_iomux_v3_setup_multiple_pads(lcd_gpio, ARRAY_SIZE(lcd_gpio)); >> + gpio_direction_output(18, 1); >> + >> + /* Turn on LDB0,IPU,IPU DI0 clocks */ >> + reg = __raw_readl(&mxc_ccm->CCGR3); >> + reg |= 0x300F; >> + writel(reg,&mxc_ccm->CCGR3); > > I think we can add constants for these - at least for these constants, > you could drop the not useful defines with offset like > MXC_CCM_CCGR5_CGx_OFFSET. There is already a patch (not yet merged) for > MX5, we should then doing the same for MX6. > Do you have a reference to the patch so I can follow precedent? >> + >> + /* set PFD3_FRAC to 0x13 == 455 MHz (480*18)/0x13 */ >> + writel(0x00003F00,&anatop->pfd_480_clr); >> + writel(0x00001300,&anatop->pfd_480_set); > > Add constants for these. They are not already defined, but they are > added when needed, see for example ehci-mx6.c > Ok. >> + >> + /* set LDB0, LDB1 clk select to 011/011 */ >> + reg = readl(&mxc_ccm->cs2cdr); >> + reg&= ~(MXC_CCM_CS2CDR_LDB_DI0_CLK_SEL_MASK >> + |MXC_CCM_CS2CDR_LDB_DI1_CLK_SEL_MASK); >> + reg |= (3<> + |(3<> + writel(reg,&mxc_ccm->cs2cdr); >> + >> + reg = readl(&mxc_ccm->cscmr2); >> + reg |= MXC_CCM_CSCMR2_LDB_DI0_IPU_DIV; >> + writel(reg,&mxc_ccm->cscmr2); >> + >> + reg = readl(&mxc_ccm->chsccdr); >> + reg&= ~(MXC_CCM_CHSCCDR_IPU1_DI0_PRE_CLK_SEL_MASK >> + |MXC_CCM_CHSCCDR_IPU1_DI0_PODF_MASK >> + |MXC_CCM_CHSCCDR_IPU1_DI0_CLK_SEL_MASK); >> + /* derive clock from LDB_DI0 */ >> + /* divide by 3 */ >> + /* derive clock from 540M PFD */ > > Wrong style for a multiline comment > Noted. These are leftovers I used while hand decoding the bits. >> + reg |= (CHSCCDR_CLK_SEL_LDB_DI0 >> + <> + |(CHSCCDR_PODF_DIVIDE_BY_3 >> + <> + |(CHSCCDR_IPU_PRE_CLK_540M_PFD >> + <> + writel(reg,&mxc_ccm->chsccdr); >> + >> + writel(0x201, IOMUXC_BASE_ADDR + 0x8); /* 16 bpp */ > > This is GPR2, right ? Nobody can easy understand. Use structure (not > offset) and constants to set it. > Yep. >> +} >> + >> +void lcd_enable(void) >> +{ >> + >> + int ret = ipuv3_fb_init(&lvds_xga, 0, IPU_PIX_FMT_RGB666); >> + if (ret) >> + printf("LCD cannot be configured: %d\n", ret); >> +} >> + >> int board_early_init_f(void) >> { >> setup_iomux_uart(); >> setup_buttons(); >> + lcd_iomux(); >> >> return 0; >> } >> @@ -396,9 +477,18 @@ int board_init(void) >> setup_sata(); >> #endif >> >> +#ifdef CONFIG_VIDEO >> + lcd_enable(); >> +#endif >> return 0; >> } >> >> +int board_late_init(void) >> +{ >> + setenv("stdout", "serial"); >> + return 0; > > This is wrong, we found a better way to do this. You should add a > overwrite_console() function, see for example the MX5 boards. > Fabio noted that as well. >> --- a/include/configs/mx6qsabrelite.h >> +++ b/include/configs/mx6qsabrelite.h >> @@ -39,9 +39,10 @@ >> #define CONFIG_REVISION_TAG >> >> /* Size of malloc() pool */ >> -#define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 2 * 1024 * 1024) >> +#define CONFIG_SYS_MALLOC_LEN (10 * 1024 * 1024) >> >> #define CONFIG_BOARD_EARLY_INIT_F >> +#define CONFIG_BOARD_LATE_INIT >> #define CONFIG_MISC_INIT_R >> #define CONFIG_MXC_GPIO >> >> @@ -121,6 +122,17 @@ >> /* Miscellaneous commands */ >> #define CONFIG_CMD_BMODE >> >> +/* Framebuffer and LCD */ >> +#define CONFIG_VIDEO >> +#define CONFIG_VIDEO_IPUV3 >> +#define CONFIG_CFB_CONSOLE >> +#define CONFIG_VGA_AS_SINGLE_DEVICE >> +#define CONFIG_VIDEO_BMP_RLE8 >> +#define CONFIG_SPLASH_SCREEN >> +#define CONFIG_BMP_16BPP >> +#define CONFIG_VIDEO_LOGO >> +#define CONFIG_IPUV3_CLK 260000000 > > ...adding also CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE > I'll send a note under separate cover about this. > Best regards, > Stefano Babic > Regards, Eric