On Mon, 13 Feb 2023 13:00:49 -0800, Ashutosh Dixit wrote:
>
> On ATSM the PL1 limit is disabled at power up. The previous uapi assumed
> that the PL1 limit is always enabled and therefore did not have a notion of
> a disabled PL1 limit. This results in erroneous PL1 limit values when the
> PL1 limit is disabled. For example at power up, the disabled ATSM PL1 limit
> was previously shown as 0 which means a low PL1 limit whereas the limit
> being disabled actually implies a high effective PL1 limit value.
>
> To get round this problem, the PL1 limit uapi is expanded to include a
> special value, -1, to designate a disabled PL1 limit.
Please don't review this patch. I have replaced this patch with a different
one in v2. Will send v2 out in a bit. Thanks!
>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx>
> ---
> .../ABI/testing/sysfs-driver-intel-i915-hwmon | 3 ++-
> drivers/gpu/drm/i915/i915_hwmon.c | 24 +++++++++++++++++++
> 2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> index 2d6a472eef885..7386c39c65cd9 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -14,7 +14,8 @@ Description: RW. Card reactive sustained (PL1/Tau) power limit in microwatts.
>
> The power controller will throttle the operating frequency
> if the power averaged over a window (typically seconds)
> - exceeds this limit.
> + exceeds this limit. A value of -1 indicates that the PL1
> + power limit is disabled.
>
> Only supported for particular Intel i915 graphics platforms.
>
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 7c20a6f47b92e..e926f2feaef4b 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -345,6 +345,8 @@ hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int chan)
> }
> }
>
> +#define PL1_DISABLE -1
> +
> /*
> * 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
> @@ -358,6 +360,14 @@ hwm_power_max_read(struct hwm_drvdata *ddat, long *val)
> intel_wakeref_t wakeref;
> u64 r, min, max;
>
> + /* Check if PL1 limit is disabled */
> + with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> + r = intel_uncore_read(ddat->uncore, hwmon->rg.pkg_rapl_limit);
> + if (!(r & PKG_PWR_LIM_1_EN)) {
> + *val = PL1_DISABLE;
> + return 0;
> + }
> +
> *val = hwm_field_read_and_scale(ddat,
> hwmon->rg.pkg_rapl_limit,
> PKG_PWR_LIM_1,
> @@ -381,8 +391,22 @@ static int
> hwm_power_max_write(struct hwm_drvdata *ddat, long val)
> {
> struct i915_hwmon *hwmon = ddat->hwmon;
> + intel_wakeref_t wakeref;
> u32 nval;
>
> + if (val == PL1_DISABLE) {
> + /* Disable PL1 limit */
> + hwm_locked_with_pm_intel_uncore_rmw(ddat, hwmon->rg.pkg_rapl_limit,
> + PKG_PWR_LIM_1_EN, 0);
> +
> + /* Verify, because PL1 limit cannot be disabled on all platforms */
> + with_intel_runtime_pm(ddat->uncore->rpm, wakeref)
> + nval = intel_uncore_read(ddat->uncore, hwmon->rg.pkg_rapl_limit);
> + if (nval & PKG_PWR_LIM_1_EN)
> + return -EPERM;
> + return 0;
> + }
> +
> /* Computation in 64-bits to avoid overflow. Round to nearest. */
> nval = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_power, SF_POWER);
> nval = PKG_PWR_LIM_1_EN | REG_FIELD_PREP(PKG_PWR_LIM_1, nval);
> --
> 2.38.0
>
