Unverified Commit 8aa73066 authored by Karthik Poosa's avatar Karthik Poosa Committed by Rodrigo Vivi
Browse files

drm/xe/hwmon: Fix xe_hwmon_power_max_write 



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: 7596d839 ("drm/xe/hwmon: Add support to manage power limits though mailbox")
Reviewed-by: default avatarRiana Tauro <riana.tauro@intel.com>
Signed-off-by: default avatarKarthik Poosa <karthik.poosa@intel.com>
Reviewed-by: default avatarBadal Nilawar <badal.nilawar@intel.com>
Link: https://lore.kernel.org/r/20250617120030.612819-1-karthik.poosa@intel.com


Signed-off-by: default avatarRodrigo Vivi <rodrigo.vivi@intel.com>
parent bcc28720

drivers/gpu/drm/xe/regs/xe_mchbar_regs.h

+1 −0
Original line number Diff line number Diff line
@@ -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)

drivers/gpu/drm/xe/xe_hwmon.c

+23 −27
Original line number Diff line number Diff line
@@ -175,8 +175,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;
@@ -195,9 +195,9 @@ 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 if (attr == PL2_HWMON_ATTR)

		val1 = uval;

		val1 = (val1 & ~clr) | set;

	else

		return -EIO;
@@ -342,7 +342,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);
@@ -378,10 +378,9 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe 
	reg_val = PWR_LIM_EN | REG_FIELD_PREP(PWR_LIM_VAL, reg_val);



	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;
@@ -591,14 +590,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);
@@ -1217,24 +1213,24 @@ xe_hwmon_get_preregistration_info(struct xe_hwmon *hwmon) 
						    &hwmon->pl1_on_boot[CHANNEL_PKG]) |

		    xe_hwmon_pcode_read_power_limit(hwmon, PL2_HWMON_ATTR, CHANNEL_CARD,

						    &hwmon->pl2_on_boot[CHANNEL_CARD]) |

		    xe_hwmon_pcode_read_power_limit(hwmon, PL1_HWMON_ATTR, CHANNEL_PKG,

		    xe_hwmon_pcode_read_power_limit(hwmon, PL2_HWMON_ATTR, CHANNEL_PKG,

						    &hwmon->pl2_on_boot[CHANNEL_PKG])) {

			drm_warn(&hwmon->xe->drm,

				 "Failed to read power limits, check GPU firmware !\n");

		} 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,

			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_write_power_limit(hwmon, PL1_HWMON_ATTR,

							 CHANNEL_PKG,

			xe_hwmon_pcode_rmw_power_limit(hwmon, PL1_HWMON_ATTR,

						       CHANNEL_PKG, PWR_LIM | PWR_LIM_TIME,

						       hwmon->pl1_on_boot[CHANNEL_PKG]);

			xe_hwmon_pcode_write_power_limit(hwmon, PL2_HWMON_ATTR,

							 CHANNEL_CARD,

			xe_hwmon_pcode_rmw_power_limit(hwmon, PL2_HWMON_ATTR,

						       CHANNEL_CARD, PWR_LIM | PWR_LIM_TIME,

						       hwmon->pl2_on_boot[CHANNEL_CARD]);

			xe_hwmon_pcode_write_power_limit(hwmon, PL2_HWMON_ATTR,

							 CHANNEL_PKG,

			xe_hwmon_pcode_rmw_power_limit(hwmon, PL2_HWMON_ATTR,

						       CHANNEL_PKG, PWR_LIM | PWR_LIM_TIME,

						       hwmon->pl2_on_boot[CHANNEL_PKG]);

			hwmon->scl_shift_power = PWR_UNIT;

			hwmon->scl_shift_energy = ENERGY_UNIT;