All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/xe/hwmon: Propagate all hardware read errors
@ 2026-10-12 11:06 Karthik Poosa
  2026-10-13  0:59 ` Rodrigo Vivi
  0 siblings, 1 reply; 3+ messages in thread
From: Karthik Poosa @ 2026-10-12 11:06 UTC (permalink / raw)
  To: intel-xe
  Cc: rodrigo.vivi, anshuman.gupta, badal.nilawar, raag.jadav,
	riana.tauro, sk.anirban, mallesh.koujalagi, soham.purkait,
	Karthik Poosa

Propagate all failures from pcode and mailbox accesses and fix missed
return-value checks found by Sashiko in
https://patchwork.freedesktop.org/patch/758843/?series=174898&rev=6.

Fixes: 7596d839f622 ("drm/xe/hwmon: Add support to manage power limits though mailbox")
Fixes: 0c5405d3aa4ad ("drm/xe/hwmon: Read energy status from PMT")
Fixes: 25e963a09e059 ("drm/xe/hwmon: Move card reactive critical power under channel card")
Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
---
 drivers/gpu/drm/xe/xe_hwmon.c | 47 +++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
index 768566648bc8..fbcc04341e29 100644
--- a/drivers/gpu/drm/xe/xe_hwmon.c
+++ b/drivers/gpu/drm/xe/xe_hwmon.c
@@ -381,17 +381,20 @@ static struct xe_reg xe_hwmon_get_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg
  * same pattern for sysfs, allow arbitrary PL1 limits to be set but display
  * clamped values when read.
  */
-static void xe_hwmon_power_max_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *value)
+static int xe_hwmon_power_max_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *value)
 {
 	u32 reg_val = 0;
 	struct xe_device *xe = hwmon->xe;
 	struct xe_reg rapl_limit, pkg_power_sku;
 	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
+	int ret = 0;
 
 	mutex_lock(&hwmon->hwmon_lock);
 
 	if (hwmon->xe->info.has_mbx_power_limits) {
-		xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &reg_val);
+		ret = xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &reg_val);
+		if (ret)
+			goto unlock;
 	} 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);
@@ -423,6 +426,7 @@ static void xe_hwmon_power_max_read(struct xe_hwmon *hwmon, u32 attr, int channe
 	}
 unlock:
 	mutex_unlock(&hwmon->hwmon_lock);
+	return ret;
 }
 
 static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channel, long value)
@@ -445,7 +449,9 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
 			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, &reg_val);
+			ret = xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &reg_val);
+			if (ret)
+				goto unlock;
 		} else {
 			reg_val = xe_mmio_rmw32(mmio, rapl_limit, PWR_LIM_EN, 0);
 			reg_val = xe_mmio_read32(mmio, rapl_limit);
@@ -502,15 +508,18 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
 	return ret;
 }
 
-static void xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int channel,
-					  long *value)
+static int xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int channel,
+					 long *value)
 {
 	struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
 	u32 reg_val;
+	int ret = 0;
 
 	if (hwmon->xe->info.has_mbx_power_limits) {
 		/* PL1 is rated max if supported. */
-		xe_hwmon_pcode_read_power_limit(hwmon, PL1_HWMON_ATTR, channel, &reg_val);
+		ret = xe_hwmon_pcode_read_power_limit(hwmon, PL1_HWMON_ATTR, channel, &reg_val);
+		if (ret)
+			return ret;
 	} else {
 		/*
 		 * This sysfs file won't be visible if REG_PKG_POWER_SKU is invalid, so valid check
@@ -524,6 +533,7 @@ static void xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int
 
 	reg_val = REG_FIELD_GET(PKG_TDP, reg_val);
 	*value = mul_u64_u32_shr(reg_val, SF_POWER, hwmon->scl_shift_power);
+	return ret;
 }
 
 /*
@@ -546,7 +556,7 @@ static void xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int
  * hwmon->scl_shift_energy of 14 bits we have 57 (63 - 20 + 14) bits before
  * energyN_input overflows. This at 1000 W is an overflow duration of 278 years.
  */
-static void
+static int
 xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
 {
 	struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
@@ -557,7 +567,7 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
 	/* Energy is supported only for card and pkg */
 	if (channel > CHANNEL_PKG) {
 		*energy = 0;
-		return;
+		return 0;
 	}
 
 	if (hwmon->xe->info.platform == XE_BATTLEMAGE) {
@@ -569,7 +579,7 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
 		if (ret != sizeof(pmt_val)) {
 			drm_warn(&hwmon->xe->drm, "energy read from pmt failed, ret %d\n", ret);
 			*energy = 0;
-			return;
+			return ret < 0 ? ret : -EIO;
 		}
 
 		if (channel == CHANNEL_PKG)
@@ -586,6 +596,7 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
 
 	*energy = mul_u64_u32_shr(ei->accum_energy, SF_ENERGY,
 				  hwmon->scl_shift_energy);
+	return 0;
 }
 
 static ssize_t
@@ -611,7 +622,8 @@ xe_hwmon_power_max_interval_show(struct device *dev, struct device_attribute *at
 			drm_err(&hwmon->xe->drm,
 				"power interval read fail, ch %d, attr %d, val 0x%08x, ret %d\n",
 				channel, power_attr, reg_val, ret);
-			reg_val = 0;
+			mutex_unlock(&hwmon->hwmon_lock);
+			return ret;
 		}
 	} else {
 		reg_val = xe_mmio_read32(mmio, xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT,
@@ -1391,7 +1403,8 @@ xe_hwmon_power_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
 		}
 	case hwmon_power_crit:
 		if (channel == CHANNEL_CARD) {
-			xe_hwmon_pcode_read_i1(hwmon, &uval);
+			if (xe_hwmon_pcode_read_i1(hwmon, &uval))
+				return 0;
 			return (uval & POWER_SETUP_I1_WATTS) ? 0644 : 0;
 		}
 		break;
@@ -1410,7 +1423,8 @@ xe_hwmon_power_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
 			}
 		}
 		if ((!(uval & PWR_LIM_EN)) && channel == CHANNEL_CARD) {
-			xe_hwmon_pcode_read_i1(hwmon, &uval);
+			if (xe_hwmon_pcode_read_i1(hwmon, &uval))
+				return 0;
 			return (uval & POWER_SETUP_I1_WATTS) ? 0444 : 0;
 		}
 		return (uval) ? 0444 : 0;
@@ -1426,11 +1440,9 @@ xe_hwmon_power_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
 	switch (attr) {
 	case hwmon_power_max:
 	case hwmon_power_cap:
-		xe_hwmon_power_max_read(hwmon, attr, channel, val);
-		return 0;
+		return xe_hwmon_power_max_read(hwmon, attr, channel, val);
 	case hwmon_power_rated_max:
-		xe_hwmon_power_rated_max_read(hwmon, attr, channel, val);
-		return 0;
+		return xe_hwmon_power_rated_max_read(hwmon, attr, channel, val);
 	case hwmon_power_crit:
 		return xe_hwmon_power_curr_crit_read(hwmon, channel, val, SF_POWER);
 	default:
@@ -1547,8 +1559,7 @@ xe_hwmon_energy_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
 {
 	switch (attr) {
 	case hwmon_energy_input:
-		xe_hwmon_energy_get(hwmon, channel, val);
-		return 0;
+		return xe_hwmon_energy_get(hwmon, channel, val);
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/xe/hwmon: Propagate all hardware read errors
  2026-10-12 11:06 [PATCH] drm/xe/hwmon: Propagate all hardware read errors Karthik Poosa
@ 2026-10-13  0:59 ` Rodrigo Vivi
  2026-10-13  2:20   ` Poosa, Karthik
  0 siblings, 1 reply; 3+ messages in thread
From: Rodrigo Vivi @ 2026-10-13  0:59 UTC (permalink / raw)
  To: Karthik Poosa
  Cc: intel-xe, anshuman.gupta, badal.nilawar, raag.jadav, riana.tauro,
	sk.anirban, mallesh.koujalagi, soham.purkait

On Mon, Oct 12, 2026 at 04:36:48PM +0530, Karthik Poosa wrote:
> Propagate all failures from pcode and mailbox accesses and fix missed
> return-value checks found by Sashiko in
> https://patchwork.freedesktop.org/patch/758843/?series=174898&rev=6.
> 
> Fixes: 7596d839f622 ("drm/xe/hwmon: Add support to manage power limits though mailbox")
> Fixes: 0c5405d3aa4ad ("drm/xe/hwmon: Read energy status from PMT")
> Fixes: 25e963a09e059 ("drm/xe/hwmon: Move card reactive critical power under channel card")
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>

Please add this as part of your re-submission of the other v10 series for the CI

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_hwmon.c | 47 +++++++++++++++++++++--------------
>  1 file changed, 29 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index 768566648bc8..fbcc04341e29 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -381,17 +381,20 @@ static struct xe_reg xe_hwmon_get_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg
>   * same pattern for sysfs, allow arbitrary PL1 limits to be set but display
>   * clamped values when read.
>   */
> -static void xe_hwmon_power_max_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *value)
> +static int xe_hwmon_power_max_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *value)
>  {
>  	u32 reg_val = 0;
>  	struct xe_device *xe = hwmon->xe;
>  	struct xe_reg rapl_limit, pkg_power_sku;
>  	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
> +	int ret = 0;
>  
>  	mutex_lock(&hwmon->hwmon_lock);
>  
>  	if (hwmon->xe->info.has_mbx_power_limits) {
> -		xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &reg_val);
> +		ret = xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &reg_val);
> +		if (ret)
> +			goto unlock;
>  	} 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);
> @@ -423,6 +426,7 @@ static void xe_hwmon_power_max_read(struct xe_hwmon *hwmon, u32 attr, int channe
>  	}
>  unlock:
>  	mutex_unlock(&hwmon->hwmon_lock);
> +	return ret;
>  }
>  
>  static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channel, long value)
> @@ -445,7 +449,9 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
>  			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, &reg_val);
> +			ret = xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &reg_val);
> +			if (ret)
> +				goto unlock;
>  		} else {
>  			reg_val = xe_mmio_rmw32(mmio, rapl_limit, PWR_LIM_EN, 0);
>  			reg_val = xe_mmio_read32(mmio, rapl_limit);
> @@ -502,15 +508,18 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
>  	return ret;
>  }
>  
> -static void xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int channel,
> -					  long *value)
> +static int xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int channel,
> +					 long *value)
>  {
>  	struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
>  	u32 reg_val;
> +	int ret = 0;
>  
>  	if (hwmon->xe->info.has_mbx_power_limits) {
>  		/* PL1 is rated max if supported. */
> -		xe_hwmon_pcode_read_power_limit(hwmon, PL1_HWMON_ATTR, channel, &reg_val);
> +		ret = xe_hwmon_pcode_read_power_limit(hwmon, PL1_HWMON_ATTR, channel, &reg_val);
> +		if (ret)
> +			return ret;
>  	} else {
>  		/*
>  		 * This sysfs file won't be visible if REG_PKG_POWER_SKU is invalid, so valid check
> @@ -524,6 +533,7 @@ static void xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int
>  
>  	reg_val = REG_FIELD_GET(PKG_TDP, reg_val);
>  	*value = mul_u64_u32_shr(reg_val, SF_POWER, hwmon->scl_shift_power);
> +	return ret;
>  }
>  
>  /*
> @@ -546,7 +556,7 @@ static void xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int
>   * hwmon->scl_shift_energy of 14 bits we have 57 (63 - 20 + 14) bits before
>   * energyN_input overflows. This at 1000 W is an overflow duration of 278 years.
>   */
> -static void
> +static int
>  xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
>  {
>  	struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
> @@ -557,7 +567,7 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
>  	/* Energy is supported only for card and pkg */
>  	if (channel > CHANNEL_PKG) {
>  		*energy = 0;
> -		return;
> +		return 0;
>  	}
>  
>  	if (hwmon->xe->info.platform == XE_BATTLEMAGE) {
> @@ -569,7 +579,7 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
>  		if (ret != sizeof(pmt_val)) {
>  			drm_warn(&hwmon->xe->drm, "energy read from pmt failed, ret %d\n", ret);
>  			*energy = 0;
> -			return;
> +			return ret < 0 ? ret : -EIO;
>  		}
>  
>  		if (channel == CHANNEL_PKG)
> @@ -586,6 +596,7 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
>  
>  	*energy = mul_u64_u32_shr(ei->accum_energy, SF_ENERGY,
>  				  hwmon->scl_shift_energy);
> +	return 0;
>  }
>  
>  static ssize_t
> @@ -611,7 +622,8 @@ xe_hwmon_power_max_interval_show(struct device *dev, struct device_attribute *at
>  			drm_err(&hwmon->xe->drm,
>  				"power interval read fail, ch %d, attr %d, val 0x%08x, ret %d\n",
>  				channel, power_attr, reg_val, ret);
> -			reg_val = 0;
> +			mutex_unlock(&hwmon->hwmon_lock);
> +			return ret;
>  		}
>  	} else {
>  		reg_val = xe_mmio_read32(mmio, xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT,
> @@ -1391,7 +1403,8 @@ xe_hwmon_power_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>  		}
>  	case hwmon_power_crit:
>  		if (channel == CHANNEL_CARD) {
> -			xe_hwmon_pcode_read_i1(hwmon, &uval);
> +			if (xe_hwmon_pcode_read_i1(hwmon, &uval))
> +				return 0;
>  			return (uval & POWER_SETUP_I1_WATTS) ? 0644 : 0;
>  		}
>  		break;
> @@ -1410,7 +1423,8 @@ xe_hwmon_power_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>  			}
>  		}
>  		if ((!(uval & PWR_LIM_EN)) && channel == CHANNEL_CARD) {
> -			xe_hwmon_pcode_read_i1(hwmon, &uval);
> +			if (xe_hwmon_pcode_read_i1(hwmon, &uval))
> +				return 0;
>  			return (uval & POWER_SETUP_I1_WATTS) ? 0444 : 0;
>  		}
>  		return (uval) ? 0444 : 0;
> @@ -1426,11 +1440,9 @@ xe_hwmon_power_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>  	switch (attr) {
>  	case hwmon_power_max:
>  	case hwmon_power_cap:
> -		xe_hwmon_power_max_read(hwmon, attr, channel, val);
> -		return 0;
> +		return xe_hwmon_power_max_read(hwmon, attr, channel, val);
>  	case hwmon_power_rated_max:
> -		xe_hwmon_power_rated_max_read(hwmon, attr, channel, val);
> -		return 0;
> +		return xe_hwmon_power_rated_max_read(hwmon, attr, channel, val);
>  	case hwmon_power_crit:
>  		return xe_hwmon_power_curr_crit_read(hwmon, channel, val, SF_POWER);
>  	default:
> @@ -1547,8 +1559,7 @@ xe_hwmon_energy_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>  {
>  	switch (attr) {
>  	case hwmon_energy_input:
> -		xe_hwmon_energy_get(hwmon, channel, val);
> -		return 0;
> +		return xe_hwmon_energy_get(hwmon, channel, val);
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/xe/hwmon: Propagate all hardware read errors
  2026-10-13  0:59 ` Rodrigo Vivi
@ 2026-10-13  2:20   ` Poosa, Karthik
  0 siblings, 0 replies; 3+ messages in thread
From: Poosa, Karthik @ 2026-10-13  2:20 UTC (permalink / raw)
  To: Rodrigo Vivi
  Cc: intel-xe, anshuman.gupta, badal.nilawar, raag.jadav, riana.tauro,
	sk.anirban, mallesh.koujalagi, soham.purkait


On 13-10-2026 06:29, Rodrigo Vivi wrote:
> On Mon, Oct 12, 2026 at 04:36:48PM +0530, Karthik Poosa wrote:
>> Propagate all failures from pcode and mailbox accesses and fix missed
>> return-value checks found by Sashiko in
>> https://patchwork.freedesktop.org/patch/758843/?series=174898&rev=6.
>>
>> Fixes: 7596d839f622 ("drm/xe/hwmon: Add support to manage power limits though mailbox")
>> Fixes: 0c5405d3aa4ad ("drm/xe/hwmon: Read energy status from PMT")
>> Fixes: 25e963a09e059 ("drm/xe/hwmon: Move card reactive critical power under channel card")
>> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
> Please add this as part of your re-submission of the other v10 series for the CI
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Added it - 
https://patchwork.freedesktop.org/patch/759632/?series=175993&rev=1
>
>> ---
>>   drivers/gpu/drm/xe/xe_hwmon.c | 47 +++++++++++++++++++++--------------
>>   1 file changed, 29 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
>> index 768566648bc8..fbcc04341e29 100644
>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
>> @@ -381,17 +381,20 @@ static struct xe_reg xe_hwmon_get_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg
>>    * same pattern for sysfs, allow arbitrary PL1 limits to be set but display
>>    * clamped values when read.
>>    */
>> -static void xe_hwmon_power_max_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *value)
>> +static int xe_hwmon_power_max_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *value)
>>   {
>>   	u32 reg_val = 0;
>>   	struct xe_device *xe = hwmon->xe;
>>   	struct xe_reg rapl_limit, pkg_power_sku;
>>   	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>> +	int ret = 0;
>>   
>>   	mutex_lock(&hwmon->hwmon_lock);
>>   
>>   	if (hwmon->xe->info.has_mbx_power_limits) {
>> -		xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &reg_val);
>> +		ret = xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &reg_val);
>> +		if (ret)
>> +			goto unlock;
>>   	} 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);
>> @@ -423,6 +426,7 @@ static void xe_hwmon_power_max_read(struct xe_hwmon *hwmon, u32 attr, int channe
>>   	}
>>   unlock:
>>   	mutex_unlock(&hwmon->hwmon_lock);
>> +	return ret;
>>   }
>>   
>>   static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channel, long value)
>> @@ -445,7 +449,9 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
>>   			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, &reg_val);
>> +			ret = xe_hwmon_pcode_read_power_limit(hwmon, attr, channel, &reg_val);
>> +			if (ret)
>> +				goto unlock;
>>   		} else {
>>   			reg_val = xe_mmio_rmw32(mmio, rapl_limit, PWR_LIM_EN, 0);
>>   			reg_val = xe_mmio_read32(mmio, rapl_limit);
>> @@ -502,15 +508,18 @@ static int xe_hwmon_power_max_write(struct xe_hwmon *hwmon, u32 attr, int channe
>>   	return ret;
>>   }
>>   
>> -static void xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int channel,
>> -					  long *value)
>> +static int xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int channel,
>> +					 long *value)
>>   {
>>   	struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
>>   	u32 reg_val;
>> +	int ret = 0;
>>   
>>   	if (hwmon->xe->info.has_mbx_power_limits) {
>>   		/* PL1 is rated max if supported. */
>> -		xe_hwmon_pcode_read_power_limit(hwmon, PL1_HWMON_ATTR, channel, &reg_val);
>> +		ret = xe_hwmon_pcode_read_power_limit(hwmon, PL1_HWMON_ATTR, channel, &reg_val);
>> +		if (ret)
>> +			return ret;
>>   	} else {
>>   		/*
>>   		 * This sysfs file won't be visible if REG_PKG_POWER_SKU is invalid, so valid check
>> @@ -524,6 +533,7 @@ static void xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int
>>   
>>   	reg_val = REG_FIELD_GET(PKG_TDP, reg_val);
>>   	*value = mul_u64_u32_shr(reg_val, SF_POWER, hwmon->scl_shift_power);
>> +	return ret;
>>   }
>>   
>>   /*
>> @@ -546,7 +556,7 @@ static void xe_hwmon_power_rated_max_read(struct xe_hwmon *hwmon, u32 attr, int
>>    * hwmon->scl_shift_energy of 14 bits we have 57 (63 - 20 + 14) bits before
>>    * energyN_input overflows. This at 1000 W is an overflow duration of 278 years.
>>    */
>> -static void
>> +static int
>>   xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
>>   {
>>   	struct xe_mmio *mmio = xe_root_tile_mmio(hwmon->xe);
>> @@ -557,7 +567,7 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
>>   	/* Energy is supported only for card and pkg */
>>   	if (channel > CHANNEL_PKG) {
>>   		*energy = 0;
>> -		return;
>> +		return 0;
>>   	}
>>   
>>   	if (hwmon->xe->info.platform == XE_BATTLEMAGE) {
>> @@ -569,7 +579,7 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
>>   		if (ret != sizeof(pmt_val)) {
>>   			drm_warn(&hwmon->xe->drm, "energy read from pmt failed, ret %d\n", ret);
>>   			*energy = 0;
>> -			return;
>> +			return ret < 0 ? ret : -EIO;
>>   		}
>>   
>>   		if (channel == CHANNEL_PKG)
>> @@ -586,6 +596,7 @@ xe_hwmon_energy_get(struct xe_hwmon *hwmon, int channel, long *energy)
>>   
>>   	*energy = mul_u64_u32_shr(ei->accum_energy, SF_ENERGY,
>>   				  hwmon->scl_shift_energy);
>> +	return 0;
>>   }
>>   
>>   static ssize_t
>> @@ -611,7 +622,8 @@ xe_hwmon_power_max_interval_show(struct device *dev, struct device_attribute *at
>>   			drm_err(&hwmon->xe->drm,
>>   				"power interval read fail, ch %d, attr %d, val 0x%08x, ret %d\n",
>>   				channel, power_attr, reg_val, ret);
>> -			reg_val = 0;
>> +			mutex_unlock(&hwmon->hwmon_lock);
>> +			return ret;
>>   		}
>>   	} else {
>>   		reg_val = xe_mmio_read32(mmio, xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT,
>> @@ -1391,7 +1403,8 @@ xe_hwmon_power_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>>   		}
>>   	case hwmon_power_crit:
>>   		if (channel == CHANNEL_CARD) {
>> -			xe_hwmon_pcode_read_i1(hwmon, &uval);
>> +			if (xe_hwmon_pcode_read_i1(hwmon, &uval))
>> +				return 0;
>>   			return (uval & POWER_SETUP_I1_WATTS) ? 0644 : 0;
>>   		}
>>   		break;
>> @@ -1410,7 +1423,8 @@ xe_hwmon_power_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
>>   			}
>>   		}
>>   		if ((!(uval & PWR_LIM_EN)) && channel == CHANNEL_CARD) {
>> -			xe_hwmon_pcode_read_i1(hwmon, &uval);
>> +			if (xe_hwmon_pcode_read_i1(hwmon, &uval))
>> +				return 0;
>>   			return (uval & POWER_SETUP_I1_WATTS) ? 0444 : 0;
>>   		}
>>   		return (uval) ? 0444 : 0;
>> @@ -1426,11 +1440,9 @@ xe_hwmon_power_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>>   	switch (attr) {
>>   	case hwmon_power_max:
>>   	case hwmon_power_cap:
>> -		xe_hwmon_power_max_read(hwmon, attr, channel, val);
>> -		return 0;
>> +		return xe_hwmon_power_max_read(hwmon, attr, channel, val);
>>   	case hwmon_power_rated_max:
>> -		xe_hwmon_power_rated_max_read(hwmon, attr, channel, val);
>> -		return 0;
>> +		return xe_hwmon_power_rated_max_read(hwmon, attr, channel, val);
>>   	case hwmon_power_crit:
>>   		return xe_hwmon_power_curr_crit_read(hwmon, channel, val, SF_POWER);
>>   	default:
>> @@ -1547,8 +1559,7 @@ xe_hwmon_energy_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
>>   {
>>   	switch (attr) {
>>   	case hwmon_energy_input:
>> -		xe_hwmon_energy_get(hwmon, channel, val);
>> -		return 0;
>> +		return xe_hwmon_energy_get(hwmon, channel, val);
>>   	default:
>>   		return -EOPNOTSUPP;
>>   	}
>> -- 
>> 2.25.1
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-13  2:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-12 11:06 [PATCH] drm/xe/hwmon: Propagate all hardware read errors Karthik Poosa
2026-10-13  0:59 ` Rodrigo Vivi
2026-10-13  2:20   ` Poosa, Karthik

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.