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

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 29 Nov 2022 21:34:27 -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)
>  {
>	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);
> +}

I think it might have been preferable to make the libpower.* changes as
part of Patch 2 (so Patch 3 would be trivial) but probably doesn't matter
so this is also:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>