Skoll RC has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40813 )
Change subject: mb/ongy/h61m-s1: Add new mainboard ......................................................................
Patch Set 17:
(44 comments)
done, Jenkins Build faild but I can't access to the result. Maybe it is just in pending state? In order to let users to choose if they want to use or not VGA BIOS, I will leave mainboard.c unchange (like in other H61 mainboards)
https://review.coreboot.org/c/coreboot/+/40813/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40813/10//COMMIT_MSG@7 PS10, Line 7: adding new mainboard : ported with autoport script
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/10//COMMIT_MSG@13 PS10, Line 13: USB boot (USB 2.0 only - USB 3.0 works sometimes but not always, don't why yet - Works when OS is loaded)
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40813/14//COMMIT_MSG@7 PS14, Line 7: mb/ongy/h61m-s1: Add new mainboard
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14//COMMIT_MSG@15 PS14, Line 15: Integrated VGA and HDMI works
Yes, I mentioned it in a new version of this file
Done
https://review.coreboot.org/c/coreboot/+/40813/14//COMMIT_MSG@17 PS14, Line 17: Seabios
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40813/15//COMMIT_MSG@11 PS15, Line 11: Tested:
You can make this look more like a list: […]
Done
https://review.coreboot.org/c/coreboot/+/40813/15//COMMIT_MSG@13 PS15, Line 13: resume
S3 resume?
Done
https://review.coreboot.org/c/coreboot/+/40813/15//COMMIT_MSG@16 PS15, Line 16: i
Capitalize
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/Kconfig File src/mainboard/ongy/Kconfig:
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/Kconfig@... PS6, Line 12: default "H61M-S1"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/Kconfig File src/mainboard/ongy/Kconfig:
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/Kconfig... PS10, Line 1: if VENDOR_ONGY
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/Kconfig.... File src/mainboard/ongy/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/Kconfig.... PS6, Line 2: bool "ongy"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... File src/mainboard/ongy/h61m-s1/Kconfig:
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... PS6, Line 1: ## : ## This file is part of the coreboot project. : ## : ## : ## This program is free software; you can redistribute it and/or modify : ## it under the terms of the GNU General Public License as published by : ## the Free Software Foundation; version 2 of the License. : ## : ## This program is distributed in the hope that it will be useful, : ## but WITHOUT ANY WARRANTY; without even the implied warranty of : ## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : ## GNU General Public License for more details. : ##
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... PS6, Line 15: if BOARD_H61M_S1
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/Kconfig:
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 26: config VGA_BIOS_FILE : string : default "pci8086,0112.rom" : : config VGA_BIOS_ID : string : default "8086,0112"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 34: onfig DRAM_RESET_GATE_GPIO : int : default 60
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... File src/mainboard/ongy/h61m-s1/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... PS6, Line 1: config BOARD_H61M_S1
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... PS6, Line 2: bool "h61m-s1"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... File src/mainboard/ongy/h61m-s1/acpi/platform.asl:
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... PS6, Line 2: /* This file is part of the coreboot project. */
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... File src/mainboard/ongy/h61m-s1/acpi_tables.c:
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... PS6, Line 1: /* : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/acpi_tables.c:
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 8: /* Disable USB ports in S3 by default */ : gnvs->s3u0 = 0; : gnvs->s3u1 = 0; : /* Disable USB ports in S5 by default */ : gnvs->s5u0 = 0; : gnvs->s5u1 = 0;
You can check other mainboards, I think they just leave the function empty
Done
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... PS10, Line 6: register "gpu_cpu_backlight" = "0x00000000"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... PS10, Line 10: register "gpu_panel_port_select" = "0" : register "gpu_panel_power_backlight_off_delay" = "0" : register "gpu_panel_power_backlight_on_delay" = "0"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... PS10, Line 14: register "gpu_panel_power_down_delay" = "0" : register "gpu_panel_power_up_delay" = "0" : register "gpu_pch_backlight" = "0x00000000"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... PS10, Line 34: register "gen2_dec" = "0x00000000" : register "gen3_dec" = "0x00000000" : register "gen4_dec" = "0x00000000"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 5: register "gfx" = "GMA_STATIC_DISPLAYS(0)" : register "gpu_dp_b_hotplug" = "4" : register "gpu_dp_c_hotplug" = "4" : register "gpu_dp_d_hotplug" = "4" : register "gpu_panel_power_cycle_delay" = "4"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 10: 0x0
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 18: 0x0
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 22: 0x0
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 25: register "docking_supported" = "0"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 27: register "pcie_hotplug_map" = "{ 0, 0, 0, 0, 0, 0, 0, 0 }"
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 64: : device pci 00.0 on # Host bridge Host bridge : subsystemid 0x8086 0x0100 : end : device pci 01.0 on # PEG : subsystemid 0x8086 0x0101 : end : device pci 02.0 on # iGPU : subsystemid 0x8086 0x2010 : end
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/15/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/40813/15/src/mainboard/ongy/h61m-s1... PS15, Line 18: Host bridge Host bridge
Oh, didn't notice that this comment is duplicated
Done
https://review.coreboot.org/c/coreboot/+/40813/15/src/mainboard/ongy/h61m-s1... PS15, Line 18: Host bridge Host bridge
Oh, didn't notice that this comment is duplicated
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... File src/mainboard/ongy/h61m-s1/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... PS6, Line 1: /* : * : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... PS6, Line 13: #define ACPI_VIDEO_DEVICE _SB.PCI0.GFX0
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... PS10, Line 4: #define ACPI_VIDEO_DEVICE _SB.PCI0.GFX0
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 4: #define ACPI_VIDEO_DEVICE _SB.PCI0.GFX0
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 22: g
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... PS14, Line 29: #include <drivers/intel/gma/acpi/default_brightness_levels.asl>
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/early_init.c:
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... PS10, Line 4: #include <stdint.h> : #include <string.h> : #include <timestamp.h> : #include <arch/byteorder.h> : #include <device/mmio.h> : #include <device/pci_ops.h> : #include <device/pnp_ops.h> : #include <console/console.h> : #include <bootblock_common.h> : #include <northbridge/intel/sandybridge/sandybridge.h> : #include <northbridge/intel/sandybridge/raminit_native.h> : #include <southbridge/intel/bd82x6x/pch.h> : #include <southbridge/intel/common/gpio.h>
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/15/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/gma-mainboard.ads:
https://review.coreboot.org/c/coreboot/+/40813/15/src/mainboard/ongy/h61m-s1... PS15, Line 13: Analog,
Tiiiny detail: the port values are aligned with an extra space: […]
Done
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... File src/mainboard/ongy/h61m-s1/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/40813/6/src/mainboard/ongy/h61m-s1/... PS6, Line 1: /* : * This program is free software; you can redistribute it and/or : * modify it under the terms of the GNU General Public License as : * published by the Free Software Foundation; version 2 of : * the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/40813/10/src/mainboard/ongy/h61m-s1... PS10, Line 1: /* : * This program is free software; you can redistribute it and/or : * modify it under the terms of the GNU General Public License as : * published by the Free Software Foundation; version 2 of : * the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */
Done
Done
https://review.coreboot.org/c/coreboot/+/40813/14/src/mainboard/ongy/h61m-s1... File src/mainboard/ongy/h61m-s1/mainboard.c:
PS14:
Yes, I never needed it with libgfxinit
https://github.com/coreboot/coreboot/blob/master/src/mainboard/gigabyte/ga-h... it is used in other similar mainboards. Maybe to let users choose if they want to use VGA BIOS or not. So I think it's better to leave this unchanged.