linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Tiger Lake PMC core driver fixes
@ 2020-10-06 22:46 David E. Box
  2020-10-06 22:47 ` [PATCH 1/3] platform/x86: pmc_core: Use descriptive names for LPM registers David E. Box
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: David E. Box @ 2020-10-06 22:46 UTC (permalink / raw)
  To: dvhart, andy, gayatri.kammela
  Cc: David E. Box, platform-driver-x86, linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/3] platform/x86: pmc_core: Use descriptive names for LPM registers
  2020-10-06 22:46 [PATCH 0/3] Tiger Lake PMC core driver fixes David E. Box
@ 2020-10-06 22:47 ` David E. Box
  2020-10-06 22:47 ` [PATCH 2/3] platform/x86: intel_pmc_core: Fix TigerLake power gating status map David E. Box
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: David E. Box @ 2020-10-06 22:47 UTC (permalink / raw)
  To: dvhart, andy, gayatri.kammela
  Cc: David E. Box, platform-driver-x86, linux-kernel, Andy Shevchenko

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 <andriy.shevchenko@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/3] platform/x86: intel_pmc_core: Fix TigerLake power gating status map
  2020-10-06 22:46 [PATCH 0/3] Tiger Lake PMC core driver fixes David E. Box
  2020-10-06 22:47 ` [PATCH 1/3] platform/x86: pmc_core: Use descriptive names for LPM registers David E. Box
@ 2020-10-06 22:47 ` David E. Box
  2020-10-06 22:47 ` [PATCH 3/3] platform/x86: intel_pmc_core: Fix the slp_s0 counter displayed value David E. Box
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: David E. Box @ 2020-10-06 22:47 UTC (permalink / raw)
  To: dvhart, andy, gayatri.kammela
  Cc: platform-driver-x86, linux-kernel, Srinivas Pandruvada,
	Andy Shevchenko, David E . Box, David E . Box

From: Gayatri Kammela <gayatri.kammela@intel.com>

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 <srinivas.pandruvada@intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: David E. Box <david.e.box@intel.com>
Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 3/3] platform/x86: intel_pmc_core: Fix the slp_s0 counter displayed value
  2020-10-06 22:46 [PATCH 0/3] Tiger Lake PMC core driver fixes David E. Box
  2020-10-06 22:47 ` [PATCH 1/3] platform/x86: pmc_core: Use descriptive names for LPM registers David E. Box
  2020-10-06 22:47 ` [PATCH 2/3] platform/x86: intel_pmc_core: Fix TigerLake power gating status map David E. Box
@ 2020-10-06 22:47 ` David E. Box
  2020-10-07 12:12 ` [PATCH 0/3] Tiger Lake PMC core driver fixes Andy Shevchenko
  2020-10-07 21:12 ` Hans de Goede
  4 siblings, 0 replies; 6+ messages in thread
From: David E. Box @ 2020-10-06 22:47 UTC (permalink / raw)
  To: dvhart, andy, gayatri.kammela
  Cc: platform-driver-x86, linux-kernel, David E . Box

From: Gayatri Kammela <gayatri.kammela@intel.com>

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 <gayatri.kammela@intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] Tiger Lake PMC core driver fixes
  2020-10-06 22:46 [PATCH 0/3] Tiger Lake PMC core driver fixes David E. Box
                   ` (2 preceding siblings ...)
  2020-10-06 22:47 ` [PATCH 3/3] platform/x86: intel_pmc_core: Fix the slp_s0 counter displayed value David E. Box
@ 2020-10-07 12:12 ` Andy Shevchenko
  2020-10-07 21:12 ` Hans de Goede
  4 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2020-10-07 12:12 UTC (permalink / raw)
  To: David E. Box, Mark Gross, Hans de Goede
  Cc: Andy Shevchenko, Gayatri Kammela, Platform Driver,
	Linux Kernel Mailing List

On Wed, Oct 7, 2020 at 1:47 AM David E. Box <david.e.box@linux.intel.com> 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 <andy.shevchenko@gmail.com>

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] Tiger Lake PMC core driver fixes
  2020-10-06 22:46 [PATCH 0/3] Tiger Lake PMC core driver fixes David E. Box
                   ` (3 preceding siblings ...)
  2020-10-07 12:12 ` [PATCH 0/3] Tiger Lake PMC core driver fixes Andy Shevchenko
@ 2020-10-07 21:12 ` Hans de Goede
  4 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2020-10-07 21:12 UTC (permalink / raw)
  To: David E. Box, dvhart, andy, gayatri.kammela
  Cc: platform-driver-x86, linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-07 21:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-06 22:46 [PATCH 0/3] Tiger Lake PMC core driver fixes David E. Box
2020-10-06 22:47 ` [PATCH 1/3] platform/x86: pmc_core: Use descriptive names for LPM registers David E. Box
2020-10-06 22:47 ` [PATCH 2/3] platform/x86: intel_pmc_core: Fix TigerLake power gating status map David E. Box
2020-10-06 22:47 ` [PATCH 3/3] platform/x86: intel_pmc_core: Fix the slp_s0 counter displayed value David E. Box
2020-10-07 12:12 ` [PATCH 0/3] Tiger Lake PMC core driver fixes Andy Shevchenko
2020-10-07 21:12 ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).