drm/xe/hwmon: Fix xe_hwmon_power_max_write
authorKarthik Poosa <karthik.poosa@intel.com>
Tue, 17 Jun 2025 12:00:30 +0000 (17:30 +0530)
committerThomas Hellström <thomas.hellstrom@linux.intel.com>
Tue, 24 Jun 2025 18:18:31 +0000 (20:18 +0200)
Prevent other bits of mailbox power limit from being overwritten with 0.
This issue was due to a missing read and modify of current power limit,
before setting a requested mailbox power limit, which is added in this
patch.

v2:
 - Improve commit message. (Anshuman)

v3:
 - Rebase.
 - Rephrase commit message. (Riana)
 - Add read-modify-write variant of xe_hwmon_pcode_write_power_limit()
   i.e. xe_hwmon_pcode_rmw_power_limit(). (Badal)
 - Use xe_hwmon_pcode_rmw_power_limit() to set mailbox power limits.
 - Remove xe_hwmon_pcode_write_power_limit() as all mailbox power limits
   writes use xe_hwmon_pcode_rmw_power_limit() only.

v4:
 - Use PWR_LIM in place of (PWR_LIM_EN | PWR_LIM_VAL) wherever
   applicable. (Riana)

Fixes: 25a2aa779fc3 ("drm/xe/hwmon: Add support to manage power limits though mailbox")
Reviewed-by: Riana Tauro <riana.tauro@intel.com>
Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>
Link: https://lore.kernel.org/r/20250617120030.612819-1-karthik.poosa@intel.com
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
(cherry picked from commit 8aa7306631f088881759398972d503757cf0c901)
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
drivers/gpu/drm/xe/xe_hwmon.c

index 5394a1373a6bbf0d886bef784ee7a5e7211d4ea1..ef2bf984723f2bb82bbfe1686b7ee356c006f73c 100644 (file)
@@ -40,6 +40,7 @@
 #define PCU_CR_PACKAGE_RAPL_LIMIT              XE_REG(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
 #define   PWR_LIM_VAL                          REG_GENMASK(14, 0)
 #define   PWR_LIM_EN                           REG_BIT(15)
+#define   PWR_LIM                              REG_GENMASK(15, 0)
 #define   PWR_LIM_TIME                         REG_GENMASK(23, 17)
 #define   PWR_LIM_TIME_X                       REG_GENMASK(23, 22)
 #define   PWR_LIM_TIME_Y                       REG_GENMASK(21, 17)
index 74f31639b37f997892d274078f2988fcca24e04f..f008e8049700110239de6b74e5b86ab42b9f0e66 100644 (file)
@@ -159,8 +159,8 @@ static int xe_hwmon_pcode_read_power_limit(const struct xe_hwmon *hwmon, u32 att
        return ret;
 }
 
-static int xe_hwmon_pcode_write_power_limit(const struct xe_hwmon *hwmon, u32 attr, u8 channel,
-                                           u32 uval)
+static int xe_hwmon_pcode_rmw_power_limit(const struct xe_hwmon *hwmon, u32 attr, u8 channel,
+                                         u32 clr, u32 set)
 {
        struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
        u32 val0, val1;
@@ -179,7 +179,7 @@ static int xe_hwmon_pcode_write_power_limit(const struct xe_hwmon *hwmon, u32 at
                        channel, val0, val1, ret);
 
        if (attr == PL1_HWMON_ATTR)
-               val0 = uval;
+               val0 = (val0 & ~clr) | set;
        else
                return -EIO;
 
@@ -339,7 +339,7 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
                if (hwmon->xe->info.has_mbx_power_limits) {
                        drm_dbg(&hwmon->xe->drm, "disabling %s on channel %d\n",
                                PWR_ATTR_TO_STR(attr), channel);
-                       xe_hwmon_pcode_write_power_limit(hwmon, attr, channel, 0);
+                       xe_hwmon_pcode_rmw_power_limit(hwmon, attr, channel, PWR_LIM_EN, 0);
                        xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &reg_val);
                } else {
                        reg_val = xe_mmio_rmw32(mmio, rapl_limit, PWR_LIM_EN, 0);
@@ -370,10 +370,9 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
        }
 
        if (hwmon->xe->info.has_mbx_power_limits)
-               ret = xe_hwmon_pcode_write_power_limit(hwmon, attr, channel, reg_val);
+               ret = xe_hwmon_pcode_rmw_power_limit(hwmon, attr, channel, PWR_LIM, reg_val);
        else
-               reg_val = xe_mmio_rmw32(mmio, rapl_limit, PWR_LIM_EN | PWR_LIM_VAL,
-                                       reg_val);
+               reg_val = xe_mmio_rmw32(mmio, rapl_limit, PWR_LIM, reg_val);
 unlock:
        mutex_unlock(&hwmon->hwmon_lock);
        return ret;
@@ -563,14 +562,11 @@ xe_hwmon_power_max_interval_store(struct device *dev, struct device_attribute *a
 
        mutex_lock(&hwmon->hwmon_lock);
 
-       if (hwmon->xe->info.has_mbx_power_limits) {
-               ret = xe_hwmon_pcode_read_power_limit(hwmon, power_attr, channel, (u32 *)&r);
-               r = (r & ~PWR_LIM_TIME) | rxy;
-               xe_hwmon_pcode_write_power_limit(hwmon, power_attr, channel, r);
-       } else {
+       if (hwmon->xe->info.has_mbx_power_limits)
+               xe_hwmon_pcode_rmw_power_limit(hwmon, power_attr, channel, PWR_LIM_TIME, rxy);
+       else
                r = xe_mmio_rmw32(mmio, xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, channel),
                                  PWR_LIM_TIME, rxy);
-       }
 
        mutex_unlock(&hwmon->hwmon_lock);
 
@@ -1138,12 +1134,12 @@ xe_hwmon_get_preregistration_info(struct xe_hwmon *hwmon)
                } else {
                        drm_info(&hwmon->xe->drm, "Using mailbox commands for power limits\n");
                        /* Write default limits to read from pcode from now on. */
-                       xe_hwmon_pcode_write_power_limit(hwmon, PL1_HWMON_ATTR,
-                                                        CHANNEL_CARD,
-                                                        hwmon->pl1_on_boot[CHANNEL_CARD]);
-                       xe_hwmon_pcode_write_power_limit(hwmon, PL1_HWMON_ATTR,
-                                                        CHANNEL_PKG,
-                                                        hwmon->pl1_on_boot[CHANNEL_PKG]);
+                       xe_hwmon_pcode_rmw_power_limit(hwmon, PL1_HWMON_ATTR,
+                                                      CHANNEL_CARD, PWR_LIM | PWR_LIM_TIME,
+                                                      hwmon->pl1_on_boot[CHANNEL_CARD]);
+                       xe_hwmon_pcode_rmw_power_limit(hwmon, PL1_HWMON_ATTR,
+                                                      CHANNEL_PKG, PWR_LIM | PWR_LIM_TIME,
+                                                      hwmon->pl1_on_boot[CHANNEL_PKG]);
                        hwmon->scl_shift_power = PWR_UNIT;
                        hwmon->scl_shift_energy = ENERGY_UNIT;
                        hwmon->scl_shift_time = TIME_UNIT;