| Message ID | 1471669765-5935-12-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Aug 20, 2016 at 10:39:10AM +0530, Sagar Arun Kamble wrote: > From: Tom O'Rourke <Tom.O'Rourke@intel.com> > > When SLPC is controlling requested frequency, the rps.cur_freq > value is not used to make the frequency request. > > Before using rps.cur_freq in sysfs or debugfs, read > requested frequency from register to get the value > most recently requested by SLPC firmware. > > v2: replace HAS_SLPC with intel_slpc_active (Paulo) > v3: Avoid magic numbers (Nick) > Use a function for repeated code (Jon) > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++ > drivers/gpu/drm/i915/i915_drv.h | 5 +++++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/i915_sysfs.c | 8 ++++++++ > 4 files changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 01ae5ee..a99a3f6 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1110,6 +1110,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused) > > intel_runtime_pm_get(dev_priv); > > + if (intel_slpc_active(dev_priv)) > + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); > + > if (IS_GEN5(dev)) { > u16 rgvswctl = I915_READ16(MEMSWCTL); > u16 rgvstat = I915_READ16(MEMSTAT_ILK); > @@ -2372,6 +2375,9 @@ static int i915_rps_boost_info(struct seq_file *m, void *data) > struct drm_i915_private *dev_priv = to_i915(dev); > struct drm_file *file; > > + if (intel_slpc_active(dev_priv)) > + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); > + > seq_printf(m, "RPS enabled? %d\n", dev_priv->rps.enabled); > seq_printf(m, "GPU busy? %s [%x]\n", > yesno(dev_priv->gt.awake), dev_priv->gt.active_engines); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 764fad0..fcd2e98 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3913,4 +3913,9 @@ bool i915_memcpy_from_wc(void *dst, const void *src, unsigned long len); > __T; \ > }) > > +static inline u8 gen9_read_requested_freq(struct drm_i915_private *dev_priv) > +{ > + return (u8) GEN9_GET_FREQUENCY(I915_READ(GEN6_RPNSWREQ)); > +} > + > #endif > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index d4adf28..1654245 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6997,6 +6997,7 @@ enum { > #define GEN6_FREQUENCY(x) ((x)<<25) > #define HSW_FREQUENCY(x) ((x)<<24) > #define GEN9_FREQUENCY(x) ((x)<<23) > +#define GEN9_GET_FREQUENCY(x) ((x)>>23) > #define GEN6_OFFSET(x) ((x)<<19) > #define GEN6_AGGRESSIVE_TURBO (0<<15) > #define GEN6_RC_VIDEO_FREQ _MMIO(0xA00C) > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index f1ffde7..8404816 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -302,6 +302,14 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev, > struct drm_device *dev = minor->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > > + if (intel_slpc_active(dev_priv)) { > + intel_runtime_pm_get(dev_priv); > + mutex_lock(&dev_priv->rps.hw_lock); > + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); > + mutex_unlock(&dev_priv->rps.hw_lock); > + intel_runtime_pm_put(dev_priv); > + } > + > return snprintf(buf, PAGE_SIZE, "%d\n", > intel_gpu_freq(dev_priv, > dev_priv->rps.cur_freq)); Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
On Sat, Aug 20, 2016 at 10:39:10AM +0530, Sagar Arun Kamble wrote: > From: Tom O'Rourke <Tom.O'Rourke@intel.com> > > When SLPC is controlling requested frequency, the rps.cur_freq > value is not used to make the frequency request. > > Before using rps.cur_freq in sysfs or debugfs, read > requested frequency from register to get the value > most recently requested by SLPC firmware. > > v2: replace HAS_SLPC with intel_slpc_active (Paulo) > v3: Avoid magic numbers (Nick) > Use a function for repeated code (Jon) > > Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++ > drivers/gpu/drm/i915/i915_drv.h | 5 +++++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/i915_sysfs.c | 8 ++++++++ > 4 files changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 01ae5ee..a99a3f6 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1110,6 +1110,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused) > > intel_runtime_pm_get(dev_priv); > > + if (intel_slpc_active(dev_priv)) > + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); Do not alter cur_freq here, as we print out RPNSWEQ and updating cur_freq defeats the purpose of showing the internal value vs the hw value. Instead add "SLPC active" to the output. > + > if (IS_GEN5(dev)) { > u16 rgvswctl = I915_READ16(MEMSWCTL); > u16 rgvstat = I915_READ16(MEMSTAT_ILK); > @@ -2372,6 +2375,9 @@ static int i915_rps_boost_info(struct seq_file *m, void *data) > struct drm_i915_private *dev_priv = to_i915(dev); > struct drm_file *file; if (intel_slpc_active(dev_priv)) return -ENODEV; > > + if (intel_slpc_active(dev_priv)) > + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); > + > seq_printf(m, "RPS enabled? %d\n", dev_priv->rps.enabled); > seq_printf(m, "GPU busy? %s [%x]\n", > yesno(dev_priv->gt.awake), dev_priv->gt.active_engines); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 764fad0..fcd2e98 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3913,4 +3913,9 @@ bool i915_memcpy_from_wc(void *dst, const void *src, unsigned long len); > __T; \ > }) > > +static inline u8 gen9_read_requested_freq(struct drm_i915_private *dev_priv) > +{ > + return (u8) GEN9_GET_FREQUENCY(I915_READ(GEN6_RPNSWREQ)); > +} Move to sysfs and look carefully at what you wrote. > #endif > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index d4adf28..1654245 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6997,6 +6997,7 @@ enum { > #define GEN6_FREQUENCY(x) ((x)<<25) > #define HSW_FREQUENCY(x) ((x)<<24) > #define GEN9_FREQUENCY(x) ((x)<<23) > +#define GEN9_GET_FREQUENCY(x) ((x)>>23) > #define GEN6_OFFSET(x) ((x)<<19) > #define GEN6_AGGRESSIVE_TURBO (0<<15) > #define GEN6_RC_VIDEO_FREQ _MMIO(0xA00C) > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index f1ffde7..8404816 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -302,6 +302,14 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev, > struct drm_device *dev = minor->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > > + if (intel_slpc_active(dev_priv)) { > + intel_runtime_pm_get(dev_priv); Use get_if_in_use and just show a stale value when the hw is asleep would be my preference. cur_freq is just our request, act_freq is the actual hw value. > + mutex_lock(&dev_priv->rps.hw_lock); Useless mutex. > + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); > + mutex_unlock(&dev_priv->rps.hw_lock); > + intel_runtime_pm_put(dev_priv); > + } > + > return snprintf(buf, PAGE_SIZE, "%d\n", > intel_gpu_freq(dev_priv, > dev_priv->rps.cur_freq)); > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 8/20/2016 1:45 PM, Chris Wilson wrote: > On Sat, Aug 20, 2016 at 10:39:10AM +0530, Sagar Arun Kamble wrote: >> From: Tom O'Rourke <Tom.O'Rourke@intel.com> >> >> When SLPC is controlling requested frequency, the rps.cur_freq >> value is not used to make the frequency request. >> >> Before using rps.cur_freq in sysfs or debugfs, read >> requested frequency from register to get the value >> most recently requested by SLPC firmware. >> >> v2: replace HAS_SLPC with intel_slpc_active (Paulo) >> v3: Avoid magic numbers (Nick) >> Use a function for repeated code (Jon) >> >> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 6 ++++++ >> drivers/gpu/drm/i915/i915_drv.h | 5 +++++ >> drivers/gpu/drm/i915/i915_reg.h | 1 + >> drivers/gpu/drm/i915/i915_sysfs.c | 8 ++++++++ >> 4 files changed, 20 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >> index 01ae5ee..a99a3f6 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -1110,6 +1110,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused) >> >> intel_runtime_pm_get(dev_priv); >> >> + if (intel_slpc_active(dev_priv)) >> + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); > Do not alter cur_freq here, as we print out RPNSWEQ and updating > cur_freq defeats the purpose of showing the internal value vs the hw > value. > > Instead add "SLPC active" to the output. Fixed in the next series. > >> + >> if (IS_GEN5(dev)) { >> u16 rgvswctl = I915_READ16(MEMSWCTL); >> u16 rgvstat = I915_READ16(MEMSTAT_ILK); >> @@ -2372,6 +2375,9 @@ static int i915_rps_boost_info(struct seq_file *m, void *data) >> struct drm_i915_private *dev_priv = to_i915(dev); >> struct drm_file *file; > if (intel_slpc_active(dev_priv)) > return -ENODEV; Fixed in the next series. >> >> + if (intel_slpc_active(dev_priv)) >> + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); >> + >> seq_printf(m, "RPS enabled? %d\n", dev_priv->rps.enabled); >> seq_printf(m, "GPU busy? %s [%x]\n", >> yesno(dev_priv->gt.awake), dev_priv->gt.active_engines); >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 764fad0..fcd2e98 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -3913,4 +3913,9 @@ bool i915_memcpy_from_wc(void *dst, const void *src, unsigned long len); >> __T; \ >> }) >> >> +static inline u8 gen9_read_requested_freq(struct drm_i915_private *dev_priv) >> +{ >> + return (u8) GEN9_GET_FREQUENCY(I915_READ(GEN6_RPNSWREQ)); >> +} Removed in the next series. > Move to sysfs and look carefully at what you wrote. > >> #endif >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index d4adf28..1654245 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -6997,6 +6997,7 @@ enum { >> #define GEN6_FREQUENCY(x) ((x)<<25) >> #define HSW_FREQUENCY(x) ((x)<<24) >> #define GEN9_FREQUENCY(x) ((x)<<23) >> +#define GEN9_GET_FREQUENCY(x) ((x)>>23) >> #define GEN6_OFFSET(x) ((x)<<19) >> #define GEN6_AGGRESSIVE_TURBO (0<<15) >> #define GEN6_RC_VIDEO_FREQ _MMIO(0xA00C) >> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c >> index f1ffde7..8404816 100644 >> --- a/drivers/gpu/drm/i915/i915_sysfs.c >> +++ b/drivers/gpu/drm/i915/i915_sysfs.c >> @@ -302,6 +302,14 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev, >> struct drm_device *dev = minor->dev; >> struct drm_i915_private *dev_priv = to_i915(dev); >> >> + if (intel_slpc_active(dev_priv)) { >> + intel_runtime_pm_get(dev_priv); > Use get_if_in_use and just show a stale value when the hw is asleep > would be my preference. cur_freq is just our request, act_freq is the > actual hw value. With cur_freq not making sense currently with SLPC, I am removing this altogether and adding new sysfs interface for knowing HW requested frequency which will be ideally SLPC requested. > >> + mutex_lock(&dev_priv->rps.hw_lock); > Useless mutex. > >> + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); >> + mutex_unlock(&dev_priv->rps.hw_lock); >> + intel_runtime_pm_put(dev_priv); >> + } >> + >> return snprintf(buf, PAGE_SIZE, "%d\n", >> intel_gpu_freq(dev_priv, >> dev_priv->rps.cur_freq)); >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 01ae5ee..a99a3f6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1110,6 +1110,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused) intel_runtime_pm_get(dev_priv); + if (intel_slpc_active(dev_priv)) + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); + if (IS_GEN5(dev)) { u16 rgvswctl = I915_READ16(MEMSWCTL); u16 rgvstat = I915_READ16(MEMSTAT_ILK); @@ -2372,6 +2375,9 @@ static int i915_rps_boost_info(struct seq_file *m, void *data) struct drm_i915_private *dev_priv = to_i915(dev); struct drm_file *file; + if (intel_slpc_active(dev_priv)) + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); + seq_printf(m, "RPS enabled? %d\n", dev_priv->rps.enabled); seq_printf(m, "GPU busy? %s [%x]\n", yesno(dev_priv->gt.awake), dev_priv->gt.active_engines); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 764fad0..fcd2e98 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3913,4 +3913,9 @@ bool i915_memcpy_from_wc(void *dst, const void *src, unsigned long len); __T; \ }) +static inline u8 gen9_read_requested_freq(struct drm_i915_private *dev_priv) +{ + return (u8) GEN9_GET_FREQUENCY(I915_READ(GEN6_RPNSWREQ)); +} + #endif diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index d4adf28..1654245 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6997,6 +6997,7 @@ enum { #define GEN6_FREQUENCY(x) ((x)<<25) #define HSW_FREQUENCY(x) ((x)<<24) #define GEN9_FREQUENCY(x) ((x)<<23) +#define GEN9_GET_FREQUENCY(x) ((x)>>23) #define GEN6_OFFSET(x) ((x)<<19) #define GEN6_AGGRESSIVE_TURBO (0<<15) #define GEN6_RC_VIDEO_FREQ _MMIO(0xA00C) diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index f1ffde7..8404816 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -302,6 +302,14 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev, struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = to_i915(dev); + if (intel_slpc_active(dev_priv)) { + intel_runtime_pm_get(dev_priv); + mutex_lock(&dev_priv->rps.hw_lock); + dev_priv->rps.cur_freq = gen9_read_requested_freq(dev_priv); + mutex_unlock(&dev_priv->rps.hw_lock); + intel_runtime_pm_put(dev_priv); + } + return snprintf(buf, PAGE_SIZE, "%d\n", intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq));