Attention is currently required from: Tim Crawford, Nico Huber, Furquan Shaikh, Jeremy Soller, Tim Wawrzynczak, Paul Menzel, Michael Niewöhner.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57034 )
Change subject: drivers/gfx/nvidia: Add driver for NVIDIA Optimus
......................................................................
Patch Set 2:
(1 comment)
File src/drivers/gfx/nvidia/acpi/gpu.asl:
https://review.coreboot.org/c/coreboot/+/57034/comment/5fef285d_84ac05e7
PS2, Line 37: // Not sure what this stuff is, but it is used to get into GC6
> I think this should be based on the datasheet for the dGPU part and/or datasheet for the Intel platf […]
I more or less know what this is doing, but I don't know why
--
To view, visit https://review.coreboot.org/c/coreboot/+/57034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2dec7aa2c8db7994f78a7cc1220502676e248465
Gerrit-Change-Number: 57034
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Sun, 22 Aug 2021 06:14:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Jeff Chase, Tim Wawrzynczak, Edward O'Callaghan.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57037 )
Change subject: mb/google/hatch: don't override variant_ramstage_init() in baseboard
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Alternatively, you can also:
>
> 1. Add two separate files: mainboard_hatch.c and mainboard_puff.c in variants/baseboard/hatch. These can be included based on CONFIG_BOARD_GOOGLE_BASEBOARD_HATCH and CONFIG_BOARD_GOOGLE_BASEBOARD_PUFF.
>
> 2. Each of these mainboard file will provide a weak definition of `variant_ramstage_init()`. For hatch, it will be empty. For puff, it will do what it is currently doing i.e. power limit + other configuration.
>
> 3. For the puff variants that you care about doing things differently, you can add non-weak implementation of `variant_ramstage_init()`.
+1
--
To view, visit https://review.coreboot.org/c/coreboot/+/57037
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic5e787c3d4c2f2c62a0dc868997b6e4c3da84c43
Gerrit-Change-Number: 57037
Gerrit-PatchSet: 1
Gerrit-Owner: Jeff Chase <jnchase(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Joe Tessler <jrt(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Jeff Chase <jnchase(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Sun, 22 Aug 2021 06:11:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com>
Comment-In-Reply-To: Jeff Chase <jnchase(a)google.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Tim Crawford, Nico Huber, Jeremy Soller, Tim Wawrzynczak, Paul Menzel, Angel Pons, Michael Niewöhner.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57034 )
Change subject: drivers/gfx/nvidia: Add driver for NVIDIA Optimus
......................................................................
Patch Set 2:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57034/comment/3ba4a0c5_f68f5554
PS2, Line 9: NVIDIA Optimus (hybrid) graphics
Is there a specific part #?
https://review.coreboot.org/c/coreboot/+/57034/comment/1055b5e7_c901c954
PS2, Line 12:
Is there a doc# for the datasheet that details the requirements? I think it would be helpful to add that information to commit message.
File src/drivers/gfx/nvidia/Kconfig:
https://review.coreboot.org/c/coreboot/+/57034/comment/5dae87fb_1916cfe8
PS2, Line 9: default 0x01
> Can't this be determined at runtime from the parent device in the devicetree (should be the PCIe RP […]
Yes, this should not be a Kconfig.
File src/drivers/gfx/nvidia/acpi/gpu.asl:
https://review.coreboot.org/c/coreboot/+/57034/comment/d5372bf4_4ff69447
PS2, Line 3: Device (\_SB.PCI0.PEGP) {
I think this should be generated using soc/intel/common/block/pcie/rtd3. That driver is supposed to generate the ACPI nodes required for power management for PCIe devices.
https://review.coreboot.org/c/coreboot/+/57034/comment/2d19d478_20928694
PS2, Line 37: // Not sure what this stuff is, but it is used to get into GC6
I think this should be based on the datasheet for the dGPU part and/or datasheet for the Intel platform. Also as Angel and Tim commented, this should be done as part of SSDT so that the resource/bus allocation at runtime can be taken into account. Currently, it is not really clear what this code is doing.
https://review.coreboot.org/c/coreboot/+/57034/comment/2b8b4c0b_2a6fab43
PS2, Line 39: CONFIG_MMCONF_BASE_ADDRESS + 0x8000
> > you mean an SSDT callback? `.acpi_fill_ssdt` ? +1 to this. […]
+1 to SSDT.
File src/drivers/gfx/nvidia/chip.h:
https://review.coreboot.org/c/coreboot/+/57034/comment/1954aec3_2bd5e17c
PS2, Line 7: /* TODO: Set GPIOs in devicetree? */
> It would also be very useful to generate the Optimus ACPI code using the SSDT generator. […]
+1 for SSDT.
File src/drivers/gfx/nvidia/nvidia.c:
https://review.coreboot.org/c/coreboot/+/57034/comment/c927ce85_4431be81
PS2, Line 13:
: // Find all BARs on GPU, mark them above 4g if prefetchable
: for (int bar = PCI_BASE_ADDRESS_0; bar <= PCI_BASE_ADDRESS_5; bar += 4) {
: struct resource *res = probe_resource(dev, bar);
:
: if (res) {
: if (res->flags & IORESOURCE_PREFETCH) {
: printk(BIOS_INFO, " BAR at 0x%02x marked above 4g\n", bar);
: res->flags |= IORESOURCE_ABOVE_4G;
: } else {
: printk(BIOS_DEBUG, " BAR at 0x%02x not prefetch\n", bar);
: }
: } else {
: printk(BIOS_DEBUG, " BAR at 0x%02x not found\n", bar);
: }
: }
: }
> Does it make sense to select `PCIEXP_HOTPLUG_PREFETCH_MEM_ABOVE_4G` instead when using the NVIDIA dr […]
Do we also need to consider whether mainboard wants to enable the dGPU pre-OS?
1. If it is required pre-OS, then yes PCIEXP_HOTPLUG_PREFETCH_MEM_ABOVE_4G will have to be set and the device power needs to be enabled early on so that the device is visible during PCIe enumeration in coreboot.
2. If it not required pre-OS, I think we should defer the resource allocation to kernel and device power-on to happen as part of ACPI. This ensures that we do not do unnecessary work in coreboot which will help optimize boot time. In that case, PCIEXP_HOTPLUG_PREFETCH_MEM_ABOVE_4G is also not required.
I think we would want to provide the flexibility to mainboard to determine when the dGPU device should be powered up and when to allocate resources to it.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2dec7aa2c8db7994f78a7cc1220502676e248465
Gerrit-Change-Number: 57034
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Sun, 22 Aug 2021 05:21:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Jeremy Soller.
Tim Crawford has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56956 )
Change subject: mb/system76/gaze16: Add System76 Gazelle 16
......................................................................
Set Ready For Review
--
To view, visit https://review.coreboot.org/c/coreboot/+/56956
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ifb90f9b73a10abf53a21738e2c466d539df9a37c
Gerrit-Change-Number: 56956
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Comment-Date: Sun, 22 Aug 2021 02:37:18 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Patrick Rudolph.
Hello build bot (Jenkins), Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/57067
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Hook up ucode for TGL-H
......................................................................
soc/intel/tigerlake: Hook up ucode for TGL-H
Hook up microcode from 3rdparty repo for:
- 06-8d-01 (CPUID signature: 0x806d1)
Verified microcode blob was found in CBFS on system76/gaze16 (TGL-H).
CBFS: Found 'cpu_microcode_blob.bin' @0x11700 size 0x18400 in mcache @0x76c2d0ac
microcode: sig=0x806d1 pf=0x2 revision=0x2c
Change-Id: Icf0d8bc700a73697f06503e9d1bb40ce26741cdf
Signed-off-by: Tim Crawford <tcrawford(a)system76.com>
---
M src/soc/intel/tigerlake/Makefile.inc
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/57067/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/57067
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icf0d8bc700a73697f06503e9d1bb40ce26741cdf
Gerrit-Change-Number: 57067
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
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: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jeff Chase, Tim Wawrzynczak, Edward O'Callaghan, Angel Pons.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57037 )
Change subject: mb/google/hatch: don't override variant_ramstage_init() in baseboard
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Right, then I'd move the `mainboard_puff_ramstage_init()` call into the weak declaration of `variant […]
Alternatively, you can also:
1. Add two separate files: mainboard_hatch.c and mainboard_puff.c in variants/baseboard/hatch. These can be included based on CONFIG_BOARD_GOOGLE_BASEBOARD_HATCH and CONFIG_BOARD_GOOGLE_BASEBOARD_PUFF.
2. Each of these mainboard file will provide a weak definition of `variant_ramstage_init()`. For hatch, it will be empty. For puff, it will do what it is currently doing i.e. power limit + other configuration.
3. For the puff variants that you care about doing things differently, you can add non-weak implementation of `variant_ramstage_init()`.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57037
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic5e787c3d4c2f2c62a0dc868997b6e4c3da84c43
Gerrit-Change-Number: 57037
Gerrit-PatchSet: 1
Gerrit-Owner: Jeff Chase <jnchase(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Joe Tessler <jrt(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jeff Chase <jnchase(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 22 Aug 2021 00:03:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jeff Chase <jnchase(a)google.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57049 )
Change subject: arch/x86: Implement cpu_info in C code
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/57049
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ic7396b8429e29739e18a189dacea3a76e571cd58
Gerrit-Change-Number: 57049
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Sat, 21 Aug 2021 23:52:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Timofey Komarov, Nico Huber, Maciej Pijanowski, Michał Żygowski, Patrick Rudolph, Maxim Polyakov, Paul Menzel, Thomas Heijligen, Edward O'Callaghan, Michael Niewöhner, Krystian Hebel, Marcello Sylvester Bauer.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50194 )
Change subject: util/liveiso: Add NixOS configs for bootable live systems
......................................................................
Patch Set 33:
(1 comment)
Patchset:
PS30:
> As long as we only have this, I would leave it at that.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/50194
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Iaf49d198e99781434bd89d2a8a125a4988b77e1c
Gerrit-Change-Number: 50194
Gerrit-PatchSet: 33
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Marcello Sylvester Bauer <sylv(a)sylv.io>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Reviewer: Maciej Pijanowski
Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Timofey Komarov <happycorsair(a)yandex.ru>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Attention: Timofey Komarov <happycorsair(a)yandex.ru>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Maciej Pijanowski
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Attention: Maxim Polyakov <max.senia.poliak(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com>
Gerrit-Attention: Marcello Sylvester Bauer <sylv(a)sylv.io>
Gerrit-Comment-Date: Sat, 21 Aug 2021 20:05:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Tim Crawford, Nico Huber, Jeremy Soller, Tim Wawrzynczak, Paul Menzel, Michael Niewöhner.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57034 )
Change subject: drivers/gfx/nvidia: Add driver for NVIDIA Optimus
......................................................................
Patch Set 2:
(1 comment)
File src/drivers/gfx/nvidia/acpi/gpu.asl:
https://review.coreboot.org/c/coreboot/+/57034/comment/2605ac0d_9a4699e3
PS2, Line 39: CONFIG_MMCONF_BASE_ADDRESS + 0x8000
> you mean an SSDT callback? `.acpi_fill_ssdt` ? +1 to this.
Yes
--
To view, visit https://review.coreboot.org/c/coreboot/+/57034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2dec7aa2c8db7994f78a7cc1220502676e248465
Gerrit-Change-Number: 57034
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Sat, 21 Aug 2021 18:57:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Tim Crawford, Nico Huber, Jeremy Soller, Paul Menzel, Angel Pons, Michael Niewöhner.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57034 )
Change subject: drivers/gfx/nvidia: Add driver for NVIDIA Optimus
......................................................................
Patch Set 2:
(5 comments)
File src/drivers/gfx/nvidia/Kconfig:
https://review.coreboot.org/c/coreboot/+/57034/comment/c73c426d_a1372d36
PS2, Line 9: default 0x01
Can't this be determined at runtime from the parent device in the devicetree (should be the PCIe RP right?)
File src/drivers/gfx/nvidia/acpi/gpu.asl:
https://review.coreboot.org/c/coreboot/+/57034/comment/8c8abe94_1bb39d0f
PS2, Line 39: CONFIG_MMCONF_BASE_ADDRESS + 0x8000
> CONFIG_MMCONF_BASE_ADDRESS + CONFIG_DRIVERS_GFX_NVIDIA_BRIDGE << 15 ? […]
you mean an SSDT callback? `.acpi_fill_ssdt` ? +1 to this.
File src/drivers/gfx/nvidia/gpu.h:
https://review.coreboot.org/c/coreboot/+/57034/comment/9e90c8f9_2fbaf7f8
PS2, Line 9: /* GPIO for GPU_PWR_EN */
: unsigned int power_gpio;
: /* GPIO for GPU_RST# */
: unsigned int reset_gpio;
I have been meaning to break out a separate power resource driver.... reminds me maybe to get doing that 😊
File src/drivers/gfx/nvidia/nvidia.c:
https://review.coreboot.org/c/coreboot/+/57034/comment/02a9c9a8_6fddabf6
PS2, Line 13:
: // Find all BARs on GPU, mark them above 4g if prefetchable
: for (int bar = PCI_BASE_ADDRESS_0; bar <= PCI_BASE_ADDRESS_5; bar += 4) {
: struct resource *res = probe_resource(dev, bar);
:
: if (res) {
: if (res->flags & IORESOURCE_PREFETCH) {
: printk(BIOS_INFO, " BAR at 0x%02x marked above 4g\n", bar);
: res->flags |= IORESOURCE_ABOVE_4G;
: } else {
: printk(BIOS_DEBUG, " BAR at 0x%02x not prefetch\n", bar);
: }
: } else {
: printk(BIOS_DEBUG, " BAR at 0x%02x not found\n", bar);
: }
: }
: }
Does it make sense to select `PCIEXP_HOTPLUG_PREFETCH_MEM_ABOVE_4G` instead when using the NVIDIA driver?
File src/drivers/gfx/nvidia/romstage.c:
https://review.coreboot.org/c/coreboot/+/57034/comment/b49ff7fe_e9ee8b23
PS2, Line 11: void nvidia_set_power(const struct nvidia_gpu_config *config)
: {
: if (!config->power_gpio || !config->reset_gpio) {
: printk(BIOS_ERR, "%s: GPU_PWR_EN and GPU_RST# must be set\n", __func__);
: return;
: }
:
: printk(BIOS_DEBUG, "%s: GPU_PWR_EN = %d\n",
: __func__, config->power_gpio);
: printk(BIOS_DEBUG, "%s: GPU_RST# = %d\n",
: __func__, config->reset_gpio);
:
: gpio_set(config->reset_gpio, 0);
: mdelay(4);
:
: if (config->enable) {
: gpio_set(config->power_gpio, 1);
: mdelay(4);
: gpio_set(config->reset_gpio, 1);
: } else {
: gpio_set(config->power_gpio, 0);
: }
:
: mdelay(4);
: }
IMO, this is up to the mainboard code to perform correctly
--
To view, visit https://review.coreboot.org/c/coreboot/+/57034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2dec7aa2c8db7994f78a7cc1220502676e248465
Gerrit-Change-Number: 57034
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Sat, 21 Aug 2021 18:40:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment