Attention is currently required from: Alicja Michalska, Angel Pons, Felix Singer.
Nicholas Chin has posted comments on this change by Alicja Michalska. ( https://review.coreboot.org/c/coreboot/+/81611?usp=email )
Change subject: Documentation: Add Erying Polestar G613 Pro ......................................................................
Patch Set 7:
(9 comments)
Patchset:
PS7: The content looks fine to me. Just some things I noticed about the rendered output.
File Documentation/mainboard/erying/tgl/tgl_matx.md:
https://review.coreboot.org/c/coreboot/+/81611/comment/486c447e_2530cafb?usp... : PS7, Line 37: By going to `Chipset -> Include CPU microcode in CBFS (Include external microcode binary)`. Markdown ignores new lines without a blank line in between, so this renders as part of the above bullet point. Adding a blank line after the bullets so that this renders as a new line at the same indent level as the main text.
https://review.coreboot.org/c/coreboot/+/81611/comment/9e7bf842_c8594eba?usp... : PS7, Line 66: If you're using [flashrom] or [flashprog] (fork of flashrom), you can skip extracting `SI_BIOS` and `SI_ME` regions from your ROM, and flash coreboot to `SI_BIOS` region by issuing the following command: Please reflow to 72 characters
https://review.coreboot.org/c/coreboot/+/81611/comment/e45d9435_90702d50?usp... : PS7, Line 112: `modprobe it87 force_id=0x8603 Missing closing backtick? ```suggestion - Configure automatic fan control using pwmconfig (`modprobe it87 force_id=0x8603`) ```
https://review.coreboot.org/c/coreboot/+/81611/comment/a13e23b3_7e707432?usp... : PS7, Line 117: - Intel Flash Descriptor (IFD) is required if you wish to flash the entire chip. : - Intel Management Engine blob is required if you wish to flash the entire chip. These render a bit awkwardly (the bullets are at the same indent level as the numbering, and have a paragraph break with the prior text). Indenting them so that the dashes are at the same level as "Required blobs" (i.e. indent by 3 spaces) makes it render as part of note 1.
https://review.coreboot.org/c/coreboot/+/81611/comment/1e579e86_4c0cfb74?usp... : PS7, Line 119: Both blobs included in `3rdparty/blobs` repository have been extracted from vendor's firmware. : IFD region has been modified using `ifdtool` to flip `MeAltDisable` flag. As above; this renders as part of the bullet point about the Intel ME. Adding a blank line after the bullets and indenting to the same level as "Required blobs" make it look better.
https://review.coreboot.org/c/coreboot/+/81611/comment/bdbd026b_f9ecf8a4?usp... : PS7, Line 123: It is possible to replace Winbond 16MB chip with 32MB equivalent, which would allow you to use LinuxBoot or implement RO + A/B VBOOT update scheme. This renders on the same line as Modifications. Add a blank line before this and indent to the same level as "Modifications" (3 spaces)
https://review.coreboot.org/c/coreboot/+/81611/comment/98562534_9a8e4fc8?usp... : PS7, Line 126: If you are using an external graphics card (AMD Radeon, Nvidia GeForce, Intel Arc), you will see output in your OS as soon as kernel initializes the card (called "modprobing" in Linux) regardless of payload you chose. : This board have been tested with: : - EDK2 : - U-Boot : - LinuxBoot (U-Root + Linux kernel) As above. I'd recommend indenting this to the same level as "Payload and ..." (i.e. 3 spaces) so that it looks like a part of note 3
https://review.coreboot.org/c/coreboot/+/81611/comment/67f9d3c6_213f5868?usp... : PS7, Line 122: 2. Modifications : It is possible to replace Winbond 16MB chip with 32MB equivalent, which would allow you to use LinuxBoot or implement RO + A/B VBOOT update scheme. : : 3. Payload and pre-OS display output : If you are using an external graphics card (AMD Radeon, Nvidia GeForce, Intel Arc), you will see output in your OS as soon as kernel initializes the card (called "modprobing" in Linux) regardless of payload you chose. : This board have been tested with: : - EDK2 : - U-Boot : - LinuxBoot (U-Root + Linux kernel) : : If you would like to see output on your iGPU before that stage (for picking a different boot medium or toggling Secure Boot setting), you need to use [MrChromebox's EDK2] fork and include [GOP driver] for TigerLake iGPU in your build. : This will allow you to see output of EDK2 (payload, boot picker) on your monitor connected to iGPU. : : If you're planning to primarly use an external card, disable iGPU by toggling `Chipset -> Disable Integrated GFX Controller (0:2:0)` and use [elly's EDK2] tree. : In order to enable loading OpROMs from PCIe devices, go to `Payload -> edk2 additional custom build parameters` and add the string: `-D LOAD_OPTION_ROMS=TRUE`. : This functionality has been tested with following GPUs, with following results: : - Nvidia GeForce RTX3080;RTX3090: Works perfectly : - AMD Radeon RX6600XT;RX7800XT: Works with ReBAR disabled, no output in EDK2 with ReBAR enabled : - Intel Arc A580: Works with ReBAR disabled, glitched framebuffer before modprobing Please reflow to 72 characters