[PATCH 1/2] platform/x86: ISST: Support SST-TF revision 2

Srinivas Pandruvada posted 2 patches 9 months, 3 weeks ago
There is a newer version of this series
[PATCH 1/2] platform/x86: ISST: Support SST-TF revision 2
Posted by Srinivas Pandruvada 9 months, 3 weeks ago
SST-TF revision 2 supports a higher number of cores per bucket, as the
current limit of 256 cores may be insufficient. To accommodate this, a
new offset, "SST_TF_INFO-8," is introduced, allowing for a higher core
count. Utilize this offset instead of the current "SST_TF_INFO-1" offset,
based on SST-TF revision 2 or higher, and if there is a non-zero core
count in any bucket.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 .../intel/speed_select_if/isst_tpmi_core.c    | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
index 9978cdd19851..bc4089d3d421 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
@@ -1328,9 +1328,14 @@ static int isst_if_get_tpmi_instance_count(void __user *argp)
 #define SST_TF_INFO_0_OFFSET	0
 #define SST_TF_INFO_1_OFFSET	8
 #define SST_TF_INFO_2_OFFSET	16
+#define SST_TF_INFO_8_OFFSET	64
+#define SST_TF_INFO_8_BUCKETS	3
 
 #define SST_TF_MAX_LP_CLIP_RATIOS	TRL_MAX_LEVELS
 
+#define SST_TF_FEATURE_REV_START	4
+#define SST_TF_FEATURE_REV_WIDTH	8
+
 #define SST_TF_LP_CLIP_RATIO_0_START	16
 #define SST_TF_LP_CLIP_RATIO_0_WIDTH	8
 
@@ -1340,10 +1345,14 @@ static int isst_if_get_tpmi_instance_count(void __user *argp)
 #define SST_TF_NUM_CORE_0_START 0
 #define SST_TF_NUM_CORE_0_WIDTH 8
 
+#define SST_TF_NUM_MOD_0_START	0
+#define SST_TF_NUM_MOD_0_WIDTH	16
+
 static int isst_if_get_turbo_freq_info(void __user *argp)
 {
 	static struct isst_turbo_freq_info turbo_freq;
 	struct tpmi_per_power_domain_info *power_domain_info;
+	u8 feature_rev;
 	int i, j;
 
 	if (copy_from_user(&turbo_freq, argp, sizeof(turbo_freq)))
@@ -1360,6 +1369,10 @@ static int isst_if_get_turbo_freq_info(void __user *argp)
 	turbo_freq.max_trl_levels = TRL_MAX_LEVELS;
 	turbo_freq.max_clip_freqs = SST_TF_MAX_LP_CLIP_RATIOS;
 
+	_read_tf_level_info("feature_rev", feature_rev, turbo_freq.level,
+			    SST_TF_INFO_0_OFFSET, SST_TF_FEATURE_REV_START,
+			    SST_TF_FEATURE_REV_WIDTH, SST_MUL_FACTOR_NONE);
+
 	for (i = 0; i < turbo_freq.max_clip_freqs; ++i)
 		_read_tf_level_info("lp_clip*", turbo_freq.lp_clip_freq_mhz[i],
 				    turbo_freq.level, SST_TF_INFO_0_OFFSET,
@@ -1376,12 +1389,32 @@ static int isst_if_get_turbo_freq_info(void __user *argp)
 					    SST_MUL_FACTOR_FREQ)
 	}
 
+	if (feature_rev >= 2) {
+		bool valid = false;
+
+		for (i = 0; i < SST_TF_INFO_8_BUCKETS; ++i) {
+			_read_tf_level_info("bucket_*_mod_count", turbo_freq.bucket_core_counts[i],
+					    turbo_freq.level, SST_TF_INFO_8_OFFSET,
+					    SST_TF_NUM_MOD_0_WIDTH * i, SST_TF_NUM_MOD_0_WIDTH,
+					    SST_MUL_FACTOR_NONE)
+
+			if (!valid && turbo_freq.bucket_core_counts[i])
+				valid = true;
+		}
+
+		if (valid)
+			goto done_core_count;
+	}
+
 	for (i = 0; i < TRL_MAX_BUCKETS; ++i)
 		_read_tf_level_info("bucket_*_core_count", turbo_freq.bucket_core_counts[i],
 				    turbo_freq.level, SST_TF_INFO_1_OFFSET,
 				    SST_TF_NUM_CORE_0_WIDTH * i, SST_TF_NUM_CORE_0_WIDTH,
 				    SST_MUL_FACTOR_NONE)
 
+
+done_core_count:
+
 	if (copy_to_user(argp, &turbo_freq, sizeof(turbo_freq)))
 		return -EFAULT;
 
-- 
2.48.1
Re: [PATCH 1/2] platform/x86: ISST: Support SST-TF revision 2
Posted by Ilpo Järvinen 9 months, 2 weeks ago
On Thu, 17 Apr 2025, Srinivas Pandruvada wrote:

> SST-TF revision 2 supports a higher number of cores per bucket, as the
> current limit of 256 cores may be insufficient. To accommodate this, a
> new offset, "SST_TF_INFO-8," is introduced, allowing for a higher core
> count. Utilize this offset instead of the current "SST_TF_INFO-1" offset,
> based on SST-TF revision 2 or higher, and if there is a non-zero core
> count in any bucket.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  .../intel/speed_select_if/isst_tpmi_core.c    | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
> index 9978cdd19851..bc4089d3d421 100644
> --- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
> +++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
> @@ -1328,9 +1328,14 @@ static int isst_if_get_tpmi_instance_count(void __user *argp)
>  #define SST_TF_INFO_0_OFFSET	0
>  #define SST_TF_INFO_1_OFFSET	8
>  #define SST_TF_INFO_2_OFFSET	16
> +#define SST_TF_INFO_8_OFFSET	64
> +#define SST_TF_INFO_8_BUCKETS	3
>  
>  #define SST_TF_MAX_LP_CLIP_RATIOS	TRL_MAX_LEVELS
>  
> +#define SST_TF_FEATURE_REV_START	4
> +#define SST_TF_FEATURE_REV_WIDTH	8
> +
>  #define SST_TF_LP_CLIP_RATIO_0_START	16
>  #define SST_TF_LP_CLIP_RATIO_0_WIDTH	8
>  
> @@ -1340,10 +1345,14 @@ static int isst_if_get_tpmi_instance_count(void __user *argp)
>  #define SST_TF_NUM_CORE_0_START 0
>  #define SST_TF_NUM_CORE_0_WIDTH 8
>  
> +#define SST_TF_NUM_MOD_0_START	0
> +#define SST_TF_NUM_MOD_0_WIDTH	16
> +
>  static int isst_if_get_turbo_freq_info(void __user *argp)
>  {
>  	static struct isst_turbo_freq_info turbo_freq;
>  	struct tpmi_per_power_domain_info *power_domain_info;
> +	u8 feature_rev;
>  	int i, j;
>  
>  	if (copy_from_user(&turbo_freq, argp, sizeof(turbo_freq)))
> @@ -1360,6 +1369,10 @@ static int isst_if_get_turbo_freq_info(void __user *argp)
>  	turbo_freq.max_trl_levels = TRL_MAX_LEVELS;
>  	turbo_freq.max_clip_freqs = SST_TF_MAX_LP_CLIP_RATIOS;
>  
> +	_read_tf_level_info("feature_rev", feature_rev, turbo_freq.level,
> +			    SST_TF_INFO_0_OFFSET, SST_TF_FEATURE_REV_START,
> +			    SST_TF_FEATURE_REV_WIDTH, SST_MUL_FACTOR_NONE);
> +
>  	for (i = 0; i < turbo_freq.max_clip_freqs; ++i)
>  		_read_tf_level_info("lp_clip*", turbo_freq.lp_clip_freq_mhz[i],
>  				    turbo_freq.level, SST_TF_INFO_0_OFFSET,
> @@ -1376,12 +1389,32 @@ static int isst_if_get_turbo_freq_info(void __user *argp)
>  					    SST_MUL_FACTOR_FREQ)
>  	}
>  
> +	if (feature_rev >= 2) {
> +		bool valid = false;
> +
> +		for (i = 0; i < SST_TF_INFO_8_BUCKETS; ++i) {
> +			_read_tf_level_info("bucket_*_mod_count", turbo_freq.bucket_core_counts[i],
> +					    turbo_freq.level, SST_TF_INFO_8_OFFSET,
> +					    SST_TF_NUM_MOD_0_WIDTH * i, SST_TF_NUM_MOD_0_WIDTH,
> +					    SST_MUL_FACTOR_NONE)
> +
> +			if (!valid && turbo_freq.bucket_core_counts[i])

I'd just drop !valid from this check.

> +				valid = true;
> +		}
> +
> +		if (valid)


Should this be named instead to something like has_tf_info_8 ? (As this is 
not really valid/invalid check but whether this new info exists or not.)

> +			goto done_core_count;
> +	}
> +
>  	for (i = 0; i < TRL_MAX_BUCKETS; ++i)
>  		_read_tf_level_info("bucket_*_core_count", turbo_freq.bucket_core_counts[i],
>  				    turbo_freq.level, SST_TF_INFO_1_OFFSET,
>  				    SST_TF_NUM_CORE_0_WIDTH * i, SST_TF_NUM_CORE_0_WIDTH,
>  				    SST_MUL_FACTOR_NONE)
>  
> +
> +done_core_count:
> +
>  	if (copy_to_user(argp, &turbo_freq, sizeof(turbo_freq)))
>  		return -EFAULT;
>  
> 

-- 
 i.
Re: [PATCH 1/2] platform/x86: ISST: Support SST-TF revision 2
Posted by srinivas pandruvada 9 months, 2 weeks ago
On Tue, 2025-04-29 at 17:23 +0300, Ilpo Järvinen wrote:
> On Thu, 17 Apr 2025, Srinivas Pandruvada wrote:
> 
> > SST-TF revision 2 supports a higher number of cores per bucket, as
> > the
> > current limit of 256 cores may be insufficient. To accommodate
> > this, a
> > new offset, "SST_TF_INFO-8," is introduced, allowing for a higher
> > core
> > count. Utilize this offset instead of the current "SST_TF_INFO-1"
> > offset,
> > based on SST-TF revision 2 or higher, and if there is a non-zero
> > core
> > count in any bucket.
> > 
...

> > +	if (feature_rev >= 2) {
> > +		bool valid = false;
> > +
> > +		for (i = 0; i < SST_TF_INFO_8_BUCKETS; ++i) {
> > +			_read_tf_level_info("bucket_*_mod_count",
> > turbo_freq.bucket_core_counts[i],
> > +					    turbo_freq.level,
> > SST_TF_INFO_8_OFFSET,
> > +					    SST_TF_NUM_MOD_0_WIDTH
> > * i, SST_TF_NUM_MOD_0_WIDTH,
> > +					    SST_MUL_FACTOR_NONE)
> > +
> > +			if (!valid &&
> > turbo_freq.bucket_core_counts[i])
> 
> I'd just drop !valid from this check.
> 
> > +				valid = true;
> > +		}
> > +
> > +		if (valid)
> 
> 
> Should this be named instead to something like has_tf_info_8 ? (As
> this is 
> not really valid/invalid check but whether this new info exists or
> not.)
We can.

Thanks,
Srinivas

> 
> > +			goto done_core_count;
> > +	}
> > +
> >  	for (i = 0; i < TRL_MAX_BUCKETS; ++i)
> >  		_read_tf_level_info("bucket_*_core_count",
> > turbo_freq.bucket_core_counts[i],
> >  				    turbo_freq.level,
> > SST_TF_INFO_1_OFFSET,
> >  				    SST_TF_NUM_CORE_0_WIDTH * i,
> > SST_TF_NUM_CORE_0_WIDTH,
> >  				    SST_MUL_FACTOR_NONE)
> >  
> > +
> > +done_core_count:
> > +
> >  	if (copy_to_user(argp, &turbo_freq, sizeof(turbo_freq)))
> >  		return -EFAULT;
> >  
> > 
>