2017-11-16 16:28:26

by Souvik Kumar Chakravarty

[permalink] [raw]
Subject: [PATCH v1 0/5] platform/x86: intel_telemetry: Fix counters and formatting

This patchset fixes https://bugzilla.kernel.org/show_bug.cgi?id=197833, and
other issues related to telemetry counters. It also cleans up formatting
and removes redundanant code.

It is rebased on top of the TESTING branch.

Souvik Kumar Chakravarty (5):
platform/x86: intel_pmc_ipc: Fix register names
platform/x86: intel_telemetry: Fix suspend stats
platform/x86: intel_telemetry: Fix local variables
platform/x86: intel_telemetry: Remove redundancies
platform/x86: intel_telemetry: Improve S0ix logs

arch/x86/include/asm/intel_pmc_ipc.h | 10 ++-
drivers/platform/x86/intel_pmc_ipc.c | 4 +-
drivers/platform/x86/intel_telemetry_debugfs.c | 101 +++++++++++++------------
3 files changed, 59 insertions(+), 56 deletions(-)

--
2.7.4


From 1584580308772470011@xxx Mon Nov 20 10:25:42 +0000 2017
X-GM-THRID: 1584579404346566342
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread


2017-11-16 15:45:34

by Souvik Kumar Chakravarty

[permalink] [raw]
Subject: [PATCH v1 1/5] platform/x86: intel_pmc_ipc: Fix register names

GCR TELEM register names have been fixed as per the External Design
Sepcification (EDS) of Apollolake and GeminiLake. This makes it
possible to fetch the complete 64-bit S0ix counter using exported APIs.

This patch also fixes some alignment issues in the macro definition
section.

Signed-off-by: Souvik Kumar Chakravarty <[email protected]>
---
arch/x86/include/asm/intel_pmc_ipc.h | 10 ++++++----
drivers/platform/x86/intel_pmc_ipc.c | 4 ++--
2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/intel_pmc_ipc.h b/arch/x86/include/asm/intel_pmc_ipc.h
index fac89eb..b41f388 100644
--- a/arch/x86/include/asm/intel_pmc_ipc.h
+++ b/arch/x86/include/asm/intel_pmc_ipc.h
@@ -3,8 +3,8 @@

/* Commands */
#define PMC_IPC_PMIC_ACCESS 0xFF
-#define PMC_IPC_PMIC_ACCESS_READ 0x0
-#define PMC_IPC_PMIC_ACCESS_WRITE 0x1
+#define PMC_IPC_PMIC_ACCESS_READ 0x0
+#define PMC_IPC_PMIC_ACCESS_WRITE 0x1
#define PMC_IPC_USB_PWR_CTRL 0xF0
#define PMC_IPC_PMIC_BLACKLIST_SEL 0xEF
#define PMC_IPC_PHY_CONFIG 0xEE
@@ -25,8 +25,10 @@

/* GCR reg offsets from gcr base*/
#define PMC_GCR_PMC_CFG_REG 0x08
-#define PMC_GCR_TELEM_DEEP_S0IX_REG 0x78
-#define PMC_GCR_TELEM_SHLW_S0IX_REG 0x80
+#define PMC_GCR_TELEM_DEEP_S0IX_LO_REG 0x78
+#define PMC_GCR_TELEM_DEEP_S0IX_HI_REG 0x7C
+#define PMC_GCR_TELEM_SHLW_S0IX_LO_REG 0x80
+#define PMC_GCR_TELEM_SHLW_S0IX_HI_REG 0x84

#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)

diff --git a/drivers/platform/x86/intel_pmc_ipc.c b/drivers/platform/x86/intel_pmc_ipc.c
index e03fa314..1c096e7 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -883,8 +883,8 @@ int intel_pmc_s0ix_counter_read(u64 *data)
if (!ipcdev.has_gcr_regs)
return -EACCES;

- deep = gcr_data_readq(PMC_GCR_TELEM_DEEP_S0IX_REG);
- shlw = gcr_data_readq(PMC_GCR_TELEM_SHLW_S0IX_REG);
+ deep = gcr_data_readq(PMC_GCR_TELEM_DEEP_S0IX_LO_REG);
+ shlw = gcr_data_readq(PMC_GCR_TELEM_SHLW_S0IX_LO_REG);

*data = S0IX_RESIDENCY_IN_USECS(deep, shlw);

--
2.7.4


From 1584366039183993373@xxx Sat Nov 18 01:39:59 +0000 2017
X-GM-THRID: 1584366039183993373
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-16 16:11:13

by Souvik Kumar Chakravarty

[permalink] [raw]
Subject: [PATCH v1 3/5] platform/x86: intel_telemetry: Fix local variables

Remove unnecesary 'static' qualifier.

Signed-off-by: Souvik Kumar Chakravarty <[email protected]>
---
drivers/platform/x86/intel_telemetry_debugfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c b/drivers/platform/x86/intel_telemetry_debugfs.c
index d0fce8c..60596a1 100644
--- a/drivers/platform/x86/intel_telemetry_debugfs.c
+++ b/drivers/platform/x86/intel_telemetry_debugfs.c
@@ -857,8 +857,8 @@ static int pm_suspend_prep_cb(void)
static int pm_suspend_exit_cb(void)
{
struct telemetry_evtlog evtlog[TELEM_MAX_OS_ALLOCATED_EVENTS];
- static u32 suspend_shlw_ctr_exit, suspend_deep_ctr_exit;
- static u64 suspend_shlw_res_exit, suspend_deep_res_exit;
+ u32 suspend_shlw_ctr_exit = 0, suspend_deep_ctr_exit = 0;
+ u64 suspend_shlw_res_exit = 0, suspend_deep_res_exit = 0;
struct telemetry_debugfs_conf *conf = debugfs_conf;
u32 shlw_lo, shlw_hi, deep_lo, deep_hi;
int ret, index;
--
2.7.4


From 1584340805995434264@xxx Fri Nov 17 18:58:55 +0000 2017
X-GM-THRID: 1584340805995434264
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-16 16:28:43

by Souvik Kumar Chakravarty

[permalink] [raw]
Subject: [PATCH v1 5/5] platform/x86: intel_telemetry: Improve S0ix logs

Suspend with shallow wakes is not a useful parameter since the phenomena
does not exist on deployed devices and is only a parameter of use during
device power-on phase. The field always reads zero. Additionally there
are other easier methods to detect it, e.g., if the S0ix counter
increments by more than one during suspend. Hence the field is superfluous
and can be removed.

This patch also slightly renames the S0ix total field for better
viewability.

Signed-off-by: Souvik Kumar Chakravarty <[email protected]>
---
drivers/platform/x86/intel_telemetry_debugfs.c | 47 +++++---------------------
1 file changed, 8 insertions(+), 39 deletions(-)

diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c b/drivers/platform/x86/intel_telemetry_debugfs.c
index 2a960f6..b40bacc0 100644
--- a/drivers/platform/x86/intel_telemetry_debugfs.c
+++ b/drivers/platform/x86/intel_telemetry_debugfs.c
@@ -98,10 +98,6 @@ static u32 suspend_shlw_ctr_temp, suspend_deep_ctr_temp;
static u64 suspend_shlw_res_temp, suspend_deep_res_temp;

struct telemetry_susp_stats {
- u32 shlw_swake_ctr;
- u32 deep_swake_ctr;
- u64 shlw_swake_res;
- u64 deep_swake_res;
u32 shlw_ctr;
u32 deep_ctr;
u64 shlw_res;
@@ -594,19 +590,15 @@ static int telem_soc_states_show(struct seq_file *s, void *unused)

seq_printf(s, "S0IX Shallow\t\t\t %10u\t %10llu\n",
s0ix_shlw_ctr -
- conf->suspend_stats.shlw_ctr -
- conf->suspend_stats.shlw_swake_ctr,
+ conf->suspend_stats.shlw_ctr,
(u64)((s0ix_shlw_res -
- conf->suspend_stats.shlw_res -
- conf->suspend_stats.shlw_swake_res)*10/192));
+ conf->suspend_stats.shlw_res)*10/192));

seq_printf(s, "S0IX Deep\t\t\t %10u\t %10llu\n",
s0ix_deep_ctr -
- conf->suspend_stats.deep_ctr -
- conf->suspend_stats.deep_swake_ctr,
+ conf->suspend_stats.deep_ctr,
(u64)((s0ix_deep_res -
- conf->suspend_stats.deep_res -
- conf->suspend_stats.deep_swake_res)*10/192));
+ conf->suspend_stats.deep_res)*10/192));

seq_printf(s, "Suspend(With S0ixShallow)\t %10u\t %10llu\n",
conf->suspend_stats.shlw_ctr,
@@ -616,14 +608,8 @@ static int telem_soc_states_show(struct seq_file *s, void *unused)
conf->suspend_stats.deep_ctr,
(u64)(conf->suspend_stats.deep_res*10)/192);

- seq_printf(s, "Suspend(With Shallow-Wakes)\t %10u\t %10llu\n",
- conf->suspend_stats.shlw_swake_ctr +
- conf->suspend_stats.deep_swake_ctr,
- (u64)((conf->suspend_stats.shlw_swake_res +
- conf->suspend_stats.deep_swake_res)*10/192));
-
- seq_printf(s, "S0IX+Suspend Total\t\t %10u\t %10llu\n", s0ix_total_ctr,
- (u64)(s0ix_total_res*10/192));
+ seq_printf(s, "TOTAL S0IX\t\t\t %10u\t %10llu\n", s0ix_total_ctr,
+ (u64)(s0ix_total_res*10/192));
seq_puts(s, "\n-------------------------------------------------\n");
seq_puts(s, "\t\tDEVICE STATES\n");
seq_puts(s, "-------------------------------------------------\n");
@@ -927,23 +913,15 @@ static int pm_suspend_exit_cb(void)
suspend_shlw_res_exit -= suspend_shlw_res_temp;
suspend_deep_res_exit -= suspend_deep_res_temp;

- if (suspend_shlw_ctr_exit == 1) {
+ if (suspend_shlw_ctr_exit > 0) {
conf->suspend_stats.shlw_ctr +=
suspend_shlw_ctr_exit;

conf->suspend_stats.shlw_res +=
suspend_shlw_res_exit;
}
- /* Shallow Wakes Case */
- else if (suspend_shlw_ctr_exit > 1) {
- conf->suspend_stats.shlw_swake_ctr +=
- suspend_shlw_ctr_exit;
-
- conf->suspend_stats.shlw_swake_res +=
- suspend_shlw_res_exit;
- }

- if (suspend_deep_ctr_exit == 1) {
+ if (suspend_deep_ctr_exit > 0) {
conf->suspend_stats.deep_ctr +=
suspend_deep_ctr_exit;

@@ -951,15 +929,6 @@ static int pm_suspend_exit_cb(void)
suspend_deep_res_exit;
}

- /* Shallow Wakes Case */
- else if (suspend_deep_ctr_exit > 1) {
- conf->suspend_stats.deep_swake_ctr +=
- suspend_deep_ctr_exit;
-
- conf->suspend_stats.deep_swake_res +=
- suspend_deep_res_exit;
- }
-
out:
suspend_prep_ok = 0;
return NOTIFY_OK;
--
2.7.4


From 1584406795349561758@xxx Sat Nov 18 12:27:47 +0000 2017
X-GM-THRID: 1584406795349561758
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-16 16:29:09

by Souvik Kumar Chakravarty

[permalink] [raw]
Subject: [PATCH v1 4/5] platform/x86: intel_telemetry: Remove redundancies

This patch removes unnecessary header files and newlines.
It also fixes some alignment issues.

Signed-off-by: Souvik Kumar Chakravarty <[email protected]>
---
drivers/platform/x86/intel_telemetry_debugfs.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c b/drivers/platform/x86/intel_telemetry_debugfs.c
index 60596a1..2a960f6 100644
--- a/drivers/platform/x86/intel_telemetry_debugfs.c
+++ b/drivers/platform/x86/intel_telemetry_debugfs.c
@@ -23,7 +23,6 @@
*/
#include <linux/debugfs.h>
#include <linux/device.h>
-#include <linux/io.h>
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/seq_file.h>
@@ -32,11 +31,10 @@
#include <asm/cpu_device_id.h>
#include <asm/intel-family.h>
#include <asm/intel_pmc_ipc.h>
-#include <asm/intel_punit_ipc.h>
#include <asm/intel_telemetry.h>

-#define DRIVER_NAME "telemetry_soc_debugfs"
-#define DRIVER_VERSION "1.0.0"
+#define DRIVER_NAME "telemetry_soc_debugfs"
+#define DRIVER_VERSION "1.0.0"

/* ApolloLake SoC Event-IDs */
#define TELEM_APL_PSS_PSTATES_ID 0x2802
@@ -252,7 +250,6 @@ static struct telem_ioss_pg_info telem_apl_ioss_pg_data[] = {
{"PRTC", 25},
};

-
struct telemetry_debugfs_conf {
struct telemetry_susp_stats suspend_stats;
struct dentry *telemetry_dbg_dir;
@@ -387,7 +384,6 @@ static int telem_pss_states_show(struct seq_file *s, void *unused)
TELEM_APL_MASK_PCS_STATE;
}

-
TELEM_CHECK_AND_PARSE_EVTS(conf->pss_idle_id,
conf->pss_idle_evts - 1,
pss_idle, evtlog[index].telem_evtlog,
@@ -407,7 +403,6 @@ static int telem_pss_states_show(struct seq_file *s, void *unused)
conf->pcs_s0ix_blkd_data,
TELEM_MASK_BYTE);

-
TELEM_CHECK_AND_PARSE_EVTS(conf->pss_wakeup_id,
conf->pss_wakeup_evts,
pss_s0ix_wakeup,
@@ -500,7 +495,6 @@ static const struct file_operations telem_pss_ops = {
.release = single_release,
};

-
static int telem_ioss_states_show(struct seq_file *s, void *unused)
{
struct telemetry_evtlog evtlog[TELEM_MAX_OS_ALLOCATED_EVENTS];
@@ -774,7 +768,6 @@ static const struct file_operations telem_pss_trc_verb_ops = {
.release = single_release,
};

-
static int telem_ioss_trc_verb_show(struct seq_file *s, void *unused)
{
u32 verbosity;
--
2.7.4


From 1584268690118731418@xxx Thu Nov 16 23:52:40 +0000 2017
X-GM-THRID: 1584096400353009036
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-16 15:42:38

by Souvik Kumar Chakravarty

[permalink] [raw]
Subject: [PATCH v1 2/5] platform/x86: intel_telemetry: Fix suspend stats

Suspend stats are not reported consistently due to a limitation in the PMC
firmware. This limitation causes a delay in updating the s0ix counters and
residencies in the telemetry log upon s0ix exit. As a consequence, reading
these counters from the suspend-exit notifier may result in zero read.

This patch fixes this issue by cross-verifying the s0ix residencies from
the GCR TELEM registers in case the counters are not incremented in the
telemetry log after suspend.

This fixes https://bugzilla.kernel.org/show_bug.cgi?id=197833

Reported-and-tested-by: Rajneesh Bhardwaj <[email protected]>
Signed-off-by: Souvik Kumar Chakravarty <[email protected]>
---
drivers/platform/x86/intel_telemetry_debugfs.c | 39 ++++++++++++++++++++++++++
1 file changed, 39 insertions(+)

diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c b/drivers/platform/x86/intel_telemetry_debugfs.c
index 4249e826..d0fce8c 100644
--- a/drivers/platform/x86/intel_telemetry_debugfs.c
+++ b/drivers/platform/x86/intel_telemetry_debugfs.c
@@ -74,6 +74,8 @@
#define TELEM_IOSS_DX_D0IX_EVTS 25
#define TELEM_IOSS_PG_EVTS 30

+#define TELEM_LO_HI_TO64(lo, hi) ((u64)(lo) + ((u64)(hi)<<32))
+
#define TELEM_DEBUGFS_CPU(model, data) \
{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long)&data}

@@ -858,6 +860,7 @@ static int pm_suspend_exit_cb(void)
static u32 suspend_shlw_ctr_exit, suspend_deep_ctr_exit;
static u64 suspend_shlw_res_exit, suspend_deep_res_exit;
struct telemetry_debugfs_conf *conf = debugfs_conf;
+ u32 shlw_lo, shlw_hi, deep_lo, deep_hi;
int ret, index;

if (!suspend_prep_ok)
@@ -890,6 +893,42 @@ static int pm_suspend_exit_cb(void)
goto out;
}

+ /* Due to some design limitations in the firmware, sometimes the
+ * counters do not get updated by the time we reach here. As a
+ * workaround, we try to see if this was a genuine case of sleep
+ * failure or not by cross-checking from PMC GCR registers directly.
+ */
+ if ((suspend_shlw_ctr_exit == suspend_shlw_ctr_temp) &&
+ (suspend_deep_ctr_exit == suspend_deep_ctr_temp)) {
+ ret = intel_pmc_gcr_read(PMC_GCR_TELEM_SHLW_S0IX_LO_REG,
+ &shlw_lo);
+ if (ret < 0)
+ goto out;
+
+ ret = intel_pmc_gcr_read(PMC_GCR_TELEM_SHLW_S0IX_HI_REG,
+ &shlw_hi);
+ if (ret < 0)
+ goto out;
+
+ ret = intel_pmc_gcr_read(PMC_GCR_TELEM_DEEP_S0IX_LO_REG,
+ &deep_lo);
+ if (ret < 0)
+ goto out;
+
+ ret = intel_pmc_gcr_read(PMC_GCR_TELEM_DEEP_S0IX_HI_REG,
+ &deep_hi);
+ if (ret < 0)
+ goto out;
+
+ suspend_shlw_res_exit = TELEM_LO_HI_TO64(shlw_lo, shlw_hi);
+ if (suspend_shlw_res_exit > suspend_shlw_res_temp)
+ suspend_shlw_ctr_exit++;
+
+ suspend_deep_res_exit = TELEM_LO_HI_TO64(deep_lo, deep_hi);
+ if (suspend_deep_res_exit > suspend_deep_res_temp)
+ suspend_deep_ctr_exit++;
+ }
+
suspend_shlw_ctr_exit -= suspend_shlw_ctr_temp;
suspend_deep_ctr_exit -= suspend_deep_ctr_temp;
suspend_shlw_res_exit -= suspend_shlw_res_temp;
--
2.7.4


From 1583053417100024577@xxx Fri Nov 03 13:56:25 +0000 2017
X-GM-THRID: 1583053417100024577
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread