Nick Vaccaro has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
mb/google/volteer: use new tigerlake memory config
Some of the common memory code that was being performed in mainboard has moved into the soc to reduce redundant code. This change adapts volteer to use tigerlake's new common code.
Define GPIO_MEM_CH_SEL to use for determining if all of memory is populated or only half.
- if value read from GPIO_MEM_CH_SEL is 1, only half of DRAM is populated
- if value read is 0, or GPIO_MEM_CH_SEL is not defined, all of DRAM is populated
BUG=b:145642089, b:145238504, b:145564831 BRANCH=none TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer, log into kernel, "cat /proc/meminfo" and verify it reports "MemTotal: 8038196 kB".
Change-Id: I32c9b8a040728d44565806eece6cf60b6b6073b6 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- A src/mainboard/google/volteer/romstage.c A src/mainboard/google/volteer/variants/baseboard/memory.c 2 files changed, 114 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38715/1
diff --git a/src/mainboard/google/volteer/romstage.c b/src/mainboard/google/volteer/romstage.c new file mode 100644 index 0000000..ded300c --- /dev/null +++ b/src/mainboard/google/volteer/romstage.c @@ -0,0 +1,45 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2020 The coreboot project Authors. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include <assert.h> +#include <baseboard/variants.h> +#include <cbfs.h> +#include <gpio.h> +#include <soc/gpio.h> +#include <soc/meminit_tgl.h> +#include <soc/romstage.h> +#include <string.h> +#include <variant/gpio.h> + +#include <fsp/soc_binding.h> + +void mainboard_memory_init_params(FSPM_UPD *mupd) +{ + FSP_M_CONFIG *mem_cfg = &mupd->FspmConfig; + struct mb_lpddr4x_cfg board_cfg; + bool half_populated = false; + + memset(&board_cfg, 0, sizeof(board_cfg)); + variant_memory_params(&board_cfg); + + /* + * If GPIO_MEM_CH_SEL is defined, check it to see if we are only + * populating half of the memory slots. + * + * The gpio will read as 1 if half populating DRAM, and will read as + * 0 if using all available memory slots. + */ +#ifdef GPIO_MEM_CH_SEL + half_populated = gpio_get(GPIO_MEM_CH_SEL); +#endif + board_cfg.half_populated = half_populated; + board_cfg.spd.read_type = READ_SPD_CBFS; + board_cfg.spd.spd_spec.spd_index = variant_memory_sku(); + + meminit_lpddr4x(mem_cfg, &board_cfg); +} diff --git a/src/mainboard/google/volteer/variants/baseboard/memory.c b/src/mainboard/google/volteer/variants/baseboard/memory.c new file mode 100644 index 0000000..6379c54 --- /dev/null +++ b/src/mainboard/google/volteer/variants/baseboard/memory.c @@ -0,0 +1,69 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2020 The coreboot project Authors. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include <baseboard/gpio.h> +#include <baseboard/variants.h> +#include <gpio.h> + +/* DQ byte map */ +static const struct mb_lpddr4x_cfg baseboard_memcfg = { + /* DQ byte map */ + .dq_map = { + { 0, 1, 2, 3, 4, 5, 6, 7, /* Byte 0 */ + 12, 13, 14, 15, 11, 10, 9, 8 }, /* Byte 1 */ + { 7, 2, 6, 3, 5, 1, 4, 0, /* Byte 2 */ + 10, 8, 9, 11, 15, 12, 14, 13 }, /* Byte 3 */ + { 3, 2, 1, 0, 4, 5, 6, 7, /* Byte 4 */ + 12, 13, 14, 15, 11, 10, 9, 8 }, /* Byte 5 */ + { 7, 0, 1, 6, 5, 4, 2, 3, /* Byte 6 */ + 15, 14, 8, 9, 10, 12, 11, 13 }, /* Byte 7 */ + { 3, 2, 1, 0, 4, 5, 6, 7, /* Byte 0 */ + 12, 13, 14, 15, 11, 10, 9, 8 }, /* Byte 1 */ + { 3, 4, 2, 5, 0, 6, 1, 7, /* Byte 2 */ + 13, 12, 11, 10, 14, 15, 9, 8 }, /* Byte 3 */ + { 3, 2, 1, 0, 7, 4, 5, 6, /* Byte 4 */ + 15, 14, 13, 12, 8, 9, 10, 11 }, /* Byte 5 */ + { 3, 4, 2, 5, 1, 0, 7, 6, /* Byte 6 */ + 15, 14, 9, 8, 12, 10, 11, 13 } /* Byte 7 */ + }, + + /* DQS CPU<>DRAM map */ + .dqs_map = { + /* Ch 0 1 2 3 */ + { 0, 1 }, { 0, 1 }, { 0, 1 }, { 0, 1 }, + { 0, 1 }, { 0, 1 }, { 0, 1 }, { 0, 1 } + }, + + .ect = 0, /* Disable Early Command Training */ + + .half_populated = 0, /* Use all of memory */ +}; + +void variant_memory_params(struct mb_lpddr4x_cfg *bcfg) +{ + memcpy(bcfg->dq_map, baseboard_memcfg.dq_map, + sizeof(baseboard_memcfg.dq_map)); + + memcpy(bcfg->dqs_map, baseboard_memcfg.dqs_map, + sizeof(baseboard_memcfg.dqs_map)); + + bcfg->ect = baseboard_memcfg.ect; + bcfg->half_populated = baseboard_memcfg.half_populated; +} + +int __weak variant_memory_sku(void) +{ + gpio_t spd_gpios[] = { + GPIO_MEM_CONFIG_0, + GPIO_MEM_CONFIG_1, + GPIO_MEM_CONFIG_2, + GPIO_MEM_CONFIG_3, + }; + + return gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)); +}
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 1:
This change is ready for review.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38715
to look at the new patch set (#2).
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
mb/google/volteer: use new tigerlake memory config
Some of the common memory code that was being performed in mainboard has moved into the soc to reduce redundant code. This change adapts volteer to use tigerlake's new common code.
Define GPIO_MEM_CH_SEL to use for determining if all of memory is populated or only half.
- if value read from GPIO_MEM_CH_SEL is 1, only half of DRAM is populated
- if value read is 0, or GPIO_MEM_CH_SEL is not defined, all of DRAM is populated
BUG=b:145642089, b:145238504, b:145564831 BRANCH=none TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer, log into kernel, "cat /proc/meminfo" and verify it reports "MemTotal: 8038196 kB".
Change-Id: I32c9b8a040728d44565806eece6cf60b6b6073b6 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- A src/mainboard/google/volteer/romstage.c M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h A src/mainboard/google/volteer/variants/baseboard/memory.c 3 files changed, 116 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38715/2
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 4:
This change is ready for review.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/romstage.c:
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... PS5, Line 27: memset(&board_cfg, 0, sizeof(board_cfg)); This can also be avoided if you go with the comment on SoC CL.
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... PS5, Line 37: #ifdef GPIO_MEM_CH_SEL I don't think this check is required. This is provided in baseboard. In the future, if there is a variant that doesn't need this, we can add the check.
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... PS5, Line 41: board_cfg.spd.read_type = READ_SPD_CBFS; : board_cfg.spd.spd_spec.spd_index = variant_memory_sku(); This can be changed to:
const struct spd_info spd = { .read_type = READ_SPD_CBFS, .spd_spec.spd_index = variant_memory_sku(); };
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/memory.c:
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... PS5, Line 47: void variant_memory_params(struct mb_lpddr4x_cfg *bcfg) : { : memcpy(bcfg->dq_map, baseboard_memcfg.dq_map, : sizeof(baseboard_memcfg.dq_map)); : : memcpy(bcfg->dqs_map, baseboard_memcfg.dqs_map, : sizeof(baseboard_memcfg.dqs_map)); : : bcfg->ect = baseboard_memcfg.ect; : bcfg->half_populated = baseboard_memcfg.half_populated; : } See my comment on the SoC CL. If you go with that change, then this function can simply be reduced to: struct mb_lpddr4x_cfg *variant_memory_params(void) { return &baseboard_memcfg; }
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 10:
This change is ready for review.
Hello Srinidhi N Kaushik, Wonkyu Kim, caveh jalali, build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38715
to look at the new patch set (#11).
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
mb/google/volteer: use new tigerlake memory config
Some of the common memory code that was being performed in mainboard has moved into the soc to reduce redundant code. This change adapts volteer to use tigerlake's new common code.
Define GPIO_MEM_CH_SEL to use for determining if all of memory is populated or only half.
- if value read from GPIO_MEM_CH_SEL is 1, only half of DRAM is populated
- if value read is 0, or GPIO_MEM_CH_SEL is not defined, all of DRAM is populated
BUG=b:145642089, b:145238504, b:145564831 BRANCH=none TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer, log into kernel, "cat /proc/meminfo" and verify it reports "MemTotal: 8038196 kB".
Change-Id: I32c9b8a040728d44565806eece6cf60b6b6073b6 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/romstage.c M src/mainboard/google/volteer/variants/baseboard/Makefile.inc M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h A src/mainboard/google/volteer/variants/baseboard/memory.c 5 files changed, 97 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38715/11
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 11:
(4 comments)
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/romstage.c:
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... PS5, Line 27: memset(&board_cfg, 0, sizeof(board_cfg));
This can also be avoided if you go with the comment on SoC CL.
Done
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... PS5, Line 37: #ifdef GPIO_MEM_CH_SEL
I don't think this check is required. This is provided in baseboard. […]
Done
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... PS5, Line 41: board_cfg.spd.read_type = READ_SPD_CBFS; : board_cfg.spd.spd_spec.spd_index = variant_memory_sku();
This can be changed to: […]
Done
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/memory.c:
https://review.coreboot.org/c/coreboot/+/38715/5/src/mainboard/google/voltee... PS5, Line 47: void variant_memory_params(struct mb_lpddr4x_cfg *bcfg) : { : memcpy(bcfg->dq_map, baseboard_memcfg.dq_map, : sizeof(baseboard_memcfg.dq_map)); : : memcpy(bcfg->dqs_map, baseboard_memcfg.dqs_map, : sizeof(baseboard_memcfg.dqs_map)); : : bcfg->ect = baseboard_memcfg.ect; : bcfg->half_populated = baseboard_memcfg.half_populated; : }
See my comment on the SoC CL. […]
Done
Hello Srinidhi N Kaushik, Wonkyu Kim, caveh jalali, Ravishankar Sarawadi, build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38715
to look at the new patch set (#12).
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
mb/google/volteer: use new tigerlake memory config
Some of the common memory code that was being performed in mainboard has moved into the soc to reduce redundant code. This change adapts volteer to use tigerlake's new common code.
BUG=b:145642089, b:145238504, b:145564831 BRANCH=none TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer, log into kernel, "cat /proc/meminfo" and verify it reports "MemTotal: 8038196 kB".
Change-Id: I32c9b8a040728d44565806eece6cf60b6b6073b6 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/romstage.c M src/mainboard/google/volteer/variants/baseboard/Makefile.inc M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h A src/mainboard/google/volteer/variants/baseboard/memory.c 5 files changed, 97 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38715/12
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 13:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38715/13/src/mainboard/google/volte... File src/mainboard/google/volteer/romstage.c:
https://review.coreboot.org/c/coreboot/+/38715/13/src/mainboard/google/volte... PS13, Line 25: lpddr4x_spd_info spd_info?
https://review.coreboot.org/c/coreboot/+/38715/13/src/mainboard/google/volte... File src/mainboard/google/volteer/variants/baseboard/memory.c:
https://review.coreboot.org/c/coreboot/+/38715/13/src/mainboard/google/volte... PS13, Line 13: /* DQ byte map */ nit: redundant comment.
https://review.coreboot.org/c/coreboot/+/38715/13/src/mainboard/google/volte... PS13, Line 45: variant_memory_params __weak so that variant can provide its own config if required.
Hello Srinidhi N Kaushik, Wonkyu Kim, caveh jalali, Ravishankar Sarawadi, build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38715
to look at the new patch set (#14).
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
mb/google/volteer: use new tigerlake memory config
Some of the common memory code that was being performed in mainboard has moved into the soc to reduce redundant code. This change adapts volteer to use tigerlake's new common code.
BUG=b:145642089, b:145238504, b:145564831 BRANCH=none TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer, log into kernel, "cat /proc/meminfo" and verify it reports "MemTotal: 8038196 kB".
Change-Id: I32c9b8a040728d44565806eece6cf60b6b6073b6 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/romstage.c M src/mainboard/google/volteer/variants/baseboard/Makefile.inc M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h A src/mainboard/google/volteer/variants/baseboard/memory.c 5 files changed, 97 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38715/14
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 14:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38715/14/src/mainboard/google/volte... File src/mainboard/google/volteer/romstage.c:
https://review.coreboot.org/c/coreboot/+/38715/14/src/mainboard/google/volte... PS14, Line 9: #include <assert.h> Probably not required anymore?
https://review.coreboot.org/c/coreboot/+/38715/14/src/mainboard/google/volte... PS14, Line 11: #include <cbfs.h> Probably not required anymore?
https://review.coreboot.org/c/coreboot/+/38715/14/src/mainboard/google/volte... PS14, Line 16: #include <string.h> Is this required?
Hello Srinidhi N Kaushik, Wonkyu Kim, caveh jalali, Ravishankar Sarawadi, build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38715
to look at the new patch set (#15).
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
mb/google/volteer: use new tigerlake memory config
Some of the common memory code that was being performed in mainboard has moved into the soc to reduce redundant code. This change adapts volteer to use tigerlake's new common code.
BUG=b:145642089, b:145238504, b:145564831 BRANCH=none TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer, log into kernel, "cat /proc/meminfo" and verify it reports "MemTotal: 8038196 kB".
Change-Id: I32c9b8a040728d44565806eece6cf60b6b6073b6 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/romstage.c M src/mainboard/google/volteer/variants/baseboard/Makefile.inc M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h A src/mainboard/google/volteer/variants/baseboard/memory.c 5 files changed, 96 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38715/15
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 15:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38715/13/src/mainboard/google/volte... File src/mainboard/google/volteer/romstage.c:
https://review.coreboot.org/c/coreboot/+/38715/13/src/mainboard/google/volte... PS13, Line 25: lpddr4x_spd_info
spd_info?
Done
https://review.coreboot.org/c/coreboot/+/38715/13/src/mainboard/google/volte... File src/mainboard/google/volteer/variants/baseboard/memory.c:
https://review.coreboot.org/c/coreboot/+/38715/13/src/mainboard/google/volte... PS13, Line 13: /* DQ byte map */
nit: redundant comment.
Done
https://review.coreboot.org/c/coreboot/+/38715/13/src/mainboard/google/volte... PS13, Line 45: variant_memory_params
__weak so that variant can provide its own config if required.
Done
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 15:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38715/14/src/mainboard/google/volte... File src/mainboard/google/volteer/romstage.c:
https://review.coreboot.org/c/coreboot/+/38715/14/src/mainboard/google/volte... PS14, Line 9: #include <assert.h>
Probably not required anymore?
Done
https://review.coreboot.org/c/coreboot/+/38715/14/src/mainboard/google/volte... PS14, Line 11: #include <cbfs.h>
Probably not required anymore?
Done
https://review.coreboot.org/c/coreboot/+/38715/14/src/mainboard/google/volte... PS14, Line 16: #include <string.h>
Is this required?
Done
Hello Srinidhi N Kaushik, Wonkyu Kim, caveh jalali, Ravishankar Sarawadi, build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38715
to look at the new patch set (#16).
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
mb/google/volteer: use new tigerlake memory config
Some of the common memory code that was being performed in mainboard has moved into the soc to reduce redundant code. This change adapts volteer to use tigerlake's new common code.
BUG=b:145642089, b:145238504, b:145564831 BRANCH=none TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer, log into kernel, "cat /proc/meminfo" and verify it reports "MemTotal: 8038196 kB".
Change-Id: I32c9b8a040728d44565806eece6cf60b6b6073b6 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/romstage.c M src/mainboard/google/volteer/variants/baseboard/Makefile.inc M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h A src/mainboard/google/volteer/variants/baseboard/memory.c 5 files changed, 93 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38715/16
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38715/16/src/mainboard/google/volte... File src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/38715/16/src/mainboard/google/volte... PS16, Line 27: __weak Function declaration doesn't need __weak. It is only required in the baseboard definition.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new tigerlake memory config ......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38715/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38715/16//COMMIT_MSG@11 PS16, Line 11: tigerlake Tiger Lake
https://review.coreboot.org/c/coreboot/+/38715/16//COMMIT_MSG@16 PS16, Line 16: log into kernel What do you mean?
Hello Srinidhi N Kaushik, Wonkyu Kim, caveh jalali, Ravishankar Sarawadi, build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38715
to look at the new patch set (#17).
Change subject: mb/google/volteer: use new Tiger Lake memory config ......................................................................
mb/google/volteer: use new Tiger Lake memory config
Some of the common memory code that was being performed in mainboard has moved into the soc to reduce redundant code. This change adapts volteer to use Tiger Lake's new common code.
BUG=b:145642089, b:145238504, b:145564831 BRANCH=none TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer, login to kernel, "cat /proc/meminfo" and verify it reports "MemTotal: 8038196 kB".
Change-Id: I32c9b8a040728d44565806eece6cf60b6b6073b6 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/romstage.c M src/mainboard/google/volteer/variants/baseboard/Makefile.inc M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h A src/mainboard/google/volteer/variants/baseboard/memory.c 5 files changed, 93 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38715/17
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new Tiger Lake memory config ......................................................................
Patch Set 17:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38715/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38715/16//COMMIT_MSG@11 PS16, Line 11: tigerlake
Tiger Lake
Done
https://review.coreboot.org/c/coreboot/+/38715/16//COMMIT_MSG@16 PS16, Line 16: log into kernel
What do you mean?
Did that help / do you understand what it means now?
https://review.coreboot.org/c/coreboot/+/38715/16/src/mainboard/google/volte... File src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h:
https://review.coreboot.org/c/coreboot/+/38715/16/src/mainboard/google/volte... PS16, Line 27: __weak
Function declaration doesn't need __weak. It is only required in the baseboard definition.
Done
Hello Srinidhi N Kaushik, Wonkyu Kim, caveh jalali, Ravishankar Sarawadi, build bot (Jenkins), Furquan Shaikh, Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38715
to look at the new patch set (#18).
Change subject: mb/google/volteer: use new Tiger Lake memory config ......................................................................
mb/google/volteer: use new Tiger Lake memory config
Some of the common memory code that was being performed in mainboard has moved into the soc to reduce redundant code. This change adapts volteer to use Tiger Lake's new common code.
BUG=b:145642089, b:145238504, b:145564831 BRANCH=none TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer, boot to kernel, "cat /proc/meminfo" and verify it reports "MemTotal: 8038196 kB".
Change-Id: I32c9b8a040728d44565806eece6cf60b6b6073b6 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/romstage.c M src/mainboard/google/volteer/variants/baseboard/Makefile.inc M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h A src/mainboard/google/volteer/variants/baseboard/memory.c 5 files changed, 93 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/38715/18
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new Tiger Lake memory config ......................................................................
Patch Set 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38715/16//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38715/16//COMMIT_MSG@16 PS16, Line 16: log into kernel
Did that help / do you understand what it means now?
I changed log to login, but realized it was correct in the first place, as log is a verb and login a noun. I've changed it completely to avoid any ambiguity.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new Tiger Lake memory config ......................................................................
Patch Set 18: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new Tiger Lake memory config ......................................................................
mb/google/volteer: use new Tiger Lake memory config
Some of the common memory code that was being performed in mainboard has moved into the soc to reduce redundant code. This change adapts volteer to use Tiger Lake's new common code.
BUG=b:145642089, b:145238504, b:145564831 BRANCH=none TEST="emerge-volteer coreboot chromeos-bootimage", flash and boot volteer, boot to kernel, "cat /proc/meminfo" and verify it reports "MemTotal: 8038196 kB".
Change-Id: I32c9b8a040728d44565806eece6cf60b6b6073b6 Signed-off-by: Nick Vaccaro nvaccaro@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38715 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/volteer/Makefile.inc A src/mainboard/google/volteer/romstage.c M src/mainboard/google/volteer/variants/baseboard/Makefile.inc M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h A src/mainboard/google/volteer/variants/baseboard/memory.c 5 files changed, 93 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/volteer/Makefile.inc b/src/mainboard/google/volteer/Makefile.inc index 6b5c065..1b6b880 100644 --- a/src/mainboard/google/volteer/Makefile.inc +++ b/src/mainboard/google/volteer/Makefile.inc @@ -9,6 +9,7 @@ bootblock-y += bootblock.c
romstage-$(CONFIG_CHROMEOS) += chromeos.c +romstage-y += romstage.c
ramstage-$(CONFIG_CHROMEOS) += chromeos.c ramstage-y += ec.c diff --git a/src/mainboard/google/volteer/romstage.c b/src/mainboard/google/volteer/romstage.c new file mode 100644 index 0000000..7e87a2a --- /dev/null +++ b/src/mainboard/google/volteer/romstage.c @@ -0,0 +1,29 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2020 The coreboot project Authors. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include <baseboard/variants.h> +#include <gpio.h> +#include <soc/gpio.h> +#include <soc/meminit_tgl.h> +#include <soc/romstage.h> +#include <variant/gpio.h> + +#include <fsp/soc_binding.h> + +void mainboard_memory_init_params(FSPM_UPD *mupd) +{ + FSP_M_CONFIG *mem_cfg = &mupd->FspmConfig; + const struct mb_lpddr4x_cfg *board_cfg = variant_memory_params(); + const struct spd_info spd_info = { + .read_type = READ_SPD_CBFS, + .spd_spec.spd_index = variant_memory_sku(), + }; + bool half_populated = gpio_get(GPIO_MEM_CH_SEL); + + meminit_lpddr4x_dimm0(mem_cfg, board_cfg, &spd_info, half_populated); +} diff --git a/src/mainboard/google/volteer/variants/baseboard/Makefile.inc b/src/mainboard/google/volteer/variants/baseboard/Makefile.inc index 30f2b46..87a8667 100644 --- a/src/mainboard/google/volteer/variants/baseboard/Makefile.inc +++ b/src/mainboard/google/volteer/variants/baseboard/Makefile.inc @@ -8,6 +8,8 @@
bootblock-y += gpio.c
+romstage-y += memory.c + ramstage-y += gpio.c
smm-y += gpio.c diff --git a/src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h index df4368b..3f8597f 100644 --- a/src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h @@ -10,6 +10,7 @@ #define __BASEBOARD_VARIANTS_H__
#include <soc/gpio.h> +#include <soc/meminit_tgl.h> #include <stddef.h> #include <vendorcode/google/chromeos/chromeos.h>
@@ -23,6 +24,7 @@
const struct cros_gpio *variant_cros_gpios(size_t *num);
+const struct mb_lpddr4x_cfg *variant_memory_params(void); int variant_memory_sku(void);
#endif /* __BASEBOARD_VARIANTS_H__ */ diff --git a/src/mainboard/google/volteer/variants/baseboard/memory.c b/src/mainboard/google/volteer/variants/baseboard/memory.c new file mode 100644 index 0000000..1117198 --- /dev/null +++ b/src/mainboard/google/volteer/variants/baseboard/memory.c @@ -0,0 +1,59 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2020 The coreboot project Authors. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include <baseboard/gpio.h> +#include <baseboard/variants.h> +#include <gpio.h> + +static const struct mb_lpddr4x_cfg baseboard_memcfg = { + /* DQ byte map */ + .dq_map = { + { 0, 1, 2, 3, 4, 5, 6, 7, /* Byte 0 */ + 12, 13, 14, 15, 11, 10, 9, 8 }, /* Byte 1 */ + { 7, 2, 6, 3, 5, 1, 4, 0, /* Byte 2 */ + 10, 8, 9, 11, 15, 12, 14, 13 }, /* Byte 3 */ + { 3, 2, 1, 0, 4, 5, 6, 7, /* Byte 4 */ + 12, 13, 14, 15, 11, 10, 9, 8 }, /* Byte 5 */ + { 7, 0, 1, 6, 5, 4, 2, 3, /* Byte 6 */ + 15, 14, 8, 9, 10, 12, 11, 13 }, /* Byte 7 */ + { 3, 2, 1, 0, 4, 5, 6, 7, /* Byte 0 */ + 12, 13, 14, 15, 11, 10, 9, 8 }, /* Byte 1 */ + { 3, 4, 2, 5, 0, 6, 1, 7, /* Byte 2 */ + 13, 12, 11, 10, 14, 15, 9, 8 }, /* Byte 3 */ + { 3, 2, 1, 0, 7, 4, 5, 6, /* Byte 4 */ + 15, 14, 13, 12, 8, 9, 10, 11 }, /* Byte 5 */ + { 3, 4, 2, 5, 1, 0, 7, 6, /* Byte 6 */ + 15, 14, 9, 8, 12, 10, 11, 13 } /* Byte 7 */ + }, + + /* DQS CPU<>DRAM map */ + .dqs_map = { + /* Ch 0 1 2 3 */ + { 0, 1 }, { 0, 1 }, { 0, 1 }, { 0, 1 }, + { 0, 1 }, { 0, 1 }, { 0, 1 }, { 0, 1 } + }, + + .ect = 0, /* Disable Early Command Training */ +}; + +const struct mb_lpddr4x_cfg *__weak variant_memory_params(void) +{ + return &baseboard_memcfg; +} + +int __weak variant_memory_sku(void) +{ + gpio_t spd_gpios[] = { + GPIO_MEM_CONFIG_0, + GPIO_MEM_CONFIG_1, + GPIO_MEM_CONFIG_2, + GPIO_MEM_CONFIG_3, + }; + + return gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)); +}
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38715 )
Change subject: mb/google/volteer: use new Tiger Lake memory config ......................................................................
Patch Set 19:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/541 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/540 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/539
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Furquan Shaikh (Code Review)
-
Nick Vaccaro (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)