Richard Spiegel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Documentation/soc/amd: Add family 15h
Create documentation for AMD family 15h, and in particular to models for which there's coreboot code: Models 60h-6Fh and 70h-7Fh.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 45 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/1
diff --git a/Documentation/soc/amd/family15h.md b/Documentation/soc/amd/family15h.md new file mode 100644 index 0000000..2e583e8 --- /dev/null +++ b/Documentation/soc/amd/family15h.md @@ -0,0 +1,44 @@ +# AMD Family 15h in coreboot + +## Abstract + +Family 15h uses the AMD CPU micro architecture _Excavator_, and is available +in several models with different number of cores and some other small +variations, to attend a broad spectrum of users. Of particular interest for +coreboot are models **60h-6Fh** (_Merlin Falcon_) and **70h-7Fh** (_Stoney Ridge_), +for which there are coreboot implementations. + +## Introduction + +Family 15h products are x86-based designs. This documentation assumes +familiarity with x86, its reset state and its early initialization requirements. + +AMD has historically required an NDA for access to the PSP specification. +coreboot relies on util/amdfwtool to build the structures and add various +other firmware to the final image. + +Support in coreboot for modern AMD products is based on AMD’s +reference code: AMD Generic Encapsulated Software Architecture +(AGESA _**TM**_). AGESA contains the technology for enabling DRAM, +configuring proprietary core logic, assistance with generating ACPI +tables, and other features. + +Some functionality, such as GPIO setting and D0/D3 of some devices such as +I2C and UART were removed from AGESA (though still available on most AGESA +images) and converted to coreboot code for granularity, speed and easy of +use reasons. +>In particular, GPIO achieved a greater control over what is being +>programmed through the use of a table that is easily created using +>macros. + +## Additional Definitions + +* PSP, Platform Security Processor: Onboard ARM processor that runs +alongside the main x86 processor; may be viewed as analogous to the +Intel<sup>R</sup> Management Engine +* FCH, Fusion Control Hub, the logical southbridge within the SOC + +## References + +1. [Merlin Falcon BKDG](https://www.amd.com/system/files/TechDocs/50742_15h_Models_60h-6Fh_BKDG.pdf) +2. [Stoney Ridge BKDG](https://www.amd.com/system/files/TechDocs/55072_AMD_Family_15h_Models_70h-7F...) diff --git a/Documentation/soc/amd/index.md b/Documentation/soc/amd/index.md index 7945b48..d6f31c8 100644 --- a/Documentation/soc/amd/index.md +++ b/Documentation/soc/amd/index.md @@ -4,5 +4,6 @@
## Technology
+- [Family 15h](family15h.md) - [Family 17h](family17h.md)
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 1:
Can someone please start a review?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 7: Of particular interest for : coreboot are models **60h-6Fh** (_Merlin Falcon_) and **70h-7Fh** (_Stoney Ridge_), : for which there are coreboot implementations. this is a bit of tautology. How about "coreboot implements support for models ..."?
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 22: _**TM**_ ™ would be the unicode character for that.
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 30: >In particular, GPIO achieved a greater control over what is being : >programmed through the use of a table that is easily created using : >macros. : who uses a table that is easily created using macros: our approach or AGESA's?
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 38: <sup>R</sup> ®
Hello Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#2).
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Documentation/soc/amd: Add family 15h
Create documentation for AMD family 15h, and in particular to models for which there's coreboot code: Models 60h-6Fh and 70h-7Fh.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 47 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/2
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 7: Of particular interest for : coreboot are models **60h-6Fh** (_Merlin Falcon_) and **70h-7Fh** (_Stoney Ridge_), : for which there are coreboot implementations.
this is a bit of tautology. How about "coreboot implements support for models ... […]
Will do.
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 22: _**TM**_
™ would be the unicode character for that.
Thanks. How do you get it? What combination of keys to make it appear?
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 30: >In particular, GPIO achieved a greater control over what is being : >programmed through the use of a table that is easily created using : >macros. :
who uses a table that is easily created using macros: our approach or AGESA's?
Our approach... AGESA's was horrible.
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 38: <sup>R</sup>
®
Thanks
Hello Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#3).
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Documentation/soc/amd: Add family 15h
Create documentation for AMD family 15h, and in particular to models for which there's coreboot code: Models 60h-6Fh and 70h-7Fh.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/3
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 7: Of particular interest for : coreboot are models **60h-6Fh** (_Merlin Falcon_) and **70h-7Fh** (_Stoney Ridge_), : for which there are coreboot implementations.
Will do.
Done
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 22: _**TM**_
Thanks. […]
depends on your OS and keyboard layout. The universal approach is searching the web for "tm unicode" which will give you something to copy&paste ;-)
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 30: >In particular, GPIO achieved a greater control over what is being : >programmed through the use of a table that is easily created using : >macros. :
Our approach... AGESA's was horrible.
Done
https://review.coreboot.org/c/coreboot/+/34561/1/Documentation/soc/amd/famil... PS1, Line 38: <sup>R</sup>
Thanks
Done
https://review.coreboot.org/c/coreboot/+/34561/3/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/3/Documentation/soc/amd/famil... PS3, Line 31: coreboot coreboot's
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34561/3/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/3/Documentation/soc/amd/famil... PS3, Line 31: coreboot
coreboot's
Done
Hello Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#4).
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Documentation/soc/amd: Add family 15h
Create documentation for AMD family 15h, and in particular to models for which there's coreboot code: Models 60h-6Fh and 70h-7Fh.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 46 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/4
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 4:
(12 comments)
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 1: Family 15h in Family 15h [SOCs|Processors]
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 5: Family 15h Same as above.
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 6: number numbers
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 8: models **60h-6Fh** (_Merlin Falcon_) and : **70h-7Fh** (_Stoney Ridge_). We also support 15h trinity - the agesa code is in vendorcode/amd/ Bald Eagle also looks like it's Family 15h. That uses a binary pi 00630f01 And I guess we also support carrizo to with the binary pi 00660F01, but that's roughly the same as merlin falcon.
Trinity uses the piledriver core, and bald eagle uses the steamroller core. https://en.wikipedia.org/wiki/Piledriver_(microarchitecture) https://en.wikipedia.org/wiki/Steamroller_(microarchitecture)
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 26: : Some functionalit In the binary PI release used by coreboot, some functionality...
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 27: GPIO setting and the setting of MTRRS and GPIO configurations along with setting
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 28: (though still available on most _AGESA_ Get rid of this, because who cares?
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 28: _AGESA_ wrap at 72 characters?
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 30: reasons remove "reasons"
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 31: > Isn't there a better way to format this?
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 38: may be viewed as analogous to the : Intel® Management Engine Maybe just discuss what it does instead of comparing it to the ME.
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 44: https://www.amd.com/system/files/TechDocs/50742_15h_Models_60h-6Fh_BKDG.pdf We're discussing whether we can mirror the documents on a coreboot server, but until then, maybe point to archive.org?
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 4:
(6 comments)
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 5: _Excavator_ This is a misleading statement. Family 15h contains way more microarchitectures than just Excavator.
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 8: models **60h-6Fh** (_Merlin Falcon_) and : **70h-7Fh** (_Stoney Ridge_). Why bold?
I concur with Martin. Since you're making this a Family 15h document, please don't make it specific to only Models 60h-6Fh and 70h-7Fh. Definitely don't word it to exclude the others. BTW coreboot also contains Family 15h 00h-0Fh support in native source. You can download a 2 year old slide deck for AMD at https://www.coreboot.org/Denver2017.
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 13: This documentation assumes : familiarity with x86, its reset state and its early initialization : requirements. I'm not sure the Family 17h warning about x86 reset adds anything to Family 15h. Family 17h is different than everything else, and the reset process was described in that other file.
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 17: AMD® Don't use (R) after the first instance in any document. For AMD, don't use it at all.
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 23: (_AGESA ™_). _AGESA_ c Why are these italicized?
By the way, this use of TM on the first AGESA but not the second is correct.
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 44: https://www.amd.com/system/files/TechDocs/50742_15h_Models_60h-6Fh_BKDG.pdf
We're discussing whether we can mirror the documents on a coreboot server, but until then, maybe poi […]
Are you worried about the URL changing? While potentially a risk, it looks like they've named it without a revision so they can keep some stability even when updating the docs. To me it seems more useful to point to the latest for as long as it exists.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 4:
(17 comments)
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 1: Family 15h in
Family 15h [SOCs|Processors]
Done
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 5: Family 15h
Same as above.
Done
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 5: _Excavator_
This is a misleading statement. […]
Ack
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 6: number
numbers
Done
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 8: models **60h-6Fh** (_Merlin Falcon_) and : **70h-7Fh** (_Stoney Ridge_).
Why bold? […]
Ack
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 8: models **60h-6Fh** (_Merlin Falcon_) and : **70h-7Fh** (_Stoney Ridge_).
We also support 15h trinity - the agesa code is in vendorcode/amd/ […]
Ack
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 13: This documentation assumes : familiarity with x86, its reset state and its early initialization : requirements.
I'm not sure the Family 17h warning about x86 reset adds anything to Family 15h. […]
Removed
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 17: AMD®
Don't use (R) after the first instance in any document. For AMD, don't use it at all.
Done
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 23: (_AGESA ™_). _AGESA_ c
Why are these italicized? […]
I wanted to call attention to AGESA, that's why it's italicized.
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 26: : Some functionalit
In the binary PI release used by coreboot, some functionality...
Done
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 27: GPIO setting and
the setting of MTRRS and GPIO configurations along with setting
Done
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 28: (though still available on most _AGESA_
Get rid of this, because who cares?
Done
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 28: _AGESA_
wrap at 72 characters?
Mostly done, though there are 3 or 4 instances of 73 characters.
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 30: reasons
remove "reasons"
Done
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 31: >
Isn't there a better way to format this?
Spelling or the use of a box comment style (>)?
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 38: may be viewed as analogous to the : Intel® Management Engine
Maybe just discuss what it does instead of comparing it to the ME.
I wanted to call attention to the fact that it's a separate processor within the SOC just like Intel's ME. As for discussing its functionality, I would need to read documentation before doing it, so it'll not be in the next patch. Future patch yes, I can do.
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 44: https://www.amd.com/system/files/TechDocs/50742_15h_Models_60h-6Fh_BKDG.pdf
Are you worried about the URL changing? While potentially a risk, it looks like they've named it wi […]
Ack... I'll side with Marshall for the time being.
Hello Paul Menzel, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#5).
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Documentation/soc/amd: Add family 15h
Create documentation for AMD family 15h, and in particular to models for which there's coreboot code: Models 60h-6Fh and 70h-7Fh.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 49 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/5
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 5:
(13 comments)
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG@7 PS5, Line 7: f cap F. Also one below.
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG@9 PS5, Line 9: Create documentation for AMD family 15h, and in particular to models for Please make this statement jive with cpu/amd/family_10-family_16h, family15th (and Richland) and pi/00630F01 (a.k.a. Bald Eagle and Kaveri)
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 5: Family 15h is available in several models with different numbers of cores : and some other small variations, This sounds like a marketing one-page. Is there any product that does not come in different numbers of cores and other variations?
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 6: attend a broad spectrum check your wording
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 7: micro architecture one word
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 9: use separate maybe "use a separate"
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 12: , which all [Family 17h](family17h.md) : will also use. I would leave Family 17h and future tense "will use" out of this document.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 19: PSP It feels really out of place to read about the PSP here! Why is it needed for an introduction to Family 15h?
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 29: the binary PI release The reader doesn't yet know what a binaryPI release is.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 29: coreboo sp
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 34: >In particular, Why blockquote here? Also, I'm not sure that discussing coreboot's GPIO library adds much to the Family 15h discussion, so why is this here at all?
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 43: Control Controller
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 45: References Models 00h-0Fh Models 10h-1Fh Models 30h-3Fh
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 5:
(13 comments)
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG@7 PS5, Line 7: f
cap F. Also one below.
Will do.
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG@9 PS5, Line 9: Create documentation for AMD family 15h, and in particular to models for
Please make this statement jive with cpu/amd/family_10-family_16h, family15th (and Richland) and pi/ […]
jive? I don't understand. Sorry, English is not my first language.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 6: attend a broad spectrum
check your wording
I use spell checking... but I know it does not detects everything. Can you explain (English is not my first language)?
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 5: Family 15h is available in several models with different numbers of cores : and some other small variations,
This sounds like a marketing one-page. […]
True...but I have to start my document somewhere, and don't have much information to add here. What was new on family 15h (not available on family 14h)... or was it just a change in transistor size (Intel's TIC-TOC)?
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 7: micro architecture
one word
Thanks, will fix.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 9: use separate
maybe "use a separate"
Thanks, will fix.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 12: , which all [Family 17h](family17h.md) : will also use.
I would leave Family 17h and future tense "will use" out of this document.
Ok. Though I'm calling attention as to why I only added references to 2 models.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 19: PSP
It feels really out of place to read about the PSP here! Why is it needed for an introduction to Fa […]
Reading back, I agree. Will remove.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 29: the binary PI release
The reader doesn't yet know what a binaryPI release is.
Meaning I should explain it before this section?
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 29: coreboo
sp
Oops....
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 34: >In particular,
Why blockquote here? Also, I'm not sure that discussing coreboot's GPIO library adds much to the Fa […]
Block quote due to problems with formatting the way I want... though now I learned a different way that might solve the problem here. I just wanted to give an example of what and why was migrated from AGESA to coreboot.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 43: Control
Controller
Thanks.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 45: References
Models 00h-0Fh […]
I was intent on only the models used on recent coreboot code.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 5:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG@9 PS5, Line 9: Create documentation for AMD family 15h, and in particular to models for
jive? I don't understand. Sorry, English is not my first language.
It is incorrect to imply there is no coreboot code for the models I mentioned above, which is what "models for which there's coreboot code..." does. So this premise is incorrect.
This document should talk about every Family 15h model in coreboot.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 6: attend a broad spectrum
I use spell checking... but I know it does not detects everything. […]
You "attend" a party but not a spectrum of users. If you want to say attend to their needs, may be a better word is "address". But as I mentioned, I really dislike this passage.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 5: Family 15h is available in several models with different numbers of cores : and some other small variations,
True...but I have to start my document somewhere, and don't have much information to add here. […]
No, there's no relation between Family 15h vs. Family 14h. Not tick-tock, not next generation, just unrelated. Its most natural predecessor is Family 10h.
All Family 15h devices use shared resources between pairs of cores. You can search Bulldozer for more info. Each pair is called a Compute Unit or CU, and don't forget that SMT is not supported.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 12: , which all [Family 17h](family17h.md) : will also use.
Ok. Though I'm calling attention as to why I only added references to 2 models.
Since this is an "AMD Family 15h document, I think they should all be addressed.
Also, you can add source code structure, i.e. the two ST and Merlin Falcon use soc and the older ones use cpu/nb/sb.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 29: the binary PI release
Meaning I should explain it before this section?
If you expect them to understand you, then yes.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 34: >In particular,
Block quote due to problems with formatting the way I want... […]
I don't see much value in discussing what was migrated from AGESA to coreboot.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 45: References
I was intent on only the models used on recent coreboot code.
I'm only asking for links to documents.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add family 15h ......................................................................
Patch Set 5:
(11 comments)
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 31: >
Spelling or the use of a box comment style (>)?
Marshall suggests I should remove it completely
https://review.coreboot.org/c/coreboot/+/34561/4/Documentation/soc/amd/famil... PS4, Line 38: may be viewed as analogous to the : Intel® Management Engine
I wanted to call attention to the fact that it's a separate processor within the SOC just like Intel […]
Ok, will remove.
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 6: attend a broad spectrum
You "attend" a party but not a spectrum of users. […]
Removed
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 5: Family 15h is available in several models with different numbers of cores : and some other small variations,
No, there's no relation between Family 15h vs. Family 14h. […]
Ack
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 7: micro architecture
Thanks, will fix.
Removed
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 9: use separate
Thanks, will fix.
Done
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 12: , which all [Family 17h](family17h.md) : will also use.
Since this is an "AMD Family 15h document, I think they should all be addressed. […]
Done
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 19: PSP
Reading back, I agree. Will remove.
Done
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 29: coreboo
Oops....
Done
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 43: Control
Thanks.
Done
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 45: References
I'm only asking for links to documents.
Done
Hello Kerry Brown, Paul Menzel, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#6).
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Documentation/soc/amd: Add Family 15h
Create documentation for AMD Family 15h.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 90 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/6
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG@7 PS5, Line 7: f
Will do.
Done
https://review.coreboot.org/c/coreboot/+/34561/5//COMMIT_MSG@9 PS5, Line 9: Create documentation for AMD family 15h, and in particular to models for
It is incorrect to imply there is no coreboot code for the models I mentioned above, which is what " […]
Done
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 29: the binary PI release
If you expect them to understand you, then yes.
Done
https://review.coreboot.org/c/coreboot/+/34561/5/Documentation/soc/amd/famil... PS5, Line 34: >In particular,
I don't see much value in discussing what was migrated from AGESA to coreboot.
Ack
https://review.coreboot.org/c/coreboot/+/34561/6/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/6/Documentation/soc/amd/famil... PS6, Line 40: ## Introduction : : Family 15h [SOC|Processors] products are x86-based designs. : : AMD has historically required an NDA for access to the PSP specification. : coreboot relies on util/amdfwtool to build the structures and add various : other firmware to the final image. : : Support in coreboot for modern AMD products is based on AMD’s : reference code: AMD Generic Encapsulated Software Architecture : (_AGESA ™_). _AGESA_ contains the technology for enabling DRAM, : configuring proprietary core logic, assistance with generating ACPI : tables, and other features. : : In the binary PI release used by coreboo, some functionality, such as the : setting of MTRRS and GPIO configurations along with setting of D0/D3 of : some devices (such as I2C and UART) were removed from _AGESA_ and converted : to coreboot code for granularity, speed and easy of use. : : >In particular, coreboot's approach to GPIO achieved a greater control : >over what is being programmed through the use of a table that is easily : >created using well defined macros. : Should have been removed! Will be on next patch.
Hello Kerry Brown, Paul Menzel, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#7).
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Documentation/soc/amd: Add Family 15h
Create documentation for AMD Family 15h.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 67 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/7
Hello Kerry Brown, Paul Menzel, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#8).
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Documentation/soc/amd: Add Family 15h
Create documentation for AMD Family 15h.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 54 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/8
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 8:
(14 comments)
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 5: Family 15h (Bulldozer) is a microprocessor microarchitecture first released in : 2011. What about something like
Family 15h is a line of AMD products first introduced in 2011. The initial microarchitecture, codenamed "Bulldozer", introduced...
You can then mention the other microarchitecture names, however I don't think they warrant much discussion of what each brought into play.
I'm not sure I would say it "evolved" into five models. In fact, looking around I see at least one more. So maybe be nonspecific here.
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 8: threads cores
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 9: thread core
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 13: replaces replace
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 13: Family 15h [SOC|Processors] products are x86-based designs that replaces previous : SMT schema with CMT Huh? AMD never had SMT in a previous product.
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 14: .. 2 periods
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 22: . All families weren't identical. I would either be less specific here or perhaps clarify you're talking about the initial BD uarch. Maybe add that resource sharing changed in the later families and other performance improvements were introduced.
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 24: A die also includes Some products include
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 26: (grounding a pin I would remove this
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 27: validated how about authenticated
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 31: _AGESA ™_). _AGESA_ Why are you italicizing?
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 37: _Merlin Falcon_) and 70h-7Fh (_Stoney Ridge_) Why are you italicizing?
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 40: ## Additional Definitions I'm not sure you need additional definitions anymore. You described PSP above and FCH isn't used anywhere else in the document. Not all Family 15h contain an integrated FCH.
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 45: extra whitespace?
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 8:
(14 comments)
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 5: Family 15h (Bulldozer) is a microprocessor microarchitecture first released in : 2011.
What about something like […]
Done
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 8: threads
cores
Done
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 9: thread
core
Done
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 13: replaces
replace
Removed
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 13: Family 15h [SOC|Processors] products are x86-based designs that replaces previous : SMT schema with CMT
Huh? AMD never had SMT in a previous product.
Misunderstood from reading Wikipedia document on Bulldozer. Removed.
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 14: ..
2 periods
Removed.
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 22: .
All families weren't identical. […]
Later models (they are all Family 15h)...Yes, added the info.
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 24: A die also includes
Some products include
Done
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 26: (grounding a pin
I would remove this
Done
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 27: validated
how about authenticated
Done
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 31: _AGESA ™_). _AGESA_
Why are you italicizing?
Done
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 37: _Merlin Falcon_) and 70h-7Fh (_Stoney Ridge_)
Why are you italicizing?
Done
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 40: ## Additional Definitions
I'm not sure you need additional definitions anymore. […]
Done
https://review.coreboot.org/c/coreboot/+/34561/8/Documentation/soc/amd/famil... PS8, Line 45:
extra whitespace?
So that the unformatted document with a regular text editor looks identical to a formatted one through markdown reader. As far as I tested, markdown readers are smart enough to ignore the extra space, so I did it on purpose.
Hello Kerry Brown, Paul Menzel, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#9).
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Documentation/soc/amd: Add Family 15h
Create documentation for AMD Family 15h.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 45 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/9
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 9:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 6: "Clustered MultiThreading" This bugged me before and I didn't say anything. I'd never heard this term used while at AMD, and I suspect it was an afterthought later to try to make a disclaimer for the underwhelming performance. In fact, reading it here was the first I'd ever come across the term.
My preference would be to use the more common terminology of Compute Unit, or CU, to describe each core pair.
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 7: replacing SMT remove
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 8: models This is uncomfortably short now. How about
Family 15h offerings matured into various models with increased performance and features targeting Enterprise, Client, and Embedded designs.
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 15: module Regardless of above, this is certainly called a compute unit and not a module.
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 20: . Could also note that Family 15h comprises 3-chip solutions, 2, and single-chip. Devices designed to contain on-die graphics (i.e. even when they're headless) are commonly referred to as APUs, not CPUs.
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 28: : could delete this
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 29: Probably want to remove the space
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 9:
(7 comments)
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 6: "Clustered MultiThreading"
This bugged me before and I didn't say anything. […]
Copy/paste from Wikipedia.
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 7: replacing SMT
remove
Done
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 8: models
This is uncomfortably short now. How about […]
Done
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 15: module
Regardless of above, this is certainly called a compute unit and not a module.
Again Wikipedia. Will change.
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 20: .
Could also note that Family 15h comprises 3-chip solutions, 2, and single-chip. […]
Ack
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 28: :
could delete this
Don't think so.
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 29:
Probably want to remove the space
Ack
Hello Kerry Brown, Paul Menzel, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#10).
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Documentation/soc/amd: Add Family 15h
Create documentation for AMD Family 15h.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 50 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/10
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 10: Code-Review-1
(1 comment)
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 6: "Clustered MultiThreading"
Copy/paste from Wikipedia.
We can't copy from wikipedia - it uses a different license. Please rewrite anything you copied.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 6: "Clustered MultiThreading"
We can't copy from wikipedia - it uses a different license. Please rewrite anything you copied.
Sorry, I was not aware. Will rewrite.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/9/Documentation/soc/amd/famil... PS9, Line 6: "Clustered MultiThreading"
Sorry, I was not aware. Will rewrite.
Done
Hello Kerry Brown, Paul Menzel, build bot (Jenkins), Martin Roth, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#11).
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Documentation/soc/amd: Add Family 15h
Create documentation for AMD Family 15h.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 49 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/11
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 11:
Ping... review please.
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 11:
ping... please review.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34561/11/Documentation/soc/amd/fami... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/11/Documentation/soc/amd/fami... PS11, Line 22: comprises 3-chip solutions, 2, and single-chip "Fam 15h setups use up to three separate chips"? The "3-chip", "2", "single" stuff looks very inconsistent.
https://review.coreboot.org/c/coreboot/+/34561/11/Documentation/soc/amd/fami... PS11, Line 34: technology It's code ;-)
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34561/11/Documentation/soc/amd/fami... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/11/Documentation/soc/amd/fami... PS11, Line 22: comprises 3-chip solutions, 2, and single-chip
"Fam 15h setups use up to three separate chips"? The "3-chip", "2", "single" stuff looks very incons […]
Done
https://review.coreboot.org/c/coreboot/+/34561/11/Documentation/soc/amd/fami... PS11, Line 34: technology
It's code ;-)
Done
Hello Kerry Brown, Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#12).
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Documentation/soc/amd: Add Family 15h
Create documentation for AMD Family 15h.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 49 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/12
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 12: Code-Review+1
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 12:
(13 comments)
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 5: AMD products How about AMD x86 products?
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 6: of "C of a "Compute...
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 7: (CU) where some parts of the processor are shared between two cores and : some parts are unique for each core. How about "...(CU); a core pair grouping with certain shared resources.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 10: (see references below). The placement of this after the "Enterprise, Client, and Embedded designs" kind of bugs me. It seems to imply to the reader that each of the references individually must be Enterprise, or Client, etc. This is incorrect, though.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 14: Initially with CU, 2 x86 cores have separate integer processors but share cache, The first CU designs were 2 x86 cores with...
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 15: A die can have up to : 8 CU ...can contain up to CUs.
I don't specifically recall this limit. You may be correct, but do you have it handy where you found this information?
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 22: Family 15h setups use up to three separate chips How about "Family 15h products range from SOCs to 3-chip solutions."?
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 23: (i.e. even when they're headless) (including headless)
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 26: which deleting this will make it read better.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 26: Some products Later SOCs
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 27: security related measures I have mixed feelings discussing this here, as no coreboot implementation supports it. BTW, mentioning fTPM seems just as important but also not supported. These features are only in UEFI with no plans to make them available in coreboot.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 28: SPI SPI flash?
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 37: implements You use a form of this word 3 times in 2 sentences. Maybe this one changes to "contains"? Below you could say "Newer products... rely on a more modern..."
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 12:
(13 comments)
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 5: AMD products
How about AMD x86 products?
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 6: of "C
of a "Compute...
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 7: (CU) where some parts of the processor are shared between two cores and : some parts are unique for each core.
How about "...(CU); a core pair grouping with certain shared resources.
I don't know. This document is already so small... I would prefer to waste words, though I agree that your way says the same thing.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 10: (see references below).
The placement of this after the "Enterprise, Client, and Embedded designs" kind of bugs me. […]
Added a phrase explicitly stating that a model can support more than 1 market.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 14: Initially with CU, 2 x86 cores have separate integer processors but share cache,
The first CU designs were 2 x86 cores with...
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 15: A die can have up to : 8 CU
...can contain up to CUs. […]
Searched the WEB for technical reviews of AMD models included on family 15h. Did not found anything with more than 8 CU.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 22: Family 15h setups use up to three separate chips
How about "Family 15h products range from SOCs to 3-chip solutions. […]
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 23: (i.e. even when they're headless)
(including headless)
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 26: Some products
Later SOCs
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 26: which
deleting this will make it read better.
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 27: security related measures
I have mixed feelings discussing this here, as no coreboot implementation supports it. […]
Even if there are no plans of making them available to coreboot, that does not mean that some company could not try to do it. I say, leave the information, as it's the PSP reality. They can always as AMD support if needed.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 28: SPI
SPI flash?
Done
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 37: implements
You use a form of this word 3 times in 2 sentences. […]
Done
Hello Kerry Brown, Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34561
to look at the new patch set (#13).
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Documentation/soc/amd: Add Family 15h
Create documentation for AMD Family 15h.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 50 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34561/13
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 13: Code-Review+2
(1 comment)
I believe you still need to update soc/amd/index.md to link to this file.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 15: A die can have up to : 8 CU
Searched the WEB for technical reviews of AMD models included on family 15h. […]
Ack
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 13:
(1 comment)
Patch Set 13: Code-Review+2
(1 comment)
I believe you still need to update soc/amd/index.md to link to this file.
Nope, it's there already.
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... File Documentation/soc/amd/family15h.md:
https://review.coreboot.org/c/coreboot/+/34561/12/Documentation/soc/amd/fami... PS12, Line 7: (CU) where some parts of the processor are shared between two cores and : some parts are unique for each core.
I don't know. This document is already so small... […]
Ack
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Patch Set 13:
Patch Set 13:
(1 comment)
Patch Set 13: Code-Review+2
(1 comment)
I believe you still need to update soc/amd/index.md to link to this file.
Nope, it's there already.
Hmm, that's weird. I typed it, saw the file then thought I deleted that text -- sorry about that.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/34561 )
Change subject: Documentation/soc/amd: Add Family 15h ......................................................................
Documentation/soc/amd: Add Family 15h
Create documentation for AMD Family 15h.
BUG=none. TEST=none.
Change-Id: Iaab4edc431329a691283121494595f3797c566c6 Signed-off-by: Richard Spiegel richard.spiegel@silverbackltd.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34561 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com --- A Documentation/soc/amd/family15h.md M Documentation/soc/amd/index.md 2 files changed, 50 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Marshall Dawson: Looks good to me, approved
diff --git a/Documentation/soc/amd/family15h.md b/Documentation/soc/amd/family15h.md new file mode 100644 index 0000000..fc41e91 --- /dev/null +++ b/Documentation/soc/amd/family15h.md @@ -0,0 +1,49 @@ +# AMD Family 15h [SOC|Processors] + +## Abstract + +Family 15h is a line of AMD x86 products first introduced in 2011. The initial +microarchitecture, codenamed "Bulldozer", introduced the concept of a "Compute +Unit" (CU) where some parts of the processor are shared between two cores and +some parts are unique for each core. Family 15h offerings matured into various +models with increased performance and features targeting Enterprise, Client, +and Embedded designs. Notice that a particular model can address more than one +market(see models references below). + +## Introduction + +The first CU designs were 2 x86 cores with separate integer processors but +sharing cache, code branch prediction engine and floating point processor. A die +can have up to 8 CU. The floating point processor is composed of two symmetrical +128-bit FMAC. Provided each x86 core is doing 128-bit floating point arithmetic, +they both do floating point simultaneously. If one is doing 256-bit floating +point, the other x86 core can't do floating point simultaneously. Later models +changed how resources were shared, and introduced other performance improvements. + +Family 15h products range from SOCs to 3-chip solutions. Devices designed to +contain on-die graphics (including headless) are commonly referred to as APUs, +not CPUs. + +Later SOCs include a Platform Security Processor (PSP), a small ARM processor +responsible for security related measures: For example, if secure boot is +enabled, the cores will not exit reset until the BIOS image within the SPI +flash is authenticated through its OEM signature, thus ensuring that only OEM +produced BIOS can run the platform. + +Support in coreboot for modern AMD products is based on AMD’s reference code: +AMD Generic Encapsulated Software Architecture (AGESA™). AGESA contains the +code for enabling DRAM, configuring proprietary core logic, assistance with +generating ACPI tables, and other features. + +While coreboot contains support for most models, some implementations use a +separate cpu/north/south bridge directory structure. Newer products for models +60h-6Fh (Merlin Falcon) and 70h-7Fh (Stoney Ridge) rely on modern SOC directory +structure. + +## References + +1. [Models 00h-0Fh BKDG](https://www.amd.com/system/files/TechDocs/42301_15h_Mod_00h-0Fh_BKDG.pdf) +2. [Models 10h-1Fh BKDG](https://www.amd.com/system/files/TechDocs/42300_15h_Mod_10h-1Fh_BKDG.pdf) +3. [Models 30h-3Fh BKDG](https://www.amd.com/system/files/TechDocs/49125_15h_Models_30h-3Fh_BKDG.pdf) +4. [Models 60h-6Fh BKDG](https://www.amd.com/system/files/TechDocs/50742_15h_Models_60h-6Fh_BKDG.pdf) +5. [Models 70h-7Fh BKDG](https://www.amd.com/system/files/TechDocs/55072_AMD_Family_15h_Models_70h-7F...) diff --git a/Documentation/soc/amd/index.md b/Documentation/soc/amd/index.md index 7945b48..d6f31c8 100644 --- a/Documentation/soc/amd/index.md +++ b/Documentation/soc/amd/index.md @@ -4,5 +4,6 @@
## Technology
+- [Family 15h](family15h.md) - [Family 17h](family17h.md)
-
Marshall Dawson (Code Review) -
Martin Roth (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Richard Spiegel (Code Review)