[PATCH v11 6/6] drm/xe/hwmon: Expose power sysfs entries based on firmware support
Poosa, Karthik
karthik.poosa at intel.com
Thu May 29 14:57:34 UTC 2025
On 29-05-2025 18:55, Rodrigo Vivi wrote:
> On Thu, May 29, 2025 at 04:04:30PM +0530, Karthik Poosa wrote:
>> Enable hwmon sysfs entries (power_xxx) only when GPU firmware
>> supports it.
>> Previously, these entries were created if the MMIO register
>> was present. Now, we enable based on the data in the register.
>>
>> Signed-off-by: Karthik Poosa <karthik.poosa at intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_hwmon.c | 65 +++++++++++++++++++++--------------
>> 1 file changed, 40 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
>> index c57c613471c3..25a89575a629 100644
>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
>> @@ -296,18 +296,12 @@ static void xe_hwmon_power_max_read(struct xe_hwmon *hwmon, u32 attr, int channe
>> if (hwmon->xe->info.has_mbx_power_limits) {
>> xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, (u32 *)®_val);
>> } else {
>> - rapl_limit = xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, channel);
>> - pkg_power_sku = xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU, channel);
>> -
>> /*
>> - * Valid check of REG_PKG_RAPL_LIMIT is already done in xe_hwmon_power_is_visible.
>> + * Valid check of these registers is already done in xe_hwmon_power_is_visible.
>> * So not checking it again here.
>> */
>> - if (!xe_reg_is_valid(pkg_power_sku)) {
>> - drm_warn(&xe->drm, "pkg_power_sku invalid\n");
>> - *value = 0;
>> - goto unlock;
>> - }
>> + rapl_limit = xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, channel);
>> + pkg_power_sku = xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU, channel);
>> reg_val = xe_mmio_read32(mmio, rapl_limit);
>> }
>>
>> @@ -652,17 +646,20 @@ static umode_t xe_hwmon_attributes_visible(struct kobject *kobj,
>> int ret = 0;
>> int channel = (index % 2) ? CHANNEL_PKG : CHANNEL_CARD;
>> u32 power_attr = (index > 1) ? PL2_HWMON_ATTR : PL1_HWMON_ATTR;
>> - u32 uval;
>> + u32 uval = 0;
>> + struct xe_reg rapl_limit;
>> + struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
>>
>> xe_pm_runtime_get(hwmon->xe);
>>
>> if (hwmon->xe->info.has_mbx_power_limits) {
>> xe_hwmon_pcode_read_power_limit(hwmon, power_attr, channel, &uval);
>> - ret = (uval & PWR_LIM_EN) ? attr->mode : 0;
>> } else if (power_attr != PL2_HWMON_ATTR) {
>> - ret = xe_reg_is_valid(xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT,
>> - channel)) ? attr->mode : 0;
>> + rapl_limit = xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, channel);
>> + if (xe_reg_is_valid(rapl_limit))
>> + uval = xe_mmio_read32(mmio, rapl_limit);
>> }
>> + ret = (uval & PWR_LIM_EN) ? attr->mode : 0;
>>
>> xe_pm_runtime_put(hwmon->xe);
>>
>> @@ -806,24 +803,20 @@ static umode_t
>> xe_hwmon_power_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>> {
>> u32 uval = 0;
>> - struct xe_reg rapl_limit;
>> + struct xe_reg reg;
>> struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
>>
>> switch (attr) {
>> case hwmon_power_max:
>> case hwmon_power_cap:
>> - case hwmon_power_label:
>> if (hwmon->xe->info.has_mbx_power_limits) {
>> xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &uval);
>> } else if (attr != PL2_HWMON_ATTR) {
>> - rapl_limit = xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, channel);
>> - if (xe_reg_is_valid(rapl_limit))
>> - uval = xe_mmio_read32(mmio, rapl_limit);
>> + reg = xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, channel);
>> + if (xe_reg_is_valid(reg))
>> + uval = xe_mmio_read32(mmio, reg);
>> }
>> if (uval & PWR_LIM_EN) {
>> - if (attr == hwmon_power_label)
>> - return 0444;
>> -
>> drm_info(&hwmon->xe->drm, "%s is supported on channel %d\n",
>> PWR_ATTR_TO_STR(attr), channel);
>> return 0664;
>> @@ -832,17 +825,39 @@ xe_hwmon_power_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>> PWR_ATTR_TO_STR(attr), channel);
>> return 0;
>> case hwmon_power_rated_max:
>> - if (hwmon->xe->info.has_mbx_power_limits)
>> + if (hwmon->xe->info.has_mbx_power_limits) {
>> return 0;
>> - else
>> - return xe_reg_is_valid(xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU,
>> - channel)) ? 0444 : 0;
>> + } else {
>> + reg = xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU, channel);
>> + if (xe_reg_is_valid(reg))
>> + uval = xe_mmio_read32(mmio, reg);
>> + return uval ? 0444 : 0;
> with these checks here, now your drm_info will never be visible... and the ifs there
> becomes bogus...
if check i.e PWR_LIM_EN check is 'needed' in xe_hwmon_power_max_read, as
power limit can be disabled by a write !
the drm_info there can provide dmesg log, that power limit was disabled
as shown below
root@:/sys/class/hwmon/hwmon5# echo 0 > power1_cap
root@:/sys/class/hwmon/hwmon5# dmesg | grep -i dis
[19645.834054] xe 0000:03:00.0: [drm:xe_hwmon_write [xe]] disabling PL2
on channel 0
root@:/sys/class/hwmon/hwmon5# cat power1_cap
0
root@:/sys/class/hwmon/hwmon5# dmesg | grep -i dis
[19703.412812] xe 0000:03:00.0: [drm] PL2 disabled for channel 0, val
0x0000000000000000
if we dont want the PLx disabled log in read, we could remove that.
> Move the entire logic here and make like there, checking enabling bit and not
> all the 32 bits...
REG_PKG_POWER_SKU defintion is different from REG_PKG_RAPL_LIMIT, it does not have enable bits. We have to check all bits of it.
>
>> + }
>> case hwmon_power_crit:
>> if (channel == CHANNEL_CARD) {
>> xe_hwmon_pcode_read_i1(hwmon, &uval);
>> return (uval & POWER_SETUP_I1_WATTS) ? 0644 : 0;
>> }
>> break;
>> + case hwmon_power_label:
>> + if (hwmon->xe->info.has_mbx_power_limits) {
>> + xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &uval);
>> + } else {
>> + reg = xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU, channel);
>> + if (xe_reg_is_valid(reg))
>> + uval = xe_mmio_read32(mmio, reg);
>> +
>> + if (!uval) {
>> + reg = xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, channel);
>> + if (xe_reg_is_valid(reg))
>> + uval = xe_mmio_read32(mmio, reg);
>> + }
>> + }
>> + if ((!(uval & PWR_LIM_EN)) && channel == CHANNEL_CARD) {
>> + xe_hwmon_pcode_read_i1(hwmon, &uval);
>> + return (uval & POWER_SETUP_I1_WATTS) ? 0444 : 0;
>> + }
>> + return (uval) ? 0444 : 0;
>> default:
>> return 0;
>> }
>> --
>> 2.25.1
>>
More information about the Intel-xe
mailing list