| Message ID | 20230228044334.3630391-1-ashutosh.dixit@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/hwmon: Accept writes of value 0 to power1_max_interval | expand |
On 28-02-2023 10:13, Ashutosh Dixit wrote: > The value shown by power1_max_interval in millisec is essentially: > ((1.x * power(2,y)) * 1000) >> 10 > Where x and y are read from a HW register. On ATSM, x and y are 0 on > power-up so the value shown is 0. > > Writes of 0 to power1_max_interval had previously been disallowed to avoid > computing ilog2(0) but this resulted in the corner-case bug > below. Therefore allow writes of 0 now but special case that write to > x = y = 0. > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7754 > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com> > --- > drivers/gpu/drm/i915/i915_hwmon.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c > index 7c20a6f47b92e..596dd2c070106 100644 > --- a/drivers/gpu/drm/i915/i915_hwmon.c > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > @@ -218,11 +218,15 @@ hwm_power1_max_interval_store(struct device *dev, > /* val in hw units */ > val = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_time, SF_TIME); > /* Convert to 1.x * power(2,y) */ > - if (!val) > - return -EINVAL; > - y = ilog2(val); > - /* x = (val - (1 << y)) >> (y - 2); */ > - x = (val - (1ul << y)) << x_w >> y; > + if (!val) { > + /* Avoid ilog2(0) */ > + y = 0; > + x = 0; > + } else { > + y = ilog2(val); > + /* x = (val - (1 << y)) >> (y - 2); */ > + x = (val - (1ul << y)) << x_w >> y; > + } Reviewed-by: Badal Nilawar <badal.nilawar@intel.com> > > rxy = REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_X, x) | REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_Y, y); >
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c index 7c20a6f47b92e..596dd2c070106 100644 --- a/drivers/gpu/drm/i915/i915_hwmon.c +++ b/drivers/gpu/drm/i915/i915_hwmon.c @@ -218,11 +218,15 @@ hwm_power1_max_interval_store(struct device *dev, /* val in hw units */ val = DIV_ROUND_CLOSEST_ULL((u64)val << hwmon->scl_shift_time, SF_TIME); /* Convert to 1.x * power(2,y) */ - if (!val) - return -EINVAL; - y = ilog2(val); - /* x = (val - (1 << y)) >> (y - 2); */ - x = (val - (1ul << y)) << x_w >> y; + if (!val) { + /* Avoid ilog2(0) */ + y = 0; + x = 0; + } else { + y = ilog2(val); + /* x = (val - (1 << y)) >> (y - 2); */ + x = (val - (1ul << y)) << x_w >> y; + } rxy = REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_X, x) | REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_Y, y);
The value shown by power1_max_interval in millisec is essentially: ((1.x * power(2,y)) * 1000) >> 10 Where x and y are read from a HW register. On ATSM, x and y are 0 on power-up so the value shown is 0. Writes of 0 to power1_max_interval had previously been disallowed to avoid computing ilog2(0) but this resulted in the corner-case bug below. Therefore allow writes of 0 now but special case that write to x = y = 0. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7754 Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com> --- drivers/gpu/drm/i915/i915_hwmon.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)