From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Karthik Poosa <karthik.poosa@intel.com>
Cc: <intel-xe@lists.freedesktop.org>, <anshuman.gupta@intel.com>,
<badal.nilawar@intel.com>, <raag.jadav@intel.com>,
<riana.tauro@intel.com>
Subject: Re: [PATCH v14 01/16] drm/xe/hwmon: Fix pcode power limit handling
Date: Thu, 15 Oct 2026 09:46:17 -0400 [thread overview]
Message-ID: <atDZKSgBi-xiKSPD@intel.com> (raw)
In-Reply-To: <20261015071434.1063785-2-karthik.poosa@intel.com>
On Thu, Oct 15, 2026 at 12:44:19PM +0530, Karthik Poosa wrote:
> Return early on pcode read failure in xe_hwmon_pcode_rmw_power_limit()
> and propagate errors to callers.
>
> Replace boot_power_limit_read with per-channel
> read_power_limit_from_pcode[], set on the first successful pcode write,
> so reads and writes stay consistent even if the default limit write at
> boot fails.
>
> Also return PWR_LIM_EN for power label so card label visibility works.
>
> v2: Move default power limit writes to helper
> xe_hwmon_pcode_set_default_power_limits().
>
> Fixes: 8aa7306631f0 ("drm/xe/hwmon: Fix xe_hwmon_power_max_write")
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
I shall push when CI returns...
> ---
> drivers/gpu/drm/xe/xe_hwmon.c | 84 +++++++++++++++++++++++------------
> 1 file changed, 55 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index fa5473ed6e7f..bb681d2cf991 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -166,8 +166,8 @@ struct xe_hwmon {
> struct xe_hwmon_energy_info ei[CHANNEL_MAX];
> /** @fi: Fan info for fanN_input */
> struct xe_hwmon_fan_info fi[FAN_MAX];
> - /** @boot_power_limit_read: is boot power limits read */
> - bool boot_power_limit_read;
> + /** @read_power_limit_from_pcode: pcode accepted power limits write for the channel */
> + bool read_power_limit_from_pcode[CHANNEL_MAX];
> /** @pl1_on_boot: power limit PL1 on boot */
> u32 pl1_on_boot[CHANNEL_MAX];
> /** @pl2_on_boot: power limit PL2 on boot */
> @@ -176,9 +176,9 @@ struct xe_hwmon {
> struct xe_hwmon_thermal_info temp;
> };
>
> -static inline int prepare_power_limit_param2(const struct xe_hwmon *hwmon)
> +static inline int prepare_power_limit_param2(const struct xe_hwmon *hwmon, int channel)
> {
> - if (hwmon->boot_power_limit_read) {
> + if (hwmon->read_power_limit_from_pcode[channel]) {
> if (hwmon->xe->info.platform >= XE_CRESCENTISLAND)
> return READ_PL_ACCEPTED;
> else
> @@ -199,7 +199,8 @@ static int xe_hwmon_pcode_read_power_limit(const struct xe_hwmon *hwmon, u32 att
> (channel == CHANNEL_CARD) ?
> READ_PSYSGPU_POWER_LIMIT :
> READ_PACKAGE_POWER_LIMIT,
> - prepare_power_limit_param2(hwmon)), &val0, &val1);
> + prepare_power_limit_param2(hwmon, channel)),
> + &val0, &val1);
>
> if (ret) {
> drm_dbg(&hwmon->xe->drm, "read failed ch %d val0 0x%08x, val1 0x%08x, ret %d\n",
> @@ -214,14 +215,14 @@ static int xe_hwmon_pcode_read_power_limit(const struct xe_hwmon *hwmon, u32 att
> else if (attr == PL2_HWMON_ATTR)
> *uval = (val1 & PWR_LIM_EN) ? val1 : 0;
> else if (attr == hwmon_power_label)
> - *uval = (val0 & PWR_LIM_EN) ? 1 : (val1 & PWR_LIM_EN) ? 1 : 0;
> + *uval = (val0 | val1) & PWR_LIM_EN;
> else
> *uval = 0;
>
> return ret;
> }
>
> -static int xe_hwmon_pcode_rmw_power_limit(const struct xe_hwmon *hwmon, u32 attr, u8 channel,
> +static int xe_hwmon_pcode_rmw_power_limit(struct xe_hwmon *hwmon, u32 attr, u8 channel,
> u32 clr, u32 set)
> {
> struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
> @@ -232,10 +233,13 @@ static int xe_hwmon_pcode_rmw_power_limit(const struct xe_hwmon *hwmon, u32 attr
> (channel == CHANNEL_CARD) ?
> READ_PSYSGPU_POWER_LIMIT :
> READ_PACKAGE_POWER_LIMIT,
> - prepare_power_limit_param2(hwmon)), &val0, &val1);
> - if (ret)
> - drm_dbg(&hwmon->xe->drm, "read failed ch %d val0 0x%08x, val1 0x%08x, ret %d\n",
> + prepare_power_limit_param2(hwmon, channel)),
> + &val0, &val1);
> + if (ret) {
> + drm_err(&hwmon->xe->drm, "read failed ch %d val0 0x%08x, val1 0x%08x, ret %d\n",
> channel, val0, val1, ret);
> + return ret;
> + }
>
> if (attr == PL1_HWMON_ATTR)
> val0 = (val0 & ~clr) | set;
> @@ -249,9 +253,36 @@ static int xe_hwmon_pcode_rmw_power_limit(const struct xe_hwmon *hwmon, u32 attr
> WRITE_PSYSGPU_POWER_LIMIT :
> WRITE_PACKAGE_POWER_LIMIT, 0),
> val0, val1, PCODE_DEFAULT_TIMEOUT_MS);
> - if (ret)
> + if (ret) {
> drm_dbg(&hwmon->xe->drm, "write failed ch %d val0 0x%08x, val1 0x%08x, ret %d\n",
> channel, val0, val1, ret);
> + return ret;
> + }
> +
> + /* Read from pcode once it holds a driver-written limit */
> + if (!hwmon->read_power_limit_from_pcode[channel])
> + hwmon->read_power_limit_from_pcode[channel] = true;
> +
> + return 0;
> +}
> +
> +static int xe_hwmon_pcode_set_default_power_limits(struct xe_hwmon *hwmon)
> +{
> + int ret = 0;
> +
> + ret |= xe_hwmon_pcode_rmw_power_limit(hwmon, PL1_HWMON_ATTR, CHANNEL_CARD,
> + PWR_LIM | PWR_LIM_TIME,
> + hwmon->pl1_on_boot[CHANNEL_CARD]);
> + ret |= xe_hwmon_pcode_rmw_power_limit(hwmon, PL1_HWMON_ATTR, CHANNEL_PKG,
> + PWR_LIM | PWR_LIM_TIME,
> + hwmon->pl1_on_boot[CHANNEL_PKG]);
> + ret |= xe_hwmon_pcode_rmw_power_limit(hwmon, PL2_HWMON_ATTR, CHANNEL_CARD,
> + PWR_LIM | PWR_LIM_TIME,
> + hwmon->pl2_on_boot[CHANNEL_CARD]);
> + ret |= xe_hwmon_pcode_rmw_power_limit(hwmon, PL2_HWMON_ATTR, CHANNEL_PKG,
> + PWR_LIM | PWR_LIM_TIME,
> + hwmon->pl2_on_boot[CHANNEL_PKG]);
> +
> return ret;
> }
>
> @@ -395,7 +426,9 @@ 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_rmw_power_limit(hwmon, attr, channel, PWR_LIM_EN, 0);
> + ret = xe_hwmon_pcode_rmw_power_limit(hwmon, attr, channel, PWR_LIM_EN, 0);
> + if (ret)
> + goto unlock;
> xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, ®_val);
> } else {
> reg_val = xe_mmio_rmw32(mmio, rapl_limit, PWR_LIM_EN, 0);
> @@ -680,15 +713,19 @@ 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)
> - xe_hwmon_pcode_rmw_power_limit(hwmon, power_attr, channel, PWR_LIM_TIME, rxy);
> - else
> + ret = count;
> + if (hwmon->xe->info.has_mbx_power_limits) {
> + ret = xe_hwmon_pcode_rmw_power_limit(hwmon, power_attr, channel, PWR_LIM_TIME, rxy);
> + if (!ret)
> + ret = count;
> + } 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);
>
> - return count;
> + return ret;
> }
>
> /* PSYS PL1 */
> @@ -1512,22 +1549,11 @@ 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_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]);
> - 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_rmw_power_limit(hwmon, PL2_HWMON_ATTR,
> - CHANNEL_PKG, PWR_LIM | PWR_LIM_TIME,
> - hwmon->pl2_on_boot[CHANNEL_PKG]);
> + if (xe_hwmon_pcode_set_default_power_limits(hwmon))
> + drm_warn(&hwmon->xe->drm, "Failed to set default power limits\n");
> hwmon->scl_shift_power = PWR_UNIT;
> hwmon->scl_shift_energy = ENERGY_UNIT;
> hwmon->scl_shift_time = TIME_UNIT;
> - hwmon->boot_power_limit_read = true;
> }
> } else {
> drm_info(&hwmon->xe->drm, "Using register for power limits\n");
> --
> 2.25.1
>
next prev parent reply other threads:[~2026-10-15 13:46 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-15 7:14 [PATCH v14 00/16] drm/xe/hwmon: Update hwmon thermal mailbox Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 01/16] drm/xe/hwmon: Fix pcode power limit handling Karthik Poosa
2026-10-15 13:46 ` Rodrigo Vivi [this message]
2026-10-15 7:14 ` [PATCH v14 02/16] drm/xe/hwmon: Decode mailbox temperature as sign-magnitude Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 03/16] drm/xe/hwmon: Fix thermal sensor data handling Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 04/16] drm/xe/hwmon: Add helpers to validate thermal sensor readings Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 05/16] drm/xe/hwmon: Handle unavailable memory controller sensors Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 06/16] drm/xe/hwmon: Detect unavailable PCIe thermal sensors Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 07/16] drm/xe/hwmon: Consolidate temperature sensor availability checks Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 08/16] drm/xe/hwmon: Cache temperature availability to reduce probe time Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 09/16] drm/xe/hwmon: use CRI-specific package and VRAM temperature registers Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 10/16] drm/xe/hwmon: Add platform-aware VRAM thermal channel support Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 11/16] drm/xe/hwmon: Decode CRI temperature registers as IEEE-754 Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 12/16] drm/xe/hwmon: Update memory controller temperature offset for CRI Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 13/16] drm/xe/hwmon: add PCIe temperature support " Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 14/16] drm/xe/hwmon: Simplify temperature channels config Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 15/16] drm/xe: Validate thermal sensor count before reads Karthik Poosa
2026-10-15 7:14 ` [PATCH v14 16/16] drm/xe/hwmon: Propagate all hardware read errors Karthik Poosa
2026-10-15 7:25 ` ✓ CI.KUnit: success for drm/xe/hwmon: Update hwmon thermal mailbox Patchwork
2026-10-15 13:48 ` ✓ Xe.CI.BAT: " Patchwork
2026-10-15 21:20 ` ✓ Xe.CI.FULL: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=atDZKSgBi-xiKSPD@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=anshuman.gupta@intel.com \
--cc=badal.nilawar@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=karthik.poosa@intel.com \
--cc=raag.jadav@intel.com \
--cc=riana.tauro@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.