This patch set adds several critical fixes for intel_pmc_core driver.
Patch 1: Uses descriptive register names for the TigerLake low power
mode registers. Not critical, but was requested in review of
Patch 2.
Patch 2: Fixes the register mapping to the correct IPs in the power
gating status register for TigerLake.
Patch 3: Fixes the slps0 residency multiplier to use the correct, platform
specific values.
David E. Box (1):
platform/x86: pmc_core: Use descriptive names for LPM registers
Gayatri Kammela (2):
platform/x86: intel_pmc_core: Fix TigerLake power gating status map
platform/x86: intel_pmc_core: Fix the slp_s0 counter displayed value
drivers/platform/x86/intel_pmc_core.c | 82 ++++++++++++++-------------
drivers/platform/x86/intel_pmc_core.h | 5 +-
2 files changed, 47 insertions(+), 40 deletions(-)
--
2.20.1
From: Gayatri Kammela <[email protected]>
slp_s0 counter value displayed via debugfs interface is calculated by
multiplying the granularity for crystal oscillator tick as 100us with
the value read from using slp_s0 offset. But the granularity of the tick
varies from platform to platform and it needs to be fixed.
Hence, specify granularity of the tick for each platform, so that the
value of the slp_s0 counter is accurate.
Signed-off-by: Gayatri Kammela <[email protected]>
Signed-off-by: David E. Box <[email protected]>
---
drivers/platform/x86/intel_pmc_core.c | 10 +++++++---
drivers/platform/x86/intel_pmc_core.h | 5 ++++-
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
index cf4006e08c69..122eb53eb595 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -154,6 +154,7 @@ static const struct pmc_reg_map spt_reg_map = {
.ltr_show_sts = spt_ltr_show_map,
.msr_sts = msr_map,
.slp_s0_offset = SPT_PMC_SLP_S0_RES_COUNTER_OFFSET,
+ .slp_s0_res_counter_step = SPT_PMC_SLP_S0_RES_COUNTER_STEP,
.ltr_ignore_offset = SPT_PMC_LTR_IGNORE_OFFSET,
.regmap_length = SPT_PMC_MMIO_REG_LEN,
.ppfear0_offset = SPT_PMC_XRAM_PPFEAR0A,
@@ -380,6 +381,7 @@ static const struct pmc_bit_map cnp_ltr_show_map[] = {
static const struct pmc_reg_map cnp_reg_map = {
.pfear_sts = ext_cnp_pfear_map,
.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+ .slp_s0_res_counter_step = SPT_PMC_SLP_S0_RES_COUNTER_STEP,
.slps0_dbg_maps = cnp_slps0_dbg_maps,
.ltr_show_sts = cnp_ltr_show_map,
.msr_sts = msr_map,
@@ -396,6 +398,7 @@ static const struct pmc_reg_map cnp_reg_map = {
static const struct pmc_reg_map icl_reg_map = {
.pfear_sts = ext_icl_pfear_map,
.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+ .slp_s0_res_counter_step = ICL_PMC_SLP_S0_RES_COUNTER_STEP,
.slps0_dbg_maps = cnp_slps0_dbg_maps,
.ltr_show_sts = cnp_ltr_show_map,
.msr_sts = msr_map,
@@ -558,6 +561,7 @@ static const struct pmc_bit_map *tgl_lpm_maps[] = {
static const struct pmc_reg_map tgl_reg_map = {
.pfear_sts = ext_tgl_pfear_map,
.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
+ .slp_s0_res_counter_step = TGL_PMC_SLP_S0_RES_COUNTER_STEP,
.ltr_show_sts = cnp_ltr_show_map,
.msr_sts = msr_map,
.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
@@ -586,9 +590,9 @@ static inline void pmc_core_reg_write(struct pmc_dev *pmcdev, int reg_offset,
writel(val, pmcdev->regbase + reg_offset);
}
-static inline u64 pmc_core_adjust_slp_s0_step(u32 value)
+static inline u64 pmc_core_adjust_slp_s0_step(struct pmc_dev *pmcdev, u32 value)
{
- return (u64)value * SPT_PMC_SLP_S0_RES_COUNTER_STEP;
+ return (u64)value * pmcdev->map->slp_s0_res_counter_step;
}
static int pmc_core_dev_state_get(void *data, u64 *val)
@@ -598,7 +602,7 @@ static int pmc_core_dev_state_get(void *data, u64 *val)
u32 value;
value = pmc_core_reg_read(pmcdev, map->slp_s0_offset);
- *val = pmc_core_adjust_slp_s0_step(value);
+ *val = pmc_core_adjust_slp_s0_step(pmcdev, value);
return 0;
}
diff --git a/drivers/platform/x86/intel_pmc_core.h b/drivers/platform/x86/intel_pmc_core.h
index 5eae55d80226..f33cd2c34835 100644
--- a/drivers/platform/x86/intel_pmc_core.h
+++ b/drivers/platform/x86/intel_pmc_core.h
@@ -30,7 +30,7 @@
#define SPT_PMC_MPHY_CORE_STS_1 0x1142
#define SPT_PMC_MPHY_COM_STS_0 0x1155
#define SPT_PMC_MMIO_REG_LEN 0x1000
-#define SPT_PMC_SLP_S0_RES_COUNTER_STEP 0x64
+#define SPT_PMC_SLP_S0_RES_COUNTER_STEP 0x68
#define PMC_BASE_ADDR_MASK ~(SPT_PMC_MMIO_REG_LEN - 1)
#define MTPMC_MASK 0xffff0000
#define PPFEAR_MAX_NUM_ENTRIES 12
@@ -185,8 +185,10 @@ enum ppfear_regs {
#define ICL_PPFEAR_NUM_ENTRIES 9
#define ICL_NUM_IP_IGN_ALLOWED 20
#define ICL_PMC_LTR_WIGIG 0x1BFC
+#define ICL_PMC_SLP_S0_RES_COUNTER_STEP 0x64
#define TGL_NUM_IP_IGN_ALLOWED 22
+#define TGL_PMC_SLP_S0_RES_COUNTER_STEP 0x7A
/*
* Tigerlake Power Management Controller register offsets
@@ -245,6 +247,7 @@ struct pmc_reg_map {
const struct pmc_bit_map *msr_sts;
const struct pmc_bit_map **lpm_sts;
const u32 slp_s0_offset;
+ const int slp_s0_res_counter_step;
const u32 ltr_ignore_offset;
const int regmap_length;
const u32 ppfear0_offset;
--
2.20.1
From: Gayatri Kammela <[email protected]>
TigerLake's LPM power gating status register has errors in the bit-to-name
mapping as well as with the marked reserved bits according to the actual
implementation. Hence, update the right bit-to-name mapping and the
reserved bits in accordance with actual implementation.
Cc: Srinivas Pandruvada <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: David E. Box <[email protected]>
Signed-off-by: Gayatri Kammela <[email protected]>
Signed-off-by: David E. Box <[email protected]>
---
drivers/platform/x86/intel_pmc_core.c | 48 +++++++++++++--------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
index ed9fdf7c8928..cf4006e08c69 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -426,30 +426,30 @@ static const struct pmc_bit_map tgl_clocksource_status_map[] = {
};
static const struct pmc_bit_map tgl_power_gating_status_map[] = {
- {"SPI_PG_STS", BIT(2)},
- {"xHCI_PG_STS", BIT(3)},
- {"PCIe_Ctrller_A_PG_STS", BIT(4)},
- {"PCIe_Ctrller_B_PG_STS", BIT(5)},
- {"PCIe_Ctrller_C_PG_STS", BIT(6)},
- {"GBE_PG_STS", BIT(7)},
- {"SATA_PG_STS", BIT(8)},
- {"HDA0_PG_STS", BIT(9)},
- {"HDA1_PG_STS", BIT(10)},
- {"HDA2_PG_STS", BIT(11)},
- {"HDA3_PG_STS", BIT(12)},
- {"PCIe_Ctrller_D_PG_STS", BIT(13)},
- {"ISIO_PG_STS", BIT(14)},
- {"SMB_PG_STS", BIT(16)},
- {"ISH_PG_STS", BIT(17)},
- {"ITH_PG_STS", BIT(19)},
- {"SDX_PG_STS", BIT(20)},
- {"xDCI_PG_STS", BIT(25)},
- {"DCI_PG_STS", BIT(26)},
- {"CSME0_PG_STS", BIT(27)},
- {"CSME_KVM_PG_STS", BIT(28)},
- {"CSME1_PG_STS", BIT(29)},
- {"CSME_CLINK_PG_STS", BIT(30)},
- {"CSME2_PG_STS", BIT(31)},
+ {"CSME_PG_STS", BIT(0)},
+ {"SATA_PG_STS", BIT(1)},
+ {"xHCI_PG_STS", BIT(2)},
+ {"UFSX2_PG_STS", BIT(3)},
+ {"OTG_PG_STS", BIT(5)},
+ {"SPA_PG_STS", BIT(6)},
+ {"SPB_PG_STS", BIT(7)},
+ {"SPC_PG_STS", BIT(8)},
+ {"SPD_PG_STS", BIT(9)},
+ {"SPE_PG_STS", BIT(10)},
+ {"SPF_PG_STS", BIT(11)},
+ {"LSX_PG_STS", BIT(13)},
+ {"P2SB_PG_STS", BIT(14)},
+ {"PSF_PG_STS", BIT(15)},
+ {"SBR_PG_STS", BIT(16)},
+ {"OPIDMI_PG_STS", BIT(17)},
+ {"THC0_PG_STS", BIT(18)},
+ {"THC1_PG_STS", BIT(19)},
+ {"GBETSN_PG_STS", BIT(20)},
+ {"GBE_PG_STS", BIT(21)},
+ {"LPSS_PG_STS", BIT(22)},
+ {"MMP_UFSX2_PG_STS", BIT(23)},
+ {"MMP_UFSX2B_PG_STS", BIT(24)},
+ {"FIA_PG_STS", BIT(25)},
{}
};
--
2.20.1
TigerLake Lower Power Mode (LPM) registers are grouped by functionality
but were given simple enumerated names in the code (lpm0, lpm1, ...).
Instead, give the register blocks names that describe their usage.
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: David E. Box <[email protected]>
---
drivers/platform/x86/intel_pmc_core.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c
index 338ea5222555..ed9fdf7c8928 100644
--- a/drivers/platform/x86/intel_pmc_core.c
+++ b/drivers/platform/x86/intel_pmc_core.c
@@ -409,7 +409,7 @@ static const struct pmc_reg_map icl_reg_map = {
.ltr_ignore_max = ICL_NUM_IP_IGN_ALLOWED,
};
-static const struct pmc_bit_map tgl_lpm0_map[] = {
+static const struct pmc_bit_map tgl_clocksource_status_map[] = {
{"USB2PLL_OFF_STS", BIT(18)},
{"PCIe/USB3.1_Gen2PLL_OFF_STS", BIT(19)},
{"PCIe_Gen3PLL_OFF_STS", BIT(20)},
@@ -425,7 +425,7 @@ static const struct pmc_bit_map tgl_lpm0_map[] = {
{}
};
-static const struct pmc_bit_map tgl_lpm1_map[] = {
+static const struct pmc_bit_map tgl_power_gating_status_map[] = {
{"SPI_PG_STS", BIT(2)},
{"xHCI_PG_STS", BIT(3)},
{"PCIe_Ctrller_A_PG_STS", BIT(4)},
@@ -453,7 +453,7 @@ static const struct pmc_bit_map tgl_lpm1_map[] = {
{}
};
-static const struct pmc_bit_map tgl_lpm2_map[] = {
+static const struct pmc_bit_map tgl_d3_status_map[] = {
{"ADSP_D3_STS", BIT(0)},
{"SATA_D3_STS", BIT(1)},
{"xHCI0_D3_STS", BIT(2)},
@@ -468,7 +468,7 @@ static const struct pmc_bit_map tgl_lpm2_map[] = {
{}
};
-static const struct pmc_bit_map tgl_lpm3_map[] = {
+static const struct pmc_bit_map tgl_vnn_req_status_map[] = {
{"GPIO_COM0_VNN_REQ_STS", BIT(1)},
{"GPIO_COM1_VNN_REQ_STS", BIT(2)},
{"GPIO_COM2_VNN_REQ_STS", BIT(3)},
@@ -493,7 +493,7 @@ static const struct pmc_bit_map tgl_lpm3_map[] = {
{}
};
-static const struct pmc_bit_map tgl_lpm4_map[] = {
+static const struct pmc_bit_map tgl_vnn_misc_status_map[] = {
{"CPU_C10_REQ_STS_0", BIT(0)},
{"PCIe_LPM_En_REQ_STS_3", BIT(3)},
{"ITH_REQ_STS_5", BIT(5)},
@@ -509,7 +509,7 @@ static const struct pmc_bit_map tgl_lpm4_map[] = {
{}
};
-static const struct pmc_bit_map tgl_lpm5_map[] = {
+static const struct pmc_bit_map tgl_signal_status_map[] = {
{"LSX_Wake0_En_STS", BIT(0)},
{"LSX_Wake0_Pol_STS", BIT(1)},
{"LSX_Wake1_En_STS", BIT(2)},
@@ -546,12 +546,12 @@ static const struct pmc_bit_map tgl_lpm5_map[] = {
};
static const struct pmc_bit_map *tgl_lpm_maps[] = {
- tgl_lpm0_map,
- tgl_lpm1_map,
- tgl_lpm2_map,
- tgl_lpm3_map,
- tgl_lpm4_map,
- tgl_lpm5_map,
+ tgl_clocksource_status_map,
+ tgl_power_gating_status_map,
+ tgl_d3_status_map,
+ tgl_vnn_req_status_map,
+ tgl_vnn_misc_status_map,
+ tgl_signal_status_map,
NULL
};
--
2.20.1
On Wed, Oct 7, 2020 at 1:47 AM David E. Box <[email protected]> wrote:
>
> This patch set adds several critical fixes for intel_pmc_core driver.
>
> Patch 1: Uses descriptive register names for the TigerLake low power
> mode registers. Not critical, but was requested in review of
> Patch 2.
>
> Patch 2: Fixes the register mapping to the correct IPs in the power
> gating status register for TigerLake.
>
> Patch 3: Fixes the slps0 residency multiplier to use the correct, platform
> specific values.
Hans, Mark, this series has been internally reviewed and tested on
affected hardware, I think it's ready to go for v5.10.
Reviewed-by: Andy Shevchenko <[email protected]>
David, I'm not a maintainer anymore here.
>
> David E. Box (1):
> platform/x86: pmc_core: Use descriptive names for LPM registers
>
> Gayatri Kammela (2):
> platform/x86: intel_pmc_core: Fix TigerLake power gating status map
> platform/x86: intel_pmc_core: Fix the slp_s0 counter displayed value
>
> drivers/platform/x86/intel_pmc_core.c | 82 ++++++++++++++-------------
> drivers/platform/x86/intel_pmc_core.h | 5 +-
> 2 files changed, 47 insertions(+), 40 deletions(-)
>
> --
> 2.20.1
>
--
With Best Regards,
Andy Shevchenko
Hi,
On 10/7/20 12:46 AM, David E. Box wrote:
> This patch set adds several critical fixes for intel_pmc_core driver.
>
> Patch 1: Uses descriptive register names for the TigerLake low power
> mode registers. Not critical, but was requested in review of
> Patch 2.
>
> Patch 2: Fixes the register mapping to the correct IPs in the power
> gating status register for TigerLake.
>
> Patch 3: Fixes the slps0 residency multiplier to use the correct, platform
> specific values.
>
> David E. Box (1):
> platform/x86: pmc_core: Use descriptive names for LPM registers
>
> Gayatri Kammela (2):
> platform/x86: intel_pmc_core: Fix TigerLake power gating status map
> platform/x86: intel_pmc_core: Fix the slp_s0 counter displayed value
Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up there once I've pushed my local branch there,
which might take a while.
Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans