Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/60190 )
Change subject: mb/google/dedede/var/bugzzy: Initialize display signals on user mode
......................................................................
mb/google/dedede/var/bugzzy: Initialize display signals on user mode
Bugzzy uses panel-built-in touch screen, it needs to set panel power
and reset signal to high for touch screen to work.
On user mode, coreboot doesn't initialize graphics since there is no
screen display before OS. So we would add a WA to initialize required
signals on user mode. It takes under 30 ms delay on booting time.
BUG=b:205496327
BRANCH=dedede
TEST=Verified touch screen worked with test coreboot
and test touch screen 028D firmware
Signed-off-by: Seunghwan Kim <sh_.kim(a)samsung.corp-partner.google.com>
Change-Id: Iaa4d16deb932f43ae1ab33ff5b4e74120ab670db
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60190
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Karthik Ramasubramanian <kramasub(a)google.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
---
M src/mainboard/google/dedede/variants/bugzzy/Makefile.inc
A src/mainboard/google/dedede/variants/bugzzy/ramstage.c
2 files changed, 60 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Karthik Ramasubramanian: Looks good to me, approved
Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/dedede/variants/bugzzy/Makefile.inc b/src/mainboard/google/dedede/variants/bugzzy/Makefile.inc
index fd60a18..7412194 100644
--- a/src/mainboard/google/dedede/variants/bugzzy/Makefile.inc
+++ b/src/mainboard/google/dedede/variants/bugzzy/Makefile.inc
@@ -1,5 +1,6 @@
## SPDX-License-Identifier: GPL-2.0-or-later
ramstage-y += gpio.c
+ramstage-y += ramstage.c
smm-y += variant.c
diff --git a/src/mainboard/google/dedede/variants/bugzzy/ramstage.c b/src/mainboard/google/dedede/variants/bugzzy/ramstage.c
new file mode 100644
index 0000000..7844b6e
--- /dev/null
+++ b/src/mainboard/google/dedede/variants/bugzzy/ramstage.c
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <bootstate.h>
+#include <arch/mmio.h>
+#include <delay.h>
+#include <bootmode.h>
+#include <device/device.h>
+#include <device/pci.h>
+#include <soc/pci_devs.h>
+#include <drivers/intel/gma/i915_reg.h>
+
+static void panel_power_on(uintptr_t igd_bar)
+{
+ setbits32((void *)(igd_bar + PCH_PP_CONTROL), PANEL_POWER_ON);
+}
+
+static void panel_reset_assert(uintptr_t igd_bar)
+{
+ clrsetbits32((void *)(igd_bar + PCH_GPIOB),
+ GPIO_CLOCK_VAL_OUT,
+ GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_DIR_OUT | GPIO_CLOCK_VAL_MASK);
+}
+
+static void panel_reset_deassert(uintptr_t igd_bar)
+{
+ const uint32_t data32 = GPIO_CLOCK_VAL_OUT |
+ GPIO_CLOCK_DIR_MASK | GPIO_CLOCK_DIR_OUT | GPIO_CLOCK_VAL_MASK;
+ setbits32((void *)(igd_bar + PCH_GPIOB), data32);
+}
+
+/*
+ * Bugzzy uses panel-built-in touch screen, it needs to set panel power and
+ * reset signal to high for touch screen to work.
+ * On user mode, coreboot doesn't initialize graphics since there is no screen
+ * display before OS. We would add this WA to initialize required signals on
+ * user mode.
+ */
+static void wa_init_display_signal(void *unused)
+{
+ struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD);
+ uintptr_t igd_bar;
+
+ if (display_init_required() || !igd_dev)
+ return;
+
+ igd_bar = find_resource(igd_dev, PCI_BASE_ADDRESS_0)->base;
+ if (!igd_bar)
+ return;
+
+ panel_power_on(igd_bar);
+ mdelay(20);
+ panel_reset_deassert(igd_bar);
+ mdelay(2);
+ panel_reset_assert(igd_bar);
+ mdelay(2);
+ panel_reset_deassert(igd_bar);
+}
+
+BOOT_STATE_INIT_ENTRY(BS_POST_DEVICE, BS_ON_ENTRY, wa_init_display_signal, NULL);
--
To view, visit https://review.coreboot.org/c/coreboot/+/60190
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaa4d16deb932f43ae1ab33ff5b4e74120ab670db
Gerrit-Change-Number: 60190
Gerrit-PatchSet: 15
Gerrit-Owner: shkim <sh_.kim(a)samsung.com>
Gerrit-Reviewer: Edward Doan <edoan(a)google.com>
Gerrit-Reviewer: Evan Green <evgreen(a)chromium.org>
Gerrit-Reviewer: Henry Sun <henrysun(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Simon Yang <simon1.yang(a)intel.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-CC: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-CC: Simon Yang <simon1.yang(a)intel.com>
Gerrit-MessageType: merged
Attention is currently required from: shkim, Henry Sun, Evan Green, SH Kim, Paul Menzel, Simon Yang, Edward Doan.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60190 )
Change subject: mb/google/dedede/var/bugzzy: Initialize display signals on user mode
......................................................................
Patch Set 14: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/60190
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaa4d16deb932f43ae1ab33ff5b4e74120ab670db
Gerrit-Change-Number: 60190
Gerrit-PatchSet: 14
Gerrit-Owner: shkim <sh_.kim(a)samsung.com>
Gerrit-Reviewer: Edward Doan <edoan(a)google.com>
Gerrit-Reviewer: Evan Green <evgreen(a)chromium.org>
Gerrit-Reviewer: Henry Sun <henrysun(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Simon Yang <simon1.yang(a)intel.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-CC: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-CC: Simon Yang <simon1.yang(a)intel.com>
Gerrit-Attention: shkim <sh_.kim(a)samsung.com>
Gerrit-Attention: Henry Sun <henrysun(a)google.com>
Gerrit-Attention: Evan Green <evgreen(a)chromium.org>
Gerrit-Attention: SH Kim <sh_.kim(a)samsung.corp-partner.google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Simon Yang <simon1.yang(a)intel.com>
Gerrit-Attention: Simon Yang <simon1.yang(a)intel.corp-partner.google.com>
Gerrit-Attention: Edward Doan <edoan(a)google.com>
Gerrit-Comment-Date: Mon, 03 Jan 2022 21:14:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Name of user not set #1004065, Stefan Reinauer.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60729 )
Change subject: util/ifdtool.h: util/ifdtool.c: Add additional regions for platforms that support them. Add Denverton platform support.
......................................................................
Patch Set 1:
(1 comment)
File util/ifdtool/ifdtool.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-137105):
https://review.coreboot.org/c/coreboot/+/60729/comment/c8e20316_f9728581
PS1, Line 43: { "Device Expansion 1", "devexp", "flashregion_5_device_expansion.bin", "SI_DEV_EXP1" },
line over 96 characters
--
To view, visit https://review.coreboot.org/c/coreboot/+/60729
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic8a4f26d833b1e628b7a72518d0a27252ec62f7c
Gerrit-Change-Number: 60729
Gerrit-PatchSet: 1
Gerrit-Owner: Name of user not set #1004065
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Name of user not set #1004065
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Comment-Date: Mon, 03 Jan 2022 20:55:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Paul Menzel, Tim Wawrzynczak, Angel Pons.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60735 )
Change subject: mb/google/sarien/var/sarien: Add VBT from Chromebook firmware
......................................................................
Patch Set 1:
(2 comments)
Patchset:
PS1:
> Where are the VBTs in the Chromium repositories?
not in a public repo unfortunately
PS1:
might as well add for ARCADA too, and then select INTEL_GMA_HAVE_VBT in Kconfig
--
To view, visit https://review.coreboot.org/c/coreboot/+/60735
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibab8a7b0b3f721ca434ac38b51528b81e66f3bb7
Gerrit-Change-Number: 60735
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 03 Jan 2022 20:41:28 +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: Felix Singer, Matt DeVillier, Tim Wawrzynczak, Angel Pons.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60735 )
Change subject: mb/google/sarien/var/sarien: Add VBT from Chromebook firmware
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Where are the VBTs in the Chromium repositories?
--
To view, visit https://review.coreboot.org/c/coreboot/+/60735
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibab8a7b0b3f721ca434ac38b51528b81e66f3bb7
Gerrit-Change-Number: 60735
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 03 Jan 2022 20:31:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Tim Wawrzynczak, Paul Menzel, Ravindra, Sridhar Siricilla, Subrata Banik, Arthur Heymans, Michael Niewöhner, Patrick Rudolph.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59359 )
Change subject: soc/intel/common: Implement ACPI CPPCv3 package to support hybrid core
......................................................................
Patch Set 10:
(1 comment)
File src/soc/intel/common/block/acpi/cpu_hybrid.c:
https://review.coreboot.org/c/coreboot/+/59359/comment/65d82019_b249bbf5
PS3, Line 102: acpi_get_cpu_hybrid_info(&cpu_hybrid_info);
: acpigen_write_method(XPPC_PACKAGE_NAME, 1);
:
: if (cpu_is_nominal_freq_supported()) {
:
: /*
: * If Nominal Frequency is supported, update Nominal Frequency and set core's
: * Nominal Performance to a scaling factor which depends on core type(big or
: * small).
: */
: acpigen_set_package_element_int(CPPC_PACKAGE_NAME, 22,
: cpu_hybrid_info.nominal_freq);
:
: /* LOCAL2_OP = (CORE >> core_id) & 1 */
: acpigen_emit_byte(SHIFT_RIGHT_OP);
: acpigen_write_integer(cpu_hybrid_info.type_mask);
: acpigen_emit_byte(ARG0_OP);
: acpigen_emit_byte(LOCAL1_OP);
: acpigen_write_and(LOCAL1_OP, ONE_OP, LOCAL2_OP);
:
: /*
: * If core id is big Core, then set big core's scaling factor
: * to core's nominal frequency otherwise set small core's scaling factor.
: */
: acpigen_write_if_lequal_op_int(LOCAL2_OP, 1);
: acpigen_set_package_element_int(CPPC_PACKAGE_NAME, 3,
: cpu_hybrid_info.big_core_nom_perf);
: acpigen_write_else();
: acpigen_set_package_element_int(CPPC_PACKAGE_NAME, 3,
: cpu_hybrid_info.small_core_nom_perf);
: acpigen_pop_len();
: }
:
: acpigen_emit_byte(RETURN_OP);
: acpigen_emit_namestring(CPPC_PACKAGE_NAME);
: acpigen_pop_len();
> Implementing CPPCv3 package generation in the SSDT gives more flexibility. For example, CPPCv3 generation is not required for all the SoC variants (ADL-N/ADL-P) within the same CPU family(ADL). So, the CPPCv3 generation can be controlled through SoC function cpu_is_nominal_freq_supported() based on the SoC variant.
I don't see the code doing that.
Also why would implementing it fully in SSDT be more flexible than a DSDT implemenation consuming SSDT namespace (using the external keyword)?
--
To view, visit https://review.coreboot.org/c/coreboot/+/59359
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icd5ea9e70bebd1e66d3cea2bcf8a6678e5cc95ca
Gerrit-Change-Number: 59359
Gerrit-PatchSet: 10
Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Ravindra <ravindra(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com>
Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ravindra <ravindra(a)intel.com>
Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Mon, 03 Jan 2022 20:16:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Heymans <arthur.heymans(a)9elements.com>
Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com>
Gerrit-MessageType: comment