Re: [Intel-gfx] [PATCH 5/7] drm/i915/hwmon: Expose card reactive critical power

Gupta, Anshuman
2022-09-21T15:07:57Z

On 9/16/2022 8:30 PM, Badal Nilawar wrote:
> From: Ashutosh Dixit <ashutosh.dixit@intel.com>
>
> Expose the card reactive critical (I1) power. I1 is exposed as
> power1_crit in microwatts (typically for client products) or as
> curr1_crit in milliamperes (typically for server).
>
> v2: Add curr1_crit functionality (Ashutosh)
> v3:
> - Use HWMON_CHANNEL_INFO to define power1_crit, curr1_crit (Badal)
> v4: Use hwm_ prefix for static functions (Ashutosh)
> v5: Updated date, kernel version in documentation
>
> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
> .../ABI/testing/sysfs-driver-intel-i915-hwmon | 26 +++++
> drivers/gpu/drm/i915/i915_hwmon.c | 95 ++++++++++++++++++-
> drivers/gpu/drm/i915/i915_reg.h | 6 ++
> 3 files changed, 126 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> index 94101f818a70..cc70596fff44 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -26,6 +26,32 @@ Description: RO. Card default power limit (default TDP setting).
>
> Only supported for particular Intel i915 graphics platforms.
>
> +What: /sys/devices/.../hwmon/hwmon/power1_crit
> +Date: September 2022
> +KernelVersion: 6
> +Contact: dri-devel@lists.freedesktop.org
> +Description: RW. Card reactive critical (I1) power limit in microwatts.
> +
> + Card reactive critical (I1) power limit in microwatts is exposed
> + for client products. The power controller will throttle the
> + operating frequency if the power averaged over a window exceeds
> + this limit.
> +
> + Only supported for particular Intel i915 graphics platforms.
> +
> +What: /sys/devices/.../hwmon/hwmon/curr1_crit
> +Date: September 2022
> +KernelVersion: 6
> +Contact: dri-devel@lists.freedesktop.org
> +Description: RW. Card reactive critical (I1) power limit in milliamperes.
> +
> + Card reactive critical (I1) power limit in milliamperes is
> + exposed for server products. The power controller will throttle
> + the operating frequency if the power averaged over a window
> + exceeds this limit.
> +
> + Only supported for particular Intel i915 graphics platforms.
> +
> What: /sys/devices/.../hwmon/hwmon/energy1_input
> Date: September 2022
> KernelVersion: 6
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index a42cfad78bef..bd9ba312c474 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -11,16 +11,19 @@
> #include "i915_hwmon.h"
> #include "i915_reg.h"
> #include "intel_mchbar_regs.h"
> +#include "intel_pcode.h"
> #include "gt/intel_gt_regs.h"
>
> /*
> * SF_* - scale factors for particular quantities according to hwmon spec.
> * - voltage - millivolts
> * - power - microwatts
> + * - curr - milliamperes
> * - energy - microjoules
> */
> #define SF_VOLTAGE 1000
> #define SF_POWER 1000000
> +#define SF_CURR 1000
> #define SF_ENERGY 1000000
>
> struct hwm_reg {
> @@ -160,11 +163,25 @@ hwm_energy(struct hwm_drvdata *ddat, long *energy)
>
> static const struct hwmon_channel_info *hwm_info[] = {
> HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
> - HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX),
> + 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),
> NULL
> };
>
> +/* I1 is exposed as power_crit or as curr_crit depending on bit 31 */
> +static int hwm_pcode_read_i1(struct drm_i915_private *i915, u32 *uval)
> +{
> + return snb_pcode_read_p(&i915->uncore, PCODE_POWER_SETUP,
> + POWER_SETUP_SUBCOMMAND_READ_I1, 0, uval);
> +}
> +
> +static int hwm_pcode_write_i1(struct drm_i915_private *i915, u32 uval)
> +{
> + return snb_pcode_write_p(&i915->uncore, PCODE_POWER_SETUP,
> + POWER_SETUP_SUBCOMMAND_WRITE_I1, 0, uval);
> +}
> +
> static umode_t
> hwm_in_is_visible(const struct hwm_drvdata *ddat, u32 attr)
> {
> @@ -198,13 +215,18 @@ hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val)
> static umode_t
> hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int chan)
> {
> + struct drm_i915_private *i915 = ddat->uncore->i915;
> struct i915_hwmon *hwmon = ddat->hwmon;
> + u32 uval;
>
> switch (attr) {
> case hwmon_power_max:
> return i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit) ? 0664 : 0;
> case hwmon_power_rated_max:
> return i915_mmio_reg_valid(hwmon->rg.pkg_power_sku) ? 0444 : 0;
> + case hwmon_power_crit:
> + return (hwm_pcode_read_i1(i915, &uval) ||
> + !(uval & POWER_SETUP_I1_WATTS)) ? 0 : 0644;
> default:
> return 0;
> }
> @@ -214,6 +236,8 @@ static int
> hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
> {
> struct i915_hwmon *hwmon = ddat->hwmon;
> + int ret;
> + u32 uval;
>
> switch (attr) {
> case hwmon_power_max:
> @@ -230,6 +254,15 @@ hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
> hwmon->scl_shift_power,
> SF_POWER);
> return 0;
> + case hwmon_power_crit:
> + ret = hwm_pcode_read_i1(ddat->uncore->i915, &uval);
> + if (ret)
> + return ret;
> + if (!(uval & POWER_SETUP_I1_WATTS))
> + return -ENODEV;
> + *val = mul_u64_u32_shr(REG_FIELD_GET(POWER_SETUP_I1_DATA_MASK, uval),
> + SF_POWER, POWER_SETUP_I1_SHIFT);
> + return 0;
> default:
> return -EOPNOTSUPP;
> }
> @@ -239,6 +272,7 @@ static int
> hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
> {
> struct i915_hwmon *hwmon = ddat->hwmon;
> + u32 uval;
>
> switch (attr) {
> case hwmon_power_max:
> @@ -248,6 +282,9 @@ hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
> hwmon->scl_shift_power,
> SF_POWER, val);
> return 0;
> + case hwmon_power_crit:
> + uval = DIV_ROUND_CLOSEST_ULL(val << POWER_SETUP_I1_SHIFT, SF_POWER);
> + return hwm_pcode_write_i1(ddat->uncore->i915, uval);
> default:
> return -EOPNOTSUPP;
> }
> @@ -279,6 +316,56 @@ hwm_energy_read(struct hwm_drvdata *ddat, u32 attr, long *val)
> }
> }
>
> +static umode_t
> +hwm_curr_is_visible(const struct hwm_drvdata *ddat, u32 attr)
> +{
> + struct drm_i915_private *i915 = ddat->uncore->i915;
> + u32 uval;
> +
> + switch (attr) {
> + case hwmon_curr_crit:
> + return (hwm_pcode_read_i1(i915, &uval) ||
> + (uval & POWER_SETUP_I1_WATTS)) ? 0 : 0644;
> + default:
> + return 0;
> + }
> +}
> +
> +static int
> +hwm_curr_read(struct hwm_drvdata *ddat, u32 attr, long *val)
> +{
> + int ret;
> + u32 uval;
> +
> + switch (attr) {
> + case hwmon_curr_crit:
> + ret = hwm_pcode_read_i1(ddat->uncore->i915, &uval);
> + if (ret)
> + return ret;
> + if (uval & POWER_SETUP_I1_WATTS)
> + return -ENODEV;
> + *val = mul_u64_u32_shr(REG_FIELD_GET(POWER_SETUP_I1_DATA_MASK, uval),
> + SF_CURR, POWER_SETUP_I1_SHIFT);
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int
> +hwm_curr_write(struct hwm_drvdata *ddat, u32 attr, long val)
> +{
> + u32 uval;
> +
> + switch (attr) {
> + case hwmon_curr_crit:
> + uval = DIV_ROUND_CLOSEST_ULL(val << POWER_SETUP_I1_SHIFT, SF_CURR);
> + return hwm_pcode_write_i1(ddat->uncore->i915, uval);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> static umode_t
> hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> u32 attr, int channel)
> @@ -292,6 +379,8 @@ hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> return hwm_power_is_visible(ddat, attr, channel);
> case hwmon_energy:
> return hwm_energy_is_visible(ddat, attr);
> + case hwmon_curr:
> + return hwm_curr_is_visible(ddat, attr);
> default:
> return 0;
> }
> @@ -310,6 +399,8 @@ hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> return hwm_power_read(ddat, attr, channel, val);
> case hwmon_energy:
> return hwm_energy_read(ddat, attr, val);
> + case hwmon_curr:
> + return hwm_curr_read(ddat, attr, val);
> default:
> return -EOPNOTSUPP;
> }
> @@ -324,6 +415,8 @@ hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> switch (type) {
> case hwmon_power:
> return hwm_power_write(ddat, attr, channel, val);
> + case hwmon_curr:
> + return hwm_curr_write(ddat, attr, val);
> default:
> return -EOPNOTSUPP;
> }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 55c35903adca..956e5298ef1e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6644,6 +6644,12 @@
> #define DG1_PCODE_STATUS 0x7E
> #define DG1_UNCORE_GET_INIT_STATUS 0x0
> #define DG1_UNCORE_INIT_STATUS_COMPLETE 0x1
> +#define PCODE_POWER_SETUP 0x7C
> +#define POWER_SETUP_SUBCOMMAND_READ_I1 0x4
> +#define POWER_SETUP_SUBCOMMAND_WRITE_I1 0x5
> +#define POWER_SETUP_I1_WATTS REG_BIT(31)
> +#define POWER_SETUP_I1_SHIFT 6 /* 10.6 fixed point format */
Could please add some comment to explain, why POWER_SETUP_I1_SHIFT = 6,
what is excatly 10.6 fixed point format ?
With that.
Reviewed-by: Anshuman Gupta <anshuman.gupta@intel.com>

> +#define POWER_SETUP_I1_DATA_MASK REG_GENMASK(15, 0)
> #define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US 0x23
> #define XEHP_PCODE_FREQUENCY_CONFIG 0x6e /* xehpsdv, pvc */
> /* XEHP_PCODE_FREQUENCY_CONFIG sub-commands (param1) */