Hello Raul Rangel, Akshu Agrawal,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/43495
to review the following change.
Change subject: soc/amd/picasso: Enable ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
soc/amd/picasso: Enable ACP_PME_EN and ACP_I2S_WAKE_EN
ACP_PME_EN and ACP_I2S_WAKE_EN are to be set in order to get I2S_Wake event on headset jack plug/unplug.
BUG=b:146317284,b:161328042 TEST=Boot and check on HDT the register value.
Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com Change-Id: I522d7497940f499fbc3181d866f2b44e979bba7a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Reviewed-by: Raul E Rangel rrangel@chromium.org Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/acp.c M src/soc/amd/picasso/include/soc/southbridge.h 2 files changed, 9 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/43495/1
diff --git a/src/soc/amd/picasso/acp.c b/src/soc/amd/picasso/acp.c index 3272acf..63f5c63 100644 --- a/src/soc/amd/picasso/acp.c +++ b/src/soc/amd/picasso/acp.c @@ -33,6 +33,10 @@ bar = (uintptr_t)res->base; write32((void *)(bar + ACP_I2S_PIN_CONFIG), cfg->acp_pin_cfg);
+ /* Enable ACP_PME_EN and ACP_I2S_WAKE_EN for I2S_WAKE event*/ + write32((void *)(bar + ACP_I2S_WAKE_EN), 1); + write32((void *)(bar + ACP_PME_EN), 1); + if (cfg->acp_pin_cfg == I2S_PINS_I2S_TDM) sb_clk_output_48Mhz(); /* Internal connection to I2S */ } diff --git a/src/soc/amd/picasso/include/soc/southbridge.h b/src/soc/amd/picasso/include/soc/southbridge.h index e9f7e2e..1242882 100644 --- a/src/soc/amd/picasso/include/soc/southbridge.h +++ b/src/soc/amd/picasso/include/soc/southbridge.h @@ -252,6 +252,11 @@ /* IO 0xf0 NCP Error */ #define NCP_WARM_BOOT BIT(7) /* Write-once */
+/* ACP registers */ +#define ACP_I2S_PIN_CONFIG 0x1400 +#define ACP_I2S_WAKE_EN 0x1414 +#define ACP_PME_EN 0x1418 + typedef struct aoac_devs { unsigned int :7; unsigned int ic2e:1; /* 7: I2C2 */
Hello Raul Rangel, Akshu Agrawal, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43495
to look at the new patch set (#2).
Change subject: soc/amd/picasso: Enable ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
soc/amd/picasso: Enable ACP_PME_EN and ACP_I2S_WAKE_EN
ACP_PME_EN and ACP_I2S_WAKE_EN are to be set in order to get I2S_Wake event on headset jack plug/unplug.
BUG=b:146317284,b:161328042 TEST=Boot and check on HDT the register value.
Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com Change-Id: I522d7497940f499fbc3181d866f2b44e979bba7a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Reviewed-by: Raul E Rangel rrangel@chromium.org Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/acp.c M src/soc/amd/picasso/include/soc/acp.h 2 files changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/43495/2
Hello build bot (Jenkins), Raul Rangel, Akshu Agrawal, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43495
to look at the new patch set (#3).
Change subject: soc/amd/picasso: Enable ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
soc/amd/picasso: Enable ACP_PME_EN and ACP_I2S_WAKE_EN
ACP_PME_EN and ACP_I2S_WAKE_EN are to be set in order to get I2S_Wake event on headset jack plug/unplug.
BUG=b:146317284,b:161328042 TEST=Boot and check on HDT the register value.
Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com Change-Id: I522d7497940f499fbc3181d866f2b44e979bba7a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Reviewed-by: Raul E Rangel rrangel@chromium.org Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/acp.c M src/soc/amd/picasso/include/soc/acp.h 2 files changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/43495/3
Hello build bot (Jenkins), Raul Rangel, Akshu Agrawal, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43495
to look at the new patch set (#4).
Change subject: soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN
This change adds support for configuring ACP_PME_EN and ACP_I2S_WAKE_EN using the mainboard setting for `acp_pme_enable` and `acp_i2s_wake_enable` in the devicetree. This is required to get I2S_Wake event on headset jack plug/unplug when using CODEC_GPI pad.
BUG=b:146317284,b:161328042
Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com Change-Id: I522d7497940f499fbc3181d866f2b44e979bba7a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Reviewed-by: Raul E Rangel rrangel@chromium.org Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/acp.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/acp.h 3 files changed, 11 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/43495/4
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43495 )
Change subject: soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/43495/4/src/soc/amd/picasso/acp.c File src/soc/amd/picasso/acp.c:
https://review.coreboot.org/c/coreboot/+/43495/4/src/soc/amd/picasso/acp.c@3... PS4, Line 38: write32((void *)(bar + ACP_PME_EN), !!cfg->acpi_pme_enable); We should technically be doing a r-m-w since the other bits are marked 'reserved'.
Hello build bot (Jenkins), Raul Rangel, Akshu Agrawal, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43495
to look at the new patch set (#5).
Change subject: soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN
This change adds support for configuring ACP_PME_EN and ACP_I2S_WAKE_EN using the mainboard setting for `acp_pme_enable` and `acp_i2s_wake_enable` in the devicetree. This is required to get I2S_Wake event on headset jack plug/unplug when using CODEC_GPI pad.
BUG=b:146317284,b:161328042
Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com Change-Id: I522d7497940f499fbc3181d866f2b44e979bba7a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Reviewed-by: Raul E Rangel rrangel@chromium.org Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/acp.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/acp.h 3 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/43495/5
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43495 )
Change subject: soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43495/4/src/soc/amd/picasso/acp.c File src/soc/amd/picasso/acp.c:
https://review.coreboot.org/c/coreboot/+/43495/4/src/soc/amd/picasso/acp.c@3... PS4, Line 38: write32((void *)(bar + ACP_PME_EN), !!cfg->acpi_pme_enable);
We should technically be doing a r-m-w since the other bits are marked 'reserved'.
Makes sense. I kept it as write32 since the other bits come out of reset as 0. But, I agree doing a r-m-w would be more appropriate.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43495 )
Change subject: soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
Patch Set 5: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/43495/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43495/5//COMMIT_MSG@11 PS5, Line 11: jack nit: to long
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43495 )
Change subject: soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43495/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43495/5//COMMIT_MSG@11 PS5, Line 11: jack
nit: to long
I was asked to reflow some of my other commit messages to 75 characters. This line has 73 characters.
Hello build bot (Jenkins), Raul Rangel, Akshu Agrawal, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43495
to look at the new patch set (#6).
Change subject: soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN
This change adds support for configuring ACP_PME_EN and ACP_I2S_WAKE_EN using the mainboard setting for `acp_pme_enable` and `acp_i2s_wake_enable` in the devicetree. This is required to get I2S_Wake event on headset jack plug/unplug when using CODEC_GPI pad.
BUG=b:146317284,b:161328042
Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com Change-Id: I522d7497940f499fbc3181d866f2b44e979bba7a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Reviewed-by: Raul E Rangel rrangel@chromium.org Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/picasso/acp.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/acp.h 3 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/43495/6
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43495 )
Change subject: soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
Patch Set 6: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43495 )
Change subject: soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43495/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43495/5//COMMIT_MSG@11 PS5, Line 11: jack
I was asked to reflow some of my other commit messages to 75 characters. […]
Done
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43495 )
Change subject: soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN ......................................................................
soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_EN
This change adds support for configuring ACP_PME_EN and ACP_I2S_WAKE_EN using the mainboard setting for `acp_pme_enable` and `acp_i2s_wake_enable` in the devicetree. This is required to get I2S_Wake event on headset jack plug/unplug when using CODEC_GPI pad.
BUG=b:146317284,b:161328042
Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com Change-Id: I522d7497940f499fbc3181d866f2b44e979bba7a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+... Reviewed-by: Raul E Rangel rrangel@chromium.org Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/43495 Reviewed-by: Aaron Durbin adurbin@chromium.org Reviewed-by: Raul Rangel rrangel@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/picasso/acp.c M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/include/soc/acp.h 3 files changed, 23 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Raul Rangel: Looks good to me, approved
diff --git a/src/soc/amd/picasso/acp.c b/src/soc/amd/picasso/acp.c index 3272acf..b598e64 100644 --- a/src/soc/amd/picasso/acp.c +++ b/src/soc/amd/picasso/acp.c @@ -15,6 +15,16 @@ #include <amdblocks/acpimmio.h> #include <commonlib/helpers.h>
+static void acp_update32(uintptr_t bar, uint32_t reg, uint32_t and_mask, uint32_t or_mask) +{ + uint32_t val; + + val = read32((void *)(bar + reg)); + val &= ~and_mask; + val |= or_mask; + write32((void *)(bar + reg), val); +} + static void init(struct device *dev) { const struct soc_amd_picasso_config *cfg; @@ -33,6 +43,10 @@ bar = (uintptr_t)res->base; write32((void *)(bar + ACP_I2S_PIN_CONFIG), cfg->acp_pin_cfg);
+ /* Enable ACP_PME_EN and ACP_I2S_WAKE_EN for I2S_WAKE event */ + acp_update32(bar, ACP_I2S_WAKE_EN, WAKE_EN_MASK, !!cfg->acp_i2s_wake_enable); + acp_update32(bar, ACP_PME_EN, PME_EN_MASK, !!cfg->acpi_pme_enable); + if (cfg->acp_pin_cfg == I2S_PINS_I2S_TDM) sb_clk_output_48Mhz(); /* Internal connection to I2S */ } diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h index 1d3ee9d..4891b22 100644 --- a/src/soc/amd/picasso/chip.h +++ b/src/soc/amd/picasso/chip.h @@ -59,6 +59,11 @@ I2S_PINS_UNCONF = 7, /* All pads will be input mode */ } acp_pin_cfg;
+ /* Enable ACP I2S wake feature (0 = disable, 1 = enable) */ + u8 acp_i2s_wake_enable; + /* Enable ACP PME (0 = disable, 1 = enable) */ + u8 acpi_pme_enable; + /** * IRQ 0 - 15 have a default trigger of edge and default polarity of high. * If you have a device that requires a different configuration you can override the diff --git a/src/soc/amd/picasso/include/soc/acp.h b/src/soc/amd/picasso/include/soc/acp.h index 8825da8..36bd6fb 100644 --- a/src/soc/amd/picasso/include/soc/acp.h +++ b/src/soc/amd/picasso/include/soc/acp.h @@ -5,5 +5,9 @@
/* Bus A D0F5 - Audio Processor */ #define ACP_I2S_PIN_CONFIG 0x1400 /* HDA, Soundwire, I2S */ +#define ACP_I2S_WAKE_EN 0x1414 +#define WAKE_EN_MASK (1 << 0) +#define ACP_PME_EN 0x1418 +#define PME_EN_MASK (1 << 0)
#endif /* __PI_PICASSO_ACP_H__ */
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Raul Rangel (Code Review)