Change in ...coreboot[master]: soc/intel/cannonlake: Modify dq_map to provide for 6 entries
Paul Fagerburg has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32475
Change subject: soc/intel/cannonlake: Modify dq_map to provide for 6 entries ......................................................................
soc/intel/cannonlake: Modify dq_map to provide for 6 entries
Intel's DQ_DQS_RComp_Info_Utility generates data for 6 entries. MRC will return errors if we don't have all 6 entries in the map.
BRANCH=none BUG=b:131103736 TEST=ensure the firmware builds without error; I don't have hardware available to test this just yet.
Change-Id: I20a768de0e4440d7dde7b717794c4e2d0c62819c Signed-off-by: Paul Fagerburg pfagerburg@chromium.org --- M src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h 1 file changed, 8 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/32475/1
diff --git a/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h b/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h index c602180..87cb6fb 100644 --- a/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h +++ b/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h @@ -52,17 +52,19 @@ /* Board-specific memory dq mapping information */ struct cnl_mb_cfg { /* - * For each channel, there are 3 sets of DQ byte mappings, + * For each channel, there are 6 sets of DQ byte mappings, * where each set has a package 0 and a package 1 value (package 0 * represents the first 64-bit lpddr4 chip combination, and package 1 * represents the second 64-bit lpddr4 chip combination). * The first three sets are for CLK, CMD, and CTL. - * The fsp package actually expects 6 sets, but the last 3 sets are - * not used in CNL, so we only define the three sets that are used - * and let the meminit_lpddr4() routine take care of clearing the + * The fsp package actually expects 6 sets, even though the last 3 sets + * are not used in CNL. + * We let the meminit_lpddr4() routine take care of clearing the * unused fields for the caller. + * Note that dq_map is only used by LPDDR; it does not need to be + * initialized for designs using DDR4. */ - uint8_t dq_map[DDR_NUM_CHANNELS][3][DDR_NUM_PACKAGES]; + uint8_t dq_map[DDR_NUM_CHANNELS][6][DDR_NUM_PACKAGES];
/* * DQS CPU<>DRAM map Ch0 and Ch1. Each array entry represents a @@ -70,6 +72,7 @@ * the memory part. The array index represents the dqs bit number * on the memory part, and the values in the array represent which * pin on the CPU that DRAM pin connects to. + * dqs_map is only used by LPDDR; same comments apply as for dq_map above. */ uint8_t dqs_map[DDR_NUM_CHANNELS][DQ_BITS_PER_DQS];
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32475 )
Change subject: soc/intel/cannonlake: Modify dq_map to provide for 6 entries ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32475/1/src/soc/intel/cannonlake/include/soc... File src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h:
https://review.coreboot.org/#/c/32475/1/src/soc/intel/cannonlake/include/soc... PS1, Line 75: * dqs_map is only used by LPDDR; same comments apply as for dq_map above. line over 80 characters
Hello Patrick Rudolph, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32475
to look at the new patch set (#2).
Change subject: soc/intel/cannonlake: Modify dq_map to provide for 6 entries ......................................................................
soc/intel/cannonlake: Modify dq_map to provide for 6 entries
Intel's DQ_DQS_RComp_Info_Utility generates data for 6 entries. MRC will return errors if we don't have all 6 entries in the map.
BRANCH=none BUG=b:131103736 TEST=ensure the firmware builds without error; I don't have hardware available to test this just yet.
Change-Id: I20a768de0e4440d7dde7b717794c4e2d0c62819c Signed-off-by: Paul Fagerburg pfagerburg@chromium.org --- M src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h 1 file changed, 9 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/32475/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32475 )
Change subject: soc/intel/cannonlake: Modify dq_map to provide for 6 entries ......................................................................
Patch Set 2: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32475 )
Change subject: soc/intel/cannonlake: Modify dq_map to provide for 6 entries ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32475/2/src/soc/intel/cannonlake/include/soc... File src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h:
https://review.coreboot.org/#/c/32475/2/src/soc/intel/cannonlake/include/soc... PS2, Line 76: * above. It would be good to remove the dqs_map setting in hatch since it is unused anyways.
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32475 )
Change subject: soc/intel/cannonlake: Modify dq_map to provide for 6 entries ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/32475/2/src/soc/intel/cannonlake/include/soc... File src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h:
https://review.coreboot.org/#/c/32475/2/src/soc/intel/cannonlake/include/soc... PS2, Line 76: * above.
It would be good to remove the dqs_map setting in hatch since it is unused anyways.
I uploaded a new CL for hatch baseboard and added you as reviewer.
Kane Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32475 )
Change subject: soc/intel/cannonlake: Modify dq_map to provide for 6 entries ......................................................................
Patch Set 2:
verified ok on kohaku
Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32475 )
Change subject: soc/intel/cannonlake: Modify dq_map to provide for 6 entries ......................................................................
soc/intel/cannonlake: Modify dq_map to provide for 6 entries
Intel's DQ_DQS_RComp_Info_Utility generates data for 6 entries. MRC will return errors if we don't have all 6 entries in the map.
BRANCH=none BUG=b:131103736 TEST=ensure the firmware builds without error; I don't have hardware available to test this just yet.
Change-Id: I20a768de0e4440d7dde7b717794c4e2d0c62819c Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32475 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h 1 file changed, 9 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h b/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h index c602180..e602a33 100644 --- a/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h +++ b/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h @@ -52,17 +52,19 @@ /* Board-specific memory dq mapping information */ struct cnl_mb_cfg { /* - * For each channel, there are 3 sets of DQ byte mappings, + * For each channel, there are 6 sets of DQ byte mappings, * where each set has a package 0 and a package 1 value (package 0 * represents the first 64-bit lpddr4 chip combination, and package 1 * represents the second 64-bit lpddr4 chip combination). * The first three sets are for CLK, CMD, and CTL. - * The fsp package actually expects 6 sets, but the last 3 sets are - * not used in CNL, so we only define the three sets that are used - * and let the meminit_lpddr4() routine take care of clearing the + * The fsp package actually expects 6 sets, even though the last 3 sets + * are not used in CNL. + * We let the meminit_lpddr4() routine take care of clearing the * unused fields for the caller. + * Note that dq_map is only used by LPDDR; it does not need to be + * initialized for designs using DDR4. */ - uint8_t dq_map[DDR_NUM_CHANNELS][3][DDR_NUM_PACKAGES]; + uint8_t dq_map[DDR_NUM_CHANNELS][6][DDR_NUM_PACKAGES];
/* * DQS CPU<>DRAM map Ch0 and Ch1. Each array entry represents a @@ -70,6 +72,8 @@ * the memory part. The array index represents the dqs bit number * on the memory part, and the values in the array represent which * pin on the CPU that DRAM pin connects to. + * dqs_map is only used by LPDDR; same comments apply as for dq_map + * above. */ uint8_t dqs_map[DDR_NUM_CHANNELS][DQ_BITS_PER_DQS];
-
build bot (Jenkins) (Code Review) -
Furquan Shaikh (Code Review)
-
Kane Chen (Code Review)
-
Paul Fagerburg (Code Review)