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/
_______________________________________________
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-leave@coreboot.org