Change in coreboot[master]: mb/clevo/kbl-u: Install Intel VGA INT15 handler

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Paul Menzel (Code Review)

25 Jul 2020 25 Jul '20

9:58 a.m.

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

mb/clevo/kbl-u: Install Intel VGA INT15 handler

Timestamp - Option ROM initialization: 77806110188 FMAP: area COREBOOT found @ 350200 (4914688 bytes) CBFS: Locating 'pci8086,5916.rom' CBFS: Found @ offset 11ce00 size 10000 In CBFS, ROM address for PCI: 00:02.0 = 0xffc6d048 PCI ROM image, vendor ID 8086, device ID 0406, ID mismatch: vendor ID 8086, device ID 5916 Copying VGA ROM Image from 0xffc6d048 to 0xc0000, 0x10000 bytes Timestamp - Option ROM copy done: 77877204844 Calling Option ROM... Unsupported software interrupt #0x15 eax 0x1905f34 Unsupported software interrupt #0x15 eax 0x5f51 Unsupported software interrupt #0x15 eax 0x5f40 Unsupported software interrupt #0x15 eax 0x5f52 Unsupported software interrupt #0x15 eax 0x5f49 Unsupported software interrupt #0x15 eax 0x3a95f49 Unsupported software interrupt #0x15 eax 0x5f14 Unsupported software interrupt #0x15 eax 0x405f35 ... Option ROM returned. VGA Option ROM was run Timestamp - Option ROM run done: 78406420234 PCI: 00:02.0 init finished in 210 msecs

Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Signed-off-by: Paul Menzel pmenzel@molgen.mpg.de --- A src/mainboard/clevo/kbl-u/mainboard.c 1 file changed, 15 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/43852/1

diff --git a/src/mainboard/clevo/kbl-u/mainboard.c b/src/mainboard/clevo/kbl-u/mainboard.c new file mode 100644 index 0000000..01cfef4 --- /dev/null +++ b/src/mainboard/clevo/kbl-u/mainboard.c @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <device/device.h> +#include <drivers/intel/gma/int15.h> + +static void mainboard_enable(struct device *dev) +{ + install_intel_vga_int15_handler(GMA_INT15_ACTIVE_LFP_EDP, + GMA_INT15_PANEL_FIT_CENTERING, + GMA_INT15_BOOT_DISPLAY_DEFAULT, 0); +} + +struct chip_operations mainboard_ops = { + .enable_dev = mainboard_enable, +};

Attachments:

attachment.htm (text/html — 4.2 KB)

Show replies by date

Paul Menzel (Code Review)

25 Jul 25 Jul

10:01 a.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 1:

Unfortunately, this seems to fail for variants.

/dev/shm/coreboot/src/mainboard/clevo/kbl-u/mainboard.c:8: undefined reference to `install_intel_vga_int15_handler'

Angel Pons (Code Review)

10:06 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... File src/mainboard/clevo/kbl-u/mainboard.c:

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... PS1, Line 8: install_intel_vga_int15_handler Did you select INTEL_INT15 in Kconfig?

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 25 Jul 2020 10:06:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Paul Menzel (Code Review)

11:10 a.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... File src/mainboard/clevo/kbl-u/mainboard.c:

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... PS1, Line 8: install_intel_vga_int15_handler

...

Did you select INTEL_INT15 in Kconfig?

I did now

$ grep INT15 .config CONFIG_INTEL_INT15=y

but it still fails with the same error.

/dev/shm/coreboot/util/crossgcc/xgcc/bin/i386-elf-ld.bfd: build/generated/ramstage.o: in function `mainboard_enable': /dev/shm/coreboot/src/mainboard/clevo/kbl-u/mainboard.c:8: undefined reference to `install_intel_vga_int15_handler'

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 25 Jul 2020 11:10:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Angel Pons (Code Review)

11:37 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... File src/mainboard/clevo/kbl-u/mainboard.c:

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... PS1, Line 8: install_intel_vga_int15_handler

...

I did now […]

Check drivers/intel/gma/Makefile.inc:

ifeq ($(CONFIG_VGA_ROM_RUN),y) ramstage-$(CONFIG_INTEL_INT15) += int15.c endif

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 25 Jul 2020 11:37:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Nico Huber (Code Review)

12:19 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... File src/mainboard/clevo/kbl-u/mainboard.c:

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... PS1, Line 8: install_intel_vga_int15_handler

...

Check drivers/intel/gma/Makefile.inc: […]

Maybe something is out-of-sync, did you try a `make clean`? If it doesn't help, you can check with `make V=1` if `int15.o` is included in the linking.

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 25 Jul 2020 12:19:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Paul Menzel (Code Review)

12:30 p.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... File src/mainboard/clevo/kbl-u/mainboard.c:

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... PS1, Line 8: install_intel_vga_int15_handler

...

Maybe something is out-of-sync, did you try a `make clean`? If it doesn't […]

``` $ grep -e INT15 -e VGA_ROM .config CONFIG_S3_VGA_ROM_RUN=y CONFIG_VGA_ROM_RUN=y CONFIG_INTEL_INT15=y ```

Thank you. `make clean` did the trick. I did `rm -rf build; make -j80` before, `make clean` seems to be do more.

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 25 Jul 2020 12:30:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Paul Menzel (Code Review)

12:35 p.m.

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 1:

The VBIOS Option ROM does not light up the screen, and TianoCore is also not showing anything.

``` PCI: 00:02.0 init FMAP: area COREBOOT found @ 350200 (4914688 bytes) CBFS: Locating 'vbt.bin' CBFS: Found @ offset 705c0 size 4a3 Found a VBT of 6144 bytes after decompression GMA: Found VBT in CBFS GMA: Found valid VBT in CBFS FMAP: area COREBOOT found @ 350200 (4914688 bytes) CBFS: Locating 'pci8086,5916.rom' CBFS: Found @ offset 102e00 size 10000 In CBFS, ROM address for PCI: 00:02.0 = 0xffc53048 Copying VGA ROM Image from 0xffc53048 to 0xc0000, 0x10000 bytes Calling Option ROM... intel_vga_int15_handler: AX=5f34 BX=0190 CX=0002 DX=03da intel_vga_int15_handler: AX=5f51 BX=c000 CX=0000 DX=03da intel_vga_int15_handler: AX=5f40 BX=0000 CX=0004 DX=0008 DISPLAY=0 intel_vga_int15_handler: AX=5f52 BX=400a CX=000f DX=0008 Unknown INT15 function 5f52! int15 call returned error. intel_vga_int15_handler: AX=5f49 BX=19ff CX=0000 DX=1dcc Unknown INT15 function 5f49! int15 call returned error. intel_vga_int15_handler: AX=5f49 BX=0050 CX=0300 DX=0040 Unknown INT15 function 5f49! int15 call returned error. intel_vga_int15_handler: AX=5f14 BX=078f CX=0002 DX=0008 Unknown INT15 function 5f14! int15 call returned error. intel_vga_int15_handler: AX=5f35 BX=c000 CX=0002 DX=03da ... Option ROM returned. VGA Option ROM was run PCI: 00:02.0 init finished in 326 msecs ```

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 25 Jul 2020 12:35:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons (Code Review)

12:38 p.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... File src/mainboard/clevo/kbl-u/mainboard.c:

https://review.coreboot.org/c/coreboot/+/43852/1/src/mainboard/clevo/kbl-u/m... PS1, Line 8: install_intel_vga_int15_handler

...

When changing Kconfig settings, I usually run `make oldconfig` to regenerate the .config file

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 25 Jul 2020 12:38:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Comment-In-Reply-To: Paul Menzel paulepanter@users.sourceforge.net Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Paul Menzel (Code Review)

26 Jul 26 Jul

9:19 p.m.

Hello Felix Singer, build bot (Jenkins), Nico Huber, Angel Pons,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43852

to look at the new patch set (#2).

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

mb/clevo/kbl-u: Install Intel VGA INT15 handler

The interrupt handler is needed, otherwise running the VBIOS Option ROM in coreboot fails:

Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Signed-off-by: Paul Menzel pmenzel@molgen.mpg.de --- M src/mainboard/clevo/kbl-u/Kconfig A src/mainboard/clevo/kbl-u/mainboard.c 2 files changed, 16 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/43852/2

Benjamin Doron (Code Review)

12 Nov 12 Nov

8:03 p.m.

Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 5:

...

Patch Set 1:

The VBIOS Option ROM does not light up the screen, and TianoCore is also not showing anything.

PCI: 00:02.0 init
FMAP: area COREBOOT found @ 350200 (4914688 bytes)
CBFS: Locating 'vbt.bin'
CBFS: Found @ offset 705c0 size 4a3
Found a VBT of 6144 bytes after decompression
GMA: Found VBT in CBFS
GMA: Found valid VBT in CBFS
FMAP: area COREBOOT found @ 350200 (4914688 bytes)
CBFS: Locating 'pci8086,5916.rom'
CBFS: Found @ offset 102e00 size 10000
In CBFS, ROM address for PCI: 00:02.0 = 0xffc53048
Copying VGA ROM Image from 0xffc53048 to 0xc0000, 0x10000 bytes
Calling Option ROM...
intel_vga_int15_handler: AX=5f34 BX=0190 CX=0002 DX=03da
intel_vga_int15_handler: AX=5f51 BX=c000 CX=0000 DX=03da
intel_vga_int15_handler: AX=5f40 BX=0000 CX=0004 DX=0008
DISPLAY=0
intel_vga_int15_handler: AX=5f52 BX=400a CX=000f DX=0008
Unknown INT15 function 5f52!
int15 call returned error.                  
intel_vga_int15_handler: AX=5f49 BX=19ff CX=0000 DX=1dcc
Unknown INT15 function 5f49!               
int15 call returned error.
intel_vga_int15_handler: AX=5f49 BX=0050 CX=0300 DX=0040
Unknown INT15 function 5f49!
int15 call returned error.
intel_vga_int15_handler: AX=5f14 BX=078f CX=0002 DX=0008
Unknown INT15 function 5f14!
int15 call returned error.
intel_vga_int15_handler: AX=5f35 BX=c000 CX=0002 DX=03da
... Option ROM returned.
VGA Option ROM was run 
PCI: 00:02.0 init finished in 326 msecs

According to the BSF script file for the SKL/KBL/CFL/AML VGABIOS, these are the int15 functions called that coreboot does not support: 5f52h: Hook to get Panel Color Depth from Setup (Mobile only) 5f49h: Hook to get the backlight inverter type, polarity and initial brightness 5f14h: Get Miscellaneous Status (Mobile only)

The information provided by the first two should be available in the VBT inside the VGABIOS. You could try using the Intel BMP (Binary Modification Program) to disable these hooks. The last hook is apparently required for DisplayPort Redriver suppoort.

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 5 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Benjamin Doron benjamin.doron00@gmail.com Gerrit-Comment-Date: Thu, 12 Nov 2020 20:03:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Paul Menzel (Code Review)

25 Nov 25 Nov

6:18 p.m.

Hello Felix Singer, build bot (Jenkins), Nico Huber, Angel Pons, Michael Niewöhner,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43852

to look at the new patch set (#11).

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

mb/clevo/kbl-u: Install Intel VGA INT15 handler

The interrupt handler is needed, otherwise running the VBIOS Option ROM in coreboot fails:

Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Signed-off-by: Paul Menzel pmenzel@molgen.mpg.de --- M src/mainboard/clevo/kbl-u/Kconfig M src/mainboard/clevo/kbl-u/ramstage.c 2 files changed, 10 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/43852/11

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 11 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Michael Niewöhner foss@mniewoehner.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Benjamin Doron benjamin.doron00@gmail.com Gerrit-MessageType: newpatchset

Felix Singer (Code Review)

6:49 p.m.

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 11: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 11 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Michael Niewöhner foss@mniewoehner.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Benjamin Doron benjamin.doron00@gmail.com Gerrit-Comment-Date: Wed, 25 Nov 2020 18:49:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Felix Singer (Code Review)

26 Nov 26 Nov

5:54 p.m.

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 11: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/11/src/mainboard/clevo/kbl-u/... File src/mainboard/clevo/kbl-u/ramstage.c:

https://review.coreboot.org/c/coreboot/+/43852/11/src/mainboard/clevo/kbl-u/... PS11, Line 21: mainboard_enable Please use the imperative, `enable_mainboard`.

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 11 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Michael Niewöhner foss@mniewoehner.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Benjamin Doron benjamin.doron00@gmail.com Gerrit-Comment-Date: Thu, 26 Nov 2020 17:54:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons (Code Review)

27 Nov 27 Nov

10:51 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/11/src/mainboard/clevo/kbl-u/... File src/mainboard/clevo/kbl-u/ramstage.c:

https://review.coreboot.org/c/coreboot/+/43852/11/src/mainboard/clevo/kbl-u/... PS11, Line 21: mainboard_enable

...

Please use the imperative, `enable_mainboard`.

Most boards use this name here, I would +2 a change that renames all of them at once

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 11 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Michael Niewöhner foss@mniewoehner.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Benjamin Doron benjamin.doron00@gmail.com Gerrit-Comment-Date: Fri, 27 Nov 2020 10:51:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer felixsinger@posteo.net Gerrit-MessageType: comment

Michael Niewöhner (Code Review)

28 Nov 28 Nov

6:52 p.m.

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/11/src/mainboard/clevo/kbl-u/... File src/mainboard/clevo/kbl-u/ramstage.c:

https://review.coreboot.org/c/coreboot/+/43852/11/src/mainboard/clevo/kbl-u/... PS11, Line 21: mainboard_enable

...

Most boards use this name here, I would +2 a change that renames all of them at once

Coreboot coding style guide says "use descriptive names". `mainboard_enable` is maybe some sort of descriptive but simply wrong. "enable mainboard" -> "enable_mainboard". Just because XYZ does this, it's not inherently correct.

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 11 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Michael Niewöhner foss@mniewoehner.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Benjamin Doron benjamin.doron00@gmail.com Gerrit-Comment-Date: Sat, 28 Nov 2020 18:52:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer felixsinger@posteo.net Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Gerrit-MessageType: comment

Michael Niewöhner (Code Review)

7:16 p.m.

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/11/src/mainboard/clevo/kbl-u/... File src/mainboard/clevo/kbl-u/ramstage.c:

https://review.coreboot.org/c/coreboot/+/43852/11/src/mainboard/clevo/kbl-u/... PS11, Line 21: mainboard_enable

...

Coreboot coding style guide says "use descriptive names". […]

uhm, I probably misunderstood, what you meant here. did you mean "change all boards to use `enable_mainboard`"?

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 11 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Michael Niewöhner foss@mniewoehner.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Benjamin Doron benjamin.doron00@gmail.com Gerrit-Comment-Date: Sat, 28 Nov 2020 19:16:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer felixsinger@posteo.net Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: Michael Niewöhner foss@mniewoehner.de Gerrit-MessageType: comment

Angel Pons (Code Review)

30 Nov 30 Nov

9:25 a.m.

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 12:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43852/11/src/mainboard/clevo/kbl-u/... File src/mainboard/clevo/kbl-u/ramstage.c:

https://review.coreboot.org/c/coreboot/+/43852/11/src/mainboard/clevo/kbl-u/... PS11, Line 21: mainboard_enable

...

uhm, I probably misunderstood, what you meant here. […]

Yeah, I meant that last thing. I don't care about the name, as long as it's the same among all boards. *mumbles something about consistency*

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 12 Gerrit-Owner: Paul Menzel paulepanter@users.sourceforge.net Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Michael Niewöhner foss@mniewoehner.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Benjamin Doron benjamin.doron00@gmail.com Gerrit-Comment-Date: Mon, 30 Nov 2020 09:25:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer felixsinger@posteo.net Comment-In-Reply-To: Angel Pons th3fanbus@gmail.com Comment-In-Reply-To: Michael Niewöhner foss@mniewoehner.de Gerrit-MessageType: comment

Felix Singer (Code Review)

23 Apr 23 Apr

12:37 p.m.

Attention is currently required from: Paul Menzel. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43852 )

Change subject: mb/clevo/kbl-u: Install Intel VGA INT15 handler ......................................................................

Patch Set 12: Code-Review-1

(1 comment)

Patchset:

PS12: I don't think we need this. Kaby Lake has libgfxinit support. So why maintaining code for an unnecessary proprietary blob?

-- To view, visit https://review.coreboot.org/c/coreboot/+/43852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I126481c745ae701dc438ac664be0f8ff5f6250c1 Gerrit-Change-Number: 43852 Gerrit-PatchSet: 12 Gerrit-Owner: Paul Menzel paulepanter@mailbox.org Gerrit-Reviewer: Felix Singer felixsinger@posteo.net Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Michael Niewöhner foss@mniewoehner.de Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-CC: Benjamin Doron benjamin.doron00@gmail.com Gerrit-Attention: Paul Menzel paulepanter@mailbox.org Gerrit-Comment-Date: Fri, 23 Apr 2021 12:37:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1316

days inactive

1588

days old

coreboot-gerrit@coreboot.org

18 comments

6 participants

tags (0)

participants (6)

Angel Pons (Code Review)
Benjamin Doron (Code Review)
Felix Singer (Code Review)
Michael Niewöhner (Code Review)
Nico Huber (Code Review)
Paul Menzel (Code Review)