[PATCH 1/3] platform/x86: Add GLK PSS Event Table

[PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Rajneesh Bhardwaj]

Some of the Primary Subsystem events differ on Gemini Lake but the IOSS
events remain same. This patch adds the updated PSS event table to enable
Telemetry driver on Gemini Lake.

Signed-off-by: Shanth Murthy <shanth.murthy@intel.com>
Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
---
 drivers/platform/x86/intel_telemetry_debugfs.c |  1 +
 drivers/platform/x86/intel_telemetry_pltdrv.c  | 35 ++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c b/drivers/platform/x86/intel_telemetry_debugfs.c
index 4cc2f4ea0a25..a0e4344b2eec 100644
--- a/drivers/platform/x86/intel_telemetry_debugfs.c
+++ b/drivers/platform/x86/intel_telemetry_debugfs.c
@@ -331,6 +331,7 @@ static struct telemetry_debugfs_conf telem_apl_debugfs_conf = {
 
 static const struct x86_cpu_id telemetry_debugfs_cpu_ids[] = {
 	TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GOLDMONT, telem_apl_debugfs_conf),
+	TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE, telem_apl_debugfs_conf),
 	{}
 };
 
diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c b/drivers/platform/x86/intel_telemetry_pltdrv.c
index 6ebdbd2b04fc..6393b3b1d5a6 100644
--- a/drivers/platform/x86/intel_telemetry_pltdrv.c
+++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
@@ -153,6 +153,30 @@ static struct telemetry_evtmap
 	{"PC2_AND_MEM_SHALLOW_IDLE_RES",	0x1D40},
 };
 
+static struct telemetry_evtmap
+	telemetry_glk_pss_default_events[TELEM_MAX_OS_ALLOCATED_EVENTS] = {
+	{"IA_CORE0_C6_RES",			0x0400},
+	{"IA_CORE0_C6_CTR",			0x0000},
+	{"IA_MODULE0_C7_RES",			0x0410},
+	{"IA_MODULE0_C7_CTR",			0x000C},
+	{"IA_C0_RES",				0x0805},
+	{"PCS_LTR",				0x2801},
+	{"PSTATES",				0x2802},
+	{"SOC_S0I3_RES",			0x0407},
+	{"SOC_S0I3_CTR",			0x0008},
+	{"PCS_S0I3_CTR",			0x0007},
+	{"PCS_C1E_RES",				0x0414},
+	{"PCS_IDLE_STATUS",			0x2806},
+	{"IA_PERF_LIMITS",			0x280B},
+	{"GT_PERF_LIMITS",			0x280C},
+	{"PCS_WAKEUP_S0IX_CTR",			0x0025},
+	{"PCS_IDLE_BLOCKED",			0x2C00},
+	{"PCS_S0IX_BLOCKED",			0x2C01},
+	{"PCS_S0IX_WAKE_REASONS",		0x2C02},
+	{"PCS_LTR_BLOCKING",			0x2C03},
+	{"PC2_AND_MEM_SHALLOW_IDLE_RES",	0x1D40},
+};
+
 /* APL specific Data */
 static struct telemetry_plt_config telem_apl_config = {
 	.pss_config = {
@@ -163,8 +187,19 @@ static struct telemetry_plt_config telem_apl_config = {
 	},
 };
 
+/* GLK specific Data */
+static struct telemetry_plt_config telem_glk_config = {
+	.pss_config = {
+		.telem_evts = telemetry_glk_pss_default_events,
+	},
+	.ioss_config = {
+		.telem_evts = telemetry_apl_ioss_default_events,
+	},
+};
+
 static const struct x86_cpu_id telemetry_cpu_ids[] = {
 	TELEM_CPU(INTEL_FAM6_ATOM_GOLDMONT, telem_apl_config),
+	TELEM_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE, telem_glk_config),
 	{}
 };
 
-- 
2.7.4

[PATCH 2/3] Telemetry: remove redundant macro definition

[Rajneesh Bhardwaj]

Telemetry driver includes intel_telemetry.h which defines
TELEM_MAX_OS_ALLOCATED_EVENTS already.

Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
---
 drivers/platform/x86/intel_telemetry_pltdrv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c b/drivers/platform/x86/intel_telemetry_pltdrv.c
index 6393b3b1d5a6..e0424d5a795a 100644
--- a/drivers/platform/x86/intel_telemetry_pltdrv.c
+++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
@@ -46,7 +46,6 @@
 #define TELEM_SAMPLING_DEFAULT_PERIOD	0xD
 
 #define TELEM_MAX_EVENTS_SRAM		28
-#define TELEM_MAX_OS_ALLOCATED_EVENTS	20
 #define TELEM_SSRAM_STARTTIME_OFFSET	8
 #define TELEM_SSRAM_EVTLOG_OFFSET	16
 
-- 
2.7.4

[PATCH 3/3] platform/x86: Add Audio domain PG status events

[Rajneesh Bhardwaj]

From: "Murthy, Shanth" <shanth.murthy@intel.com>

This patch adds events to ioss telemetry to read the power gating status
for the audio domain.

Signed-off-by: Shanth Murthy <shanth.murthy@intel.com>
Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
---
 arch/x86/include/asm/intel_telemetry.h        | 2 +-
 drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/intel_telemetry.h b/arch/x86/include/asm/intel_telemetry.h
index 85029b58d0cd..4eeae0a4f9a2 100644
--- a/arch/x86/include/asm/intel_telemetry.h
+++ b/arch/x86/include/asm/intel_telemetry.h
@@ -17,7 +17,7 @@
 #define INTEL_TELEMETRY_H
 
 #define TELEM_MAX_EVENTS_SRAM		28
-#define TELEM_MAX_OS_ALLOCATED_EVENTS	20
+#define TELEM_MAX_OS_ALLOCATED_EVENTS	25
 
 enum telemetry_unit {
 	TELEM_PSS = 0,
diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c b/drivers/platform/x86/intel_telemetry_pltdrv.c
index e0424d5a795a..9ec437442deb 100644
--- a/drivers/platform/x86/intel_telemetry_pltdrv.c
+++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
@@ -125,6 +125,12 @@ static struct telemetry_evtmap
 	{"PMC_S0IX_BLOCKING_MISC_IPS_PG",       0x6008},
 	{"PMC_S0IX_BLOCK_IPS_VNN_REQ",          0x6009},
 	{"PMC_S0IX_BLOCK_IPS_CLOCKS",           0x600B},
+	{"AVSPGD1_IP_POWER_GATED_FW_COUNTER",	0x388D},
+	{"AVSPGD2_IP_POWER_GATED_FW_COUNTER",	0x388C},
+	{"AVSPGD3_IP_POWER_GATED_FW_COUNTER",	0x388B},
+	{"AVSPGD4_IP_POWER_GATED_FW_COUNTER",	0x388A},
+	{"AUDIO_VNN_REQ_MISC_COUNTER",		0x182B},
+
 };
 
 
-- 
2.7.4

RE: [PATCH 3/3] platform/x86: Add Audio domain PG status events

[Chakravarty, Souvik K]

> -----Original Message-----
> From: Bhardwaj, Rajneesh
> Sent: Friday, July 14, 2017 5:24 PM
> To: platform-driver-x86@vger.kernel.org
> Cc: dvhart@infradead.org; andy@infradead.org; linux-
> kernel@vger.kernel.org; Murthy, Shanth <shanth.murthy@intel.com>;
> Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>; Bhardwaj,
> Rajneesh <rajneesh.bhardwaj@intel.com>
> Subject: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> 
> From: "Murthy, Shanth" <shanth.murthy@intel.com>
> 
> This patch adds events to ioss telemetry to read the power gating status for
> the audio domain.
> 
> Signed-off-by: Shanth Murthy <shanth.murthy@intel.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
> ---
>  arch/x86/include/asm/intel_telemetry.h        | 2 +-
>  drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/intel_telemetry.h
> b/arch/x86/include/asm/intel_telemetry.h
> index 85029b58d0cd..4eeae0a4f9a2 100644
> --- a/arch/x86/include/asm/intel_telemetry.h
> +++ b/arch/x86/include/asm/intel_telemetry.h
> @@ -17,7 +17,7 @@
>  #define INTEL_TELEMETRY_H
> 
>  #define TELEM_MAX_EVENTS_SRAM		28
> -#define TELEM_MAX_OS_ALLOCATED_EVENTS	20
> +#define TELEM_MAX_OS_ALLOCATED_EVENTS	25

This is something that should not be done without testing. There was an understanding to not use more than 20 counters in OS, so that tools like SoCWatch could use the rest 8 without total reconfiguration of the events. With this change we are not leaving much for tools and it may break SoCwatch/IMON or other tools dependent on using just 8. 
The better way for this would be for the userspace tool to configure the extra 5 events via add_config() call and not increase the OS usage ceiling.

Or you could add some more sysfs apis for get/set events and do it via a daemon/script.

<snip>....

RE: [PATCH 3/3] platform/x86: Add Audio domain PG status events

[Chakravarty, Souvik K]

> -----Original Message-----
> From: Bhardwaj, Rajneesh
> Sent: Friday, July 14, 2017 5:24 PM
> To: platform-driver-x86@vger.kernel.org
> Cc: dvhart@infradead.org; andy@infradead.org; linux-
> kernel@vger.kernel.org; Murthy, Shanth <shanth.murthy@intel.com>;
> Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>; Bhardwaj,
> Rajneesh <rajneesh.bhardwaj@intel.com>
> Subject: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> 
> From: "Murthy, Shanth" <shanth.murthy@intel.com>
> 
> This patch adds events to ioss telemetry to read the power gating status for
> the audio domain.
> 
> Signed-off-by: Shanth Murthy <shanth.murthy@intel.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
> ---
>  arch/x86/include/asm/intel_telemetry.h        | 2 +-
>  drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/intel_telemetry.h
> b/arch/x86/include/asm/intel_telemetry.h
> index 85029b58d0cd..4eeae0a4f9a2 100644
> --- a/arch/x86/include/asm/intel_telemetry.h
> +++ b/arch/x86/include/asm/intel_telemetry.h
> @@ -17,7 +17,7 @@
>  #define INTEL_TELEMETRY_H
> 
>  #define TELEM_MAX_EVENTS_SRAM		28
> -#define TELEM_MAX_OS_ALLOCATED_EVENTS	20
> +#define TELEM_MAX_OS_ALLOCATED_EVENTS	25

This is something that should not be done without testing. There was an understanding to not use more than 20 counters in OS, so that tools like SoCWatch could use the rest 8 without total reconfiguration of the events. With this change we are not leaving much for tools and it may break SoCwatch/IMON or other tools dependent on using just 8. 
The better way for this would be for the userspace tool to configure the extra 5 events via add_config() call and not increase the OS usage ceiling.

Or you could add some more sysfs apis for get/set events and do it via a daemon/script.

<snip>....

RE: [PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Chakravarty, Souvik K]

+1 From me.

> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-
> x86-owner@vger.kernel.org] On Behalf Of Rajneesh Bhardwaj
> Sent: Friday, July 14, 2017 5:24 PM
> To: platform-driver-x86@vger.kernel.org
> Cc: dvhart@infradead.org; andy@infradead.org; linux-
> kernel@vger.kernel.org; Murthy, Shanth <shanth.murthy@intel.com>;
> Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>; Bhardwaj,
> Rajneesh <rajneesh.bhardwaj@intel.com>
> Subject: [PATCH 1/3] platform/x86: Add GLK PSS Event Table
> 
> Some of the Primary Subsystem events differ on Gemini Lake but the IOSS
> events remain same. This patch adds the updated PSS event table to enable
> Telemetry driver on Gemini Lake.
> 
> Signed-off-by: Shanth Murthy <shanth.murthy@intel.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
> ---
>  drivers/platform/x86/intel_telemetry_debugfs.c |  1 +
> drivers/platform/x86/intel_telemetry_pltdrv.c  | 35
> ++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index 4cc2f4ea0a25..a0e4344b2eec 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -331,6 +331,7 @@ static struct telemetry_debugfs_conf
> telem_apl_debugfs_conf = {
> 
>  static const struct x86_cpu_id telemetry_debugfs_cpu_ids[] = {
>  	TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GOLDMONT,
> telem_apl_debugfs_conf),
> +	TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE,
> +telem_apl_debugfs_conf),
>  	{}
>  };
> 
> diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> b/drivers/platform/x86/intel_telemetry_pltdrv.c
> index 6ebdbd2b04fc..6393b3b1d5a6 100644
> --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> @@ -153,6 +153,30 @@ static struct telemetry_evtmap
>  	{"PC2_AND_MEM_SHALLOW_IDLE_RES",	0x1D40},
>  };
> 
> +static struct telemetry_evtmap
> +
> 	telemetry_glk_pss_default_events[TELEM_MAX_OS_ALLOCATED_E
> VENTS] = {
> +	{"IA_CORE0_C6_RES",			0x0400},
> +	{"IA_CORE0_C6_CTR",			0x0000},
> +	{"IA_MODULE0_C7_RES",			0x0410},
> +	{"IA_MODULE0_C7_CTR",			0x000C},
> +	{"IA_C0_RES",				0x0805},
> +	{"PCS_LTR",				0x2801},
> +	{"PSTATES",				0x2802},
> +	{"SOC_S0I3_RES",			0x0407},
> +	{"SOC_S0I3_CTR",			0x0008},
> +	{"PCS_S0I3_CTR",			0x0007},
> +	{"PCS_C1E_RES",				0x0414},
> +	{"PCS_IDLE_STATUS",			0x2806},
> +	{"IA_PERF_LIMITS",			0x280B},
> +	{"GT_PERF_LIMITS",			0x280C},
> +	{"PCS_WAKEUP_S0IX_CTR",			0x0025},
> +	{"PCS_IDLE_BLOCKED",			0x2C00},
> +	{"PCS_S0IX_BLOCKED",			0x2C01},
> +	{"PCS_S0IX_WAKE_REASONS",		0x2C02},
> +	{"PCS_LTR_BLOCKING",			0x2C03},
> +	{"PC2_AND_MEM_SHALLOW_IDLE_RES",	0x1D40},
> +};
> +
>  /* APL specific Data */
>  static struct telemetry_plt_config telem_apl_config = {
>  	.pss_config = {
> @@ -163,8 +187,19 @@ static struct telemetry_plt_config
> telem_apl_config = {
>  	},
>  };
> 
> +/* GLK specific Data */
> +static struct telemetry_plt_config telem_glk_config = {
> +	.pss_config = {
> +		.telem_evts = telemetry_glk_pss_default_events,
> +	},
> +	.ioss_config = {
> +		.telem_evts = telemetry_apl_ioss_default_events,
> +	},
> +};
> +
>  static const struct x86_cpu_id telemetry_cpu_ids[] = {
>  	TELEM_CPU(INTEL_FAM6_ATOM_GOLDMONT, telem_apl_config),
> +	TELEM_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE, telem_glk_config),
>  	{}
>  };
> 
> --
> 2.7.4

RE: [PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Chakravarty, Souvik K]

+1 From me.

> -----Original Message-----
> From: platform-driver-x86-owner@vger.kernel.org [mailto:platform-driver-
> x86-owner@vger.kernel.org] On Behalf Of Rajneesh Bhardwaj
> Sent: Friday, July 14, 2017 5:24 PM
> To: platform-driver-x86@vger.kernel.org
> Cc: dvhart@infradead.org; andy@infradead.org; linux-
> kernel@vger.kernel.org; Murthy, Shanth <shanth.murthy@intel.com>;
> Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>; Bhardwaj,
> Rajneesh <rajneesh.bhardwaj@intel.com>
> Subject: [PATCH 1/3] platform/x86: Add GLK PSS Event Table
> 
> Some of the Primary Subsystem events differ on Gemini Lake but the IOSS
> events remain same. This patch adds the updated PSS event table to enable
> Telemetry driver on Gemini Lake.
> 
> Signed-off-by: Shanth Murthy <shanth.murthy@intel.com>
> Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
> ---
>  drivers/platform/x86/intel_telemetry_debugfs.c |  1 +
> drivers/platform/x86/intel_telemetry_pltdrv.c  | 35
> ++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel_telemetry_debugfs.c
> b/drivers/platform/x86/intel_telemetry_debugfs.c
> index 4cc2f4ea0a25..a0e4344b2eec 100644
> --- a/drivers/platform/x86/intel_telemetry_debugfs.c
> +++ b/drivers/platform/x86/intel_telemetry_debugfs.c
> @@ -331,6 +331,7 @@ static struct telemetry_debugfs_conf
> telem_apl_debugfs_conf = {
> 
>  static const struct x86_cpu_id telemetry_debugfs_cpu_ids[] = {
>  	TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GOLDMONT,
> telem_apl_debugfs_conf),
> +	TELEM_DEBUGFS_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE,
> +telem_apl_debugfs_conf),
>  	{}
>  };
> 
> diff --git a/drivers/platform/x86/intel_telemetry_pltdrv.c
> b/drivers/platform/x86/intel_telemetry_pltdrv.c
> index 6ebdbd2b04fc..6393b3b1d5a6 100644
> --- a/drivers/platform/x86/intel_telemetry_pltdrv.c
> +++ b/drivers/platform/x86/intel_telemetry_pltdrv.c
> @@ -153,6 +153,30 @@ static struct telemetry_evtmap
>  	{"PC2_AND_MEM_SHALLOW_IDLE_RES",	0x1D40},
>  };
> 
> +static struct telemetry_evtmap
> +
> 	telemetry_glk_pss_default_events[TELEM_MAX_OS_ALLOCATED_E
> VENTS] = {
> +	{"IA_CORE0_C6_RES",			0x0400},
> +	{"IA_CORE0_C6_CTR",			0x0000},
> +	{"IA_MODULE0_C7_RES",			0x0410},
> +	{"IA_MODULE0_C7_CTR",			0x000C},
> +	{"IA_C0_RES",				0x0805},
> +	{"PCS_LTR",				0x2801},
> +	{"PSTATES",				0x2802},
> +	{"SOC_S0I3_RES",			0x0407},
> +	{"SOC_S0I3_CTR",			0x0008},
> +	{"PCS_S0I3_CTR",			0x0007},
> +	{"PCS_C1E_RES",				0x0414},
> +	{"PCS_IDLE_STATUS",			0x2806},
> +	{"IA_PERF_LIMITS",			0x280B},
> +	{"GT_PERF_LIMITS",			0x280C},
> +	{"PCS_WAKEUP_S0IX_CTR",			0x0025},
> +	{"PCS_IDLE_BLOCKED",			0x2C00},
> +	{"PCS_S0IX_BLOCKED",			0x2C01},
> +	{"PCS_S0IX_WAKE_REASONS",		0x2C02},
> +	{"PCS_LTR_BLOCKING",			0x2C03},
> +	{"PC2_AND_MEM_SHALLOW_IDLE_RES",	0x1D40},
> +};
> +
>  /* APL specific Data */
>  static struct telemetry_plt_config telem_apl_config = {
>  	.pss_config = {
> @@ -163,8 +187,19 @@ static struct telemetry_plt_config
> telem_apl_config = {
>  	},
>  };
> 
> +/* GLK specific Data */
> +static struct telemetry_plt_config telem_glk_config = {
> +	.pss_config = {
> +		.telem_evts = telemetry_glk_pss_default_events,
> +	},
> +	.ioss_config = {
> +		.telem_evts = telemetry_apl_ioss_default_events,
> +	},
> +};
> +
>  static const struct x86_cpu_id telemetry_cpu_ids[] = {
>  	TELEM_CPU(INTEL_FAM6_ATOM_GOLDMONT, telem_apl_config),
> +	TELEM_CPU(INTEL_FAM6_ATOM_GEMINI_LAKE, telem_glk_config),
>  	{}
>  };
> 
> --
> 2.7.4

Re: [PATCH 3/3] platform/x86: Add Audio domain PG status events

[Rajneesh Bhardwaj]

On Mon, Jul 17, 2017 at 10:48:30AM +0530, Chakravarty, Souvik K wrote:
> 
> 
> > -----Original Message-----
> > From: Bhardwaj, Rajneesh
> > Sent: Friday, July 14, 2017 5:24 PM
> > To: platform-driver-x86@vger.kernel.org
> > Cc: dvhart@infradead.org; andy@infradead.org; linux-
> > kernel@vger.kernel.org; Murthy, Shanth <shanth.murthy@intel.com>;
> > Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>; Bhardwaj,
> > Rajneesh <rajneesh.bhardwaj@intel.com>
> > Subject: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> > 
> > From: "Murthy, Shanth" <shanth.murthy@intel.com>
> > 
> > This patch adds events to ioss telemetry to read the power gating status for
> > the audio domain.
> > 
> > Signed-off-by: Shanth Murthy <shanth.murthy@intel.com>
> > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
> > ---
> >  arch/x86/include/asm/intel_telemetry.h        | 2 +-
> >  drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++++++
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/intel_telemetry.h
> > b/arch/x86/include/asm/intel_telemetry.h
> > index 85029b58d0cd..4eeae0a4f9a2 100644
> > --- a/arch/x86/include/asm/intel_telemetry.h
> > +++ b/arch/x86/include/asm/intel_telemetry.h
> > @@ -17,7 +17,7 @@
> >  #define INTEL_TELEMETRY_H
> > 
> >  #define TELEM_MAX_EVENTS_SRAM		28
> > -#define TELEM_MAX_OS_ALLOCATED_EVENTS	20
> > +#define TELEM_MAX_OS_ALLOCATED_EVENTS	25
> 
> This is something that should not be done without testing. There was an understanding to not use more than 20 counters in OS, so that tools like SoCWatch could use the rest 8 without total reconfiguration of the events. With this change we are not leaving much for tools and it may break SoCwatch/IMON or other tools dependent on using just 8. 
> The better way for this would be for the userspace tool to configure the extra 5 events via add_config() call and not increase the OS usage ceiling.
> 
> Or you could add some more sysfs apis for get/set events and do it via a daemon/script.
> 
> <snip>....

Thanks for the review and the feedback Souvik. Ideally user space tools
should not put a restriction such as this one. It would be good to document
such details.

Is there a plan to support more than 20 events in telemetry driver? If not
then we can probably think of removing few events and add audio related
events maintaining total count of 20?

For now i will respin the patches and drop this one. 

-- 
Best Regards,
Rajneesh

Re: [PATCH 3/3] platform/x86: Add Audio domain PG status events

[Rajneesh Bhardwaj]

On Mon, Jul 17, 2017 at 10:48:30AM +0530, Chakravarty, Souvik K wrote:
> 
> 
> > -----Original Message-----
> > From: Bhardwaj, Rajneesh
> > Sent: Friday, July 14, 2017 5:24 PM
> > To: platform-driver-x86@vger.kernel.org
> > Cc: dvhart@infradead.org; andy@infradead.org; linux-
> > kernel@vger.kernel.org; Murthy, Shanth <shanth.murthy@intel.com>;
> > Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>; Bhardwaj,
> > Rajneesh <rajneesh.bhardwaj@intel.com>
> > Subject: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> > 
> > From: "Murthy, Shanth" <shanth.murthy@intel.com>
> > 
> > This patch adds events to ioss telemetry to read the power gating status for
> > the audio domain.
> > 
> > Signed-off-by: Shanth Murthy <shanth.murthy@intel.com>
> > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
> > ---
> >  arch/x86/include/asm/intel_telemetry.h        | 2 +-
> >  drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++++++
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/intel_telemetry.h
> > b/arch/x86/include/asm/intel_telemetry.h
> > index 85029b58d0cd..4eeae0a4f9a2 100644
> > --- a/arch/x86/include/asm/intel_telemetry.h
> > +++ b/arch/x86/include/asm/intel_telemetry.h
> > @@ -17,7 +17,7 @@
> >  #define INTEL_TELEMETRY_H
> > 
> >  #define TELEM_MAX_EVENTS_SRAM		28
> > -#define TELEM_MAX_OS_ALLOCATED_EVENTS	20
> > +#define TELEM_MAX_OS_ALLOCATED_EVENTS	25
> 
> This is something that should not be done without testing. There was an understanding to not use more than 20 counters in OS, so that tools like SoCWatch could use the rest 8 without total reconfiguration of the events. With this change we are not leaving much for tools and it may break SoCwatch/IMON or other tools dependent on using just 8. 
> The better way for this would be for the userspace tool to configure the extra 5 events via add_config() call and not increase the OS usage ceiling.
> 
> Or you could add some more sysfs apis for get/set events and do it via a daemon/script.
> 
> <snip>....

Thanks for the review and the feedback Souvik. Ideally user space tools
should not put a restriction such as this one. It would be good to document
such details.

Is there a plan to support more than 20 events in telemetry driver? If not
then we can probably think of removing few events and add audio related
events maintaining total count of 20?

For now i will respin the patches and drop this one. 

-- 
Best Regards,
Rajneesh

RE: [PATCH 3/3] platform/x86: Add Audio domain PG status events

[Chakravarty, Souvik K]

> -----Original Message-----
> From: Bhardwaj, Rajneesh
> Sent: Wednesday, July 19, 2017 7:08 PM
> To: Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>
> Cc: platform-driver-x86@vger.kernel.org; dvhart@infradead.org;
> andy@infradead.org; linux-kernel@vger.kernel.org; Murthy, Shanth
> <shanth.murthy@intel.com>
> Subject: Re: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> 
> On Mon, Jul 17, 2017 at 10:48:30AM +0530, Chakravarty, Souvik K wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bhardwaj, Rajneesh
> > > Sent: Friday, July 14, 2017 5:24 PM
> > > To: platform-driver-x86@vger.kernel.org
> > > Cc: dvhart@infradead.org; andy@infradead.org; linux-
> > > kernel@vger.kernel.org; Murthy, Shanth <shanth.murthy@intel.com>;
> > > Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>; Bhardwaj,
> > > Rajneesh <rajneesh.bhardwaj@intel.com>
> > > Subject: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> > >
> > > From: "Murthy, Shanth" <shanth.murthy@intel.com>
> > >
> > > This patch adds events to ioss telemetry to read the power gating
> > > status for the audio domain.
> > >
> > > Signed-off-by: Shanth Murthy <shanth.murthy@intel.com>
> > > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
> > > ---
> > >  arch/x86/include/asm/intel_telemetry.h        | 2 +-
> > >  drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++++++
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/include/asm/intel_telemetry.h
> > > b/arch/x86/include/asm/intel_telemetry.h
> > > index 85029b58d0cd..4eeae0a4f9a2 100644
> > > --- a/arch/x86/include/asm/intel_telemetry.h
> > > +++ b/arch/x86/include/asm/intel_telemetry.h
> > > @@ -17,7 +17,7 @@
> > >  #define INTEL_TELEMETRY_H
> > >
> > >  #define TELEM_MAX_EVENTS_SRAM		28
> > > -#define TELEM_MAX_OS_ALLOCATED_EVENTS	20
> > > +#define TELEM_MAX_OS_ALLOCATED_EVENTS	25
> >
> > This is something that should not be done without testing. There was an
> understanding to not use more than 20 counters in OS, so that tools like
> SoCWatch could use the rest 8 without total reconfiguration of the events.
> With this change we are not leaving much for tools and it may break
> SoCwatch/IMON or other tools dependent on using just 8.
> > The better way for this would be for the userspace tool to configure the
> extra 5 events via add_config() call and not increase the OS usage ceiling.
> >
> > Or you could add some more sysfs apis for get/set events and do it via a
> daemon/script.
> >
> > <snip>....
> 
> Thanks for the review and the feedback Souvik. Ideally user space tools
> should not put a restriction such as this one. It would be good to document
> such details.
Theoretically yes. But many of the tools have no maintaince budget/owner. 

> 
> Is there a plan to support more than 20 events in telemetry driver? If not
> then we can probably think of removing few events and add audio related
> events maintaining total count of 20?

Sure...why not.

> 
> For now i will respin the patches and drop this one.

Thanks. 
I will also add support for an 'add' sysfs entry so that you can configure it easily and not have to fiddle with the default set.

> 
> --
> Best Regards,
> Rajneesh

RE: [PATCH 3/3] platform/x86: Add Audio domain PG status events

[Chakravarty, Souvik K]

> -----Original Message-----
> From: Bhardwaj, Rajneesh
> Sent: Wednesday, July 19, 2017 7:08 PM
> To: Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>
> Cc: platform-driver-x86@vger.kernel.org; dvhart@infradead.org;
> andy@infradead.org; linux-kernel@vger.kernel.org; Murthy, Shanth
> <shanth.murthy@intel.com>
> Subject: Re: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> 
> On Mon, Jul 17, 2017 at 10:48:30AM +0530, Chakravarty, Souvik K wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bhardwaj, Rajneesh
> > > Sent: Friday, July 14, 2017 5:24 PM
> > > To: platform-driver-x86@vger.kernel.org
> > > Cc: dvhart@infradead.org; andy@infradead.org; linux-
> > > kernel@vger.kernel.org; Murthy, Shanth <shanth.murthy@intel.com>;
> > > Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>; Bhardwaj,
> > > Rajneesh <rajneesh.bhardwaj@intel.com>
> > > Subject: [PATCH 3/3] platform/x86: Add Audio domain PG status events
> > >
> > > From: "Murthy, Shanth" <shanth.murthy@intel.com>
> > >
> > > This patch adds events to ioss telemetry to read the power gating
> > > status for the audio domain.
> > >
> > > Signed-off-by: Shanth Murthy <shanth.murthy@intel.com>
> > > Signed-off-by: Rajneesh Bhardwaj <rajneesh.bhardwaj@intel.com>
> > > ---
> > >  arch/x86/include/asm/intel_telemetry.h        | 2 +-
> > >  drivers/platform/x86/intel_telemetry_pltdrv.c | 6 ++++++
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/include/asm/intel_telemetry.h
> > > b/arch/x86/include/asm/intel_telemetry.h
> > > index 85029b58d0cd..4eeae0a4f9a2 100644
> > > --- a/arch/x86/include/asm/intel_telemetry.h
> > > +++ b/arch/x86/include/asm/intel_telemetry.h
> > > @@ -17,7 +17,7 @@
> > >  #define INTEL_TELEMETRY_H
> > >
> > >  #define TELEM_MAX_EVENTS_SRAM		28
> > > -#define TELEM_MAX_OS_ALLOCATED_EVENTS	20
> > > +#define TELEM_MAX_OS_ALLOCATED_EVENTS	25
> >
> > This is something that should not be done without testing. There was an
> understanding to not use more than 20 counters in OS, so that tools like
> SoCWatch could use the rest 8 without total reconfiguration of the events.
> With this change we are not leaving much for tools and it may break
> SoCwatch/IMON or other tools dependent on using just 8.
> > The better way for this would be for the userspace tool to configure the
> extra 5 events via add_config() call and not increase the OS usage ceiling.
> >
> > Or you could add some more sysfs apis for get/set events and do it via a
> daemon/script.
> >
> > <snip>....
> 
> Thanks for the review and the feedback Souvik. Ideally user space tools
> should not put a restriction such as this one. It would be good to document
> such details.
Theoretically yes. But many of the tools have no maintaince budget/owner. 

> 
> Is there a plan to support more than 20 events in telemetry driver? If not
> then we can probably think of removing few events and add audio related
> events maintaining total count of 20?

Sure...why not.

> 
> For now i will respin the patches and drop this one.

Thanks. 
I will also add support for an 'add' sysfs entry so that you can configure it easily and not have to fiddle with the default set.

> 
> --
> Best Regards,
> Rajneesh

Re: [PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Darren Hart]

On Mon, Jul 17, 2017 at 05:18:54AM +0000, Chakravarty, Souvik K wrote:
> +1 From me.

Souvik, as the listed maintainer for this driver, what I require from
you is a complete Acked-by or a Reviewed-by line, preferably the latter,
and preferably after an actual review with comments. +1 has no semantic
meaning in the Linux kernel development process, and one line approvals,
especially from the same company, have little incremental value.

For details, please see:
Documentation/process/5.Posting.rst
Documentation/submitting-patches.rst

Thanks,

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Darren Hart]

On Mon, Jul 17, 2017 at 05:18:54AM +0000, Chakravarty, Souvik K wrote:
> +1 From me.

Souvik, as the listed maintainer for this driver, what I require from
you is a complete Acked-by or a Reviewed-by line, preferably the latter,
and preferably after an actual review with comments. +1 has no semantic
meaning in the Linux kernel development process, and one line approvals,
especially from the same company, have little incremental value.

For details, please see:
Documentation/process/5.Posting.rst
Documentation/submitting-patches.rst

Thanks,

-- 
Darren Hart
VMware Open Source Technology Center

RE: [PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Chakravarty, Souvik K]

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Friday, July 21, 2017 2:36 AM
> To: Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>
> Cc: Bhardwaj, Rajneesh <rajneesh.bhardwaj@intel.com>; platform-driver-
> x86@vger.kernel.org; andy@infradead.org; linux-kernel@vger.kernel.org;
> Murthy, Shanth <shanth.murthy@intel.com>
> Subject: Re: [PATCH 1/3] platform/x86: Add GLK PSS Event Table
> 
> On Mon, Jul 17, 2017 at 05:18:54AM +0000, Chakravarty, Souvik K wrote:
> > +1 From me.
> 
> Souvik, as the listed maintainer for this driver, what I require from you is a
> complete Acked-by or a Reviewed-by line, preferably the latter, and
> preferably after an actual review with comments. +1 has no semantic
> meaning in the Linux kernel development process, and one line approvals,
> especially from the same company, have little incremental value.
> 
> For details, please see:
> Documentation/process/5.Posting.rst
> Documentation/submitting-patches.rst

Thanks Darren...my mistake :(
Will do.

> 
> Thanks,
> 
> --
> Darren Hart
> VMware Open Source Technology Center

RE: [PATCH 1/3] platform/x86: Add GLK PSS Event Table

[Chakravarty, Souvik K]

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Friday, July 21, 2017 2:36 AM
> To: Chakravarty, Souvik K <souvik.k.chakravarty@intel.com>
> Cc: Bhardwaj, Rajneesh <rajneesh.bhardwaj@intel.com>; platform-driver-
> x86@vger.kernel.org; andy@infradead.org; linux-kernel@vger.kernel.org;
> Murthy, Shanth <shanth.murthy@intel.com>
> Subject: Re: [PATCH 1/3] platform/x86: Add GLK PSS Event Table
> 
> On Mon, Jul 17, 2017 at 05:18:54AM +0000, Chakravarty, Souvik K wrote:
> > +1 From me.
> 
> Souvik, as the listed maintainer for this driver, what I require from you is a
> complete Acked-by or a Reviewed-by line, preferably the latter, and
> preferably after an actual review with comments. +1 has no semantic
> meaning in the Linux kernel development process, and one line approvals,
> especially from the same company, have little incremental value.
> 
> For details, please see:
> Documentation/process/5.Posting.rst
> Documentation/submitting-patches.rst

Thanks Darren...my mistake :(
Will do.

> 
> Thanks,
> 
> --
> Darren Hart
> VMware Open Source Technology Center