Skip to contentSkip to conversation

PPC4xx: PLB4 Arbiter and Memory Queue Optimizations

PATCH New Prodyut Hazarika 3 participants 5 messages Aug 11, 2008

lore
Share & tools
lore

Prodyut Hazarika

author
#0 lore raw

PATCH 1/1 PPC4xx: PLB4 Arbiter and Memory Queue Optimizations

PLB4 Crossbar Arbiter core is instantiated on 405EX/ 440EP/ 440EPx/ 440GR/ 440GRx/ 440SP/ 440SPe/ 460EX/ 460GT For optimal performance, the read pipeline depth must be set to 4.
The current code enables it only for bamboo, sequoia and yosemite boards.
Better approach is to define CONFIG_PLB4_CROSSBAR_ARBITER_CORE for the above PPC4xx families, and to remove the duplicated code from board specfic files.

Also, added register definitions for MemoryQueue related registers, and enabled Memory Queue optimizatios for 460EX/GT.

Signed-off-by Prodyut Hazarika <[email protected]>

---

 board/amcc/bamboo/bamboo.c     |   17 -----------
 board/amcc/sequoia/sequoia.c   |   17 -----------
 board/amcc/yosemite/yosemite.c |   17 -----------
 cpu/ppc4xx/44x_spd_ddr2.c      |   34 ++++++++++++++++------
 cpu/ppc4xx/cpu_init.c          |   15 ++++++++++
 include/asm-ppc/ppc4xx-sdram.h |   16 ++++++++++
 include/ppc440.h               |   49 +--------------------------------
 include/ppc4xx.h               |   60
++++++++++++++++++++++++++++++++++++++++
 8 files changed, 116 insertions(+), 109 deletions(-)
board/amcc/bamboo/bamboo.c +0 -17
index f415701..761e418 100644
@@ -500,23 +500,6 @@ int pci_pre_init(struct pci_controller *hose)
 	addr = mfdcr(plb4_acr) | 0xa0000000;	/* Was 0x8---- */
 	mtdcr(plb4_acr, addr);
 
-	/*-------------------------------------------------------------------------+
-	  | Set Nebula PLB4 arbiter to fair mode.
-	  +-------------------------------------------------------------------------*/
-	/* Segment0 */
-	addr = (mfdcr(plb0_acr) & ~plb0_acr_ppm_mask) | plb0_acr_ppm_fair;
-	addr = (addr & ~plb0_acr_hbu_mask) | plb0_acr_hbu_enabled;
-	addr = (addr & ~plb0_acr_rdp_mask) | plb0_acr_rdp_4deep;
-	addr = (addr & ~plb0_acr_wrp_mask) | plb0_acr_wrp_2deep;
-	mtdcr(plb0_acr, addr);
-
-	/* Segment1 */
-	addr = (mfdcr(plb1_acr) & ~plb1_acr_ppm_mask) | plb1_acr_ppm_fair;
-	addr = (addr & ~plb1_acr_hbu_mask) | plb1_acr_hbu_enabled;
-	addr = (addr & ~plb1_acr_rdp_mask) | plb1_acr_rdp_4deep;
-	addr = (addr & ~plb1_acr_wrp_mask) | plb1_acr_wrp_2deep;
-	mtdcr(plb1_acr, addr);
-
 	return 1;
 }
 #endif /* defined(CONFIG_PCI) */
board/amcc/sequoia/sequoia.c +0 -17
index b833092..639db09 100644
@@ -372,23 +372,6 @@ int pci_pre_init(struct pci_controller *hose)
 	addr = mfdcr(plb4_acr) | 0xa0000000;	/* Was 0x8---- */
 	mtdcr(plb4_acr, addr);
 
-	/*
-	 * Set Nebula PLB4 arbiter to fair mode.
-	 */
-	/* Segment0 */
-	addr = (mfdcr(plb0_acr) & ~plb0_acr_ppm_mask) | plb0_acr_ppm_fair;
-	addr = (addr & ~plb0_acr_hbu_mask) | plb0_acr_hbu_enabled;
-	addr = (addr & ~plb0_acr_rdp_mask) | plb0_acr_rdp_4deep;
-	addr = (addr & ~plb0_acr_wrp_mask) | plb0_acr_wrp_2deep;
-	mtdcr(plb0_acr, addr);
-
-	/* Segment1 */
-	addr = (mfdcr(plb1_acr) & ~plb1_acr_ppm_mask) | plb1_acr_ppm_fair;
-	addr = (addr & ~plb1_acr_hbu_mask) | plb1_acr_hbu_enabled;
-	addr = (addr & ~plb1_acr_rdp_mask) | plb1_acr_rdp_4deep;
-	addr = (addr & ~plb1_acr_wrp_mask) | plb1_acr_wrp_2deep;
-	mtdcr(plb1_acr, addr);
-
 #ifdef CONFIG_PCI_PNP
 	hose->fixup_irq = sequoia_pci_fixup_irq;
 #endif
board/amcc/yosemite/yosemite.c +0 -17
index 05be40a..b18637d 100644
@@ -366,23 +366,6 @@ int pci_pre_init(struct pci_controller *hose)
 	addr = mfdcr(plb4_acr) | 0xa0000000;	/* Was 0x8---- */
 	mtdcr(plb4_acr, addr);
 
-	/*-------------------------------------------------------------------------+
-	  | Set Nebula PLB4 arbiter to fair mode.
-	  +-------------------------------------------------------------------------*/
-	/* Segment0 */
-	addr = (mfdcr(plb0_acr) & ~plb0_acr_ppm_mask) | plb0_acr_ppm_fair;
-	addr = (addr & ~plb0_acr_hbu_mask) | plb0_acr_hbu_enabled;
-	addr = (addr & ~plb0_acr_rdp_mask) | plb0_acr_rdp_4deep;
-	addr = (addr & ~plb0_acr_wrp_mask) | plb0_acr_wrp_2deep;
-	mtdcr(plb0_acr, addr);
-
-	/* Segment1 */
-	addr = (mfdcr(plb1_acr) & ~plb1_acr_ppm_mask) | plb1_acr_ppm_fair;
-	addr = (addr & ~plb1_acr_hbu_mask) | plb1_acr_hbu_enabled;
-	addr = (addr & ~plb1_acr_rdp_mask) | plb1_acr_rdp_4deep;
-	addr = (addr & ~plb1_acr_wrp_mask) | plb1_acr_wrp_2deep;
-	mtdcr(plb1_acr, addr);
-
 	return 1;
 }
 #endif	/* defined(CONFIG_PCI) */
cpu/ppc4xx/44x_spd_ddr2.c +24 -10
index e9940e8..042ad1f 100644
@@ -2241,16 +2241,30 @@ static void program_memory_queue(unsigned long *dimm_populated,
 	}
 
 #if defined(CONFIG_460EX) || defined(CONFIG_460GT)
-	/*
-	 * Enable high bandwidth access on 460EX/GT.
-	 * This should/could probably be done on other
-	 * PPC's too, like 440SPe.
-	 * This is currently not used, but with this setup
-	 * it is possible to use it later on in e.g. the Linux
-	 * EMAC driver for performance gain.
-	 */
-	mtdcr(SDRAM_PLBADDULL, 0x00000000); /* MQ0_BAUL */
-	mtdcr(SDRAM_PLBADDUHB, 0x00000008); /* MQ0_BAUH */
+	{
+		/*
+		* Enable high bandwidth access on 460EX/GT.
+		* This should/could probably be done on other
+		* PPC's too, like 440SPe.
+		* This is currently not used, but with this setup
+		* it is possible to use it later on in e.g. the Linux
+		* EMAC driver for performance gain.
+		*/
+		
+		unsigned long val;
+
+		mtdcr(SDRAM_PLBADDULL, 0x00000000); /* MQ0_BAUL */
+		mtdcr(SDRAM_PLBADDUHB, 0x00000008); /* MQ0_BAUH */
+
+		val = (mfdcr(SDRAM_CONF1HB) | SDRAM_CONF1HB_AAFR | SDRAM_CONF1HB_RPEN | SDRAM_CONF1HB_RFTE);
+		mtdcr(SDRAM_CONF1HB, val); 
+
+		val = (mfdcr(SDRAM_CONF1LL) | SDRAM_CONF1LL_AAFR | SDRAM_CONF1LL_RPEN | SDRAM_CONF1LL_RFTE);
+		mtdcr(SDRAM_CONF1LL, val); 
+
+		val = (mfdcr(SDRAM_CONFPATHB) | SDRAM_CONFPATHB_TPEN);
+		mtdcr(SDRAM_CONFPATHB, val); 
+	}
 #endif
 }
cpu/ppc4xx/cpu_init.c +15 -0
index ac64279..f7c9200 100644
@@ -288,6 +288,21 @@ cpu_init_f (void)
 	val |= 0x400;
 	mtsdr(SDR0_USB2HOST_CFG, val);
 #endif /* CONFIG_460EX */
+
+#ifdef CONFIG_PLB4_CROSSBAR_ARBITER_CORE
+	/* Segment0 */
+	mtdcr(plb0_acr, plb0_acr_ppm_fair    | 
+			plb0_acr_hbu_enabled |
+			plb0_acr_rdp_4deep   | 
+			plb0_acr_wrp_2deep);
+
+	/* Segment1 */
+	mtdcr(plb1_acr, plb1_acr_ppm_fair    | 
+			plb1_acr_hbu_enabled |
+			plb1_acr_rdp_4deep   | 
+			plb1_acr_wrp_2deep);
+#endif
+
 }
 
 /*
include/asm-ppc/ppc4xx-sdram.h +16 -0
index e151f0c..1abff94 100644
@@ -266,15 +266,31 @@
 #define SDRAM_R2BAS	(SDRAMQ_DCR_BASE+0x2)	/* rank 2 base address & size  */
 #define SDRAM_R3BAS	(SDRAMQ_DCR_BASE+0x3)	/* rank 3 base address & size  */
 #define SDRAM_CONF1HB	(SDRAMQ_DCR_BASE+0x5)	/* configuration 1 HB          */
+#define SDRAM_CONF1HB_AAFR   0x80000000         /* Address Ack on First Request - Bit 0 */
+#define SDRAM_CONF1HB_PRPD   0x00080000         /* PLB Read pipeline Disable - Bit 12 */
+#define SDRAM_CONF1HB_PWPD   0x00040000         /* PLB Write pipeline Disable - Bit 13 */
+#define SDRAM_CONF1HB_PRW    0x00020000         /* PLB Read Wait - Bit 14 */
+#define SDRAM_CONF1HB_RPEN   0x00000800         /* Read Passing Enable - Bit 20 */
+#define SDRAM_CONF1HB_RFTE   0x00000400         /* Read Flow Through Enable - Bit 21 */
+
 #define SDRAM_ERRSTATHB	(SDRAMQ_DCR_BASE+0x7)	/* error status HB             */
 #define SDRAM_ERRADDUHB	(SDRAMQ_DCR_BASE+0x8)	/* error address upper 32 HB   */
 #define SDRAM_ERRADDLHB	(SDRAMQ_DCR_BASE+0x9)	/* error address lower 32 HB   */
 #define SDRAM_PLBADDULL	(SDRAMQ_DCR_BASE+0xA)	/* PLB base address upper 32 LL */
 #define SDRAM_CONF1LL	(SDRAMQ_DCR_BASE+0xB)	/* configuration 1 LL          */
+#define SDRAM_CONF1LL_AAFR   0x80000000         /* Address Ack on First Request - Bit 0 */
+#define SDRAM_CONF1LL_PRPD   0x00080000         /* PLB Read pipeline Disable - Bit 12 */
+#define SDRAM_CONF1LL_PWPD   0x00040000         /* PLB Write pipeline Disable - Bit 13 */
+#define SDRAM_CONF1LL_PRW    0x00020000         /* PLB Read Wait - Bit 14 */
+#define SDRAM_CONF1LL_RPEN   0x00000800         /* Read Passing Enable - Bit 20 */
+#define SDRAM_CONF1LL_RFTE   0x00000400         /* Read Flow Through Enable - Bit 21 */
+
 #define SDRAM_ERRSTATLL	(SDRAMQ_DCR_BASE+0xC)	/* error status LL             */
 #define SDRAM_ERRADDULL	(SDRAMQ_DCR_BASE+0xD)	/* error address upper 32 LL   */
 #define SDRAM_ERRADDLLL	(SDRAMQ_DCR_BASE+0xE)	/* error address lower 32 LL   */
 #define SDRAM_CONFPATHB	(SDRAMQ_DCR_BASE+0xF)	/* configuration between paths */
+#define SDRAM_CONFPATHB_TPEN  0x08000000        /* Transaction Passing Enable - Bit 4 */
+
 #define SDRAM_PLBADDUHB	(SDRAMQ_DCR_BASE+0x10)	/* PLB base address upper 32 LL */
 
 #if !defined(CONFIG_405EX)
include/ppc440.h +1 -48

61 lines not on this page · raw message

include/ppc4xx.h +60 -0

66 lines not on this page · raw message

--------------------------------------------------------

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message.

#1 lore raw

PATCH 1/1 PPC4xx: PLB4 Arbiter and Memory Queue Optimizations

Quoted 4 lines · Prodyut Hazarika

PLB4 Crossbar Arbiter core is instantiated on 405EX/ 440EP/ 440EPx/ 440GR/ 440GRx/ 440SP/ 440SPe/ 460EX/ 460GT For optimal performance, the read pipeline depth must be set to 4. The current code enables it only for bamboo, sequoia and yosemite boards.

That's not correct. Other non-AMCC 440EP/EPx boards have it set too.

Quoted 3 lines

Better approach is to define CONFIG_PLB4_CROSSBAR_ARBITER_CORE for the above PPC4xx families, and to remove the duplicated code from board specfic files.

OK. But on which 4xx boards did you test this change? And could you please remove the duplicated code from the "other", non-AMCC boards too with your next patch version (DU440, PMC440, korat, lwmon5, pcs440ep). I added the other board maintainers to CC.

Quoted 4 lines

Also, added register definitions for MemoryQueue related registers, and enabled Memory Queue optimizatios for 460EX/GT.

Signed-off-by Prodyut Hazarika <[email protected]>

This patch doesn't apply:

[stefan at kubuntu u-boot-ppc4xx (master)]$ git-am -3 patches_amcc/\[PATCH\]\
PPC4xx\:\ PLB4\ Arbiter\ and\ Memory\ Queue\ Optimizations.mbox Applying PPC4xx: PLB4 Arbiter and Memory Queue Optimizations .dotest/patch:125: trailing whitespace.

.dotest/patch:132: trailing whitespace.

                mtdcr(SDRAM_CONF1HB, val);
.dotest/patch:135: trailing whitespace.
                mtdcr(SDRAM_CONF1LL, val);
.dotest/patch:138: trailing whitespace.
                mtdcr(SDRAM_CONFPATHB, val);

.dotest/patch:154: trailing whitespace.
mtdcr(plb0_acr, plb0_acr_ppm_fair |
fatal: corrupt patch at line 387
Patch failed at 0001.
When you have resolved this problem run "git-am -3 --resolved". If you would prefer to skip this patch, instead run "git-am -3 --skip".

There are several whitespace problems which should be fixed too. But the main problem is that the patch doesn't apply at all. How did you generate this patch? I strongly encourage you to use the git tools to handle patches (git-format-patch and git-send-email).

Please find some review comments below.

Quoted 108 lines

---

 board/amcc/bamboo/bamboo.c     |   17 -----------
 board/amcc/sequoia/sequoia.c   |   17 -----------
 board/amcc/yosemite/yosemite.c |   17 -----------
 cpu/ppc4xx/44x_spd_ddr2.c      |   34 ++++++++++++++++------
 cpu/ppc4xx/cpu_init.c          |   15 ++++++++++
 include/asm-ppc/ppc4xx-sdram.h |   16 ++++++++++
 include/ppc440.h               |   49 +--------------------------------
 include/ppc4xx.h               |   60
++++++++++++++++++++++++++++++++++++++++
 8 files changed, 116 insertions(+), 109 deletions(-)
board/amcc/bamboo/bamboo.c +1 -3
index f415701..761e418 100644
@@ -500,23 +500,6 @@ int pci_pre_init(struct pci_controller *hose)
 	addr = mfdcr(plb4_acr) | 0xa0000000;	/* Was 0x8---- */
 	mtdcr(plb4_acr, addr);
 
-	/*-----------------------------------------------------------------------
--+ -	  | Set Nebula PLB4 arbiter to fair mode.
-	 
+-------------------------------------------------------------------------*

/ - /* Segment0 */

board/amcc/bamboo/bamboo.c +0 -13
-	addr = (mfdcr(plb0_acr) & ~plb0_acr_ppm_mask) | plb0_acr_ppm_fair;
-	addr = (addr & ~plb0_acr_hbu_mask) | plb0_acr_hbu_enabled;
-	addr = (addr & ~plb0_acr_rdp_mask) | plb0_acr_rdp_4deep;
-	addr = (addr & ~plb0_acr_wrp_mask) | plb0_acr_wrp_2deep;
-	mtdcr(plb0_acr, addr);
-
-	/* Segment1 */
-	addr = (mfdcr(plb1_acr) & ~plb1_acr_ppm_mask) | plb1_acr_ppm_fair;
-	addr = (addr & ~plb1_acr_hbu_mask) | plb1_acr_hbu_enabled;
-	addr = (addr & ~plb1_acr_rdp_mask) | plb1_acr_rdp_4deep;
-	addr = (addr & ~plb1_acr_wrp_mask) | plb1_acr_wrp_2deep;
-	mtdcr(plb1_acr, addr);
-
 	return 1;
 }
 #endif /* defined(CONFIG_PCI) */
board/amcc/sequoia/sequoia.c +0 -17
index b833092..639db09 100644
@@ -372,23 +372,6 @@ int pci_pre_init(struct pci_controller *hose)
 	addr = mfdcr(plb4_acr) | 0xa0000000;	/* Was 0x8---- */
 	mtdcr(plb4_acr, addr);
 
-	/*
-	 * Set Nebula PLB4 arbiter to fair mode.
-	 */
-	/* Segment0 */
-	addr = (mfdcr(plb0_acr) & ~plb0_acr_ppm_mask) | plb0_acr_ppm_fair;
-	addr = (addr & ~plb0_acr_hbu_mask) | plb0_acr_hbu_enabled;
-	addr = (addr & ~plb0_acr_rdp_mask) | plb0_acr_rdp_4deep;
-	addr = (addr & ~plb0_acr_wrp_mask) | plb0_acr_wrp_2deep;
-	mtdcr(plb0_acr, addr);
-
-	/* Segment1 */
-	addr = (mfdcr(plb1_acr) & ~plb1_acr_ppm_mask) | plb1_acr_ppm_fair;
-	addr = (addr & ~plb1_acr_hbu_mask) | plb1_acr_hbu_enabled;
-	addr = (addr & ~plb1_acr_rdp_mask) | plb1_acr_rdp_4deep;
-	addr = (addr & ~plb1_acr_wrp_mask) | plb1_acr_wrp_2deep;
-	mtdcr(plb1_acr, addr);
-
 #ifdef CONFIG_PCI_PNP
 	hose->fixup_irq = sequoia_pci_fixup_irq;
 #endif
(quoted diff) +0 -0

b/board/amcc/yosemite/yosemite.c index 05be40a..b18637d 100644

board/amcc/yosemite/yosemite.c +1 -3
@@ -366,23 +366,6 @@ int pci_pre_init(struct pci_controller *hose)
 	addr = mfdcr(plb4_acr) | 0xa0000000;	/* Was 0x8---- */
 	mtdcr(plb4_acr, addr);
 
-	/*-----------------------------------------------------------------------
--+ -	  | Set Nebula PLB4 arbiter to fair mode.
-	 
+-------------------------------------------------------------------------*

/ - /* Segment0 */

board/amcc/yosemite/yosemite.c +0 -13
-	addr = (mfdcr(plb0_acr) & ~plb0_acr_ppm_mask) | plb0_acr_ppm_fair;
-	addr = (addr & ~plb0_acr_hbu_mask) | plb0_acr_hbu_enabled;
-	addr = (addr & ~plb0_acr_rdp_mask) | plb0_acr_rdp_4deep;
-	addr = (addr & ~plb0_acr_wrp_mask) | plb0_acr_wrp_2deep;
-	mtdcr(plb0_acr, addr);
-
-	/* Segment1 */
-	addr = (mfdcr(plb1_acr) & ~plb1_acr_ppm_mask) | plb1_acr_ppm_fair;
-	addr = (addr & ~plb1_acr_hbu_mask) | plb1_acr_hbu_enabled;
-	addr = (addr & ~plb1_acr_rdp_mask) | plb1_acr_rdp_4deep;
-	addr = (addr & ~plb1_acr_wrp_mask) | plb1_acr_wrp_2deep;
-	mtdcr(plb1_acr, addr);
-
 	return 1;
 }
 #endif	/* defined(CONFIG_PCI) */
cpu/ppc4xx/44x_spd_ddr2.c +0 -1
index e9940e8..042ad1f 100644
@@ -2241,16 +2241,30 @@ static void program_memory_queue(unsigned long
 *dimm_populated, }
 
 #if defined(CONFIG_460EX) || defined(CONFIG_460GT)
-	/*

cpu/ppc4xx/44x_spd_ddr2.c static void program_memory_queue(unsigned long · line 2244

Quoted 10 lines
cpu/ppc4xx/44x_spd_ddr2.c +1 -9
-	 * Enable high bandwidth access on 460EX/GT.
-	 * This should/could probably be done on other
-	 * PPC's too, like 440SPe.
-	 * This is currently not used, but with this setup
-	 * it is possible to use it later on in e.g. the Linux
-	 * EMAC driver for performance gain.
-	 */
-	mtdcr(SDRAM_PLBADDULL, 0x00000000); /* MQ0_BAUL */
-	mtdcr(SDRAM_PLBADDUHB, 0x00000008); /* MQ0_BAUH */
+	{

I don't particularly like this extra "{" level without need. Please remove it.

cpu/ppc4xx/44x_spd_ddr2.c static void program_memory_queue(unsigned long *dimm_populated, · line 2252

Quoted 8 lines
cpu/ppc4xx/44x_spd_ddr2.c +8 -0
+		/*
+		* Enable high bandwidth access on 460EX/GT.
+		* This should/could probably be done on other
+		* PPC's too, like 440SPe.
+		* This is currently not used, but with this setup
+		* it is possible to use it later on in e.g. the Linux
+		* EMAC driver for performance gain.
+		*/

Indentation is not correct. Should be:

Quoted 2 lines
cpu/ppc4xx/44x_spd_ddr2.c +2 -0
+		/*
+		 * Enable high bandwidth access on 460EX/GT.

...

Quoted 9 lines
cpu/ppc4xx/44x_spd_ddr2.c +7 -0
+
+		unsigned long val;
+
+		mtdcr(SDRAM_PLBADDULL, 0x00000000); /* MQ0_BAUL */
+		mtdcr(SDRAM_PLBADDUHB, 0x00000008); /* MQ0_BAUH */
+
+		val = (mfdcr(SDRAM_CONF1HB) | SDRAM_CONF1HB_AAFR | SDRAM_CONF1HB_RPEN |

SDRAM_CONF1HB_RFTE); + mtdcr(SDRAM_CONF1HB, val);
+

Please wrap those too long lines.

Quoted 21 lines
cpu/ppc4xx/44x_spd_ddr2.c +1 -0
+		val = (mfdcr(SDRAM_CONF1LL) | SDRAM_CONF1LL_AAFR | SDRAM_CONF1LL_RPEN |

SDRAM_CONF1LL_RFTE); + mtdcr(SDRAM_CONF1LL, val);

cpu/ppc4xx/44x_spd_ddr2.c +4 -0
+
+		val = (mfdcr(SDRAM_CONFPATHB) | SDRAM_CONFPATHB_TPEN);
+		mtdcr(SDRAM_CONFPATHB, val);
+	}
 #endif
 }
cpu/ppc4xx/cpu_init.c +4 -0
index ac64279..f7c9200 100644
@@ -288,6 +288,21 @@ cpu_init_f (void)
 	val |= 0x400;
 	mtsdr(SDR0_USB2HOST_CFG, val);
 #endif /* CONFIG_460EX */
+
+#ifdef CONFIG_PLB4_CROSSBAR_ARBITER_CORE
+	/* Segment0 */
+	mtdcr(plb0_acr, plb0_acr_ppm_fair    |

cpu/ppc4xx/cpu_init.c cpu_init_f (void) · line 304

Quoted 10 lines
cpu/ppc4xx/cpu_init.c +10 -0
+			plb0_acr_hbu_enabled |
+			plb0_acr_rdp_4deep   |
+			plb0_acr_wrp_2deep);
+
+	/* Segment1 */
+	mtdcr(plb1_acr, plb1_acr_ppm_fair    |
+			plb1_acr_hbu_enabled |
+			plb1_acr_rdp_4deep   |
+			plb1_acr_wrp_2deep);
+#endif

How about 440EPx/GRx and it's problem with the PLB4A0_ACR[WRP]. Currently this gets cleared in a board specific file because of the MAL burst problems in Linux when its set. Shouldn't we configure it correctly in this place for all 440EPx/GRx and remove it from the board specific code?

cpu/ppc4xx/cpu_init.c cpu_init_f (void) · line 305

Quoted 1 line
cpu/ppc4xx/cpu_init.c +1 -0
+

Please remove this unnecessary empty line.

Quoted 168 lines
cpu/ppc4xx/cpu_init.c +0 -0
 }
 
 /*
(quoted diff) +0 -0

b/include/asm-ppc/ppc4xx-sdram.h index e151f0c..1abff94 100644

include/asm-ppc/ppc4xx-sdram.h +0 -0
@@ -266,15 +266,31 @@
 #define SDRAM_R2BAS	(SDRAMQ_DCR_BASE+0x2)	/* rank 2 base address & size 
 */ #define SDRAM_R3BAS	(SDRAMQ_DCR_BASE+0x3)	/* rank 3 base address & size 
 */ #define SDRAM_CONF1HB	(SDRAMQ_DCR_BASE+0x5)	/* configuration 1 HB       
  */ +#define SDRAM_CONF1HB_AAFR   0x80000000         /* Address Ack on

First Request - Bit 0 */ +#define SDRAM_CONF1HB_PRPD 0x00080000
/* PLB Read pipeline Disable - Bit 12 */ +#define SDRAM_CONF1HB_PWPD
0x00040000 /* PLB Write pipeline Disable - Bit 13 */ +#define
SDRAM_CONF1HB_PRW 0x00020000 /* PLB Read Wait - Bit 14 */
+#define SDRAM_CONF1HB_RPEN 0x00000800 /* Read Passing Enable -
Bit 20 */ +#define SDRAM_CONF1HB_RFTE 0x00000400 /* Read Flow
Through Enable - Bit 21 */ +
#define SDRAM_ERRSTATHB (SDRAMQ_DCR_BASE+0x7) /* error status HB
*/ #define SDRAM_ERRADDUHB (SDRAMQ_DCR_BASE+0x8) /* error address upper
32 HB */ #define SDRAM_ERRADDLHB (SDRAMQ_DCR_BASE+0x9) /* error address
lower 32 HB */ #define SDRAM_PLBADDULL (SDRAMQ_DCR_BASE+0xA) /* PLB base
address upper 32 LL */ #define SDRAM_CONF1LL (SDRAMQ_DCR_BASE+0xB) /*
configuration 1 LL */ +#define SDRAM_CONF1LL_AAFR 0x80000000

    /* Address Ack on First Request - Bit 0 */ +#define SDRAM_CONF1LL_PRPD

0x00080000 /* PLB Read pipeline Disable - Bit 12 */ +#define
SDRAM_CONF1LL_PWPD 0x00040000 /* PLB Write pipeline Disable - Bit
13 */ +#define SDRAM_CONF1LL_PRW 0x00020000 /* PLB Read Wait -
Bit 14 */ +#define SDRAM_CONF1LL_RPEN 0x00000800 /* Read Passing
Enable - Bit 20 */ +#define SDRAM_CONF1LL_RFTE 0x00000400 /* Read
Flow Through Enable - Bit 21 */ +
#define SDRAM_ERRSTATLL (SDRAMQ_DCR_BASE+0xC) /* error status LL
*/ #define SDRAM_ERRADDULL (SDRAMQ_DCR_BASE+0xD) /* error address upper
32 LL */ #define SDRAM_ERRADDLLL (SDRAMQ_DCR_BASE+0xE) /* error address
lower 32 LL */ #define SDRAM_CONFPATHB (SDRAMQ_DCR_BASE+0xF) /*
configuration between paths */ +#define SDRAM_CONFPATHB_TPEN 0x08000000

    /* Transaction Passing Enable - Bit 4 */ +

#define SDRAM_PLBADDUHB (SDRAMQ_DCR_BASE+0x10) /* PLB base address upper
32 LL */

#if !defined(CONFIG_405EX)

include/ppc440.h +0 -47
index c581f1b..8d3a7e3 100644
@@ -422,53 +422,6 @@
 
 #define PLB4_ACR_WRP		(0x80000000 >> 7)
 
-/* Nebula PLB4 Arbiter - PowerPC440EP */
-#define PLB_ARBITER_BASE   0x80
-
-#define plb0_revid                (PLB_ARBITER_BASE+ 0x00)
-#define plb0_acr                  (PLB_ARBITER_BASE+ 0x01)
-#define   plb0_acr_ppm_mask             0xF0000000
-#define   plb0_acr_ppm_fixed            0x00000000
-#define   plb0_acr_ppm_fair             0xD0000000
-#define   plb0_acr_hbu_mask             0x08000000
-#define   plb0_acr_hbu_disabled         0x00000000
-#define   plb0_acr_hbu_enabled          0x08000000
-#define   plb0_acr_rdp_mask             0x06000000
-#define   plb0_acr_rdp_disabled         0x00000000
-#define   plb0_acr_rdp_2deep            0x02000000
-#define   plb0_acr_rdp_3deep            0x04000000
-#define   plb0_acr_rdp_4deep            0x06000000
-#define   plb0_acr_wrp_mask             0x01000000
-#define   plb0_acr_wrp_disabled         0x00000000
-#define   plb0_acr_wrp_2deep            0x01000000
-
-#define plb0_besrl                (PLB_ARBITER_BASE+ 0x02)
-#define plb0_besrh                (PLB_ARBITER_BASE+ 0x03)
-#define plb0_bearl                (PLB_ARBITER_BASE+ 0x04)
-#define plb0_bearh                (PLB_ARBITER_BASE+ 0x05)
-#define plb0_ccr                  (PLB_ARBITER_BASE+ 0x08)
-
-#define plb1_acr                  (PLB_ARBITER_BASE+ 0x09)
-#define   plb1_acr_ppm_mask             0xF0000000
-#define   plb1_acr_ppm_fixed            0x00000000
-#define   plb1_acr_ppm_fair             0xD0000000
-#define   plb1_acr_hbu_mask             0x08000000
-#define   plb1_acr_hbu_disabled         0x00000000
-#define   plb1_acr_hbu_enabled          0x08000000
-#define   plb1_acr_rdp_mask             0x06000000
-#define   plb1_acr_rdp_disabled         0x00000000
-#define   plb1_acr_rdp_2deep            0x02000000
-#define   plb1_acr_rdp_3deep            0x04000000
-#define   plb1_acr_rdp_4deep            0x06000000
-#define   plb1_acr_wrp_mask             0x01000000
-#define   plb1_acr_wrp_disabled         0x00000000
-#define   plb1_acr_wrp_2deep            0x01000000
-
-#define plb1_besrl                (PLB_ARBITER_BASE+ 0x0A)
-#define plb1_besrh                (PLB_ARBITER_BASE+ 0x0B)
-#define plb1_bearl                (PLB_ARBITER_BASE+ 0x0C)
-#define plb1_bearh                (PLB_ARBITER_BASE+ 0x0D)
-
 /* Pin Function Control Register 1 */
 #define SDR0_PFC1                    0x4101
 #define   SDR0_PFC1_U1ME_MASK         0x02000000    /* UART1 Mode Enable

*/ @@ -742,7 +695,7 @@
#define SDR0_PFC1_PLB_PME_PLB4_SEL 0x00001000 /* PLB3 Performance
Monitor Enable */ #define SDR0_PFC1_GFGGI_MASK 0x0000000F /*
GPT Frequency Generation Gated In */

include/ppc440.h +1 -1
-#endif /* 440EP || 440GR || 440EPX || 440GRX */
+#endif  /* 440EP || 440GR || 440EPX || 440GRX */

/*-------------------------------------------------------------------------
----

| L2 Cache

include/ppc4xx.h +46 -0
index 1d06da8..9dabeee 100644
@@ -45,6 +45,66 @@
 #define CONFIG_SDRAM_PPC4xx_IBM_DDR2	/* IBM DDR(2) controller */
 #endif
 
+/* PLB4 CrossBar Arbiter Core supported across PPC4xx families */
+#if defined(CONFIG_440EP) || defined(CONFIG_440EPX) || \
+    defined(CONFIG_440GR) || defined(CONFIG_440GRX) || \
+    defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
+    defined(CONFIG_460EX) || defined(CONFIG_460GT)  || \
+    defined(CONFIG_405EX)
+#define CONFIG_PLB4_CROSSBAR_ARBITER_CORE	/*PLB4 CrossBar Arbiter Core */
+#endif
+
+#ifdef CONFIG_PLB4_CROSSBAR_ARBITER_CORE
+
+#define PLB_ARBITER_BASE   0x80
+
+#define plb0_revid                (PLB_ARBITER_BASE+ 0x00)
+#define plb0_acr                  (PLB_ARBITER_BASE+ 0x01)
+#define   plb0_acr_ppm_mask             0xF0000000
+#define   plb0_acr_ppm_fixed            0x00000000
+#define   plb0_acr_ppm_fair             0xD0000000
+#define   plb0_acr_hbu_mask             0x08000000
+#define   plb0_acr_hbu_disabled         0x00000000
+#define   plb0_acr_hbu_enabled          0x08000000
+#define   plb0_acr_rdp_mask             0x06000000
+#define   plb0_acr_rdp_disabled         0x00000000
+#define   plb0_acr_rdp_2deep            0x02000000
+#define   plb0_acr_rdp_3deep            0x04000000
+#define   plb0_acr_rdp_4deep            0x06000000
+#define   plb0_acr_wrp_mask             0x01000000
+#define   plb0_acr_wrp_disabled         0x00000000
+#define   plb0_acr_wrp_2deep            0x01000000
+
+#define plb0_besrl                (PLB_ARBITER_BASE+ 0x02)
+#define plb0_besrh                (PLB_ARBITER_BASE+ 0x03)
+#define plb0_bearl                (PLB_ARBITER_BASE+ 0x04)
+#define plb0_bearh                (PLB_ARBITER_BASE+ 0x05)
+#define plb0_ccr                  (PLB_ARBITER_BASE+ 0x08)
+
+#define plb1_acr                  (PLB_ARBITER_BASE+ 0x09)
+#define   plb1_acr_ppm_mask             0xF0000000
+#define   plb1_acr_ppm_fixed            0x00000000
+#define   plb1_acr_ppm_fair             0xD0000000
+#define   plb1_acr_hbu_mask             0x08000000
+#define   plb1_acr_hbu_disabled         0x00000000
+#define   plb1_acr_hbu_enabled          0x08000000
+#define   plb1_acr_rdp_mask             0x06000000
+#define   plb1_acr_rdp_disabled         0x00000000
+#define   plb1_acr_rdp_2deep            0x02000000

include/ppc4xx.h line 103

Quoted 10 lines
include/ppc4xx.h +10 -0
+#define   plb1_acr_rdp_3deep            0x04000000
+#define   plb1_acr_rdp_4deep            0x06000000
+#define   plb1_acr_wrp_mask             0x01000000
+#define   plb1_acr_wrp_disabled         0x00000000
+#define   plb1_acr_wrp_2deep            0x01000000
+
+#define plb1_besrl                (PLB_ARBITER_BASE+ 0x0A)
+#define plb1_besrh                (PLB_ARBITER_BASE+ 0x0B)
+#define plb1_bearl                (PLB_ARBITER_BASE+ 0x0C)
+#define plb1_bearh                (PLB_ARBITER_BASE+ 0x0D)

I would really prefer to have uppercase defines here. I know that this is not your "invention" but still we should probably clean this up now, that we are busy with these PLB arbiter defines. So could you please change those defines to uppercase? Thanks.

Please address the comments above and re-submit. Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

author
#2 lore raw

PATCH 1/1 PPC4xx: PLB4 Arbiter and Memory Queue Optimizations

Hi Stefan,
Please put your views on the replies below:

Quoted 1 line

OK. But on which 4xx boards did you test this change? And could you

please remove the duplicated code from the "other", non-AMCC boards too with your > next patch version (DU440, PMC440, korat, lwmon5, pcs440ep). I added the other board maintainers to CC.

I have tested on Canyonlands, Glacier, Yosemite and Kilauea boards. I will make the changes for the other non-AMCC boards and resubmit. I cannot test on non-AMCC boards, though I don't think this patch can cause an issue.

Quoted 5 lines

cpu/ppc4xx/44x_spd_ddr2.c static void program_memory_queue(unsigned long *dimm_populated, · line 2244

Quoted 3 lines
cpu/ppc4xx/44x_spd_ddr2.c +1 -2
-	mtdcr(SDRAM_PLBADDULL, 0x00000000); /* MQ0_BAUL */
-	mtdcr(SDRAM_PLBADDUHB, 0x00000008); /* MQ0_BAUH */
+	{

I don't particularly like this extra "{" level without need. Please

remove it.

The { is kept to avoid warnings. Without the "{", I would need to define the local variable near
The beginning of the function. That would cause warning (Unused variable) for non 440EX/GT families.
Please advice whether you want this warning to come up, if I remove the "{"

Quoted 1 line

How about 440EPx/GRx and it's problem with the PLB4A0_ACR[WRP].

Currently this gets cleared in a board specific file because of the MAL burst

Quoted 1 line

problems in Linux when its set. Shouldn't we configure it correctly in

this place for all 440EPx/GRx and remove it from the board specific code?

This patch is getting bigger and bigger. I will put that cleanup in a separate patch, since it for a different set of cleanups.

Quoted 3 lines

include/ppc4xx.h line 103

Quoted 2 lines
include/ppc4xx.h +2 -0
+#define plb1_bearl                (PLB_ARBITER_BASE+ 0x0C)
+#define plb1_bearh                (PLB_ARBITER_BASE+ 0x0D)

I would really prefer to have uppercase defines here. I know that this

is not your "invention" but still we should probably clean this up now, that

Quoted 1 line

we are busy with these PLB arbiter defines. So could you please change

those defines to uppercase? Thanks.

I will change all to uppercase and resubmit.

Best Regards,
Prodyut Hazarika

================================
Staff S/W Engineer
AMCC
================================
--------------------------------------------------------

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message.

#3 lore raw

PATCH 1/1 PPC4xx: PLB4 Arbiter and Memory Queue Optimizations

Dear Prodyut Hazarika,

Quoted 7 lines · In message <0CA0A16855646F4FA96D25A158E299D604D290EC@SDCEXCHANGE01.ad.amcc.com>…
Quoted 2 lines

I don't particularly like this extra "{" level without need. Please

remove it.

The { is kept to avoid warnings. Without the "{", I would need to define the local variable near
The beginning of the function. That would cause warning (Unused

Yes, that's where variable definitions belong.

Quoted 2 lines

variable) for non 440EX/GT families.
Please advice whether you want this warning to come up, if I remove the

No, warnings are not allowed. You can #ifdef the declaration, can't you?

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
When you die, the first thing you lose is your life. The  next  thing
is the illusions.                       - Terry Pratchett, _Pyramids_
#4 lore raw

PATCH 1/1 PPC4xx: PLB4 Arbiter and Memory Queue Optimizations

In reply to Prodyut Hazarika

Hi Prodyut,

Quoted 8 lines · Prodyut Hazarika

Please put your views on the replies below:

Quoted 1 line

OK. But on which 4xx boards did you test this change? And could you

please remove the duplicated code from the "other", non-AMCC boards too with your > next patch version (DU440, PMC440, korat, lwmon5, pcs440ep). I added the other board maintainers to CC.

I have tested on Canyonlands, Glacier, Yosemite and Kilauea boards.

OK, thanks.

Quoted 4 lines

I
will make the changes for the other non-AMCC boards and resubmit. I cannot test on non-AMCC boards, though I don't think this patch can cause an issue.

Of course you can't test this on non-AMCC boards. Thanks for further cleaning up.

Quoted 6 lines
Quoted 5 lines

cpu/ppc4xx/44x_spd_ddr2.c static void program_memory_queue(unsigned long *dimm_populated, · line 2244

Quoted 3 lines
cpu/ppc4xx/44x_spd_ddr2.c +1 -2
-	mtdcr(SDRAM_PLBADDULL, 0x00000000); /* MQ0_BAUL */
-	mtdcr(SDRAM_PLBADDUHB, 0x00000008); /* MQ0_BAUH */
+	{

I don't particularly like this extra "{" level without need. Please

Quoted 8 lines

remove it.

The { is kept to avoid warnings. Without the "{", I would need to define the local variable near
The beginning of the function. That would cause warning (Unused variable) for non 440EX/GT families.
Please advice whether you want this warning to come up, if I remove the "{"

I knew why you did it this way (have been there before), but your "solution" is not the preferred one. As Wolfgang already explained the variable declarations should go to the beginning of the function and if warnings are seen on other platforms, then the declaration needs an #ifdef too.

Quoted 9 lines
Quoted 1 line

How about 440EPx/GRx and it's problem with the PLB4A0_ACR[WRP].

Currently this gets cleared in a board specific file because of the MAL burst

Quoted 1 line

problems in Linux when its set. Shouldn't we configure it correctly in

this place for all 440EPx/GRx and remove it from the board specific code?

This patch is getting bigger and bigger. I will put that cleanup in a separate patch, since it for a different set of cleanups.

OK. I just came to my mind after looking at the arbiter defines and their usage. We just should not forget to clean this up soon.

Quoted 10 lines
Quoted 4 lines

include/ppc4xx.h line 103

Quoted 2 lines
include/ppc4xx.h +2 -0
+#define plb1_bearl                (PLB_ARBITER_BASE+ 0x0C)
+#define plb1_bearh                (PLB_ARBITER_BASE+ 0x0D)

I would really prefer to have uppercase defines here. I know that this

is not your "invention" but still we should probably clean this up now, that

Quoted 1 line

we are busy with these PLB arbiter defines. So could you please change

those defines to uppercase? Thanks.

I will change all to uppercase and resubmit.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================