| Message ID | 1464252760-23902-10-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 26, 2016 at 09:52:39AM +0100, Chris Wilson wrote: > Instead of flushing the outstanding enabling, remember the requested > frequency to apply when the powersave work runs. I didn't see a patch to move the frequency init to happen before debugfs init. So methinks we still need the flush. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 30 ++------------------------ > drivers/gpu/drm/i915/i915_sysfs.c | 42 +++++++------------------------------ > 2 files changed, 10 insertions(+), 62 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 615cef736356..a49c7a0019b7 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1193,8 +1193,6 @@ static int i915_frequency_info(struct seq_file *m, void *unused) > > intel_runtime_pm_get(dev_priv); > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > if (IS_GEN5(dev)) { > u16 rgvswctl = I915_READ16(MEMSWCTL); > u16 rgvstat = I915_READ16(MEMSTAT_ILK); > @@ -1881,8 +1879,6 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused) > > intel_runtime_pm_get(dev_priv); > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); > if (ret) > goto out; > @@ -4970,20 +4966,11 @@ i915_max_freq_get(void *data, u64 *val) > { > struct drm_device *dev = data; > struct drm_i915_private *dev_priv = dev->dev_private; > - int ret; > > if (INTEL_INFO(dev)->gen < 6) > return -ENODEV; > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > - ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); > - if (ret) > - return ret; > - > *val = intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit); > - mutex_unlock(&dev_priv->rps.hw_lock); > - > return 0; > } > > @@ -4998,8 +4985,6 @@ i915_max_freq_set(void *data, u64 val) > if (INTEL_INFO(dev)->gen < 6) > return -ENODEV; > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > DRM_DEBUG_DRIVER("Manually setting max freq to %llu\n", val); > > ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); > @@ -5037,20 +5022,11 @@ i915_min_freq_get(void *data, u64 *val) > { > struct drm_device *dev = data; > struct drm_i915_private *dev_priv = dev->dev_private; > - int ret; > > - if (INTEL_INFO(dev)->gen < 6) > + if (INTEL_GEN(dev_priv) < 6) > return -ENODEV; > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > - ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); > - if (ret) > - return ret; > - > *val = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit); > - mutex_unlock(&dev_priv->rps.hw_lock); > - > return 0; > } > > @@ -5062,11 +5038,9 @@ i915_min_freq_set(void *data, u64 val) > u32 hw_max, hw_min; > int ret; > > - if (INTEL_INFO(dev)->gen < 6) > + if (INTEL_GEN(dev_priv) < 6) > return -ENODEV; > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > DRM_DEBUG_DRIVER("Manually setting min freq to %llu\n", val); > > ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); > diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c > index a6e90fe05a1e..915e97cdc4d5 100644 > --- a/drivers/gpu/drm/i915/i915_sysfs.c > +++ b/drivers/gpu/drm/i915/i915_sysfs.c > @@ -271,8 +271,6 @@ static ssize_t gt_act_freq_mhz_show(struct device *kdev, > struct drm_i915_private *dev_priv = dev->dev_private; > int ret; > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > intel_runtime_pm_get(dev_priv); > > mutex_lock(&dev_priv->rps.hw_lock); > @@ -303,19 +301,9 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev, > struct drm_minor *minor = dev_to_drm_minor(kdev); > struct drm_device *dev = minor->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - int ret; > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > - intel_runtime_pm_get(dev_priv); > - > - mutex_lock(&dev_priv->rps.hw_lock); > - ret = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq); > - mutex_unlock(&dev_priv->rps.hw_lock); > - > - intel_runtime_pm_put(dev_priv); > - > - return snprintf(buf, PAGE_SIZE, "%d\n", ret); > + return snprintf(buf, PAGE_SIZE, "%d\n", > + intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq)); > } > > static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev, > @@ -335,15 +323,10 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute > struct drm_minor *minor = dev_to_drm_minor(kdev); > struct drm_device *dev = minor->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - int ret; > - > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > > - mutex_lock(&dev_priv->rps.hw_lock); > - ret = intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit); > - mutex_unlock(&dev_priv->rps.hw_lock); > - > - return snprintf(buf, PAGE_SIZE, "%d\n", ret); > + return snprintf(buf, PAGE_SIZE, "%d\n", > + intel_gpu_freq(dev_priv, > + dev_priv->rps.max_freq_softlimit)); > } > > static ssize_t gt_max_freq_mhz_store(struct device *kdev, > @@ -360,8 +343,6 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev, > if (ret) > return ret; > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > intel_runtime_pm_get(dev_priv); > > mutex_lock(&dev_priv->rps.hw_lock); > @@ -403,15 +384,10 @@ static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute > struct drm_minor *minor = dev_to_drm_minor(kdev); > struct drm_device *dev = minor->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - int ret; > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > - mutex_lock(&dev_priv->rps.hw_lock); > - ret = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit); > - mutex_unlock(&dev_priv->rps.hw_lock); > - > - return snprintf(buf, PAGE_SIZE, "%d\n", ret); > + return snprintf(buf, PAGE_SIZE, "%d\n", > + intel_gpu_freq(dev_priv, > + dev_priv->rps.min_freq_softlimit)); > } > > static ssize_t gt_min_freq_mhz_store(struct device *kdev, > @@ -428,8 +404,6 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev, > if (ret) > return ret; > > - flush_delayed_work(&dev_priv->rps.delayed_resume_work); > - > intel_runtime_pm_get(dev_priv); > > mutex_lock(&dev_priv->rps.hw_lock); > -- > 2.8.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, May 26, 2016 at 02:45:09PM +0300, Ville Syrjälä wrote: > On Thu, May 26, 2016 at 09:52:39AM +0100, Chris Wilson wrote: > > Instead of flushing the outstanding enabling, remember the requested > > frequency to apply when the powersave work runs. > > I didn't see a patch to move the frequency init to happen before > debugfs init. So methinks we still need the flush. Considering debugfs_init is exposing us to userspace, it should be part of the registration phase (i.e. last thing) after we have all the hw probed / minimally prepared. -Chris
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 615cef736356..a49c7a0019b7 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1193,8 +1193,6 @@ static int i915_frequency_info(struct seq_file *m, void *unused) intel_runtime_pm_get(dev_priv); - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - if (IS_GEN5(dev)) { u16 rgvswctl = I915_READ16(MEMSWCTL); u16 rgvstat = I915_READ16(MEMSTAT_ILK); @@ -1881,8 +1879,6 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused) intel_runtime_pm_get(dev_priv); - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); if (ret) goto out; @@ -4970,20 +4966,11 @@ i915_max_freq_get(void *data, u64 *val) { struct drm_device *dev = data; struct drm_i915_private *dev_priv = dev->dev_private; - int ret; if (INTEL_INFO(dev)->gen < 6) return -ENODEV; - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - - ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); - if (ret) - return ret; - *val = intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit); - mutex_unlock(&dev_priv->rps.hw_lock); - return 0; } @@ -4998,8 +4985,6 @@ i915_max_freq_set(void *data, u64 val) if (INTEL_INFO(dev)->gen < 6) return -ENODEV; - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - DRM_DEBUG_DRIVER("Manually setting max freq to %llu\n", val); ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); @@ -5037,20 +5022,11 @@ i915_min_freq_get(void *data, u64 *val) { struct drm_device *dev = data; struct drm_i915_private *dev_priv = dev->dev_private; - int ret; - if (INTEL_INFO(dev)->gen < 6) + if (INTEL_GEN(dev_priv) < 6) return -ENODEV; - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - - ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); - if (ret) - return ret; - *val = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit); - mutex_unlock(&dev_priv->rps.hw_lock); - return 0; } @@ -5062,11 +5038,9 @@ i915_min_freq_set(void *data, u64 val) u32 hw_max, hw_min; int ret; - if (INTEL_INFO(dev)->gen < 6) + if (INTEL_GEN(dev_priv) < 6) return -ENODEV; - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - DRM_DEBUG_DRIVER("Manually setting min freq to %llu\n", val); ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock); diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index a6e90fe05a1e..915e97cdc4d5 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -271,8 +271,6 @@ static ssize_t gt_act_freq_mhz_show(struct device *kdev, struct drm_i915_private *dev_priv = dev->dev_private; int ret; - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - intel_runtime_pm_get(dev_priv); mutex_lock(&dev_priv->rps.hw_lock); @@ -303,19 +301,9 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev, struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; - int ret; - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - - intel_runtime_pm_get(dev_priv); - - mutex_lock(&dev_priv->rps.hw_lock); - ret = intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq); - mutex_unlock(&dev_priv->rps.hw_lock); - - intel_runtime_pm_put(dev_priv); - - return snprintf(buf, PAGE_SIZE, "%d\n", ret); + return snprintf(buf, PAGE_SIZE, "%d\n", + intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq)); } static ssize_t vlv_rpe_freq_mhz_show(struct device *kdev, @@ -335,15 +323,10 @@ static ssize_t gt_max_freq_mhz_show(struct device *kdev, struct device_attribute struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; - int ret; - - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - mutex_lock(&dev_priv->rps.hw_lock); - ret = intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit); - mutex_unlock(&dev_priv->rps.hw_lock); - - return snprintf(buf, PAGE_SIZE, "%d\n", ret); + return snprintf(buf, PAGE_SIZE, "%d\n", + intel_gpu_freq(dev_priv, + dev_priv->rps.max_freq_softlimit)); } static ssize_t gt_max_freq_mhz_store(struct device *kdev, @@ -360,8 +343,6 @@ static ssize_t gt_max_freq_mhz_store(struct device *kdev, if (ret) return ret; - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - intel_runtime_pm_get(dev_priv); mutex_lock(&dev_priv->rps.hw_lock); @@ -403,15 +384,10 @@ static ssize_t gt_min_freq_mhz_show(struct device *kdev, struct device_attribute struct drm_minor *minor = dev_to_drm_minor(kdev); struct drm_device *dev = minor->dev; struct drm_i915_private *dev_priv = dev->dev_private; - int ret; - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - - mutex_lock(&dev_priv->rps.hw_lock); - ret = intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit); - mutex_unlock(&dev_priv->rps.hw_lock); - - return snprintf(buf, PAGE_SIZE, "%d\n", ret); + return snprintf(buf, PAGE_SIZE, "%d\n", + intel_gpu_freq(dev_priv, + dev_priv->rps.min_freq_softlimit)); } static ssize_t gt_min_freq_mhz_store(struct device *kdev, @@ -428,8 +404,6 @@ static ssize_t gt_min_freq_mhz_store(struct device *kdev, if (ret) return ret; - flush_delayed_work(&dev_priv->rps.delayed_resume_work); - intel_runtime_pm_get(dev_priv); mutex_lock(&dev_priv->rps.hw_lock);
Instead of flushing the outstanding enabling, remember the requested frequency to apply when the powersave work runs. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_debugfs.c | 30 ++------------------------ drivers/gpu/drm/i915/i915_sysfs.c | 42 +++++++------------------------------ 2 files changed, 10 insertions(+), 62 deletions(-)