Re: [PATCH v2 3/7] platform/x86/intel/pmc: Enable Pkgc blocking residency counter

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 6210 bytes --]

On Wed, 8 Apr 2026, Xi Pardee wrote:

> Enable the Package C-state blocking counter in the PMT telemetry
> region. This counter reports the number of 10 µs intervals during
> which a Package C-state 10.2/3 entry was blocked for the specified
> reasons.
> 
> Create a common helper for pmc_core_pkgc_ltr_blocker_show() and
> pmc_core_pkgc_blocker_residency_show() as these two functions
> share similar logic.

Please don't do back and forth changes like this within a series. You 
should add it in the right form from the beginning.

-- 
 i.

> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> ---
>  drivers/platform/x86/intel/pmc/core.c | 40 ++++++++++++++++++++-------
>  drivers/platform/x86/intel/pmc/core.h |  8 ++++++
>  2 files changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index 5c519942ec58c..94ae098a155a6 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -1071,29 +1071,44 @@ static int pmc_core_die_c6_us_show(struct seq_file *s, void *unused)
>  }
>  DEFINE_SHOW_ATTRIBUTE(pmc_core_die_c6_us);
>  
> -static int pmc_core_pkgc_ltr_blocker_show(struct seq_file *s, void *unused)
> +static int pmc_core_pkgc_counters_show(struct seq_file *s,
> +				       struct telem_endpoint *ep,
> +				       u32 offset, const char **counters)
>  {
> -	struct pmc_dev *pmcdev = s->private;
> -	const char **pkgc_ltr_blocker_counters;
>  	unsigned int i;
>  	u32 counter;
>  	int ret;
>  
> -	pkgc_ltr_blocker_counters = pmcdev->pkgc_ltr_blocker_counters;
> -	for (i = 0; pkgc_ltr_blocker_counters[i]; i++) {
> -		ret = pmt_telem_read32(pmcdev->pc_ep,
> -				       pmcdev->pkgc_ltr_blocker_offset + i,
> -				       &counter, 1);
> -
> +	for (i = 0; counters[i]; i++) {
> +		ret = pmt_telem_read32(ep, offset + i, &counter, 1);
>  		if (ret)
>  			return ret;
> -		seq_printf(s, "%-30s %-30u\n", pkgc_ltr_blocker_counters[i], counter);
> +		seq_printf(s, "%-30s %-30u\n", counters[i], counter);
>  	}
>  
>  	return 0;
>  }
> +
> +static int pmc_core_pkgc_ltr_blocker_show(struct seq_file *s, void *unused)
> +{
> +	struct pmc_dev *pmcdev = s->private;
> +
> +	return pmc_core_pkgc_counters_show(s, pmcdev->pc_ep,
> +					   pmcdev->pkgc_ltr_blocker_offset,
> +					   pmcdev->pkgc_ltr_blocker_counters);
> +}
>  DEFINE_SHOW_ATTRIBUTE(pmc_core_pkgc_ltr_blocker);
>  
> +static int pmc_core_pkgc_blocker_residency_show(struct seq_file *s, void *unused)
> +{
> +	struct pmc_dev *pmcdev = s->private;
> +
> +	return pmc_core_pkgc_counters_show(s, pmcdev->pc_ep,
> +					   pmcdev->pkgc_blocker_offset,
> +					   pmcdev->pkgc_blocker_counters);
> +}
> +DEFINE_SHOW_ATTRIBUTE(pmc_core_pkgc_blocker_residency);
> +
>  static int pmc_core_lpm_latch_mode_show(struct seq_file *s, void *unused)
>  {
>  	struct pmc_dev *pmcdev = s->private;
> @@ -1381,6 +1396,8 @@ void pmc_core_punit_pmt_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_de
>  		pmcdev->pc_ep = ep;
>  		pmcdev->pkgc_ltr_blocker_counters = pmc_dev_info->pkgc_ltr_blocker_counters;
>  		pmcdev->pkgc_ltr_blocker_offset = pmc_dev_info->pkgc_ltr_blocker_offset;
> +		pmcdev->pkgc_blocker_counters = pmc_dev_info->pkgc_blocker_counters;
> +		pmcdev->pkgc_blocker_offset = pmc_dev_info->pkgc_blocker_offset;
>  	}
>  }
>  
> @@ -1510,6 +1527,9 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev, struct pmc_dev_info
>  		debugfs_create_file("pkgc_ltr_blocker_show", 0444,
>  				    pmcdev->dbgfs_dir, pmcdev,
>  				    &pmc_core_pkgc_ltr_blocker_fops);
> +		debugfs_create_file("pkgc_blocker_residency_show", 0444,
> +				    pmcdev->dbgfs_dir, pmcdev,
> +				    &pmc_core_pkgc_blocker_residency_fops);
>  	}
>  
>  }
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index a20aab73c1409..829b1dee3f636 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -455,6 +455,8 @@ struct pmc {
>   *
>   * @pkgc_ltr_blocker_counters: Array of PKGC LTR blocker counters
>   * @pkgc_ltr_blocker_offset: Offset to PKGC LTR blockers in telemetry region
> + * @pkgc_blocker_counters: Array of PKGC blocker counters
> + * @pkgc_blocker_offset: Offset to PKGC blocker in telemetry region
>   *
>   * pmc_dev contains info about power management controller device.
>   */
> @@ -480,6 +482,8 @@ struct pmc_dev {
>  
>  	const char **pkgc_ltr_blocker_counters;
>  	u32 pkgc_ltr_blocker_offset;
> +	const char **pkgc_blocker_counters;
> +	u32 pkgc_blocker_offset;
>  };
>  
>  enum pmc_index {
> @@ -495,6 +499,7 @@ enum pmc_index {
>   * @dmu_guids:		List of Die Management Unit GUID
>   * @pc_guid:		GUID for telemetry region to read PKGC blocker info
>   * @pkgc_ltr_blocker_offset: Offset to PKGC LTR blockers in telemetry region
> + * @pkgc_blocker_offset:Offset to PKGC blocker in telemetry region
>   * @regmap_list:	Pointer to a list of pmc_info structure that could be
>   *			available for the platform. When set, this field implies
>   *			SSRAM support.
> @@ -502,6 +507,7 @@ enum pmc_index {
>   *			specific attributes of the primary PMC
>   * @sub_req_show:	File operations to show substate requirements
>   * @pkgc_ltr_blocker_counters: Array of PKGC LTR blocker counters
> + * @pkgc_blocker_counters: Array of PKGC blocker counters
>   * @suspend:		Function to perform platform specific suspend
>   * @resume:		Function to perform platform specific resume
>   * @init:		Function to perform platform specific init action
> @@ -512,10 +518,12 @@ struct pmc_dev_info {
>  	u32 *dmu_guids;
>  	u32 pc_guid;
>  	u32 pkgc_ltr_blocker_offset;
> +	u32 pkgc_blocker_offset;
>  	struct pmc_info *regmap_list;
>  	const struct pmc_reg_map *map;
>  	const struct file_operations *sub_req_show;
>  	const char **pkgc_ltr_blocker_counters;
> +	const char **pkgc_blocker_counters;
>  	void (*suspend)(struct pmc_dev *pmcdev);
>  	int (*resume)(struct pmc_dev *pmcdev);
>  	int (*init)(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info);
>