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.
–
Jeremy
One Emacs to rule them all
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/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 ?
Hardcode it with a Kconfig constant is good enough I think.
<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.
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:
- `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 ?
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