Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default
The FSP_CAR option has additional configuration options whose default values result in boot failures. Since default values shall always boot, default to the open-source CAR NEM Enhanced implementation instead. This also allows us to get rid of an unnecessary vendor-specific special case.
Change-Id: I30b1808f91701c07dce6f1de08c213150e8a675a Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig 2 files changed, 2 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/34287/1
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 4235b7a..f859cd5 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -269,8 +269,7 @@
choice prompt "Cache-as-ram implementation" - default USE_CANNONLAKE_CAR_NEM_ENHANCED if MAINBOARD_HAS_CHROMEOS - default USE_CANNONLAKE_FSP_CAR + default USE_CANNONLAKE_CAR_NEM_ENHANCED help This option allows you to select how cache-as-ram (CAR) is set up.
diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index a2261a0..5dca44b 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -172,8 +172,7 @@
choice prompt "Cache-as-ram implementation" - default USE_ICELAKE_CAR_NEM_ENHANCED if MAINBOARD_HAS_CHROMEOS - default USE_ICELAKE_FSP_CAR + default USE_ICELAKE_CAR_NEM_ENHANCED help This option allows you to select how cache-as-ram (CAR) is set up.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/1/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/1/src/soc/intel/cannonlake/Kc... PS1, Line 273: If the default is being changed here, all the mainboards that are affected (and were relying on this default) should be updated prior to this CL to ensure we do not intentionally break any boards.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/1/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/1/src/soc/intel/cannonlake/Kc... PS1, Line 273:
If the default is being changed here, all the mainboards that are affected (and were relying on this […]
This is not about hardware compatibility as far as I can see.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/1/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/1/src/soc/intel/cannonlake/Kc... PS1, Line 273:
This is not about hardware compatibility as far as I can see.
@Furquan, I just checked and the only CNL users are Intel and Google boards, and all are selecting MAINBOARD_HAS_CHROMEOS, so this change should not affect them.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 1: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 1:
Patch Set 1: Code-Review+1
Subrata, thanks for your review :)
It would nice to boot-test this patch, just in case. Is there anybody who could do it? Thanks in advance.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/1/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/1/src/soc/intel/cannonlake/Kc... PS1, Line 273:
@Furquan, I just checked and the only CNL users are Intel and Google boards, and all are selecting MAINBOARD_HAS_CHROMEOS, so this change should not affect them.
I'm sure there are plenty of non-Intel/Google CNL boards in development / not yet upstreamed / being worked on in private trees, so that needs to be taken into account as well
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34287/1//COMMIT_MSG@10 PS1, Line 10: shall /s/shall/should ?
I'm not sure either is true though, since many boards are not bootable without blobs not supplied by coreboot (and the default is not to include those blobs). But I get what you're going for
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34287/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34287/1//COMMIT_MSG@10 PS1, Line 10: shall
/s/shall/should ? […]
Ideally, one should only need to select their mainboard vendor and model (and a payload) to get a coreboot.rom that is able to boot. I am not sure if it's an actual requirement, but it makes lots of sense.
I thought FSP was included from 3rdparty/fsp by default, though?
https://review.coreboot.org/c/coreboot/+/34287/1/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/1/src/soc/intel/cannonlake/Kc... PS1, Line 273:
@Furquan, I just checked and the only CNL users are Intel and Google boards, and all are selecting […]
This change is actually beneficial for these CNL boards.
It is changing the default option to what all the CNL boards in the (upstream) tree use, which should be tested and working :)
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34287/1//COMMIT_MSG@10 PS1, Line 10: shall
Ideally, one should only need to select their mainboard vendor and model (and a payload) to get a co […]
It's a good goal to be able to boot the default, but there are plenty of instances when that's not the case. I don't know that we include ME binaries anywhere, for example.
There are also a number of older (5 year old) boards that used the FSP from before there was an FSP repo. I have no idea if anyone ever went back and changed the defaults to pull in the FSPs.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34287/1//COMMIT_MSG@10 PS1, Line 10: shall
It's a good goal to be able to boot the default, but there are plenty of instances when that's not t […]
Most people don't need to add ME binaries to build a coreboot.rom that boots, as they are usually on the flash chip already. But considering the situation for older FSP, as well as mrc and refcode, I think "should" fits better.
Ack
Hello Patrick Rudolph, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34287
to look at the new patch set (#2).
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default
The FSP_CAR option has additional configuration options whose default values result in boot failures. Since default values should always boot, default to the open-source CAR NEM Enhanced implementation instead. This also allows us to get rid of an unnecessary vendor-specific special case.
Change-Id: I30b1808f91701c07dce6f1de08c213150e8a675a Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig 2 files changed, 2 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/34287/2
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2: Code-Review-1
(1 comment)
Did somebody check if the FSP-T option actually works?
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... PS2, Line 272: default USE_CANNONLAKE_CAR_NEM_ENHANCED Unless we make it a policy to always set a default, I would prefer not to clutter the tree with unnecessary lines.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... PS2, Line 272: default USE_CANNONLAKE_CAR_NEM_ENHANCED
Unless we make it a policy to always set a default, I would prefer […]
I would recommend bringing the topic up on the mailing list. Once we have a policy w.r.t. explicit defaults on choices, I have no problem with writing a patch uniformizing the defaults on Kconfig choices.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... PS2, Line 272: default USE_CANNONLAKE_CAR_NEM_ENHANCED
I would recommend bringing the topic up on the mailing list. Once we have a policy w.r.t. […]
'always set a default' -- I'm confused. Why wouldn't we want a default?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... PS2, Line 272: default USE_CANNONLAKE_CAR_NEM_ENHANCED
'always set a default' -- I'm confused. […]
The default in a choice is to pick the first entry, so in this case, there's a default, just no statement for it. As far as I understand Nico he prefers to not use statements that merely restate default behavior.
What to standardize on is better discussed on the list than here though.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
Patch Set 1:
Patch Set 1: Code-Review+1
Subrata, thanks for your review :)
It would nice to boot-test this patch, just in case. Is there anybody who could do it? Thanks in advance.
it looks good bt u can wait for me till monday for tested result
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2: -Code-Review
(1 comment)
I've just boot-tested all affected boards (empty set, assuming abuilds configurations). So I don't think we should put more effort into this.
However, on the FSP-T integration side, I've learned by now that my initial hunch why my board isn't booting must have been wrong: I thought it was because I left CPU_MICROCODE_CBFS_LEN/_LOC at 0. But the FSP integration guide for CFL (what I tested) says this would be ok.
So it would be really nice if somebody could confirm if the FSP-T integration works/ever worked upstream.
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... PS2, Line 272: default USE_CANNONLAKE_CAR_NEM_ENHANCED
The default in a choice is to pick the first entry, so in this case, there's a default, just no stat […]
I just made some rough statistics of current occurences:
$ # overall choices: $ git grep "^choice$" | wc -l 125
$ # choices with conditional default: $ git grep -A2 "^choice$" | grep $'\t''default [^[:space:]]+[[:space:]]' | wc -l 13
$ # choices with unconditional, explicit default: $ git grep -A2 "^choice$" | grep $'\t''default [^[:space:]]+$' | wc -l 58
So I'm surprised to see that stating the obvious is already the prevailing pattern: 58:54 (57:54 excluding this patch :-P). If somebody wants to bikeshed my statistics: just remove the `| wc -l` at the end and you'll see that there are some clusters, e.g. if you'd remove payloads from the statistics the result would look much different ;)
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
Patch Set 2: -Code-Review
(1 comment)
I've just boot-tested all affected boards (empty set, assuming abuilds configurations). So I don't think we should put more effort into this.
However, on the FSP-T integration side, I've learned by now that my initial hunch why my board isn't booting must have been wrong: I thought it was because I left CPU_MICROCODE_CBFS_LEN/_LOC at 0. But the FSP integration guide for CFL (what I tested) says this would be ok.
So it would be really nice if somebody could confirm if the FSP-T integration works/ever worked upstream.
HI Maurice/BT/KW/Lean Sheng, can you please share your opinion on FSP-T integration work in upstream ?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2: -Code-Review
(1 comment)
I've just boot-tested all affected boards (empty set, assuming abuilds configurations). So I don't think we should put more effort into this.
However, on the FSP-T integration side, I've learned by now that my initial hunch why my board isn't booting must have been wrong: I thought it was because I left CPU_MICROCODE_CBFS_LEN/_LOC at 0. But the FSP integration guide for CFL (what I tested) says this would be ok.
So it would be really nice if somebody could confirm if the FSP-T integration works/ever worked upstream.
HI Maurice/BT/KW/Lean Sheng, can you please share your opinion on FSP-T integration work in upstream ?
i remember to implement chrome-coreboot was using FSP_T till SKL platform and i have validated that flow. so code should be in upsteam for sure
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2: -Code-Review
(1 comment)
I've just boot-tested all affected boards (empty set, assuming abuilds configurations). So I don't think we should put more effort into this.
However, on the FSP-T integration side, I've learned by now that my initial hunch why my board isn't booting must have been wrong: I thought it was because I left CPU_MICROCODE_CBFS_LEN/_LOC at 0. But the FSP integration guide for CFL (what I tested) says this would be ok.
So it would be really nice if somebody could confirm if the FSP-T integration works/ever worked upstream.
HI Maurice/BT/KW/Lean Sheng, can you please share your opinion on FSP-T integration work in upstream ?
i remember to implement chrome-coreboot was using FSP_T till SKL platform and i have validated that flow. so code should be in upsteam for sure
I mean the integration into soc/intel/cannonlake/ in particular. Also, AFAIR, cros firmware wasn't using FSP 2 for SKL. FSP 1.1 was similar, but uses different code paths, AFAICT.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/34287/2/src/soc/intel/cannonlake/Kc... PS2, Line 272: default USE_CANNONLAKE_CAR_NEM_ENHANCED
I just made some rough statistics of current occurences: […]
Personally, I think it's a good practice to always set a default. If an additional option gets added at the top of the choice, that changes the default value unless there's a specific default set.
Also, since there's no way to select a value inside a choice for a board, the only method is with defaults. https://doc.coreboot.org/getting_started/kconfig.html#choice
For example, if a board wanted to set USE_CANNONLAKE_FSP_CAR, the only way to do it is in a saved .config file.
If we set: default USE_CANNONLAKE_FSP_CAR if DEFAULT_FSP_CAR, that allows the choice to be set correctly somewhere else.
If we do decide to force a default for every choice, we can easily enforce that with the kconfig lint tool.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
Patch Set 2: Code-Review+2
Thank you! :)
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34287/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34287/2//COMMIT_MSG@13 PS2, Line 13: In #coreboot@irc.freenode.net it was mentioned, that FSP-T is needed for Secure Boot, and that all affected boards in the repository use `MAINBOARD_HAS_CHROMEOS`, so this change actually has no effect. If this is correct, could you please extend the commit message correspondingly?
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
Patch Set 2:
(1 comment)
Hi Sachin/ Amol, could you provide some info on FSP-T w.r.t Bootguard? There is question where FSP-T support is still necessary in open source, it would be great to provide some insight on how it is important for us to have this. Thanks
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2: Code-Review+1
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
Patch Set 2: Code-Review-1
(1 comment)
Did somebody check if the FSP-T option actually works?
Hi Nico, FSP-T is working and verified on CFL & WHL Intel RVP platform.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
Did somebody check if the FSP-T option actually works?
Hi Nico, FSP-T is working and verified on CFL & WHL Intel RVP platform.
Thanks, Lean. Can you share your defconfig, please? And did you use the current "Coffee Lake FSP 7.0.64.40" as available on Github?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 2:
Lean, please consider providing some documentation on the FSP-T vs Open Source CAR decision in Documentation/, the trade-offs (what do you gain by using FSP-T), what binaries to use, how to configure it, ...
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default
The FSP_CAR option has additional configuration options whose default values result in boot failures. Since default values should always boot, default to the open-source CAR NEM Enhanced implementation instead. This also allows us to get rid of an unnecessary vendor-specific special case.
Change-Id: I30b1808f91701c07dce6f1de08c213150e8a675a Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34287 Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Subrata Banik subrata.banik@intel.com Reviewed-by: Lean Sheng Tan lean.sheng.tan@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/Kconfig M src/soc/intel/icelake/Kconfig 2 files changed, 2 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Subrata Banik: Looks good to me, approved Lean Sheng Tan: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 4235b7a..f859cd5 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -269,8 +269,7 @@
choice prompt "Cache-as-ram implementation" - default USE_CANNONLAKE_CAR_NEM_ENHANCED if MAINBOARD_HAS_CHROMEOS - default USE_CANNONLAKE_FSP_CAR + default USE_CANNONLAKE_CAR_NEM_ENHANCED help This option allows you to select how cache-as-ram (CAR) is set up.
diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index a2261a0..5dca44b 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -172,8 +172,7 @@
choice prompt "Cache-as-ram implementation" - default USE_ICELAKE_CAR_NEM_ENHANCED if MAINBOARD_HAS_CHROMEOS - default USE_ICELAKE_FSP_CAR + default USE_ICELAKE_CAR_NEM_ENHANCED help This option allows you to select how cache-as-ram (CAR) is set up.
Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34287 )
Change subject: soc/intel/{cnl,icl}: Always use CAR NEM enhanced by default ......................................................................
Patch Set 3:
Patch Set 2:
Did somebody check if the FSP-T option actually works?
Hi Nico, FSP-T is working and verified on CFL & WHL Intel RVP platform.
Thanks, Lean. Can you share your defconfig, please? And did you use the current "Coffee Lake FSP 7.0.64.40" as available on Github?
Hi Nico, Yes,I am using the FSP available on Github :) For config I am using this: CONFIG_FSP_USE_REPO=y CONFIG_USE_CANNONLAKE_FSP_CAR=y CONFIG_CPU_MICROCODE_CBFS_LEN=0x27000 CONFIG_CPU_MICROCODE_CBFS_LOC=0xffe20000
-
Aaron Durbin (Code Review) -
Angel Pons (Code Review)
-
Furquan Shaikh (Code Review)
-
Lean Sheng Tan (Code Review)
-
Martin Roth (Code Review)
-
Matt DeVillier (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)