[U-Boot] PPC440GX: DDR ECC init time.

[U-Boot] PPC440GX: DDR ECC init time.

[Wouter Eckhardt]

Hi all,

I'm making quite good progress porting U-Boot (2009.03) to my custom
PPC440GX board. Right now I'm trying to solve a little problem I have
with board start-up time when I enable ECC on DDR RAM. The board
literally takes minutes to initialize RAM. I'm guessing this is due to
the fact that ecc_init() fills the entire RAM (the comments already
suggest some performance enhancements can be implemented).

I've tried solving this by shortly enabling the D cache before writing
RAM and disabling the D cache afterwards, using the function
change_tlb(). However, if I enable the D cache using change_tlb() and
supply it with the same parameters ecc_init() receives, I get an
exception when change_tlb() invalidates the cache. Commenting out that
call (just to try it) solves the exception problem, but the RAM
initialization time does not really seem to improve.

What can I do to speed up this ECC initialization? Have I forgotten
something when I try to enable the D cache? Any suggestions are welcome.

Kind regards,
Met vriendelijke groet,
Wouter Eckhardt
Engineer
wouter.eckhardt at prodrive.nl
tel. +31 40 2676187
Prodrive B.V.
Postbus 28030
5602 JA Eindhoven
Ekkersrijt 5025
5692 EB Son
The Netherlands
tel. +31 40 2676200
fax: +31 40 2676201
www.prodrive.nl


Disclaimer: The information contained in this email, including any attachments is 
confidential and is for the sole use of the intended recipient(s). Any unauthorized 
review, use, disclosure or distribution is prohibited. If you are not the intended 
recipient, please notify the sender immediately by replying to this message and 
destroy all copies of this message and any attachments.

[U-Boot] PPC440GX: DDR ECC init time.

[Stefan Roese]

Hi Wouter,

On Friday 04 September 2009 14:34:49 Wouter Eckhardt wrote:
> I'm making quite good progress porting U-Boot (2009.03) to my custom
> PPC440GX board.

2009.03 is already "old". I suggest you use the 2009.09 release.

> Right now I'm trying to solve a little problem I have
> with board start-up time when I enable ECC on DDR RAM. The board
> literally takes minutes to initialize RAM. I'm guessing this is due to
> the fact that ecc_init() fills the entire RAM (the comments already
> suggest some performance enhancements can be implemented).

d-cache is the solution.
 
> I've tried solving this by shortly enabling the D cache before writing
> RAM and disabling the D cache afterwards, using the function
> change_tlb(). However, if I enable the D cache using change_tlb() and
> supply it with the same parameters ecc_init() receives, I get an
> exception when change_tlb() invalidates the cache.

Did you flush the caches? You need to be careful here, when changing TLB 
attributes.

Which DDR2 init code are you using btw? A specific custom code with fixed 
settings? Or the 4xx common SPD code? I suggest you take a look at the common 
DDR2 code (44x_spd_ddr2.c). ECC handling is done there already with caches 
enabled. This should give you an idea.

Cheers,
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

[U-Boot] PPC440GX: DDR ECC init time.

[Wouter Eckhardt]

Hi Stefan,

Thanks for the quick reply.

> 
> 2009.03 is already "old". I suggest you use the 2009.09 release.
> 

Okay, shouldn't be too much trouble. (You actually meant .08, right? :-)

> 
> d-cache is the solution.
> 

That's what I thought as well. Seems that coming up with the solution
was a bit easier than actually implementing it...

> 
> Did you flush the caches? You need to be careful here, when changing
TLB
> attributes.
> 
> Which DDR2 init code are you using btw? A specific custom code with
fixed
> settings? Or the 4xx common SPD code? I suggest you take a look at the
> common
> DDR2 code (44x_spd_ddr2.c). ECC handling is done there already with
caches
> enabled. This should give you an idea.

I didn't flush the cache (seemed a bit pointless since they're not in
use at that point anyway, right?). I'll give it a whirl. I'll also look
into the other ECC initialization. 

I actually thought ECC initialization was only done in sdram.c (after a
quick search for CONFIG_SDRAM_ECC). That probably also answers your next
question. My SDRAM initialization is the same one as is used for the
ALPR board and that uses the common code, as far as I know.

Kind regards,
Wouter Eckhardt.

Disclaimer: The information contained in this email, including any attachments is 
confidential and is for the sole use of the intended recipient(s). Any unauthorized 
review, use, disclosure or distribution is prohibited. If you are not the intended 
recipient, please notify the sender immediately by replying to this message and 
destroy all copies of this message and any attachments.

[U-Boot] PPC440GX: DDR ECC init time.

[Stefan Roese]

Hi Wouter,

On Friday 04 September 2009 15:06:52 Wouter Eckhardt wrote:
> > 2009.03 is already "old". I suggest you use the 2009.09 release.
> 
> Okay, shouldn't be too much trouble. (You actually meant .08, right? :-)

Of course. :)
 
> > d-cache is the solution.
> 
> That's what I thought as well. Seems that coming up with the solution
> was a bit easier than actually implementing it...
> 
> > Did you flush the caches? You need to be careful here, when changing
> > TLB attributes.
> >
> > Which DDR2 init code are you using btw? A specific custom code with
> > fixed
> > settings? Or the 4xx common SPD code? I suggest you take a look at the
> > common
> > DDR2 code (44x_spd_ddr2.c). ECC handling is done there already with
> > caches enabled. This should give you an idea.
> 
> I didn't flush the cache (seemed a bit pointless since they're not in
> use at that point anyway, right?). I'll give it a whirl. I'll also look
> into the other ECC initialization.

Good.
 
> I actually thought ECC initialization was only done in sdram.c (after a
> quick search for CONFIG_SDRAM_ECC). That probably also answers your next
> question. My SDRAM initialization is the same one as is used for the
> ALPR board and that uses the common code, as far as I know.

Right. After looking at it, the ECC init is done in this common file. But the 
cache handling is missing here. I suggest you try to port this stuff from the 
DDR2 init file I mentioned in my last mail.
 
Cheers,
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

[U-Boot] PPC440GX: DDR ECC init time.

[Wouter Eckhardt]

Hi Stefan,

> > I didn't flush the cache (seemed a bit pointless since they're not
in
> > use at that point anyway, right?). I'll give it a whirl. I'll also
look
> > into the other ECC initialization.
> 
> Good.
> 
> > I actually thought ECC initialization was only done in sdram.c
(after a
> > quick search for CONFIG_SDRAM_ECC). That probably also answers your
next
> > question. My SDRAM initialization is the same one as is used for the
> > ALPR board and that uses the common code, as far as I know.
> 
> Right. After looking at it, the ECC init is done in this common file.
But
> the
> cache handling is missing here. I suggest you try to port this stuff
from
> the
> DDR2 init file I mentioned in my last mail.

Well, I've been trying to work this into my U-Boot. I haven't succeeded
so far. Basically, the cache stuff in 4xx_spd_ddr2.c consists of setting
up a TLB without the CACHE_INHIBITED bit and then using some cache
instructions to fill up memory. That's what I've been trying to do, but
my call to change_tlb() hangs because of the invalidate_dcache() call
(bad trap exceptions). What could be going on here?

I'll try and set up the DDR TLB dynamically instead of statically (in
init.S) and see if I can get that working.

By the way, I've also stumbled upon some other VERY strange behavior. If
I leave the ecc_init() in its original state and just add in a puts(" ")
call at the beginning of the function, ECC generation is finished VERY
quickly. What influence could adding the puts() call possibly have on
the speed of generating ECC values in DDR?

Kind regards,
Wouter Eckhardt.

Disclaimer: The information contained in this email, including any attachments is 
confidential and is for the sole use of the intended recipient(s). Any unauthorized 
review, use, disclosure or distribution is prohibited. If you are not the intended 
recipient, please notify the sender immediately by replying to this message and 
destroy all copies of this message and any attachments.

[U-Boot] PPC440GX: DDR ECC init time.

[Stefan Roese]

On Monday 07 September 2009 15:57:19 Wouter Eckhardt wrote:
> Well, I've been trying to work this into my U-Boot. I haven't succeeded
> so far. Basically, the cache stuff in 4xx_spd_ddr2.c consists of setting
> up a TLB without the CACHE_INHIBITED bit and then using some cache
> instructions to fill up memory. That's what I've been trying to do, but
> my call to change_tlb() hangs because of the invalidate_dcache() call
> (bad trap exceptions). What could be going on here?
> 
> I'll try and set up the DDR TLB dynamically instead of statically (in
> init.S) and see if I can get that working.

Yes. That's what I would do as well.
 
> By the way, I've also stumbled upon some other VERY strange behavior. If
> I leave the ecc_init() in its original state and just add in a puts(" ")
> call at the beginning of the function, ECC generation is finished VERY
> quickly. What influence could adding the puts() call possibly have on
> the speed of generating ECC values in DDR?

That's strange indeed. I suspect a problem in the code then. Try looking at 
the generated assembler code and/or debug with an BDI2000/3000.

Cheers,
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

[U-Boot] PPC440GX: DDR ECC init time.

[Felix Radensky]

Hi,  Stefan

Stefan Roese wrote:
> On Monday 07 September 2009 15:57:19 Wouter Eckhardt wrote:
>   
>> Well, I've been trying to work this into my U-Boot. I haven't succeeded
>> so far. Basically, the cache stuff in 4xx_spd_ddr2.c consists of setting
>> up a TLB without the CACHE_INHIBITED bit and then using some cache
>> instructions to fill up memory. That's what I've been trying to do, but
>> my call to change_tlb() hangs because of the invalidate_dcache() call
>> (bad trap exceptions). What could be going on here?
>>
>> I'll try and set up the DDR TLB dynamically instead of statically (in
>> init.S) and see if I can get that working.
>>     
>
> Yes. That's what I would do as well.
>  
>   
>> By the way, I've also stumbled upon some other VERY strange behavior. If
>> I leave the ecc_init() in its original state and just add in a puts(" ")
>> call at the beginning of the function, ECC generation is finished VERY
>> quickly. What influence could adding the puts() call possibly have on
>> the speed of generating ECC values in DDR?
>>     
>
> That's strange indeed. I suspect a problem in the code then. Try looking at 
> the generated assembler code and/or debug with an BDI2000/3000.
>   
Not exactly related to the subject under discussion, but I thought I'd 
mention it.

I had problems with ecc_init() on a custom 460EX board with soldered DDR2.
Right after ecc_init() u-boot was crashing on PLB access. I've modified 
the code
to use program_ecc_addr() instead of ecc_init(), and problem was solved. 
I was
wandering why use two different ECC initialization routines for SPD and 
soldered
cases, when program_ecc_addr() can do the job in both cases, while 
ecc_init()
apparently has issues ?

Thanks.

Felix.

> Cheers,
> 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
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>   

[U-Boot] PPC440GX: DDR ECC init time.

[Stefan Roese]

Hi Felix,

On Tuesday 08 September 2009 11:19:41 Felix Radensky wrote:
> Not exactly related to the subject under discussion, but I thought I'd
> mention it.
> 
> I had problems with ecc_init() on a custom 460EX board with soldered DDR2.
> Right after ecc_init() u-boot was crashing on PLB access. I've modified
> the code
> to use program_ecc_addr() instead of ecc_init(), and problem was solved.
> I was
> wandering why use two different ECC initialization routines for SPD and
> soldered
> cases, when program_ecc_addr() can do the job in both cases, while
> ecc_init()
> apparently has issues ?

Most likely historic reasons. I added Grant Erickson to Cc, IIRC he 
added/tested this ecc_init() code. Maybe he can shed some more light into 
this.

But from looking at it, it seems to me that we should get rid of one of those 
routines. program_ecc_addr() seems more generic to me. Patches welcome. ;)

Thanks.

Cheers,
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

[U-Boot] PPC440GX: DDR ECC init time.

[Felix Radensky]

Hi, Stefan

Stefan Roese wrote:
> Hi Felix,
>
> On Tuesday 08 September 2009 11:19:41 Felix Radensky wrote:
>   
>> Not exactly related to the subject under discussion, but I thought I'd
>> mention it.
>>
>> I had problems with ecc_init() on a custom 460EX board with soldered DDR2.
>> Right after ecc_init() u-boot was crashing on PLB access. I've modified
>> the code
>> to use program_ecc_addr() instead of ecc_init(), and problem was solved.
>> I was
>> wandering why use two different ECC initialization routines for SPD and
>> soldered
>> cases, when program_ecc_addr() can do the job in both cases, while
>> ecc_init()
>> apparently has issues ?
>>     
>
> Most likely historic reasons. I added Grant Erickson to Cc, IIRC he 
> added/tested this ecc_init() code. Maybe he can shed some more light into 
> this.
>
> But from looking at it, it seems to me that we should get rid of one of those 
> routines. program_ecc_addr() seems more generic to me. Patches welcome. ;)
>   

OK, unless Grant objects I'll try to cook up a patch, most probably next 
week.
I have 405EX and 460EX boards with ECC enabled soldered DDR2, so
I'll be able to test these cases. It would be great if someone could 
test SPD case,
to see that I didn't break anything.

Felix.
> Thanks.
>
> Cheers,
> 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
>   

[U-Boot] PPC440GX: DDR ECC init time.

[Stefan Roese]

Felix,

On Tuesday 08 September 2009 12:05:30 Felix Radensky wrote:
> > Most likely historic reasons. I added Grant Erickson to Cc, IIRC he
> > added/tested this ecc_init() code. Maybe he can shed some more light into
> > this.
> >
> > But from looking at it, it seems to me that we should get rid of one of
> > those routines. program_ecc_addr() seems more generic to me. Patches
> > welcome. ;)
> 
> OK, unless Grant objects I'll try to cook up a patch, most probably next
> week.
> I have 405EX and 460EX boards with ECC enabled soldered DDR2, so
> I'll be able to test these cases. It would be great if someone could
> test SPD case,
> to see that I didn't break anything.

Great. I'll test it on Katmai (440SPe DDR2 with ECC enabled).

Thanks. 

Cheers,
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

[U-Boot] PPC440GX: DDR ECC init time.

[Grant Erickson]

On 9/8/09 2:49 AM, Stefan Roese wrote:
> On Tuesday 08 September 2009 11:19:41 Felix Radensky wrote:
>> Not exactly related to the subject under discussion, but I thought I'd
>> mention it.
>> 
>> I had problems with ecc_init() on a custom 460EX board with soldered DDR2.
>> Right after ecc_init() u-boot was crashing on PLB access. I've modified
>> the code
>> to use program_ecc_addr() instead of ecc_init(), and problem was solved.
>> I was
>> wandering why use two different ECC initialization routines for SPD and
>> soldered
>> cases, when program_ecc_addr() can do the job in both cases, while
>> ecc_init()
>> apparently has issues ?
> 
> Most likely historic reasons. I added Grant Erickson to Cc, IIRC he
> added/tested this ecc_init() code. Maybe he can shed some more light into
> this.
> 
> But from looking at it, it seems to me that we should get rid of one of those
> routines. program_ecc_addr() seems more generic to me. Patches welcome. ;)
> 
> Thanks.
> 
> Cheers,
> Stefan

Felix:

Stefan is correct. IIRC, bits of the various 4xx/44x RAM initialization
functions were evolving in parallel at that time. I believe I used ecc_init
at the time because it was more fitting to the soldered DDR2 405EXr board I
was using vs. program_ecc_addr which was more tuned to 440, SPD socketed
RAM.

Looking at the code today, that no longer appears to be the case. So, I
agree with Stefan. For the present, these implementations should be merged
and unified and any duplicate code eliminated.

Regards,

Grant