Dear coreboot developers,
In order to improve the user experience we would like to display a message on the screen while memory training is in progress. As a first step, we are working on Alder Lake and Raptor Lake platforms and are leveraging the `libgfxinit' and VGA text mode. We have libgfxinit patches for Tiger Lake, Alder Lake (and Raptor Lake) support under review but this is not the object of this email (see https://review.coreboot.org/c/libgfxinit/+/65087 if you are interested)
In the last few weeks, we have been working on a proof-of-concept to ensure that we could overcome all the technical hardware challenges to initialize VGA text mode and setup the display before we jump into FSP memory.
Even though we are still working on some stability issues, we have a "working" but hacky patch :-). But we have an aggressive timeline and it works good enough that we believe it is time to start an open discussion as the surface of the patch is important (linker scripts, configuration flags logic ...). The changes have been reduced to a unique patch after a lot of clean-up (https://review.coreboot.org/c/coreboot/+/69757). This patch will be split up in a couple of patches once we are done with some in progress development.
We identified three majors "issues" for which we are seeking feedback, guidance or help.
PCI resource allocation =======================
https://review.coreboot.org/c/coreboot/+/69757/2/src/soc/intel/alderlake/romstage/graphics.c#64
We need to configure the graphics PCI device to perform graphic initialization. It requires to perform an early romstage PCI resource allocation. For the proof-of-concept, we assigned the same resources than the coreboot resource allocator would assign during ramstage. We cannot leave hardcoded values like these. Any suggestion on how to cleanly handle early PCI resources allocation ?
Configuration flags and "duplicated" ====================================
https://review.coreboot.org/c/coreboot/+/69757/2/src/drivers/intel/fsp2_0/Kconfig https://review.coreboot.org/c/coreboot/+/69757/2/src/lib/Kconfig https://review.coreboot.org/c/coreboot/+/69757/2/src/device/Kconfig
We had to "duplicate" some configuration flags such as `MAINBOARD_USE_LIBGFXINIT' as a `ROMSTAGE_MAINBOARD_USE_LIBGFXINIT' for instance. We want to separate configuration between romstage and ramstage to offer flexibility to use to have VGA text mode in romstage without any graphics initialization in ramstage or a graphic initialization with the GOP driver in ramstage ...
We would appreciate some review and feedback on our clearly not perfect `Kconfig' changes. Any suggestion is welcome.
libgfxinit and global initialized variables ===========================================
https://review.coreboot.org/c/coreboot/+/69757/1/src/arch/x86/car.ld
`libgfxinit' has some initialized global variables ending up in the `.data' section.
Global *initialized* variables are not permitted in cache-as-ram and with the current `car.ld' linker script I hit compilation link errors due the following list of variables. Using objdump I collected their initial value (note that I did not exhaustively checked all the `[...]_E' ones but only a sample of them).
Address Type Name Value ----------------------------------------------------------------------------- ffffff60 d `hw__debug__register_write_delay_nanoseconds' 00 ffffff68 d `hw__debug__start_of_line' 01 ffffff2c d `hw__gfx__gma__registers__gtt_32__range_aXnnn' 00 20 00 00 ffffff28 d `hw__gfx__gma__registers__gtt_64__range_aXnnn' 00 80 00 00 ffffff30 d `hw__gfx__gma__registers__regs__range_aXnnn' 00 00 00 00 ffffff6e D `gma_E' 00 ... ffffff70 D `gma__gfx_init_E' 00 ... ffffff6a D `hw__debug_E' 00 ... ffffff6c D `hw__debug_sink_E' 00 ... ffffff02 D `hw__gfx__dp_aux_ch_E' 00 ... ffffff04 D `hw__gfx__dp_dual_mode_E' 00 ... ffffff06 D `hw__gfx__dp_info_E' 00 ... ffffff08 D `hw__gfx__dp_training_E' 00 ... ffffff0a D `hw__gfx__edid_E' 00 ... ffffff0c D `hw__gfx__framebuffer_filler_E' 00 ... ffffff3a D `hw__gfx__gma_E' 00 ... ffffff3c D `hw__gfx__gma__combo_phy_E' 00 ... ffffff00 D `hw__gfx__gma__config_E' 00 ... ffffff0e D `hw__gfx__gma__config_helpers_E' 00 ... ffffff10 D `hw__gfx__gma__connector_info_E' 00 ... ffffff42 D `hw__gfx__gma__connectors_E' 00 ... ffffff3e D `hw__gfx__gma__connectors__combo_phy_E' 00 ... ffffff40 D `hw__gfx__gma__connectors__tc_E' 00 ... ffffff12 D `hw__gfx__gma__display_probing_E' 00 ... ffffff14 D `hw__gfx__gma__dp_aux_request_E' 00 ... ffffff16 D `hw__gfx__gma__i2c_E' 00 ... ffffff38 D `hw__gfx__gma__initialized' 00 ... ffffff18 D `hw__gfx__gma__panel_E' 00 ... ffffff1a D `hw__gfx__gma__pch__fdi_E' 00 ... ffffff1c D `hw__gfx__gma__pch__sideband_E' 00 ... ffffff1e D `hw__gfx__gma__pch__transcoder_E' 00 ... ffffff20 D `hw__gfx__gma__pch__vga_E' 00 ... ffffff22 D `hw__gfx__gma__pcode_E' 00 ... ffffff24 D `hw__gfx__gma__pipe_setup_E' 00 ... ffffff48 D `hw__gfx__gma__plls_E' 00 ... ffffff44 D `hw__gfx__gma__plls__combo_phy_E' 00 ... ffffff46 D `hw__gfx__gma__plls__dkl_E' 00 ... ffffff4a D `hw__gfx__gma__port_detect_E' 00 ... ffffff50 D `hw__gfx__gma__power_and_clocks_E' 00 ... ffffff4c D `hw__gfx__gma__power_and_clocks__tgl_E' 00 ... ffffff4e D `hw__gfx__gma__power_and_clocks__xelpd_E' 00 ... ffffff34 D `hw__gfx__gma__registers_E' 00 ... ffffff36 D `hw__gfx__gma__transcoder_E' 00 ... ffffff52 D `hw__mmio_range_E' 00 ... ffffff56 D `hw__mmio_regs_E' 00 ... ffffff54 D `hw__pci__dev_E' 00 ... ffffff58 D `hw__pci__mmconf_E' 00 ... ffffff5a D `hw__port_io_E' 00 ... ffffff5c D `hw__time_E' 00 ... ffffff72 D `hw__time__timer_E' 00 ...
The symbols ending with `_E' are empty and initialized by GNAT generated code (cf. `___elabs' functions) which is called using the `$stage_adainit()' function. Therefor, I think it is safe to places these into the `_bss' region of the `.car.data' section. What I don't understand though is why isn't the GNAT compiler placing them into the `.bss' region of the object file. Any idea ? I could not find any GNAT compiler option for this but I am clearly not an ADA compiler expert :-)
Now we are left with the following symbols: - `hw__debug__register_write_delay_nanoseconds' is zero so installing it in the `_bss' region of the `.car.data' section is "technically" fine. - `hw__debug__start_of_line': Reverting the logic of this boolean makes it work - The following three symbols are the most challenging ones: - `hw__gfx__gma__registers__gtt_32__range_aXnnn' - `hw__gfx__gma__registers__gtt_64__range_aXnnn' - `hw__gfx__gma__registers__regs__range_aXnnn' They seem to be entirely by the compiler and I don't know how to control them. Surprisingly, putting them in the `_bss' region of the `.car.data' section which is making them zeros is not causing any apparent issue while their initial value should not be zeros. I am thinking that these could be used for code verification purposes only. Does anyone knows before I adventure in to assembly code to uncover their role ?
Regards,
-- *Jeremy* /One Emacs to rule them all/
Hi Jeremy,
welcome to the mailinglist! It's nice to see libgfxinit working for you and in this special case. :)
On 22.11.22 03:51, Compostella, Jeremy wrote:
In order to improve the user experience we would like to display a message on the screen while memory training is in progress.
I was wondering so far what the use case would be. Thinking about it for a moment, I guess this makes much sense. coreboot users are used to a very quick boot and the memory training takes very long; and will probably take even longer in the future. I could imagine users (even myself) getting impatient and hitting the power button. Is that the reason for the on-screen message? How long does the memory training actually take?
Nico
Nico Huber nico.h@gmx.de writes:
Hi Jeremy,
welcome to the mailinglist! It's nice to see libgfxinit working for you and in this special case. :)
On 22.11.22 03:51, Compostella, Jeremy wrote:
In order to improve the user experience we would like to display a message on the screen while memory training is in progress.
I was wondering so far what the use case would be. Thinking about it for a moment, I guess this makes much sense. coreboot users are used to a very quick boot and the memory training takes very long; and will probably take even longer in the future. I could imagine users (even myself) getting impatient and hitting the power button. Is that the reason for the on-screen message?
Indeed this is the main motivation behind this. However, very early graphic initialization opens the door to other feature such as displaying early boot coreboot message for debug purposes ...
How long does the memory training actually take?
It depends on plenty of parameters such as the SoC generation, the type of RAM, the size of RAM, the number of RAM modules ... I have seen it vary from 30s to two minutes over the last 9 years of work of various platform and projects.
Regarding the libgfxinit integration, I don't know if you noticed but I made some progress and I am going to need your help to close on it.
1. Using the static MMIO driver got me rid of the `hw__gfx__gma__registers__{gtt_32,gtt_64,regs}__range_aXnnn' initialized symbols. This is a suggestion you made a long time ago and I am glad I was able to leverage it. 2. I modified two boolean variables of the libhwbase debug module (https://review.coreboot.org/c/libhwbase/+/69854) to make their "default logical value" `False', remove their default value setting and move their symbol to the `.bss' section. 3. I performed the same operation for the remaining variable in libgfxinit (https://review.coreboot.org/c/coreboot/+/69757) 4. Then I am left with elaboration functions only which I noticed are zeros and it allowed to make a "clean" and simple change in the linker script (https://review.coreboot.org/c/coreboot/+/69757/4/src/arch/x86/car.ld#70)
As an alternative to #2 and #3 which I guess is going to be a concern for you (machine checker ...) we could create a variable naming convention which would could used by the linker script to filter symbols in the `.bss' section.
What do think ? Do you have any other suggestion ? Do you think the current implementation/proposal is acceptable ?
Hi
It's surprising this works before raminit, nice find! I'm not so convinced it's an improvement for enduser friendliness. Is waiting at a screen that tells you raminit is being done better than waiting at a black screen? It might just be more confusing? But I'm not an enduser so I can't really say.
About the implementation details:
PCI resource allocation
=======================
< https://review.coreboot.org/c/coreboot/+/69757/2/src/soc/intel/alderlake/rom...
We need to configure the graphics PCI device to perform graphic initialization. It requires to perform an early romstage PCI resource allocation. For the proof-of-concept, we assigned the same resources than the coreboot resource allocator would assign during ramstage. We cannot leave hardcoded values like these. Any suggestion on how to cleanly handle early PCI resources allocation ?
Hardcode it with a Kconfig constant is good enough I think.
<
https://review.coreboot.org/c/coreboot/+/69757/2/src/drivers/intel/fsp2_0/Kc...
https://review.coreboot.org/c/coreboot/+/69757/2/src/lib/Kconfig https://review.coreboot.org/c/coreboot/+/69757/2/src/device/Kconfig
We had to "duplicate" some configuration flags such as `MAINBOARD_USE_LIBGFXINIT' as a `ROMSTAGE_MAINBOARD_USE_LIBGFXINIT' for instance. We want to separate configuration between romstage and ramstage to offer flexibility to use to have VGA text mode in romstage without any graphics initialization in ramstage or a graphic initialization with the GOP driver in ramstage ...
We would appreciate some review and feedback on our clearly not perfect `Kconfig' changes. Any suggestion is welcome.
I'd would have the option to run libgfxinit in romstage at the soc level
but still depend on MAINBOARD_USE_LIBGFXINIT.
libgfxinit and global initialized variables
===========================================
https://review.coreboot.org/c/coreboot/+/69757/1/src/arch/x86/car.ld
`libgfxinit' has some initialized global variables ending up in the `.data' section.
Global *initialized* variables are not permitted in cache-as-ram and with the current `car.ld' linker script I hit compilation link errors due the following list of variables. Using objdump I collected their initial value (note that I did not exhaustively checked all the `[...]_E' ones but only a sample of them).
Address Type Name Value
ffffff60 d `hw__debug__register_write_delay_nanoseconds' 00
ffffff68 d `hw__debug__start_of_line' 01
ffffff2c d `hw__gfx__gma__registers__gtt_32__range_aXnnn' 00 20 00 00 ffffff28 d `hw__gfx__gma__registers__gtt_64__range_aXnnn' 00 80 00 00 ffffff30 d `hw__gfx__gma__registers__regs__range_aXnnn' 00 00 00 00 ffffff6e D `gma_E' 00 ...
ffffff70 D `gma__gfx_init_E' 00 ...
ffffff6a D `hw__debug_E' 00 ...
ffffff6c D `hw__debug_sink_E' 00 ...
ffffff02 D `hw__gfx__dp_aux_ch_E' 00 ...
ffffff04 D `hw__gfx__dp_dual_mode_E' 00 ...
ffffff06 D `hw__gfx__dp_info_E' 00 ...
ffffff08 D `hw__gfx__dp_training_E' 00 ...
ffffff0a D `hw__gfx__edid_E' 00 ...
ffffff0c D `hw__gfx__framebuffer_filler_E' 00 ...
ffffff3a D `hw__gfx__gma_E' 00 ...
ffffff3c D `hw__gfx__gma__combo_phy_E' 00 ...
ffffff00 D `hw__gfx__gma__config_E' 00 ...
ffffff0e D `hw__gfx__gma__config_helpers_E' 00 ...
ffffff10 D `hw__gfx__gma__connector_info_E' 00 ...
ffffff42 D `hw__gfx__gma__connectors_E' 00 ...
ffffff3e D `hw__gfx__gma__connectors__combo_phy_E' 00 ...
ffffff40 D `hw__gfx__gma__connectors__tc_E' 00 ...
ffffff12 D `hw__gfx__gma__display_probing_E' 00 ...
ffffff14 D `hw__gfx__gma__dp_aux_request_E' 00 ...
ffffff16 D `hw__gfx__gma__i2c_E' 00 ...
ffffff38 D `hw__gfx__gma__initialized' 00 ...
ffffff18 D `hw__gfx__gma__panel_E' 00 ...
ffffff1a D `hw__gfx__gma__pch__fdi_E' 00 ...
ffffff1c D `hw__gfx__gma__pch__sideband_E' 00 ...
ffffff1e D `hw__gfx__gma__pch__transcoder_E' 00 ...
ffffff20 D `hw__gfx__gma__pch__vga_E' 00 ...
ffffff22 D `hw__gfx__gma__pcode_E' 00 ...
ffffff24 D `hw__gfx__gma__pipe_setup_E' 00 ...
ffffff48 D `hw__gfx__gma__plls_E' 00 ...
ffffff44 D `hw__gfx__gma__plls__combo_phy_E' 00 ...
ffffff46 D `hw__gfx__gma__plls__dkl_E' 00 ...
ffffff4a D `hw__gfx__gma__port_detect_E' 00 ...
ffffff50 D `hw__gfx__gma__power_and_clocks_E' 00 ...
ffffff4c D `hw__gfx__gma__power_and_clocks__tgl_E' 00 ...
ffffff4e D `hw__gfx__gma__power_and_clocks__xelpd_E' 00 ...
ffffff34 D `hw__gfx__gma__registers_E' 00 ...
ffffff36 D `hw__gfx__gma__transcoder_E' 00 ...
ffffff52 D `hw__mmio_range_E' 00 ...
ffffff56 D `hw__mmio_regs_E' 00 ...
ffffff54 D `hw__pci__dev_E' 00 ...
ffffff58 D `hw__pci__mmconf_E' 00 ...
ffffff5a D `hw__port_io_E' 00 ...
ffffff5c D `hw__time_E' 00 ...
ffffff72 D `hw__time__timer_E' 00 ...
The symbols ending with `_E' are empty and initialized by GNAT generated code (cf. `___elabs' functions) which is called using the `$stage_adainit()' function. Therefor, I think it is safe to places these into the `_bss' region of the `.car.data' section. What I don't understand though is why isn't the GNAT compiler placing them into the `.bss' region of the object file. Any idea ? I could not find any GNAT compiler option for this but I am clearly not an ADA compiler expert :-)
If you are going to have the linker use .data in .bss try to only do that for those symbols and not the whole data section as the illegal globals check is a good one to keep.
Now we are left with the following symbols:
- `hw__debug__register_write_delay_nanoseconds' is zero so installing it in the `_bss' region of the `.car.data' section is "technically" fine.
- `hw__debug__start_of_line': Reverting the logic of this boolean makes it
work
- The following three symbols are the most challenging ones:
They seem to be entirely by the compiler and I don't know how to control them. Surprisingly, putting them in the `_bss' region of the `.car.data' section which is making them zeros is not causing any apparent issue while their initial value should not be zeros. I am thinking that these could be used for code verification purposes only. Does anyone knows before I adventure in to assembly code to uncover their role ?
- `hw__gfx__gma__registers__gtt_32__range_aXnnn'
- `hw__gfx__gma__registers__gtt_64__range_aXnnn'
- `hw__gfx__gma__registers__regs__range_aXnnn'
One trick you could use is to put a backup of the .data section in .rodata with objcopy and copy it back at runtime very early in the stage. This would make it possible to have a .data section on CAR stages in general.
Kind regards
Arthur
On Tue, Nov 22, 2022 at 3:52 AM Compostella, Jeremy < jeremy.compostella@intel.com> wrote:
Dear coreboot developers,
In order to improve the user experience we would like to display a message on the screen while memory training is in progress. As a first step, we are working on Alder Lake and Raptor Lake platforms and are leveraging the `libgfxinit' and VGA text mode. We have libgfxinit patches for Tiger Lake, Alder Lake (and Raptor Lake) support under review but this is not the object of this email (see https://review.coreboot.org/c/libgfxinit/+/65087 if you are interested)
In the last few weeks, we have been working on a proof-of-concept to ensure that we could overcome all the technical hardware challenges to initialize VGA text mode and setup the display before we jump into FSP memory.
Even though we are still working on some stability issues, we have a "working" but hacky patch :-). But we have an aggressive timeline and it works good enough that we believe it is time to start an open discussion as the surface of the patch is important (linker scripts, configuration flags logic ...). The changes have been reduced to a unique patch after a lot of clean-up (https://review.coreboot.org/c/coreboot/+/69757). This patch will be split up in a couple of patches once we are done with some in progress development.
We identified three majors "issues" for which we are seeking feedback, guidance or help.
PCI resource allocation
< https://review.coreboot.org/c/coreboot/+/69757/2/src/soc/intel/alderlake/rom...
We need to configure the graphics PCI device to perform graphic initialization. It requires to perform an early romstage PCI resource allocation. For the proof-of-concept, we assigned the same resources than the coreboot resource allocator would assign during ramstage. We cannot leave hardcoded values like these. Any suggestion on how to cleanly handle early PCI resources allocation ?
Configuration flags and "duplicated"
< https://review.coreboot.org/c/coreboot/+/69757/2/src/drivers/intel/fsp2_0/Kc...
https://review.coreboot.org/c/coreboot/+/69757/2/src/lib/Kconfig https://review.coreboot.org/c/coreboot/+/69757/2/src/device/Kconfig
We had to "duplicate" some configuration flags such as `MAINBOARD_USE_LIBGFXINIT' as a `ROMSTAGE_MAINBOARD_USE_LIBGFXINIT' for instance. We want to separate configuration between romstage and ramstage to offer flexibility to use to have VGA text mode in romstage without any graphics initialization in ramstage or a graphic initialization with the GOP driver in ramstage ...
We would appreciate some review and feedback on our clearly not perfect `Kconfig' changes. Any suggestion is welcome.
libgfxinit and global initialized variables
https://review.coreboot.org/c/coreboot/+/69757/1/src/arch/x86/car.ld
`libgfxinit' has some initialized global variables ending up in the `.data' section.
Global *initialized* variables are not permitted in cache-as-ram and with the current `car.ld' linker script I hit compilation link errors due the following list of variables. Using objdump I collected their initial value (note that I did not exhaustively checked all the `[...]_E' ones but only a sample of them).
Address Type Name Value
ffffff60 d `hw__debug__register_write_delay_nanoseconds' 00
ffffff68 d `hw__debug__start_of_line' 01
ffffff2c d `hw__gfx__gma__registers__gtt_32__range_aXnnn' 00 20 00 00 ffffff28 d `hw__gfx__gma__registers__gtt_64__range_aXnnn' 00 80 00 00 ffffff30 d `hw__gfx__gma__registers__regs__range_aXnnn' 00 00 00 00 ffffff6e D `gma_E' 00 ...
ffffff70 D `gma__gfx_init_E' 00 ...
ffffff6a D `hw__debug_E' 00 ...
ffffff6c D `hw__debug_sink_E' 00 ...
ffffff02 D `hw__gfx__dp_aux_ch_E' 00 ...
ffffff04 D `hw__gfx__dp_dual_mode_E' 00 ...
ffffff06 D `hw__gfx__dp_info_E' 00 ...
ffffff08 D `hw__gfx__dp_training_E' 00 ...
ffffff0a D `hw__gfx__edid_E' 00 ...
ffffff0c D `hw__gfx__framebuffer_filler_E' 00 ...
ffffff3a D `hw__gfx__gma_E' 00 ...
ffffff3c D `hw__gfx__gma__combo_phy_E' 00 ...
ffffff00 D `hw__gfx__gma__config_E' 00 ...
ffffff0e D `hw__gfx__gma__config_helpers_E' 00 ...
ffffff10 D `hw__gfx__gma__connector_info_E' 00 ...
ffffff42 D `hw__gfx__gma__connectors_E' 00 ...
ffffff3e D `hw__gfx__gma__connectors__combo_phy_E' 00 ...
ffffff40 D `hw__gfx__gma__connectors__tc_E' 00 ...
ffffff12 D `hw__gfx__gma__display_probing_E' 00 ...
ffffff14 D `hw__gfx__gma__dp_aux_request_E' 00 ...
ffffff16 D `hw__gfx__gma__i2c_E' 00 ...
ffffff38 D `hw__gfx__gma__initialized' 00 ...
ffffff18 D `hw__gfx__gma__panel_E' 00 ...
ffffff1a D `hw__gfx__gma__pch__fdi_E' 00 ...
ffffff1c D `hw__gfx__gma__pch__sideband_E' 00 ...
ffffff1e D `hw__gfx__gma__pch__transcoder_E' 00 ...
ffffff20 D `hw__gfx__gma__pch__vga_E' 00 ...
ffffff22 D `hw__gfx__gma__pcode_E' 00 ...
ffffff24 D `hw__gfx__gma__pipe_setup_E' 00 ...
ffffff48 D `hw__gfx__gma__plls_E' 00 ...
ffffff44 D `hw__gfx__gma__plls__combo_phy_E' 00 ...
ffffff46 D `hw__gfx__gma__plls__dkl_E' 00 ...
ffffff4a D `hw__gfx__gma__port_detect_E' 00 ...
ffffff50 D `hw__gfx__gma__power_and_clocks_E' 00 ...
ffffff4c D `hw__gfx__gma__power_and_clocks__tgl_E' 00 ...
ffffff4e D `hw__gfx__gma__power_and_clocks__xelpd_E' 00 ...
ffffff34 D `hw__gfx__gma__registers_E' 00 ...
ffffff36 D `hw__gfx__gma__transcoder_E' 00 ...
ffffff52 D `hw__mmio_range_E' 00 ...
ffffff56 D `hw__mmio_regs_E' 00 ...
ffffff54 D `hw__pci__dev_E' 00 ...
ffffff58 D `hw__pci__mmconf_E' 00 ...
ffffff5a D `hw__port_io_E' 00 ...
ffffff5c D `hw__time_E' 00 ...
ffffff72 D `hw__time__timer_E' 00 ...
The symbols ending with `_E' are empty and initialized by GNAT generated code (cf. `___elabs' functions) which is called using the `$stage_adainit()' function. Therefor, I think it is safe to places these into the `_bss' region of the `.car.data' section. What I don't understand though is why isn't the GNAT compiler placing them into the `.bss' region of the object file. Any idea ? I could not find any GNAT compiler option for this but I am clearly not an ADA compiler expert :-)
Now we are left with the following symbols:
- `hw__debug__register_write_delay_nanoseconds' is zero so installing it in the `_bss' region of the `.car.data' section is "technically" fine.
- `hw__debug__start_of_line': Reverting the logic of this boolean makes it
work
- The following three symbols are the most challenging ones:
They seem to be entirely by the compiler and I don't know how to control them. Surprisingly, putting them in the `_bss' region of the `.car.data' section which is making them zeros is not causing any apparent issue while their initial value should not be zeros. I am thinking that these could be used for code verification purposes only. Does anyone knows before I adventure in to assembly code to uncover their role ?
- `hw__gfx__gma__registers__gtt_32__range_aXnnn'
- `hw__gfx__gma__registers__gtt_64__range_aXnnn'
- `hw__gfx__gma__registers__regs__range_aXnnn'
Regards,
-- *Jeremy* /One Emacs to rule them all/ _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
I like the overall idea.
On 11/22/22 14:07, Arthur Heymans wrote:
Hi
It's surprising this works before raminit, nice find! I'm not so convinced it's an improvement for enduser friendliness. Is waiting at a screen that tells you raminit is being done better than waiting at a black screen? It might just be more confusing? But I'm not an enduser so I can't really say.
What if FSP reported the progress of various training steps (e.g. ECT started/finished) using the vga text calls? @Jeremy? Would that make more sense @Arthur?
It would probably need a new UPD to enable such behavior.
Best regards,
Michał Żygowski michal.zygowski@3mdeb.com writes:
I like the overall idea.
On 11/22/22 14:07, Arthur Heymans wrote:
Hi It's surprising this works before raminit, nice find! I'm not so convinced it's an improvement for enduser friendliness. Is waiting at a screen that tells you raminit is being done better than waiting at a black screen? It might just be more confusing? But I'm not an enduser so I can't really say.
What if FSP reported the progress of various training steps (e.g. ECT started/finished) using the vga text calls? @Jeremy? Would that make more sense @Arthur?
This is a good suggestion. We'll keep that in mind.
Hi *Arthur*,
Hardcode it with a Kconfig constant is good enough I think.
That's also what I guessed and this is what I did. I actually reused the existing `CONFIG_GFX_GMA_DEFAULT_MMIO' Kconfig for that purpose. I had to make a twist in the Makefile to convert to ADA number representation though (see [drivers/intel/gma/Makefile.inc#65]).
I'd would have the option to run libgfxinit in romstage at the soc level but still depend on MAINBOARD_USE_LIBGFXINIT.
Could you clarify your thought ? I could not make sense of this.
if you are going to have the linker use .data in .bss try to only do that for those symbols and not the whole data section as the illegal globals check is a good one to keep.
I also have exactly that (see [arch/x86/car.ld#70]).
One trick you could use is to put a backup of the .data section in .rodata with objcopy and copy it back at runtime very early in the stage. This would make it possible to have a .data section on CAR stages in general.
This is something I have not explored. I thought about trying something like that but I could not even find any `.rodata' section in the romstage binary or `car.ld' script. But this is good suggestion, if my current proposal is not accepted or lead to too many issues I'll have a look into this.
Instead of involving objcpy, I I could try to create a region in `.car.data' of the size of the `.data' section, make the linker script move all the `.data' symbols to a `.init_data' section and modify `src/arch/x86/assembly_entry.S' to perform the copy at runtime before jumping into `car_stage_entry' after clearing the `.bss' section.
Hi *Arthur*,
Regarding the last point, I looked a little bit more into it and I feel like objcopy may indeed be needed :sad:
Regards,
Dear community,
I have followed suggestions from this mail thread, fixed issues, cleaned the code, reworked Makefiles, fixed stability issues ... and I have submitted the following patches for review:
1. Add option to use Ada code in romstage https://review.coreboot.org/c/coreboot/+/70274 2. lib: Hook up libhwbase in romstage https://review.coreboot.org/c/coreboot/+/70275 3. drivers/intel/gma: Hook up libgfxinit in romstage https://review.coreboot.org/c/coreboot/+/70276 4. drivers/pc80/vga: display a string centered on a line https://review.coreboot.org/c/coreboot/+/70277 5. drivers/pc80/vga: Add romstage support https://review.coreboot.org/c/coreboot/+/70278 6. soc/intel/common/block: Add Intel VGA early graphics support https://review.coreboot.org/c/coreboot/+/70299 7. drivers/intel/fsp2_0/memory_init: inform user of memory training https://review.coreboot.org/c/coreboot/+/70300 8. drivers/intel/gma: Enable AlderLake libgfxinit support https://review.coreboot.org/c/coreboot/+/70301 9. cpu/intel/car/romstage: Clear graphics before exiting romstage https://review.coreboot.org/c/coreboot/+/70302 10. soc/intel/alderlake: Add romstage early graphics support https://review.coreboot.org/c/coreboot/+/70303 11. mb/google/brya: Add romstage early graphics for brya and skolas https://review.coreboot.org/c/coreboot/+/70304
Thanks for your support,
Hi Jeremy,
This is a fun and really useful improvement!
Compostella, Jeremy wrote:
I have followed suggestions from this mail thread, fixed issues, cleaned the code, reworked Makefiles, fixed stability issues ...
How much do you think can be reused by older platforms, including Intel platforms not using FSP in coreboot?
Thanks
//Peter
-
Arthur Heymans -
Compostella, Jeremy -
Michał Żygowski -
Nico Huber -
Peter Stuge