Franklin (Yanjin) He has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39579 )
Change subject: soc/intel/apollolake: Allow toggling of GMM in devicetree in Gemini Lake
......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39579/3/src/soc/intel/apollolake/i…
File src/soc/intel/apollolake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/39579/3/src/soc/intel/apollolake/i…
PS3, Line 49: #define SA_DEV_SLOT_GMM 0x08
> In GLK, GNA device ID is 0x03. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/39579
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I72b1dd78705894f0462c7fbe89b76551950c2392
Gerrit-Change-Number: 39579
Gerrit-PatchSet: 4
Gerrit-Owner: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 18 Mar 2020 04:07:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Hello build bot (Jenkins), Furquan Shaikh, Edward O'Callaghan, Andrey Petrov, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39579
to look at the new patch set (#4).
Change subject: soc/intel/apollolake: Allow toggling of GMM in devicetree in Gemini Lake
......................................................................
soc/intel/apollolake: Allow toggling of GMM in devicetree in Gemini Lake
Enables Gaussian Mixture Model (GMM) if the pci device is enabled in the
devicetree for Gemini Lake
This ports commit 03ddd190fd6a2e91b16e6fd8a101cf4e11d7cd7b
BUG=b:151115705
BRANCH=none
TEST=Flashed to Chromebook, PCI device enabled in cbmem, userspace app
that uses device still works
Change-Id: I72b1dd78705894f0462c7fbe89b76551950c2392
Signed-off-by: Franklin He <franklinh(a)google.com>
---
M src/soc/intel/apollolake/chip.c
M src/soc/intel/apollolake/include/soc/pci_devs.h
2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/39579/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/39579
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I72b1dd78705894f0462c7fbe89b76551950c2392
Gerrit-Change-Number: 39579
Gerrit-PatchSet: 4
Gerrit-Owner: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-MessageType: newpatchset
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39502 )
Change subject: mb/google/deltaur: add deltaur mainboard initial support
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39502/5/src/mainboard/google/delta…
File src/mainboard/google/deltaur/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/39502/5/src/mainboard/google/delta…
PS5, Line 5: 0x333000
> Yeah Volteer has it as 0x4ff000
We can cut off RW_DIAG, we still have empty room. eDiag only use 8MB now.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39502
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib98f328df22f39e7d9d625a3292954881ee15b15
Gerrit-Change-Number: 39502
Gerrit-PatchSet: 5
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-CC: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-CC: Selma Bensaid <selma.bensaid(a)intel.corp-partner.google.com>
Gerrit-CC: Varun Joshi <varun.joshi(a)intel.com>
Gerrit-Comment-Date: Wed, 18 Mar 2020 03:54:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bora Guvendik <bora.guvendik(a)intel.com>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-MessageType: comment
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39579 )
Change subject: soc/intel/apollolake: Allow toggling of GMM in devicetree in Gemini Lake
......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39579/3/src/soc/intel/apollolake/i…
File src/soc/intel/apollolake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/39579/3/src/soc/intel/apollolake/i…
PS3, Line 49: #define SA_DEV_SLOT_GMM 0x08
In GLK, GNA device ID is 0x03.
Please refer to Document#: 569262(Glk EDS Vol-1 rev2-7). Same comment applies for the change in the devicetree.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39579
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I72b1dd78705894f0462c7fbe89b76551950c2392
Gerrit-Change-Number: 39579
Gerrit-PatchSet: 3
Gerrit-Owner: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 18 Mar 2020 01:53:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39502 )
Change subject: mb/google/deltaur: add deltaur mainboard initial support
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39502/5/src/mainboard/google/delta…
File src/mainboard/google/deltaur/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/39502/5/src/mainboard/google/delta…
PS5, Line 5: 0x333000
> This will need to be bigger actually.
Yeah Volteer has it as 0x4ff000
--
To view, visit https://review.coreboot.org/c/coreboot/+/39502
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib98f328df22f39e7d9d625a3292954881ee15b15
Gerrit-Change-Number: 39502
Gerrit-PatchSet: 5
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-CC: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-CC: Selma Bensaid <selma.bensaid(a)intel.corp-partner.google.com>
Gerrit-CC: Varun Joshi <varun.joshi(a)intel.com>
Gerrit-Comment-Date: Wed, 18 Mar 2020 01:20:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-MessageType: comment
Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39538 )
Change subject: soc/intel/skylake: Configure ASPM and L1 substates for PCH root ports
......................................................................
Patch Set 12: Code-Review+1
(1 comment)
Open to the below if it would be considered cleaner. Or alternatives.
"if (config == 0); else if (config)" (the enum would be changed back)
https://review.coreboot.org/c/coreboot/+/39538/6/src/soc/intel/skylake/chip…
File src/soc/intel/skylake/chip.c:
https://review.coreboot.org/c/coreboot/+/39538/6/src/soc/intel/skylake/chip…
PS6, Line 221: PcieRpAspm
> With ASPM_DISABLED, coreboot reports that it enabled L1 for ASPM? However, the errors reported by AE […]
No, my mistake. Perhaps a build-time error? In any event, this works as expected.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39538
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I36150858485715016158595c832c142b0582ddb8
Gerrit-Change-Number: 39538
Gerrit-PatchSet: 12
Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 18 Mar 2020 00:31:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Benjamin Doron <benjamin.doron00(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39579 )
Change subject: soc/intel/apollolake: Allow toggling of GMM in devicetree in Gemini Lake
......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39579/1/src/soc/intel/apollolake/c…
File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/39579/1/src/soc/intel/apollolake/c…
PS1, Line 637: silconfig->Gmm = dev ? dev->enabled : 0;
> Ack
I disagree with Paul, this should be precisely the same as was done in commit 03ddd190fd6a2e91b16e6fd8a101cf4e11d7cd7b. I don't see the need to change that unless I missed something?
--
To view, visit https://review.coreboot.org/c/coreboot/+/39579
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I72b1dd78705894f0462c7fbe89b76551950c2392
Gerrit-Change-Number: 39579
Gerrit-PatchSet: 3
Gerrit-Owner: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 18 Mar 2020 00:18:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Franklin (Yanjin) He <franklinh(a)google.com>
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Franklin (Yanjin) He has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39579 )
Change subject: soc/intel/apollolake: Allow toggling of GMM in devicetree in Gemini Lake
......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39579/1/src/soc/intel/apollolake/c…
File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/39579/1/src/soc/intel/apollolake/c…
PS1, Line 637: silconfig->Gmm = dev ? dev->enabled : 0;
> The device is integrated on the SoC for all gemini lake CPUs so we are hoping to enable it for all v […]
Ack
--
To view, visit https://review.coreboot.org/c/coreboot/+/39579
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I72b1dd78705894f0462c7fbe89b76551950c2392
Gerrit-Change-Number: 39579
Gerrit-PatchSet: 3
Gerrit-Owner: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 18 Mar 2020 00:14:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Franklin (Yanjin) He <franklinh(a)google.com>
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Bora Guvendik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39502 )
Change subject: mb/google/deltaur: add deltaur mainboard initial support
......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39502/5/src/mainboard/google/delta…
File src/mainboard/google/deltaur/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/39502/5/src/mainboard/google/delta…
PS5, Line 5: 0x333000
This will need to be bigger actually.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39502
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ib98f328df22f39e7d9d625a3292954881ee15b15
Gerrit-Change-Number: 39502
Gerrit-PatchSet: 5
Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 18 Mar 2020 00:10:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Franklin (Yanjin) He has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39579 )
Change subject: soc/intel/apollolake: Allow toggling of GMM in devicetree in Gemini Lake
......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39579/1/src/soc/intel/apollolake/c…
File src/soc/intel/apollolake/chip.c:
https://review.coreboot.org/c/coreboot/+/39579/1/src/soc/intel/apollolake/c…
PS1, Line 635: // Enables or disables GMM in devicetree
> The comment is not needed, as the code is obvious.
Done
https://review.coreboot.org/c/coreboot/+/39579/1/src/soc/intel/apollolake/c…
PS1, Line 637: silconfig->Gmm = dev ? dev->enabled : 0;
> Is it required on all the GLK devices or is it required for select variants? […]
The device is integrated on the SoC for all gemini lake CPUs so we are hoping to enable it for all variants of Gemini Lake designs. At least that is the message from Intel.
--
To view, visit https://review.coreboot.org/c/coreboot/+/39579
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I72b1dd78705894f0462c7fbe89b76551950c2392
Gerrit-Change-Number: 39579
Gerrit-PatchSet: 3
Gerrit-Owner: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Franklin (Yanjin) He <franklinh(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Comment-Date: Wed, 18 Mar 2020 00:03:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Franklin (Yanjin) He <franklinh(a)google.com>
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment