Mimoja has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34475 )
Change subject: [WIP] Add Razer Blade Stealth (2016) H2U ......................................................................
Patch Set 7:
(14 comments)
Patch Set 7:
(8 comments)
Thanks for starting this port!
Thanks a lot for the initial review! <3
https://review.coreboot.org/c/coreboot/+/34475/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34475/6//COMMIT_MSG@2 PS6, Line 2: Author: Johanna Schander johanna.schander@joblift.de
Don't you want to use your private email address? Or is this related to your work?
Changed. Thanks :)
https://review.coreboot.org/c/coreboot/+/34475/6//COMMIT_MSG@10 PS6, Line 10: - A way to determin equipped ram (help appreciated)
This is usually done via GPIOs. […]
Only have this one board and don't have the schematics
https://review.coreboot.org/c/coreboot/+/34475/6//COMMIT_MSG@16 PS6, Line 16: - BUG: Dmesg: ioapic 2 has no mapping iommu, interrupt remapping will be disabled
You're missing the right acpi entries for the seconds ioapic. Take a look into the original acpi.
The original dump looks like this: ACPI: APIC 0x0000000000000000 000084 (v03 ALASKA A M I 01072009 AMI 00010013)
I do not really understand what i am looking at here, so for the moment I will need to read a bit on the topic :)
acpidump > acpidump.log cpixtract -sAPIC acpidump.log iasl -d apic.dat
https://review.coreboot.org/c/coreboot/+/34475/6//COMMIT_MSG@42 PS6, Line 42: - Onboard Keyboard in SeaBIOS
how is the keyboard connected? usb, ps/2, i2c?
USB 2.0 Port 9
https://review.coreboot.org/c/coreboot/+/34475/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34475/7//COMMIT_MSG@27 PS7, Line 27: Even tho it has a 16Mb chip equipped (W25Q128.V) only the first 8Mb are used and mapped. The rest should be left empty (0xFF)
Sounds like Intel Firmware Descriptor (IFD) region stuff? […]
Fixed to MB. Does not appear to be a IFD related. I did not invest to much time tho...
https://review.coreboot.org/c/coreboot/+/34475/7//COMMIT_MSG@38 PS7, Line 38: - Webcam, but higher resolutions appear to fail.
The webcam is usually a USB device, shouldn't be affected much I guess. […]
It was a software problem, that was documented with the vendor firmware as well
https://review.coreboot.org/c/coreboot/+/34475/7//COMMIT_MSG@41 PS7, Line 41: - Headphones
Probably missing or incorrect HDA configuration
Yes - most likely. Did not have the time to look into it so far.
https://review.coreboot.org/c/coreboot/+/34475/7//COMMIT_MSG@42 PS7, Line 42: - Onboard Keyboard in SeaBIOS
Missing PS/2 keyboard init, or SuperIO/EC wants some configuration
Keyboard is usb - not sure how to configure the SuperIO. Can i dump the configuration from the vendor firmware?
https://review.coreboot.org/c/coreboot/+/34475/7/src/mainboard/razer/blade_s... File src/mainboard/razer/blade_stealth_kbl/Kconfig:
https://review.coreboot.org/c/coreboot/+/34475/7/src/mainboard/razer/blade_s... PS7, Line 16: select HAVE_IFD_BIN # mainline compatible? : select HAVE_ME_BIN # mainline compatible?
Please drop these
Done
https://review.coreboot.org/c/coreboot/+/34475/7/src/mainboard/razer/blade_s... PS7, Line 24: INTEL_GMA_ADD_VBT
Ensure the VBT is inside the mainboard directory as 'data. […]
I am not 100% sure it is working, it does not get added to CBFS.. Is that normal?
Name Offset Type Size Comp cbfs master header 0x0 cbfs header 32 none fallback/romstage 0x80 stage 39276 none fallback/ramstage 0x9a80 stage 101045 none config 0x22580 raw 524 none revision 0x22800 raw 679 none bootsplash.jpg 0x22b00 bootsplash 13597 none spd.bin 0x26080 spd 512 none payload_config 0x262c0 raw 1593 none payload_revision 0x26940 raw 235 none (empty) 0x26a80 null 792 none fspm.bin 0x26dc0 fsp 405504 none (empty) 0x89e00 null 3992 none fsps.bin 0x8adc0 fsp 188416 none pci8086,5916.rom 0xb8e00 optionrom 65536 none fallback/postcar 0xc8e80 stage 20544 none fallback/dsdt.aml 0xcdf00 raw 10588 none fallback/payload 0xd08c0 simple elf 68067 none (empty) 0xe1300 null 3014680 none cpu_microcode_blob.bin 0x3c1340 microcode 397312 none (empty) 0x4223c0 null 1579480 none bootblock 0x5a3dc0 bootblock 49152 none
https://review.coreboot.org/c/coreboot/+/34475/7/src/mainboard/razer/blade_s... PS7, Line 53: : config DEVICETREE : string : default "devicetree.cb"
Default value, should not need to be overriden
Removed
https://review.coreboot.org/c/coreboot/+/34475/7/src/mainboard/razer/blade_s... PS7, Line 62: config NO_POST
Looks like Jenkins doesn't like this
Moved to select
https://review.coreboot.org/c/coreboot/+/34475/7/src/mainboard/razer/blade_s... File src/mainboard/razer/blade_stealth_kbl/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/34475/7/src/mainboard/razer/blade_s... PS7, Line 56: register "SaGv" = "3"
I think there's an enum for SaGv values
Yes, you are right. lives in chip.h
https://review.coreboot.org/c/coreboot/+/34475/6/src/mainboard/razer/blade_s... File src/mainboard/razer/blade_stealth_kbl/gpio.h:
https://review.coreboot.org/c/coreboot/+/34475/6/src/mainboard/razer/blade_s... PS6, Line 4: * Copyright (C) 2015 Google Inc.
Do you copied it? if not, this should be your copyright, even it's unclear if this is high enough fo […]
Thanks. Changed.