coreboot-gerrit July 2020

coreboot-gerrit@coreboot.org

1 participants
3167 discussions

Change in coreboot[master]: soc/amd/picasso: Enable eSPI capability for Picasso
by Furquan Shaikh (Code Review) 07 Jul '20

07 Jul '20

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41075 ) Change subject: soc/amd/picasso: Enable eSPI capability for Picasso ...................................................................... soc/amd/picasso: Enable eSPI capability for Picasso This change selects SOC_AMD_COMMON_BLOCK_HAS_ESPI which enables the capability for using eSPI on Picasso. BUG=b:153675913,b:154445472 Change-Id: I4876f1bff4305a23e8ccc48a2d0d3b64cdc9703d Signed-off-by: Furquan Shaikh <furquan(a)google.com> --- M src/soc/amd/picasso/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/41075/1 diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 3d69966..a304819 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -29,6 +29,7 @@ select UDELAY_TSC select SOC_AMD_COMMON select SOC_AMD_COMMON_BLOCK + select SOC_AMD_COMMON_BLOCK_HAS_ESPI select SOC_AMD_COMMON_BLOCK_IOMMU select SOC_AMD_COMMON_BLOCK_ACPIMMIO select SOC_AMD_COMMON_BLOCK_BANKED_GPIOS -- To view, visit https://review.coreboot.org/c/coreboot/+/41075 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4876f1bff4305a23e8ccc48a2d0d3b64cdc9703d Gerrit-Change-Number: 41075 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: newchange

6 13

Change in coreboot[master]: armv7: mmu: Use 'tlbimva' to invalidate TLB entries
by Sam Lewis (Code Review) 07 Jul '20

07 Jul '20

Sam Lewis has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43137 ) Change subject: armv7: mmu: Use 'tlbimva' to invalidate TLB entries ...................................................................... armv7: mmu: Use 'tlbimva' to invalidate TLB entries The tlbimvaa operation (invalidate unified TLB by MVA, all address space identifiers) is only available on armv7 processors that support Multiprocessing Extensions. When used on processors that do not support the extensions it causes an "undefined instruction" exception. This patch changes the MMU table entry filling code to use the tlbimva (invalidate unified TLB entry by MVA and address space identifier) operation for invalidating TLB entries, which is supported on all armv7 processors. As address space identifiers are not used in TLB entries in coreboot (all entries are set as global), these two operations can safely be used interchangebly. The ASID value supplied to the operation is not checked for global TLB entries. More information as well as the data formats for the tlbimvaa and tlbimba operations are detailed in the "ARM Architecture Reference Manual ARMv7-A" edition, issue "C.c" page B4-1747. TEST: Booted Beaglebone Black (my current in progress port) Change-Id: Ie7dfb4adab20dc7eecb1b20aa2ee6355215a1521 Signed-off-by: Sam Lewis <sam.vr.lewis(a)gmail.com> --- M src/arch/arm/armv7/mmu.c M src/arch/arm/include/armv7/arch/cache.h 2 files changed, 8 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/43137/1 diff --git a/src/arch/arm/armv7/mmu.c b/src/arch/arm/armv7/mmu.c index d823c61..51b4860 100644 --- a/src/arch/arm/armv7/mmu.c +++ b/src/arch/arm/armv7/mmu.c @@ -118,7 +118,7 @@ /* Invalidate the TLB entries. */ for (i = start_idx; i < end_idx; i++) - tlbimvaa(offset + (i << shift)); + tlbimva(offset + (i << shift)); dsb(); isb(); } @@ -152,7 +152,7 @@ *pgd_entry = (pte_t)(uintptr_t)table | ATTR_NEXTLEVEL; dccmvac((uintptr_t)pgd_entry); dsb(); - tlbimvaa(start_addr); + tlbimva(start_addr); dsb(); isb(); diff --git a/src/arch/arm/include/armv7/arch/cache.h b/src/arch/arm/include/armv7/arch/cache.h index 600ec46..efd1a19 100644 --- a/src/arch/arm/include/armv7/arch/cache.h +++ b/src/arch/arm/include/armv7/arch/cache.h @@ -79,6 +79,12 @@ asm volatile ("mcr p15, 0, %0, c8, c7, 3" : : "r" (mva) : "memory"); } +/* invalidate unified TLB by MVA and ASID */ +static inline void tlbimva(unsigned long mva) +{ + asm volatile ("mcr p15, 0, %0, c8, c7, 1" : : "r" (mva) : "memory"); +} + /* write data access control register (DACR) */ static inline void write_dacr(uint32_t val) { -- To view, visit https://review.coreboot.org/c/coreboot/+/43137 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie7dfb4adab20dc7eecb1b20aa2ee6355215a1521 Gerrit-Change-Number: 43137 Gerrit-PatchSet: 1 Gerrit-Owner: Sam Lewis <sam.vr.lewis(a)gmail.com> Gerrit-MessageType: newchange

2 7

Change in coreboot[master]: libpayload: cbgfx: Replace bilinear resampling with Lanczos
by Julius Werner (Code Review) 07 Jul '20

07 Jul '20

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42792 ) Change subject: libpayload: cbgfx: Replace bilinear resampling with Lanczos ...................................................................... Patch Set 16: (3 comments) > Note that puff-cq might fail due to the size increase of depthcharge.elf. This happened to me when I tried to enable this with Groot UI, even with the lowered bmpblk screen resolution (CL:2282241). Hmm... how tightly did you pack it? On a Trogdor this takes about 1.3KB extra for me (about half of that would be the sine stuff), which is nothing compared to the 100+KB for the whole depthcharge image or for any individual hi_res language. In general, any libpayload/depthcharge patch could always increase your binary sizes a bit, so if your image is packed super tight there's always that risk. https://review.coreboot.org/c/coreboot/+/42792/15//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/42792/15//COMMIT_MSG@9 PS15, Line 9: This patch improves the image resampling (scaling) code in CBGFX to use : the Lanczos algorithm that is widely considered the "best" resampling : algorithm (e.g. also the first choice in Python's PIL library). > Do you have a test image, the differences can easily be seen with? Added https://review.coreboot.org/c/coreboot/+/42792/15//COMMIT_MSG@12 PS15, Line 12: and therefore slower than bilinear : resampling > It’d be great if you could provide some specific data: On <device> there is <1 ms> difference. […] Well, you can measure it by applying/reverting the patch. Added an example from my tests. https://review.coreboot.org/c/coreboot/+/42792/15/payloads/libpayload/drive… File payloads/libpayload/drivers/video/graphics.c: https://review.coreboot.org/c/coreboot/+/42792/15/payloads/libpayload/drive… PS15, Line 751: if (equals++ >= (SSZ * SSZ)) > Too many leading tabs - consider code refactoring Just to clarify, I'm not planning to address this unless someone feels very strongly about it. I can't have less indentation levels due to the amount of nested loops this needs, I think the part over here is still about as readable as it's gonna get, and trying to factor out inner parts of the loops into separate functions (with all the different state that would have to be passed through to there) would just make this harder to follow, I think. -- To view, visit https://review.coreboot.org/c/coreboot/+/42792 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idde6f61865bfac2801ee4fff40ac64e4ebddff1a Gerrit-Change-Number: 42792 Gerrit-PatchSet: 16 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 07 Jul 2020 21:31:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: build bot (Jenkins) <no-reply(a)coreboot.org> Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: libpayload: cbgfx: Replace bilinear resampling with Lanczos
by Julius Werner (Code Review) 07 Jul '20

07 Jul '20

Hello Hung-Te Lin, Shelley Chen, build bot (Jenkins), Yu-Ping Wu, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/42792 to look at the new patch set (#16). Change subject: libpayload: cbgfx: Replace bilinear resampling with Lanczos ...................................................................... libpayload: cbgfx: Replace bilinear resampling with Lanczos This patch improves the image resampling (scaling) code in CBGFX to use the Lanczos algorithm that is widely considered the "best" resampling algorithm (e.g. also the first choice in Python's PIL library). It is of course much more elaborate and therefore slower than bilinear resampling, but a lot of the difference can be made up with optimizations, and the resulting code was found to still produce acceptable speeds for existing Chrome OS UI use cases (on an Arm Cortex-A55 device, time to scale an image to 1101x593 went from ~88ms to ~275ms, a little over 3x slowdown). Nevertheless, if this should be too slow for anyone there's also an option to tune it down a little, but still much better than bilinear (same operation was ~170ms with this). Example images (scaled up by a factor of 7): Old (bilinear): https://i.imgur.com/ytr2n4Z.png New (Lanczos a=3): https://i.imgur.com/f0vKluM.png Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: Idde6f61865bfac2801ee4fff40ac64e4ebddff1a --- M payloads/libpayload/Kconfig M payloads/libpayload/drivers/video/graphics.c 2 files changed, 303 insertions(+), 85 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/92/42792/16 -- To view, visit https://review.coreboot.org/c/coreboot/+/42792 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idde6f61865bfac2801ee4fff40ac64e4ebddff1a Gerrit-Change-Number: 42792 Gerrit-PatchSet: 16 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: volteer: enable VBOOT_EC_EFS
by Nick Vaccaro (Code Review) 07 Jul '20

07 Jul '20

Nick Vaccaro has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/43044 ) Change subject: volteer: enable VBOOT_EC_EFS ...................................................................... Removed Code-Review+2 by Nick Vaccaro <nvaccaro(a)google.com> -- To view, visit https://review.coreboot.org/c/coreboot/+/43044 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie71bb169242b2511b4a4102d46a6981d72d07806 Gerrit-Change-Number: 43044 Gerrit-PatchSet: 1 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Daisuke Nojiri <dnojiri(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(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)users.sourceforge.net> Gerrit-MessageType: deleteVote

1 0

Change in coreboot[master]: drivers/intel/mipi_camera: Add guard variables to protect shared reso...
by Tim Wawrzynczak (Code Review) 07 Jul '20

07 Jul '20

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43003 ) Change subject: drivers/intel/mipi_camera: Add guard variables to protect shared resource ...................................................................... Patch Set 5: (14 comments) https://review.coreboot.org/c/coreboot/+/43003/5//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/43003/5//COMMIT_MSG@7 PS5, Line 7: drivers/intel/mipi_camera: Add guard variables to protect shared resource : : This change updates the mipi_camera driver to handle shared power resource : between multiple cameras. This is achieved by adding a guard variable and : methods manipulate the guard variable and guard the call to actual method : while enables or disables the resource. This protects the shared lines : from being enabled or disabled multiple times while the resources are : being used by multiple cameras. : reflow for 72 characters wide https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… File src/drivers/intel/mipi_camera/camera.c: https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 26: .cnt = 0 it should get initialized to 0 as it's a file-level static variable, so .cnt=0 shouldn't be required, just `static struct camera_resource_manager res_mgr;` https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 477: printk(BIOS_ERR, "Unsupported power operation: %x\n", type); : printk(BIOS_ERR, "OS camera driver will likely not work"); nit: this can be combined into one printk call: printk(BIOS_ERR, "Unsupported power operation: %x\n" "OS camera driver will likely not work", type); https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 485: if (action == ENABLE) { : snprintf(method_name, sizeof(method_name), ENABLE_METHOD_FORMAT, : res_index); : } else if (action == DISABLE) { : snprintf(method_name, sizeof(method_name), DISABLE_METHOD_FORMAT, : res_index); : } else { : snprintf(method_name, sizeof(method_name), UNKNOWN_METHOD_FORMAT, : res_index); : acpigen_write_debug_string("Unsupported action"); : printk(BIOS_ERR, "Unsupported resource action: %x\n", action); : } a switch statement may be more clear on intention here. Also, why not just return from the function if the action type is unknown? https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 502: void *res_config Sugnan, I'm not a big fan of the void* for res_config... Could we maybe work with a tagged anonymous union instead? e.g., struct resource_config { enum action_type action; enum ctrl_type type; union { struct clk_config *clk_conf; struct gpio_config *gpio_conf; }; }; Technically it's not as type-safe (b/c of the tagged union), but if you add a few functions for creating them and getting values out (make_resource_config, etc.), then it could work. I think it would greatly simplify this patch. WDYT? https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 502: uint8_t enum action_type https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 502: uint8_t enum ctrl_type https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 569: printk(BIOS_ERR, "Unsupported power operation: %x\n", seq->ops[i].type); : printk(BIOS_ERR, "OS camera driver will likely not work\n"); nit: you can squash this into one printk call https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 571: res_config = NULL; : break; : } : : if (res_config == NULL) : continue; just return from within the `default:` block, I think it's easier to see the intent. https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 582: printk(BIOS_ERR, "Unable to add guarded camera resource\n"); : printk(BIOS_ERR, "OS camera driver will likely not work\n"); nit: you can squash this into one printk call https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 596: acpigen_write_if(); : acpigen_emit_byte(LEQUAL_OP); : acpigen_emit_namestring(varname); : acpigen_write_integer(0); acpigen_write_if_lequal_namestr_int https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 602: : acpigen_emit_byte(ADD_OP); : acpigen_emit_namestring(varname); : acpigen_emit_byte(0x01); : acpigen_emit_namestring(varname); : acpigen_pop_len(); /* _ON */ there is an INCREMENT_OP 😊 https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 612: acpigen_emit_byte(SUBTRACT_OP); : acpigen_emit_namestring(varname); : acpigen_emit_byte(0x01); : acpigen_emit_namestring(varname); DECREMENT_OP https://review.coreboot.org/c/coreboot/+/43003/5/src/drivers/intel/mipi_cam… PS5, Line 616: acpigen_write_if(); : acpigen_emit_byte(LEQUAL_OP); : acpigen_emit_namestring(varname); : acpigen_write_integer(0); acpigen_write_if_lequal_namestr_int -- To view, visit https://review.coreboot.org/c/coreboot/+/43003 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1468459d5bbb2fb07bef4e0590c96dd4dbab0d9c Gerrit-Change-Number: 43003 Gerrit-PatchSet: 5 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 07 Jul 2020 20:58:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: vendorcode/google/chromeos: Add vendor_dptf_* functions
by Tim Wawrzynczak (Code Review) 07 Jul '20

07 Jul '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41894 ) Change subject: vendorcode/google/chromeos: Add vendor_dptf_* functions ...................................................................... vendorcode/google/chromeos: Add vendor_dptf_* functions This patch adds in the rest of the missing pieces from what is in the static DPTF ASL implementation. Much of it seems written for ChromeOS systems, making assumptions about \_SB.PCI0.LPCB.EC0.* methods that may exist on other systems. Seeing also that they were only used for ChromeOS devices in coreboot, the corresponding generation of the AML was moved to vendorcode/google/chromeos. BUG=b:143539650 TEST=compiles Change-Id: Ib30072d1d0748b31bcab240a0fd0e2f12d34aaa4 Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/vendorcode/google/chromeos/Makefile.inc A src/vendorcode/google/chromeos/dptf.c 2 files changed, 308 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/41894/1 diff --git a/src/vendorcode/google/chromeos/Makefile.inc b/src/vendorcode/google/chromeos/Makefile.inc index fc00374..936a298 100644 --- a/src/vendorcode/google/chromeos/Makefile.inc +++ b/src/vendorcode/google/chromeos/Makefile.inc @@ -13,6 +13,7 @@ ramstage-$(CONFIG_USE_SAR) += sar.c ramstage-$(CONFIG_CHROMEOS_DSM_CALIB) += dsm_calib.c ramstage-$(CONFIG_TPM_CR50) += cr50_enable_update.c +ramstage-$(CONFIG_DRIVERS_INTEL_DPTF) += dptf.c bootblock-y += watchdog.c verstage-y += watchdog.c diff --git a/src/vendorcode/google/chromeos/dptf.c b/src/vendorcode/google/chromeos/dptf.c new file mode 100644 index 0000000..cbf7b48 --- /dev/null +++ b/src/vendorcode/google/chromeos/dptf.c @@ -0,0 +1,307 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <acpi/acpigen.h> +#include <acpi/acpigen_dptf.h> +#include <device/device.h> + +/* DPTF Event types */ +enum { + TRIP_POINTS_CHANGED_EVENT = 0x81, + THERMAL_EVENT = 0x90, +}; + +/* EC constants */ +enum { + EC_FAN_DUTY_AUTO = 0xFF, +}; + +static void write_charger_PPPC(void) +{ + acpigen_write_method_serialized("PPPC", 0); + + /* + * Convert size of PPSS table to index + * + * Store (SizeOf (PPSS), Local0) + * Decrement (Local0) + */ + acpigen_write_store(); + acpigen_emit_byte(SIZEOF_OP); + acpigen_emit_namestring("PPSS"); + acpigen_emit_byte(LOCAL0_OP); + acpigen_emit_byte(DECREMENT_OP); + acpigen_emit_byte(LOCAL0_OP); + + /* + * Check if charging is disabled (AC removed) + * + * If (\_SB.PCI0.LPCB.EC0.ACEX () = Zero) { + * Return (Local0) + * } + */ + acpigen_write_if(); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_namestring("\\_SB.PCI0.LPCB.EC0.ACEX"); + acpigen_emit_byte(ZERO_OP); + acpigen_emit_byte(RETURN_OP); + acpigen_emit_byte(LOCAL0_OP); + acpigen_pop_len(); /* If */ + + /* + * Return highest power state + * + * Return (0) + */ + acpigen_emit_byte(RETURN_OP); + acpigen_emit_byte(ZERO_OP); + + acpigen_pop_len(); /* Method */ +} + +static void write_charger_SPPC(void) +{ + /* + * SPPC - Set charger current limit + * Method(SPPC, 1) { + * Store (DeRefOf (Index (DeRefOf (Index + * (PPSS, ToInteger (Arg0))), 4)), Local0) + * \_SB.PCI0.LPCB.EC0.CHGS (Local0) + * } + */ + + acpigen_write_method_serialized("SPPC", 1); + + /* Retrieve Control (index 4) for specified PPSS level */ + acpigen_emit_byte(STORE_OP); + acpigen_emit_byte(DEREF_OP); + acpigen_emit_byte(INDEX_OP); + acpigen_emit_byte(DEREF_OP); + acpigen_emit_byte(INDEX_OP); + acpigen_emit_namestring("PPSS"); + acpigen_write_to_integer(ARG0_OP, ZERO_OP); + acpigen_emit_byte(ZERO_OP); /* 3rd arg to Index */ + acpigen_write_integer(4); /* Index */ + acpigen_emit_byte(ZERO_OP); /* 3rd arg to Index */ + acpigen_emit_byte(LOCAL0_OP); + + /* Pass Control value to EC to limit charging */ + acpigen_emit_namestring("\\_SB.PCI0.LPCB.EC0.CHGS"); + acpigen_emit_byte(LOCAL0_OP); + acpigen_pop_len(); /* Method */ +} + +static void write_fan_fst(void) +{ + /* TFST is a package that is used to store data from FAND */ + acpigen_write_name("TFST"); + acpigen_write_package(3); + acpigen_write_integer(0); /* Revision */ + acpigen_write_integer(0); /* Control */ + acpigen_write_integer(0); /* Speed */ + acpigen_pop_len(); /* Package */ + + /* _FST */ + acpigen_write_method_serialized("_FST", 0); + acpigen_write_store(); + acpigen_emit_namestring("\\_SB.PCI0.LPCB.EC0.FAND"); + acpigen_emit_byte(INDEX_OP); + acpigen_emit_namestring("TFST"); + acpigen_write_integer(1); + acpigen_emit_byte(ZERO_OP); /* 3rd arg to Index */ + acpigen_emit_byte(RETURN_OP); + acpigen_emit_namestring("TFST"); + acpigen_pop_len(); /* Method _FST */ +} + +static void write_fan_fsl(void) +{ + /* _FSL */ + acpigen_write_method_serialized("_FSL", 1); + acpigen_write_store(); + acpigen_emit_byte(ARG0_OP); + acpigen_emit_namestring("\\_SB.PCI0.LPCB.EC0.FAND"); + acpigen_pop_len(); /* Method _FSL */ +} + +/* Note: requires manual insertion of acpigen_pop_len() for the If */ +static void write_is_policy_enabled(unsigned int index, bool enabled) +{ + /* + * If (And (LEqual (Deref (Index (IDSP, index)), Arg0)) + * (LEqual (Arg1, enabled))) + */ + acpigen_write_if(); + acpigen_emit_byte(LAND_OP); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_byte(DEREF_OP); + acpigen_emit_byte(INDEX_OP); + acpigen_emit_namestring("IDSP"); + acpigen_write_integer(index); + acpigen_emit_byte(ZERO_OP); /* 3rd arg of index - unused */ + acpigen_emit_byte(ARG0_OP); /* end lequal */ + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_byte(ARG1_OP); + acpigen_write_integer(enabled ? 1 : 0); +} + +static void write_dptf_OSC(bool active_policy_enabled) +{ + /* + * Arg0: Buffer containing UUID + * Arg1: "Integer containing Revision ID of buffer format", but Linux passes whether + * it is enabling (1) or disabling (0) the policy in Arg1. + * Arg2: Integer containing count of entries in Arg3 + * Arg3: Buffer containing list of DWORD capabilities + * Return: Buffer containing list of DWORD capabilities + */ + acpigen_write_method_serialized("_OSC", 4); + + /* Is the OS passing us the Passive Policy? */ + write_is_policy_enabled(0, true); + acpigen_emit_namestring("^TINI"); + acpigen_emit_namestring("^TCHG.INIT"); + acpigen_pop_len(); /* If */ + + /* If the Active Policy is disabled, disable DPTF fan control in the EC */ + if (active_policy_enabled) { + write_is_policy_enabled(2, false); + acpigen_write_store(); + acpigen_write_integer(EC_FAN_DUTY_AUTO); + acpigen_emit_namestring("\\_SB.PCI0.LPCB.EC0.FAND"); + acpigen_pop_len(); /* If */ + } + + acpigen_emit_byte(RETURN_OP); + acpigen_emit_byte(ARG3_OP); + acpigen_pop_len(); /* Method _OSC */ +} + +void vendor_dptf_write_dppm_methods(bool active_policy_enabled, bool tsr_en[4]) +{ + enum dptf_participant p; + char name[16]; + int i; + + acpigen_write_scope("\\_SB.DPTF"); + write_dptf_OSC(active_policy_enabled); + + /* TEVT */ + if (CONFIG(EC_SUPPORTS_DPTF_TEVT)) { + acpigen_write_method("TEVT", 0); + + /* Local0 = ToInteger(Arg0) */ + acpigen_write_to_integer(ARG0_OP, LOCAL0_OP); + for (p = DPTF_TSR0, i = 0; p <= DPTF_TSR3; ++p, ++i) { + if (!tsr_en[i]) + continue; + + snprintf(name, sizeof(name), "^TSR%1d", i); + + /* If (Local0 = i) Notify (TSRx, 0x90) */ + acpigen_write_if(); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_byte(LOCAL0_OP); + acpigen_write_integer(i); /* TMPI */ + acpigen_emit_byte(NOTIFY_OP); + acpigen_emit_namestring(name); + acpigen_write_integer(THERMAL_EVENT); + acpigen_pop_len(); /* If */ + } + + acpigen_pop_len(); /* Method */ + } + + /* TINI */ + acpigen_write_method("TINI", 0); + for (i = 0; i < DPTF_MAX_TSR; ++i) { + if (!tsr_en[i]) + continue; + snprintf(name, sizeof(name), "^TSR%1d.PATD", i); + acpigen_emit_namestring(name); + } + + acpigen_pop_len(); /* Method */ + + /* TPET */ + acpigen_write_method("TPET", 0); + for (p = DPTF_TSR0, i = 0; p <= DPTF_TSR3; ++p, ++i) { + if (!tsr_en[i]) + continue; + + snprintf(name, sizeof(name), "^TSR%1d", i); + acpigen_emit_byte(NOTIFY_OP); + acpigen_emit_namestring(name); + acpigen_write_integer(TRIP_POINTS_CHANGED_EVENT); + } + + acpigen_pop_len(); /* Method */ + acpigen_pop_len(); /* Scope */ +} + +static void write_charger_init(void) +{ + /* Initialize charger participant */ + acpigen_write_method_serialized("INIT", 0); + + /* Disable charge limit */ + acpigen_emit_namestring("\\_SB.PCI0.LPCB.EC0.CHGD"); + acpigen_pop_len(); /* Method */ +} + +void vendor_dptf_write_charger_methods(void) +{ + dptf_write_scope(DPTF_CHARGER); + + write_charger_PPPC(); + write_charger_SPPC(); + write_charger_init(); + + acpigen_pop_len(); /* Scope */ +} + +void vendor_dptf_write_fan_methods(void) +{ + dptf_write_scope(DPTF_FAN); + + write_fan_fsl(); + write_fan_fst(); + + acpigen_pop_len(); /* Scope */ +} + +void vendor_dptf_write_thermal_methods(int tsr_index) +{ + dptf_write_scope((enum dptf_participant)(DPTF_TSR0 + tsr_index)); + + /* _TMP - read temperature from EC */ + acpigen_write_method_serialized("_TMP", 0); + acpigen_emit_byte(RETURN_OP); + acpigen_emit_namestring("\\_SB.PCI0.LPCB.EC0.TSRD"); + acpigen_emit_namestring("TMPI"); + acpigen_pop_len(); /* Method _TMP */ + + /* PATC - Aux trip point count */ + acpigen_write_name_integer("PATC", 2); + + /* PAT0 - Set Aux trip point 0 */ + acpigen_write_method_serialized("PAT0", 1); + acpigen_emit_namestring("\\_SB.PCI0.LPCB.EC0.PAT0"); + acpigen_emit_namestring("TMPI"); + acpigen_emit_byte(ARG0_OP); + acpigen_pop_len(); /* Method PAT0 */ + + /* PAT1 - Set Aux trip point 1 */ + acpigen_write_method_serialized("PAT1", 1); + acpigen_emit_namestring("\\_SB.PCI0.LPCB.EC0.PAT1"); + acpigen_emit_namestring("TMPI"); + acpigen_emit_byte(ARG0_OP); + acpigen_pop_len(); /* Method PAT0 */ + + /* PATD - Disable Aux trip point */ + acpigen_write_method_serialized("PATD", 0); + acpigen_emit_namestring("\\_SB.PCI0.LPCB.EC0.PATD"); + acpigen_emit_namestring("TMPI"); + acpigen_pop_len(); /* Method PAT0 */ + + acpigen_pop_len(); /* Scope */ +} -- To view, visit https://review.coreboot.org/c/coreboot/+/41894 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib30072d1d0748b31bcab240a0fd0e2f12d34aaa4 Gerrit-Change-Number: 41894 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-MessageType: newchange

5 19

Change in coreboot[master]: acpigen: Add acpigen_notify
by Tim Wawrzynczak (Code Review) 07 Jul '20

07 Jul '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42102 ) Change subject: acpigen: Add acpigen_notify ...................................................................... acpigen: Add acpigen_notify A fairly common thing in ACPI is notifying a device when some kind of device-specific event happens; this function simplifies writing this pattern. Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: I0f18db9cc836ec9249604452f03ed9b4c6478827 --- M src/acpi/acpigen.c M src/include/acpi/acpigen.h 2 files changed, 10 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/42102/1 diff --git a/src/acpi/acpigen.c b/src/acpi/acpigen.c index efc5a16..d687a33 100644 --- a/src/acpi/acpigen.c +++ b/src/acpi/acpigen.c @@ -1953,3 +1953,10 @@ (((uint64_t)address->part_id & 0xffff) << 8) | (((uint64_t)address->class & 0xff))); } + +void acpigen_notify(const char *namestr, int value) +{ + acpigen_emit_byte(NOTIFY_OP); + acpigen_emit_namestring(namestr); + acpigen_write_integer(value); +} diff --git a/src/include/acpi/acpigen.h b/src/include/acpi/acpigen.h index dffde85..e0519ab 100644 --- a/src/include/acpi/acpigen.h +++ b/src/include/acpi/acpigen.h @@ -508,4 +508,7 @@ void acpigen_resource_qword(u16 res_type, u16 gen_flags, u16 type_flags, u64 gran, u64 range_min, u64 range_max, u64 translation, u64 length); +/* Emits Notify(namestr, value) */ +void acpigen_notify(const char *namestr, int value); + #endif /* __ACPI_ACPIGEN_H__ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/42102 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f18db9cc836ec9249604452f03ed9b4c6478827 Gerrit-Change-Number: 42102 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newchange

4 4

Change in coreboot[master]: soc/intel/common/block: Add new block DTT
by Tim Wawrzynczak (Code Review) 07 Jul '20

07 Jul '20

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41893 ) Change subject: soc/intel/common/block: Add new block DTT ...................................................................... soc/intel/common/block: Add new block DTT Intel Dynamic Tuning Technology is the name of a PCI device on some Intel SoCs. This minimal PCI driver is only used now for SSDT generation on TGL devices. Change-Id: Ib52f35e4e020ca3e6ab8b32cc3bf7df36041926e Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/include/device/pci_ids.h A src/soc/intel/common/block/dtt/Kconfig A src/soc/intel/common/block/dtt/Makefile.inc A src/soc/intel/common/block/dtt/dtt.c M src/soc/intel/tigerlake/Kconfig 5 files changed, 37 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/41893/1 diff --git a/src/include/device/pci_ids.h b/src/include/device/pci_ids.h index 4b17567..054088d 100644 --- a/src/include/device/pci_ids.h +++ b/src/include/device/pci_ids.h @@ -3714,6 +3714,9 @@ #define PCI_DEVICE_ID_GrP_6SERIES_1_WIFI 0x51f0 #define PCI_DEVICE_ID_GrP_6SERIES_2_WIFI 0x7af0 +/* Intel Dynamic Tuning Technology Device */ +#define PCI_DEVICE_ID_INTEL_TGL_DTT 0x9A03 + #define PCI_VENDOR_ID_COMPUTONE 0x8e0e #define PCI_DEVICE_ID_COMPUTONE_IP2EX 0x0291 #define PCI_DEVICE_ID_COMPUTONE_PG 0x0302 diff --git a/src/soc/intel/common/block/dtt/Kconfig b/src/soc/intel/common/block/dtt/Kconfig new file mode 100644 index 0000000..063c475 --- /dev/null +++ b/src/soc/intel/common/block/dtt/Kconfig @@ -0,0 +1,7 @@ +config SOC_INTEL_COMMON_BLOCK_DTT + bool + default n + help + Minimal PCI Driver for enabling SSDT generation of Intel + Dynamic Tuning Technology (DTT) policies and controls, also + known as Intel DPTF (Dynamic Platform and Thermal Framework) diff --git a/src/soc/intel/common/block/dtt/Makefile.inc b/src/soc/intel/common/block/dtt/Makefile.inc new file mode 100644 index 0000000..fccccf2 --- /dev/null +++ b/src/soc/intel/common/block/dtt/Makefile.inc @@ -0,0 +1,3 @@ +ramstage-$(CONFIG_SOC_INTEL_COMMON_BLOCK_DTT) += dtt.c + + diff --git a/src/soc/intel/common/block/dtt/dtt.c b/src/soc/intel/common/block/dtt/dtt.c new file mode 100644 index 0000000..025bbe5 --- /dev/null +++ b/src/soc/intel/common/block/dtt/dtt.c @@ -0,0 +1,23 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <device/device.h> +#include <device/pci.h> +#include <device/pci_ids.h> + +static const unsigned short pci_device_ids[] = { + PCI_DEVICE_ID_INTEL_TGL_DTT, +}; + +static struct device_operations dptf_dev_ops = { + .read_resources = pci_dev_read_resources, + .set_resources = pci_dev_set_resources, + .enable_resources = pci_dev_enable_resources, + .scan_bus = scan_generic_bus, + .ops_pci = &pci_dev_ops_pci, +}; + +static const struct pci_driver pch_dptf __pci_driver = { + .ops = &dptf_dev_ops, + .vendor = PCI_VENDOR_ID_INTEL, + .devices = pci_device_ids, +}; diff --git a/src/soc/intel/tigerlake/Kconfig b/src/soc/intel/tigerlake/Kconfig index e0d29fb..ab82ea7 100644 --- a/src/soc/intel/tigerlake/Kconfig +++ b/src/soc/intel/tigerlake/Kconfig @@ -44,6 +44,7 @@ select SOC_INTEL_COMMON_BLOCK_CHIP_CONFIG select SOC_INTEL_COMMON_BLOCK_CPU select SOC_INTEL_COMMON_BLOCK_CPU_MPINIT + select SOC_INTEL_COMMON_BLOCK_DTT select SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT select SOC_INTEL_COMMON_BLOCK_GSPI_VERSION_2 select SOC_INTEL_COMMON_BLOCK_HDA -- To view, visit https://review.coreboot.org/c/coreboot/+/41893 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib52f35e4e020ca3e6ab8b32cc3bf7df36041926e Gerrit-Change-Number: 41893 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

5 9

Change in coreboot[master]: volteer: enable VBOOT_EC_EFS
by Tim Wawrzynczak (Code Review) 07 Jul '20

07 Jul '20

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43044 ) Change subject: volteer: enable VBOOT_EC_EFS ...................................................................... Patch Set 1: Code-Review-1 this config option is deprecated, it is used for EFS v1, not v2, see b/157372086#comment12 I don't believe that a config option is required for EFS v2 -- To view, visit https://review.coreboot.org/c/coreboot/+/43044 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie71bb169242b2511b4a4102d46a6981d72d07806 Gerrit-Change-Number: 43044 Gerrit-PatchSet: 1 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Daisuke Nojiri <dnojiri(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(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)users.sourceforge.net> Gerrit-Comment-Date: Tue, 07 Jul 2020 19:42:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2020