Anil Kumar K has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/77698?usp=email )
Change subject: Rex: Add debug prints in RAMOOP ACPI
......................................................................
Abandoned
--
To view, visit https://review.coreboot.org/c/coreboot/+/77698?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I3f09b95467f32eacf85c84978e146949b25704e6
Gerrit-Change-Number: 77698
Gerrit-PatchSet: 1
Gerrit-Owner: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.corp-partner.google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-MessageType: abandon
Attention is currently required from: Arthur Heymans, Julius Werner, Kapil Porwal, Lean Sheng Tan, Nick Vaccaro, Werner Zeh.
Hello Arthur Heymans, Julius Werner, Kapil Porwal, Lean Sheng Tan, Nick Vaccaro, Werner Zeh, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/78080?usp=email
to look at the new patch set (#4).
Change subject: soc/intel/cmn/gfx: Add API to report presence of external display
......................................................................
soc/intel/cmn/gfx: Add API to report presence of external display
This patch implements an API to report the presence of an external
display on Intel silicon. The API uses information from the transcoder
and framebuffer to determine if an external display is connected.
For example, if the transcoder is attached to any DDI ports other than
DDI-A (eDP), and the framebuffer is initialized, then it is likely
that an external display is present.
This information can be used by payloads to determine whether or not
to power on the display, even if eDP is not initialized.
BUG=b:299137940
TEST=Build and boot google/rex
Scenarios:
Booting with eDP alone: is_external_display value is 0
Booting with eDP + HDMI: is_external_display value is 0
Booting with HDMI alone: is_external_display value is 1
Booting with USB-C display alone: is_external_display value is 1
Change-Id: I77436940978c7fa9368d79394b46a5e794c32e42
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/common/block/graphics/graphics.c
1 file changed, 54 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/78080/4
--
To view, visit https://review.coreboot.org/c/coreboot/+/78080?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I77436940978c7fa9368d79394b46a5e794c32e42
Gerrit-Change-Number: 78080
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Arthur Heymans, Julius Werner, Kapil Porwal, Lean Sheng Tan, Nick Vaccaro, Werner Zeh.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78080?usp=email )
Change subject: soc/intel/cmn/gfx: Add API to report presence of external display
......................................................................
Patch Set 3:
(1 comment)
File src/soc/intel/common/block/graphics/graphics.c:
https://review.coreboot.org/c/coreboot/+/78080/comment/8f5cba2b_55563746 :
PS1, Line 59: #if CONFIG(REPORT_EXTERNAL_DISPLAY_AT_PREBOOT)
> > So you're saying that those other platforms would return 0 from report_external_display() (because […]
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/78080?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I77436940978c7fa9368d79394b46a5e794c32e42
Gerrit-Change-Number: 78080
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Comment-Date: Mon, 02 Oct 2023 21:28:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Andrey Petrov, Arthur Heymans, Julius Werner, Karthik Ramasubramanian, Nick Vaccaro, Yu-Ping Wu.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77796?usp=email )
Change subject: {commonlib, libpayload}:Â Add "is_external display" in coreboot table
......................................................................
Patch Set 4:
(3 comments)
File payloads/libpayload/include/coreboot_tables.h:
https://review.coreboot.org/c/coreboot/+/77796/comment/210c3530_4c05bb76 :
PS2, Line 241: u8 is_external_display;
> Let's make this a `flags` field and define is_external_display as bit 0.
Acknowledged
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/77796/comment/1919faed_65b01774 :
PS2, Line 452: on-display
> Nit: on-board
Acknowledged
File src/drivers/intel/fsp2_0/graphics.c:
https://review.coreboot.org/c/coreboot/+/77796/comment/496d6474_b43a2ff3 :
PS2, Line 126: framebuffer_bar
> +1
Acknowledged
--
To view, visit https://review.coreboot.org/c/coreboot/+/77796?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I0fa7eee4c5a50371a7a66c6ca1ac2c7d046d010b
Gerrit-Change-Number: 77796
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Mon, 02 Oct 2023 21:27:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Julius Werner, Kapil Porwal, Lean Sheng Tan, Nick Vaccaro, Werner Zeh.
Hello Arthur Heymans, Julius Werner, Kapil Porwal, Lean Sheng Tan, Nick Vaccaro, Werner Zeh, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/78080?usp=email
to look at the new patch set (#3).
Change subject: soc/intel/cmn/gfx: Add API to report presence of external display
......................................................................
soc/intel/cmn/gfx: Add API to report presence of external display
This patch implements an API to report the presence of an external
display on Intel silicon. The API uses information from the transcoder
and framebuffer to determine if an external display is connected.
For example, if the transcoder is attached to any DDI ports other than
DDI-A (eDP), and the framebuffer is initialized, then it is likely
that an external display is present.
This information can be used by payloads to determine whether or not
to power on the display, even if eDP is not initialized.
BUG=b:299137940
TEST=Build and boot google/rex
Scenarios:
Booting with eDP alone: is_external_display value is 0
Booting with eDP + HDMI: is_external_display value is 0
Booting with HDMI alone: is_external_display value is 1
Booting with USB-C display alone: is_external_display value is 1
Change-Id: I77436940978c7fa9368d79394b46a5e794c32e42
Signed-off-by: Subrata Banik <subratabanik(a)google.com>
---
M src/soc/intel/common/block/graphics/graphics.c
1 file changed, 52 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/78080/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/78080?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I77436940978c7fa9368d79394b46a5e794c32e42
Gerrit-Change-Number: 78080
Gerrit-PatchSet: 3
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Patrick Rudolph.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78133?usp=email )
Change subject: cpu/intel/model_206ax: Print supported C-states
......................................................................
Patch Set 5:
(7 comments)
Patchset:
PS5:
might also be an idea to split the patch into the part that introduces the common code functionality and the part that uses that in the intel cpu code.
didn't do a full in-depth review when i commented the last time, but i've done a much more in-depth review now
File src/arch/x86/cpu_common.c:
https://review.coreboot.org/c/coreboot/+/78133/comment/735a820e_1e1153a0 :
PS5, Line 196: int cpu_get_c_state_support(const int lvl)
hmm, i'd make both return value and parameter unsigned integers, since those won't and shouldn't be negative. should the value range of the lvl parameter also be checked?
https://review.coreboot.org/c/coreboot/+/78133/comment/d7364a83_33a34241 :
PS5, Line 201: 1
might be good to have a define for this bit
File src/cpu/intel/model_206ax/acpi.c:
https://review.coreboot.org/c/coreboot/+/78133/comment/80807737_44c1ca25 :
PS5, Line 103: h, l
it would make the code easier to read if you'd use 'state' and 'substate' as variable names
https://review.coreboot.org/c/coreboot/+/78133/comment/73fd868c_4c344b02 :
PS5, Line 104:
maybe add a comment that C0 is always supported; was wondering why i was initialized as 1, but that seems to be the reason
https://review.coreboot.org/c/coreboot/+/78133/comment/1223326b_12ef286c :
PS5, Line 109: > l
i'd drop this or make it != 0 and drop the + 1 in line 106 too; at least for me that would make the code easier to read
https://review.coreboot.org/c/coreboot/+/78133/comment/455852b1_51ba28ca :
PS5, Line 110: strcpy(&str[strlen(str)], c_state_names[i]);
: strcpy(&str[strlen(str)], " ");
i'd just use printk directly instead of assembling a string and then printing it. the advantage of not assembling a string would be that that will eliminate the possibility of out of bounds writes
--
To view, visit https://review.coreboot.org/c/coreboot/+/78133?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I713712a1a104714cbf3091782e564e7e784cf21d
Gerrit-Change-Number: 78133
Gerrit-PatchSet: 5
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Mon, 02 Oct 2023 21:12:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph, Paul Menzel.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78193?usp=email )
Change subject: cpu/intel/model_206ax: Use haswell cstate_map
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/78193?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ie7076d11720d55a4ac11318cbbdab9f75d08e15e
Gerrit-Change-Number: 78193
Gerrit-PatchSet: 2
Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Mon, 02 Oct 2023 20:57:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment