diff mbox series

[v5,7/7] drm/i915/gt: Adding new sysfs frequency attributes

Message ID 20220217144158.21555-8-andi.shyti@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series Introduce multitile support | expand

Commit Message

Andi Shyti Feb. 17, 2022, 2:41 p.m. UTC 
From: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>

This patch adds the following new sysfs frequency attributes;
	- punit_req_freq_mhz
	- throttle_reason_status
	- throttle_reason_pl1
	- throttle_reason_pl2
	- throttle_reason_pl4
	- throttle_reason_thermal
	- throttle_reason_prochot
	- throttle_reason_ratl
	- throttle_reason_vr_thermalert
	- throttle_reason_vr_tdc

Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
Cc: Dale B Stimson <dale.b.stimson@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 142 ++++++++++++++++++++
 drivers/gpu/drm/i915/gt/intel_rps.c         |  83 ++++++++++++
 drivers/gpu/drm/i915/gt/intel_rps.h         |  10 ++
 drivers/gpu/drm/i915/i915_reg.h             |  11 ++
 4 files changed, 246 insertions(+)

Comments

Andi Shyti Feb. 17, 2022, 3:45 p.m. UTC | #1 
Hi,

I forgot to add some note to this patch...

[...]

> +static ssize_t throttle_reason_status_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool status = !!intel_rps_read_throttle_reason_status(rps);

why are these boolean? Can't we send whatever we read from the
register?

[...]

> +#define GT0_PERF_LIMIT_REASONS		_MMIO(0x1381A8)
> +#define   GT0_PERF_LIMIT_REASONS_MASK	0x00000de3

This mask is really weird! Sujaritha, can you please explain it?

It looks something like this,

  REG_GENMASK(11, 6) | REG_GENMASK(2, 0)

But I don't know if it improves any readability, in any case, the
mask is not clear.

> +#define   PROCHOT_MASK			BIT(1)
> +#define   THERMAL_LIMIT_MASK		BIT(2)
> +#define   RATL_MASK			BIT(6)
> +#define   VR_THERMALERT_MASK		BIT(7)
> +#define   VR_TDC_MASK			BIT(8)
> +#define   POWER_LIMIT_4_MASK		BIT(9)
> +#define   POWER_LIMIT_1_MASK		BIT(11)
> +#define   POWER_LIMIT_2_MASK		BIT(12)

I hope I got these right. Sujaritha, can you please check?

Andi
Sundaresan, Sujaritha Feb. 17, 2022, 5:06 p.m. UTC | #2 
On 2/17/2022 7:45 AM, Andi Shyti wrote:
> Hi,
>
> I forgot to add some note to this patch...
>
> [...]
>
>> +static ssize_t throttle_reason_status_show(struct device *dev,
>> +					   struct device_attribute *attr,
>> +					   char *buff)
>> +{
>> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
>> +	struct intel_rps *rps = &gt->rps;
>> +	bool status = !!intel_rps_read_throttle_reason_status(rps);
> why are these boolean? Can't we send whatever we read from the
> register?
Didn't want to report out the register mask value since the sysfs is 
supposed to a 0/1 status.
>
> [...]
>
>> +#define GT0_PERF_LIMIT_REASONS		_MMIO(0x1381A8)
>> +#define   GT0_PERF_LIMIT_REASONS_MASK	0x00000de3
> This mask is really weird! Sujaritha, can you please explain it?
>
> It looks something like this,
>
>    REG_GENMASK(11, 6) | REG_GENMASK(2, 0)
>
> But I don't know if it improves any readability, in any case, the
> mask is not clear.

This is meant to be an overall status flag as a one stop shop check for 
any kind of throttling.

>
>> +#define   PROCHOT_MASK			BIT(1)
>> +#define   THERMAL_LIMIT_MASK		BIT(2)
>> +#define   RATL_MASK			BIT(6)
>> +#define   VR_THERMALERT_MASK		BIT(7)
>> +#define   VR_TDC_MASK			BIT(8)
>> +#define   POWER_LIMIT_4_MASK		BIT(9)
>> +#define   POWER_LIMIT_1_MASK		BIT(11)
>> +#define   POWER_LIMIT_2_MASK		BIT(12)
> I hope I got these right. Sujaritha, can you please check?
>
> Andi

Yes these are correct.

Thanks,

Suja
Michal Wajdeczko Feb. 28, 2022, 8:37 p.m. UTC | #3 
On 17.02.2022 15:41, Andi Shyti wrote:
> From: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> 
> This patch adds the following new sysfs frequency attributes;
> 	- punit_req_freq_mhz
> 	- throttle_reason_status
> 	- throttle_reason_pl1
> 	- throttle_reason_pl2
> 	- throttle_reason_pl4
> 	- throttle_reason_thermal
> 	- throttle_reason_prochot
> 	- throttle_reason_ratl
> 	- throttle_reason_vr_thermalert
> 	- throttle_reason_vr_tdc
> 
> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Dale B Stimson <dale.b.stimson@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 142 ++++++++++++++++++++
>  drivers/gpu/drm/i915/gt/intel_rps.c         |  83 ++++++++++++
>  drivers/gpu/drm/i915/gt/intel_rps.h         |  10 ++
>  drivers/gpu/drm/i915/i915_reg.h             |  11 ++
>  4 files changed, 246 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> index 8e86b8f675f1..8be676cd1607 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -463,6 +463,141 @@ static ssize_t rps_rp_mhz_show(struct device *dev,
>  static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR;
>  static const struct attribute * const gen6_gt_attrs[]  = GEN6_GT_ATTR;
>  
> +static ssize_t punit_req_freq_mhz_show(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	u32 preq = intel_rps_read_punit_req_frequency(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%d\n", preq);

%u since preq is u32

and use sysfs_emit (also in below show functions)

> +}
> +
> +static ssize_t throttle_reason_status_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool status = !!intel_rps_read_throttle_reason_status(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", status);
> +}
> +
> +static ssize_t throttle_reason_pl1_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool pl1 = !!intel_rps_read_throttle_reason_pl1(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", pl1);
> +}
> +
> +static ssize_t throttle_reason_pl2_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool pl2 = !!intel_rps_read_throttle_reason_pl2(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", pl2);
> +}
> +
> +static ssize_t throttle_reason_pl4_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool pl4 = !!intel_rps_read_throttle_reason_pl4(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", pl4);
> +}
> +
> +static ssize_t throttle_reason_thermal_show(struct device *dev,
> +					    struct device_attribute *attr,
> +					    char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool thermal = !!intel_rps_read_throttle_reason_thermal(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", thermal);
> +}
> +
> +static ssize_t throttle_reason_prochot_show(struct device *dev,
> +					    struct device_attribute *attr,
> +					    char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool prochot = !!intel_rps_read_throttle_reason_prochot(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", prochot);
> +}
> +
> +static ssize_t throttle_reason_ratl_show(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool ratl = !!intel_rps_read_throttle_reason_ratl(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", ratl);
> +}
> +
> +static ssize_t throttle_reason_vr_thermalert_show(struct device *dev,
> +						  struct device_attribute *attr,
> +						  char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool thermalert = !!intel_rps_read_throttle_reason_vr_thermalert(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", thermalert);
> +}
> +
> +static ssize_t throttle_reason_vr_tdc_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool tdc = !!intel_rps_read_throttle_reason_vr_tdc(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", tdc);
> +}
> +
> +static DEVICE_ATTR_RO(punit_req_freq_mhz);
> +static DEVICE_ATTR_RO(throttle_reason_status);
> +static DEVICE_ATTR_RO(throttle_reason_pl1);
> +static DEVICE_ATTR_RO(throttle_reason_pl2);
> +static DEVICE_ATTR_RO(throttle_reason_pl4);
> +static DEVICE_ATTR_RO(throttle_reason_thermal);
> +static DEVICE_ATTR_RO(throttle_reason_prochot);
> +static DEVICE_ATTR_RO(throttle_reason_ratl);
> +static DEVICE_ATTR_RO(throttle_reason_vr_thermalert);
> +static DEVICE_ATTR_RO(throttle_reason_vr_tdc);
> +
> +static const struct attribute *freq_attrs[] = {
> +	&dev_attr_punit_req_freq_mhz.attr,
> +	&dev_attr_throttle_reason_status.attr,
> +	&dev_attr_throttle_reason_pl1.attr,
> +	&dev_attr_throttle_reason_pl2.attr,
> +	&dev_attr_throttle_reason_pl4.attr,
> +	&dev_attr_throttle_reason_thermal.attr,
> +	&dev_attr_throttle_reason_prochot.attr,
> +	&dev_attr_throttle_reason_ratl.attr,
> +	&dev_attr_throttle_reason_vr_thermalert.attr,
> +	&dev_attr_throttle_reason_vr_tdc.attr,
> +	NULL
> +};
> +
>  static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
>  				const struct attribute * const *attrs)
>  {
> @@ -493,4 +628,11 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
>  	if (ret)
>  		drm_warn(&gt->i915->drm,
>  			 "failed to create gt%u RPS sysfs files", gt->info.id);
> +
> +	ret = sysfs_create_files(kobj, freq_attrs);
> +	if (ret)
> +		drm_warn(&gt->i915->drm,
> +			 "failed to create gt%u throttle sysfs files",
> +			 gt->info.id);

nit: would be nice to see %pe why it failed

> +
>  }
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index fd95449ed46d..94c78cfaf9c9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -2286,6 +2286,89 @@ void intel_rps_lower_unslice(struct intel_rps *rps)
>  	mutex_unlock(&rps->lock);
>  }
>  
> +static u32 __rps_read_mmio(struct intel_gt *gt, i915_reg_t reg32)

this doesn't look like "rps" helper, rather like "gt" so it should have
different prefix and maybe even be exported by the gt or uncore ?

unless you wanted:

static u32 __rps_read_mmio(struct intel_rps *rps, i915_reg_t reg32)
{
	struct intel_gt *gt = rps_to_gt(rps);

> +{
> +	intel_wakeref_t wakeref;
> +	u32 val;
> +
> +	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> +		val = intel_uncore_read(gt->uncore, reg32);
> +
> +	return val;
> +}
> +
> +u32 intel_rps_read_throttle_reason_status(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 status = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & GT0_PERF_LIMIT_REASONS_MASK;
> +
> +	return status;
> +}
> +
> +u32 intel_rps_read_throttle_reason_pl1(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 pl1 = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & POWER_LIMIT_1_MASK;
> +
> +	return pl1;
> +}
> +
> +u32 intel_rps_read_throttle_reason_pl2(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 pl2 = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & POWER_LIMIT_2_MASK;
> +
> +	return pl2;
> +}
> +
> +u32 intel_rps_read_throttle_reason_pl4(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 pl4 = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & POWER_LIMIT_4_MASK;
> +
> +	return pl4;
> +}
> +
> +u32 intel_rps_read_throttle_reason_thermal(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 thermal = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & THERMAL_LIMIT_MASK;
> +
> +	return thermal;
> +}
> +
> +u32 intel_rps_read_throttle_reason_prochot(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 prochot = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & PROCHOT_MASK;
> +
> +	return prochot;
> +}
> +
> +u32 intel_rps_read_throttle_reason_ratl(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 ratl = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & RATL_MASK;
> +
> +	return ratl;
> +}
> +
> +u32 intel_rps_read_throttle_reason_vr_thermalert(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 thermalert = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & VR_THERMALERT_MASK;
> +
> +	return thermalert;
> +}

shouldn't we return bool by all of these functions as used/expected in
show() counterparts ?

> +
> +u32 intel_rps_read_throttle_reason_vr_tdc(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 tdc = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & VR_TDC_MASK;
> +
> +	return tdc;
> +}
> +
>  /* External interface for intel_ips.ko */
>  
>  static struct drm_i915_private __rcu *ips_mchdev;
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h b/drivers/gpu/drm/i915/gt/intel_rps.h
> index c6d76a3d1331..b45ab983895c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.h
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.h
> @@ -47,6 +47,16 @@ u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps);
>  u32 intel_rps_read_state_cap(struct intel_rps *rps);
>  void intel_rps_raise_unslice(struct intel_rps *rps);
>  void intel_rps_lower_unslice(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_status(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_pl1(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_pl2(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_pl4(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_thermal(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_prochot(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_ratl(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_vr_thermalert(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_vr_tdc(struct intel_rps *rps);
>  
>  void gen5_rps_irq_handler(struct intel_rps *rps);
>  void gen6_rps_irq_handler(struct intel_rps *rps, u32 pm_iir);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2243d9d1d941..c4f53e5404cd 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1836,6 +1836,17 @@
>  #define GEN9_RP_STATE_LIMITS	_MMIO(0x138148)
>  #define XEHPSDV_RP_STATE_CAP	_MMIO(0x250014)
>  
> +#define GT0_PERF_LIMIT_REASONS		_MMIO(0x1381A8)
> +#define   GT0_PERF_LIMIT_REASONS_MASK	0x00000de3

this mask is different that other (FIELD_PREP/GET wont work) so maybe we
should name it in special way ?

> +#define   PROCHOT_MASK			BIT(1)
> +#define   THERMAL_LIMIT_MASK		BIT(2)
> +#define   RATL_MASK			BIT(6)
> +#define   VR_THERMALERT_MASK		BIT(7)
> +#define   VR_TDC_MASK			BIT(8)
> +#define   POWER_LIMIT_4_MASK		BIT(9)
> +#define   POWER_LIMIT_1_MASK		BIT(11)
> +#define   POWER_LIMIT_2_MASK		BIT(12)

REG_BIT ?

Michal

> +
>  #define CHV_CLK_CTL1			_MMIO(0x101100)
>  #define VLV_CLK_CTL2			_MMIO(0x101104)
>  #define   CLK_CTL2_CZCOUNT_30NS_SHIFT	28
Andrzej Hajda March 3, 2022, 11:17 a.m. UTC | #4 
On 17.02.2022 15:41, Andi Shyti wrote:
> From: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>
> This patch adds the following new sysfs frequency attributes;
> 	- punit_req_freq_mhz
> 	- throttle_reason_status
> 	- throttle_reason_pl1
> 	- throttle_reason_pl2
> 	- throttle_reason_pl4
> 	- throttle_reason_thermal
> 	- throttle_reason_prochot
> 	- throttle_reason_ratl
> 	- throttle_reason_vr_thermalert
> 	- throttle_reason_vr_tdc
>
> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Signed-off-by: Andi Shyti <andi.shyti@linux.intel.com>
> Cc: Dale B Stimson <dale.b.stimson@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c | 142 ++++++++++++++++++++
>   drivers/gpu/drm/i915/gt/intel_rps.c         |  83 ++++++++++++
>   drivers/gpu/drm/i915/gt/intel_rps.h         |  10 ++
>   drivers/gpu/drm/i915/i915_reg.h             |  11 ++
>   4 files changed, 246 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> index 8e86b8f675f1..8be676cd1607 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
> @@ -463,6 +463,141 @@ static ssize_t rps_rp_mhz_show(struct device *dev,
>   static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR;
>   static const struct attribute * const gen6_gt_attrs[]  = GEN6_GT_ATTR;
>   
> +static ssize_t punit_req_freq_mhz_show(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	u32 preq = intel_rps_read_punit_req_frequency(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%d\n", preq);
> +}
> +
> +static ssize_t throttle_reason_status_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool status = !!intel_rps_read_throttle_reason_status(rps);

!! is not necessary if you assign to bool variable, here and below.

> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", status);
> +}
> +
> +static ssize_t throttle_reason_pl1_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool pl1 = !!intel_rps_read_throttle_reason_pl1(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", pl1);
> +}
> +
> +static ssize_t throttle_reason_pl2_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool pl2 = !!intel_rps_read_throttle_reason_pl2(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", pl2);
> +}
> +
> +static ssize_t throttle_reason_pl4_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool pl4 = !!intel_rps_read_throttle_reason_pl4(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", pl4);
> +}
> +
> +static ssize_t throttle_reason_thermal_show(struct device *dev,
> +					    struct device_attribute *attr,
> +					    char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool thermal = !!intel_rps_read_throttle_reason_thermal(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", thermal);
> +}
> +
> +static ssize_t throttle_reason_prochot_show(struct device *dev,
> +					    struct device_attribute *attr,
> +					    char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool prochot = !!intel_rps_read_throttle_reason_prochot(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", prochot);
> +}
> +
> +static ssize_t throttle_reason_ratl_show(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool ratl = !!intel_rps_read_throttle_reason_ratl(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", ratl);
> +}
> +
> +static ssize_t throttle_reason_vr_thermalert_show(struct device *dev,
> +						  struct device_attribute *attr,
> +						  char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool thermalert = !!intel_rps_read_throttle_reason_vr_thermalert(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", thermalert);
> +}
> +
> +static ssize_t throttle_reason_vr_tdc_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buff)
> +{
> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> +	struct intel_rps *rps = &gt->rps;
> +	bool tdc = !!intel_rps_read_throttle_reason_vr_tdc(rps);
> +
> +	return scnprintf(buff, PAGE_SIZE, "%u\n", tdc);
> +}

Nine, almost the same functions are asking for proper abstraction, a'la:
struct intel_rps_bool_attribute {
     struct device_attribute attr;
     u32 (*func)(struct intel_rps *rps);
}
And for example:

static INTEL_RPS_BOOL_ATTR_RO(throttle_reason_status, intel_rps_read_throttle_reason_status);


Or at least some macro magic to hide this code redundancy.

Regards
Andrzej

> +
> +static DEVICE_ATTR_RO(punit_req_freq_mhz);
> +static DEVICE_ATTR_RO(throttle_reason_status);
> +static DEVICE_ATTR_RO(throttle_reason_pl1);
> +static DEVICE_ATTR_RO(throttle_reason_pl2);
> +static DEVICE_ATTR_RO(throttle_reason_pl4);
> +static DEVICE_ATTR_RO(throttle_reason_thermal);
> +static DEVICE_ATTR_RO(throttle_reason_prochot);
> +static DEVICE_ATTR_RO(throttle_reason_ratl);
> +static DEVICE_ATTR_RO(throttle_reason_vr_thermalert);
> +static DEVICE_ATTR_RO(throttle_reason_vr_tdc);
> +
> +static const struct attribute *freq_attrs[] = {
> +	&dev_attr_punit_req_freq_mhz.attr,
> +	&dev_attr_throttle_reason_status.attr,
> +	&dev_attr_throttle_reason_pl1.attr,
> +	&dev_attr_throttle_reason_pl2.attr,
> +	&dev_attr_throttle_reason_pl4.attr,
> +	&dev_attr_throttle_reason_thermal.attr,
> +	&dev_attr_throttle_reason_prochot.attr,
> +	&dev_attr_throttle_reason_ratl.attr,
> +	&dev_attr_throttle_reason_vr_thermalert.attr,
> +	&dev_attr_throttle_reason_vr_tdc.attr,
> +	NULL
> +};
> +
>   static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
>   				const struct attribute * const *attrs)
>   {
> @@ -493,4 +628,11 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
>   	if (ret)
>   		drm_warn(&gt->i915->drm,
>   			 "failed to create gt%u RPS sysfs files", gt->info.id);
> +
> +	ret = sysfs_create_files(kobj, freq_attrs);
> +	if (ret)
> +		drm_warn(&gt->i915->drm,
> +			 "failed to create gt%u throttle sysfs files",
> +			 gt->info.id);
> +
>   }
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index fd95449ed46d..94c78cfaf9c9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -2286,6 +2286,89 @@ void intel_rps_lower_unslice(struct intel_rps *rps)
>   	mutex_unlock(&rps->lock);
>   }
>   
> +static u32 __rps_read_mmio(struct intel_gt *gt, i915_reg_t reg32)
> +{
> +	intel_wakeref_t wakeref;
> +	u32 val;
> +
> +	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> +		val = intel_uncore_read(gt->uncore, reg32);
> +
> +	return val;
> +}
> +
> +u32 intel_rps_read_throttle_reason_status(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 status = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & GT0_PERF_LIMIT_REASONS_MASK;
> +
> +	return status;
> +}
> +
> +u32 intel_rps_read_throttle_reason_pl1(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 pl1 = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & POWER_LIMIT_1_MASK;
> +
> +	return pl1;
> +}
> +
> +u32 intel_rps_read_throttle_reason_pl2(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 pl2 = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & POWER_LIMIT_2_MASK;
> +
> +	return pl2;
> +}
> +
> +u32 intel_rps_read_throttle_reason_pl4(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 pl4 = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & POWER_LIMIT_4_MASK;
> +
> +	return pl4;
> +}
> +
> +u32 intel_rps_read_throttle_reason_thermal(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 thermal = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & THERMAL_LIMIT_MASK;
> +
> +	return thermal;
> +}
> +
> +u32 intel_rps_read_throttle_reason_prochot(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 prochot = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & PROCHOT_MASK;
> +
> +	return prochot;
> +}
> +
> +u32 intel_rps_read_throttle_reason_ratl(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 ratl = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & RATL_MASK;
> +
> +	return ratl;
> +}
> +
> +u32 intel_rps_read_throttle_reason_vr_thermalert(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 thermalert = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & VR_THERMALERT_MASK;
> +
> +	return thermalert;
> +}
> +
> +u32 intel_rps_read_throttle_reason_vr_tdc(struct intel_rps *rps)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 tdc = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & VR_TDC_MASK;
> +
> +	return tdc;
> +}
> +
>   /* External interface for intel_ips.ko */
>   
>   static struct drm_i915_private __rcu *ips_mchdev;
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h b/drivers/gpu/drm/i915/gt/intel_rps.h
> index c6d76a3d1331..b45ab983895c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.h
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.h
> @@ -47,6 +47,16 @@ u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps);
>   u32 intel_rps_read_state_cap(struct intel_rps *rps);
>   void intel_rps_raise_unslice(struct intel_rps *rps);
>   void intel_rps_lower_unslice(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_status(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_pl1(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_pl2(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_pl4(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_thermal(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_prochot(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_ratl(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_vr_thermalert(struct intel_rps *rps);
> +u32 intel_rps_read_throttle_reason_vr_tdc(struct intel_rps *rps);
>   
>   void gen5_rps_irq_handler(struct intel_rps *rps);
>   void gen6_rps_irq_handler(struct intel_rps *rps, u32 pm_iir);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2243d9d1d941..c4f53e5404cd 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1836,6 +1836,17 @@
>   #define GEN9_RP_STATE_LIMITS	_MMIO(0x138148)
>   #define XEHPSDV_RP_STATE_CAP	_MMIO(0x250014)
>   
> +#define GT0_PERF_LIMIT_REASONS		_MMIO(0x1381A8)
> +#define   GT0_PERF_LIMIT_REASONS_MASK	0x00000de3
> +#define   PROCHOT_MASK			BIT(1)
> +#define   THERMAL_LIMIT_MASK		BIT(2)
> +#define   RATL_MASK			BIT(6)
> +#define   VR_THERMALERT_MASK		BIT(7)
> +#define   VR_TDC_MASK			BIT(8)
> +#define   POWER_LIMIT_4_MASK		BIT(9)
> +#define   POWER_LIMIT_1_MASK		BIT(11)
> +#define   POWER_LIMIT_2_MASK		BIT(12)
> +
>   #define CHV_CLK_CTL1			_MMIO(0x101100)
>   #define VLV_CLK_CTL2			_MMIO(0x101104)
>   #define   CLK_CTL2_CZCOUNT_30NS_SHIFT	28
Andi Shyti March 14, 2022, 12:38 a.m. UTC | #5 
Hi Michal,

[...]

> > +static ssize_t punit_req_freq_mhz_show(struct device *dev,
> > +				       struct device_attribute *attr,
> > +				       char *buff)
> > +{
> > +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
> > +	struct intel_rps *rps = &gt->rps;
> > +	u32 preq = intel_rps_read_punit_req_frequency(rps);
> > +
> > +	return scnprintf(buff, PAGE_SIZE, "%d\n", preq);
> 
> %u since preq is u32
> 
> and use sysfs_emit (also in below show functions)

sure! I'll change them.

[...]

> >  static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
> >  				const struct attribute * const *attrs)
> >  {
> > @@ -493,4 +628,11 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
> >  	if (ret)
> >  		drm_warn(&gt->i915->drm,
> >  			 "failed to create gt%u RPS sysfs files", gt->info.id);
> > +
> > +	ret = sysfs_create_files(kobj, freq_attrs);
> > +	if (ret)
> > +		drm_warn(&gt->i915->drm,
> > +			 "failed to create gt%u throttle sysfs files",
> > +			 gt->info.id);
> 
> nit: would be nice to see %pe why it failed

[...]

I will add it to the other cases as well.

> > +static u32 __rps_read_mmio(struct intel_gt *gt, i915_reg_t reg32)
> 
> this doesn't look like "rps" helper, rather like "gt" so it should have
> different prefix and maybe even be exported by the gt or uncore ?
> 
> unless you wanted:
> 
> static u32 __rps_read_mmio(struct intel_rps *rps, i915_reg_t reg32)
> {
> 	struct intel_gt *gt = rps_to_gt(rps);
> 
> > +{
> > +	intel_wakeref_t wakeref;
> > +	u32 val;
> > +
> > +	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
> > +		val = intel_uncore_read(gt->uncore, reg32);
> > +
> > +	return val;
> > +}

Yes, you are right!

@Sujaritha: shall I move "__rps_read_mmio()" in intel_gt.c and
call it intel_gt_read_mmio()?

[...]

> > +u32 intel_rps_read_throttle_reason_vr_thermalert(struct intel_rps *rps)
> > +{
> > +	struct intel_gt *gt = rps_to_gt(rps);
> > +	u32 thermalert = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & VR_THERMALERT_MASK;
> > +
> > +	return thermalert;
> > +}
> 
> shouldn't we return bool by all of these functions as used/expected in
> show() counterparts ?

Suja?

[...]

> > +#define GT0_PERF_LIMIT_REASONS		_MMIO(0x1381A8)
> > +#define   GT0_PERF_LIMIT_REASONS_MASK	0x00000de3
> 
> this mask is different that other (FIELD_PREP/GET wont work) so maybe we
> should name it in special way ?

As far as I understood this is still a mask and used as such.
This mask is actually telling that there is some throttling going
on.

It looks weird because there are some unwanted bits in between
the interesting bits.

> > +#define   PROCHOT_MASK			BIT(1)
> > +#define   THERMAL_LIMIT_MASK		BIT(2)
> > +#define   RATL_MASK			BIT(6)
> > +#define   VR_THERMALERT_MASK		BIT(7)
> > +#define   VR_TDC_MASK			BIT(8)
> > +#define   POWER_LIMIT_4_MASK		BIT(9)
> > +#define   POWER_LIMIT_1_MASK		BIT(11)
> > +#define   POWER_LIMIT_2_MASK		BIT(12)
> 
> REG_BIT ?

yes!

Thanks, Michal!
Andi
Sundaresan, Sujaritha March 14, 2022, 1:32 a.m. UTC | #6 
On 3/13/2022 5:38 PM, Andi Shyti wrote:
> Hi Michal,
>
> [...]
>
>>> +static ssize_t punit_req_freq_mhz_show(struct device *dev,
>>> +				       struct device_attribute *attr,
>>> +				       char *buff)
>>> +{
>>> +	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
>>> +	struct intel_rps *rps = &gt->rps;
>>> +	u32 preq = intel_rps_read_punit_req_frequency(rps);
>>> +
>>> +	return scnprintf(buff, PAGE_SIZE, "%d\n", preq);
>> %u since preq is u32
>>
>> and use sysfs_emit (also in below show functions)
> sure! I'll change them.
>
> [...]
>
>>>   static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
>>>   				const struct attribute * const *attrs)
>>>   {
>>> @@ -493,4 +628,11 @@ void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
>>>   	if (ret)
>>>   		drm_warn(&gt->i915->drm,
>>>   			 "failed to create gt%u RPS sysfs files", gt->info.id);
>>> +
>>> +	ret = sysfs_create_files(kobj, freq_attrs);
>>> +	if (ret)
>>> +		drm_warn(&gt->i915->drm,
>>> +			 "failed to create gt%u throttle sysfs files",
>>> +			 gt->info.id);
>> nit: would be nice to see %pe why it failed
> [...]
>
> I will add it to the other cases as well.
>
>>> +static u32 __rps_read_mmio(struct intel_gt *gt, i915_reg_t reg32)
>> this doesn't look like "rps" helper, rather like "gt" so it should have
>> different prefix and maybe even be exported by the gt or uncore ?
>>
>> unless you wanted:
>>
>> static u32 __rps_read_mmio(struct intel_rps *rps, i915_reg_t reg32)
>> {
>> 	struct intel_gt *gt = rps_to_gt(rps);
>>
>>> +{
>>> +	intel_wakeref_t wakeref;
>>> +	u32 val;
>>> +
>>> +	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
>>> +		val = intel_uncore_read(gt->uncore, reg32);
>>> +
>>> +	return val;
>>> +}
> Yes, you are right!
>
> @Sujaritha: shall I move "__rps_read_mmio()" in intel_gt.c and
> call it intel_gt_read_mmio()?
>
> [...]
Sure since it is kind of a gt helper, makes sense to have gt prefix.
>
>>> +u32 intel_rps_read_throttle_reason_vr_thermalert(struct intel_rps *rps)
>>> +{
>>> +	struct intel_gt *gt = rps_to_gt(rps);
>>> +	u32 thermalert = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & VR_THERMALERT_MASK;
>>> +
>>> +	return thermalert;
>>> +}
>> shouldn't we return bool by all of these functions as used/expected in
>> show() counterparts ?
> Suja?
>
> [...]
Yes we can make this bool as well.
>
>>> +#define GT0_PERF_LIMIT_REASONS		_MMIO(0x1381A8)
>>> +#define   GT0_PERF_LIMIT_REASONS_MASK	0x00000de3
>> this mask is different that other (FIELD_PREP/GET wont work) so maybe we
>> should name it in special way ?
> As far as I understood this is still a mask and used as such.
> This mask is actually telling that there is some throttling going
> on.
>
> It looks weird because there are some unwanted bits in between
> the interesting bits.
>
>>> +#define   PROCHOT_MASK			BIT(1)
>>> +#define   THERMAL_LIMIT_MASK		BIT(2)
>>> +#define   RATL_MASK			BIT(6)
>>> +#define   VR_THERMALERT_MASK		BIT(7)
>>> +#define   VR_TDC_MASK			BIT(8)
>>> +#define   POWER_LIMIT_4_MASK		BIT(9)
>>> +#define   POWER_LIMIT_1_MASK		BIT(11)
>>> +#define   POWER_LIMIT_2_MASK		BIT(12)
>> REG_BIT ?
> yes!
>
> Thanks, Michal!
> Andi
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
index 8e86b8f675f1..8be676cd1607 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs_pm.c
@@ -463,6 +463,141 @@  static ssize_t rps_rp_mhz_show(struct device *dev,
 static const struct attribute * const gen6_rps_attrs[] = GEN6_RPS_ATTR;
 static const struct attribute * const gen6_gt_attrs[]  = GEN6_GT_ATTR;
 
+static ssize_t punit_req_freq_mhz_show(struct device *dev,
+				       struct device_attribute *attr,
+				       char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_rps *rps = &gt->rps;
+	u32 preq = intel_rps_read_punit_req_frequency(rps);
+
+	return scnprintf(buff, PAGE_SIZE, "%d\n", preq);
+}
+
+static ssize_t throttle_reason_status_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_rps *rps = &gt->rps;
+	bool status = !!intel_rps_read_throttle_reason_status(rps);
+
+	return scnprintf(buff, PAGE_SIZE, "%u\n", status);
+}
+
+static ssize_t throttle_reason_pl1_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_rps *rps = &gt->rps;
+	bool pl1 = !!intel_rps_read_throttle_reason_pl1(rps);
+
+	return scnprintf(buff, PAGE_SIZE, "%u\n", pl1);
+}
+
+static ssize_t throttle_reason_pl2_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_rps *rps = &gt->rps;
+	bool pl2 = !!intel_rps_read_throttle_reason_pl2(rps);
+
+	return scnprintf(buff, PAGE_SIZE, "%u\n", pl2);
+}
+
+static ssize_t throttle_reason_pl4_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_rps *rps = &gt->rps;
+	bool pl4 = !!intel_rps_read_throttle_reason_pl4(rps);
+
+	return scnprintf(buff, PAGE_SIZE, "%u\n", pl4);
+}
+
+static ssize_t throttle_reason_thermal_show(struct device *dev,
+					    struct device_attribute *attr,
+					    char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_rps *rps = &gt->rps;
+	bool thermal = !!intel_rps_read_throttle_reason_thermal(rps);
+
+	return scnprintf(buff, PAGE_SIZE, "%u\n", thermal);
+}
+
+static ssize_t throttle_reason_prochot_show(struct device *dev,
+					    struct device_attribute *attr,
+					    char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_rps *rps = &gt->rps;
+	bool prochot = !!intel_rps_read_throttle_reason_prochot(rps);
+
+	return scnprintf(buff, PAGE_SIZE, "%u\n", prochot);
+}
+
+static ssize_t throttle_reason_ratl_show(struct device *dev,
+					 struct device_attribute *attr,
+					 char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_rps *rps = &gt->rps;
+	bool ratl = !!intel_rps_read_throttle_reason_ratl(rps);
+
+	return scnprintf(buff, PAGE_SIZE, "%u\n", ratl);
+}
+
+static ssize_t throttle_reason_vr_thermalert_show(struct device *dev,
+						  struct device_attribute *attr,
+						  char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_rps *rps = &gt->rps;
+	bool thermalert = !!intel_rps_read_throttle_reason_vr_thermalert(rps);
+
+	return scnprintf(buff, PAGE_SIZE, "%u\n", thermalert);
+}
+
+static ssize_t throttle_reason_vr_tdc_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buff)
+{
+	struct intel_gt *gt = intel_gt_sysfs_get_drvdata(dev, attr->attr.name);
+	struct intel_rps *rps = &gt->rps;
+	bool tdc = !!intel_rps_read_throttle_reason_vr_tdc(rps);
+
+	return scnprintf(buff, PAGE_SIZE, "%u\n", tdc);
+}
+
+static DEVICE_ATTR_RO(punit_req_freq_mhz);
+static DEVICE_ATTR_RO(throttle_reason_status);
+static DEVICE_ATTR_RO(throttle_reason_pl1);
+static DEVICE_ATTR_RO(throttle_reason_pl2);
+static DEVICE_ATTR_RO(throttle_reason_pl4);
+static DEVICE_ATTR_RO(throttle_reason_thermal);
+static DEVICE_ATTR_RO(throttle_reason_prochot);
+static DEVICE_ATTR_RO(throttle_reason_ratl);
+static DEVICE_ATTR_RO(throttle_reason_vr_thermalert);
+static DEVICE_ATTR_RO(throttle_reason_vr_tdc);
+
+static const struct attribute *freq_attrs[] = {
+	&dev_attr_punit_req_freq_mhz.attr,
+	&dev_attr_throttle_reason_status.attr,
+	&dev_attr_throttle_reason_pl1.attr,
+	&dev_attr_throttle_reason_pl2.attr,
+	&dev_attr_throttle_reason_pl4.attr,
+	&dev_attr_throttle_reason_thermal.attr,
+	&dev_attr_throttle_reason_prochot.attr,
+	&dev_attr_throttle_reason_ratl.attr,
+	&dev_attr_throttle_reason_vr_thermalert.attr,
+	&dev_attr_throttle_reason_vr_tdc.attr,
+	NULL
+};
+
 static int intel_sysfs_rps_init(struct intel_gt *gt, struct kobject *kobj,
 				const struct attribute * const *attrs)
 {
@@ -493,4 +628,11 @@  void intel_gt_sysfs_pm_init(struct intel_gt *gt, struct kobject *kobj)
 	if (ret)
 		drm_warn(&gt->i915->drm,
 			 "failed to create gt%u RPS sysfs files", gt->info.id);
+
+	ret = sysfs_create_files(kobj, freq_attrs);
+	if (ret)
+		drm_warn(&gt->i915->drm,
+			 "failed to create gt%u throttle sysfs files",
+			 gt->info.id);
+
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index fd95449ed46d..94c78cfaf9c9 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -2286,6 +2286,89 @@  void intel_rps_lower_unslice(struct intel_rps *rps)
 	mutex_unlock(&rps->lock);
 }
 
+static u32 __rps_read_mmio(struct intel_gt *gt, i915_reg_t reg32)
+{
+	intel_wakeref_t wakeref;
+	u32 val;
+
+	with_intel_runtime_pm(gt->uncore->rpm, wakeref)
+		val = intel_uncore_read(gt->uncore, reg32);
+
+	return val;
+}
+
+u32 intel_rps_read_throttle_reason_status(struct intel_rps *rps)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 status = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & GT0_PERF_LIMIT_REASONS_MASK;
+
+	return status;
+}
+
+u32 intel_rps_read_throttle_reason_pl1(struct intel_rps *rps)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 pl1 = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & POWER_LIMIT_1_MASK;
+
+	return pl1;
+}
+
+u32 intel_rps_read_throttle_reason_pl2(struct intel_rps *rps)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 pl2 = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & POWER_LIMIT_2_MASK;
+
+	return pl2;
+}
+
+u32 intel_rps_read_throttle_reason_pl4(struct intel_rps *rps)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 pl4 = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & POWER_LIMIT_4_MASK;
+
+	return pl4;
+}
+
+u32 intel_rps_read_throttle_reason_thermal(struct intel_rps *rps)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 thermal = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & THERMAL_LIMIT_MASK;
+
+	return thermal;
+}
+
+u32 intel_rps_read_throttle_reason_prochot(struct intel_rps *rps)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 prochot = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & PROCHOT_MASK;
+
+	return prochot;
+}
+
+u32 intel_rps_read_throttle_reason_ratl(struct intel_rps *rps)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 ratl = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & RATL_MASK;
+
+	return ratl;
+}
+
+u32 intel_rps_read_throttle_reason_vr_thermalert(struct intel_rps *rps)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 thermalert = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & VR_THERMALERT_MASK;
+
+	return thermalert;
+}
+
+u32 intel_rps_read_throttle_reason_vr_tdc(struct intel_rps *rps)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 tdc = __rps_read_mmio(gt, GT0_PERF_LIMIT_REASONS) & VR_TDC_MASK;
+
+	return tdc;
+}
+
 /* External interface for intel_ips.ko */
 
 static struct drm_i915_private __rcu *ips_mchdev;
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.h b/drivers/gpu/drm/i915/gt/intel_rps.h
index c6d76a3d1331..b45ab983895c 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.h
+++ b/drivers/gpu/drm/i915/gt/intel_rps.h
@@ -47,6 +47,16 @@  u32 intel_rps_read_punit_req_frequency(struct intel_rps *rps);
 u32 intel_rps_read_state_cap(struct intel_rps *rps);
 void intel_rps_raise_unslice(struct intel_rps *rps);
 void intel_rps_lower_unslice(struct intel_rps *rps);
+u32 intel_rps_read_throttle_reason(struct intel_rps *rps);
+u32 intel_rps_read_throttle_reason_status(struct intel_rps *rps);
+u32 intel_rps_read_throttle_reason_pl1(struct intel_rps *rps);
+u32 intel_rps_read_throttle_reason_pl2(struct intel_rps *rps);
+u32 intel_rps_read_throttle_reason_pl4(struct intel_rps *rps);
+u32 intel_rps_read_throttle_reason_thermal(struct intel_rps *rps);
+u32 intel_rps_read_throttle_reason_prochot(struct intel_rps *rps);
+u32 intel_rps_read_throttle_reason_ratl(struct intel_rps *rps);
+u32 intel_rps_read_throttle_reason_vr_thermalert(struct intel_rps *rps);
+u32 intel_rps_read_throttle_reason_vr_tdc(struct intel_rps *rps);
 
 void gen5_rps_irq_handler(struct intel_rps *rps);
 void gen6_rps_irq_handler(struct intel_rps *rps, u32 pm_iir);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2243d9d1d941..c4f53e5404cd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1836,6 +1836,17 @@ 
 #define GEN9_RP_STATE_LIMITS	_MMIO(0x138148)
 #define XEHPSDV_RP_STATE_CAP	_MMIO(0x250014)
 
+#define GT0_PERF_LIMIT_REASONS		_MMIO(0x1381A8)
+#define   GT0_PERF_LIMIT_REASONS_MASK	0x00000de3
+#define   PROCHOT_MASK			BIT(1)
+#define   THERMAL_LIMIT_MASK		BIT(2)
+#define   RATL_MASK			BIT(6)
+#define   VR_THERMALERT_MASK		BIT(7)
+#define   VR_TDC_MASK			BIT(8)
+#define   POWER_LIMIT_4_MASK		BIT(9)
+#define   POWER_LIMIT_1_MASK		BIT(11)
+#define   POWER_LIMIT_2_MASK		BIT(12)
+
 #define CHV_CLK_CTL1			_MMIO(0x101100)
 #define VLV_CLK_CTL2			_MMIO(0x101104)
 #define   CLK_CTL2_CZCOUNT_30NS_SHIFT	28