On Thu, 15 Dec 2022, Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> wrote:
> HW allows arbitrary PL1 limits to be set but silently clamps these values
> to "typical but not guaranteed" min/max values in pkg_power_sku
> register. Follow the same pattern for sysfs, allow arbitrary PL1 limits to
> be set but display clamped values when read.
The commit message lacks the most important thing: why?
BR,
Jani.
>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_hwmon.c | 39 ++++++++++++++++++++----
> drivers/gpu/drm/i915/intel_mchbar_regs.h | 2 ++
> 2 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index cca7a4350ec8f..1225bc432f0d5 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -359,6 +359,38 @@ hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int chan)
> }
> }
>
> +/*
> + * HW allows arbitrary PL1 limits to be set but silently clamps these values to
> + * "typical but not guaranteed" min/max values in rg.pkg_power_sku. Follow the
> + * same pattern for sysfs, allow arbitrary PL1 limits to be set but display
> + * clamped values when read. Write/read I1 also follows the same pattern.
> + */
> +static int
> +hwm_power_max_read(struct hwm_drvdata *ddat, long *val)
> +{
> + struct i915_hwmon *hwmon = ddat->hwmon;
> + intel_wakeref_t wakeref;
> + u64 r, min, max;
> +
> + *val = hwm_field_read_and_scale(ddat,
> + hwmon->rg.pkg_rapl_limit,
> + PKG_PWR_LIM_1,
> + hwmon->scl_shift_power,
> + SF_POWER);
> +
> + with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> + r = intel_uncore_read64(ddat->uncore, hwmon->rg.pkg_power_sku);
> + min = REG_FIELD_GET(PKG_MIN_PWR, r);
> + min = mul_u64_u32_shr(min, SF_POWER, hwmon->scl_shift_power);
> + max = REG_FIELD_GET(PKG_MAX_PWR, r);
> + max = mul_u64_u32_shr(max, SF_POWER, hwmon->scl_shift_power);
> +
> + if (min && max)
> + *val = clamp_t(u64, *val, min, max);
> +
> + return 0;
> +}
> +
> static int
> hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
> {
> @@ -368,12 +400,7 @@ hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
>
> switch (attr) {
> case hwmon_power_max:
> - *val = hwm_field_read_and_scale(ddat,
> - hwmon->rg.pkg_rapl_limit,
> - PKG_PWR_LIM_1,
> - hwmon->scl_shift_power,
> - SF_POWER);
> - return 0;
> + return hwm_power_max_read(ddat, val);
> case hwmon_power_rated_max:
> *val = hwm_field_read_and_scale(ddat,
> hwmon->rg.pkg_power_sku,
> diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> index f93e9af43ac35..73900c098d591 100644
> --- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
> +++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> @@ -194,6 +194,8 @@
> */
> #define PCU_PACKAGE_POWER_SKU _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5930)
> #define PKG_PKG_TDP GENMASK_ULL(14, 0)
> +#define PKG_MIN_PWR GENMASK_ULL(30, 16)
> +#define PKG_MAX_PWR GENMASK_ULL(46, 32)
> #define PKG_MAX_WIN GENMASK_ULL(54, 48)
> #define PKG_MAX_WIN_X GENMASK_ULL(54, 53)
> #define PKG_MAX_WIN_Y GENMASK_ULL(52, 48)
--
Jani Nikula, Intel Open Source Graphics Center
