drm/i915: Configurable GT idle frequency
diff mbox series

Message ID 20190415230526.1145629-1-bob.j.paauwe@intel.com
State New
Headers show
Series
  • drm/i915: Configurable GT idle frequency
Related show

Commit Message

Bob Paauwe April 15, 2019, 11:05 p.m. UTC
There are real-time use cases where having deterministic CPU processes
can be more important than GPU power/performance. Parking the GPU at a
specific freqency by setting idle, min and max prohibits the normal
dynamic GPU frequency switching which can introduce significant PCI-E
latency. This adds the ability to configure the GPU "idle" frequecy
using the same method that already exists for minimum and maximum
frequencies.

In addition, parking the idle frequency may reduce spool up latencies
on GPU workloads.

Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
---
 drivers/gpu/drm/i915/i915_sysfs.c | 60 +++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

Comments

Vanshidhar Konda April 16, 2019, 12:33 a.m. UTC | #1
On Mon, Apr 15, 2019 at 04:05:26PM -0700, Bob Paauwe wrote:
>There are real-time use cases where having deterministic CPU processes
>can be more important than GPU power/performance. Parking the GPU at a
>specific freqency by setting idle, min and max prohibits the normal
>dynamic GPU frequency switching which can introduce significant PCI-E
>latency. This adds the ability to configure the GPU "idle" frequecy
>using the same method that already exists for minimum and maximum
>frequencies.
>
>In addition, parking the idle frequency may reduce spool up latencies
>on GPU workloads.
>
>Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
>---
> drivers/gpu/drm/i915/i915_sysfs.c | 60 +++++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
>index 41313005af42..62612c23d514 100644
>--- a/drivers/gpu/drm/i915/i915_sysfs.c
>+++ b/drivers/gpu/drm/i915/i915_sysfs.c
>@@ -454,11 +454,69 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> 	return ret ?: count;
> }
>
>+static ssize_t gt_idle_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
>+{
>+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
>+
>+	return snprintf(buf, PAGE_SIZE, "%d\n",
>+			intel_gpu_freq(dev_priv,
>+				       dev_priv->gt_pm.rps.idle_freq));
>+}
>+
>+static ssize_t gt_idle_freq_mhz_store(struct device *kdev,
>+				      struct device_attribute *attr,
>+				      const char *buf, size_t count)
>+{
>+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
>+	struct intel_rps *rps = &dev_priv->gt_pm.rps;
>+	intel_wakeref_t wakeref;
>+	u32 val;

val can probably just be u8. max_freq, min_freq, etc. are only u8 in
struct intel_rps *rps.

>+	ssize_t ret;
>+
>+	ret = kstrtou32(buf, 0, &val);
>+	if (ret)
>+		return ret;
>+
>+	wakeref = intel_runtime_pm_get(dev_priv);
>+
>+	mutex_lock(&dev_priv->pcu_lock);
>+
>+	val = intel_freq_opcode(dev_priv, val);
>+
>+	if (val < rps->min_freq ||
>+	    val > rps->max_freq ||
>+	    val > rps->max_freq_softlimit) {
>+		mutex_unlock(&dev_priv->pcu_lock);
>+		intel_runtime_pm_put(dev_priv, wakeref);
>+		return -EINVAL;
>+	}
>+
>+	rps->idle_freq = val;
>+
>+	val = clamp_t(int, rps->cur_freq,
>+		      rps->idle_freq,
>+		      rps->max_freq_softlimit);

This should probably be clamped to u8 instead of int.

Vanshi

>+
>+	/*
>+	 * If the current freq is at the old idle freq we should
>+	 * ajust it to the new idle.  Calling *_set_rps will also
>+	 * update the interrupt limits and PMINTRMSK if ncessary.
>+	 */
>+	ret = intel_set_rps(dev_priv, val);
>+
>+	mutex_unlock(&dev_priv->pcu_lock);
>+
>+	intel_runtime_pm_put(dev_priv, wakeref);
>+
>+	return ret ?: count;
>+}
>+
> static DEVICE_ATTR_RO(gt_act_freq_mhz);
> static DEVICE_ATTR_RO(gt_cur_freq_mhz);
> static DEVICE_ATTR_RW(gt_boost_freq_mhz);
> static DEVICE_ATTR_RW(gt_max_freq_mhz);
> static DEVICE_ATTR_RW(gt_min_freq_mhz);
>+static DEVICE_ATTR_RW(gt_idle_freq_mhz);
>
> static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
>
>@@ -492,6 +550,7 @@ static const struct attribute * const gen6_attrs[] = {
> 	&dev_attr_gt_boost_freq_mhz.attr,
> 	&dev_attr_gt_max_freq_mhz.attr,
> 	&dev_attr_gt_min_freq_mhz.attr,
>+	&dev_attr_gt_idle_freq_mhz.attr,
> 	&dev_attr_gt_RP0_freq_mhz.attr,
> 	&dev_attr_gt_RP1_freq_mhz.attr,
> 	&dev_attr_gt_RPn_freq_mhz.attr,
>@@ -504,6 +563,7 @@ static const struct attribute * const vlv_attrs[] = {
> 	&dev_attr_gt_boost_freq_mhz.attr,
> 	&dev_attr_gt_max_freq_mhz.attr,
> 	&dev_attr_gt_min_freq_mhz.attr,
>+	&dev_attr_gt_idle_freq_mhz.attr,
> 	&dev_attr_gt_RP0_freq_mhz.attr,
> 	&dev_attr_gt_RP1_freq_mhz.attr,
> 	&dev_attr_gt_RPn_freq_mhz.attr,
>-- 
>2.19.2
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Bob Paauwe April 16, 2019, 3:30 p.m. UTC | #2
On Mon, 15 Apr 2019 17:33:30 -0700
Vanshidhar Konda <vanshidhar.r.konda@intel.com> wrote:

> On Mon, Apr 15, 2019 at 04:05:26PM -0700, Bob Paauwe wrote:
> >There are real-time use cases where having deterministic CPU processes
> >can be more important than GPU power/performance. Parking the GPU at a
> >specific freqency by setting idle, min and max prohibits the normal
> >dynamic GPU frequency switching which can introduce significant PCI-E
> >latency. This adds the ability to configure the GPU "idle" frequecy
> >using the same method that already exists for minimum and maximum
> >frequencies.
> >
> >In addition, parking the idle frequency may reduce spool up latencies
> >on GPU workloads.
> >
> >Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
> >---
> > drivers/gpu/drm/i915/i915_sysfs.c | 60 +++++++++++++++++++++++++++++++
> > 1 file changed, 60 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> >index 41313005af42..62612c23d514 100644
> >--- a/drivers/gpu/drm/i915/i915_sysfs.c
> >+++ b/drivers/gpu/drm/i915/i915_sysfs.c
> >@@ -454,11 +454,69 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
> > 	return ret ?: count;
> > }
> >
> >+static ssize_t gt_idle_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
> >+{
> >+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
> >+
> >+	return snprintf(buf, PAGE_SIZE, "%d\n",
> >+			intel_gpu_freq(dev_priv,
> >+				       dev_priv->gt_pm.rps.idle_freq));
> >+}
> >+
> >+static ssize_t gt_idle_freq_mhz_store(struct device *kdev,
> >+				      struct device_attribute *attr,
> >+				      const char *buf, size_t count)
> >+{
> >+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
> >+	struct intel_rps *rps = &dev_priv->gt_pm.rps;
> >+	intel_wakeref_t wakeref;
> >+	u32 val;  
> 
> val can probably just be u8. max_freq, min_freq, etc. are only u8 in
> struct intel_rps *rps.

Using u32 is consistent with all the other _store functions in the file
and changing it would also mean changing the kstrtou32 call below. I'd
rather this function stay consistent with the min/max/boost frequency
functions.  Changing to u8 would be a separate change and should be
applied to all the similar functions.

> 
> >+	ssize_t ret;
> >+
> >+	ret = kstrtou32(buf, 0, &val);
> >+	if (ret)
> >+		return ret;
> >+
> >+	wakeref = intel_runtime_pm_get(dev_priv);
> >+
> >+	mutex_lock(&dev_priv->pcu_lock);
> >+
> >+	val = intel_freq_opcode(dev_priv, val);
> >+
> >+	if (val < rps->min_freq ||
> >+	    val > rps->max_freq ||
> >+	    val > rps->max_freq_softlimit) {
> >+		mutex_unlock(&dev_priv->pcu_lock);
> >+		intel_runtime_pm_put(dev_priv, wakeref);
> >+		return -EINVAL;
> >+	}
> >+
> >+	rps->idle_freq = val;
> >+
> >+	val = clamp_t(int, rps->cur_freq,
> >+		      rps->idle_freq,
> >+		      rps->max_freq_softlimit);  
> 
> This should probably be clamped to u8 instead of int.

Similar to above, this is consistent with the other similar functions.

> 
> Vanshi
> 
> >+
> >+	/*
> >+	 * If the current freq is at the old idle freq we should
> >+	 * ajust it to the new idle.  Calling *_set_rps will also
> >+	 * update the interrupt limits and PMINTRMSK if ncessary.
> >+	 */
> >+	ret = intel_set_rps(dev_priv, val);
> >+
> >+	mutex_unlock(&dev_priv->pcu_lock);
> >+
> >+	intel_runtime_pm_put(dev_priv, wakeref);
> >+
> >+	return ret ?: count;
> >+}
> >+
> > static DEVICE_ATTR_RO(gt_act_freq_mhz);
> > static DEVICE_ATTR_RO(gt_cur_freq_mhz);
> > static DEVICE_ATTR_RW(gt_boost_freq_mhz);
> > static DEVICE_ATTR_RW(gt_max_freq_mhz);
> > static DEVICE_ATTR_RW(gt_min_freq_mhz);
> >+static DEVICE_ATTR_RW(gt_idle_freq_mhz);
> >
> > static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
> >
> >@@ -492,6 +550,7 @@ static const struct attribute * const gen6_attrs[] = {
> > 	&dev_attr_gt_boost_freq_mhz.attr,
> > 	&dev_attr_gt_max_freq_mhz.attr,
> > 	&dev_attr_gt_min_freq_mhz.attr,
> >+	&dev_attr_gt_idle_freq_mhz.attr,
> > 	&dev_attr_gt_RP0_freq_mhz.attr,
> > 	&dev_attr_gt_RP1_freq_mhz.attr,
> > 	&dev_attr_gt_RPn_freq_mhz.attr,
> >@@ -504,6 +563,7 @@ static const struct attribute * const vlv_attrs[] = {
> > 	&dev_attr_gt_boost_freq_mhz.attr,
> > 	&dev_attr_gt_max_freq_mhz.attr,
> > 	&dev_attr_gt_min_freq_mhz.attr,
> >+	&dev_attr_gt_idle_freq_mhz.attr,
> > 	&dev_attr_gt_RP0_freq_mhz.attr,
> > 	&dev_attr_gt_RP1_freq_mhz.attr,
> > 	&dev_attr_gt_RPn_freq_mhz.attr,
> >-- 
> >2.19.2
> >
> >_______________________________________________
> >Intel-gfx mailing list
> >Intel-gfx@lists.freedesktop.org
> >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Vanshidhar Konda April 16, 2019, 3:48 p.m. UTC | #3
On Tue, Apr 16, 2019 at 08:30:22AM -0700, Bob Paauwe wrote:
>On Mon, 15 Apr 2019 17:33:30 -0700
>Vanshidhar Konda <vanshidhar.r.konda@intel.com> wrote:
>
>> On Mon, Apr 15, 2019 at 04:05:26PM -0700, Bob Paauwe wrote:
>> >There are real-time use cases where having deterministic CPU processes
>> >can be more important than GPU power/performance. Parking the GPU at a
>> >specific freqency by setting idle, min and max prohibits the normal
>> >dynamic GPU frequency switching which can introduce significant PCI-E
>> >latency. This adds the ability to configure the GPU "idle" frequecy
>> >using the same method that already exists for minimum and maximum
>> >frequencies.
>> >
>> >In addition, parking the idle frequency may reduce spool up latencies
>> >on GPU workloads.
>> >
>> >Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com>
>> >---
>> > drivers/gpu/drm/i915/i915_sysfs.c | 60 +++++++++++++++++++++++++++++++
>> > 1 file changed, 60 insertions(+)
>> >
>> >diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
>> >index 41313005af42..62612c23d514 100644
>> >--- a/drivers/gpu/drm/i915/i915_sysfs.c
>> >+++ b/drivers/gpu/drm/i915/i915_sysfs.c
>> >@@ -454,11 +454,69 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev,
>> > 	return ret ?: count;
>> > }
>> >
>> >+static ssize_t gt_idle_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
>> >+{
>> >+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
>> >+
>> >+	return snprintf(buf, PAGE_SIZE, "%d\n",
>> >+			intel_gpu_freq(dev_priv,
>> >+				       dev_priv->gt_pm.rps.idle_freq));
>> >+}
>> >+
>> >+static ssize_t gt_idle_freq_mhz_store(struct device *kdev,
>> >+				      struct device_attribute *attr,
>> >+				      const char *buf, size_t count)
>> >+{
>> >+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
>> >+	struct intel_rps *rps = &dev_priv->gt_pm.rps;
>> >+	intel_wakeref_t wakeref;
>> >+	u32 val;
>>
>> val can probably just be u8. max_freq, min_freq, etc. are only u8 in
>> struct intel_rps *rps.
>
>Using u32 is consistent with all the other _store functions in the file
>and changing it would also mean changing the kstrtou32 call below. I'd
>rather this function stay consistent with the min/max/boost frequency
>functions.  Changing to u8 would be a separate change and should be
>applied to all the similar functions.
>
Thanks for pointing that out.

I recently joined the team, so not sure if you would like someone else
to review as well.

Reviewed-by: Vanshidhar Konda <vanshidhar.r.konda@intel.com>

>>
>> >+	ssize_t ret;
>> >+
>> >+	ret = kstrtou32(buf, 0, &val);
>> >+	if (ret)
>> >+		return ret;
>> >+
>> >+	wakeref = intel_runtime_pm_get(dev_priv);
>> >+
>> >+	mutex_lock(&dev_priv->pcu_lock);
>> >+
>> >+	val = intel_freq_opcode(dev_priv, val);
>> >+
>> >+	if (val < rps->min_freq ||
>> >+	    val > rps->max_freq ||
>> >+	    val > rps->max_freq_softlimit) {
>> >+		mutex_unlock(&dev_priv->pcu_lock);
>> >+		intel_runtime_pm_put(dev_priv, wakeref);
>> >+		return -EINVAL;
>> >+	}
>> >+
>> >+	rps->idle_freq = val;
>> >+
>> >+	val = clamp_t(int, rps->cur_freq,
>> >+		      rps->idle_freq,
>> >+		      rps->max_freq_softlimit);
>>
>> This should probably be clamped to u8 instead of int.
>
>Similar to above, this is consistent with the other similar functions.
>
>>
>> Vanshi
>>
>> >+
>> >+	/*
>> >+	 * If the current freq is at the old idle freq we should
>> >+	 * ajust it to the new idle.  Calling *_set_rps will also
>> >+	 * update the interrupt limits and PMINTRMSK if ncessary.
>> >+	 */
>> >+	ret = intel_set_rps(dev_priv, val);
>> >+
>> >+	mutex_unlock(&dev_priv->pcu_lock);
>> >+
>> >+	intel_runtime_pm_put(dev_priv, wakeref);
>> >+
>> >+	return ret ?: count;
>> >+}
>> >+
>> > static DEVICE_ATTR_RO(gt_act_freq_mhz);
>> > static DEVICE_ATTR_RO(gt_cur_freq_mhz);
>> > static DEVICE_ATTR_RW(gt_boost_freq_mhz);
>> > static DEVICE_ATTR_RW(gt_max_freq_mhz);
>> > static DEVICE_ATTR_RW(gt_min_freq_mhz);
>> >+static DEVICE_ATTR_RW(gt_idle_freq_mhz);
>> >
>> > static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
>> >
>> >@@ -492,6 +550,7 @@ static const struct attribute * const gen6_attrs[] = {
>> > 	&dev_attr_gt_boost_freq_mhz.attr,
>> > 	&dev_attr_gt_max_freq_mhz.attr,
>> > 	&dev_attr_gt_min_freq_mhz.attr,
>> >+	&dev_attr_gt_idle_freq_mhz.attr,
>> > 	&dev_attr_gt_RP0_freq_mhz.attr,
>> > 	&dev_attr_gt_RP1_freq_mhz.attr,
>> > 	&dev_attr_gt_RPn_freq_mhz.attr,
>> >@@ -504,6 +563,7 @@ static const struct attribute * const vlv_attrs[] = {
>> > 	&dev_attr_gt_boost_freq_mhz.attr,
>> > 	&dev_attr_gt_max_freq_mhz.attr,
>> > 	&dev_attr_gt_min_freq_mhz.attr,
>> >+	&dev_attr_gt_idle_freq_mhz.attr,
>> > 	&dev_attr_gt_RP0_freq_mhz.attr,
>> > 	&dev_attr_gt_RP1_freq_mhz.attr,
>> > 	&dev_attr_gt_RPn_freq_mhz.attr,
>> >--
>> >2.19.2
>> >
>> >_______________________________________________
>> >Intel-gfx mailing list
>> >Intel-gfx@lists.freedesktop.org
>> >https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
>-- 
>--
>Bob Paauwe
>Bob.J.Paauwe@intel.com
>IOTG / PED Software Organization
>Intel Corp.  Folsom, CA
>(916) 356-6193
>
Chris Wilson April 16, 2019, 3:56 p.m. UTC | #4
Quoting Bob Paauwe (2019-04-16 00:05:26)
> There are real-time use cases where having deterministic CPU processes
> can be more important than GPU power/performance. Parking the GPU at a
> specific freqency by setting idle, min and max prohibits the normal
> dynamic GPU frequency switching which can introduce significant PCI-E
> latency. This adds the ability to configure the GPU "idle" frequecy
> using the same method that already exists for minimum and maximum
> frequencies.

What exactly is the problem? We never use idle frequency while active,
always restarting at max(cur, rpe). So for the simple minded among us,
where is the igt demonstrating the issue?
-Chris
Bob Paauwe April 23, 2019, 6:05 p.m. UTC | #5
On Tue, 16 Apr 2019 16:56:26 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Quoting Bob Paauwe (2019-04-16 00:05:26)
> > There are real-time use cases where having deterministic CPU processes
> > can be more important than GPU power/performance. Parking the GPU at a
> > specific freqency by setting idle, min and max prohibits the normal
> > dynamic GPU frequency switching which can introduce significant PCI-E
> > latency. This adds the ability to configure the GPU "idle" frequecy
> > using the same method that already exists for minimum and maximum
> > frequencies.  
> 
> What exactly is the problem? We never use idle frequency while active,
> always restarting at max(cur, rpe). So for the simple minded among us,
> where is the igt demonstrating the issue?
> -Chris

To follow up and close this.  When I requested more details on the use
case and data for this request, I was informed that a different
solution is being pursued and this patch is no longer needed.

Thanks for the reviews and comments.
Bob

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index 41313005af42..62612c23d514 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -454,11 +454,69 @@  static ssize_t gt_min_freq_mhz_store(struct device *kdev,
 	return ret ?: count;
 }
 
+static ssize_t gt_idle_freq_mhz_show(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n",
+			intel_gpu_freq(dev_priv,
+				       dev_priv->gt_pm.rps.idle_freq));
+}
+
+static ssize_t gt_idle_freq_mhz_store(struct device *kdev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
+	struct intel_rps *rps = &dev_priv->gt_pm.rps;
+	intel_wakeref_t wakeref;
+	u32 val;
+	ssize_t ret;
+
+	ret = kstrtou32(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	wakeref = intel_runtime_pm_get(dev_priv);
+
+	mutex_lock(&dev_priv->pcu_lock);
+
+	val = intel_freq_opcode(dev_priv, val);
+
+	if (val < rps->min_freq ||
+	    val > rps->max_freq ||
+	    val > rps->max_freq_softlimit) {
+		mutex_unlock(&dev_priv->pcu_lock);
+		intel_runtime_pm_put(dev_priv, wakeref);
+		return -EINVAL;
+	}
+
+	rps->idle_freq = val;
+
+	val = clamp_t(int, rps->cur_freq,
+		      rps->idle_freq,
+		      rps->max_freq_softlimit);
+
+	/*
+	 * If the current freq is at the old idle freq we should
+	 * ajust it to the new idle.  Calling *_set_rps will also
+	 * update the interrupt limits and PMINTRMSK if ncessary.
+	 */
+	ret = intel_set_rps(dev_priv, val);
+
+	mutex_unlock(&dev_priv->pcu_lock);
+
+	intel_runtime_pm_put(dev_priv, wakeref);
+
+	return ret ?: count;
+}
+
 static DEVICE_ATTR_RO(gt_act_freq_mhz);
 static DEVICE_ATTR_RO(gt_cur_freq_mhz);
 static DEVICE_ATTR_RW(gt_boost_freq_mhz);
 static DEVICE_ATTR_RW(gt_max_freq_mhz);
 static DEVICE_ATTR_RW(gt_min_freq_mhz);
+static DEVICE_ATTR_RW(gt_idle_freq_mhz);
 
 static DEVICE_ATTR_RO(vlv_rpe_freq_mhz);
 
@@ -492,6 +550,7 @@  static const struct attribute * const gen6_attrs[] = {
 	&dev_attr_gt_boost_freq_mhz.attr,
 	&dev_attr_gt_max_freq_mhz.attr,
 	&dev_attr_gt_min_freq_mhz.attr,
+	&dev_attr_gt_idle_freq_mhz.attr,
 	&dev_attr_gt_RP0_freq_mhz.attr,
 	&dev_attr_gt_RP1_freq_mhz.attr,
 	&dev_attr_gt_RPn_freq_mhz.attr,
@@ -504,6 +563,7 @@  static const struct attribute * const vlv_attrs[] = {
 	&dev_attr_gt_boost_freq_mhz.attr,
 	&dev_attr_gt_max_freq_mhz.attr,
 	&dev_attr_gt_min_freq_mhz.attr,
+	&dev_attr_gt_idle_freq_mhz.attr,
 	&dev_attr_gt_RP0_freq_mhz.attr,
 	&dev_attr_gt_RP1_freq_mhz.attr,
 	&dev_attr_gt_RPn_freq_mhz.attr,