coreboot-gerrit March 2022

coreboot-gerrit@coreboot.org

1 participants
6700 discussions

Change in coreboot[master]: drivers/intel/fsp2_0: Add native implementation for FSP Debug Handler
by Subrata Banik (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Arthur Heymans, Andrey Petrov. Hello build bot (Jenkins), Tim Wawrzynczak, Christian Walter, Arthur Heymans, Andrey Petrov, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63007 to look at the new patch set (#3). Change subject: drivers/intel/fsp2_0: Add native implementation for FSP Debug Handler ...................................................................... drivers/intel/fsp2_0: Add native implementation for FSP Debug Handler This patch implements coreboot native debug handler to manage the FSP event messages. `FSP Event Handlers` feature introduced in FSP to generate event messages to aid in the debugging of firmware issues. This eliminates the need for FSP to directly write debug messages to the UART and FSP might not need to know the board related UART port configuration. Instead FSP signals the bootloader to inform it of a new debug message. This allows the coreboot to provide board specific methods of reporting debug messages, example: legacy UART or LPSS UART etc. This implementation has several advantages as: 1. FSP relies on XIP `DebugLib` driver even while printing FSP-S debug messages, hence, without ROM being cached, post `romstage` would results into sluggish boot with FSP debug enabled. This patch utilities coreboot native debug implementation which is XIP during FSP-M and relocatable to DRAM based resource for FSP-S. 2. This patch simplifies the FSP DebugLib implementation and remove the need to have serial port library. Instead coreboot `printk` can be used for display FSP serial messages. Additionally, unifies the debug library between coreboot and FSP. 3. This patch is also useful to get debug prints even with FSP non-serial image (refer to `Note` below) as FSP PEIMs are now leveraging coreboot debug library instead FSP `NULL` DebugLib reference for release build. 4. Can optimize the FSP binary size by removing the DebugLib dependency from most of FSP PEIMs, for example: on Alder Lake FSP-M debug binary size is reduced by ~100KB+ and FSP-S debug library size is also reduced by ~300KB+ (FSP-S debug and release binary size is exactly same with this code changes). The total savings is ~400KB for each FSP copy, and in case of Chrome AP firmware with 3 copies, the total savings would be 400KB * 3 = ~1.2MB. Note: Need to modify FSP source code to remove `MDEPKG_NDEBUG` as compilation flag for release build and generate FSP binary with non-NULL FSP debug wrapper module injected (to allow FSP event handler to execute even with FSP non-serial image) in the final FSP.fd. BUG=b:225544587 TEST=Able to build and boot brya. Also, verified the FSP debug log is exactly same before and with this code change. Signed-off-by: Subrata Banik <subratabanik(a)google.com> Change-Id: I1018e67d70492b18c76531f9e78d3b58fa435cd4 --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/Makefile.inc A src/drivers/intel/fsp2_0/fsp_debug_event.c A src/drivers/intel/fsp2_0/include/fsp/fsp_debug_event.h 4 files changed, 56 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/63007/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/63007 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1018e67d70492b18c76531f9e78d3b58fa435cd4 Gerrit-Change-Number: 63007 Gerrit-PatchSet: 3 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: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: device: change set_resources to WARNING
by Paul Menzel (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Terry Chen, Tim Wawrzynczak, Nick Vaccaro. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63078 ) Change subject: device: change set_resources to WARNING ...................................................................... Patch Set 3: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63078/comment/fb7b3c82_e7ad3bb1 PS3, Line 7: device: change set_resources to WARNING Maybe: > device: Demote missing set resources message from error to warnings https://review.coreboot.org/c/coreboot/+/63078/comment/e5740fc3_56bb5802 PS3, Line 9: While running power_Resume/control that shows "NONE missing set_resources". The message does seem more like a warning , therefore switch to BIOS_WARN instead. 1. Please wrap lines after 72 characters. 2. Please remove the space before the comma. https://review.coreboot.org/c/coreboot/+/63078/comment/76b136f0_b21070af PS3, Line 9: shows shows the error -- To view, visit https://review.coreboot.org/c/coreboot/+/63078 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie7e7923265c84999b97a075f0757e7a8d8963b2d Gerrit-Change-Number: 63078 Gerrit-PatchSet: 3 Gerrit-Owner: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anfernee Chen <anfernee_chen(a)wistron.corp-partner.google.com> Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-CC: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com> Gerrit-CC: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Raihow Shi <raihow_shi(a)wistron.corp-partner.google.com> Gerrit-CC: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-CC: Zoey Wu <zoey_wu(a)wistron.corp-partner.google.com> Gerrit-Attention: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Thu, 24 Mar 2022 08:10:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Enable FSP_DEBUG_EVENT_HANDLER_IN_COREBOOT Kconfig
by Subrata Banik (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Tim Wawrzynczak, Arthur Heymans, Eric Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63009 ) Change subject: soc/intel/alderlake: Enable FSP_DEBUG_EVENT_HANDLER_IN_COREBOOT Kconfig ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/alderlake/Kconfig: https://review.coreboot.org/c/coreboot/+/63009/comment/5c63d9b8_ee14da99 PS2, Line 43: select FSP_DEBUG_EVENT_HANDLER_IN_COREBOOT > > > Can we depend CONFIG_CONSOLE_SERIAL or CONFIG_UART_DEBUG? I think the normal image, we don't need to know FSP log. > > > > Isn't the whole point to be console agnostic and let coreboot do the console printing wherever it seems fit? Even without series I still see value in printing output to for instance the cbmem console. > > Unless u have debug FSP image in repository, you won't see the FSP debug log with normal image. But I understood your point, and that can only happen when we have unified FSP binary which is debug always then, it's all about coreboot, how coreboot decides to control the serial log enable/disable. i'm working on a further change list to keep a reduced debug binary so we don't need to have separate FSP build flag while debugging any issue, rather can use coreboot to enable the FSP debug in verbose mode. @Eric, marking this resolved. please feel free to reopen if u think otherwise? -- To view, visit https://review.coreboot.org/c/coreboot/+/63009 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4a0530a282657e379a00c3e7d0ed8148dd5e9196 Gerrit-Change-Number: 63009 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 07:57:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: device: change set_resources to WARNING
by Terry Chen (Code Review) 24 Mar '22

24 Mar '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63078 to look at the new patch set (#3). Change subject: device: change set_resources to WARNING ...................................................................... device: change set_resources to WARNING While running power_Resume/control that shows "NONE missing set_resources". The message does seem more like a warning , therefore switch to BIOS_WARN instead. BUG=b:223936777 TEST=USE="project_primus" emerge-brya coreboot chromeos-bootimage test_that -b {BOARD_NAME} {device IP} f:.*power_Resume/control Signed-off-by: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Change-Id: Ie7e7923265c84999b97a075f0757e7a8d8963b2d --- M src/device/device.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/63078/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/63078 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie7e7923265c84999b97a075f0757e7a8d8963b2d Gerrit-Change-Number: 63078 Gerrit-PatchSet: 3 Gerrit-Owner: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: device: change set_resources to WARNING
by Terry Chen (Code Review) 24 Mar '22

24 Mar '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63078 to look at the new patch set (#2). Change subject: device: change set_resources to WARNING ...................................................................... device: change set_resources to WARNING While running power_Resume/control that shows "NONE missing set_resources". The message does seem more like a warning , therefore switch to BIOS_WARN instead. BUG=b:223936777 TEST=USE="project_primus" emerge-brya coreboot chromeos-bootimage test_that -b {BOARD_NAME} {device IP} f:.*power_Resume/control Signed-off-by: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Change-Id: Ie7e7923265c84999b97a075f0757e7a8d8963b2d --- M src/device/device.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/63078/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63078 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie7e7923265c84999b97a075f0757e7a8d8963b2d Gerrit-Change-Number: 63078 Gerrit-PatchSet: 2 Gerrit-Owner: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: drivers/intel/fsp2_0: Add native implementation for FSP Debug Handler
by Subrata Banik (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Arthur Heymans, Andrey Petrov. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63007 ) Change subject: drivers/intel/fsp2_0: Add native implementation for FSP Debug Handler ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/63007/comment/c4b5e064_164e4cfd PS2, Line 29: serialport > nit: […] Ack File src/drivers/intel/fsp2_0/Kconfig: https://review.coreboot.org/c/coreboot/+/63007/comment/c69d5270_d11d4ade PS2, Line 355: _IN_COREBOOT > nit: this part of the name seems redundant 😊 Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/63007 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1018e67d70492b18c76531f9e78d3b58fa435cd4 Gerrit-Change-Number: 63007 Gerrit-PatchSet: 2 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: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 07:43:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: device: change set_resources to WARNING
by Terry Chen (Code Review) 24 Mar '22

24 Mar '22

Terry Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63078 ) Change subject: device: change set_resources to WARNING ...................................................................... device: change set_resources to WARNING BUG=b:223936777 TEST=USE="project_primus" emerge-brya coreboot chromeos-bootimage Signed-off-by: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Change-Id: Ie7e7923265c84999b97a075f0757e7a8d8963b2d --- M src/device/device.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/63078/1 diff --git a/src/device/device.c b/src/device/device.c index 8337d55..f860b49 100644 --- a/src/device/device.c +++ b/src/device/device.c @@ -277,7 +277,7 @@ continue; if (!curdev->ops || !curdev->ops->set_resources) { - printk(BIOS_ERR, "%s missing set_resources\n", + printk(BIOS_WARNING, "%s missing set_resources\n", dev_path(curdev)); continue; } -- To view, visit https://review.coreboot.org/c/coreboot/+/63078 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie7e7923265c84999b97a075f0757e7a8d8963b2d Gerrit-Change-Number: 63078 Gerrit-PatchSet: 1 Gerrit-Owner: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: ec/google: Add PSRC and PBOK method
by Varshit B Pandya (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Subrata Banik, Tim Wawrzynczak, Rizwan Qureshi. Varshit B Pandya has removed Felix Held from this change. ( https://review.coreboot.org/c/coreboot/+/62945 ) Change subject: ec/google: Add PSRC and PBOK method ...................................................................... Removed cc Felix Held. -- To view, visit https://review.coreboot.org/c/coreboot/+/62945 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia536a9e1528f6e415247eca9ed2b0b685eb73196 Gerrit-Change-Number: 62945 Gerrit-PatchSet: 4 Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Poornima Tom <poornima.tom(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-MessageType: deleteReviewer

1 0

Change in coreboot[master]: drivers/intel/fsp2_0: Add native implementation for FSP Debug Handler
by Subrata Banik (Code Review) 24 Mar '22

24 Mar '22

Attention is currently required from: Arthur Heymans, Andrey Petrov. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63007 ) Change subject: drivers/intel/fsp2_0: Add native implementation for FSP Debug Handler ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Fun fact: the USF spec thinks this a bad idea: https://universalscalablefirmware.github.io/documentation/6_debug.html#trac…. It says the Soc Abstraction Layer (basically FSP) should be in charge of selecting where console gets outputted. I guess the term BIOS being used their is misleading, because the definition on BIOS is now evolved over time and unless specified some thing like silicon ref aka FSP or platform code aka coreboot, this is not clear when someone says "BIOS shall support sending trace messages to all BIOS accessible HW interfaces on the platform.' I can replace BIOS with coreboot here for my easy understanding and say "yes, coreboot would like to support sending traces to coreboot accessible HW interface for the platform." -- To view, visit https://review.coreboot.org/c/coreboot/+/63007 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1018e67d70492b18c76531f9e78d3b58fa435cd4 Gerrit-Change-Number: 63007 Gerrit-PatchSet: 2 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: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Mar 2022 07:23:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: driver/wifi: Add _DSM method for DDRRFIM
by Subrata Banik (Code Review) 24 Mar '22

24 Mar '22

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/61020 ) Change subject: driver/wifi: Add _DSM method for DDRRFIM ...................................................................... driver/wifi: Add _DSM method for DDRRFIM coreboot needs to propagate the CnviDdrRfim value info of the feature enable/disable state into the CNVi via the WiFi DSM ACPI object. This will be consumed by the Wi-Fi driver and it will act according to CB enablement configuration. This patch adds _DSM method for that. Add support for following 2 functions in _DSM method - Function 0: Function Support Query Returns a bitmask of functions supported. - Function 3: RFI enablement 0 Feature Enable 1 Feature Disable Note: Wifi Dsm already has provision for SAR. This patch will add additional support to return RFIM structure based on UUID. BUG=b:201724512 TEST=Build, boot brya0 and dump SSDT entries Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method { ToBuffer (Arg0, Local0) If ((Local0 == ToUUID ("7266172c-220b-4b29-814f-75e4dd26b5fd"))) { ToInteger (Arg2, Local1) If ((Local1 == Zero)) { Return (Buffer (One) { 0x09 }) } If ((Local1 == One)){} If ((Local1 == 0x02)){} If ((Local1 == 0x03)) { Return (Zero) } Return (Buffer (One) { 0x00 }) } Return (Buffer (One) { 0x00 }) } Signed-off-by: Varshit B Pandya <varshit.b.pandya(a)intel.com> Change-Id: I217b736df3d4224a6732d1941a160abcddbd8f37 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61020 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Reviewed-by: Ronak Kanabar <ronak.kanabar(a)intel.com> Reviewed-by: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> --- M src/drivers/wifi/generic/acpi.c 1 file changed, 66 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Maulik V Vaghela: Looks good to me, approved Ronak Kanabar: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/drivers/wifi/generic/acpi.c b/src/drivers/wifi/generic/acpi.c index ef18f6d..65940d6 100644 --- a/src/drivers/wifi/generic/acpi.c +++ b/src/drivers/wifi/generic/acpi.c @@ -10,11 +10,15 @@ #include <wrdd.h> #include "chip.h" +#include "wifi.h" #include "wifi_private.h" /* WIFI Domain type */ #define DOMAIN_TYPE_WIFI 0x7 +/* Maximum number DSM UUID bifurcations in _DSM */ +#define MAX_DSM_FUNCS 2 + /* * WIFI ACPI NAME = "WF" + hex value of last 8 bits of dev_path_encode + '\0' * The above representation returns unique and consistent name every time @@ -29,6 +33,9 @@ /* ID for the Wifi DmaProperty _DSD */ #define ACPI_DSD_DMA_PROPERTY_UUID "70D24161-6DD5-4C9E-8070-705531292865" +/* Unique ID for CnviDdrRfim entry in WIFI _DSM */ +#define ACPI_DSM_RFIM_WIFI_UUID "7266172C-220B-4B29-814F-75E4DD26B5FD" + __weak int get_wifi_sar_limits(union wifi_sar_limits *sar_limits) { return -1; @@ -145,6 +152,12 @@ acpigen_write_return_integer(dsm_config->unii_4); } +static void wifi_dsm_ddrrfim_func3_cb(void *ptr) +{ + const bool is_cnvi_ddr_rfim_enabled = *(bool *)ptr; + acpigen_write_return_integer(is_cnvi_ddr_rfim_enabled ? 1 : 0); +} + static void (*wifi_dsm_callbacks[])(void *) = { NULL, /* Function 0 */ wifi_dsm_srd_active_channels, /* Function 1 */ @@ -156,6 +169,17 @@ wifi_dsm_unii4_control_enable, /* Function 7 */ }; +/* + * The current DSM2 table is only exporting one function (function 3), some more + * functions are reserved so marking them NULL. +*/ +static void (*wifi_dsm2_callbacks[])(void *) = { + NULL, /* Function 0 */ + NULL, /* Function 1 */ + NULL, /* Function 2 */ + wifi_dsm_ddrrfim_func3_cb, /* Function 3 */ +}; + void wifi_emit_dsm(struct dsm_profile *dsm) { int i; @@ -462,9 +486,9 @@ acpigen_write_package_end(); } -static void emit_sar_acpi_structures(const struct device *dev) +static void emit_sar_acpi_structures(const struct device *dev, struct dsm_profile *dsm) { - union wifi_sar_limits sar_limits; + union wifi_sar_limits sar_limits = {{NULL, NULL, NULL, NULL, NULL} }; /* * If device type is PCI, ensure that the device has Intel vendor ID. CBFS SAR and SAR @@ -484,7 +508,10 @@ sar_emit_wgds(sar_limits.wgds); sar_emit_ppag(sar_limits.ppag); sar_emit_wtas(sar_limits.wtas); - wifi_emit_dsm(sar_limits.dsm); + + /* copy the dsm data to be later used for creating _DSM function */ + if (sar_limits.dsm != NULL) + memcpy(dsm, &sar_limits.dsm, sizeof(struct dsm_profile)); free(sar_limits.sar); } @@ -506,11 +533,16 @@ static void wifi_ssdt_write_properties(const struct device *dev, const char *scope) { + bool is_cnvi_ddr_rfim_enabled = false; + + const struct drivers_wifi_generic_config *config = dev->chip_info; + if (dev && config) + is_cnvi_ddr_rfim_enabled = config->enable_cnvi_ddr_rfim; + /* Scope */ acpigen_write_scope(scope); if (dev->path.type == DEVICE_PATH_GENERIC) { - const struct drivers_wifi_generic_config *config = dev->chip_info; if (config) { /* Wake capabilities */ acpigen_write_PRW(config->wake, ACPI_S3); @@ -549,9 +581,37 @@ acpigen_pop_len(); } + struct dsm_uuid dsm_ids[MAX_DSM_FUNCS]; + /* We will need a copy dsm data to be used later for creating _DSM function */ + struct dsm_profile dsm = {0}; + uint8_t dsm_count = 0; + /* Fill Wifi sar related ACPI structures */ - if (CONFIG(USE_SAR)) - emit_sar_acpi_structures(dev); + if (CONFIG(USE_SAR)) { + emit_sar_acpi_structures(dev, &dsm); + + if (dsm.supported_functions != 0) { + for (int i = 1; i < ARRAY_SIZE(wifi_dsm_callbacks); i++) + if (!(dsm.supported_functions & (1 << i))) + wifi_dsm_callbacks[i] = NULL; + + dsm_ids[dsm_count].uuid = ACPI_DSM_OEM_WIFI_UUID; + dsm_ids[dsm_count].callbacks = &wifi_dsm_callbacks[0]; + dsm_ids[dsm_count].count = ARRAY_SIZE(wifi_dsm_callbacks); + dsm_ids[dsm_count].arg = NULL; + dsm_count++; + } + } + + if (is_cnvi_ddr_rfim_enabled) { + dsm_ids[dsm_count].uuid = ACPI_DSM_RFIM_WIFI_UUID; + dsm_ids[dsm_count].callbacks = &wifi_dsm2_callbacks[0]; + dsm_ids[dsm_count].count = ARRAY_SIZE(wifi_dsm2_callbacks); + dsm_ids[dsm_count].arg = &is_cnvi_ddr_rfim_enabled; + dsm_count++; + } + + acpigen_write_dsm_uuid_arr(dsm_ids, dsm_count); acpigen_pop_len(); /* Scope */ 24 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/coreboot/+/61020 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I217b736df3d4224a6732d1941a160abcddbd8f37 Gerrit-Change-Number: 61020 Gerrit-PatchSet: 27 Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2022