Re: [Intel-gfx] [PATCH] drm/i915/hwmon: Accept writes of value 0 to power1_max_interval

[Nilawar, Badal]

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 
---
  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 
  
  	rxy = REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_X, x) | REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_Y, y);
  

[Intel-gfx] [PATCH] drm/i915/hwmon: Accept writes of value 0 to power1_max_interval

[Ashutosh Dixit]

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 
---
 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;
+   }
 
rxy = REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_X, x) | 
REG_FIELD_PREP(PKG_PWR_LIM_1_TIME_Y, y);
 
-- 
2.38.0