Attention is currently required from: Martin L Roth, Angel Pons, Julius Werner.
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/67918 )
Change subject: tests: Add option for debug symbols & no optimization
......................................................................
Patch Set 3:
(2 comments)
File tests/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/67918/comment/32c6f3ad_79f61939
PS2, Line 183: GDB_DEBUG=y
> Done. […]
I think it's good solution for now.
I have to check tests build-system and fix all these variables some time. :)
File tests/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/67918/comment/870815b3_4fbd1956
PS3, Line 13: # Enable GDB debug build if requested
: GDB_DEBUG?=0
1. unnecessary space at lines start
2. add space around assign operator
--
To view, visit https://review.coreboot.org/c/coreboot/+/67918
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3a644dcccb7e15473413b775da8f70617afaefce
Gerrit-Change-Number: 67918
Gerrit-PatchSet: 3
Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Comment-Date: Mon, 31 Oct 2022 07:49:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com>
Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-MessageType: comment
Jakub Czapiga has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68923 )
Change subject: acpigen: Always inline helper functions
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/68923/comment/9dc25e9f_f88139ef
PS3, Line 12: normally
> I am no compiler expert, but aren’t todays compilers do pretty unexpected choices regarding these th […]
Hi Paul. I see there is a need for some backstory for this change:
In free time I'm working on a profiler for coreboot (CB:65606). I had problems with inline functions in headers. I had to modify acpigen.h the same way as I did here to make it compile correctly, because GCC was sometimes treating these functions as inline and sometimes it did not. Making them force-inlined simply solved the problem. Also, that was my inent when I was introducing these functions. They had to be simple readable wrappers (macros were rejected during review) and never used the other way.
I want to provide way to enable or disable profiling of inline functions (reaaaaly expensive, but might be useful in rare cases), so it should work at least with common code.
Regarding things compilers do to optimize code: Yes, they do pretty wild stuff these days, but this change is more related to linking process than compilation alone, because builds were failing during linking stage.
--
To view, visit https://review.coreboot.org/c/coreboot/+/68923
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibf747573940fe5e76199f327f4e5bc32b4f8c470
Gerrit-Change-Number: 68923
Gerrit-PatchSet: 3
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Mon, 31 Oct 2022 07:44:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Matt DeVillier, Nelson Ye, Tim Van Patten, Eric Peers, Fred Reitberger, Karthikeyan Ramasubramanian, Felix Held.
EricKY Cheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68470 )
Change subject: soc/amd/acpi: Expand 5 DPTC thermal profiles acpigen support for Alib
......................................................................
Patch Set 15:
(1 comment)
File src/soc/amd/common/block/acpi/alib.c:
https://review.coreboot.org/c/coreboot/+/68470/comment/71a272a1_c99dcc86
PS12, Line 111: acpigen_write_method_serialized("DTTB", 0);
> Please re-add the comment block, so we know how the generated ASL should look.
Re-add the comment block from B to F
--
To view, visit https://review.coreboot.org/c/coreboot/+/68470
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9e6d5c0fc6f492340c935899920d9ee7c9396256
Gerrit-Change-Number: 68470
Gerrit-PatchSet: 15
Gerrit-Owner: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Chris Wang <chris.wang(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Eric Peers <epeers(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nelson Ye <nelson_ye(a)compal.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Nelson Ye <nelson_ye(a)compal.corp-partner.google.com>
Gerrit-Attention: Tim Van Patten <timvp(a)google.com>
Gerrit-Attention: Eric Peers <epeers(a)google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 31 Oct 2022 07:39:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Van Patten <timvp(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, EricKY Cheng, Matt DeVillier, Nelson Ye, Tim Van Patten, Eric Peers, Fred Reitberger, Karthikeyan Ramasubramanian, Felix Held.
Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Matt DeVillier, Chris Wang, Tim Van Patten, Eric Peers, Fred Reitberger, Karthikeyan Ramasubramanian, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/68470
to look at the new patch set (#15).
Change subject: soc/amd/acpi: Expand 5 DPTC thermal profiles acpigen support for Alib
......................................................................
soc/amd/acpi: Expand 5 DPTC thermal profiles acpigen support for Alib
Update acpigen_write_alib_dptc() to support extra 5 thermal profiles.
User can use these profiles for dynamic thermal table switching support.
BUG=b:232946420
TEST=emerge-skyrim coreboot
Signed-off-by: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com>
Change-Id: I9e6d5c0fc6f492340c935899920d9ee7c9396256
---
M src/soc/amd/common/block/acpi/alib.c
M src/soc/amd/common/block/include/amdblocks/alib.h
2 files changed, 172 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/68470/15
--
To view, visit https://review.coreboot.org/c/coreboot/+/68470
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9e6d5c0fc6f492340c935899920d9ee7c9396256
Gerrit-Change-Number: 68470
Gerrit-PatchSet: 15
Gerrit-Owner: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Chris Wang <chris.wang(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Eric Peers <epeers(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nelson Ye <nelson_ye(a)compal.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Nelson Ye <nelson_ye(a)compal.corp-partner.google.com>
Gerrit-Attention: Tim Van Patten <timvp(a)google.com>
Gerrit-Attention: Eric Peers <epeers(a)google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jason Glenesk, Raul Rangel, EricKY Cheng, Matt DeVillier, Nelson Ye, Tim Van Patten, Eric Peers, Fred Reitberger, Karthikeyan Ramasubramanian, Felix Held.
Chris Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68470 )
Change subject: soc/amd/acpi: Expand 5 DPTC thermal profiles acpigen support for Alib
......................................................................
Patch Set 14:
(1 comment)
File src/soc/amd/common/block/include/amdblocks/alib.h:
https://review.coreboot.org/c/coreboot/+/68470/comment/db3f82c5_cfd74917
PS12, Line 24: ALIB_DPTC_STT_MIN_LIMIT_ID = 0x20,
: ALIB_DPTC_STT_M1_ID = 0x21,
: ALIB_DPTC_STT_M2_ID = 0x22,
: ALIB_DPTC_STT_C_APU_ID = 0x23,
: ALIB_DPTC_STT_SKIN_TEMPERATURE_LIMIT_APU_ID = 0x24,
: ALIB_DPTC_STT_ALPHA_APU_ID = 0x25,
: ALIB_DPTC_STT_ERROR_COEFF_ID = 0x26,
: ALIB_DPTC_STT_ERROR_RATE_COEFF_ID = 0x27,
> These values don't match "AMD Generic Encapsulated Software Architecture (AGESATM) Interface Specifi […]
Hi Tim,
Yes, we can validate the value via agt command, you can read back the value by `agt -i=2 -SttM1Coeff` for example, and you can find the parameters with `agt dptc` to check.
--
To view, visit https://review.coreboot.org/c/coreboot/+/68470
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9e6d5c0fc6f492340c935899920d9ee7c9396256
Gerrit-Change-Number: 68470
Gerrit-PatchSet: 14
Gerrit-Owner: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Chris Wang <chris.wang(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Eric Peers <epeers(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nelson Ye <nelson_ye(a)compal.corp-partner.google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com>
Gerrit-Attention: Nelson Ye <nelson_ye(a)compal.corp-partner.google.com>
Gerrit-Attention: Tim Van Patten <timvp(a)google.com>
Gerrit-Attention: Eric Peers <epeers(a)google.com>
Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>
Gerrit-Attention: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Mon, 31 Oct 2022 07:38:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Van Patten <timvp(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Anjaneya "Reddy" Chagam, Johnny Lin, Christian Walter, Shuming Chu (Shuming), TangYiwei.
Shuming Chu (Shuming) has uploaded a new patch set (#3) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/68912 )
Change subject: soc/intel/xeon_sp: Don't sort struct device cpus for numa
......................................................................
soc/intel/xeon_sp: Don't sort struct device cpus for numa
Currently the xeon_sp code reassign struct devices apic_id so that srat
entries can be added in a certain order.
This is not a good idea as it break thread local storage which contains
a pointer to it's struct device cpu.
This moves the sorting to of the lapic_ids to the srat table generation
and adds the numa node id in each core init entry (now it is done in
parallel too as a bonus).
Change-Id: I372bcea1932d28e9bf712cc712f19a76fe3199b1
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/soc/intel/xeon_sp/cpx/cpu.c
M src/soc/intel/xeon_sp/include/soc/util.h
M src/soc/intel/xeon_sp/nb_acpi.c
M src/soc/intel/xeon_sp/skx/cpu.c
M src/soc/intel/xeon_sp/util.c
5 files changed, 74 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/68912/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/68912
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I372bcea1932d28e9bf712cc712f19a76fe3199b1
Gerrit-Change-Number: 68912
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: TangYiwei
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuming Chu (Shuming) <s1218944(a)gmail.com>
Gerrit-Attention: TangYiwei
Gerrit-MessageType: newpatchset
Attention is currently required from: Anjaneya "Reddy" Chagam, Johnny Lin, Christian Walter, Shuming Chu (Shuming), TangYiwei.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68912 )
Change subject: soc/intel/xeon_sp: Don't sort struct device cpus for numa
......................................................................
Patch Set 2: Verified+1
(2 comments)
File src/soc/intel/xeon_sp/nb_acpi.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161738):
https://review.coreboot.org/c/coreboot/+/68912/comment/f1281e60_dbd19fed
PS2, Line 70: cpu->path.apic.node_id, cpu->path.apic.apic_id);
line length of 98 exceeds 96 columns
File src/soc/intel/xeon_sp/skx/cpu.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-161738):
https://review.coreboot.org/c/coreboot/+/68912/comment/7704fbf4_6079b875
PS2, Line 64: __func__, dev_path(cpu), cpu_index(), cpu->path.apic.apic_id, cpu->path.apic.node_id);
line length of 101 exceeds 96 columns
--
To view, visit https://review.coreboot.org/c/coreboot/+/68912
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I372bcea1932d28e9bf712cc712f19a76fe3199b1
Gerrit-Change-Number: 68912
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com>
Gerrit-Reviewer: TangYiwei
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Tim Chu <Tim.Chu(a)quantatw.com>
Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com>
Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com>
Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Attention: Shuming Chu (Shuming) <s1218944(a)gmail.com>
Gerrit-Attention: TangYiwei
Gerrit-Comment-Date: Mon, 31 Oct 2022 07:25:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment