2024-06-14 21:10:34

by Ricardo Neri

[permalink] [raw]
Subject: [PATCH v3 0/2] drivers: thermal: intel: Use model-specific bitmasks for temperature registers

Hi,

Here is v3 of the patchset to use model-specific bitmasks to read TCC
offset and the digital temperature readout of IA32_[PACKAGE]_THERM_STATUS.

You can read the details and motivation in the cover letter of v1[1]. V2
is here [2].

Changes since v2:
* Dropped patch 3/3 ("hwmon: (coretemp) Extend the bitmask to read
temperature to 0xff") as it has been merged in v6.10-rc1.
* Used the new X86_MATCH_VFM() macro. (Rafael)
* Added Reviewed-by tags from Rui. Thanks!
* Rebased on Rafael's testing branch (based on v6.10-rc3).

I have tested these patches on Alder Lake, Meteor Lake, and Grand Ridge
systems.

These patches apply cleanly on top of the `testing` branch of the linux-pm
repository.

Thanks and BR,
Ricardo

[1]. https://lore.kernel.org/linux-pm/[email protected]/
[2]. https://lore.kernel.org/all/[email protected]/

Ricardo Neri (2):
thermal: intel: intel_tcc: Add model checks for temperature registers
thermal: intel: intel_tcc_cooling: Use a model-specific bitmask for
TCC offset

drivers/thermal/intel/intel_tcc.c | 177 +++++++++++++++++++++-
drivers/thermal/intel/intel_tcc_cooling.c | 2 +-
include/linux/intel_tcc.h | 1 +
3 files changed, 174 insertions(+), 6 deletions(-)

--
2.34.1



2024-06-14 21:10:54

by Ricardo Neri

[permalink] [raw]
Subject: [PATCH v3 2/2] thermal: intel: intel_tcc_cooling: Use a model-specific bitmask for TCC offset

The TCC offset field in the register MSR_TEMPERATURE_TARGET is not
architectural. The TCC library provides a model-specific bitmask. Use it to
determine the maximum TCC offset.

Suggested-by: Zhang Rui <[email protected]>
Reviewed-by: Zhang Rui <[email protected]>
Signed-off-by: Ricardo Neri <[email protected]>
---
Cc: Daniel Lezcano <[email protected]>
Cc: Lukasz Luba <[email protected]>
Cc: Srinivas Pandruvada <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected] # v6.7+
---
Changes since v2:
* None

Changes since v1:
* Used renamed function intel_tcc_get_offset_mask().
---
drivers/thermal/intel/intel_tcc_cooling.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/intel/intel_tcc_cooling.c b/drivers/thermal/intel/intel_tcc_cooling.c
index 63696e7d7b3c..17110ffa80bb 100644
--- a/drivers/thermal/intel/intel_tcc_cooling.c
+++ b/drivers/thermal/intel/intel_tcc_cooling.c
@@ -20,7 +20,7 @@ static struct thermal_cooling_device *tcc_cdev;
static int tcc_get_max_state(struct thermal_cooling_device *cdev, unsigned long
*state)
{
- *state = 0x3f;
+ *state = intel_tcc_get_offset_mask();
return 0;
}

--
2.34.1


2024-06-14 21:10:56

by Ricardo Neri

[permalink] [raw]
Subject: [PATCH v3 1/2] thermal: intel: intel_tcc: Add model checks for temperature registers

The register MSR_TEMPERATURE_TARGET is not architectural. Its fields may be
defined differently for each processor model. TCC_OFFSET is an example of
such case.

Despite being specified as architectural, the registers IA32_[PACKAGE]_
THERM_STATUS have become model-specific: in recent processors, the
digital temperature readout uses bits [23:16] whereas the Intel Software
Developer's manual specifies bits [22:16].

Create an array of processor models and their bitmasks for TCC_OFFSET and
the digital temperature readout fields. Do not include recent processors.
Instead, use the bitmasks of these recent processors as default.

Use these model-specific bitmasks when reading TCC_OFFSET or the
temperature sensors.

Initialize a model-specific data structure during subsys_initcall() to
have it ready when thermal drivers are loaded.

Expose the new interface intel_tcc_get_offset_mask(). The
intel_tcc_cooling driver will use it.

Reviewed-by: Zhang Rui <[email protected]>
Signed-off-by: Ricardo Neri <[email protected]>
---
Cc: Daniel Lezcano <[email protected]>
Cc: Lukasz Luba <[email protected]>
Cc: Srinivas Pandruvada <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected] # v6.7+
---
Changes since v2:
* Replaced X86_MATCH_INTEL_FAM6_MODEL() macros with the new
X86_MATCH_VFM(INTEL_*) macro. (Rafael)

Changes since v1:
* Renamed TCC_FAM6_MODEL_TEMP_MASKS as TCC_MODEL_TEMP_MASKS. (Rui)
* Renamed get_tcc_offset_mask() as intel_tcc_get_offset_mask(). (Rui)
* Do not export intel_tcc_get_temp_mask() as it is no longer used
outside intel_tcc.c
* Dropped stub functions for digital temperature readout and TCC
offset. They are not needed as users select CONFIG_INTEL_TCC.
---
drivers/thermal/intel/intel_tcc.c | 177 +++++++++++++++++++++++++++++-
include/linux/intel_tcc.h | 1 +
2 files changed, 173 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/intel/intel_tcc.c b/drivers/thermal/intel/intel_tcc.c
index 5e8b7f34b395..c86654f28aa5 100644
--- a/drivers/thermal/intel/intel_tcc.c
+++ b/drivers/thermal/intel/intel_tcc.c
@@ -6,8 +6,170 @@

#include <linux/errno.h>
#include <linux/intel_tcc.h>
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
#include <asm/msr.h>

+/**
+ * struct temp_masks - Bitmasks for temperature readings
+ * @tcc_offset: TCC offset in MSR_TEMPERATURE_TARGET
+ * @digital_readout: Digital readout in MSR_IA32_THERM_STATUS
+ * @pkg_digital_readout: Digital readout in MSR_IA32_PACKAGE_THERM_STATUS
+ *
+ * Bitmasks to extract the fields of the MSR_TEMPERATURE and IA32_[PACKAGE]_
+ * THERM_STATUS registers for different processor models.
+ *
+ * The bitmask of TjMax is not included in this structure. It is always 0xff.
+ */
+struct temp_masks {
+ u32 tcc_offset;
+ u32 digital_readout;
+ u32 pkg_digital_readout;
+};
+
+#define TCC_MODEL_TEMP_MASKS(model, _tcc_offset, _digital_readout, \
+ _pkg_digital_readout) \
+ static const struct temp_masks temp_##model __initconst = { \
+ .tcc_offset = _tcc_offset, \
+ .digital_readout = _digital_readout, \
+ .pkg_digital_readout = _pkg_digital_readout \
+ }
+
+TCC_MODEL_TEMP_MASKS(nehalem, 0, 0x7f, 0x7f);
+TCC_MODEL_TEMP_MASKS(haswell_x, 0xf, 0x7f, 0x7f);
+TCC_MODEL_TEMP_MASKS(broadwell, 0x3f, 0x7f, 0x7f);
+TCC_MODEL_TEMP_MASKS(goldmont, 0x7f, 0x7f, 0x7f);
+TCC_MODEL_TEMP_MASKS(tigerlake, 0x3f, 0xff, 0xff);
+TCC_MODEL_TEMP_MASKS(sapphirerapids, 0x3f, 0x7f, 0xff);
+
+/* Use these masks for processors not included in @tcc_cpu_ids. */
+static struct temp_masks intel_tcc_temp_masks __ro_after_init = {
+ .tcc_offset = 0x7f,
+ .digital_readout = 0xff,
+ .pkg_digital_readout = 0xff,
+};
+
+static const struct x86_cpu_id intel_tcc_cpu_ids[] __initconst = {
+ X86_MATCH_VFM(INTEL_CORE_YONAH, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_CORE2_MEROM, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_CORE2_MEROM_L, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_CORE2_PENRYN, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_CORE2_DUNNINGTON, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_NEHALEM, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_NEHALEM_G, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_NEHALEM_EP, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_NEHALEM_EX, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_WESTMERE, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_WESTMERE_EP, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_WESTMERE_EX, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_SANDYBRIDGE, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_SANDYBRIDGE_X, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_IVYBRIDGE, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_IVYBRIDGE_X, &temp_haswell_x),
+ X86_MATCH_VFM(INTEL_HASWELL, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_HASWELL_X, &temp_haswell_x),
+ X86_MATCH_VFM(INTEL_HASWELL_L, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_HASWELL_G, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_BROADWELL, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_BROADWELL_G, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_BROADWELL_X, &temp_haswell_x),
+ X86_MATCH_VFM(INTEL_BROADWELL_D, &temp_haswell_x),
+ X86_MATCH_VFM(INTEL_SKYLAKE_L, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_SKYLAKE, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_SKYLAKE_X, &temp_haswell_x),
+ X86_MATCH_VFM(INTEL_KABYLAKE_L, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_KABYLAKE, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_COMETLAKE, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_COMETLAKE_L, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_CANNONLAKE_L, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ICELAKE_X, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ICELAKE_D, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ICELAKE, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ICELAKE_L, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ICELAKE_NNPI, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ROCKETLAKE, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_TIGERLAKE_L, &temp_tigerlake),
+ X86_MATCH_VFM(INTEL_TIGERLAKE, &temp_tigerlake),
+ X86_MATCH_VFM(INTEL_SAPPHIRERAPIDS_X, &temp_sapphirerapids),
+ X86_MATCH_VFM(INTEL_EMERALDRAPIDS_X, &temp_sapphirerapids),
+ X86_MATCH_VFM(INTEL_LAKEFIELD, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ALDERLAKE, &temp_tigerlake),
+ X86_MATCH_VFM(INTEL_ALDERLAKE_L, &temp_tigerlake),
+ X86_MATCH_VFM(INTEL_RAPTORLAKE, &temp_tigerlake),
+ X86_MATCH_VFM(INTEL_RAPTORLAKE_P, &temp_tigerlake),
+ X86_MATCH_VFM(INTEL_RAPTORLAKE_S, &temp_tigerlake),
+ X86_MATCH_VFM(INTEL_ATOM_BONNELL, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_ATOM_BONNELL_MID, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_ATOM_SALTWELL, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_ATOM_SALTWELL_MID, &temp_nehalem),
+ X86_MATCH_VFM(INTEL_ATOM_SILVERMONT, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ATOM_SILVERMONT_D, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ATOM_SILVERMONT_MID, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ATOM_AIRMONT, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ATOM_AIRMONT_MID, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ATOM_AIRMONT_NP, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ATOM_GOLDMONT, &temp_goldmont),
+ X86_MATCH_VFM(INTEL_ATOM_GOLDMONT_D, &temp_goldmont),
+ X86_MATCH_VFM(INTEL_ATOM_GOLDMONT_PLUS, &temp_goldmont),
+ X86_MATCH_VFM(INTEL_ATOM_TREMONT_D, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ATOM_TREMONT, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ATOM_TREMONT_L, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_ATOM_GRACEMONT, &temp_tigerlake),
+ X86_MATCH_VFM(INTEL_XEON_PHI_KNL, &temp_broadwell),
+ X86_MATCH_VFM(INTEL_XEON_PHI_KNM, &temp_broadwell),
+ {}
+};
+
+static int __init intel_tcc_init(void)
+{
+ const struct x86_cpu_id *id;
+
+ id = x86_match_cpu(intel_tcc_cpu_ids);
+ if (id)
+ memcpy(&intel_tcc_temp_masks, (const void *)id->driver_data,
+ sizeof(intel_tcc_temp_masks));
+
+ return 0;
+}
+/*
+ * Use subsys_initcall to ensure temperature bitmasks are initialized before
+ * the drivers that use this library.
+ */
+subsys_initcall(intel_tcc_init);
+
+/**
+ * intel_tcc_get_offset_mask() - Returns the bitmask to read TCC offset
+ *
+ * Get the model-specific bitmask to extract TCC_OFFSET from the MSR
+ * TEMPERATURE_TARGET register. If the mask is 0, it means the processor does
+ * not support TCC offset.
+ *
+ * Return: The model-specific bitmask for TCC offset.
+ */
+u32 intel_tcc_get_offset_mask(void)
+{
+ return intel_tcc_temp_masks.tcc_offset;
+}
+EXPORT_SYMBOL_NS(intel_tcc_get_offset_mask, INTEL_TCC);
+
+/**
+ * get_temp_mask() - Returns the model-specific bitmask for temperature
+ *
+ * @pkg: true: Package Thermal Sensor. false: Core Thermal Sensor.
+ *
+ * Get the model-specific bitmask to extract the temperature reading from the
+ * MSR_IA32_[PACKAGE]_THERM_STATUS register.
+ *
+ * Callers must check if the thermal status registers are supported.
+ *
+ * Return: The model-specific bitmask for temperature reading
+ */
+static u32 get_temp_mask(bool pkg)
+{
+ return pkg ? intel_tcc_temp_masks.pkg_digital_readout :
+ intel_tcc_temp_masks.digital_readout;
+}
+
/**
* intel_tcc_get_tjmax() - returns the default TCC activation Temperature
* @cpu: cpu that the MSR should be run on, nagative value means any cpu.
@@ -56,7 +218,7 @@ int intel_tcc_get_offset(int cpu)
if (err)
return err;

- return (low >> 24) & 0x3f;
+ return (low >> 24) & intel_tcc_temp_masks.tcc_offset;
}
EXPORT_SYMBOL_NS_GPL(intel_tcc_get_offset, INTEL_TCC);

@@ -76,7 +238,10 @@ int intel_tcc_set_offset(int cpu, int offset)
u32 low, high;
int err;

- if (offset < 0 || offset > 0x3f)
+ if (!intel_tcc_temp_masks.tcc_offset)
+ return -ENODEV;
+
+ if (offset < 0 || offset > intel_tcc_temp_masks.tcc_offset)
return -EINVAL;

if (cpu < 0)
@@ -90,7 +255,7 @@ int intel_tcc_set_offset(int cpu, int offset)
if (low & BIT(31))
return -EPERM;

- low &= ~(0x3f << 24);
+ low &= ~(intel_tcc_temp_masks.tcc_offset << 24);
low |= offset << 24;

if (cpu < 0)
@@ -113,8 +278,8 @@ EXPORT_SYMBOL_NS_GPL(intel_tcc_set_offset, INTEL_TCC);
*/
int intel_tcc_get_temp(int cpu, int *temp, bool pkg)
{
- u32 low, high;
u32 msr = pkg ? MSR_IA32_PACKAGE_THERM_STATUS : MSR_IA32_THERM_STATUS;
+ u32 low, high, mask;
int tjmax, err;

tjmax = intel_tcc_get_tjmax(cpu);
@@ -132,7 +297,9 @@ int intel_tcc_get_temp(int cpu, int *temp, bool pkg)
if (!(low & BIT(31)))
return -ENODATA;

- *temp = tjmax - ((low >> 16) & 0x7f);
+ mask = get_temp_mask(pkg);
+
+ *temp = tjmax - ((low >> 16) & mask);

return 0;
}
diff --git a/include/linux/intel_tcc.h b/include/linux/intel_tcc.h
index 8ff8eabb4a98..fa788817acfc 100644
--- a/include/linux/intel_tcc.h
+++ b/include/linux/intel_tcc.h
@@ -14,5 +14,6 @@ int intel_tcc_get_tjmax(int cpu);
int intel_tcc_get_offset(int cpu);
int intel_tcc_set_offset(int cpu, int offset);
int intel_tcc_get_temp(int cpu, int *temp, bool pkg);
+u32 intel_tcc_get_offset_mask(void);

#endif /* __INTEL_TCC_H__ */
--
2.34.1