Change in coreboot[master]: soc/intel/tigerlake: Allow fine grained control of S0iX states
Hello Jes Klinke,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44355
to review the following change.
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
soc/intel/tigerlake: Allow fine grained control of S0iX states
Expose FSP parameter to enable/disable each individual substate.
See https://review.coreboot.org/c/coreboot/+/43741 for context.
Change-Id: I8a0cf820e20961486813067c6945fe07bc4899f7 Signed-off-by: jbk@chromium.org --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 10 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/44355/1
diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h index dc910ff..6c5daa5 100644 --- a/src/soc/intel/tigerlake/chip.h +++ b/src/soc/intel/tigerlake/chip.h @@ -78,6 +78,9 @@
/* Enable S0iX support */ int s0ix_enable; + /* S0iX: Selectively enable individual sub-states */ + uint8_t LpmStateEnableMask; + /* Support for TCSS xhci, xdci, TBT PCIe root ports and DMA controllers */ uint8_t TcssD3HotDisable; /* Support for TBT PCIe root ports and DMA controllers with D3Hot->D3Cold */ diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c index a61a025..53ca11e 100644 --- a/src/soc/intel/tigerlake/fsp_params.c +++ b/src/soc/intel/tigerlake/fsp_params.c @@ -204,6 +204,13 @@ sizeof(params->SataPortsDevSlp)); }
+ /* S0iX: Selectively enable individual sub-states. + * + * LPM0-s0i2.0, LPM1-s0i2.1, LPM2-s0i2.2, LPM3-s0i3.0, + * LPM4-s0i3.1, LPM5-s0i3.2, LPM6-s0i3.3, LPM7-s0i3.4 + */ + params->LpmStateEnableMask = config->LpmStateEnableMask; + /* * Power Optimizer for DMI and SATA. * DmiPwrOptimizeDisable and SataPwrOptimizeDisable is default to 0.
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44355/1/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/1/src/soc/intel/tigerlake/chi... PS1, Line 82: I would like this to default to 0xFF for any board that does not mention this setting in their devicetree.cb. Is that possible? If not, and if this struct is zero-filled by default, maybe I should instead call this LpmStateDisableMask, to make it into its bitwise negation, then a default of zero will result in all substates being enabled by default.
Hello build bot (Jenkins), Furquan Shaikh, Jes Klinke, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44355
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
soc/intel/tigerlake: Allow fine grained control of S0iX states
Expose FSP parameter to enable/disable each individual substate.
See https://review.coreboot.org/c/coreboot/+/43741 for context.
Change-Id: I8a0cf820e20961486813067c6945fe07bc4899f7 Signed-off-by: jbk@chromium.org --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 22 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/44355/2
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44355/1/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/1/src/soc/intel/tigerlake/chi... PS1, Line 82:
I would like this to default to 0xFF for any board that does not mention this setting in their devic […]
If you are open to introducing a new element "LpmStateEnable", then its default value of '0' (when not configured in devicetree) can be used to assign a default value of 0xff to the LpmStateEnableMask. Otherwise, assign the value configured in the devicetree.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 2: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/44355/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44355/2//COMMIT_MSG@12 PS2, Line 12: BUG=?
https://review.coreboot.org/c/coreboot/+/44355/2/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/2/src/soc/intel/tigerlake/chi... PS2, Line 94: uint8_t enum lpm_state_mask
https://review.coreboot.org/c/coreboot/+/44355/2/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44355/2/src/soc/intel/tigerlake/fsp... PS2, Line 212: params->LpmStateEnableMask = config->LpmStateEnableMask; Just a note: This change should land with the mainboard change as a single unit. Else, S0ix will be broken for volteer.
Hello build bot (Jenkins), Furquan Shaikh, Jes Klinke, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44355
to look at the new patch set (#3).
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
soc/intel/tigerlake: Allow fine grained control of S0iX states
Expose FSP parameter to enable/disable each individual substate.
See https://review.coreboot.org/c/coreboot/+/43741 for context.
BUG=b:154333137
Change-Id: I8a0cf820e20961486813067c6945fe07bc4899f7 Signed-off-by: jbk@chromium.org --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/44355/3
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44355/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44355/2//COMMIT_MSG@12 PS2, Line 12:
BUG=?
Done
https://review.coreboot.org/c/coreboot/+/44355/2/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/2/src/soc/intel/tigerlake/chi... PS2, Line 94: uint8_t
enum lpm_state_mask
Done
https://review.coreboot.org/c/coreboot/+/44355/1/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/1/src/soc/intel/tigerlake/chi... PS1, Line 82:
If you are open to introducing a new element "LpmStateEnable", then its default value of '0' (when n […]
I see. I have considered making a special case for the value 0, (as it would not make sense to disable every single substate, while keeping s0ix_enable). However, there is a theoretical problem, as I am also introducing code that will programmatically override bit 7 of the LpsStateEnableMask before passing it to the FSP. In the extremelty unlikely case that a board wants to disable every substate except S03.4, and hence sets LpmStateEnableMask to "0x80" in devicetree, and then the cr50 firmware is too old, causing my logic to clear the bit7, in order to disable S03.4 until the cr50 firmware has been upgraded, then it instead has the unintended effect of enabling every other substate.
I will reverse the meaning of this field, calling it LpmStateDisableMask. That way, the default of 0 will naturally result in new boards having every substate enabled, unless they request otherwise, without any special handling or surprises.
https://review.coreboot.org/c/coreboot/+/44355/2/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44355/2/src/soc/intel/tigerlake/fsp... PS2, Line 212: params->LpmStateEnableMask = config->LpmStateEnableMask;
Just a note: This change should land with the mainboard change as a single unit. […]
I see. I have decided to negate the meaning of this field, such that by default, every substate will be enabled (as was the case before this CL.)
Hello build bot (Jenkins), Furquan Shaikh, Jes Klinke, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44355
to look at the new patch set (#4).
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
soc/intel/tigerlake: Allow fine grained control of S0iX states
Expose FSP parameter to enable/disable each individual substate.
See https://review.coreboot.org/c/coreboot/+/43741 for context.
BUG=b:154333137
Change-Id: I8a0cf820e20961486813067c6945fe07bc4899f7 Signed-off-by: jbk@chromium.org --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/44355/4
Hello build bot (Jenkins), Furquan Shaikh, Jes Klinke, Julius Werner, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44355
to look at the new patch set (#5).
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
soc/intel/tigerlake: Allow fine grained control of S0iX states
Expose FSP parameter to enable/disable each individual substate.
See https://review.coreboot.org/c/coreboot/+/43741 for context.
BUG=b:154333137
Change-Id: I8a0cf820e20961486813067c6945fe07bc4899f7 Signed-off-by: jbk@chromium.org --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/44355/5
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 3:
Only now do I realize that the LpmStateEnableMask I am working with in mainboard_silicon_init_params() is actually the params struct, which already has LpmStateEnableMask, and not the config struct which I am adding a field to in this CL. https://review.coreboot.org/c/coreboot/+/44359/4/src/mainboard/google/voltee...
This means, as far as I can tell, that this CL is unnecessary.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 5: Code-Review+2
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 5: Code-Review-1
According to my prior comment, I do not believe this CL is necessary for my feature. I am delaying fully abandoning it, though, until I am sure.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 5:
Patch Set 5: Code-Review-1
According to my prior comment, I do not believe this CL is necessary for my feature. I am delaying fully abandoning it, though, until I am sure.
In my opinion, it is better to keep all the FSP_S UPD updates within the SoC code and only touch soc_config from mainboard. Thus, let's add LpmStateDisableMask as you have in this CL. (In the follow-up CL, you will have to perform the config setting in a different place than it is currently done - I will post a comment there).
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... PS5, Line 68: LPM_S0i3_4 = BIT(7), It might be helpful to have an enum which says all S0ix substates LPM_S0ix_all = LPM_S0i2_0 | LPM_S0i2_1 | LPM_S0i2_2 | LPM_S0i3_0 | LPM_S0i3_1 | LPM_S0i3_2 | LPM_S0i3_3 | LPM_S0i3_4;
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... PS5, Line 93: enable disable
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... PS5, Line 93: It will be good to mention here that by default all S0ix substates are enabled unless any substate is explicitly disabled by mainboard using this parameter.
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/fsp... PS5, Line 213: ~config->LpmStateDisableMask LPM_S0ix_all & ~config->LpmStateDisableMask;
Ensures that this will set the enable bits correctly even if the default UPD value changes in the future.
Hello build bot (Jenkins), Furquan Shaikh, Jes Klinke, Julius Werner, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44355
to look at the new patch set (#6).
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
soc/intel/tigerlake: Allow fine grained control of S0iX states
Expose FSP parameter to enable/disable each individual substate.
See https://review.coreboot.org/c/coreboot/+/43741 for context.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: I8a0cf820e20961486813067c6945fe07bc4899f7 Signed-off-by: jbk@chromium.org --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 25 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/44355/6
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 6:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... PS5, Line 68: LPM_S0i3_4 = BIT(7),
It might be helpful to have an enum which says all S0ix substates […]
Done
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... PS5, Line 93:
It will be good to mention here that by default all S0ix substates are enabled unless any substate i […]
Done
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/chi... PS5, Line 93: enable
disable
Done
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/fsp... File src/soc/intel/tigerlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/44355/5/src/soc/intel/tigerlake/fsp... PS5, Line 213: ~config->LpmStateDisableMask
LPM_S0ix_all & ~config->LpmStateDisableMask; […]
Done
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... PS6, Line 69: LPM_S0iX_ALL = LPM_S0i2_0 | LPM_S0i2_1 | LPM_S0i2_2 code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... PS6, Line 69: LPM_S0iX_ALL = LPM_S0i2_0 | LPM_S0i2_1 | LPM_S0i2_2 please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... PS6, Line 70: | LPM_S0i3_0 | LPM_S0i3_1 | LPM_S0i3_2 | LPM_S0i3_3 | LPM_S0i3_4, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... PS6, Line 70: | LPM_S0i3_0 | LPM_S0i3_1 | LPM_S0i3_2 | LPM_S0i3_3 | LPM_S0i3_4, please, no space before tabs
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... PS6, Line 70: | LPM_S0i3_0 | LPM_S0i3_1 | LPM_S0i3_2 | LPM_S0i3_3 | LPM_S0i3_4, please, no spaces at the start of a line
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 6: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... PS6, Line 69: tab instead of space
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... PS6, Line 70: same here
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44355/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44355/6//COMMIT_MSG@9 PS6, Line 9: Expose FSP parameter to enable/disable each individual substate. This change doesn't really expose FSP parameter, but provides a knob for mainboard to selectively disable S0ix substates.
Hello build bot (Jenkins), Furquan Shaikh, Jes Klinke, Julius Werner, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44355
to look at the new patch set (#7).
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
soc/intel/tigerlake: Allow fine grained control of S0iX states
Expose devicetree parameter to enable/disable each individual substate.
See https://review.coreboot.org/c/coreboot/+/43741 for context.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: I8a0cf820e20961486813067c6945fe07bc4899f7 Signed-off-by: jbk@chromium.org --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 25 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/44355/7
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/44355/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44355/6//COMMIT_MSG@9 PS6, Line 9: Expose FSP parameter to enable/disable each individual substate.
This change doesn't really expose FSP parameter, but provides a knob for mainboard to selectively di […]
Ack
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... File src/soc/intel/tigerlake/chip.h:
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... PS6, Line 69:
tab instead of space
Done
https://review.coreboot.org/c/coreboot/+/44355/6/src/soc/intel/tigerlake/chi... PS6, Line 70:
same here
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 7: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
soc/intel/tigerlake: Allow fine grained control of S0iX states
Expose devicetree parameter to enable/disable each individual substate.
See https://review.coreboot.org/c/coreboot/+/43741 for context.
TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x BUG=b:154333137
Change-Id: I8a0cf820e20961486813067c6945fe07bc4899f7 Signed-off-by: jbk@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/44355 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/fsp_params.c 2 files changed, 25 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/tigerlake/chip.h b/src/soc/intel/tigerlake/chip.h index dc910ff..2da63ed 100644 --- a/src/soc/intel/tigerlake/chip.h +++ b/src/soc/intel/tigerlake/chip.h @@ -56,6 +56,20 @@ #define FIVR_ENABLE_ALL_SX (FIVR_ENABLE_S0i1_S0i2 | FIVR_ENABLE_S0i3 | \ FIVR_ENABLE_S3 | FIVR_ENABLE_S4 | FIVR_ENABLE_S5)
+/* Bit values for use in LpmStateEnableMask. */ +enum lpm_state_mask { + LPM_S0i2_0 = BIT(0), + LPM_S0i2_1 = BIT(1), + LPM_S0i2_2 = BIT(2), + LPM_S0i3_0 = BIT(3), + LPM_S0i3_1 = BIT(4), + LPM_S0i3_2 = BIT(5), + LPM_S0i3_3 = BIT(6), + LPM_S0i3_4 = BIT(7), + LPM_S0iX_ALL = LPM_S0i2_0 | LPM_S0i2_1 | LPM_S0i2_2 + | LPM_S0i3_0 | LPM_S0i3_1 | LPM_S0i3_2 | LPM_S0i3_3 | LPM_S0i3_4, +}; + struct soc_intel_tigerlake_config {
/* Common struct containing soc config data required by common code */ @@ -78,6 +92,9 @@
/* Enable S0iX support */ int s0ix_enable; + /* S0iX: Selectively disable individual sub-states, by default all are enabled. */ + enum lpm_state_mask LpmStateDisableMask; + /* Support for TCSS xhci, xdci, TBT PCIe root ports and DMA controllers */ uint8_t TcssD3HotDisable; /* Support for TBT PCIe root ports and DMA controllers with D3Hot->D3Cold */ diff --git a/src/soc/intel/tigerlake/fsp_params.c b/src/soc/intel/tigerlake/fsp_params.c index a61a025..0a5fbe7 100644 --- a/src/soc/intel/tigerlake/fsp_params.c +++ b/src/soc/intel/tigerlake/fsp_params.c @@ -204,6 +204,14 @@ sizeof(params->SataPortsDevSlp)); }
+ /* S0iX: Selectively enable individual sub-states, + * by default all are enabled. + * + * LPM0-s0i2.0, LPM1-s0i2.1, LPM2-s0i2.2, LPM3-s0i3.0, + * LPM4-s0i3.1, LPM5-s0i3.2, LPM6-s0i3.3, LPM7-s0i3.4 + */ + params->LpmStateEnableMask = LPM_S0iX_ALL & ~config->LpmStateDisableMask; + /* * Power Optimizer for DMI and SATA. * DmiPwrOptimizeDisable and SataPwrOptimizeDisable is default to 0.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44355 )
Change subject: soc/intel/tigerlake: Allow fine grained control of S0iX states ......................................................................
Patch Set 8: Code-Review+2
-
build bot (Jenkins) (Code Review) -
Furquan Shaikh (Code Review)
-
Jes Klinke (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Patrick Georgi (Code Review)