Chris Wilson <[email protected]> writes: > Instead of flushing the outstanding enabling, remember the requested > frequency to apply when the powersave work runs. >
As all the query side values will get initialized only after the first rc6 initialization is done, you will introduce a window of ~1second where userspace will get uninitliazed values from sysfs reads. The debugfs side with i915_frequency_info and i915_ring_freq_table are fine as it is a boon that they will not force a flush and show the current hardware/driver state. The 'why' part of the commit message is on the thin side, so I don't know if these will nullify your goals but: consider leaving the flushing still in place for sysfs query side or split out the rps freq value initialization from rps enabling. -Mika > Signed-off-by: Chris Wilson <[email protected]> > Cc: Ville Syrjälä <[email protected]> > --- > 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 e4409ddc60e5..7c4a60b87eb5 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1205,8 +1205,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); > @@ -1896,8 +1894,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; > @@ -4957,20 +4953,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; > } > > @@ -4985,8 +4972,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); > @@ -5024,20 +5009,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; > } > > @@ -5049,11 +5025,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 > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
