[U-Boot] [PATCH V2 2/2] i.MX6: mx6qsabrelite: Add splash screen support
Eric Nelson
eric.nelson at boundarydevices.com
Tue Sep 18 15:28:52 CEST 2012
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<eric.nelson at boundarydevices.com>
>> ---
>
>> 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<micrel.h>
>> #include<miiphy.h>
>> #include<netdev.h>
>> +#include<linux/fb.h>
>> +#include<ipu_pixfmt.h>
>> +#include<asm/arch/crm_regs.h>
>> 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<<MXC_CCM_CS2CDR_LDB_DI0_CLK_SEL_OFFSET)
>> + |(3<<MXC_CCM_CS2CDR_LDB_DI1_CLK_SEL_OFFSET);
>> + 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
>> + <<MXC_CCM_CHSCCDR_IPU1_DI0_CLK_SEL_OFFSET)
>> + |(CHSCCDR_PODF_DIVIDE_BY_3
>> + <<MXC_CCM_CHSCCDR_IPU1_DI0_PODF_OFFSET)
>> + |(CHSCCDR_IPU_PRE_CLK_540M_PFD
>> + <<MXC_CCM_CHSCCDR_IPU1_DI0_PRE_CLK_SEL_OFFSET);
>> + 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
More information about the U-Boot
mailing list