public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/3] video: ipu: avoid overflow issue
@ 2016-03-09  8:07 Peng Fan
  2016-03-09  8:07 ` [U-Boot] [PATCH 2/3] imx: mx6: correct IPU clock Peng Fan
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Peng Fan @ 2016-03-09  8:07 UTC (permalink / raw)
  To: u-boot

Multiplication, as "clk->parent->rate * 16" may overflow. So use
do_div to avoid such issue.

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
---
 drivers/video/ipu_common.c | 73 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 54 insertions(+), 19 deletions(-)

diff --git a/drivers/video/ipu_common.c b/drivers/video/ipu_common.c
index 9f85102..36d4b23 100644
--- a/drivers/video/ipu_common.c
+++ b/drivers/video/ipu_common.c
@@ -19,6 +19,7 @@
 #include <asm/errno.h>
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/crm_regs.h>
+#include <div64.h>
 #include "ipu.h"
 #include "ipu_regs.h"
 
@@ -275,50 +276,84 @@ static inline void ipu_ch_param_set_buffer(uint32_t ch, int bufNum,
 
 static void ipu_pixel_clk_recalc(struct clk *clk)
 {
-	u32 div = __raw_readl(DI_BS_CLKGEN0(clk->id));
-	if (div == 0)
-		clk->rate = 0;
-	else
-		clk->rate = (clk->parent->rate * 16) / div;
+	u32 div;
+	u64 final_rate = (unsigned long long)clk->parent->rate * 16;
+
+	div = __raw_readl(DI_BS_CLKGEN0(clk->id));
+	debug("read BS_CLKGEN0 div:%d, final_rate:%lld, prate:%ld\n",
+	      div, final_rate, clk->parent->rate);
+
+	clk->rate = 0;
+	if (div != 0) {
+		do_div(final_rate, div);
+		clk->rate = final_rate;
+	}
 }
 
 static unsigned long ipu_pixel_clk_round_rate(struct clk *clk,
 	unsigned long rate)
 {
-	u32 div, div1;
-	u32 tmp;
+	u64 div, final_rate;
+	u32 remainder;
+	u64 parent_rate = (unsigned long long)clk->parent->rate * 16;
+
 	/*
 	 * Calculate divider
 	 * Fractional part is 4 bits,
 	 * so simply multiply by 2^4 to get fractional part.
 	 */
-	tmp = (clk->parent->rate * 16);
-	div = tmp / rate;
-
+	div = parent_rate;
+	remainder = do_div(div, rate);
+	/* Round the divider value */
+	if (remainder > (rate / 2))
+		div++;
 	if (div < 0x10)            /* Min DI disp clock divider is 1 */
 		div = 0x10;
 	if (div & ~0xFEF)
 		div &= 0xFF8;
 	else {
-		div1 = div & 0xFE0;
-		if ((tmp/div1 - tmp/div) < rate / 4)
-			div = div1;
-		else
-			div &= 0xFF8;
+		/* Round up divider if it gets us closer to desired pix clk */
+		if ((div & 0xC) == 0xC) {
+			div += 0x10;
+			div &= ~0xF;
+		}
 	}
-	return (clk->parent->rate * 16) / div;
+	final_rate = parent_rate;
+	do_div(final_rate, div);
+
+	return final_rate;
 }
 
 static int ipu_pixel_clk_set_rate(struct clk *clk, unsigned long rate)
 {
-	u32 div = (clk->parent->rate * 16) / rate;
+	u64 div, parent_rate;
+	u32 remainder;
+
+	parent_rate = (unsigned long long)clk->parent->rate * 16;
+	div = parent_rate;
+	remainder = do_div(div, rate);
+	/* Round the divider value */
+	if (remainder > (rate / 2))
+		div++;
+
+	/* Round up divider if it gets us closer to desired pix clk */
+	if ((div & 0xC) == 0xC) {
+		div += 0x10;
+		div &= ~0xF;
+	}
+	if (div > 0x1000)
+		debug("Overflow, DI_BS_CLKGEN0 div:0x%x\n", (u32)div);
 
 	__raw_writel(div, DI_BS_CLKGEN0(clk->id));
 
-	/* Setup pixel clock timing */
+	/*
+	 * Setup pixel clock timing
+	 * Down time is half of period
+	 */
 	__raw_writel((div / 16) << 16, DI_BS_CLKGEN1(clk->id));
 
-	clk->rate = (clk->parent->rate * 16) / div;
+	clk->rate = (u64)(clk->parent->rate * 16) / div;
+
 	return 0;
 }
 
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 2/3] imx: mx6: correct IPU clock
  2016-03-09  8:07 [U-Boot] [PATCH 1/3] video: ipu: avoid overflow issue Peng Fan
@ 2016-03-09  8:07 ` Peng Fan
  2016-03-14 21:56   ` Anatolij Gustschin
  2017-09-05  2:37   ` [U-Boot] [U-Boot,2/3] " Eric Nelson
  2016-03-09  8:07 ` [U-Boot] [PATCH 3/3] imx: mx6: hdmi: handle overflow condition Peng Fan
  2016-03-14 21:56 ` [U-Boot] [PATCH 1/3] video: ipu: avoid overflow issue Anatolij Gustschin
  2 siblings, 2 replies; 15+ messages in thread
From: Peng Fan @ 2016-03-09  8:07 UTC (permalink / raw)
  To: u-boot

The CONFIG_IPUV3_CLK should be 264000000, to i.MX6DL, it should be
198000000.

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Peter Robinson <pbrobinson@gmail.com>
---
 include/configs/mx6sabre_common.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/configs/mx6sabre_common.h b/include/configs/mx6sabre_common.h
index 29d1f91..a6d821b 100644
--- a/include/configs/mx6sabre_common.h
+++ b/include/configs/mx6sabre_common.h
@@ -225,7 +225,11 @@
 #define CONFIG_BMP_16BPP
 #define CONFIG_VIDEO_LOGO
 #define CONFIG_VIDEO_BMP_LOGO
-#define CONFIG_IPUV3_CLK 260000000
+#ifdef CONFIG_MX6DL
+#define CONFIG_IPUV3_CLK 198000000
+#else
+#define CONFIG_IPUV3_CLK 264000000
+#endif
 #define CONFIG_IMX_HDMI
 #define CONFIG_IMX_VIDEO_SKIP
 
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6: hdmi: handle overflow condition
  2016-03-09  8:07 [U-Boot] [PATCH 1/3] video: ipu: avoid overflow issue Peng Fan
  2016-03-09  8:07 ` [U-Boot] [PATCH 2/3] imx: mx6: correct IPU clock Peng Fan
@ 2016-03-09  8:07 ` Peng Fan
  2016-03-14 21:57   ` Anatolij Gustschin
  2016-03-14 21:56 ` [U-Boot] [PATCH 1/3] video: ipu: avoid overflow issue Anatolij Gustschin
  2 siblings, 1 reply; 15+ messages in thread
From: Peng Fan @ 2016-03-09  8:07 UTC (permalink / raw)
  To: u-boot

If HDMI_IH_FC_STAT2_OVERFLOW_MASK is set, we need to
do TMDS software reset and write to clear fc_invidconf register.
We need minimum 3 times to write to clear the fc_invidconf
register, so choose 5 loops here.

Signed-off-by: Peng Fan <van.freenix@gmail.com>
Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/cpu/armv7/mx6/soc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c
index a34675c..91a3deb 100644
--- a/arch/arm/cpu/armv7/mx6/soc.c
+++ b/arch/arm/cpu/armv7/mx6/soc.c
@@ -548,7 +548,8 @@ void imx_setup_hdmi(void)
 {
 	struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
 	struct hdmi_regs *hdmi  = (struct hdmi_regs *)HDMI_ARB_BASE_ADDR;
-	int reg;
+	int reg, count;
+	u8 val;
 
 	/* Turn on HDMI PHY clock */
 	reg = readl(&mxc_ccm->CCGR2);
@@ -565,6 +566,16 @@ void imx_setup_hdmi(void)
 		 |(CHSCCDR_IPU_PRE_CLK_540M_PFD
 		 << MXC_CCM_CHSCCDR_IPU1_DI0_PRE_CLK_SEL_OFFSET);
 	writel(reg, &mxc_ccm->chsccdr);
+
+	/* Clear the overflow condition */
+	if (readb(&hdmi->ih_fc_stat2) & HDMI_IH_FC_STAT2_OVERFLOW_MASK) {
+		/* TMDS software reset */
+		writeb((u8)~HDMI_MC_SWRSTZ_TMDSSWRST_REQ, &hdmi->mc_swrstz);
+		val = readb(&hdmi->fc_invidconf);
+		/* Need minimum 3 times to write to clear the register */
+		for (count = 0 ; count < 5 ; count++)
+			writeb(val, &hdmi->fc_invidconf);
+	}
 }
 #endif
 
-- 
2.6.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/3] video: ipu: avoid overflow issue
  2016-03-09  8:07 [U-Boot] [PATCH 1/3] video: ipu: avoid overflow issue Peng Fan
  2016-03-09  8:07 ` [U-Boot] [PATCH 2/3] imx: mx6: correct IPU clock Peng Fan
  2016-03-09  8:07 ` [U-Boot] [PATCH 3/3] imx: mx6: hdmi: handle overflow condition Peng Fan
@ 2016-03-14 21:56 ` Anatolij Gustschin
  2 siblings, 0 replies; 15+ messages in thread
From: Anatolij Gustschin @ 2016-03-14 21:56 UTC (permalink / raw)
  To: u-boot

Hi,

On Wed,  9 Mar 2016 16:07:21 +0800
Peng Fan van.freenix at gmail.com wrote:

> Multiplication, as "clk->parent->rate * 16" may overflow. So use
> do_div to avoid such issue.
> 
> Signed-off-by: Peng Fan <van.freenix@gmail.com>
> Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
> Cc: Anatolij Gustschin <agust@denx.de>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> ---
>  drivers/video/ipu_common.c | 73 ++++++++++++++++++++++++++++++++++------------
>  1 file changed, 54 insertions(+), 19 deletions(-)

applied to u-boot-video/master, thanks!

--
Anatolij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 2/3] imx: mx6: correct IPU clock
  2016-03-09  8:07 ` [U-Boot] [PATCH 2/3] imx: mx6: correct IPU clock Peng Fan
@ 2016-03-14 21:56   ` Anatolij Gustschin
  2017-09-05  2:37   ` [U-Boot] [U-Boot,2/3] " Eric Nelson
  1 sibling, 0 replies; 15+ messages in thread
From: Anatolij Gustschin @ 2016-03-14 21:56 UTC (permalink / raw)
  To: u-boot

On Wed,  9 Mar 2016 16:07:22 +0800
Peng Fan van.freenix at gmail.com wrote:

> The CONFIG_IPUV3_CLK should be 264000000, to i.MX6DL, it should be
> 198000000.
> 
> Signed-off-by: Peng Fan <van.freenix@gmail.com>
> Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Peter Robinson <pbrobinson@gmail.com>
> ---
>  include/configs/mx6sabre_common.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

applied to u-boot-video/master, thanks!

--
Anatolij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 3/3] imx: mx6: hdmi: handle overflow condition
  2016-03-09  8:07 ` [U-Boot] [PATCH 3/3] imx: mx6: hdmi: handle overflow condition Peng Fan
@ 2016-03-14 21:57   ` Anatolij Gustschin
  0 siblings, 0 replies; 15+ messages in thread
From: Anatolij Gustschin @ 2016-03-14 21:57 UTC (permalink / raw)
  To: u-boot

On Wed,  9 Mar 2016 16:07:23 +0800
Peng Fan van.freenix at gmail.com wrote:

> If HDMI_IH_FC_STAT2_OVERFLOW_MASK is set, we need to
> do TMDS software reset and write to clear fc_invidconf register.
> We need minimum 3 times to write to clear the fc_invidconf
> register, so choose 5 loops here.
> 
> Signed-off-by: Peng Fan <van.freenix@gmail.com>
> Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> ---
>  arch/arm/cpu/armv7/mx6/soc.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

applied to u-boot-video/master, thanks!

--
Anatolij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot,2/3] imx: mx6: correct IPU clock
  2016-03-09  8:07 ` [U-Boot] [PATCH 2/3] imx: mx6: correct IPU clock Peng Fan
  2016-03-14 21:56   ` Anatolij Gustschin
@ 2017-09-05  2:37   ` Eric Nelson
  2017-09-05  2:43     ` Eric Nelson
                       ` (3 more replies)
  1 sibling, 4 replies; 15+ messages in thread
From: Eric Nelson @ 2017-09-05  2:37 UTC (permalink / raw)
  To: u-boot

Hi Peng,

Pardon the reference to an old update, but do you have a description
of the symptoms that brought about this patch?

On 03/09/2016 01:07 AM, Peng Fan wrote:
> The CONFIG_IPUV3_CLK should be 264000000, to i.MX6DL, it should be
> 198000000.
> 
> Signed-off-by: Peng Fan <van.freenix@gmail.com>
> Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Peter Robinson <pbrobinson@gmail.com>
> ---
>   include/configs/mx6sabre_common.h | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/configs/mx6sabre_common.h b/include/configs/mx6sabre_common.h
> index 29d1f91..a6d821b 100644
> --- a/include/configs/mx6sabre_common.h
> +++ b/include/configs/mx6sabre_common.h
> @@ -225,7 +225,11 @@
>   #define CONFIG_BMP_16BPP
>   #define CONFIG_VIDEO_LOGO
>   #define CONFIG_VIDEO_BMP_LOGO
> -#define CONFIG_IPUV3_CLK 260000000
> +#ifdef CONFIG_MX6DL
> +#define CONFIG_IPUV3_CLK 198000000
> +#else
> +#define CONFIG_IPUV3_CLK 264000000
> +#endif


Note that this should probably be applied for other boards
which are compiled for multiple CPU types.

At least the Boundary Nitrogen boards, but probably others
like Wand have ordering options for DL or Solo processors
and may need the reduced clock rate.


>   #define CONFIG_IMX_HDMI
>   #define CONFIG_IMX_VIDEO_SKIP
>   

Please advise,


Eric

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot,2/3] imx: mx6: correct IPU clock
  2017-09-05  2:37   ` [U-Boot] [U-Boot,2/3] " Eric Nelson
@ 2017-09-05  2:43     ` Eric Nelson
  2017-09-05 12:56     ` Fabio Estevam
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Eric Nelson @ 2017-09-05  2:43 UTC (permalink / raw)
  To: u-boot

Hi all,

Adding my normal e-mail account to the chain, since the other account
isn't registered on the list.

On 09/04/2017 07:37 PM, Eric Nelson wrote:
> Hi Peng,
> 
> Pardon the reference to an old update, but do you have a description
> of the symptoms that brought about this patch?
> 
> On 03/09/2016 01:07 AM, Peng Fan wrote:
>> The CONFIG_IPUV3_CLK should be 264000000, to i.MX6DL, it should be
>> 198000000.
>>
>> Signed-off-by: Peng Fan <van.freenix@gmail.com>
>> Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
>> Cc: Stefano Babic <sbabic@denx.de>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> Cc: Peter Robinson <pbrobinson@gmail.com>
>> ---
>>   include/configs/mx6sabre_common.h | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/configs/mx6sabre_common.h 
>> b/include/configs/mx6sabre_common.h
>> index 29d1f91..a6d821b 100644
>> --- a/include/configs/mx6sabre_common.h
>> +++ b/include/configs/mx6sabre_common.h
>> @@ -225,7 +225,11 @@
>>   #define CONFIG_BMP_16BPP
>>   #define CONFIG_VIDEO_LOGO
>>   #define CONFIG_VIDEO_BMP_LOGO
>> -#define CONFIG_IPUV3_CLK 260000000
>> +#ifdef CONFIG_MX6DL
>> +#define CONFIG_IPUV3_CLK 198000000
>> +#else
>> +#define CONFIG_IPUV3_CLK 264000000
>> +#endif
> 
> 
> Note that this should probably be applied for other boards
> which are compiled for multiple CPU types.
> 
> At least the Boundary Nitrogen boards, but probably others
> like Wand have ordering options for DL or Solo processors
> and may need the reduced clock rate.
> 
> 
>>   #define CONFIG_IMX_HDMI
>>   #define CONFIG_IMX_VIDEO_SKIP
> 
> Please advise,
> 
> 
> Eric

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot,2/3] imx: mx6: correct IPU clock
  2017-09-05  2:37   ` [U-Boot] [U-Boot,2/3] " Eric Nelson
  2017-09-05  2:43     ` Eric Nelson
@ 2017-09-05 12:56     ` Fabio Estevam
  2017-09-05 13:30       ` Stefano Babic
  2017-09-06  2:18     ` Peng Fan
  2017-09-06  9:37     ` Ye Li
  3 siblings, 1 reply; 15+ messages in thread
From: Fabio Estevam @ 2017-09-05 12:56 UTC (permalink / raw)
  To: u-boot

Hi Eric,

On Mon, Sep 4, 2017 at 11:37 PM, Eric Nelson <ericnelsonaz@gmail.com> wrote:

>> --- a/include/configs/mx6sabre_common.h
>> +++ b/include/configs/mx6sabre_common.h
>> @@ -225,7 +225,11 @@
>>   #define CONFIG_BMP_16BPP
>>   #define CONFIG_VIDEO_LOGO
>>   #define CONFIG_VIDEO_BMP_LOGO
>> -#define CONFIG_IPUV3_CLK 260000000
>> +#ifdef CONFIG_MX6DL
>> +#define CONFIG_IPUV3_CLK 198000000
>> +#else
>> +#define CONFIG_IPUV3_CLK 264000000
>> +#endif
>
>
>
> Note that this should probably be applied for other boards
> which are compiled for multiple CPU types.
>
> At least the Boundary Nitrogen boards, but probably others
> like Wand have ordering options for DL or Solo processors
> and may need the reduced clock rate.

Agreed. The clock frequency decision should be done in run-time rather
than in build-time.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot,2/3] imx: mx6: correct IPU clock
  2017-09-05 12:56     ` Fabio Estevam
@ 2017-09-05 13:30       ` Stefano Babic
  2017-09-05 13:41         ` Eric Nelson
  0 siblings, 1 reply; 15+ messages in thread
From: Stefano Babic @ 2017-09-05 13:30 UTC (permalink / raw)
  To: u-boot

On 05/09/2017 14:56, Fabio Estevam wrote:
> Hi Eric,
> 
> On Mon, Sep 4, 2017 at 11:37 PM, Eric Nelson <ericnelsonaz@gmail.com> wrote:
> 
>>> --- a/include/configs/mx6sabre_common.h
>>> +++ b/include/configs/mx6sabre_common.h
>>> @@ -225,7 +225,11 @@
>>>   #define CONFIG_BMP_16BPP
>>>   #define CONFIG_VIDEO_LOGO
>>>   #define CONFIG_VIDEO_BMP_LOGO
>>> -#define CONFIG_IPUV3_CLK 260000000
>>> +#ifdef CONFIG_MX6DL
>>> +#define CONFIG_IPUV3_CLK 198000000
>>> +#else
>>> +#define CONFIG_IPUV3_CLK 264000000
>>> +#endif
>>
>>
>>
>> Note that this should probably be applied for other boards
>> which are compiled for multiple CPU types.
>>
>> At least the Boundary Nitrogen boards, but probably others
>> like Wand have ordering options for DL or Solo processors
>> and may need the reduced clock rate.
> 
> Agreed. The clock frequency decision should be done in run-time rather
> than in build-time.

I agree, too. We have mechanism to take decisions at run time, at least
based on SOC type. Anyway, Anatolji has already merged this - should be
better to revert it ?

Regards,
Stefano


-- 
=====================================================================
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 at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot,2/3] imx: mx6: correct IPU clock
  2017-09-05 13:30       ` Stefano Babic
@ 2017-09-05 13:41         ` Eric Nelson
  2017-09-06 13:52           ` Fabio Estevam
  0 siblings, 1 reply; 15+ messages in thread
From: Eric Nelson @ 2017-09-05 13:41 UTC (permalink / raw)
  To: u-boot

Hi Stefano,

On 09/05/2017 06:30 AM, Stefano Babic wrote:
> On 05/09/2017 14:56, Fabio Estevam wrote:
>> Hi Eric,
>>
>> On Mon, Sep 4, 2017 at 11:37 PM, Eric Nelson <ericnelsonaz@gmail.com> wrote:
>>
>>>> --- a/include/configs/mx6sabre_common.h
>>>> +++ b/include/configs/mx6sabre_common.h
>>>> @@ -225,7 +225,11 @@
>>>>    #define CONFIG_BMP_16BPP
>>>>    #define CONFIG_VIDEO_LOGO
>>>>    #define CONFIG_VIDEO_BMP_LOGO
>>>> -#define CONFIG_IPUV3_CLK 260000000
>>>> +#ifdef CONFIG_MX6DL
>>>> +#define CONFIG_IPUV3_CLK 198000000
>>>> +#else
>>>> +#define CONFIG_IPUV3_CLK 264000000
>>>> +#endif
>>>
>>>
>>>
>>> Note that this should probably be applied for other boards
>>> which are compiled for multiple CPU types.
>>>
>>> At least the Boundary Nitrogen boards, but probably others
>>> like Wand have ordering options for DL or Solo processors
>>> and may need the reduced clock rate.
>>
>> Agreed. The clock frequency decision should be done in run-time rather
>> than in build-time.
> 
> I agree, too. We have mechanism to take decisions at run time, at least
> based on SOC type. Anyway, Anatolji has already merged this - should be
> better to revert it ?
> 

I don't think it should be reverted until we have a run-time decision
in place, or we'll re-introduce whatever problem the higher rate
caused, at least on SABRE boards with Solo or Dual-Lite processors.

I'm still wondering whether Peng has a description of the ramifications
of the higher rate on DL/Solo processors.

Regards,


Eric

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot,2/3] imx: mx6: correct IPU clock
  2017-09-05  2:37   ` [U-Boot] [U-Boot,2/3] " Eric Nelson
  2017-09-05  2:43     ` Eric Nelson
  2017-09-05 12:56     ` Fabio Estevam
@ 2017-09-06  2:18     ` Peng Fan
  2017-09-06  9:37     ` Ye Li
  3 siblings, 0 replies; 15+ messages in thread
From: Peng Fan @ 2017-09-06  2:18 UTC (permalink / raw)
  To: u-boot

On Mon, Sep 04, 2017 at 07:37:01PM -0700, Eric Nelson wrote:
>Hi Peng,
>
>Pardon the reference to an old update, but do you have a description
>of the symptoms that brought about this patch?

Sorry for late reply. Runtime calculation is better.

The clk here is IPU HSP clock, which default sources mmdc ch clock.
To DL, the mmdc ch clock is 396M and the IPU HSP podf is 2, so the lock
is 198M.

Regards,
Peng.

>
>On 03/09/2016 01:07 AM, Peng Fan wrote:
>>The CONFIG_IPUV3_CLK should be 264000000, to i.MX6DL, it should be
>>198000000.
>>
>>Signed-off-by: Peng Fan <van.freenix@gmail.com>
>>Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
>>Cc: Stefano Babic <sbabic@denx.de>
>>Cc: Fabio Estevam <fabio.estevam@nxp.com>
>>Cc: Peter Robinson <pbrobinson@gmail.com>
>>---
>>  include/configs/mx6sabre_common.h | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>>diff --git a/include/configs/mx6sabre_common.h b/include/configs/mx6sabre_common.h
>>index 29d1f91..a6d821b 100644
>>--- a/include/configs/mx6sabre_common.h
>>+++ b/include/configs/mx6sabre_common.h
>>@@ -225,7 +225,11 @@
>>  #define CONFIG_BMP_16BPP
>>  #define CONFIG_VIDEO_LOGO
>>  #define CONFIG_VIDEO_BMP_LOGO
>>-#define CONFIG_IPUV3_CLK 260000000
>>+#ifdef CONFIG_MX6DL
>>+#define CONFIG_IPUV3_CLK 198000000
>>+#else
>>+#define CONFIG_IPUV3_CLK 264000000
>>+#endif
>
>
>Note that this should probably be applied for other boards
>which are compiled for multiple CPU types.
>
>At least the Boundary Nitrogen boards, but probably others
>like Wand have ordering options for DL or Solo processors
>and may need the reduced clock rate.
>
>
>>  #define CONFIG_IMX_HDMI
>>  #define CONFIG_IMX_VIDEO_SKIP
>
>Please advise,
>
>
>Eric

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot,2/3] imx: mx6: correct IPU clock
  2017-09-05  2:37   ` [U-Boot] [U-Boot,2/3] " Eric Nelson
                       ` (2 preceding siblings ...)
  2017-09-06  2:18     ` Peng Fan
@ 2017-09-06  9:37     ` Ye Li
  2017-09-06 17:38       ` Eric Nelson
  3 siblings, 1 reply; 15+ messages in thread
From: Ye Li @ 2017-09-06  9:37 UTC (permalink / raw)
  To: u-boot

On 9/5/2017 6:33 PM, Eric Nelson wrote:
> Hi Peng,
>
> Pardon the reference to an old update, but do you have a description
> of the symptoms that brought about this patch?
>
> On 03/09/2016 01:07 AM, Peng Fan wrote:
>> The CONFIG_IPUV3_CLK should be 264000000, to i.MX6DL, it should be
>> 198000000.
>>
>> Signed-off-by: Peng Fan <van.freenix@gmail.com>
>> Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
>> Cc: Stefano Babic <sbabic@denx.de>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> Cc: Peter Robinson <pbrobinson@gmail.com>
>> ---
>>    include/configs/mx6sabre_common.h | 6 +++++-
>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/configs/mx6sabre_common.h b/include/configs/mx6sabre_common.h
>> index 29d1f91..a6d821b 100644
>> --- a/include/configs/mx6sabre_common.h
>> +++ b/include/configs/mx6sabre_common.h
>> @@ -225,7 +225,11 @@
>>    #define CONFIG_BMP_16BPP
>>    #define CONFIG_VIDEO_LOGO
>>    #define CONFIG_VIDEO_BMP_LOGO
>> -#define CONFIG_IPUV3_CLK 260000000
>> +#ifdef CONFIG_MX6DL
>> +#define CONFIG_IPUV3_CLK 198000000
>> +#else
>> +#define CONFIG_IPUV3_CLK 264000000
>> +#endif
>
>
> Note that this should probably be applied for other boards
> which are compiled for multiple CPU types.
>
> At least the Boundary Nitrogen boards, but probably others
> like Wand have ordering options for DL or Solo processors
> and may need the reduced clock rate.
>
>
>>    #define CONFIG_IMX_HDMI
>>    #define CONFIG_IMX_VIDEO_SKIP
>>
>
> Please advise,
>
>
> Eric
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>

CONFIG_IPUV3_CLK is used to indicate the default rate for IPU HSP clock. 
The IPU driver in u-boot won't calculate the HSP clock rate according to 
CCM registers, it needs this setting to know current rate. 198Mhz is the 
correct value on DL not the 264Mhz.

If you select IPU DI clock (pixel clock) derived from HSP clock not the 
external clock like LDB DI clock, I believe the 264Mhz will cause problem.

Best regards,
Ye Li

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot,2/3] imx: mx6: correct IPU clock
  2017-09-05 13:41         ` Eric Nelson
@ 2017-09-06 13:52           ` Fabio Estevam
  0 siblings, 0 replies; 15+ messages in thread
From: Fabio Estevam @ 2017-09-06 13:52 UTC (permalink / raw)
  To: u-boot

Hi Eric/Stefano,

On Tue, Sep 5, 2017 at 10:41 AM, Eric Nelson <eric@nelint.com> wrote:

>
> I don't think it should be reverted until we have a run-time decision
> in place, or we'll re-introduce whatever problem the higher rate
> caused, at least on SABRE boards with Solo or Dual-Lite processors.

I have just sent  a RFC patch that introduces the IPU clock setting in
run-time on mx6.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [U-Boot,2/3] imx: mx6: correct IPU clock
  2017-09-06  9:37     ` Ye Li
@ 2017-09-06 17:38       ` Eric Nelson
  0 siblings, 0 replies; 15+ messages in thread
From: Eric Nelson @ 2017-09-06 17:38 UTC (permalink / raw)
  To: u-boot

Thanks Ye (and Peng).

On 09/06/2017 02:37 AM, Ye Li wrote:
> On 9/5/2017 6:33 PM, Eric Nelson wrote:
>> Hi Peng,
>>
>> Pardon the reference to an old update, but do you have a description
>> of the symptoms that brought about this patch?
>>
>> On 03/09/2016 01:07 AM, Peng Fan wrote:
>>> The CONFIG_IPUV3_CLK should be 264000000, to i.MX6DL, it should be
>>> 198000000.
>>>
>>> Signed-off-by: Peng Fan <van.freenix@gmail.com>
>>> Signed-off-by: Sandor Yu <sandor.yu@nxp.com>
>>> Cc: Stefano Babic <sbabic@denx.de>
>>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>>> Cc: Peter Robinson <pbrobinson@gmail.com>
>>> ---
>>>     include/configs/mx6sabre_common.h | 6 +++++-
>>>     1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/configs/mx6sabre_common.h b/include/configs/mx6sabre_common.h
>>> index 29d1f91..a6d821b 100644
>>> --- a/include/configs/mx6sabre_common.h
>>> +++ b/include/configs/mx6sabre_common.h
>>> @@ -225,7 +225,11 @@
>>>     #define CONFIG_BMP_16BPP
>>>     #define CONFIG_VIDEO_LOGO
>>>     #define CONFIG_VIDEO_BMP_LOGO
>>> -#define CONFIG_IPUV3_CLK 260000000
>>> +#ifdef CONFIG_MX6DL
>>> +#define CONFIG_IPUV3_CLK 198000000
>>> +#else
>>> +#define CONFIG_IPUV3_CLK 264000000
>>> +#endif
>>
>>
>> Note that this should probably be applied for other boards
>> which are compiled for multiple CPU types.
>>
>> At least the Boundary Nitrogen boards, but probably others
>> like Wand have ordering options for DL or Solo processors
>> and may need the reduced clock rate.
>>
>>
>>>     #define CONFIG_IMX_HDMI
>>>     #define CONFIG_IMX_VIDEO_SKIP
>>>
>>
>> Please advise,
>>
>>
>> Eric
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>>
> 
> CONFIG_IPUV3_CLK is used to indicate the default rate for IPU HSP clock.
> The IPU driver in u-boot won't calculate the HSP clock rate according to
> CCM registers, it needs this setting to know current rate. 198Mhz is the
> correct value on DL not the 264Mhz.
> 
> If you select IPU DI clock (pixel clock) derived from HSP clock not the
> external clock like LDB DI clock, I believe the 264Mhz will cause problem.
> 

Do you know what sort of problem was seen (if any)?

Please advise,


Eric

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-09-06 17:38 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-09  8:07 [U-Boot] [PATCH 1/3] video: ipu: avoid overflow issue Peng Fan
2016-03-09  8:07 ` [U-Boot] [PATCH 2/3] imx: mx6: correct IPU clock Peng Fan
2016-03-14 21:56   ` Anatolij Gustschin
2017-09-05  2:37   ` [U-Boot] [U-Boot,2/3] " Eric Nelson
2017-09-05  2:43     ` Eric Nelson
2017-09-05 12:56     ` Fabio Estevam
2017-09-05 13:30       ` Stefano Babic
2017-09-05 13:41         ` Eric Nelson
2017-09-06 13:52           ` Fabio Estevam
2017-09-06  2:18     ` Peng Fan
2017-09-06  9:37     ` Ye Li
2017-09-06 17:38       ` Eric Nelson
2016-03-09  8:07 ` [U-Boot] [PATCH 3/3] imx: mx6: hdmi: handle overflow condition Peng Fan
2016-03-14 21:57   ` Anatolij Gustschin
2016-03-14 21:56 ` [U-Boot] [PATCH 1/3] video: ipu: avoid overflow issue Anatolij Gustschin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox