| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <23dc7824-50cd-4ba3-be5a-df141e8fe69a@intel.com>
Date: Fri, 9 Aug 2024 15:03:20 +0530
From: "Nilawar, Badal" <badal.nilawar@...el.com>
To: Raag Jadav <raag.jadav@...el.com>, <jani.nikula@...ux.intel.com>,
<joonas.lahtinen@...ux.intel.com>, <rodrigo.vivi@...el.com>,
<tursulin@...ulin.net>, <airlied@...il.com>, <daniel@...ll.ch>,
<linux@...ck-us.net>, <andi.shyti@...ux.intel.com>
CC: <intel-gfx@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>,
<linux-hwmon@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<anshuman.gupta@...el.com>, <riana.tauro@...el.com>,
<ashutosh.dixit@...el.com>, <karthik.poosa@...el.com>,
<andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v4] drm/i915/hwmon: expose fan speed
On 09-08-2024 11:45, Raag Jadav wrote:
> Add hwmon support for fan1_input attribute, which will expose fan speed
> in RPM. With this in place we can monitor fan speed using lm-sensors tool.
>
> $ sensors
> i915-pci-0300
> Adapter: PCI adapter
> in0: 653.00 mV
> fan1: 3833 RPM
> power1: N/A (max = 43.00 W)
> energy1: 32.02 kJ
>
> v2:
> - Add mutex protection
> - Handle overflow
> - Add ABI documentation
> - Aesthetic adjustments (Riana)
>
> v3:
> - Declare rotations as "long" and drop redundant casting
> - Change date and version in ABI documentation
> - Add commenter name in changelog (Riana)
>
> v4:
> - Fix wakeref leak
> - Drop switch case and simplify hwm_fan_xx() (Andi)
>
> Signed-off-by: Raag Jadav <raag.jadav@...el.com>
> Reviewed-by: Riana Tauro <riana.tauro@...el.com>
> ---
> .../ABI/testing/sysfs-driver-intel-i915-hwmon | 8 ++
> drivers/gpu/drm/i915/gt/intel_gt_regs.h | 2 +
> drivers/gpu/drm/i915/i915_hwmon.c | 81 +++++++++++++++++++
> 3 files changed, 91 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> index 92fe7c5c5ac1..be4141a7522f 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -75,3 +75,11 @@ Description: RO. Energy input of device or gt in microjoules.
> for the gt.
>
> Only supported for particular Intel i915 graphics platforms.
> +
> +What: /sys/bus/pci/drivers/i915/.../hwmon/hwmon<i>/fan1_input
> +Date: November 2024
Why November?
> +KernelVersion: 6.12
> +Contact: intel-gfx@...ts.freedesktop.org
> +Description: RO. Fan speed of device in RPM.
> +
> + Only supported for particular Intel i915 graphics platforms.
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index e42b3a5d4e63..57a3c83d3655 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -1553,6 +1553,8 @@
> #define VLV_RENDER_C0_COUNT _MMIO(0x138118)
> #define VLV_MEDIA_C0_COUNT _MMIO(0x13811c)
>
> +#define PCU_PWM_FAN_SPEED _MMIO(0x138140)
> +
> #define GEN12_RPSTAT1 _MMIO(0x1381b4)
> #define GEN12_VOLTAGE_MASK REG_GENMASK(10, 0)
> #define GEN12_CAGF_MASK REG_GENMASK(19, 11)
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 49db3e09826c..bafa5a11ed0f 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -36,6 +36,7 @@ struct hwm_reg {
> i915_reg_t pkg_rapl_limit;
> i915_reg_t energy_status_all;
> i915_reg_t energy_status_tile;
> + i915_reg_t fan_speed;
> };
>
> struct hwm_energy_info {
> @@ -43,11 +44,17 @@ struct hwm_energy_info {
> long accum_energy; /* Accumulated energy for energy1_input */
> };
>
> +struct hwm_fan_info {
> + u32 reg_val_prev;
> + u32 time_prev;
> +};
> +
> struct hwm_drvdata {
> struct i915_hwmon *hwmon;
> struct intel_uncore *uncore;
> struct device *hwmon_dev;
> struct hwm_energy_info ei; /* Energy info for energy1_input */
> + struct hwm_fan_info fi; /* Fan info for fan1_input */
> char name[12];
> int gt_n;
> bool reset_in_progress;
> @@ -276,6 +283,7 @@ static const struct hwmon_channel_info * const hwm_info[] = {
> HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX | HWMON_P_CRIT),
> HWMON_CHANNEL_INFO(energy, HWMON_E_INPUT),
> HWMON_CHANNEL_INFO(curr, HWMON_C_CRIT),
> + HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
> NULL
> };
>
> @@ -613,6 +621,63 @@ hwm_curr_write(struct hwm_drvdata *ddat, u32 attr, long val)
> }
> }
>
> +static umode_t
> +hwm_fan_is_visible(const struct hwm_drvdata *ddat, u32 attr)
> +{
> + struct i915_hwmon *hwmon = ddat->hwmon;
> +
> + return attr == hwmon_fan_input &&
> + i915_mmio_reg_valid(hwmon->rg.fan_speed) ? 0444 : 0;
> +}
> +
> +static int
> +hwm_fan_read(struct hwm_drvdata *ddat, u32 attr, long *val)
> +{
> + struct i915_hwmon *hwmon = ddat->hwmon;
> + struct hwm_fan_info *fi = &ddat->fi;
> + u32 reg_val, pulses, time, time_now;
> + intel_wakeref_t wakeref;
> + long rotations;
> + int ret = 0;
> +
> + if (attr != hwmon_fan_input)
> + return -EOPNOTSUPP;
Using a switch case in rev3 is more logical here. It will also simplify
adding more fan attributes in the future. This is why switch cases are
used in other parts of the file.
Regards,
Badal
> +
> + wakeref = intel_runtime_pm_get(ddat->uncore->rpm);
> + mutex_lock(&hwmon->hwmon_lock);
> +
> + reg_val = intel_uncore_read(ddat->uncore, hwmon->rg.fan_speed);
> + time_now = jiffies_to_msecs(jiffies);
> +
> + /* Handle overflow */
> + if (reg_val >= fi->reg_val_prev)
> + pulses = reg_val - fi->reg_val_prev;
> + else
> + pulses = UINT_MAX - fi->reg_val_prev + reg_val;
> +
> + /*
> + * HW register value is accumulated count of pulses from
> + * PWM fan with the scale of 2 pulses per rotation.
> + */
> + rotations = pulses >> 1;
> + time = time_now - fi->time_prev;
> +
> + if (unlikely(!time)) {
> + ret = -EAGAIN;
> + goto exit;
> + }
> +
> + /* Convert to minutes for calculating RPM */
> + *val = DIV_ROUND_UP(rotations * (60 * MSEC_PER_SEC), time);
> +
> + fi->reg_val_prev = reg_val;
> + fi->time_prev = time_now;
> +exit:
> + mutex_unlock(&hwmon->hwmon_lock);
> + intel_runtime_pm_put(ddat->uncore->rpm, wakeref);
> + return ret;
> +}
> +
> static umode_t
> hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> u32 attr, int channel)
> @@ -628,6 +693,8 @@ hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> return hwm_energy_is_visible(ddat, attr);
> case hwmon_curr:
> return hwm_curr_is_visible(ddat, attr);
> + case hwmon_fan:
> + return hwm_fan_is_visible(ddat, attr);
> default:
> return 0;
> }
> @@ -648,6 +715,8 @@ hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> return hwm_energy_read(ddat, attr, val);
> case hwmon_curr:
> return hwm_curr_read(ddat, attr, val);
> + case hwmon_fan:
> + return hwm_fan_read(ddat, attr, val);
> default:
> return -EOPNOTSUPP;
> }
> @@ -739,12 +808,14 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
> hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
> hwmon->rg.energy_status_all = PCU_PACKAGE_ENERGY_STATUS;
> hwmon->rg.energy_status_tile = INVALID_MMIO_REG;
> + hwmon->rg.fan_speed = PCU_PWM_FAN_SPEED;
> } else {
> hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
> hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
> hwmon->rg.energy_status_all = INVALID_MMIO_REG;
> hwmon->rg.energy_status_tile = INVALID_MMIO_REG;
> + hwmon->rg.fan_speed = INVALID_MMIO_REG;
> }
>
> with_intel_runtime_pm(uncore->rpm, wakeref) {
> @@ -755,6 +826,16 @@ hwm_get_preregistration_info(struct drm_i915_private *i915)
> if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku_unit))
> val_sku_unit = intel_uncore_read(uncore,
> hwmon->rg.pkg_power_sku_unit);
> +
> + /*
> + * Store the initial fan register value, so that we can use it for
> + * initial fan speed calculation.
> + */
> + if (i915_mmio_reg_valid(hwmon->rg.fan_speed)) {
> + ddat->fi.reg_val_prev = intel_uncore_read(uncore,
> + hwmon->rg.fan_speed);
> + ddat->fi.time_prev = jiffies_to_msecs(jiffies);
> + }
> }
>
> hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, val_sku_unit);
Powered by blists - more mailing lists