Maulik V Vaghela has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
soc/intel/jasperlake: Disable IPU based on devicetree
FSP enables IPU 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: I0f9a40e85427fd88bb12a40770ecf7b939b1d8cd Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/romstage/fsp_params.c 1 file changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/44270/1
diff --git a/src/soc/intel/jasperlake/romstage/fsp_params.c b/src/soc/intel/jasperlake/romstage/fsp_params.c index 809ae80..9207a82 100644 --- a/src/soc/intel/jasperlake/romstage/fsp_params.c +++ b/src/soc/intel/jasperlake/romstage/fsp_params.c @@ -66,10 +66,16 @@ m_cfg->CpuTraceHubMode = config->TraceHubMode; }
+ /* IPU configuration */ + /* By default IPU is enabled, need to disable in FSP if pci device is disabled */ + dev = pcidev_path_on_root(SA_DEVFN_IPU); + if (!is_dev_enabled(dev)) { + m_cfg->SaIpuEnable = 0; + } + /* Change VmxEnable UPD value according to ENABLE_VMX Kconfig */ m_cfg->VmxEnable = CONFIG(ENABLE_VMX);
- /* Enable SMBus controller based on config */ m_cfg->SmbusEnable = config->SmbusEnable;
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44270/1/src/soc/intel/jasperlake/ro... File src/soc/intel/jasperlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44270/1/src/soc/intel/jasperlake/ro... PS1, Line 72: if (!is_dev_enabled(dev)) { braces {} are not necessary for single statement blocks
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44270
to look at the new patch set (#2).
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
soc/intel/jasperlake: Disable IPU based on devicetree
FSP enables IPU 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: I0f9a40e85427fd88bb12a40770ecf7b939b1d8cd Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/romstage/fsp_params.c 1 file changed, 6 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/44270/2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
Patch Set 2: Code-Review+2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44270/2/src/soc/intel/jasperlake/ro... File src/soc/intel/jasperlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44270/2/src/soc/intel/jasperlake/ro... PS2, Line 73: m_cfg->SaIpuEnable = 0; or m_cfg->SaIpuEnable = is_dev_enabled(dev);
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
Patch Set 2: -Code-Review
(1 comment)
https://review.coreboot.org/c/coreboot/+/44270/2/src/soc/intel/jasperlake/ro... File src/soc/intel/jasperlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44270/2/src/soc/intel/jasperlake/ro... PS2, Line 73: m_cfg->SaIpuEnable = 0;
or m_cfg->SaIpuEnable = is_dev_enabled(dev);
That is much better.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44270/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44270/2//COMMIT_MSG@11 PS2, Line 11: device enablement in devicetree. Please re-flow for 75 characters per line.
Hello build bot (Jenkins), Justin TerAvest, Krishna P Bhat D, Aamir Bohra, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44270
to look at the new patch set (#3).
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
soc/intel/jasperlake: Disable IPU based on devicetree
FSP enables IPU 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: I0f9a40e85427fd88bb12a40770ecf7b939b1d8cd Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/romstage/fsp_params.c 1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/44270/3
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44270/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44270/2//COMMIT_MSG@11 PS2, Line 11: device enablement in devicetree.
Please re-flow for 75 characters per line.
Done
https://review.coreboot.org/c/coreboot/+/44270/2/src/soc/intel/jasperlake/ro... File src/soc/intel/jasperlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44270/2/src/soc/intel/jasperlake/ro... PS2, Line 73: m_cfg->SaIpuEnable = 0;
That is much better.
Done
Hello build bot (Jenkins), Justin TerAvest, Krishna P Bhat D, Aamir Bohra, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44270
to look at the new patch set (#4).
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
soc/intel/jasperlake: Disable IPU based on devicetree
FSP enables IPU 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: I0f9a40e85427fd88bb12a40770ecf7b939b1d8cd Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/romstage/fsp_params.c 1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/44270/4
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
Patch Set 4: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/44270/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44270/4//COMMIT_MSG@9 PS4, Line 9: IPU What is IPU? Spell it out once?
https://review.coreboot.org/c/coreboot/+/44270/4/src/soc/intel/jasperlake/ro... File src/soc/intel/jasperlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44270/4/src/soc/intel/jasperlake/ro... PS4, Line 72: In the future, please do not mix in style changes.
Hello build bot (Jenkins), Justin TerAvest, Paul Menzel, Krishna P Bhat D, Aamir Bohra, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44270
to look at the new patch set (#5).
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
soc/intel/jasperlake: Disable 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: I0f9a40e85427fd88bb12a40770ecf7b939b1d8cd Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/romstage/fsp_params.c 1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/44270/5
Hello build bot (Jenkins), Justin TerAvest, Paul Menzel, Krishna P Bhat D, Aamir Bohra, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44270
to look at the new patch set (#6).
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
soc/intel/jasperlake: Disable 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: I0f9a40e85427fd88bb12a40770ecf7b939b1d8cd Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/romstage/fsp_params.c 1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/44270/6
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44270/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44270/4//COMMIT_MSG@9 PS4, Line 9: IPU
What is IPU? Spell it out once?
Done
https://review.coreboot.org/c/coreboot/+/44270/4/src/soc/intel/jasperlake/ro... File src/soc/intel/jasperlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44270/4/src/soc/intel/jasperlake/ro... PS4, Line 72:
In the future, please do not mix in style changes.
Sure..will take care of it in future
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
Patch Set 6: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/44270/6/src/soc/intel/jasperlake/ro... File src/soc/intel/jasperlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44270/6/src/soc/intel/jasperlake/ro... PS6, Line 70: /* By default IPU is enabled, need to disable in FSP if pci device is disabled */ nit: not needed?
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Disable IPU based on devicetree ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44270/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44270/6//COMMIT_MSG@7 PS6, Line 7: Disable Configure?
Hello build bot (Jenkins), Justin TerAvest, Paul Menzel, Krishna P Bhat D, Aamir Bohra, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44270
to look at the new patch set (#7).
Change subject: soc/intel/jasperlake: Configure IPU based on devicetree ......................................................................
soc/intel/jasperlake: 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: I0f9a40e85427fd88bb12a40770ecf7b939b1d8cd Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/romstage/fsp_params.c 1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/44270/7
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Configure IPU based on devicetree ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44270/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44270/6//COMMIT_MSG@7 PS6, Line 7: Disable
Configure?
Done
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Configure IPU based on devicetree ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44270/6/src/soc/intel/jasperlake/ro... File src/soc/intel/jasperlake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44270/6/src/soc/intel/jasperlake/ro... PS6, Line 70: /* By default IPU is enabled, need to disable in FSP if pci device is disabled */
nit: not needed?
Done
Hello build bot (Jenkins), Justin TerAvest, Paul Menzel, Krishna P Bhat D, Aamir Bohra, Ronak Kanabar, Divagar Mohandass, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44270
to look at the new patch set (#8).
Change subject: soc/intel/jasperlake: Configure IPU based on devicetree ......................................................................
soc/intel/jasperlake: 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: I0f9a40e85427fd88bb12a40770ecf7b939b1d8cd Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com --- M src/soc/intel/jasperlake/romstage/fsp_params.c 1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/44270/8
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Configure IPU based on devicetree ......................................................................
Patch Set 8: Code-Review+2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Configure IPU based on devicetree ......................................................................
Patch Set 8: Code-Review+2
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44270 )
Change subject: soc/intel/jasperlake: Configure IPU based on devicetree ......................................................................
soc/intel/jasperlake: 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: I0f9a40e85427fd88bb12a40770ecf7b939b1d8cd Signed-off-by: Maulik V Vaghela maulik.v.vaghela@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44270 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aamir Bohra aamir.bohra@intel.com Reviewed-by: Karthik Ramasubramanian kramasub@google.com --- M src/soc/intel/jasperlake/romstage/fsp_params.c 1 file changed, 4 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Aamir Bohra: Looks good to me, approved Karthik Ramasubramanian: Looks good to me, approved
diff --git a/src/soc/intel/jasperlake/romstage/fsp_params.c b/src/soc/intel/jasperlake/romstage/fsp_params.c index 809ae80..dccdebf 100644 --- a/src/soc/intel/jasperlake/romstage/fsp_params.c +++ b/src/soc/intel/jasperlake/romstage/fsp_params.c @@ -66,10 +66,13 @@ m_cfg->CpuTraceHubMode = config->TraceHubMode; }
+ /* IPU configuration */ + dev = pcidev_path_on_root(SA_DEVFN_IPU); + m_cfg->SaIpuEnable = is_dev_enabled(dev); + /* Change VmxEnable UPD value according to ENABLE_VMX Kconfig */ m_cfg->VmxEnable = CONFIG(ENABLE_VMX);
- /* Enable SMBus controller based on config */ m_cfg->SmbusEnable = config->SmbusEnable;
-
Aamir Bohra (Code Review) -
build bot (Jenkins) (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Maulik V Vaghela (Code Review)
-
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)