Ravishankar Sarawadi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Configure IPU based on devicetree ......................................................................
soc/intel/tigerlake: Configure IPU based on devicetree
FSP enables IPU (Imaging Processing Unit) by default even if its disabled in devicetree. We need to fill FSP upd based on the device enablement in devicetree.
BUG=None BRANCH=None TEST=IPU is disabled and doesn't show in lspci.
Change-Id: Ieff57fb0ebc8522546d6b34da6ca2f2f845bf61d --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/44627/1
diff --git a/src/soc/intel/tigerlake/romstage/fsp_params.c b/src/soc/intel/tigerlake/romstage/fsp_params.c index acb366b..0387a1d 100644 --- a/src/soc/intel/tigerlake/romstage/fsp_params.c +++ b/src/soc/intel/tigerlake/romstage/fsp_params.c @@ -188,6 +188,10 @@ /* Change VmxEnable UPD value according to ENABLE_VMX Kconfig */ m_cfg->VmxEnable = CONFIG(ENABLE_VMX);
+ /* IPU configuration */ + dev = pcidev_path_on_root(SA_DEVFN_IPU); + m_cfg->SaIpuEnable = is_dev_enabled(dev); + /* Command Pins Mirrored */ m_cfg->CmdMirror[0] = config->CmdMirror;
Ravishankar Sarawadi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Configure IPU based on devicetree ......................................................................
Patch Set 1:
Nick with this CL, I think we should update variants with MIPI camera support to enable IPU in override DT or should we keep default baseboard IPU DT setting ON and use override to turn it off? What do you suggest?
Hello Nick Vaccaro, Raj Astekar, Patrick Rudolph, Daniel H Kang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44627
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Configure IPU based on devicetree ......................................................................
soc/intel/tigerlake: Configure IPU based on devicetree
FSP enables IPU (Imaging Processing Unit) by default even if its disabled in devicetree. We need to fill FSP upd based on the device enablement in devicetree.
BUG=None BRANCH=None TEST=IPU is disabled and doesn't show in lspci.
Change-Id: Ieff57fb0ebc8522546d6b34da6ca2f2f845bf61d Signed-off-by: Ravi Sarawadi ravishankar.sarawadi@intel.com --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/44627/2
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Configure IPU based on devicetree ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44627/2/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44627/2/src/soc/intel/tigerlake/rom... PS2, Line 171: m_cfg->VtdIpuEnable = 0x1; : m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS; I think we also need to check IPU status for this.
Ravishankar Sarawadi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Configure IPU based on devicetree ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44627/2/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44627/2/src/soc/intel/tigerlake/rom... PS2, Line 171: m_cfg->VtdIpuEnable = 0x1; : m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS;
I think we also need to check IPU status for this.
John can you please confirm if check is needed?
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Configure IPU based on devicetree ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44627/2/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44627/2/src/soc/intel/tigerlake/rom... PS2, Line 171: m_cfg->VtdIpuEnable = 0x1; : m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS;
John can you please confirm if check is needed?
Yes, please consider to move IPU configuration before the VT-d setting. Then add check for both of GPU and IPU like: if (m_cfg->InternalGfx) { m_cfg->VtdIgdEnable = 0x1; m_cfg->VtdBaseAddress[0] = GFXVT_BASE_ADDRESS; }
if (m_cfg->SaIpuEnable) { m_cfg->VtdIpuEnable = 0x1; m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS; }
Hello build bot (Jenkins), John Zhao, Nick Vaccaro, Raj Astekar, Patrick Rudolph, Daniel H Kang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44627
to look at the new patch set (#3).
Change subject: soc/intel/tigerlake: Fix IPU and Vtd config ......................................................................
soc/intel/tigerlake: Fix IPU and Vtd config
- FSP enables IPU (Imaging Processing Unit) by default even if its disabled in devicetree. We need to fill FSP upd based on the device enablement in devicetree.
- Enable Vtd IPU and IGD settings only if respective IPs are enabled.
BUG=None BRANCH=None TEST=IPU is disabled and doesn't show in lspci.
Change-Id: Ieff57fb0ebc8522546d6b34da6ca2f2f845bf61d Signed-off-by: Ravi Sarawadi ravishankar.sarawadi@intel.com --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 14 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/44627/3
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Fix IPU and Vtd config ......................................................................
Patch Set 3: Code-Review+2
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Fix IPU and Vtd config ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44627/3/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44627/3/src/soc/intel/tigerlake/rom... PS3, Line 176: if (m_cfg->InternalGfx) { : m_cfg->VtdIgdEnable = 0x1; : m_cfg->VtdBaseAddress[0] = GFXVT_BASE_ADDRESS; : } : : if (m_cfg->SaIpuEnable) { : m_cfg->VtdIpuEnable = 0x1; : m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS; : } This is minor but can we move this code up after line 172? Then we have sequential code for VtdBaseAddress array.
Hello build bot (Jenkins), John Zhao, Nick Vaccaro, Raj Astekar, Patrick Rudolph, Daniel H Kang,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44627
to look at the new patch set (#4).
Change subject: soc/intel/tigerlake: Fix IPU and Vtd config ......................................................................
soc/intel/tigerlake: Fix IPU and Vtd config
- FSP enables IPU (Imaging Processing Unit) by default even if its disabled in devicetree. We need to fill FSP upd based on the device enablement in devicetree.
- Enable Vtd IPU and IGD settings only if respective IPs are enabled.
BUG=None BRANCH=None TEST=IPU is disabled and doesn't show in lspci.
Change-Id: Ieff57fb0ebc8522546d6b34da6ca2f2f845bf61d Signed-off-by: Ravi Sarawadi ravishankar.sarawadi@intel.com --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 15 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/44627/4
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Fix IPU and Vtd config ......................................................................
Patch Set 4: Code-Review+2
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Fix IPU and Vtd config ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/44627/3/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44627/3/src/soc/intel/tigerlake/rom... PS3, Line 176: if (m_cfg->InternalGfx) { : m_cfg->VtdIgdEnable = 0x1; : m_cfg->VtdBaseAddress[0] = GFXVT_BASE_ADDRESS; : } : : if (m_cfg->SaIpuEnable) { : m_cfg->VtdIpuEnable = 0x1; : m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS; : }
This is minor but can we move this code up after line 172? […]
Done
Daniel H Kang has uploaded a new patch set (#5) to the change originally created by Ravishankar Sarawadi. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Fix IPU and Vtd config ......................................................................
soc/intel/tigerlake: Fix IPU and Vtd config
- FSP enables IPU (Imaging Processing Unit) by default even if its disabled in devicetree. We need to fill FSP upd based on the device enablement in devicetree.
- Enable Vtd IPU and IGD settings only if respective IPs are enabled.
BUG=165340186 BRANCH=None TEST=IPU is disabled and doesn't show in lspci.
Change-Id: Ieff57fb0ebc8522546d6b34da6ca2f2f845bf61d Signed-off-by: Ravi Sarawadi ravishankar.sarawadi@intel.com --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 15 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/44627/5
Daniel H Kang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Fix IPU and Vtd config ......................................................................
Patch Set 5: Code-Review+1
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Fix IPU and Vtd config ......................................................................
soc/intel/tigerlake: Fix IPU and Vtd config
- FSP enables IPU (Imaging Processing Unit) by default even if its disabled in devicetree. We need to fill FSP upd based on the device enablement in devicetree.
- Enable Vtd IPU and IGD settings only if respective IPs are enabled.
BUG=165340186 BRANCH=None TEST=IPU is disabled and doesn't show in lspci.
Change-Id: Ieff57fb0ebc8522546d6b34da6ca2f2f845bf61d Signed-off-by: Ravi Sarawadi ravishankar.sarawadi@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44627 Reviewed-by: Daniel H Kang daniel.h.kang@intel.corp-partner.google.com Reviewed-by: John Zhao john.zhao@intel.com Reviewed-by: Wonkyu Kim wonkyu.kim@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/tigerlake/romstage/fsp_params.c 1 file changed, 15 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified John Zhao: Looks good to me, approved Wonkyu Kim: Looks good to me, approved Daniel H Kang: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/tigerlake/romstage/fsp_params.c b/src/soc/intel/tigerlake/romstage/fsp_params.c index acb366b..2ba276d 100644 --- a/src/soc/intel/tigerlake/romstage/fsp_params.c +++ b/src/soc/intel/tigerlake/romstage/fsp_params.c @@ -158,6 +158,10 @@ memcpy(m_cfg->PchHdaAudioLinkSndwEnable, config->PchHdaAudioLinkSndwEnable, sizeof(m_cfg->PchHdaAudioLinkSndwEnable));
+ /* IPU configuration */ + dev = pcidev_path_on_root(SA_DEVFN_IPU); + m_cfg->SaIpuEnable = is_dev_enabled(dev); + /* Vt-D config */ cpu_id = cpu_get_cpuid(); if (cpu_id == CPUID_TIGERLAKE_A0) { @@ -166,11 +170,18 @@ } else { /* Enable VT-d support for QS platform */ m_cfg->VtdDisable = 0; - m_cfg->VtdIgdEnable = 0x1; - m_cfg->VtdBaseAddress[0] = GFXVT_BASE_ADDRESS; - m_cfg->VtdIpuEnable = 0x1; - m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS; m_cfg->VtdIopEnable = 0x1; + + if (m_cfg->InternalGfx) { + m_cfg->VtdIgdEnable = 0x1; + m_cfg->VtdBaseAddress[0] = GFXVT_BASE_ADDRESS; + } + + if (m_cfg->SaIpuEnable) { + m_cfg->VtdIpuEnable = 0x1; + m_cfg->VtdBaseAddress[1] = IPUVT_BASE_ADDRESS; + } + m_cfg->VtdBaseAddress[2] = VTVC0_BASE_ADDRESS;
if (m_cfg->TcssDma0En || m_cfg->TcssDma1En)
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44627 )
Change subject: soc/intel/tigerlake: Fix IPU and Vtd config ......................................................................
Patch Set 6:
Automatic boot test returned (PASS/FAIL/TOTAL): 5/2/7 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/16406 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16405 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16404 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16403 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/16402 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : FAIL : https://lava.9esec.io/r/16408 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/16407
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Daniel H Kang (Code Review)
-
John Zhao (Code Review)
-
Patrick Georgi (Code Review)
-
Ravishankar Sarawadi (Code Review)
-
Wonkyu Kim (Code Review)