diff mbox

[U-Boot,V2,07/14] imx: mx6: add clock api for lcdif

Message ID 1445341184-22272-8-git-send-email-Peng.Fan@freescale.com
State Changes Requested
Delegated to: Stefano Babic
Headers show

Commit Message

Peng Fan Oct. 20, 2015, 11:39 a.m. UTC
Implement mxs_set_lcdclk, enable_lcdif_clock and enable_pll_video.
The three API can be used to configure lcdif related clock when
CONFIG_VIDEO_MXS enabled.

Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
Cc: Stefano Babic <sbabic@denx.de>
---

V2:
 none

 arch/arm/cpu/armv7/mx6/clock.c        | 239 ++++++++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-mx6/clock.h |   2 +
 2 files changed, 241 insertions(+)

Comments

Stefano Babic Oct. 20, 2015, 1:39 p.m. UTC | #1
Hi Peng,

On 20/10/2015 13:39, Peng Fan wrote:
> Implement mxs_set_lcdclk, enable_lcdif_clock and enable_pll_video.
> The three API can be used to configure lcdif related clock when
> CONFIG_VIDEO_MXS enabled.
> 
> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
> 
> V2:
>  none
> 
>  arch/arm/cpu/armv7/mx6/clock.c        | 239 ++++++++++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-mx6/clock.h |   2 +
>  2 files changed, 241 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c
> index 11efd12..8a88378 100644
> --- a/arch/arm/cpu/armv7/mx6/clock.c
> +++ b/arch/arm/cpu/armv7/mx6/clock.c
> @@ -473,6 +473,245 @@ static u32 get_mmdc_ch0_clk(void)
>  	}
>  }
>  
> +#if defined(CONFIG_VIDEO_MXS)
> +static int enable_pll_video(u32 pll_div, u32 pll_num, u32 pll_denom,
> +			    u32 post_div)
> +{
> +	u32 reg = 0;
> +	ulong start;
> +
> +	debug("pll5 div = %d, num = %d, denom = %d\n",
> +	      pll_div, pll_num, pll_denom);
> +
> +	/* Power up PLL5 video */
> +	writel(BM_ANADIG_PLL_VIDEO_POWERDOWN |
> +	       BM_ANADIG_PLL_VIDEO_BYPASS |
> +	       BM_ANADIG_PLL_VIDEO_DIV_SELECT |
> +	       BM_ANADIG_PLL_VIDEO_POST_DIV_SELECT,
> +	       &imx_ccm->analog_pll_video_clr);
> +
> +	/* Set div, num and denom */
> +	switch (post_div) {
> +	case 1:
> +		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
> +		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x2),
> +		       &imx_ccm->analog_pll_video_set);
> +		break;
> +	case 2:
> +		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
> +		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x1),
> +		       &imx_ccm->analog_pll_video_set);
> +		break;
> +	case 4:
> +		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
> +		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x0),
> +		       &imx_ccm->analog_pll_video_set);
> +		break;
> +	default:
> +		puts("Wrong test_div!\n");
> +		return -EINVAL;
> +	}
> +
> +	writel(BF_ANADIG_PLL_VIDEO_NUM_A(pll_num),
> +	       &imx_ccm->analog_pll_video_num);
> +	writel(BF_ANADIG_PLL_VIDEO_DENOM_B(pll_denom),
> +	       &imx_ccm->analog_pll_video_denom);
> +
> +	/* Wait PLL5 lock */
> +	start = get_timer(0);	/* Get current timestamp */
> +
> +	do {
> +		reg = readl(&imx_ccm->analog_pll_video);
> +		if (reg & BM_ANADIG_PLL_VIDEO_LOCK) {
> +			/* Enable PLL out */
> +			writel(BM_ANADIG_PLL_VIDEO_ENABLE,
> +			       &imx_ccm->analog_pll_video_set);
> +			return 0;
> +		}
> +	} while (get_timer(0) < (start + 10)); /* Wait 10ms */
> +
> +	printf("Lock PLL5 timeout\n");

Maybe puts() is better here

> +
> +	return -ETIME;
> +}
> +
> +/*
> + * 24M--> PLL_VIDEO -> LCDIFx_PRED -> LCDIFx_PODF -> LCD
> + *
> + * 'freq' using KHz as unit, see driver/video/mxsfb.c.
> + */
> +void mxs_set_lcdclk(u32 base_addr, u32 freq)
> +{
> +	u32 reg = 0;
> +	u32 hck = MXC_HCLK / 1000;
> +	/* DIV_SELECT ranges from 27 to 54 */
> +	u32 min = hck * 27;
> +	u32 max = hck * 54;
> +	u32 temp, best = 0;
> +	u32 i, j, max_pred = 8, max_postd = 8, pred = 1, postd = 1;
> +	u32 pll_div, pll_num, pll_denom, post_div = 1;
> +
> +	debug("mxs_set_lcdclk, freq = %dKHz\n", freq);
> +
> +	if ((!is_cpu_type(MXC_CPU_MX6SX)) && !is_cpu_type(MXC_CPU_MX6UL))
> +		return;
> +
> +	if (base_addr == LCDIF1_BASE_ADDR) {

As mentioned before: base_addr is really an index, it is not used as
address. The interface is then confusing.

> +		reg = readl(&imx_ccm->cscdr2);
> +		/* Can't change clocks when clock not from pre-mux */
> +		if ((reg & MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK) != 0)
> +			return;
> +	}
> +
> +	if (is_cpu_type(MXC_CPU_MX6SX)) {
> +		reg = readl(&imx_ccm->cscdr2);
> +		/* Can't change clocks when clock not from pre-mux */
> +		if ((reg & MXC_CCM_CSCDR2_LCDIF2_CLK_SEL_MASK) != 0)
> +			return;
> +	}
> +
> +	temp = freq * max_pred * max_postd;
> +	if (temp > max) {
> +		puts("Please decrease freq, too large!\n");
> +		return;
> +	}
> +	if (temp < min) {
> +		/*
> +		 * Register: PLL_VIDEO
> +		 * Bit Field: POST_DIV_SELECT
> +		 * 00 — Divide by 4.
> +		 * 01 — Divide by 2.
> +		 * 10 — Divide by 1.
> +		 * 11 — Reserved
> +		 * No need to check post_div(1)
> +		 */
> +		for (post_div = 2; post_div <= 4; post_div <<= 1) {
> +			if ((temp * post_div) > min) {
> +				freq *= post_div;
> +				break;
> +			}
> +		}
> +
> +		if (post_div > 4) {
> +			printf("Fail to set rate to %dkhz", freq);
> +			return;
> +		}
> +	}

It is not clear what happens in the calling function in error case. The
error is not propagate to the caller. Of course, we see the error on the
console. Is it enough ? The caller will tzry to go on setting the LCD
controller even in case this function fails.


> +
> +	/* Choose the best pred and postd to match freq for lcd */
> +	for (i = 1; i <= max_pred; i++) {
> +		for (j = 1; j <= max_postd; j++) {
> +			temp = freq * i * j;
> +			if (temp > max || temp < min)
> +				continue;
> +			if (best == 0 || temp < best) {
> +				best = temp;
> +				pred = i;
> +				postd = j;
> +			}
> +		}
> +	}
> +
> +	if (best == 0) {
> +		printf("Fail to set rate to %dKHz", freq);
> +		return;
> +	}
> +
> +	debug("best %d, pred = %d, postd = %d\n", best, pred, postd);
> +
> +	pll_div = best / hck;
> +	pll_denom = 1000000;
> +	pll_num = (best - hck * pll_div) * pll_denom / hck;
> +
> +	/*
> +	 *                                  pll_num
> +	 *             (24MHz * (pll_div + --------- ))
> +	 *                                 pll_denom
> +	 *freq KHz =  --------------------------------
> +	 *             post_div * pred * postd * 1000
> +	 */
> +
> +	if (base_addr == LCDIF1_BASE_ADDR) {
> +		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
> +			return;
> +
> +		/* Select pre-lcd clock to PLL5 and set pre divider */
> +		clrsetbits_le32(&imx_ccm->cscdr2,
> +				MXC_CCM_CSCDR2_LCDIF1_PRED_SEL_MASK |
> +				MXC_CCM_CSCDR2_LCDIF1_PRE_DIV_MASK,
> +				(0x2 << MXC_CCM_CSCDR2_LCDIF1_PRED_SEL_OFFSET) |
> +				((pred - 1) <<
> +				 MXC_CCM_CSCDR2_LCDIF1_PRE_DIV_OFFSET));
> +
> +		/* Set the post divider */
> +		clrsetbits_le32(&imx_ccm->cbcmr,
> +				MXC_CCM_CBCMR_LCDIF1_PODF_MASK,
> +				((postd - 1) <<
> +				 MXC_CCM_CBCMR_LCDIF1_PODF_OFFSET));
> +	}
> +
> +	if (is_cpu_type(MXC_CPU_MX6SX)) {
> +		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
> +			return;

This is not very good readable. If the cpu is i.MX6SX and LCD is LCDIF1,
is enable_pll_video called twice ? Why ?

Should we not use something like:

	if (lcd == LCDIF1 || is_cpu_type(MXC_CPU_MX6SX)) {
		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
		return;

	}

> +
> +		/* Select pre-lcd clock to PLL5 and set pre divider */
> +		clrsetbits_le32(&imx_ccm->cscdr2,
> +				MXC_CCM_CSCDR2_LCDIF2_PRED_SEL_MASK |
> +				MXC_CCM_CSCDR2_LCDIF2_PRE_DIV_MASK,
> +				(0x2 << MXC_CCM_CSCDR2_LCDIF2_PRED_SEL_OFFSET) |
> +				((pred - 1) <<
> +				 MXC_CCM_CSCDR2_LCDIF2_PRE_DIV_OFFSET));
> +
> +		/* Set the post divider */
> +		clrsetbits_le32(&imx_ccm->cscmr1,
> +				MXC_CCM_CSCMR1_LCDIF2_PODF_MASK,
> +				((postd - 1) <<
> +				 MXC_CCM_CSCMR1_LCDIF2_PODF_OFFSET));
> +	}
> +}
> +
> +void enable_lcdif_clock(u32 index)
> +{

You see, you are already using an index. Now you have a mix sometimes
with an enumeration (this index), sometimes with "base_addr".

> +	u32 reg = 0;
> +	u32 lcdif_clk_sel_mask, lcdif_ccgr3_mask;
> +
> +	if (is_cpu_type(MXC_CPU_MX6SX)) {
> +		if (index > 1)
> +			return;
> +		/* Set to pre-mux clock at default */
> +		lcdif_clk_sel_mask = (index == 1) ?
> +			MXC_CCM_CSCDR2_LCDIF2_CLK_SEL_MASK :
> +			MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK;
> +		lcdif_ccgr3_mask = (index == 1) ?
> +			(MXC_CCM_CCGR3_LCDIF2_PIX_MASK |
> +			 MXC_CCM_CCGR3_DISP_AXI_MASK) :
> +			(MXC_CCM_CCGR3_LCDIF1_PIX_MASK |
> +			 MXC_CCM_CCGR3_DISP_AXI_MASK);
> +	} else if (is_cpu_type(MXC_CPU_MX6UL)) {
> +		if (index > 0)
> +			return;
> +		/* Set to pre-mux clock at default */
> +		lcdif_clk_sel_mask = MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK;
> +		lcdif_ccgr3_mask =  MXC_CCM_CCGR3_LCDIF1_PIX_MASK;
> +	} else {
> +		return;
> +	}
> +
> +	reg = readl(&imx_ccm->cscdr2);
> +	reg &= ~lcdif_clk_sel_mask;
> +	writel(reg, &imx_ccm->cscdr2);
> +
> +	/* Enable the LCDIF pix clock */
> +	reg = readl(&imx_ccm->CCGR3);
> +	reg |= lcdif_ccgr3_mask;
> +	writel(reg, &imx_ccm->CCGR3);
> +
> +	reg = readl(&imx_ccm->CCGR2);
> +	reg |= MXC_CCM_CCGR2_LCD_MASK;
> +	writel(reg, &imx_ccm->CCGR2);
> +}
> +#endif
> +
>  #ifdef CONFIG_FSL_QSPI
>  /* qspi_num can be from 0 - 1 */
>  void enable_qspi_clk(int qspi_num)
> diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h
> index 2b220d6..15d14f0 100644
> --- a/arch/arm/include/asm/arch-mx6/clock.h
> +++ b/arch/arm/include/asm/arch-mx6/clock.h
> @@ -66,6 +66,8 @@ int enable_spi_clk(unsigned char enable, unsigned spi_num);
>  void enable_ipu_clock(void);
>  int enable_fec_anatop_clock(int fec_id, enum enet_freq freq);
>  void enable_enet_clk(unsigned char enable);
> +void enable_lcdif_clock(unsigned int index);
>  void enable_qspi_clk(int qspi_num);
>  void enable_thermal_clk(void);
> +void mxs_set_lcdclk(u32 base_addr, u32 freq);
>  #endif /* __ASM_ARCH_CLOCK_H */
> 

Best regards,
Stefano Babic
Peng Fan Oct. 26, 2015, 3:13 a.m. UTC | #2
Hi Stefano,

On Tue, Oct 20, 2015 at 03:39:32PM +0200, Stefano Babic wrote:
>Hi Peng,
>
>On 20/10/2015 13:39, Peng Fan wrote:
>> Implement mxs_set_lcdclk, enable_lcdif_clock and enable_pll_video.
>> The three API can be used to configure lcdif related clock when
>> CONFIG_VIDEO_MXS enabled.
>> 
>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
>> Cc: Stefano Babic <sbabic@denx.de>
>> ---
>> 
>> V2:
>>  none
>> 
>>  arch/arm/cpu/armv7/mx6/clock.c        | 239 ++++++++++++++++++++++++++++++++++
>>  arch/arm/include/asm/arch-mx6/clock.h |   2 +
>>  2 files changed, 241 insertions(+)
>> 
>> diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c
>> index 11efd12..8a88378 100644
>> --- a/arch/arm/cpu/armv7/mx6/clock.c
>> +++ b/arch/arm/cpu/armv7/mx6/clock.c
>> @@ -473,6 +473,245 @@ static u32 get_mmdc_ch0_clk(void)
>>  	}
>>  }
>>  
>> +#if defined(CONFIG_VIDEO_MXS)
>> +static int enable_pll_video(u32 pll_div, u32 pll_num, u32 pll_denom,
>> +			    u32 post_div)
>> +{
>> +	u32 reg = 0;
>> +	ulong start;
>> +
>> +	debug("pll5 div = %d, num = %d, denom = %d\n",
>> +	      pll_div, pll_num, pll_denom);
>> +
>> +	/* Power up PLL5 video */
>> +	writel(BM_ANADIG_PLL_VIDEO_POWERDOWN |
>> +	       BM_ANADIG_PLL_VIDEO_BYPASS |
>> +	       BM_ANADIG_PLL_VIDEO_DIV_SELECT |
>> +	       BM_ANADIG_PLL_VIDEO_POST_DIV_SELECT,
>> +	       &imx_ccm->analog_pll_video_clr);
>> +
>> +	/* Set div, num and denom */
>> +	switch (post_div) {
>> +	case 1:
>> +		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
>> +		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x2),
>> +		       &imx_ccm->analog_pll_video_set);
>> +		break;
>> +	case 2:
>> +		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
>> +		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x1),
>> +		       &imx_ccm->analog_pll_video_set);
>> +		break;
>> +	case 4:
>> +		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
>> +		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x0),
>> +		       &imx_ccm->analog_pll_video_set);
>> +		break;
>> +	default:
>> +		puts("Wrong test_div!\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	writel(BF_ANADIG_PLL_VIDEO_NUM_A(pll_num),
>> +	       &imx_ccm->analog_pll_video_num);
>> +	writel(BF_ANADIG_PLL_VIDEO_DENOM_B(pll_denom),
>> +	       &imx_ccm->analog_pll_video_denom);
>> +
>> +	/* Wait PLL5 lock */
>> +	start = get_timer(0);	/* Get current timestamp */
>> +
>> +	do {
>> +		reg = readl(&imx_ccm->analog_pll_video);
>> +		if (reg & BM_ANADIG_PLL_VIDEO_LOCK) {
>> +			/* Enable PLL out */
>> +			writel(BM_ANADIG_PLL_VIDEO_ENABLE,
>> +			       &imx_ccm->analog_pll_video_set);
>> +			return 0;
>> +		}
>> +	} while (get_timer(0) < (start + 10)); /* Wait 10ms */
>> +
>> +	printf("Lock PLL5 timeout\n");
>
>Maybe puts() is better here

Thanks, will use puts in V3.

>
>> +
>> +	return -ETIME;
>> +}
>> +
>> +/*
>> + * 24M--> PLL_VIDEO -> LCDIFx_PRED -> LCDIFx_PODF -> LCD
>> + *
>> + * 'freq' using KHz as unit, see driver/video/mxsfb.c.
>> + */
>> +void mxs_set_lcdclk(u32 base_addr, u32 freq)
>> +{
>> +	u32 reg = 0;
>> +	u32 hck = MXC_HCLK / 1000;
>> +	/* DIV_SELECT ranges from 27 to 54 */
>> +	u32 min = hck * 27;
>> +	u32 max = hck * 54;
>> +	u32 temp, best = 0;
>> +	u32 i, j, max_pred = 8, max_postd = 8, pred = 1, postd = 1;
>> +	u32 pll_div, pll_num, pll_denom, post_div = 1;
>> +
>> +	debug("mxs_set_lcdclk, freq = %dKHz\n", freq);
>> +
>> +	if ((!is_cpu_type(MXC_CPU_MX6SX)) && !is_cpu_type(MXC_CPU_MX6UL))
>> +		return;
>> +
>> +	if (base_addr == LCDIF1_BASE_ADDR) {
>
>As mentioned before: base_addr is really an index, it is not used as
>address. The interface is then confusing.

Will switch to use index.

>
>> +		reg = readl(&imx_ccm->cscdr2);
>> +		/* Can't change clocks when clock not from pre-mux */
>> +		if ((reg & MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK) != 0)
>> +			return;
>> +	}
>> +
>> +	if (is_cpu_type(MXC_CPU_MX6SX)) {
>> +		reg = readl(&imx_ccm->cscdr2);
>> +		/* Can't change clocks when clock not from pre-mux */
>> +		if ((reg & MXC_CCM_CSCDR2_LCDIF2_CLK_SEL_MASK) != 0)
>> +			return;
>> +	}
>> +
>> +	temp = freq * max_pred * max_postd;
>> +	if (temp > max) {
>> +		puts("Please decrease freq, too large!\n");
>> +		return;
>> +	}
>> +	if (temp < min) {
>> +		/*
>> +		 * Register: PLL_VIDEO
>> +		 * Bit Field: POST_DIV_SELECT
>> +		 * 00 — Divide by 4.
>> +		 * 01 — Divide by 2.
>> +		 * 10 — Divide by 1.
>> +		 * 11 — Reserved
>> +		 * No need to check post_div(1)
>> +		 */
>> +		for (post_div = 2; post_div <= 4; post_div <<= 1) {
>> +			if ((temp * post_div) > min) {
>> +				freq *= post_div;
>> +				break;
>> +			}
>> +		}
>> +
>> +		if (post_div > 4) {
>> +			printf("Fail to set rate to %dkhz", freq);
>> +			return;
>> +		}
>> +	}
>
>It is not clear what happens in the calling function in error case. The
>error is not propagate to the caller. Of course, we see the error on the
>console. Is it enough ? The caller will tzry to go on setting the LCD
>controller even in case this function fails.

Maybe we should change the prototype of 
"void mxs_set_lcdclk(u32 freq)" to "int mxs_set_lcdclk(u32 index, u32 freq);"

>
>
>> +
>> +	/* Choose the best pred and postd to match freq for lcd */
>> +	for (i = 1; i <= max_pred; i++) {
>> +		for (j = 1; j <= max_postd; j++) {
>> +			temp = freq * i * j;
>> +			if (temp > max || temp < min)
>> +				continue;
>> +			if (best == 0 || temp < best) {
>> +				best = temp;
>> +				pred = i;
>> +				postd = j;
>> +			}
>> +		}
>> +	}
>> +
>> +	if (best == 0) {
>> +		printf("Fail to set rate to %dKHz", freq);
>> +		return;
>> +	}
>> +
>> +	debug("best %d, pred = %d, postd = %d\n", best, pred, postd);
>> +
>> +	pll_div = best / hck;
>> +	pll_denom = 1000000;
>> +	pll_num = (best - hck * pll_div) * pll_denom / hck;
>> +
>> +	/*
>> +	 *                                  pll_num
>> +	 *             (24MHz * (pll_div + --------- ))
>> +	 *                                 pll_denom
>> +	 *freq KHz =  --------------------------------
>> +	 *             post_div * pred * postd * 1000
>> +	 */
>> +
>> +	if (base_addr == LCDIF1_BASE_ADDR) {
>> +		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
>> +			return;
>> +
>> +		/* Select pre-lcd clock to PLL5 and set pre divider */
>> +		clrsetbits_le32(&imx_ccm->cscdr2,
>> +				MXC_CCM_CSCDR2_LCDIF1_PRED_SEL_MASK |
>> +				MXC_CCM_CSCDR2_LCDIF1_PRE_DIV_MASK,
>> +				(0x2 << MXC_CCM_CSCDR2_LCDIF1_PRED_SEL_OFFSET) |
>> +				((pred - 1) <<
>> +				 MXC_CCM_CSCDR2_LCDIF1_PRE_DIV_OFFSET));
>> +
>> +		/* Set the post divider */
>> +		clrsetbits_le32(&imx_ccm->cbcmr,
>> +				MXC_CCM_CBCMR_LCDIF1_PODF_MASK,
>> +				((postd - 1) <<
>> +				 MXC_CCM_CBCMR_LCDIF1_PODF_OFFSET));
>> +	}
>> +
>> +	if (is_cpu_type(MXC_CPU_MX6SX)) {
>> +		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
>> +			return;
>
>This is not very good readable. If the cpu is i.MX6SX and LCD is LCDIF1,
>is enable_pll_video called twice ? Why ?

My bad. Will fix this in V3.

>
>Should we not use something like:
>
>	if (lcd == LCDIF1 || is_cpu_type(MXC_CPU_MX6SX)) {
>		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
>		return;
>
>	}
>
>> +
>> +		/* Select pre-lcd clock to PLL5 and set pre divider */
>> +		clrsetbits_le32(&imx_ccm->cscdr2,
>> +				MXC_CCM_CSCDR2_LCDIF2_PRED_SEL_MASK |
>> +				MXC_CCM_CSCDR2_LCDIF2_PRE_DIV_MASK,
>> +				(0x2 << MXC_CCM_CSCDR2_LCDIF2_PRED_SEL_OFFSET) |
>> +				((pred - 1) <<
>> +				 MXC_CCM_CSCDR2_LCDIF2_PRE_DIV_OFFSET));
>> +
>> +		/* Set the post divider */
>> +		clrsetbits_le32(&imx_ccm->cscmr1,
>> +				MXC_CCM_CSCMR1_LCDIF2_PODF_MASK,
>> +				((postd - 1) <<
>> +				 MXC_CCM_CSCMR1_LCDIF2_PODF_OFFSET));
>> +	}
>> +}
>> +
>> +void enable_lcdif_clock(u32 index)
>> +{
>
>You see, you are already using an index. Now you have a mix sometimes
>with an enumeration (this index), sometimes with "base_addr".

Will fix global in the patch set about usage of base_addr. Switch to use index.

Thanks,
Peng.

>
>> +	u32 reg = 0;
>> +	u32 lcdif_clk_sel_mask, lcdif_ccgr3_mask;
>> +
>> +	if (is_cpu_type(MXC_CPU_MX6SX)) {
>> +		if (index > 1)
>> +			return;
>> +		/* Set to pre-mux clock at default */
>> +		lcdif_clk_sel_mask = (index == 1) ?
>> +			MXC_CCM_CSCDR2_LCDIF2_CLK_SEL_MASK :
>> +			MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK;
>> +		lcdif_ccgr3_mask = (index == 1) ?
>> +			(MXC_CCM_CCGR3_LCDIF2_PIX_MASK |
>> +			 MXC_CCM_CCGR3_DISP_AXI_MASK) :
>> +			(MXC_CCM_CCGR3_LCDIF1_PIX_MASK |
>> +			 MXC_CCM_CCGR3_DISP_AXI_MASK);
>> +	} else if (is_cpu_type(MXC_CPU_MX6UL)) {
>> +		if (index > 0)
>> +			return;
>> +		/* Set to pre-mux clock at default */
>> +		lcdif_clk_sel_mask = MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK;
>> +		lcdif_ccgr3_mask =  MXC_CCM_CCGR3_LCDIF1_PIX_MASK;
>> +	} else {
>> +		return;
>> +	}
>> +
>> +	reg = readl(&imx_ccm->cscdr2);
>> +	reg &= ~lcdif_clk_sel_mask;
>> +	writel(reg, &imx_ccm->cscdr2);
>> +
>> +	/* Enable the LCDIF pix clock */
>> +	reg = readl(&imx_ccm->CCGR3);
>> +	reg |= lcdif_ccgr3_mask;
>> +	writel(reg, &imx_ccm->CCGR3);
>> +
>> +	reg = readl(&imx_ccm->CCGR2);
>> +	reg |= MXC_CCM_CCGR2_LCD_MASK;
>> +	writel(reg, &imx_ccm->CCGR2);
>> +}
>> +#endif
>> +
>>  #ifdef CONFIG_FSL_QSPI
>>  /* qspi_num can be from 0 - 1 */
>>  void enable_qspi_clk(int qspi_num)
>> diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h
>> index 2b220d6..15d14f0 100644
>> --- a/arch/arm/include/asm/arch-mx6/clock.h
>> +++ b/arch/arm/include/asm/arch-mx6/clock.h
>> @@ -66,6 +66,8 @@ int enable_spi_clk(unsigned char enable, unsigned spi_num);
>>  void enable_ipu_clock(void);
>>  int enable_fec_anatop_clock(int fec_id, enum enet_freq freq);
>>  void enable_enet_clk(unsigned char enable);
>> +void enable_lcdif_clock(unsigned int index);
>>  void enable_qspi_clk(int qspi_num);
>>  void enable_thermal_clk(void);
>> +void mxs_set_lcdclk(u32 base_addr, u32 freq);
>>  #endif /* __ASM_ARCH_CLOCK_H */
>> 
>
>Best regards,
>Stefano Babic
>
>-- 
>=====================================================================
>DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
>=====================================================================
Peng Fan Oct. 27, 2015, 7:36 a.m. UTC | #3
Hi Stefano,

On Tue, Oct 20, 2015 at 03:39:32PM +0200, Stefano Babic wrote:
>Hi Peng,
>
>On 20/10/2015 13:39, Peng Fan wrote:
>> Implement mxs_set_lcdclk, enable_lcdif_clock and enable_pll_video.
>> The three API can be used to configure lcdif related clock when
>> CONFIG_VIDEO_MXS enabled.
>> 
>> Signed-off-by: Peng Fan <Peng.Fan@freescale.com>
>> Cc: Stefano Babic <sbabic@denx.de>
>> ---
>> 
>> V2:
>>  none
>> 
>>  arch/arm/cpu/armv7/mx6/clock.c        | 239 ++++++++++++++++++++++++++++++++++
>>  arch/arm/include/asm/arch-mx6/clock.h |   2 +
>>  2 files changed, 241 insertions(+)
>> 
>> diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c
>> index 11efd12..8a88378 100644
>> --- a/arch/arm/cpu/armv7/mx6/clock.c
>> +++ b/arch/arm/cpu/armv7/mx6/clock.c
>> @@ -473,6 +473,245 @@ static u32 get_mmdc_ch0_clk(void)
>>  	}
>>  }
>>  
>> +#if defined(CONFIG_VIDEO_MXS)
>> +static int enable_pll_video(u32 pll_div, u32 pll_num, u32 pll_denom,
>> +			    u32 post_div)
>> +{
>> +	u32 reg = 0;
>> +	ulong start;
>> +
>> +	debug("pll5 div = %d, num = %d, denom = %d\n",
>> +	      pll_div, pll_num, pll_denom);
>> +
>> +	/* Power up PLL5 video */
>> +	writel(BM_ANADIG_PLL_VIDEO_POWERDOWN |
>> +	       BM_ANADIG_PLL_VIDEO_BYPASS |
>> +	       BM_ANADIG_PLL_VIDEO_DIV_SELECT |
>> +	       BM_ANADIG_PLL_VIDEO_POST_DIV_SELECT,
>> +	       &imx_ccm->analog_pll_video_clr);
>> +
>> +	/* Set div, num and denom */
>> +	switch (post_div) {
>> +	case 1:
>> +		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
>> +		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x2),
>> +		       &imx_ccm->analog_pll_video_set);
>> +		break;
>> +	case 2:
>> +		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
>> +		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x1),
>> +		       &imx_ccm->analog_pll_video_set);
>> +		break;
>> +	case 4:
>> +		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
>> +		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x0),
>> +		       &imx_ccm->analog_pll_video_set);
>> +		break;
>> +	default:
>> +		puts("Wrong test_div!\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	writel(BF_ANADIG_PLL_VIDEO_NUM_A(pll_num),
>> +	       &imx_ccm->analog_pll_video_num);
>> +	writel(BF_ANADIG_PLL_VIDEO_DENOM_B(pll_denom),
>> +	       &imx_ccm->analog_pll_video_denom);
>> +
>> +	/* Wait PLL5 lock */
>> +	start = get_timer(0);	/* Get current timestamp */
>> +
>> +	do {
>> +		reg = readl(&imx_ccm->analog_pll_video);
>> +		if (reg & BM_ANADIG_PLL_VIDEO_LOCK) {
>> +			/* Enable PLL out */
>> +			writel(BM_ANADIG_PLL_VIDEO_ENABLE,
>> +			       &imx_ccm->analog_pll_video_set);
>> +			return 0;
>> +		}
>> +	} while (get_timer(0) < (start + 10)); /* Wait 10ms */
>> +
>> +	printf("Lock PLL5 timeout\n");
>
>Maybe puts() is better here
>
>> +
>> +	return -ETIME;
>> +}
>> +
>> +/*
>> + * 24M--> PLL_VIDEO -> LCDIFx_PRED -> LCDIFx_PODF -> LCD
>> + *
>> + * 'freq' using KHz as unit, see driver/video/mxsfb.c.
>> + */
>> +void mxs_set_lcdclk(u32 base_addr, u32 freq)
>> +{
>> +	u32 reg = 0;
>> +	u32 hck = MXC_HCLK / 1000;
>> +	/* DIV_SELECT ranges from 27 to 54 */
>> +	u32 min = hck * 27;
>> +	u32 max = hck * 54;
>> +	u32 temp, best = 0;
>> +	u32 i, j, max_pred = 8, max_postd = 8, pred = 1, postd = 1;
>> +	u32 pll_div, pll_num, pll_denom, post_div = 1;
>> +
>> +	debug("mxs_set_lcdclk, freq = %dKHz\n", freq);
>> +
>> +	if ((!is_cpu_type(MXC_CPU_MX6SX)) && !is_cpu_type(MXC_CPU_MX6UL))
>> +		return;
>> +
>> +	if (base_addr == LCDIF1_BASE_ADDR) {
>
>As mentioned before: base_addr is really an index, it is not used as
>address. The interface is then confusing.
>
>> +		reg = readl(&imx_ccm->cscdr2);
>> +		/* Can't change clocks when clock not from pre-mux */
>> +		if ((reg & MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK) != 0)
>> +			return;
>> +	}
>> +
>> +	if (is_cpu_type(MXC_CPU_MX6SX)) {
>> +		reg = readl(&imx_ccm->cscdr2);
>> +		/* Can't change clocks when clock not from pre-mux */
>> +		if ((reg & MXC_CCM_CSCDR2_LCDIF2_CLK_SEL_MASK) != 0)
>> +			return;
>> +	}
>> +
>> +	temp = freq * max_pred * max_postd;
>> +	if (temp > max) {
>> +		puts("Please decrease freq, too large!\n");
>> +		return;
>> +	}
>> +	if (temp < min) {
>> +		/*
>> +		 * Register: PLL_VIDEO
>> +		 * Bit Field: POST_DIV_SELECT
>> +		 * 00 — Divide by 4.
>> +		 * 01 — Divide by 2.
>> +		 * 10 — Divide by 1.
>> +		 * 11 — Reserved
>> +		 * No need to check post_div(1)
>> +		 */
>> +		for (post_div = 2; post_div <= 4; post_div <<= 1) {
>> +			if ((temp * post_div) > min) {
>> +				freq *= post_div;
>> +				break;
>> +			}
>> +		}
>> +
>> +		if (post_div > 4) {
>> +			printf("Fail to set rate to %dkhz", freq);
>> +			return;
>> +		}
>> +	}
>
>It is not clear what happens in the calling function in error case. The
>error is not propagate to the caller. Of course, we see the error on the
>console. Is it enough ? The caller will tzry to go on setting the LCD
>controller even in case this function fails.
>
>
>> +
>> +	/* Choose the best pred and postd to match freq for lcd */
>> +	for (i = 1; i <= max_pred; i++) {
>> +		for (j = 1; j <= max_postd; j++) {
>> +			temp = freq * i * j;
>> +			if (temp > max || temp < min)
>> +				continue;
>> +			if (best == 0 || temp < best) {
>> +				best = temp;
>> +				pred = i;
>> +				postd = j;
>> +			}
>> +		}
>> +	}
>> +
>> +	if (best == 0) {
>> +		printf("Fail to set rate to %dKHz", freq);
>> +		return;
>> +	}
>> +
>> +	debug("best %d, pred = %d, postd = %d\n", best, pred, postd);
>> +
>> +	pll_div = best / hck;
>> +	pll_denom = 1000000;
>> +	pll_num = (best - hck * pll_div) * pll_denom / hck;
>> +
>> +	/*
>> +	 *                                  pll_num
>> +	 *             (24MHz * (pll_div + --------- ))
>> +	 *                                 pll_denom
>> +	 *freq KHz =  --------------------------------
>> +	 *             post_div * pred * postd * 1000
>> +	 */
>> +
>> +	if (base_addr == LCDIF1_BASE_ADDR) {
>> +		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
>> +			return;
>> +
>> +		/* Select pre-lcd clock to PLL5 and set pre divider */
>> +		clrsetbits_le32(&imx_ccm->cscdr2,
>> +				MXC_CCM_CSCDR2_LCDIF1_PRED_SEL_MASK |
>> +				MXC_CCM_CSCDR2_LCDIF1_PRE_DIV_MASK,
>> +				(0x2 << MXC_CCM_CSCDR2_LCDIF1_PRED_SEL_OFFSET) |
>> +				((pred - 1) <<
>> +				 MXC_CCM_CSCDR2_LCDIF1_PRE_DIV_OFFSET));
>> +
>> +		/* Set the post divider */
>> +		clrsetbits_le32(&imx_ccm->cbcmr,
>> +				MXC_CCM_CBCMR_LCDIF1_PODF_MASK,
>> +				((postd - 1) <<
>> +				 MXC_CCM_CBCMR_LCDIF1_PODF_OFFSET));
>> +	}
>> +
>> +	if (is_cpu_type(MXC_CPU_MX6SX)) {
>> +		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
>> +			return;
>
>This is not very good readable. If the cpu is i.MX6SX and LCD is LCDIF1,
>is enable_pll_video called twice ? Why ?
>
>Should we not use something like:
>
>	if (lcd == LCDIF1 || is_cpu_type(MXC_CPU_MX6SX)) {
>		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
>		return;
>
>	}
>
>> +
>> +		/* Select pre-lcd clock to PLL5 and set pre divider */
>> +		clrsetbits_le32(&imx_ccm->cscdr2,
>> +				MXC_CCM_CSCDR2_LCDIF2_PRED_SEL_MASK |
>> +				MXC_CCM_CSCDR2_LCDIF2_PRE_DIV_MASK,
>> +				(0x2 << MXC_CCM_CSCDR2_LCDIF2_PRED_SEL_OFFSET) |
>> +				((pred - 1) <<
>> +				 MXC_CCM_CSCDR2_LCDIF2_PRE_DIV_OFFSET));
>> +
>> +		/* Set the post divider */
>> +		clrsetbits_le32(&imx_ccm->cscmr1,
>> +				MXC_CCM_CSCMR1_LCDIF2_PODF_MASK,
>> +				((postd - 1) <<
>> +				 MXC_CCM_CSCMR1_LCDIF2_PODF_OFFSET));
>> +	}
>> +}
>> +
>> +void enable_lcdif_clock(u32 index)
>> +{
>
>You see, you are already using an index. Now you have a mix sometimes
>with an enumeration (this index), sometimes with "base_addr".

After a thought, I think use base_addr is better. In drivers/video/mxs_fb.c,
MXS_LCDIF_BASE is used, I do not want to introduce another macro such as
MXS_LCDIF_INDEX. So choose base_addr can avoid add more macro definitions
in board header files. I'll switch to use base_addr in the patch set.

Regards,
Peng.
>
>> +	u32 reg = 0;
>> +	u32 lcdif_clk_sel_mask, lcdif_ccgr3_mask;
>> +
>> +	if (is_cpu_type(MXC_CPU_MX6SX)) {
>> +		if (index > 1)
>> +			return;
>> +		/* Set to pre-mux clock at default */
>> +		lcdif_clk_sel_mask = (index == 1) ?
>> +			MXC_CCM_CSCDR2_LCDIF2_CLK_SEL_MASK :
>> +			MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK;
>> +		lcdif_ccgr3_mask = (index == 1) ?
>> +			(MXC_CCM_CCGR3_LCDIF2_PIX_MASK |
>> +			 MXC_CCM_CCGR3_DISP_AXI_MASK) :
>> +			(MXC_CCM_CCGR3_LCDIF1_PIX_MASK |
>> +			 MXC_CCM_CCGR3_DISP_AXI_MASK);
>> +	} else if (is_cpu_type(MXC_CPU_MX6UL)) {
>> +		if (index > 0)
>> +			return;
>> +		/* Set to pre-mux clock at default */
>> +		lcdif_clk_sel_mask = MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK;
>> +		lcdif_ccgr3_mask =  MXC_CCM_CCGR3_LCDIF1_PIX_MASK;
>> +	} else {
>> +		return;
>> +	}
>> +
>> +	reg = readl(&imx_ccm->cscdr2);
>> +	reg &= ~lcdif_clk_sel_mask;
>> +	writel(reg, &imx_ccm->cscdr2);
>> +
>> +	/* Enable the LCDIF pix clock */
>> +	reg = readl(&imx_ccm->CCGR3);
>> +	reg |= lcdif_ccgr3_mask;
>> +	writel(reg, &imx_ccm->CCGR3);
>> +
>> +	reg = readl(&imx_ccm->CCGR2);
>> +	reg |= MXC_CCM_CCGR2_LCD_MASK;
>> +	writel(reg, &imx_ccm->CCGR2);
>> +}
>> +#endif
>> +
>>  #ifdef CONFIG_FSL_QSPI
>>  /* qspi_num can be from 0 - 1 */
>>  void enable_qspi_clk(int qspi_num)
>> diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h
>> index 2b220d6..15d14f0 100644
>> --- a/arch/arm/include/asm/arch-mx6/clock.h
>> +++ b/arch/arm/include/asm/arch-mx6/clock.h
>> @@ -66,6 +66,8 @@ int enable_spi_clk(unsigned char enable, unsigned spi_num);
>>  void enable_ipu_clock(void);
>>  int enable_fec_anatop_clock(int fec_id, enum enet_freq freq);
>>  void enable_enet_clk(unsigned char enable);
>> +void enable_lcdif_clock(unsigned int index);
>>  void enable_qspi_clk(int qspi_num);
>>  void enable_thermal_clk(void);
>> +void mxs_set_lcdclk(u32 base_addr, u32 freq);
>>  #endif /* __ASM_ARCH_CLOCK_H */
>> 
>
>Best regards,
>Stefano Babic
>
>-- 
>=====================================================================
>DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
>=====================================================================
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c
index 11efd12..8a88378 100644
--- a/arch/arm/cpu/armv7/mx6/clock.c
+++ b/arch/arm/cpu/armv7/mx6/clock.c
@@ -473,6 +473,245 @@  static u32 get_mmdc_ch0_clk(void)
 	}
 }
 
+#if defined(CONFIG_VIDEO_MXS)
+static int enable_pll_video(u32 pll_div, u32 pll_num, u32 pll_denom,
+			    u32 post_div)
+{
+	u32 reg = 0;
+	ulong start;
+
+	debug("pll5 div = %d, num = %d, denom = %d\n",
+	      pll_div, pll_num, pll_denom);
+
+	/* Power up PLL5 video */
+	writel(BM_ANADIG_PLL_VIDEO_POWERDOWN |
+	       BM_ANADIG_PLL_VIDEO_BYPASS |
+	       BM_ANADIG_PLL_VIDEO_DIV_SELECT |
+	       BM_ANADIG_PLL_VIDEO_POST_DIV_SELECT,
+	       &imx_ccm->analog_pll_video_clr);
+
+	/* Set div, num and denom */
+	switch (post_div) {
+	case 1:
+		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
+		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x2),
+		       &imx_ccm->analog_pll_video_set);
+		break;
+	case 2:
+		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
+		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x1),
+		       &imx_ccm->analog_pll_video_set);
+		break;
+	case 4:
+		writel(BF_ANADIG_PLL_VIDEO_DIV_SELECT(pll_div) |
+		       BF_ANADIG_PLL_VIDEO_POST_DIV_SELECT(0x0),
+		       &imx_ccm->analog_pll_video_set);
+		break;
+	default:
+		puts("Wrong test_div!\n");
+		return -EINVAL;
+	}
+
+	writel(BF_ANADIG_PLL_VIDEO_NUM_A(pll_num),
+	       &imx_ccm->analog_pll_video_num);
+	writel(BF_ANADIG_PLL_VIDEO_DENOM_B(pll_denom),
+	       &imx_ccm->analog_pll_video_denom);
+
+	/* Wait PLL5 lock */
+	start = get_timer(0);	/* Get current timestamp */
+
+	do {
+		reg = readl(&imx_ccm->analog_pll_video);
+		if (reg & BM_ANADIG_PLL_VIDEO_LOCK) {
+			/* Enable PLL out */
+			writel(BM_ANADIG_PLL_VIDEO_ENABLE,
+			       &imx_ccm->analog_pll_video_set);
+			return 0;
+		}
+	} while (get_timer(0) < (start + 10)); /* Wait 10ms */
+
+	printf("Lock PLL5 timeout\n");
+
+	return -ETIME;
+}
+
+/*
+ * 24M--> PLL_VIDEO -> LCDIFx_PRED -> LCDIFx_PODF -> LCD
+ *
+ * 'freq' using KHz as unit, see driver/video/mxsfb.c.
+ */
+void mxs_set_lcdclk(u32 base_addr, u32 freq)
+{
+	u32 reg = 0;
+	u32 hck = MXC_HCLK / 1000;
+	/* DIV_SELECT ranges from 27 to 54 */
+	u32 min = hck * 27;
+	u32 max = hck * 54;
+	u32 temp, best = 0;
+	u32 i, j, max_pred = 8, max_postd = 8, pred = 1, postd = 1;
+	u32 pll_div, pll_num, pll_denom, post_div = 1;
+
+	debug("mxs_set_lcdclk, freq = %dKHz\n", freq);
+
+	if ((!is_cpu_type(MXC_CPU_MX6SX)) && !is_cpu_type(MXC_CPU_MX6UL))
+		return;
+
+	if (base_addr == LCDIF1_BASE_ADDR) {
+		reg = readl(&imx_ccm->cscdr2);
+		/* Can't change clocks when clock not from pre-mux */
+		if ((reg & MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK) != 0)
+			return;
+	}
+
+	if (is_cpu_type(MXC_CPU_MX6SX)) {
+		reg = readl(&imx_ccm->cscdr2);
+		/* Can't change clocks when clock not from pre-mux */
+		if ((reg & MXC_CCM_CSCDR2_LCDIF2_CLK_SEL_MASK) != 0)
+			return;
+	}
+
+	temp = freq * max_pred * max_postd;
+	if (temp > max) {
+		puts("Please decrease freq, too large!\n");
+		return;
+	}
+	if (temp < min) {
+		/*
+		 * Register: PLL_VIDEO
+		 * Bit Field: POST_DIV_SELECT
+		 * 00 — Divide by 4.
+		 * 01 — Divide by 2.
+		 * 10 — Divide by 1.
+		 * 11 — Reserved
+		 * No need to check post_div(1)
+		 */
+		for (post_div = 2; post_div <= 4; post_div <<= 1) {
+			if ((temp * post_div) > min) {
+				freq *= post_div;
+				break;
+			}
+		}
+
+		if (post_div > 4) {
+			printf("Fail to set rate to %dkhz", freq);
+			return;
+		}
+	}
+
+	/* Choose the best pred and postd to match freq for lcd */
+	for (i = 1; i <= max_pred; i++) {
+		for (j = 1; j <= max_postd; j++) {
+			temp = freq * i * j;
+			if (temp > max || temp < min)
+				continue;
+			if (best == 0 || temp < best) {
+				best = temp;
+				pred = i;
+				postd = j;
+			}
+		}
+	}
+
+	if (best == 0) {
+		printf("Fail to set rate to %dKHz", freq);
+		return;
+	}
+
+	debug("best %d, pred = %d, postd = %d\n", best, pred, postd);
+
+	pll_div = best / hck;
+	pll_denom = 1000000;
+	pll_num = (best - hck * pll_div) * pll_denom / hck;
+
+	/*
+	 *                                  pll_num
+	 *             (24MHz * (pll_div + --------- ))
+	 *                                 pll_denom
+	 *freq KHz =  --------------------------------
+	 *             post_div * pred * postd * 1000
+	 */
+
+	if (base_addr == LCDIF1_BASE_ADDR) {
+		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
+			return;
+
+		/* Select pre-lcd clock to PLL5 and set pre divider */
+		clrsetbits_le32(&imx_ccm->cscdr2,
+				MXC_CCM_CSCDR2_LCDIF1_PRED_SEL_MASK |
+				MXC_CCM_CSCDR2_LCDIF1_PRE_DIV_MASK,
+				(0x2 << MXC_CCM_CSCDR2_LCDIF1_PRED_SEL_OFFSET) |
+				((pred - 1) <<
+				 MXC_CCM_CSCDR2_LCDIF1_PRE_DIV_OFFSET));
+
+		/* Set the post divider */
+		clrsetbits_le32(&imx_ccm->cbcmr,
+				MXC_CCM_CBCMR_LCDIF1_PODF_MASK,
+				((postd - 1) <<
+				 MXC_CCM_CBCMR_LCDIF1_PODF_OFFSET));
+	}
+
+	if (is_cpu_type(MXC_CPU_MX6SX)) {
+		if (enable_pll_video(pll_div, pll_num, pll_denom, post_div))
+			return;
+
+		/* Select pre-lcd clock to PLL5 and set pre divider */
+		clrsetbits_le32(&imx_ccm->cscdr2,
+				MXC_CCM_CSCDR2_LCDIF2_PRED_SEL_MASK |
+				MXC_CCM_CSCDR2_LCDIF2_PRE_DIV_MASK,
+				(0x2 << MXC_CCM_CSCDR2_LCDIF2_PRED_SEL_OFFSET) |
+				((pred - 1) <<
+				 MXC_CCM_CSCDR2_LCDIF2_PRE_DIV_OFFSET));
+
+		/* Set the post divider */
+		clrsetbits_le32(&imx_ccm->cscmr1,
+				MXC_CCM_CSCMR1_LCDIF2_PODF_MASK,
+				((postd - 1) <<
+				 MXC_CCM_CSCMR1_LCDIF2_PODF_OFFSET));
+	}
+}
+
+void enable_lcdif_clock(u32 index)
+{
+	u32 reg = 0;
+	u32 lcdif_clk_sel_mask, lcdif_ccgr3_mask;
+
+	if (is_cpu_type(MXC_CPU_MX6SX)) {
+		if (index > 1)
+			return;
+		/* Set to pre-mux clock at default */
+		lcdif_clk_sel_mask = (index == 1) ?
+			MXC_CCM_CSCDR2_LCDIF2_CLK_SEL_MASK :
+			MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK;
+		lcdif_ccgr3_mask = (index == 1) ?
+			(MXC_CCM_CCGR3_LCDIF2_PIX_MASK |
+			 MXC_CCM_CCGR3_DISP_AXI_MASK) :
+			(MXC_CCM_CCGR3_LCDIF1_PIX_MASK |
+			 MXC_CCM_CCGR3_DISP_AXI_MASK);
+	} else if (is_cpu_type(MXC_CPU_MX6UL)) {
+		if (index > 0)
+			return;
+		/* Set to pre-mux clock at default */
+		lcdif_clk_sel_mask = MXC_CCM_CSCDR2_LCDIF1_CLK_SEL_MASK;
+		lcdif_ccgr3_mask =  MXC_CCM_CCGR3_LCDIF1_PIX_MASK;
+	} else {
+		return;
+	}
+
+	reg = readl(&imx_ccm->cscdr2);
+	reg &= ~lcdif_clk_sel_mask;
+	writel(reg, &imx_ccm->cscdr2);
+
+	/* Enable the LCDIF pix clock */
+	reg = readl(&imx_ccm->CCGR3);
+	reg |= lcdif_ccgr3_mask;
+	writel(reg, &imx_ccm->CCGR3);
+
+	reg = readl(&imx_ccm->CCGR2);
+	reg |= MXC_CCM_CCGR2_LCD_MASK;
+	writel(reg, &imx_ccm->CCGR2);
+}
+#endif
+
 #ifdef CONFIG_FSL_QSPI
 /* qspi_num can be from 0 - 1 */
 void enable_qspi_clk(int qspi_num)
diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h
index 2b220d6..15d14f0 100644
--- a/arch/arm/include/asm/arch-mx6/clock.h
+++ b/arch/arm/include/asm/arch-mx6/clock.h
@@ -66,6 +66,8 @@  int enable_spi_clk(unsigned char enable, unsigned spi_num);
 void enable_ipu_clock(void);
 int enable_fec_anatop_clock(int fec_id, enum enet_freq freq);
 void enable_enet_clk(unsigned char enable);
+void enable_lcdif_clock(unsigned int index);
 void enable_qspi_clk(int qspi_num);
 void enable_thermal_clk(void);
+void mxs_set_lcdclk(u32 base_addr, u32 freq);
 #endif /* __ASM_ARCH_CLOCK_H */