Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31615
Change subject: mb/google/cyan: fix RAM training on edgar variant ......................................................................
mb/google/cyan: fix RAM training on edgar variant
Several cyan variants require memory init parameters passed to FSP for handling of specific Micron modules; edgar was missing this, leading to boot failure after the MRC cache was populated. Add the required memory init parameters for edgar.
Test: build/boot on edgar board with affected Micron memory modules, verify subsequent boots successful with populated MRC cache.
Change-Id: I6a2bc30b54ff1a17c854a90dfcb2308d27ee2be7 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/mainboard/google/cyan/variants/edgar/Makefile.inc A src/mainboard/google/cyan/variants/edgar/romstage.c 2 files changed, 48 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/31615/1
diff --git a/src/mainboard/google/cyan/variants/edgar/Makefile.inc b/src/mainboard/google/cyan/variants/edgar/Makefile.inc index 2e8b02c..ad9ac8a 100644 --- a/src/mainboard/google/cyan/variants/edgar/Makefile.inc +++ b/src/mainboard/google/cyan/variants/edgar/Makefile.inc @@ -14,6 +14,7 @@ ## GNU General Public License for more details. ##
+romstage-y += romstage.c romstage-y += spd_util.c
ramstage-y += gpio.c diff --git a/src/mainboard/google/cyan/variants/edgar/romstage.c b/src/mainboard/google/cyan/variants/edgar/romstage.c new file mode 100644 index 0000000..12fef77 --- /dev/null +++ b/src/mainboard/google/cyan/variants/edgar/romstage.c @@ -0,0 +1,47 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2013 Google Inc. + * Copyright (C) 2015 Intel Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <soc/romstage.h> +#include <baseboard/variants.h> +#include <mainboard/google/cyan/spd/spd_util.h> + +void variant_memory_init_params(MEMORY_INIT_UPD *memory_params) +{ + int ram_id = get_ramid(); + + /* + * RAMID = 5 - 4GiB Micron MT52L256M32D1PF-107 + * RAMID = 7 - 2GiB Micron MT52L256M32D1PF-107 + */ + if (ram_id == 5 || ram_id == 7) { + + /* + * For new micron part, it requires read/receive + * enable training before sending cmds to get MR8. + * To override dram geometry settings as below: + * + * PcdDramWidth = x32 + * PcdDramDensity = 8Gb + * PcdDualRankDram = disable + */ + memory_params->PcdRxOdtLimitChannel0 = 1; + memory_params->PcdRxOdtLimitChannel1 = 1; + memory_params->PcdDisableAutoDetectDram = 1; + memory_params->PcdDramWidth = 2; + memory_params->PcdDramDensity = 3; + memory_params->PcdDualRankDram = 0; + } +}
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31615 )
Change subject: mb/google/cyan: fix RAM training on edgar variant ......................................................................
Patch Set 1: Code-Review+1
(3 comments)
Does this have a boot time penalty?
https://review.coreboot.org/#/c/31615/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31615/1//COMMIT_MSG@11 PS1, Line 11: leading to boot failure after : the MRC cache was populated Maybe be more specific, that there was just a black screen and nothing happening?
https://review.coreboot.org/#/c/31615/1//COMMIT_MSG@13 PS1, Line 13: init parameters for edgar. Where did you get these from?
https://review.coreboot.org/#/c/31615/1//COMMIT_MSG@14 PS1, Line 14: Is there already a corresponding commit in the downstream Chromium repositories?
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31615 )
Change subject: mb/google/cyan: fix RAM training on edgar variant ......................................................................
Patch Set 1:
Did you find an explanation for, why the second model with the exact same components kept working? Maybe add that too.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31615 )
Change subject: mb/google/cyan: fix RAM training on edgar variant ......................................................................
Patch Set 1: Code-Review+1
Hello Angel Pons, Paul Menzel, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31615
to look at the new patch set (#2).
Change subject: mb/google/cyan: fix RAM training on edgar variant ......................................................................
mb/google/cyan: fix RAM training on edgar variant
Adapted from Chromium commit 5351dc0d [Edgar: To set the RX ODT limit and dram geometry with RAMID detection]
Several cyan variants require memory init parameters be passed to FSP for handling of specific Micron modules; without these, RAM init will fail when loading training data from the MRC cache, and boot will halt. This was missed when I upstreamed edgar along with the other cyan variants, so add the required memory init parameters for edgar as per its source Chromium branch.
Test: build/boot on edgar board with affected Micron memory modules, verify boot successful with populated MRC cache.
Change-Id: I6a2bc30b54ff1a17c854a90dfcb2308d27ee2be7 Signed-off-by: Matt DeVillier matt.devillier@gmail.com --- M src/mainboard/google/cyan/variants/edgar/Makefile.inc A src/mainboard/google/cyan/variants/edgar/romstage.c 2 files changed, 48 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/31615/2
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31615 )
Change subject: mb/google/cyan: fix RAM training on edgar variant ......................................................................
Patch Set 1:
(3 comments)
Does this have a boot time penalty?
no, it allows the board (with specific Micron modules) to actually boot with a populated MRC cache. Without this patch, boot fails at ram init when loading training data from MRC cache
https://review.coreboot.org/#/c/31615/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31615/1//COMMIT_MSG@11 PS1, Line 11: leading to boot failure after : the MRC cache was populated
Maybe be more specific, that there was just a black screen and nothing happening?
done
https://review.coreboot.org/#/c/31615/1//COMMIT_MSG@13 PS1, Line 13: init parameters for edgar.
Where did you get these from?
added info on source Chromium commit
https://review.coreboot.org/#/c/31615/1//COMMIT_MSG@14 PS1, Line 14:
Is there already a corresponding commit in the downstream Chromium repositories?
info added
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31615 )
Change subject: mb/google/cyan: fix RAM training on edgar variant ......................................................................
Patch Set 2:
Thank you very much for the updated commit messages.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31615 )
Change subject: mb/google/cyan: fix RAM training on edgar variant ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/31615/2/src/mainboard/google/cyan/variants/e... File src/mainboard/google/cyan/variants/edgar/romstage.c:
https://review.coreboot.org/#/c/31615/2/src/mainboard/google/cyan/variants/e... PS2, Line 5: * Copyright (C) 2015 Intel Corp. the code seems younger than that ;)
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31615 )
Change subject: mb/google/cyan: fix RAM training on edgar variant ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31615/2/src/mainboard/google/cyan/variants/e... File src/mainboard/google/cyan/variants/edgar/romstage.c:
https://review.coreboot.org/#/c/31615/2/src/mainboard/google/cyan/variants/e... PS2, Line 5: * Copyright (C) 2015 Intel Corp.
the code seems younger than that ;)
copied from header of file from which this was sourced :)
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31615 )
Change subject: mb/google/cyan: fix RAM training on edgar variant ......................................................................
mb/google/cyan: fix RAM training on edgar variant
Adapted from Chromium commit 5351dc0d [Edgar: To set the RX ODT limit and dram geometry with RAMID detection]
Several cyan variants require memory init parameters be passed to FSP for handling of specific Micron modules; without these, RAM init will fail when loading training data from the MRC cache, and boot will halt. This was missed when I upstreamed edgar along with the other cyan variants, so add the required memory init parameters for edgar as per its source Chromium branch.
Test: build/boot on edgar board with affected Micron memory modules, verify boot successful with populated MRC cache.
Change-Id: I6a2bc30b54ff1a17c854a90dfcb2308d27ee2be7 Signed-off-by: Matt DeVillier matt.devillier@gmail.com Reviewed-on: https://review.coreboot.org/c/31615 Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/cyan/variants/edgar/Makefile.inc A src/mainboard/google/cyan/variants/edgar/romstage.c 2 files changed, 48 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/cyan/variants/edgar/Makefile.inc b/src/mainboard/google/cyan/variants/edgar/Makefile.inc index 2e8b02c..ad9ac8a 100644 --- a/src/mainboard/google/cyan/variants/edgar/Makefile.inc +++ b/src/mainboard/google/cyan/variants/edgar/Makefile.inc @@ -14,6 +14,7 @@ ## GNU General Public License for more details. ##
+romstage-y += romstage.c romstage-y += spd_util.c
ramstage-y += gpio.c diff --git a/src/mainboard/google/cyan/variants/edgar/romstage.c b/src/mainboard/google/cyan/variants/edgar/romstage.c new file mode 100644 index 0000000..12fef77 --- /dev/null +++ b/src/mainboard/google/cyan/variants/edgar/romstage.c @@ -0,0 +1,47 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2013 Google Inc. + * Copyright (C) 2015 Intel Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <soc/romstage.h> +#include <baseboard/variants.h> +#include <mainboard/google/cyan/spd/spd_util.h> + +void variant_memory_init_params(MEMORY_INIT_UPD *memory_params) +{ + int ram_id = get_ramid(); + + /* + * RAMID = 5 - 4GiB Micron MT52L256M32D1PF-107 + * RAMID = 7 - 2GiB Micron MT52L256M32D1PF-107 + */ + if (ram_id == 5 || ram_id == 7) { + + /* + * For new micron part, it requires read/receive + * enable training before sending cmds to get MR8. + * To override dram geometry settings as below: + * + * PcdDramWidth = x32 + * PcdDramDensity = 8Gb + * PcdDualRankDram = disable + */ + memory_params->PcdRxOdtLimitChannel0 = 1; + memory_params->PcdRxOdtLimitChannel1 = 1; + memory_params->PcdDisableAutoDetectDram = 1; + memory_params->PcdDramWidth = 2; + memory_params->PcdDramDensity = 3; + memory_params->PcdDualRankDram = 0; + } +}
-
Angel Pons (Code Review) -
Matt DeVillier (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)