Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/intel/{apl,glk}: add options to configure GPU ......................................................................
soc/intel/{apl,glk}: add options to configure GPU
Adds options to select the primary GPU device and configure IGD, which allows to override the appropriate FSP options in the SoC code. These changes do not affect the configuration of the boards with the Apollo Lake and Gemini Lake processors, because if these parameters are not defined in the devicetree, they will be set to the default values.
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 64 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/1
diff --git a/src/soc/intel/apollolake/chip.h b/src/soc/intel/apollolake/chip.h index 40cd39b..0375cec 100644 --- a/src/soc/intel/apollolake/chip.h +++ b/src/soc/intel/apollolake/chip.h @@ -44,6 +44,46 @@ /* Common structure containing soc config data required by common code*/ struct soc_intel_common_config common_soc_config;
+ /* Select DVMT 5.0 Pre-Allocated (Fixed) Graphics Memory size */ + enum { + DVMT_64MB = 2, /* Default */ + DVMT_96MB, + DVMT_128MB, + DVMT_160MB, + DVMT_192MB, + DVMT_224MB, + DVMT_256MB, + DVMT_288MB, + DVMT_320MB, + DVMT_352MB, + DVMT_384MB, + DVMT_416MB, + DVMT_448MB, + DVMT_480MB, + DVMT_512MB, + } igd_dvmt_50_pre_alloc_size; + + /* Select the Aperture Size for GPU device */ + enum { + APERTURE_128MB = 1, /* Default */ + APERTURE_256MB, + APERTURE_512MB, + } igd_aperture_size; + + /* Select the GTT Size for GPU device */ + enum { + GTT_2MB = 1, + GTT_4MB, + GTT_8MB, /* Default */ + } igd_gtt_size; + + /* Select primary GPU device */ + enum { + PRIMARY_AUTO = 0, /* Default */ + PRIMARY_IGD = 2, + PRIMARY_PCI = 3, + } primary_gpu; + /* * Mapping from PCIe root port to CLKREQ input on the SOC. The SOC has * four CLKREQ inputs, but six root ports. Root ports without an diff --git a/src/soc/intel/apollolake/romstage.c b/src/soc/intel/apollolake/romstage.c index 05cd0db..1576802 100644 --- a/src/soc/intel/apollolake/romstage.c +++ b/src/soc/intel/apollolake/romstage.c @@ -264,12 +264,34 @@ } }
+static void soc_gpu_init_params(FSPM_UPD *mupd) +{ + FSP_M_CONFIG *m_cfg = &mupd->FspmConfig; + const struct soc_intel_apollolake_config *soc_cfg = config_of_soc(); + const struct device *dev = pcidev_path_on_root(SA_DEVFN_IGD); + + m_cfg->PrimaryVideoAdaptor = soc_cfg->primary_gpu; + if (dev && dev->enabled && (soc_cfg->primary_gpu != PRIMARY_PCI)) { + /* + * Override FSP settings for IGD only if they are set in the devicetree. + * Otherwise, the default values from UPD will be used for them + */ + if (soc_cfg->igd_dvmt_50_pre_alloc_size) + m_cfg->IgdDvmt50PreAlloc = soc_cfg->igd_dvmt_50_pre_alloc_size; + + if (soc_cfg->igd_aperture_size) + m_cfg->IgdApertureSize = soc_cfg->igd_aperture_size; + + if (soc_cfg->igd_gtt_size) + m_cfg->GttSize = soc_cfg->igd_gtt_size; + } +} + static void soc_memory_init_params(FSPM_UPD *mupd) { #if CONFIG(SOC_INTEL_GLK) /* Only for GLK */ FSP_M_CONFIG *m_cfg = &mupd->FspmConfig; - m_cfg->PrmrrSize = get_prmrr_size();
/* @@ -307,6 +329,7 @@ check_full_retrain(mupd);
fill_console_params(mupd); + soc_gpu_init_params(mupd);
if (CONFIG(SOC_INTEL_GLK)) soc_memory_init_params(mupd);
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/intel/{apl,glk}: add options to configure GPU ......................................................................
Patch Set 1:
Why make these devicetree settings? Shouldn't these be user-configurable, e.g. Kconfig? Should it be synchronized with CONFIG_ONBOARD_VGA_IS_PRIMARY?
Hello Felix Singer, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#2).
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY
This patch allows to install the primary graphics device, based on the ONBOARD_VGA_IS_PRIMARY config. Thus, there is no need to do this in the board configuration.
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 20 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/2
Hello Felix Singer, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#3).
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY
This patch allows to install the primary graphics device, based on the ONBOARD_VGA_IS_PRIMARY config. Thus, there is no need to do this in the board configuration.
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/3
Hello Felix Singer, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#4).
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY
This patch allows to install the primary graphics device, based on the ONBOARD_VGA_IS_PRIMARY config. Thus, there is no need to do this in the board configuration.
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/4
Hello Felix Singer, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#5).
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY
This patch allows to install the primary graphics device, based on the ONBOARD_VGA_IS_PRIMARY config. Thus, there is no need to do this in the board configuration.
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 20 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/5
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 5:
Please update all existing board to select ONBOARD_VGA_IS_PRIMARY by default to keep the current behaviour.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 5:
What does FSP do if we set it to PRIMARY_PCI but there is no gfx card plugged in?
Also, please note that the two options don't do the same, ONBOARD_VGA_IS_PRIMARY takes the devicetree into account, for FSP it's just IGD vs. anything else. I wouldn't mind some unification in that area, though.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39374/7/src/soc/intel/apollolake/ro... File src/soc/intel/apollolake/romstage.c:
https://review.coreboot.org/c/coreboot/+/39374/7/src/soc/intel/apollolake/ro... PS7, Line 259: PrimaryVideoAdaptor What does it do and why is it needed?
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 7:
(1 comment)
Patch Set 5:
What does FSP do if we set it to PRIMARY_PCI but there is no gfx card plugged in?
In this case, IGD should be used, since this option set the priority device only. Unfortunately, I can not test this with Appollo Lake CPU, because my board doesn't have the necessary connectors, but it was tested on the boards with Skylake (CB:32044).
https://review.coreboot.org/c/coreboot/+/39374/7/src/soc/intel/apollolake/ro... File src/soc/intel/apollolake/romstage.c:
https://review.coreboot.org/c/coreboot/+/39374/7/src/soc/intel/apollolake/ro... PS7, Line 259: PrimaryVideoAdaptor
What does it do and why is it needed?
I suppose that it is necessary to select an active graphics device https://github.com/intel/FSP/blob/dd34b8c451a23c62a972419287021abfb7d01c2d/A...
In the case of Skylake (See CB:32172), when Display_PEG was selected, Linux was output to an external graphics device. At the same time, IGD was also enabled.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39374/7/src/soc/intel/apollolake/ro... File src/soc/intel/apollolake/romstage.c:
https://review.coreboot.org/c/coreboot/+/39374/7/src/soc/intel/apollolake/ro... PS7, Line 259: PrimaryVideoAdaptor
I suppose that it is necessary to select an active graphics device […]
I assume it configures decoding of legacy VGA cycles. And it is actually necessary to do that early if FSP locks the GGC register. Would be good to check if it does. Otherwise, we could ignore this option and do it properly in coreboot.
Did somebody test what FSPs "AUTO" option does? If it prefers PCI over IGD, I would use that here (unless it falls back to IGD if no PCI is detected anyway).
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39374/7/src/soc/intel/apollolake/ro... File src/soc/intel/apollolake/romstage.c:
https://review.coreboot.org/c/coreboot/+/39374/7/src/soc/intel/apollolake/ro... PS7, Line 259: PrimaryVideoAdaptor
I assume it configures decoding of legacy VGA cycles. And it is actually […]
For Skylake AUTO is equivalent to IDG. I don't think that in our case intel did differently.
I suggest adding an option to install this from the devicetree, as was done earlier ( https://review.coreboot.org/c/coreboot/+/39374/1/src/soc/intel/apollolake/ro... ) to configure this from the board configuration.
I will try to test when I can get to the office but it just will not be soon. I'll try to connect a graphics card through the mini PCIe -> PCIe x1 slot adapter.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 7:
Just a ping to myself that I wanted to test if the register gets locked early....
Hello Felix Singer, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#16).
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY
This patch allows to install the primary graphics device, based on the ONBOARD_VGA_IS_PRIMARY config. Thus, there is no need to do this in the board configuration.
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 20 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/16
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 17:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3, This is completely wrong. FSP actually honors:
PRIMARY_IGD = 0, PRIMARY_PCI = 1,
Other values are handled as if they were PCI.
Hello Felix Singer, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#18).
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY
This patch allows to install the primary graphics device, based on the ONBOARD_VGA_IS_PRIMARY config. Thus, there is no need to do this in the board configuration.
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 20 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/18
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
This is completely wrong. FSP actually honors: […]
But judging by the UPD (https://github.com/intel/FSP/blob/master/ApolloLakeFspBinPkg/Include/FspmUpd...), this is correct. Or maybe I misunderstood this description?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 19:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
But judging by the UPD (https://github. […]
The UPD description is completely wrong. Code within FSP only checks whether this variable is zero or not.
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 20:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
The UPD description is completely wrong. […]
Unfortunately I'm unable to load the image when this parameter is set to 0: https://pastebin.com/TmevCE70 "UEFIPayload" payload, edk2-stable201903-1569-g3e63a91
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 22:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
Unfortunately I'm unable to load the image when this parameter is set to 0: […]
Ack
https://review.coreboot.org/c/coreboot/+/39374/7/src/soc/intel/apollolake/ro... File src/soc/intel/apollolake/romstage.c:
https://review.coreboot.org/c/coreboot/+/39374/7/src/soc/intel/apollolake/ro... PS7, Line 259: PrimaryVideoAdaptor
For Skylake AUTO is equivalent to IDG. I don't think that in our case intel did differently. […]
Ack
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 22:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
Ack
Uhm, this isn't solved, though. Did you see any difference in FSP behavior when choosing IGD over PCI? The only difference is that a non-zero value sets a bit in GGC to indicate that the IGD isn't the primary video adaptor.
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: set GPU based on CONFIG_ONBOARD_VGA_IS_PRIMARY ......................................................................
Patch Set 22:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/17/src/soc/intel/apollolake/c... PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
Uhm, this isn't solved, though. […]
This works the same as for Skylake. If the option is set to PRIMARY_IGD, then Linux output video to the monitor that is connected to the port from IGD. But if you set it to PRIMARY_PCI, then the video will be output to the monitor connected to the external GPU.
I connect an external GPU to miniPCIe and tested it.
Hello Felix Singer, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#23).
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
soc/{apl,glk}: Allow to select the primary graphics device
Allow to select the primary graphics device between the internal IGD and the external PCIe GPU depending on the ONBOARD_VGA_IS_PRIMARY config.
TEST = Set PRIMARY_PCI and boot Linux on the Kontron mAL10 [1] with the Silicon Motion SM750 chip in the miniPCIe video adapter. As a result, it shows the Tianocore logo/setup menu and the Xubuntu desktop.
[1] https://review.coreboot.org/c/coreboot/+/39133
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 20 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/23
Hello Felix Singer, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#24).
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
soc/{apl,glk}: Allow to select the primary graphics device
Allow to select the primary graphics device between the IGD and the external PCIe GPU depending on the ONBOARD_VGA_IS_PRIMARY config.
TEST = Set PRIMARY_PCI and boot Linux on the Kontron mAL10 [1] with the miniPCIe video adapter on the Silicon Motion SM750 controller. As a result, it shows the Tianocore logo/setup menu and the desktop on the display.
[1] https://review.coreboot.org/c/coreboot/+/39133
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 20 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/24
Hello Felix Singer, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#25).
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
soc/{apl,glk}: Allow to select the primary graphics device
Allow to select the primary graphics device between the IGD and the external PCIe GPU depending on the ONBOARD_VGA_IS_PRIMARY config. The option sets the priority only. This means that if a high priority is set for an external PCI device and it is not connected/not enabled, then the device with a lower priority will be used, in our case it is IGD.
TEST = Set PRIMARY_PCI and boot Linux on the Kontron mAL10 [1] with the miniPCIe video adapter on the Silicon Motion SM750 controller. As a result, it shows the Tianocore logo/setup menu and the desktop on the display.
[1] https://review.coreboot.org/c/coreboot/+/39133
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 20 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/25
Hello Felix Singer, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#27).
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
soc/{apl,glk}: Allow to select the primary graphics device
Allow to select the primary graphics device between the IGD and the external PCIe GPU depending on the ONBOARD_VGA_IS_PRIMARY config. The option sets the priority only. This means that if a high priority is set for an external PCI device and it is not connected/not enabled, then the device with a lower priority will be used, in our case it is IGD.
TEST = Set PRIMARY_PCI and boot Linux on the Kontron mAL10 [1] with the miniPCIe video adapter on the Silicon Motion SM750 controller. As a result, it shows the Tianocore logo/setup menu and the desktop on the display.
[1] https://review.coreboot.org/c/coreboot/+/39133
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/chip.h M src/soc/intel/apollolake/romstage.c 2 files changed, 16 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/27
Attention is currently required from: Maxim Polyakov, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 29:
(2 comments)
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/1fd46468_45a64403 PS29, Line 33: }; `chip.h` is an API between our platform/driver and mainboard code. This is FSP specific, though. It should be moved to the code. I'd place it on the top inside soc_gpu_init_params().
If I interpreted your test results correctly, this change looks good otherwise.
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/fabc38f9_787ba9f6 PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
This works the same as for Skylake. […]
Angel, did you check some code drop or the current binary? If it really makes a difference if we set 2 or 3, this change seems valid. However, I have to say: as it is common when we add features that nobody needs yet, it can waste a lot of time. We don't know what will happen first: a) FSP vanishes, b) somebody will make use of this feature.
Attention is currently required from: Maxim Polyakov, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 29: Code-Review+1
Attention is currently required from: Maxim Polyakov, Angel Pons. Hello Felix Singer, build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#30).
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
soc/{apl,glk}: Allow to select the primary graphics device
Allow to select the primary graphics device between the IGD and the external PCIe GPU depending on the ONBOARD_VGA_IS_PRIMARY config. The option sets the priority only. This means that if a high priority is set for an external PCI device and it is not connected/not enabled, then the device with a lower priority will be used, in our case it is IGD.
TEST = Set PRIMARY_PCI and boot Linux on the Kontron mAL10 [1] with the miniPCIe video adapter on the Silicon Motion SM750 controller. As a result, the display connected to an external GPU device shows the Tianocore logo + setup menu and the desktop.
[1] https://review.coreboot.org/c/coreboot/+/39133
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/romstage.c 1 file changed, 13 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/30
Attention is currently required from: Nico Huber, Angel Pons. Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 30:
(1 comment)
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/a2bcd562_28eaf3ff PS29, Line 33: };
`chip.h` is an API between our platform/driver and mainboard code. This […]
Done
Attention is currently required from: Nico Huber, Maxim Polyakov. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 30: Code-Review+1
(1 comment)
File src/soc/intel/apollolake/romstage.c:
https://review.coreboot.org/c/coreboot/+/39374/comment/fa7b824a_b4488ace PS30, Line 239: mupd->FspmConfig.PrimaryVideoAdaptor = : is_devfn_enabled(SA_DEVFN_IGD) && CONFIG(ONBOARD_VGA_IS_PRIMARY) ? : GPU_PRIMARY_IGD : GPU_PRIMARY_PCI; nit: I'd do the Kconfig check first so that the compiler can optimise things out when it's false. Also, I'd split this into multiple statements for clarity:
if (CONFIG(ONBOARD_VGA_IS_PRIMARY) && is_devfn_enabled(SA_DEVFN_IGD)) mupd->FspmConfig.PrimaryVideoAdaptor = GPU_PRIMARY_IGD; else mupd->FspmConfig.PrimaryVideoAdaptor = GPU_PRIMARY_PCI;
Attention is currently required from: Nico Huber, Maxim Polyakov. Hello Felix Singer, build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#31).
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
soc/{apl,glk}: Allow to select the primary graphics device
Allow to select the primary graphics device between the IGD and the external PCIe GPU depending on the ONBOARD_VGA_IS_PRIMARY config. The option sets the priority only. This means that if a high priority is set for an external PCI device and it is not connected/not enabled, then the device with a lower priority will be used, in our case it is IGD.
TEST = Set PRIMARY_PCI and boot Linux on the Kontron mAL10 [1] with the miniPCIe video adapter on the Silicon Motion SM750 controller. As a result, the display connected to an external GPU device shows the Tianocore logo + setup menu and the desktop.
[1] https://review.coreboot.org/c/coreboot/+/39133
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/romstage.c 1 file changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/31
Attention is currently required from: Nico Huber, Angel Pons. Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 31:
(1 comment)
File src/soc/intel/apollolake/romstage.c:
https://review.coreboot.org/c/coreboot/+/39374/comment/0ded6962_624a5288 PS30, Line 239: mupd->FspmConfig.PrimaryVideoAdaptor = : is_devfn_enabled(SA_DEVFN_IGD) && CONFIG(ONBOARD_VGA_IS_PRIMARY) ? : GPU_PRIMARY_IGD : GPU_PRIMARY_PCI;
nit: I'd do the Kconfig check first so that the compiler can optimise things out when it's false. […]
Thx Done
Attention is currently required from: Nico Huber, Maxim Polyakov, Angel Pons, Werner Zeh. Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 31: Code-Review+1
(1 comment)
Patchset:
PS31: I have tested your patch on mc_apl5. Our graphic works whit this patch.
Attention is currently required from: Nico Huber, Maxim Polyakov, Angel Pons, Werner Zeh. Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 31:
(1 comment)
Patchset:
PS31:
I have tested your patch on mc_apl5. Our graphic works whit this patch.
with
Attention is currently required from: Nico Huber, Mario Scheithauer, Angel Pons, Werner Zeh. Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 31:
(3 comments)
Patchset:
PS31:
with
Ok, thanks for testing!
PS31: @Angel, @Nico, do you have any other comments?
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/5f015490_e52509e9 PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
Angel, did you check some code drop or the current binary? If it really […]
In any case, these changes will help us get rid of this parameter in the romstage (CB:39375, CB:56517 and for each new port of the motherboard). Also, some products based on the Kontron module don't use IGD and connecting an external graphics card is very useful for testing.
Attention is currently required from: Nico Huber, Maxim Polyakov, Mario Scheithauer, Angel Pons. Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 31: Code-Review+1
(1 comment)
File src/soc/intel/apollolake/romstage.c:
https://review.coreboot.org/c/coreboot/+/39374/comment/4e36d7c6_c703d63a PS31, Line 234: /* Select primary GPU device */ Move this comment below the enum?
Attention is currently required from: Nico Huber, Maxim Polyakov, Mario Scheithauer, Angel Pons. Hello Felix Singer, build bot (Jenkins), Nico Huber, Paul Menzel, Mario Scheithauer, Angel Pons, Werner Zeh, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39374
to look at the new patch set (#32).
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
soc/{apl,glk}: Allow to select the primary graphics device
Allow to select the primary graphics device between the IGD and the external PCIe GPU depending on the ONBOARD_VGA_IS_PRIMARY config. The option sets the priority only. This means that if a high priority is set for an external PCI device and it is not connected/not enabled, then the device with a lower priority will be used, in our case it is IGD.
TEST = Set PRIMARY_PCI and boot Linux on the Kontron mAL10 [1] with the miniPCIe video adapter on the Silicon Motion SM750 controller. As a result, the display connected to an external GPU device shows the Tianocore logo + setup menu and the desktop.
[1] https://review.coreboot.org/c/coreboot/+/39133
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/apollolake/romstage.c 1 file changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/39374/32
Attention is currently required from: Nico Huber, Mario Scheithauer, Angel Pons, Werner Zeh. Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 32:
(1 comment)
File src/soc/intel/apollolake/romstage.c:
https://review.coreboot.org/c/coreboot/+/39374/comment/6c242882_e2b460a1 PS31, Line 234: /* Select primary GPU device */
Move this comment below the enum?
Done
Attention is currently required from: Nico Huber, Maxim Polyakov, Mario Scheithauer, Werner Zeh. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 32:
(1 comment)
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/d16c9655_52184c5d PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
Angel, did you check some code drop or the current binary? If it really makes a difference if we set 2 or 3, this change seems valid. However, I have to say: as it is common when we add features that nobody needs yet, it can waste a lot of time. We don't know what will happen first: a) FSP vanishes, b) somebody will make use of this feature.
I didn't check the current binary.
Attention is currently required from: Maxim Polyakov, Mario Scheithauer, Werner Zeh. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 32: Code-Review+2
Attention is currently required from: Maxim Polyakov, Mario Scheithauer, Angel Pons, Werner Zeh. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 32:
(1 comment)
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/81b8310c_4597981d PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
Angel, did you check some code drop or the current binary? If it really […]
Maybe as a conclusive test: Set ONBOARD_VGA_IS_PRIMARY and boot with an external card plugged.
Attention is currently required from: Mario Scheithauer, Angel Pons, Werner Zeh. Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 32:
(2 comments)
Patchset:
PS32: Thanks!
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/53b8cf7f_a580e5f2 PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
Angel, did you check some code drop or the current binary? If it really […]
Ack
Attention is currently required from: Nico Huber, Mario Scheithauer, Angel Pons, Werner Zeh. Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 32:
(1 comment)
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/0358d379_d9565444 PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
Ack
I don't have an external adapter at the moment, but I will try to contact the company's engineers so that they check it again. This will take more time.
Attention is currently required from: Nico Huber, Mario Scheithauer, Angel Pons, Werner Zeh. Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 32:
(1 comment)
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/9e41e918_74f4bfd2 PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
I don't have an external adapter at the moment, but I will try to contact the company's engineers so […]
If ONBOARD_VGA_IS_PRIMARY is set, coreboot/tianocore shows the splashscreen/setup menu only on the display connected to IGD and there is nothing on the display with an external GPU.
Attention is currently required from: Nico Huber, Maxim Polyakov, Angel Pons, Werner Zeh. Mario Scheithauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 32: Code-Review+1
(1 comment)
Patchset:
PS32: Hi, I tested this patch on mc_apl5 and found no issues. So far we have used the default value (0:AUTO) for PrimaryVideoAdaptor. But I can't test an external adapter either, unfortunately.
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
soc/{apl,glk}: Allow to select the primary graphics device
Allow to select the primary graphics device between the IGD and the external PCIe GPU depending on the ONBOARD_VGA_IS_PRIMARY config. The option sets the priority only. This means that if a high priority is set for an external PCI device and it is not connected/not enabled, then the device with a lower priority will be used, in our case it is IGD.
TEST = Set PRIMARY_PCI and boot Linux on the Kontron mAL10 [1] with the miniPCIe video adapter on the Silicon Motion SM750 controller. As a result, the display connected to an external GPU device shows the Tianocore logo + setup menu and the desktop.
[1] https://review.coreboot.org/c/coreboot/+/39133
Change-Id: Idcd117217cf412ee0722aff52db4b3c8ec2a226c Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39374 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Mario Scheithauer mario.scheithauer@siemens.com --- M src/soc/intel/apollolake/romstage.c 1 file changed, 14 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Mario Scheithauer: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/apollolake/romstage.c b/src/soc/intel/apollolake/romstage.c index 13a55dc..1c92b89 100644 --- a/src/soc/intel/apollolake/romstage.c +++ b/src/soc/intel/apollolake/romstage.c @@ -229,6 +229,19 @@ } }
+static void soc_gpu_init_params(FSPM_UPD *mupd) +{ + enum { + GPU_PRIMARY_IGD = 2, + GPU_PRIMARY_PCI = 3, + }; + /* Select primary GPU device */ + if (CONFIG(ONBOARD_VGA_IS_PRIMARY) && is_devfn_enabled(SA_DEVFN_IGD)) + mupd->FspmConfig.PrimaryVideoAdaptor = GPU_PRIMARY_IGD; + else + mupd->FspmConfig.PrimaryVideoAdaptor = GPU_PRIMARY_PCI; +} + static void soc_memory_init_params(FSPM_UPD *mupd) { #if CONFIG(SOC_INTEL_GEMINILAKE) @@ -268,6 +281,7 @@ check_full_retrain(mupd);
fill_console_params(mupd); + soc_gpu_init_params(mupd);
if (CONFIG(SOC_INTEL_GEMINILAKE)) soc_memory_init_params(mupd);
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 33:
Automatic boot test returned (PASS/FAIL/TOTAL): 12 / 1 / 13
PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/76741 PASS: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/76740 PASS: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/76739 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload TianoCore : https://lava.9esec.io/r/76738 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload TianoCore : https://lava.9esec.io/r/76737 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload TianoCore : https://lava.9esec.io/r/76736 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload SeaBIOS : https://lava.9esec.io/r/76735 FAIL: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload TianoCore : https://lava.9esec.io/r/76734 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload SeaBIOS : https://lava.9esec.io/r/76733 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/76732 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/76731 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/76730 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/76729
Please note: This test is under development and might not be accurate at all!
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 33:
(1 comment)
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/f5c9434d_7e9a6de8 PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
If ONBOARD_VGA_IS_PRIMARY is set, coreboot/tianocore shows the splashscreen/setup menu only on the d […]
I have stumbled across this again... First of all Angels is right, the comment in the FspmUpd.h does not match the implementation and hence is simply wrong. It turned out that we do have issues on an older GFX driver in Linux with this wrong setting. So since this is Apollo Lake specific I would vote for a change to align with the FSP implementation and not the comment in the header file. I can reach out to Intel to see if this can be resolved on the FSP side so that the comment is corrected.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 33:
(1 comment)
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/8e006c7a_41dcdd9d PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
I have stumbled across this again... […]
Just to be sure, you are using the latest public FSP binary?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 33:
(1 comment)
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/69344eee_d20ae41a PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
Just to be sure, you are using the latest public FSP binary?
Also, do you have ONBOARD_VGA_IS_PRIMARY set in your .config?
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39374 )
Change subject: soc/{apl,glk}: Allow to select the primary graphics device ......................................................................
Patch Set 33:
(1 comment)
File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/c/coreboot/+/39374/comment/ee274f14_dfa0f7c0 PS17, Line 45: PRIMARY_AUTO = 0, : PRIMARY_IGD = 2, : PRIMARY_PCI = 3,
Just to be sure, you are using the latest public FSP binary?
Yes, the latest one available on GitHub
Also, do you have ONBOARD_VGA_IS_PRIMARY set in your .config?
Yes, I can confirm that as well.
I have uploaded a patch (CB:62889) and started to talk with Intel to fix this in the header file.
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Mario Scheithauer (Code Review)
-
Maxim Polyakov (Code Review)
-
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Werner Zeh (Code Review)