Revert "sunxi: dram: Use divisor P=1 for PLL5"

PATCH New Hans de Goede 5 participants 12 messages Oct 15, 2014 – Oct 22, 2014

Subsystems (2)

Arm (32-bit) Arm/Allwinner sunXi SoC support

lore

Share & tools

lore

Hans de Goede

author

2014-10-15 10:12 UTC #0 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

This change breaks various hardcoded assumptions in the linux-sunxi-3.4 kernels, causing a divide by 0 error in the linux-sunxi-3.4 nand driver, rendering 3.4 kernels unbootable. Besides this problem, it also breaks LCD output (on devices without nand).

Fixes for these issues are being added to the linux-sunxi-3.4 kernel, but for now it seems better to revert this.

This reverts commit 013f2d746955147439215a4939655c9ed6bdd866.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/sunxi/dram.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

arch/arm/cpu/armv7/sunxi/dram.c +17 ~~-11~~

index 584f742..8c4dd51 100644
@@ -256,7 +256,7 @@ static void mctl_setup_dram_clock(u32 clk, u32 mbus_clk)
 	/* PLL5P and PLL6 are the potential clock sources for MBUS */
 	u32 pll6x_div, pll5p_div;
 	u32 pll6x_clk = clock_get_pll6() / 1000000;
-	u32 pll5p_clk = clk / 24 * 48;
+	u32 pll5p_clk = clk / 24 * 24;
 	u32 pll5p_rate, pll6x_rate;
 #ifdef CONFIG_SUN7I
 	pll6x_clk *= 2; /* sun7i uses PLL6*2, sun5i uses just PLL6 */

@@ -269,40 +269,46 @@ static void mctl_setup_dram_clock(u32 clk, u32 mbus_clk)
 	reg_val &= ~CCM_PLL5_CTRL_N_MASK;		/* set N to 0 (x0) */
 	reg_val &= ~CCM_PLL5_CTRL_P_MASK;		/* set P to 0 (x1) */
 	if (clk >= 540 && clk < 552) {
-		/* dram = 540MHz, pll5p = 1080MHz */
-		pll5p_clk = 1080;
+		/* dram = 540MHz, pll5p = 540MHz */
+		pll5p_clk = 540;
 		reg_val |= CCM_PLL5_CTRL_M(CCM_PLL5_CTRL_M_X(2));
 		reg_val |= CCM_PLL5_CTRL_K(CCM_PLL5_CTRL_K_X(3));
 		reg_val |= CCM_PLL5_CTRL_N(CCM_PLL5_CTRL_N_X(15));
+		reg_val |= CCM_PLL5_CTRL_P(1);
 	} else if (clk >= 512 && clk < 528) {
-		/* dram = 512MHz, pll5p = 1536MHz */
-		pll5p_clk = 1536;
+		/* dram = 512MHz, pll5p = 384MHz */
+		pll5p_clk = 384;
 		reg_val |= CCM_PLL5_CTRL_M(CCM_PLL5_CTRL_M_X(3));
 		reg_val |= CCM_PLL5_CTRL_K(CCM_PLL5_CTRL_K_X(4));
 		reg_val |= CCM_PLL5_CTRL_N(CCM_PLL5_CTRL_N_X(16));
+		reg_val |= CCM_PLL5_CTRL_P(2);
 	} else if (clk >= 496 && clk < 504) {
-		/* dram = 496MHz, pll5p = 1488MHz */
-		pll5p_clk = 1488;
+		/* dram = 496MHz, pll5p = 372MHz */
+		pll5p_clk = 372;
 		reg_val |= CCM_PLL5_CTRL_M(CCM_PLL5_CTRL_M_X(3));
 		reg_val |= CCM_PLL5_CTRL_K(CCM_PLL5_CTRL_K_X(2));
 		reg_val |= CCM_PLL5_CTRL_N(CCM_PLL5_CTRL_N_X(31));
+		reg_val |= CCM_PLL5_CTRL_P(2);
 	} else if (clk >= 468 && clk < 480) {
-		/* dram = 468MHz, pll5p = 936MHz */
-		pll5p_clk = 936;
+		/* dram = 468MHz, pll5p = 468MHz */
+		pll5p_clk = 468;
 		reg_val |= CCM_PLL5_CTRL_M(CCM_PLL5_CTRL_M_X(2));
 		reg_val |= CCM_PLL5_CTRL_K(CCM_PLL5_CTRL_K_X(3));
 		reg_val |= CCM_PLL5_CTRL_N(CCM_PLL5_CTRL_N_X(13));
+		reg_val |= CCM_PLL5_CTRL_P(1);
 	} else if (clk >= 396 && clk < 408) {
-		/* dram = 396MHz, pll5p = 792MHz */
-		pll5p_clk = 792;
+		/* dram = 396MHz, pll5p = 396MHz */
+		pll5p_clk = 396;
 		reg_val |= CCM_PLL5_CTRL_M(CCM_PLL5_CTRL_M_X(2));
 		reg_val |= CCM_PLL5_CTRL_K(CCM_PLL5_CTRL_K_X(3));
 		reg_val |= CCM_PLL5_CTRL_N(CCM_PLL5_CTRL_N_X(11));
+		reg_val |= CCM_PLL5_CTRL_P(1);
 	} else 	{
 		/* any other frequency that is a multiple of 24 */
 		reg_val |= CCM_PLL5_CTRL_M(CCM_PLL5_CTRL_M_X(2));
 		reg_val |= CCM_PLL5_CTRL_K(CCM_PLL5_CTRL_K_X(2));
 		reg_val |= CCM_PLL5_CTRL_N(CCM_PLL5_CTRL_N_X(clk / 24));
+		reg_val |= CCM_PLL5_CTRL_P(CCM_PLL5_CTRL_P_X(2));
 	}
 	reg_val &= ~CCM_PLL5_CTRL_VCO_GAIN;		/* PLL VCO Gain off */
 	reg_val |= CCM_PLL5_CTRL_EN;			/* PLL On */

Signature · 1 line

2.1.0

Siarhei Siamashka

2014-10-15 10:34 UTC #1 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

Quoted 9 lines · On Wed, 15 Oct 2014 12:12:11 +0200 Hans de Goede <[email protected]> wrote:

This change breaks various hardcoded assumptions in the linux-sunxi-3.4 kernels, causing a divide by 0 error in the linux-sunxi-3.4 nand driver, rendering 3.4 kernels unbootable. Besides this problem, it also breaks LCD output (on devices without nand).
Fixes for these issues are being added to the linux-sunxi-3.4 kernel, but for now it seems better to revert this.
This reverts commit 013f2d746955147439215a4939655c9ed6bdd866.

As mentioned in the commit message
https://www.mail-archive.com/linux-sunxi at googlegroups.com/msg07884.html
the hardcoded assumptions in the sunxi-3.4 kernel are still bad with or without this revert if somebody tries to use higher DRAM clock speeds.

The patch in question just smokes out bugs, which are otherwise still there.

Going forward, the mainline kernel needs the G2D driver and it needs to run at a decent clock speed to be provide reasonable performance. Low PLL5P clock speed is limiting our options.

This revert does not do anyone any favour and only postpones the resolution of the problem.

Signature · 2 lines

Best regards,
Siarhei Siamashka

Hans de Goede

author

2014-10-16 08:48 UTC #2 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

Hi,

Quoted 22 lines · Siarhei Siamashka

Quoted 9 lines · On Wed, 15 Oct 2014 12:12:11 +0200 Hans de Goede <[email protected]> wrote:
This change breaks various hardcoded assumptions in the linux-sunxi-3.4 kernels, causing a divide by 0 error in the linux-sunxi-3.4 nand driver, rendering 3.4 kernels unbootable. Besides this problem, it also breaks LCD output (on devices without nand).
Fixes for these issues are being added to the linux-sunxi-3.4 kernel, but for now it seems better to revert this.
This reverts commit 013f2d746955147439215a4939655c9ed6bdd866.
As mentioned in the commit message
https://www.mail-archive.com/linux-sunxi at googlegroups.com/msg07884.html
the hardcoded assumptions in the sunxi-3.4 kernel are still bad with or without this revert if somebody tries to use higher DRAM clock speeds.
The patch in question just smokes out bugs, which are otherwise still there.

Quoted 6 lines

Going forward, the mainline kernel needs the G2D driver and it needs to run at a decent clock speed to be provide reasonable performance. Low PLL5P clock speed is limiting our options.
This revert does not do anyone any favour and only postpones the resolution of the problem.

I agree that we need to fix the linux-sunxi-3.4 kernels to work properly with different pll5 settings, which is why I've actually send a fix for some pll5 issues to the linux-sunxi list and acked your patches which fix other bits.

I can understand you want to have optimal settings for the G2D stuff, and I'm not against that. But getting the G2D stuff upstream is something which is still far in the future. I really want people to be able to start using upstream u-boot, starting today. And it will take some time for the sunxi-3.4 fixes to trickle down to users.

So for now I believe we really should revert the PLL5 u-boot changes as they break some kernels from booting.

Then once getting G2D support upstream really comes into the picture (e.g. upstream kernel patches are posted and being reviewed), then we can talk about re-applying the PLL5 changes, but for now I believe that reverting them is best.

Regards,

Hans

Ian Campbell

2014-10-16 09:49 UTC #3 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

Quoted 2 lines · Hans de Goede

So for now I believe we really should revert the PLL5 u-boot changes as they break some kernels from booting.

This seems reasonable to me.

Acked-by Ian Campbell <[email protected]>

Quoted 16 lines

Then once getting G2D support upstream really comes into the picture (e.g. upstream kernel patches are posted and being reviewed), then we can talk about re-applying the PLL5 changes, but for now I believe that reverting them is best.
Regards,
Hans

Siarhei Siamashka

2014-10-16 09:59 UTC #4 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

In reply to Hans de Goede

Quoted 32 lines · On Thu, 16 Oct 2014 10:48:34 +0200 Hans de Goede <[email protected]> wrote:

Hi,
Quoted 22 lines · Siarhei Siamashka
Quoted 9 lines · On Wed, 15 Oct 2014 12:12:11 +0200 Hans de Goede <[email protected]> wrote:
This change breaks various hardcoded assumptions in the linux-sunxi-3.4 kernels, causing a divide by 0 error in the linux-sunxi-3.4 nand driver, rendering 3.4 kernels unbootable. Besides this problem, it also breaks LCD output (on devices without nand).
Fixes for these issues are being added to the linux-sunxi-3.4 kernel, but for now it seems better to revert this.
This reverts commit 013f2d746955147439215a4939655c9ed6bdd866.
As mentioned in the commit message
https://www.mail-archive.com/linux-sunxi at googlegroups.com/msg07884.html
the hardcoded assumptions in the sunxi-3.4 kernel are still bad with or without this revert if somebody tries to use higher DRAM clock speeds.
The patch in question just smokes out bugs, which are otherwise still there.
Quoted 6 lines
Going forward, the mainline kernel needs the G2D driver and it needs to run at a decent clock speed to be provide reasonable performance. Low PLL5P clock speed is limiting our options.
This revert does not do anyone any favour and only postpones the resolution of the problem.

Quoted 7 lines

I agree that we need to fix the linux-sunxi-3.4 kernels to work properly with different pll5 settings, which is why I've actually send a fix for some pll5 issues to the linux-sunxi list and acked your patches which fix other bits.
I can understand you want to have optimal settings for the G2D stuff, and I'm not against that. But getting the G2D stuff upstream is something which is still far in the future.

The future is happening right now ;-)

Luc has implemented the HDMI support in u-boot and ensured that it works with the simplefb driver, providing framebuffer in the mainline kernel. This has code has been available for several months for any geeks, willing to fish out and cherry pick the patches. But right now this needs to be more easily available for the users and developers.

The mainline kernel got all the basic essentials and has been ready for the graphics and multimedia since around version 3.16

As anyone can see, the sunxi-3.4 kernel provides a rather rotten foundation. And even though we have a reasonably good support for both proprietary and free reverse engineered drivers with sunxi-3.4, it still makes a lot of sense to get the graphics drivers in the mainline kernel ASAP.

Quoted 11 lines

I really want people to be able to start using upstream u-boot, starting today. And it will take some time for the sunxi-3.4 fixes to trickle down to users.
So for now I believe we really should revert the PLL5 u-boot changes as they break some kernels from booting.
Then once getting G2D support upstream really comes into the picture (e.g. upstream kernel patches are posted and being reviewed), then we can talk about re-applying the PLL5 changes, but for now I believe that reverting them is best.

You are the boss. We are all free to have our own private branches anyway.

Signature · 2 lines

Best regards,
Siarhei Siamashka

Jeroen Hofstee

2014-10-16 20:10 UTC #5 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

In reply to Hans de Goede

Hi Hans,

Quoted 5 lines · Hans de Goede

[snip]
So for now I believe we really should revert the PLL5 u-boot changes as they break some kernels from booting.

U-boot and the kernel should be independent. Hence u-boot cannot break kernels from booting or the kernel is bugged and should be fixed.

Regards,
Jeroen

Tom Rini

2014-10-21 20:58 UTC #6 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

In reply to Hans de Goede

Quoted 36 lines · Hans de Goede

Hi,
Quoted 22 lines · Siarhei Siamashka
Quoted 9 lines · On Wed, 15 Oct 2014 12:12:11 +0200 Hans de Goede <[email protected]> wrote:
This change breaks various hardcoded assumptions in the linux-sunxi-3.4 kernels, causing a divide by 0 error in the linux-sunxi-3.4 nand driver, rendering 3.4 kernels unbootable. Besides this problem, it also breaks LCD output (on devices without nand).
Fixes for these issues are being added to the linux-sunxi-3.4 kernel, but for now it seems better to revert this.
This reverts commit 013f2d746955147439215a4939655c9ed6bdd866.
As mentioned in the commit message
https://www.mail-archive.com/linux-sunxi at googlegroups.com/msg07884.html
the hardcoded assumptions in the sunxi-3.4 kernel are still bad with or without this revert if somebody tries to use higher DRAM clock speeds.
The patch in question just smokes out bugs, which are otherwise still there.
Quoted 6 lines
Going forward, the mainline kernel needs the G2D driver and it needs to run at a decent clock speed to be provide reasonable performance. Low PLL5P clock speed is limiting our options.
This revert does not do anyone any favour and only postpones the resolution of the problem.
I agree that we need to fix the linux-sunxi-3.4 kernels to work properly with different pll5 settings, which is why I've actually send a fix for some pll5 issues to the linux-sunxi list and acked your patches which fix other bits.

Quoted 5 lines

I can understand you want to have optimal settings for the G2D stuff, and I'm not against that. But getting the G2D stuff upstream is something which is still far in the future. I really want people to be able to start using upstream u-boot, starting today. And it will take some time for the sunxi-3.4 fixes to trickle down to users.

We should be doing things right, in mainline. To bring up a different example, on TI OMAP4 parts at least for a long time in order to use mainline U-Boot on older kernels you had to manually add CONFIG_SOMETHING_OR_ANOTHER to enable additional clocks/mux that the old kernels had incorrectly relied on U-Boot to set. If we must do strange things to support old and incorrect but in the wild kernels we need to
(a) make it opt-in (easier now with Kconfig!) and (b) schedule a removal of the hack all the same.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141021/e8b8ad48/attachment.pgp>

Ian Campbell

2014-10-22 08:14 UTC #7 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

Quoted 8 lines · Tom Rini

We should be doing things right, in mainline. To bring up a different example, on TI OMAP4 parts at least for a long time in order to use mainline U-Boot on older kernels you had to manually add CONFIG_SOMETHING_OR_ANOTHER to enable additional clocks/mux that the old kernels had incorrectly relied on U-Boot to set. If we must do strange things to support old and incorrect but in the wild kernels we need to
(a) make it opt-in (easier now with Kconfig!) and (b) schedule a removal of the hack all the same.

A Kconfig option does sound like a reasonable compromise.

Ian.

Hans de Goede

author

2014-10-22 08:35 UTC #8 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

Hi,

Quoted 11 lines · Ian Campbell

Quoted 8 lines · Tom Rini
We should be doing things right, in mainline. To bring up a different example, on TI OMAP4 parts at least for a long time in order to use mainline U-Boot on older kernels you had to manually add CONFIG_SOMETHING_OR_ANOTHER to enable additional clocks/mux that the old kernels had incorrectly relied on U-Boot to set. If we must do strange things to support old and incorrect but in the wild kernels we need to
(a) make it opt-in (easier now with Kconfig!) and (b) schedule a removal of the hack all the same.
A Kconfig option does sound like a reasonable compromise.

Ok, I will look into this, my plan for now is to call it OLD_KERNEL_COMPAT, so that if we come across more cases like this we've one config option for them, rather then a ton of small isolated config options.

Regards,

Hans

Ian Campbell

2014-10-22 09:19 UTC #9 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

Quoted 13 lines · Hans de Goede

Hi,
Quoted 10 lines · Ian Campbell
Quoted 8 lines · Tom Rini
We should be doing things right, in mainline. To bring up a different example, on TI OMAP4 parts at least for a long time in order to use mainline U-Boot on older kernels you had to manually add CONFIG_SOMETHING_OR_ANOTHER to enable additional clocks/mux that the old kernels had incorrectly relied on U-Boot to set. If we must do strange things to support old and incorrect but in the wild kernels we need to
(a) make it opt-in (easier now with Kconfig!) and (b) schedule a removal of the hack all the same.

Quoted 5 lines

Quoted 1 line
A Kconfig option does sound like a reasonable compromise.
Ok, I will look into this, my plan for now is to call it OLD_KERNEL_COMPAT, so that if we come across more cases like this we've one config option for them, rather then a ton of small isolated config options.

Is it particular to "old" kernels as such, or is it more to do with Allwinner SDK (and derived) kernels? Is it worth trying to keep workarounds for such kernels separated from workarounds for old mainline kernels?

Ian.

Hans de Goede

author

2014-10-22 09:26 UTC #10 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

Hi,

Quoted 16 lines · Ian Campbell

Quoted 15 lines · Hans de Goede
Hi,
Quoted 11 lines · Ian Campbell
Quoted 8 lines · Tom Rini
We should be doing things right, in mainline. To bring up a different example, on TI OMAP4 parts at least for a long time in order to use mainline U-Boot on older kernels you had to manually add CONFIG_SOMETHING_OR_ANOTHER to enable additional clocks/mux that the old kernels had incorrectly relied on U-Boot to set. If we must do strange things to support old and incorrect but in the wild kernels we need to
(a) make it opt-in (easier now with Kconfig!) and (b) schedule a removal of the hack all the same.
A Kconfig option does sound like a reasonable compromise.

Quoted 8 lines

Quoted 3 lines
Ok, I will look into this, my plan for now is to call it OLD_KERNEL_COMPAT, so that if we come across more cases like this we've one config option for them, rather then a ton of small isolated config options.
Is it particular to "old" kernels as such, or is it more to do with Allwinner SDK (and derived) kernels? Is it worth trying to keep workarounds for such kernels separated from workarounds for old mainline kernels?

AFAIK the first mainline kernels with sunxi support are recent enough that they don't need any workarounds. TBH I don't think differentiating between the 2 brings us anything.

Regards,

Hans

Ian Campbell

2014-10-22 09:42 UTC #11 lore raw

PATCH 1/1 Revert "sunxi: dram: Use divisor P=1 for PLL5"

Quoted 23 lines · Hans de Goede

Hi,
Quoted 20 lines · Ian Campbell
Quoted 18 lines · Hans de Goede
Hi,
Quoted 11 lines · Ian Campbell
Quoted 8 lines · Tom Rini
We should be doing things right, in mainline. To bring up a different example, on TI OMAP4 parts at least for a long time in order to use mainline U-Boot on older kernels you had to manually add CONFIG_SOMETHING_OR_ANOTHER to enable additional clocks/mux that the old kernels had incorrectly relied on U-Boot to set. If we must do strange things to support old and incorrect but in the wild kernels we need to
(a) make it opt-in (easier now with Kconfig!) and (b) schedule a removal of the hack all the same.
A Kconfig option does sound like a reasonable compromise.
Ok, I will look into this, my plan for now is to call it OLD_KERNEL_COMPAT, so that if we come across more cases like this we've one config option for them, rather then a ton of small isolated config options.

Quoted 8 lines

Quoted 4 lines
Is it particular to "old" kernels as such, or is it more to do with Allwinner SDK (and derived) kernels? Is it worth trying to keep workarounds for such kernels separated from workarounds for old mainline kernels?
AFAIK the first mainline kernels with sunxi support are recent enough that they don't need any workarounds. TBH I don't think differentiating between the 2 brings us anything.

OK.

Ian.