Re: [Intel-gfx] [PATCH v6 2/3] drm/i915/hwmon: Add hwmon support in libpower for dgfx

["Tauro, Riana" <riana.tauro@intel.com>]

On 12/7/2022 10:56 AM, Dixit, Ashutosh wrote:
> On Tue, 06 Dec 2022 21:17:46 -0800, Riana Tauro wrote:
>>
>> diff --git a/drivers/gpu/drm/i915/selftests/libpower.c b/drivers/gpu/drm/i915/selftests/libpower.c
>> index c66e993c5f85..3d4d2dc74a54 100644
>> --- a/drivers/gpu/drm/i915/selftests/libpower.c
>> +++ b/drivers/gpu/drm/i915/selftests/libpower.c
>> @@ -6,29 +6,28 @@
>>   #include <asm/msr.h>
>>
>>   #include "i915_drv.h"
>> +#include "i915_hwmon.h"
>>   #include "libpower.h"
>>
>> -bool libpower_supported(const struct drm_i915_private *i915)
>> -{
>> -	/* Discrete cards require hwmon integration */
>> -	if (IS_DGFX(i915))
>> -		return false;
>> -
>> -	return libpower_get_energy_uJ();
>> -}
>> -
>> -u64 libpower_get_energy_uJ(void)
>> +u64 libpower_get_energy_uJ(struct intel_gt *gt)
> 
> Hi Riana,
> 
> Sorry, we can't do this otherwise the build breaks at this commit which we
> can't do. Note that the callers of libpower_get_energy_uJ are still using
> the version without any args (see Patch 3). That is why I had R-b'd patch 3
> so that we don't have to do this. It's not really needed.
> 
> If you really want to do this we'll need to port the version without the gt
> arg to Patch 2 and then convert everything to the version with gt arg in
> Patch 3.
> 
> I really don't think it is worth it. Maybe just go back to the previous
> version of the series and modify Patch 2 and we'll be done.
> 
> Thanks.
> --
> Ashutosh

Hi Ashutosh

Sorry, didn't check the build after that patch. I'll revert back to 
previous version

Thanks
Riana
> 
> 
>>   {
>> 	unsigned long long power;
>> 	u32 units;
>> +	long energy_uJ = 0;
>>
>> -	if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power))
>> -		return 0;
>> +	if (IS_DGFX(gt->i915)) {
>> +		if (i915_hwmon_get_energy(gt, &energy_uJ))
>> +			return 0;
>> +	} else {
>> +		if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power))
>> +			return 0;
>>
>> -	units = (power & 0x1f00) >> 8;
>> +		units = (power & 0x1f00) >> 8;
>>
>> -	if (rdmsrl_safe(MSR_PP1_ENERGY_STATUS, &power))
>> -		return 0;
>> +		if (rdmsrl_safe(MSR_PP1_ENERGY_STATUS, &power))
>> +			return 0;
>>
>> -	return (1000000 * power) >> units; /* convert to uJ */
>> +		energy_uJ = (1000000 * power) >> units; /* convert to uJ */
>> +	}
>> +	return energy_uJ;
>>   }
>> diff --git a/drivers/gpu/drm/i915/selftests/libpower.h b/drivers/gpu/drm/i915/selftests/libpower.h
>> index 5352981eb946..e4410a886654 100644
>> --- a/drivers/gpu/drm/i915/selftests/libpower.h
>> +++ b/drivers/gpu/drm/i915/selftests/libpower.h
>> @@ -8,10 +8,12 @@
>>
>>   #include <linux/types.h>
>>
>> -struct drm_i915_private;
>> +struct intel_gt;
>>
>> -bool libpower_supported(const struct drm_i915_private *i915);
>> -
>> -u64 libpower_get_energy_uJ(void);
>> +u64 libpower_get_energy_uJ(struct intel_gt *gt);
>>
>> +static inline bool libpower_supported(struct intel_gt *gt)
>> +{
>> +	return libpower_get_energy_uJ(gt);
>> +}
>>   #endif /* SELFTEST_LIBPOWER_H */
>> --
>> 2.25.1
>>