diff mbox series

[3/7] drm/i915/hwmon: Power PL1 limit and TDP setting

Message ID 20220916150054.807590-4-badal.nilawar@intel.com (mailing list archive)
State Handled Elsewhere
Headers show
Series drm/i915: Add HWMON support | expand

Commit Message

Nilawar, Badal Sept. 16, 2022, 3 p.m. UTC 
From: Dale B Stimson <dale.b.stimson@intel.com>

Use i915 HWMON to display/modify dGfx power PL1 limit and TDP setting.

v2:
  - Fix review comments (Ashutosh)
  - Do not restore power1_max upon module unload/load sequence
    because on production systems modules are always loaded
    and not unloaded/reloaded (Ashutosh)
  - Fix review comments (Jani)
  - Remove endianness conversion (Ashutosh)
v3: Add power1_rated_max (Ashutosh)
v4:
  - Use macro HWMON_CHANNEL_INFO to define power channel (Guenter)
  - Update the date and kernel version in Documentation (Badal)
v5: Use hwm_ prefix for static functions (Ashutosh)
v6:
  - Fix review comments (Ashutosh)
  - Update date, kernel version in documentation

Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Riana Tauro <riana.tauro@intel.com>
Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
Acked-by: Guenter Roeck <linux@roeck-us.net>
---
 .../ABI/testing/sysfs-driver-intel-i915-hwmon |  20 +++
 drivers/gpu/drm/i915/i915_hwmon.c             | 158 +++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h               |   5 +
 drivers/gpu/drm/i915/intel_mchbar_regs.h      |   6 +
 4 files changed, 187 insertions(+), 2 deletions(-)

Comments

Dixit, Ashutosh Sept. 21, 2022, 12:02 a.m. UTC | #1 
On Fri, 16 Sep 2022 08:00:50 -0700, Badal Nilawar wrote:
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> index e2974f928e58..bc061238e35c 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -5,3 +5,23 @@ Contact:	dri-devel@lists.freedesktop.org
>  Description:	RO. Current Voltage in millivolt.
>
>		Only supported for particular Intel i915 graphics platforms.
> +
> +What:		/sys/devices/.../hwmon/hwmon<i>/power1_max
> +Date:		September 2022
> +KernelVersion:	6

Maybe we should ask someone but even if we merge this today to drm-tip this
will appear in kernel.org Linus' version only in 6.2. So I think we should
set this as 6.2 on all patches.

Except for this, thanks for making the changes, this is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Tvrtko Ursulin Sept. 21, 2022, 11:44 a.m. UTC | #2 
On 21/09/2022 01:02, Dixit, Ashutosh wrote:
> On Fri, 16 Sep 2022 08:00:50 -0700, Badal Nilawar wrote:
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>> index e2974f928e58..bc061238e35c 100644
>> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>> @@ -5,3 +5,23 @@ Contact:	dri-devel@lists.freedesktop.org
>>   Description:	RO. Current Voltage in millivolt.
>>
>> 		Only supported for particular Intel i915 graphics platforms.
>> +
>> +What:		/sys/devices/.../hwmon/hwmon<i>/power1_max
>> +Date:		September 2022
>> +KernelVersion:	6
> 
> Maybe we should ask someone but even if we merge this today to drm-tip this
> will appear in kernel.org Linus' version only in 6.2. So I think we should
> set this as 6.2 on all patches.

Correct, if merged today it will appear in 6.2 so please change to that 
before merging.

As for the date that's harder to predict and I am not really sure how 
best to handle it. Crystal ball predicts February 2023 fwiw so maybe go 
with that for now. Seems less important than the release for me anyway.

Regards,

Tvrtko

> Except for this, thanks for making the changes, this is:
> 
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Gupta, Anshuman Sept. 21, 2022, 11:45 a.m. UTC | #3 
On 9/16/2022 8:30 PM, Badal Nilawar wrote:
> From: Dale B Stimson <dale.b.stimson@intel.com>
> 
> Use i915 HWMON to display/modify dGfx power PL1 limit and TDP setting.
> 
> v2:
>    - Fix review comments (Ashutosh)
>    - Do not restore power1_max upon module unload/load sequence
>      because on production systems modules are always loaded
>      and not unloaded/reloaded (Ashutosh)
>    - Fix review comments (Jani)
>    - Remove endianness conversion (Ashutosh)
> v3: Add power1_rated_max (Ashutosh)
> v4:
>    - Use macro HWMON_CHANNEL_INFO to define power channel (Guenter)
>    - Update the date and kernel version in Documentation (Badal)
> v5: Use hwm_ prefix for static functions (Ashutosh)
> v6:
>    - Fix review comments (Ashutosh)
>    - Update date, kernel version in documentation
> 
> Cc: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> ---
>   .../ABI/testing/sysfs-driver-intel-i915-hwmon |  20 +++
>   drivers/gpu/drm/i915/i915_hwmon.c             | 158 +++++++++++++++++-
>   drivers/gpu/drm/i915/i915_reg.h               |   5 +
>   drivers/gpu/drm/i915/intel_mchbar_regs.h      |   6 +
>   4 files changed, 187 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> index e2974f928e58..bc061238e35c 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
> @@ -5,3 +5,23 @@ Contact:	dri-devel@lists.freedesktop.org
>   Description:	RO. Current Voltage in millivolt.
>   
>   		Only supported for particular Intel i915 graphics platforms.
> +
> +What:		/sys/devices/.../hwmon/hwmon<i>/power1_max
> +Date:		September 2022
> +KernelVersion:	6
> +Contact:	dri-devel@lists.freedesktop.org
> +Description:	RW. Card reactive sustained  (PL1/Tau) power limit in microwatts.
> +
> +		The power controller will throttle the operating frequency
> +		if the power averaged over a window (typically seconds)
> +		exceeds this limit.
> +
> +		Only supported for particular Intel i915 graphics platforms.
> +
> +What:		/sys/devices/.../hwmon/hwmon<i>/power1_rated_max
> +Date:		September 2022
> +KernelVersion:	6
> +Contact:	dri-devel@lists.freedesktop.org
> +Description:	RO. Card default power limit (default TDP setting).
> +
> +		Only supported for particular Intel i915 graphics platforms.
> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
> index 45745afa5c5b..5183cf51a49b 100644
> --- a/drivers/gpu/drm/i915/i915_hwmon.c
> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
> @@ -16,11 +16,16 @@
>   /*
>    * SF_* - scale factors for particular quantities according to hwmon spec.
>    * - voltage  - millivolts
> + * - power  - microwatts
>    */
>   #define SF_VOLTAGE	1000
> +#define SF_POWER	1000000
>   
>   struct hwm_reg {
>   	i915_reg_t gt_perf_status;
> +	i915_reg_t pkg_power_sku_unit;
> +	i915_reg_t pkg_power_sku;
> +	i915_reg_t pkg_rapl_limit;
>   };
>   
>   struct hwm_drvdata {
> @@ -34,10 +39,68 @@ struct i915_hwmon {
>   	struct hwm_drvdata ddat;
>   	struct mutex hwmon_lock;		/* counter overflow logic and rmw */
>   	struct hwm_reg rg;
> +	int scl_shift_power;
>   };
>   
> +static void
> +hwm_locked_with_pm_intel_uncore_rmw(struct hwm_drvdata *ddat,
> +				    i915_reg_t reg, u32 clear, u32 set)
> +{
> +	struct i915_hwmon *hwmon = ddat->hwmon;
> +	struct intel_uncore *uncore = ddat->uncore;
> +	intel_wakeref_t wakeref;
> +
> +	mutex_lock(&hwmon->hwmon_lock);
> +
> +	with_intel_runtime_pm(uncore->rpm, wakeref)
> +		intel_uncore_rmw(uncore, reg, clear, set);
> +
> +	mutex_unlock(&hwmon->hwmon_lock);
> +}
> +
> +/*
> + * This function's return type of u64 allows for the case where the scaling
> + * of the field taken from the 32-bit register value might cause a result to
> + * exceed 32 bits.
> + */
> +static u64
> +hwm_field_read_and_scale(struct hwm_drvdata *ddat, i915_reg_t rgadr,
> +			 u32 field_msk, int nshift, u32 scale_factor)
> +{
> +	struct intel_uncore *uncore = ddat->uncore;
> +	intel_wakeref_t wakeref;
> +	u32 reg_value;
> +
> +	with_intel_runtime_pm(uncore->rpm, wakeref)
> +		reg_value = intel_uncore_read(uncore, rgadr);
> +
> +	reg_value = REG_FIELD_GET(field_msk, reg_value);
> +
> +	return mul_u64_u32_shr(reg_value, scale_factor, nshift);
> +}
> +
> +static void
> +hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
> +			  u32 field_msk, int nshift,
> +			  unsigned int scale_factor, long lval)
> +{
> +	u32 nval;
> +	u32 bits_to_clear;
> +	u32 bits_to_set;
> +
> +	/* Computation in 64-bits to avoid overflow. Round to nearest. */
> +	nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
> +
> +	bits_to_clear = field_msk;
> +	bits_to_set = FIELD_PREP(field_msk, nval);
> +
> +	hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
> +					    bits_to_clear, bits_to_set);
> +}
> +
>   static const struct hwmon_channel_info *hwm_info[] = {
>   	HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
> +	HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX),
>   	NULL
>   };
>   
> @@ -71,6 +134,64 @@ hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val)
>   	}
>   }
>   
> +static umode_t
> +hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int chan)
> +{
> +	struct i915_hwmon *hwmon = ddat->hwmon;
> +
> +	switch (attr) {
> +	case hwmon_power_max:
> +		return i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit) ? 0664 : 0;
> +	case hwmon_power_rated_max:
> +		return i915_mmio_reg_valid(hwmon->rg.pkg_power_sku) ? 0444 : 0;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static int
> +hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
> +{
> +	struct i915_hwmon *hwmon = ddat->hwmon;
> +
> +	switch (attr) {
> +	case hwmon_power_max:
> +		*val = hwm_field_read_and_scale(ddat,
> +						hwmon->rg.pkg_rapl_limit,
> +						PKG_PWR_LIM_1,
> +						hwmon->scl_shift_power,
> +						SF_POWER);
> +		return 0;
> +	case hwmon_power_rated_max:
> +		*val = hwm_field_read_and_scale(ddat,
> +						hwmon->rg.pkg_power_sku,
> +						PKG_PKG_TDP,It seems a dead code, pkg_power_sky register in initialized with 
INVALID_MMMIO_REG, why are we exposing this, unless i am missing something ?
Br,
Anshuman.
> +						hwmon->scl_shift_power,
> +						SF_POWER);
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int
> +hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
> +{
> +	struct i915_hwmon *hwmon = ddat->hwmon;
> +
> +	switch (attr) {
> +	case hwmon_power_max:
> +		hwm_field_scale_and_write(ddat,
> +					  hwmon->rg.pkg_rapl_limit,
> +					  PKG_PWR_LIM_1,
> +					  hwmon->scl_shift_power,
> +					  SF_POWER, val);
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
>   static umode_t
>   hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
>   	       u32 attr, int channel)
> @@ -80,6 +201,8 @@ hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
>   	switch (type) {
>   	case hwmon_in:
>   		return hwm_in_is_visible(ddat, attr);
> +	case hwmon_power:
> +		return hwm_power_is_visible(ddat, attr, channel);
>   	default:
>   		return 0;
>   	}
> @@ -94,6 +217,8 @@ hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>   	switch (type) {
>   	case hwmon_in:
>   		return hwm_in_read(ddat, attr, val);
> +	case hwmon_power:
> +		return hwm_power_read(ddat, attr, channel, val);
>   	default:
>   		return -EOPNOTSUPP;
>   	}
> @@ -103,7 +228,11 @@ static int
>   hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>   	  int channel, long val)
>   {
> +	struct hwm_drvdata *ddat = dev_get_drvdata(dev);
> +
>   	switch (type) {
> +	case hwmon_power:
> +		return hwm_power_write(ddat, attr, channel, val);
>   	default:
>   		return -EOPNOTSUPP;
>   	}
> @@ -124,11 +253,36 @@ static void
>   hwm_get_preregistration_info(struct drm_i915_private *i915)
>   {
>   	struct i915_hwmon *hwmon = i915->hwmon;
> +	struct intel_uncore *uncore = &i915->uncore;
> +	intel_wakeref_t wakeref;
> +	u32 val_sku_unit;
>   
> -	if (IS_DG1(i915) || IS_DG2(i915))
> +	if (IS_DG1(i915) || IS_DG2(i915)) {
>   		hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
> -	else
> +		hwmon->rg.pkg_power_sku_unit = PCU_PACKAGE_POWER_SKU_UNIT;
> +		hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> +		hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
> +	} else {
>   		hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
> +		hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
> +		hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
> +		hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
> +	}
> +
> +	with_intel_runtime_pm(uncore->rpm, wakeref) {
> +		/*
> +		 * The contents of register hwmon->rg.pkg_power_sku_unit do not change,
> +		 * so read it once and store the shift values.
> +		 */
> +		if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku_unit)) {
> +			val_sku_unit = intel_uncore_read(uncore,
> +							 hwmon->rg.pkg_power_sku_unit);
> +		} else {
> +			val_sku_unit = 0;
> +		}
> +
> +		hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, val_sku_unit);
> +	}
>   }
>   
>   void i915_hwmon_register(struct drm_i915_private *i915)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1a9bd829fc7e..55c35903adca 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1807,6 +1807,11 @@
>   #define   POWER_LIMIT_1_MASK		REG_BIT(10)
>   #define   POWER_LIMIT_2_MASK		REG_BIT(11)
>   
> +/*
> + * *_PACKAGE_POWER_SKU - SKU power and timing parameters.
> + */
> +#define   PKG_PKG_TDP			GENMASK_ULL(14, 0)
> +
>   #define CHV_CLK_CTL1			_MMIO(0x101100)
>   #define VLV_CLK_CTL2			_MMIO(0x101104)
>   #define   CLK_CTL2_CZCOUNT_30NS_SHIFT	28
> diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> index ffc702b79579..b74df11977c6 100644
> --- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
> +++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
> @@ -189,6 +189,10 @@
>   #define  DG1_QCLK_RATIO_MASK			REG_GENMASK(9, 2)
>   #define  DG1_QCLK_REFERENCE			REG_BIT(10)
>   
> +#define PCU_PACKAGE_POWER_SKU_UNIT		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5938)
> +#define   PKG_PWR_UNIT				REG_GENMASK(3, 0)
> +#define   PKG_TIME_UNIT				REG_GENMASK(19, 16)
> +
>   #define GEN6_GT_PERF_STATUS			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5948)
>   #define GEN6_RP_STATE_LIMITS			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
>   #define GEN6_RP_STATE_CAP			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5998)
> @@ -198,6 +202,8 @@
>   
>   #define GEN10_FREQ_INFO_REC			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5ef0)
>   #define   RPE_MASK				REG_GENMASK(15, 8)
> +#define PCU_PACKAGE_RAPL_LIMIT			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
> +#define   PKG_PWR_LIM_1				REG_GENMASK(14, 0)
>   
>   /* snb MCH registers for priority tuning */
>   #define MCH_SSKPD				_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
Nilawar, Badal Sept. 21, 2022, 2:53 p.m. UTC | #4 
On 21-09-2022 17:15, Gupta, Anshuman wrote:
> 
> 
> On 9/16/2022 8:30 PM, Badal Nilawar wrote:
>> From: Dale B Stimson <dale.b.stimson@intel.com>
>>
>> Use i915 HWMON to display/modify dGfx power PL1 limit and TDP setting.
>>
>> v2:
>>    - Fix review comments (Ashutosh)
>>    - Do not restore power1_max upon module unload/load sequence
>>      because on production systems modules are always loaded
>>      and not unloaded/reloaded (Ashutosh)
>>    - Fix review comments (Jani)
>>    - Remove endianness conversion (Ashutosh)
>> v3: Add power1_rated_max (Ashutosh)
>> v4:
>>    - Use macro HWMON_CHANNEL_INFO to define power channel (Guenter)
>>    - Update the date and kernel version in Documentation (Badal)
>> v5: Use hwm_ prefix for static functions (Ashutosh)
>> v6:
>>    - Fix review comments (Ashutosh)
>>    - Update date, kernel version in documentation
>>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
>> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>> Acked-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>>   .../ABI/testing/sysfs-driver-intel-i915-hwmon |  20 +++
>>   drivers/gpu/drm/i915/i915_hwmon.c             | 158 +++++++++++++++++-
>>   drivers/gpu/drm/i915/i915_reg.h               |   5 +
>>   drivers/gpu/drm/i915/intel_mchbar_regs.h      |   6 +
>>   4 files changed, 187 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon 
>> b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>> index e2974f928e58..bc061238e35c 100644
>> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>> @@ -5,3 +5,23 @@ Contact:    dri-devel@lists.freedesktop.org
>>   Description:    RO. Current Voltage in millivolt.
>>           Only supported for particular Intel i915 graphics platforms.
>> +
>> +What:        /sys/devices/.../hwmon/hwmon<i>/power1_max
>> +Date:        September 2022
>> +KernelVersion:    6
>> +Contact:    dri-devel@lists.freedesktop.org
>> +Description:    RW. Card reactive sustained  (PL1/Tau) power limit in 
>> microwatts.
>> +
>> +        The power controller will throttle the operating frequency
>> +        if the power averaged over a window (typically seconds)
>> +        exceeds this limit.
>> +
>> +        Only supported for particular Intel i915 graphics platforms.
>> +
>> +What:        /sys/devices/.../hwmon/hwmon<i>/power1_rated_max
>> +Date:        September 2022
>> +KernelVersion:    6
>> +Contact:    dri-devel@lists.freedesktop.org
>> +Description:    RO. Card default power limit (default TDP setting).
>> +
>> +        Only supported for particular Intel i915 graphics platforms.
>> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c 
>> b/drivers/gpu/drm/i915/i915_hwmon.c
>> index 45745afa5c5b..5183cf51a49b 100644
>> --- a/drivers/gpu/drm/i915/i915_hwmon.c
>> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
>> @@ -16,11 +16,16 @@
>>   /*
>>    * SF_* - scale factors for particular quantities according to hwmon 
>> spec.
>>    * - voltage  - millivolts
>> + * - power  - microwatts
>>    */
>>   #define SF_VOLTAGE    1000
>> +#define SF_POWER    1000000
>>   struct hwm_reg {
>>       i915_reg_t gt_perf_status;
>> +    i915_reg_t pkg_power_sku_unit;
>> +    i915_reg_t pkg_power_sku;
>> +    i915_reg_t pkg_rapl_limit;
>>   };
>>   struct hwm_drvdata {
>> @@ -34,10 +39,68 @@ struct i915_hwmon {
>>       struct hwm_drvdata ddat;
>>       struct mutex hwmon_lock;        /* counter overflow logic and 
>> rmw */
>>       struct hwm_reg rg;
>> +    int scl_shift_power;
>>   };
>> +static void
>> +hwm_locked_with_pm_intel_uncore_rmw(struct hwm_drvdata *ddat,
>> +                    i915_reg_t reg, u32 clear, u32 set)
>> +{
>> +    struct i915_hwmon *hwmon = ddat->hwmon;
>> +    struct intel_uncore *uncore = ddat->uncore;
>> +    intel_wakeref_t wakeref;
>> +
>> +    mutex_lock(&hwmon->hwmon_lock);
>> +
>> +    with_intel_runtime_pm(uncore->rpm, wakeref)
>> +        intel_uncore_rmw(uncore, reg, clear, set);
>> +
>> +    mutex_unlock(&hwmon->hwmon_lock);
>> +}
>> +
>> +/*
>> + * This function's return type of u64 allows for the case where the 
>> scaling
>> + * of the field taken from the 32-bit register value might cause a 
>> result to
>> + * exceed 32 bits.
>> + */
>> +static u64
>> +hwm_field_read_and_scale(struct hwm_drvdata *ddat, i915_reg_t rgadr,
>> +             u32 field_msk, int nshift, u32 scale_factor)
>> +{
>> +    struct intel_uncore *uncore = ddat->uncore;
>> +    intel_wakeref_t wakeref;
>> +    u32 reg_value;
>> +
>> +    with_intel_runtime_pm(uncore->rpm, wakeref)
>> +        reg_value = intel_uncore_read(uncore, rgadr);
>> +
>> +    reg_value = REG_FIELD_GET(field_msk, reg_value);
>> +
>> +    return mul_u64_u32_shr(reg_value, scale_factor, nshift);
>> +}
>> +
>> +static void
>> +hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
>> +              u32 field_msk, int nshift,
>> +              unsigned int scale_factor, long lval)
>> +{
>> +    u32 nval;
>> +    u32 bits_to_clear;
>> +    u32 bits_to_set;
>> +
>> +    /* Computation in 64-bits to avoid overflow. Round to nearest. */
>> +    nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
>> +
>> +    bits_to_clear = field_msk;
>> +    bits_to_set = FIELD_PREP(field_msk, nval);
>> +
>> +    hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
>> +                        bits_to_clear, bits_to_set);
>> +}
>> +
>>   static const struct hwmon_channel_info *hwm_info[] = {
>>       HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
>> +    HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX),
>>       NULL
>>   };
>> @@ -71,6 +134,64 @@ hwm_in_read(struct hwm_drvdata *ddat, u32 attr, 
>> long *val)
>>       }
>>   }
>> +static umode_t
>> +hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int chan)
>> +{
>> +    struct i915_hwmon *hwmon = ddat->hwmon;
>> +
>> +    switch (attr) {
>> +    case hwmon_power_max:
>> +        return i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit) ? 0664 : 0;
>> +    case hwmon_power_rated_max:
>> +        return i915_mmio_reg_valid(hwmon->rg.pkg_power_sku) ? 0444 : 0;
>> +    default:
>> +        return 0;
>> +    }
>> +}
>> +
>> +static int
>> +hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
>> +{
>> +    struct i915_hwmon *hwmon = ddat->hwmon;
>> +
>> +    switch (attr) {
>> +    case hwmon_power_max:
>> +        *val = hwm_field_read_and_scale(ddat,
>> +                        hwmon->rg.pkg_rapl_limit,
>> +                        PKG_PWR_LIM_1,
>> +                        hwmon->scl_shift_power,
>> +                        SF_POWER);
>> +        return 0;
>> +    case hwmon_power_rated_max:
>> +        *val = hwm_field_read_and_scale(ddat,
>> +                        hwmon->rg.pkg_power_sku,
>> +                        PKG_PKG_TDP,It seems a dead code, 
>> pkg_power_sky register in initialized with 
> INVALID_MMMIO_REG, why are we exposing this, unless i am missing 
> something ?
Agree that for platforms considered in this series does not support 
hwmon_power_rated_max. In fact hwm_power_is_visible will not allow to 
create sysfs entry if pkg_power_sku is not supported. Considering future 
dgfx platforms we didn't remove this entry. In future for supported 
platforms we just need to assign valid register to pkg_power_sku.

Regards,
Badal
> Br,
> Anshuman.
>> +                        hwmon->scl_shift_power,
>> +                        SF_POWER);
>> +        return 0;
>> +    default:
>> +        return -EOPNOTSUPP;
>> +    }
>> +}
>> +
>> +static int
>> +hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
>> +{
>> +    struct i915_hwmon *hwmon = ddat->hwmon;
>> +
>> +    switch (attr) {
>> +    case hwmon_power_max:
>> +        hwm_field_scale_and_write(ddat,
>> +                      hwmon->rg.pkg_rapl_limit,
>> +                      PKG_PWR_LIM_1,
>> +                      hwmon->scl_shift_power,
>> +                      SF_POWER, val);
>> +        return 0;
>> +    default:
>> +        return -EOPNOTSUPP;
>> +    }
>> +}
>> +
>>   static umode_t
>>   hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
>>              u32 attr, int channel)
>> @@ -80,6 +201,8 @@ hwm_is_visible(const void *drvdata, enum 
>> hwmon_sensor_types type,
>>       switch (type) {
>>       case hwmon_in:
>>           return hwm_in_is_visible(ddat, attr);
>> +    case hwmon_power:
>> +        return hwm_power_is_visible(ddat, attr, channel);
>>       default:
>>           return 0;
>>       }
>> @@ -94,6 +217,8 @@ hwm_read(struct device *dev, enum 
>> hwmon_sensor_types type, u32 attr,
>>       switch (type) {
>>       case hwmon_in:
>>           return hwm_in_read(ddat, attr, val);
>> +    case hwmon_power:
>> +        return hwm_power_read(ddat, attr, channel, val);
>>       default:
>>           return -EOPNOTSUPP;
>>       }
>> @@ -103,7 +228,11 @@ static int
>>   hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>>         int channel, long val)
>>   {
>> +    struct hwm_drvdata *ddat = dev_get_drvdata(dev);
>> +
>>       switch (type) {
>> +    case hwmon_power:
>> +        return hwm_power_write(ddat, attr, channel, val);
>>       default:
>>           return -EOPNOTSUPP;
>>       }
>> @@ -124,11 +253,36 @@ static void
>>   hwm_get_preregistration_info(struct drm_i915_private *i915)
>>   {
>>       struct i915_hwmon *hwmon = i915->hwmon;
>> +    struct intel_uncore *uncore = &i915->uncore;
>> +    intel_wakeref_t wakeref;
>> +    u32 val_sku_unit;
>> -    if (IS_DG1(i915) || IS_DG2(i915))
>> +    if (IS_DG1(i915) || IS_DG2(i915)) {
>>           hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
>> -    else
>> +        hwmon->rg.pkg_power_sku_unit = PCU_PACKAGE_POWER_SKU_UNIT;
>> +        hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
>> +        hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
>> +    } else {
>>           hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
>> +        hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
>> +        hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
>> +        hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
>> +    }
>> +
>> +    with_intel_runtime_pm(uncore->rpm, wakeref) {
>> +        /*
>> +         * The contents of register hwmon->rg.pkg_power_sku_unit do 
>> not change,
>> +         * so read it once and store the shift values.
>> +         */
>> +        if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku_unit)) {
>> +            val_sku_unit = intel_uncore_read(uncore,
>> +                             hwmon->rg.pkg_power_sku_unit);
>> +        } else {
>> +            val_sku_unit = 0;
>> +        }
>> +
>> +        hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, 
>> val_sku_unit);
>> +    }
>>   }
>>   void i915_hwmon_register(struct drm_i915_private *i915)
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 1a9bd829fc7e..55c35903adca 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1807,6 +1807,11 @@
>>   #define   POWER_LIMIT_1_MASK        REG_BIT(10)
>>   #define   POWER_LIMIT_2_MASK        REG_BIT(11)
>> +/*
>> + * *_PACKAGE_POWER_SKU - SKU power and timing parameters.
>> + */
>> +#define   PKG_PKG_TDP            GENMASK_ULL(14, 0)
>> +
>>   #define CHV_CLK_CTL1            _MMIO(0x101100)
>>   #define VLV_CLK_CTL2            _MMIO(0x101104)
>>   #define   CLK_CTL2_CZCOUNT_30NS_SHIFT    28
>> diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h 
>> b/drivers/gpu/drm/i915/intel_mchbar_regs.h
>> index ffc702b79579..b74df11977c6 100644
>> --- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
>> +++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
>> @@ -189,6 +189,10 @@
>>   #define  DG1_QCLK_RATIO_MASK            REG_GENMASK(9, 2)
>>   #define  DG1_QCLK_REFERENCE            REG_BIT(10)
>> +#define PCU_PACKAGE_POWER_SKU_UNIT        
>> _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5938)
>> +#define   PKG_PWR_UNIT                REG_GENMASK(3, 0)
>> +#define   PKG_TIME_UNIT                REG_GENMASK(19, 16)
>> +
>>   #define GEN6_GT_PERF_STATUS            _MMIO(MCHBAR_MIRROR_BASE_SNB 
>> + 0x5948)
>>   #define GEN6_RP_STATE_LIMITS            _MMIO(MCHBAR_MIRROR_BASE_SNB 
>> + 0x5994)
>>   #define GEN6_RP_STATE_CAP            _MMIO(MCHBAR_MIRROR_BASE_SNB + 
>> 0x5998)
>> @@ -198,6 +202,8 @@
>>   #define GEN10_FREQ_INFO_REC            _MMIO(MCHBAR_MIRROR_BASE_SNB 
>> + 0x5ef0)
>>   #define   RPE_MASK                REG_GENMASK(15, 8)
>> +#define PCU_PACKAGE_RAPL_LIMIT            
>> _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
>> +#define   PKG_PWR_LIM_1                REG_GENMASK(14, 0)
>>   /* snb MCH registers for priority tuning */
>>   #define MCH_SSKPD                _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
Gupta, Anshuman Sept. 22, 2022, 7:08 a.m. UTC | #5 
On 9/21/2022 8:23 PM, Nilawar, Badal wrote:
> 
> 
> On 21-09-2022 17:15, Gupta, Anshuman wrote:
>>
>>
>> On 9/16/2022 8:30 PM, Badal Nilawar wrote:
>>> From: Dale B Stimson <dale.b.stimson@intel.com>
>>>
>>> Use i915 HWMON to display/modify dGfx power PL1 limit and TDP setting.
>>>
>>> v2:
>>>    - Fix review comments (Ashutosh)
>>>    - Do not restore power1_max upon module unload/load sequence
>>>      because on production systems modules are always loaded
>>>      and not unloaded/reloaded (Ashutosh)
>>>    - Fix review comments (Jani)
>>>    - Remove endianness conversion (Ashutosh)
>>> v3: Add power1_rated_max (Ashutosh)
>>> v4:
>>>    - Use macro HWMON_CHANNEL_INFO to define power channel (Guenter)
>>>    - Update the date and kernel version in Documentation (Badal)
>>> v5: Use hwm_ prefix for static functions (Ashutosh)
>>> v6:
>>>    - Fix review comments (Ashutosh)
>>>    - Update date, kernel version in documentation
>>>
>>> Cc: Guenter Roeck <linux@roeck-us.net>
>>> Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com>
>>> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>> Acked-by: Guenter Roeck <linux@roeck-us.net>
>>> ---
>>>   .../ABI/testing/sysfs-driver-intel-i915-hwmon |  20 +++
>>>   drivers/gpu/drm/i915/i915_hwmon.c             | 158 +++++++++++++++++-
>>>   drivers/gpu/drm/i915/i915_reg.h               |   5 +
>>>   drivers/gpu/drm/i915/intel_mchbar_regs.h      |   6 +
>>>   4 files changed, 187 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon 
>>> b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>>> index e2974f928e58..bc061238e35c 100644
>>> --- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>>> +++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
>>> @@ -5,3 +5,23 @@ Contact:    dri-devel@lists.freedesktop.org
>>>   Description:    RO. Current Voltage in millivolt.
>>>           Only supported for particular Intel i915 graphics platforms.
>>> +
>>> +What:        /sys/devices/.../hwmon/hwmon<i>/power1_max
>>> +Date:        September 2022
>>> +KernelVersion:    6
>>> +Contact:    dri-devel@lists.freedesktop.org
>>> +Description:    RW. Card reactive sustained  (PL1/Tau) power limit 
>>> in microwatts.
>>> +
>>> +        The power controller will throttle the operating frequency
>>> +        if the power averaged over a window (typically seconds)
>>> +        exceeds this limit.
>>> +
>>> +        Only supported for particular Intel i915 graphics platforms.
>>> +
>>> +What:        /sys/devices/.../hwmon/hwmon<i>/power1_rated_max
>>> +Date:        September 2022
>>> +KernelVersion:    6
>>> +Contact:    dri-devel@lists.freedesktop.org
>>> +Description:    RO. Card default power limit (default TDP setting).
>>> +
>>> +        Only supported for particular Intel i915 graphics platforms.
>>> diff --git a/drivers/gpu/drm/i915/i915_hwmon.c 
>>> b/drivers/gpu/drm/i915/i915_hwmon.c
>>> index 45745afa5c5b..5183cf51a49b 100644
>>> --- a/drivers/gpu/drm/i915/i915_hwmon.c
>>> +++ b/drivers/gpu/drm/i915/i915_hwmon.c
>>> @@ -16,11 +16,16 @@
>>>   /*
>>>    * SF_* - scale factors for particular quantities according to 
>>> hwmon spec.
>>>    * - voltage  - millivolts
>>> + * - power  - microwatts
>>>    */
>>>   #define SF_VOLTAGE    1000
>>> +#define SF_POWER    1000000
>>>   struct hwm_reg {
>>>       i915_reg_t gt_perf_status;
>>> +    i915_reg_t pkg_power_sku_unit;
>>> +    i915_reg_t pkg_power_sku;
>>> +    i915_reg_t pkg_rapl_limit;
>>>   };
>>>   struct hwm_drvdata {
>>> @@ -34,10 +39,68 @@ struct i915_hwmon {
>>>       struct hwm_drvdata ddat;
>>>       struct mutex hwmon_lock;        /* counter overflow logic and 
>>> rmw */
>>>       struct hwm_reg rg;
>>> +    int scl_shift_power;
>>>   };
>>> +static void
>>> +hwm_locked_with_pm_intel_uncore_rmw(struct hwm_drvdata *ddat,
>>> +                    i915_reg_t reg, u32 clear, u32 set)
>>> +{
>>> +    struct i915_hwmon *hwmon = ddat->hwmon;
>>> +    struct intel_uncore *uncore = ddat->uncore;
>>> +    intel_wakeref_t wakeref;
>>> +
>>> +    mutex_lock(&hwmon->hwmon_lock);
>>> +
>>> +    with_intel_runtime_pm(uncore->rpm, wakeref)
>>> +        intel_uncore_rmw(uncore, reg, clear, set);
>>> +
>>> +    mutex_unlock(&hwmon->hwmon_lock);
>>> +}
>>> +
>>> +/*
>>> + * This function's return type of u64 allows for the case where the 
>>> scaling
>>> + * of the field taken from the 32-bit register value might cause a 
>>> result to
>>> + * exceed 32 bits.
>>> + */
>>> +static u64
>>> +hwm_field_read_and_scale(struct hwm_drvdata *ddat, i915_reg_t rgadr,
>>> +             u32 field_msk, int nshift, u32 scale_factor)
>>> +{
>>> +    struct intel_uncore *uncore = ddat->uncore;
>>> +    intel_wakeref_t wakeref;
>>> +    u32 reg_value;
>>> +
>>> +    with_intel_runtime_pm(uncore->rpm, wakeref)
>>> +        reg_value = intel_uncore_read(uncore, rgadr);
>>> +
>>> +    reg_value = REG_FIELD_GET(field_msk, reg_value);
>>> +
>>> +    return mul_u64_u32_shr(reg_value, scale_factor, nshift);
>>> +}
>>> +
>>> +static void
>>> +hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
>>> +              u32 field_msk, int nshift,
>>> +              unsigned int scale_factor, long lval)
>>> +{
>>> +    u32 nval;
>>> +    u32 bits_to_clear;
>>> +    u32 bits_to_set;
>>> +
>>> +    /* Computation in 64-bits to avoid overflow. Round to nearest. */
>>> +    nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
>>> +
>>> +    bits_to_clear = field_msk;
>>> +    bits_to_set = FIELD_PREP(field_msk, nval);
>>> +
>>> +    hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
>>> +                        bits_to_clear, bits_to_set);
>>> +}
>>> +
>>>   static const struct hwmon_channel_info *hwm_info[] = {
>>>       HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
>>> +    HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX),
>>>       NULL
>>>   };
>>> @@ -71,6 +134,64 @@ hwm_in_read(struct hwm_drvdata *ddat, u32 attr, 
>>> long *val)
>>>       }
>>>   }
>>> +static umode_t
>>> +hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int 
>>> chan)
>>> +{
>>> +    struct i915_hwmon *hwmon = ddat->hwmon;
>>> +
>>> +    switch (attr) {
>>> +    case hwmon_power_max:
>>> +        return i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit) ? 0664 
>>> : 0;
>>> +    case hwmon_power_rated_max:
>>> +        return i915_mmio_reg_valid(hwmon->rg.pkg_power_sku) ? 0444 : 0;
>>> +    default:
>>> +        return 0;
>>> +    }
>>> +}
>>> +
>>> +static int
>>> +hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
>>> +{
>>> +    struct i915_hwmon *hwmon = ddat->hwmon;
>>> +
>>> +    switch (attr) {
>>> +    case hwmon_power_max:
>>> +        *val = hwm_field_read_and_scale(ddat,
>>> +                        hwmon->rg.pkg_rapl_limit,
>>> +                        PKG_PWR_LIM_1,
>>> +                        hwmon->scl_shift_power,
>>> +                        SF_POWER);
>>> +        return 0;
>>> +    case hwmon_power_rated_max:
>>> +        *val = hwm_field_read_and_scale(ddat,
>>> +                        hwmon->rg.pkg_power_sku,
>>> +                        PKG_PKG_TDP,It seems a dead code, 
>>> pkg_power_sky register in initialized with 
>> INVALID_MMMIO_REG, why are we exposing this, unless i am missing 
>> something ?
> Agree that for platforms considered in this series does not support 
> hwmon_power_rated_max. In fact hwm_power_is_visible will not allow to 
> create sysfs entry if pkg_power_sku is not supported. Considering future 
> dgfx platforms we didn't remove this entry. In future for supported 
> platforms we just need to assign valid register to pkg_power_sku.
AFAIU PACKAGE_POWER_SKU reg is valid for both DG1 and DG2 from BSpec:51862
So we need to define the register.
See once more comment below,
> 
> Regards,
> Badal
>> Br,
>> Anshuman.
>>> +                        hwmon->scl_shift_power,
>>> +                        SF_POWER);
>>> +        return 0;
>>> +    default:
>>> +        return -EOPNOTSUPP;
>>> +    }
>>> +}
>>> +
>>> +static int
/snip
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index 1a9bd829fc7e..55c35903adca 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -1807,6 +1807,11 @@
>>>   #define   POWER_LIMIT_1_MASK        REG_BIT(10)
>>>   #define   POWER_LIMIT_2_MASK        REG_BIT(11)
>>> +/*
>>> + * *_PACKAGE_POWER_SKU - SKU power and timing parameters.
>>> + */
>>> +#define   PKG_PKG_TDP            GENMASK_ULL(14, 0)
Define register above this definition, GENMASK should follow
by a register.
Br,
Anshuman.
>>> +
>>>   #define CHV_CLK_CTL1            _MMIO(0x101100)
>>>   #define VLV_CLK_CTL2            _MMIO(0x101104)
>>>   #define   CLK_CTL2_CZCOUNT_30NS_SHIFT    28
>>> diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h 
>>> b/drivers/gpu/drm/i915/intel_mchbar_regs.h
>>> index ffc702b79579..b74df11977c6 100644
>>> --- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
>>> +++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
>>> @@ -189,6 +189,10 @@
>>>   #define  DG1_QCLK_RATIO_MASK            REG_GENMASK(9, 2)
>>>   #define  DG1_QCLK_REFERENCE            REG_BIT(10)
>>> +#define PCU_PACKAGE_POWER_SKU_UNIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 
>>> 0x5938)
>>> +#define   PKG_PWR_UNIT                REG_GENMASK(3, 0)
>>> +#define   PKG_TIME_UNIT                REG_GENMASK(19, 16)
>>> +
>>>   #define GEN6_GT_PERF_STATUS            _MMIO(MCHBAR_MIRROR_BASE_SNB 
>>> + 0x5948)
>>>   #define GEN6_RP_STATE_LIMITS            
>>> _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
>>>   #define GEN6_RP_STATE_CAP            _MMIO(MCHBAR_MIRROR_BASE_SNB + 
>>> 0x5998)
>>> @@ -198,6 +202,8 @@
>>>   #define GEN10_FREQ_INFO_REC            _MMIO(MCHBAR_MIRROR_BASE_SNB 
>>> + 0x5ef0)
>>>   #define   RPE_MASK                REG_GENMASK(15, 8)
>>> +#define PCU_PACKAGE_RAPL_LIMIT _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
>>> +#define   PKG_PWR_LIM_1                REG_GENMASK(14, 0)
>>>   /* snb MCH registers for priority tuning */
>>>   #define MCH_SSKPD                _MMIO(MCHBAR_MIRROR_BASE_SNB + 
>>> 0x5d10)
Dixit, Ashutosh Sept. 23, 2022, 2:26 a.m. UTC | #6 
On Thu, 22 Sep 2022 00:08:46 -0700, Gupta, Anshuman wrote:
>

Hi Anshuman,

> On 9/21/2022 8:23 PM, Nilawar, Badal wrote:
> >
> > On 21-09-2022 17:15, Gupta, Anshuman wrote:
> >>
> >>> +static int
> >>> +hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
> >>> +{
> >>> +    struct i915_hwmon *hwmon = ddat->hwmon;
> >>> +
> >>> +    switch (attr) {
> >>> +    case hwmon_power_max:
> >>> +        *val = hwm_field_read_and_scale(ddat,
> >>> +                        hwmon->rg.pkg_rapl_limit,
> >>> +                        PKG_PWR_LIM_1,
> >>> +                        hwmon->scl_shift_power,
> >>> +                        SF_POWER);
> >>> +        return 0;
> >>> +    case hwmon_power_rated_max:
> >>> +        *val = hwm_field_read_and_scale(ddat,
> >>> +                        hwmon->rg.pkg_power_sku,
> >>> +                        PKG_PKG_TDP,It seems a dead code,
> >>> pkg_power_sky register in initialized with
> >> INVALID_MMMIO_REG, why are we exposing this, unless i am missing
> >> something ?
> > Agree that for platforms considered in this series does not support
> > hwmon_power_rated_max. In fact hwm_power_is_visible will not allow to
> > create sysfs entry if pkg_power_sku is not supported. Considering future
> > dgfx platforms we didn't remove this entry. In future for supported
> > platforms we just need to assign valid register to pkg_power_sku.
>
> AFAIU PACKAGE_POWER_SKU reg is valid for both DG1 and DG2 from BSpec:51862
> So we need to define the register.
> See once more comment below,

Thanks for pointing out, I didn't know where to look for it. We will add
it. Thanks to Badal for locating the register too.

> >
> > Regards,
> > Badal
> >> Br,
> >> Anshuman.
> >>> +                        hwmon->scl_shift_power,
> >>> +                        SF_POWER);
> >>> +        return 0;
> >>> +    default:
> >>> +        return -EOPNOTSUPP;
> >>> +    }
> >>> +}
> >>> +
> >>> +static int
> /snip
> >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >>> b/drivers/gpu/drm/i915/i915_reg.h
> >>> index 1a9bd829fc7e..55c35903adca 100644
> >>> --- a/drivers/gpu/drm/i915/i915_reg.h
> >>> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >>> @@ -1807,6 +1807,11 @@
> >>>   #define   POWER_LIMIT_1_MASK        REG_BIT(10)
> >>>   #define   POWER_LIMIT_2_MASK        REG_BIT(11)
> >>> +/*
> >>> + * *_PACKAGE_POWER_SKU - SKU power and timing parameters.
> >>> + */
> >>> +#define   PKG_PKG_TDP            GENMASK_ULL(14, 0)
> Define register above this definition, GENMASK should follow
> by a register.

Will do.

Thanks.
--
Ashutosh
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
index e2974f928e58..bc061238e35c 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
+++ b/Documentation/ABI/testing/sysfs-driver-intel-i915-hwmon
@@ -5,3 +5,23 @@  Contact:	dri-devel@lists.freedesktop.org
 Description:	RO. Current Voltage in millivolt.
 
 		Only supported for particular Intel i915 graphics platforms.
+
+What:		/sys/devices/.../hwmon/hwmon<i>/power1_max
+Date:		September 2022
+KernelVersion:	6
+Contact:	dri-devel@lists.freedesktop.org
+Description:	RW. Card reactive sustained  (PL1/Tau) power limit in microwatts.
+
+		The power controller will throttle the operating frequency
+		if the power averaged over a window (typically seconds)
+		exceeds this limit.
+
+		Only supported for particular Intel i915 graphics platforms.
+
+What:		/sys/devices/.../hwmon/hwmon<i>/power1_rated_max
+Date:		September 2022
+KernelVersion:	6
+Contact:	dri-devel@lists.freedesktop.org
+Description:	RO. Card default power limit (default TDP setting).
+
+		Only supported for particular Intel i915 graphics platforms.
diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c
index 45745afa5c5b..5183cf51a49b 100644
--- a/drivers/gpu/drm/i915/i915_hwmon.c
+++ b/drivers/gpu/drm/i915/i915_hwmon.c
@@ -16,11 +16,16 @@ 
 /*
  * SF_* - scale factors for particular quantities according to hwmon spec.
  * - voltage  - millivolts
+ * - power  - microwatts
  */
 #define SF_VOLTAGE	1000
+#define SF_POWER	1000000
 
 struct hwm_reg {
 	i915_reg_t gt_perf_status;
+	i915_reg_t pkg_power_sku_unit;
+	i915_reg_t pkg_power_sku;
+	i915_reg_t pkg_rapl_limit;
 };
 
 struct hwm_drvdata {
@@ -34,10 +39,68 @@  struct i915_hwmon {
 	struct hwm_drvdata ddat;
 	struct mutex hwmon_lock;		/* counter overflow logic and rmw */
 	struct hwm_reg rg;
+	int scl_shift_power;
 };
 
+static void
+hwm_locked_with_pm_intel_uncore_rmw(struct hwm_drvdata *ddat,
+				    i915_reg_t reg, u32 clear, u32 set)
+{
+	struct i915_hwmon *hwmon = ddat->hwmon;
+	struct intel_uncore *uncore = ddat->uncore;
+	intel_wakeref_t wakeref;
+
+	mutex_lock(&hwmon->hwmon_lock);
+
+	with_intel_runtime_pm(uncore->rpm, wakeref)
+		intel_uncore_rmw(uncore, reg, clear, set);
+
+	mutex_unlock(&hwmon->hwmon_lock);
+}
+
+/*
+ * This function's return type of u64 allows for the case where the scaling
+ * of the field taken from the 32-bit register value might cause a result to
+ * exceed 32 bits.
+ */
+static u64
+hwm_field_read_and_scale(struct hwm_drvdata *ddat, i915_reg_t rgadr,
+			 u32 field_msk, int nshift, u32 scale_factor)
+{
+	struct intel_uncore *uncore = ddat->uncore;
+	intel_wakeref_t wakeref;
+	u32 reg_value;
+
+	with_intel_runtime_pm(uncore->rpm, wakeref)
+		reg_value = intel_uncore_read(uncore, rgadr);
+
+	reg_value = REG_FIELD_GET(field_msk, reg_value);
+
+	return mul_u64_u32_shr(reg_value, scale_factor, nshift);
+}
+
+static void
+hwm_field_scale_and_write(struct hwm_drvdata *ddat, i915_reg_t rgadr,
+			  u32 field_msk, int nshift,
+			  unsigned int scale_factor, long lval)
+{
+	u32 nval;
+	u32 bits_to_clear;
+	u32 bits_to_set;
+
+	/* Computation in 64-bits to avoid overflow. Round to nearest. */
+	nval = DIV_ROUND_CLOSEST_ULL((u64)lval << nshift, scale_factor);
+
+	bits_to_clear = field_msk;
+	bits_to_set = FIELD_PREP(field_msk, nval);
+
+	hwm_locked_with_pm_intel_uncore_rmw(ddat, rgadr,
+					    bits_to_clear, bits_to_set);
+}
+
 static const struct hwmon_channel_info *hwm_info[] = {
 	HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
+	HWMON_CHANNEL_INFO(power, HWMON_P_MAX | HWMON_P_RATED_MAX),
 	NULL
 };
 
@@ -71,6 +134,64 @@  hwm_in_read(struct hwm_drvdata *ddat, u32 attr, long *val)
 	}
 }
 
+static umode_t
+hwm_power_is_visible(const struct hwm_drvdata *ddat, u32 attr, int chan)
+{
+	struct i915_hwmon *hwmon = ddat->hwmon;
+
+	switch (attr) {
+	case hwmon_power_max:
+		return i915_mmio_reg_valid(hwmon->rg.pkg_rapl_limit) ? 0664 : 0;
+	case hwmon_power_rated_max:
+		return i915_mmio_reg_valid(hwmon->rg.pkg_power_sku) ? 0444 : 0;
+	default:
+		return 0;
+	}
+}
+
+static int
+hwm_power_read(struct hwm_drvdata *ddat, u32 attr, int chan, long *val)
+{
+	struct i915_hwmon *hwmon = ddat->hwmon;
+
+	switch (attr) {
+	case hwmon_power_max:
+		*val = hwm_field_read_and_scale(ddat,
+						hwmon->rg.pkg_rapl_limit,
+						PKG_PWR_LIM_1,
+						hwmon->scl_shift_power,
+						SF_POWER);
+		return 0;
+	case hwmon_power_rated_max:
+		*val = hwm_field_read_and_scale(ddat,
+						hwmon->rg.pkg_power_sku,
+						PKG_PKG_TDP,
+						hwmon->scl_shift_power,
+						SF_POWER);
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int
+hwm_power_write(struct hwm_drvdata *ddat, u32 attr, int chan, long val)
+{
+	struct i915_hwmon *hwmon = ddat->hwmon;
+
+	switch (attr) {
+	case hwmon_power_max:
+		hwm_field_scale_and_write(ddat,
+					  hwmon->rg.pkg_rapl_limit,
+					  PKG_PWR_LIM_1,
+					  hwmon->scl_shift_power,
+					  SF_POWER, val);
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
 static umode_t
 hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
 	       u32 attr, int channel)
@@ -80,6 +201,8 @@  hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type,
 	switch (type) {
 	case hwmon_in:
 		return hwm_in_is_visible(ddat, attr);
+	case hwmon_power:
+		return hwm_power_is_visible(ddat, attr, channel);
 	default:
 		return 0;
 	}
@@ -94,6 +217,8 @@  hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 	switch (type) {
 	case hwmon_in:
 		return hwm_in_read(ddat, attr, val);
+	case hwmon_power:
+		return hwm_power_read(ddat, attr, channel, val);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -103,7 +228,11 @@  static int
 hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 	  int channel, long val)
 {
+	struct hwm_drvdata *ddat = dev_get_drvdata(dev);
+
 	switch (type) {
+	case hwmon_power:
+		return hwm_power_write(ddat, attr, channel, val);
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -124,11 +253,36 @@  static void
 hwm_get_preregistration_info(struct drm_i915_private *i915)
 {
 	struct i915_hwmon *hwmon = i915->hwmon;
+	struct intel_uncore *uncore = &i915->uncore;
+	intel_wakeref_t wakeref;
+	u32 val_sku_unit;
 
-	if (IS_DG1(i915) || IS_DG2(i915))
+	if (IS_DG1(i915) || IS_DG2(i915)) {
 		hwmon->rg.gt_perf_status = GEN12_RPSTAT1;
-	else
+		hwmon->rg.pkg_power_sku_unit = PCU_PACKAGE_POWER_SKU_UNIT;
+		hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
+		hwmon->rg.pkg_rapl_limit = PCU_PACKAGE_RAPL_LIMIT;
+	} else {
 		hwmon->rg.gt_perf_status = INVALID_MMIO_REG;
+		hwmon->rg.pkg_power_sku_unit = INVALID_MMIO_REG;
+		hwmon->rg.pkg_power_sku = INVALID_MMIO_REG;
+		hwmon->rg.pkg_rapl_limit = INVALID_MMIO_REG;
+	}
+
+	with_intel_runtime_pm(uncore->rpm, wakeref) {
+		/*
+		 * The contents of register hwmon->rg.pkg_power_sku_unit do not change,
+		 * so read it once and store the shift values.
+		 */
+		if (i915_mmio_reg_valid(hwmon->rg.pkg_power_sku_unit)) {
+			val_sku_unit = intel_uncore_read(uncore,
+							 hwmon->rg.pkg_power_sku_unit);
+		} else {
+			val_sku_unit = 0;
+		}
+
+		hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, val_sku_unit);
+	}
 }
 
 void i915_hwmon_register(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1a9bd829fc7e..55c35903adca 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1807,6 +1807,11 @@ 
 #define   POWER_LIMIT_1_MASK		REG_BIT(10)
 #define   POWER_LIMIT_2_MASK		REG_BIT(11)
 
+/*
+ * *_PACKAGE_POWER_SKU - SKU power and timing parameters.
+ */
+#define   PKG_PKG_TDP			GENMASK_ULL(14, 0)
+
 #define CHV_CLK_CTL1			_MMIO(0x101100)
 #define VLV_CLK_CTL2			_MMIO(0x101104)
 #define   CLK_CTL2_CZCOUNT_30NS_SHIFT	28
diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
index ffc702b79579..b74df11977c6 100644
--- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
+++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
@@ -189,6 +189,10 @@ 
 #define  DG1_QCLK_RATIO_MASK			REG_GENMASK(9, 2)
 #define  DG1_QCLK_REFERENCE			REG_BIT(10)
 
+#define PCU_PACKAGE_POWER_SKU_UNIT		_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5938)
+#define   PKG_PWR_UNIT				REG_GENMASK(3, 0)
+#define   PKG_TIME_UNIT				REG_GENMASK(19, 16)
+
 #define GEN6_GT_PERF_STATUS			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5948)
 #define GEN6_RP_STATE_LIMITS			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5994)
 #define GEN6_RP_STATE_CAP			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5998)
@@ -198,6 +202,8 @@ 
 
 #define GEN10_FREQ_INFO_REC			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5ef0)
 #define   RPE_MASK				REG_GENMASK(15, 8)
+#define PCU_PACKAGE_RAPL_LIMIT			_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
+#define   PKG_PWR_LIM_1				REG_GENMASK(14, 0)
 
 /* snb MCH registers for priority tuning */
 #define MCH_SSKPD				_MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)