| Message ID | 1343165630-21604-4-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 24 Jul 2012 23:33:44 +0200 Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > - Take the dev->struct_mutex around access the corresponding state > (and adjusting the rps hw state). > - Add an assert to gen6_set_rps to ensure we don't forget about this > in the future. > - Don't set up the min/max_freq files if it doesn't apply to the hw. > And do the same for the gen6+ cache sharing file while at it. > > v2: Move the gen6+ checks into the read/write callbacks. Thanks to the > awesome drm midlayer we can't check this when registering the debugfs > files, because the driver is not yet fully set up, specifically the > ->load callback hasn't run yet. > > Oh how I despise & loathe this disaster ... > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> I think it's wise to use some kind of HAS_RPS() macro, but otherwise: Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 27 +++++++++++++++++++++++++++ > drivers/gpu/drm/i915/intel_pm.c | 2 ++ > 2 files changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 1312b79..2499610 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1710,8 +1710,13 @@ i915_max_freq_read(struct file *filp, > char buf[80]; > int len; > > + if (!(IS_GEN6(dev) || IS_GEN7(dev))) > + return -ENODEV; > + > + mutex_lock(&dev->struct_mutex); > len = snprintf(buf, sizeof(buf), > "max freq: %d\n", dev_priv->max_delay * 50); > + mutex_unlock(&dev->struct_mutex); > > if (len > sizeof(buf)) > len = sizeof(buf); > @@ -1730,6 +1735,9 @@ i915_max_freq_write(struct file *filp, > char buf[20]; > int val = 1; > > + if (!(IS_GEN6(dev) || IS_GEN7(dev))) > + return -ENODEV; > + > if (cnt > 0) { > if (cnt > sizeof(buf) - 1) > return -EINVAL; > @@ -1743,12 +1751,14 @@ i915_max_freq_write(struct file *filp, > > DRM_DEBUG_DRIVER("Manually setting max freq to %d\n", val); > > + mutex_lock(&dev->struct_mutex); > /* > * Turbo will still be enabled, but won't go above the set value. > */ > dev_priv->max_delay = val / 50; > > gen6_set_rps(dev, val / 50); > + mutex_unlock(&dev->struct_mutex); > > return cnt; > } > @@ -1770,8 +1780,13 @@ i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max, > char buf[80]; > int len; > > + if (!(IS_GEN6(dev) || IS_GEN7(dev))) > + return -ENODEV; > + > + mutex_lock(&dev->struct_mutex); > len = snprintf(buf, sizeof(buf), > "min freq: %d\n", dev_priv->min_delay * 50); > + mutex_unlock(&dev->struct_mutex); > > if (len > sizeof(buf)) > len = sizeof(buf); > @@ -1788,6 +1803,9 @@ i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt, > char buf[20]; > int val = 1; > > + if (!(IS_GEN6(dev) || IS_GEN7(dev))) > + return -ENODEV; > + > if (cnt > 0) { > if (cnt > sizeof(buf) - 1) > return -EINVAL; > @@ -1801,12 +1819,14 @@ i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt, > > DRM_DEBUG_DRIVER("Manually setting min freq to %d\n", val); > > + mutex_lock(&dev->struct_mutex); > /* > * Turbo will still be enabled, but won't go below the set value. > */ > dev_priv->min_delay = val / 50; > > gen6_set_rps(dev, val / 50); > + mutex_unlock(&dev->struct_mutex); > > return cnt; > } > @@ -1831,6 +1851,9 @@ i915_cache_sharing_read(struct file *filp, > u32 snpcr; > int len; > > + if (!(IS_GEN6(dev) || IS_GEN7(dev))) > + return -ENODEV; > + > mutex_lock(&dev_priv->dev->struct_mutex); > snpcr = I915_READ(GEN6_MBCUNIT_SNPCR); > mutex_unlock(&dev_priv->dev->struct_mutex); > @@ -1857,6 +1880,9 @@ i915_cache_sharing_write(struct file *filp, > u32 snpcr; > int val = 1; > > + if (!(IS_GEN6(dev) || IS_GEN7(dev))) > + return -ENODEV; > + > if (cnt > 0) { > if (cnt > sizeof(buf) - 1) > return -EINVAL; > @@ -2061,6 +2087,7 @@ int i915_debugfs_init(struct drm_minor *minor) > &i915_cache_sharing_fops); > if (ret) > return ret; > + > ret = i915_debugfs_create(minor->debugfs_root, minor, > "i915_ring_stop", > &i915_ring_stop_fops); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 21a0088..d5af41b 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2272,6 +2272,8 @@ void gen6_set_rps(struct drm_device *dev, u8 val) > struct drm_i915_private *dev_priv = dev->dev_private; > u32 limits; > > + WARN_ON(!mutex_is_locked(&dev->struct_mutex)); > + > limits = 0; > if (val >= dev_priv->max_delay) > val = dev_priv->max_delay;
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 1312b79..2499610 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1710,8 +1710,13 @@ i915_max_freq_read(struct file *filp, char buf[80]; int len; + if (!(IS_GEN6(dev) || IS_GEN7(dev))) + return -ENODEV; + + mutex_lock(&dev->struct_mutex); len = snprintf(buf, sizeof(buf), "max freq: %d\n", dev_priv->max_delay * 50); + mutex_unlock(&dev->struct_mutex); if (len > sizeof(buf)) len = sizeof(buf); @@ -1730,6 +1735,9 @@ i915_max_freq_write(struct file *filp, char buf[20]; int val = 1; + if (!(IS_GEN6(dev) || IS_GEN7(dev))) + return -ENODEV; + if (cnt > 0) { if (cnt > sizeof(buf) - 1) return -EINVAL; @@ -1743,12 +1751,14 @@ i915_max_freq_write(struct file *filp, DRM_DEBUG_DRIVER("Manually setting max freq to %d\n", val); + mutex_lock(&dev->struct_mutex); /* * Turbo will still be enabled, but won't go above the set value. */ dev_priv->max_delay = val / 50; gen6_set_rps(dev, val / 50); + mutex_unlock(&dev->struct_mutex); return cnt; } @@ -1770,8 +1780,13 @@ i915_min_freq_read(struct file *filp, char __user *ubuf, size_t max, char buf[80]; int len; + if (!(IS_GEN6(dev) || IS_GEN7(dev))) + return -ENODEV; + + mutex_lock(&dev->struct_mutex); len = snprintf(buf, sizeof(buf), "min freq: %d\n", dev_priv->min_delay * 50); + mutex_unlock(&dev->struct_mutex); if (len > sizeof(buf)) len = sizeof(buf); @@ -1788,6 +1803,9 @@ i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt, char buf[20]; int val = 1; + if (!(IS_GEN6(dev) || IS_GEN7(dev))) + return -ENODEV; + if (cnt > 0) { if (cnt > sizeof(buf) - 1) return -EINVAL; @@ -1801,12 +1819,14 @@ i915_min_freq_write(struct file *filp, const char __user *ubuf, size_t cnt, DRM_DEBUG_DRIVER("Manually setting min freq to %d\n", val); + mutex_lock(&dev->struct_mutex); /* * Turbo will still be enabled, but won't go below the set value. */ dev_priv->min_delay = val / 50; gen6_set_rps(dev, val / 50); + mutex_unlock(&dev->struct_mutex); return cnt; } @@ -1831,6 +1851,9 @@ i915_cache_sharing_read(struct file *filp, u32 snpcr; int len; + if (!(IS_GEN6(dev) || IS_GEN7(dev))) + return -ENODEV; + mutex_lock(&dev_priv->dev->struct_mutex); snpcr = I915_READ(GEN6_MBCUNIT_SNPCR); mutex_unlock(&dev_priv->dev->struct_mutex); @@ -1857,6 +1880,9 @@ i915_cache_sharing_write(struct file *filp, u32 snpcr; int val = 1; + if (!(IS_GEN6(dev) || IS_GEN7(dev))) + return -ENODEV; + if (cnt > 0) { if (cnt > sizeof(buf) - 1) return -EINVAL; @@ -2061,6 +2087,7 @@ int i915_debugfs_init(struct drm_minor *minor) &i915_cache_sharing_fops); if (ret) return ret; + ret = i915_debugfs_create(minor->debugfs_root, minor, "i915_ring_stop", &i915_ring_stop_fops); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 21a0088..d5af41b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2272,6 +2272,8 @@ void gen6_set_rps(struct drm_device *dev, u8 val) struct drm_i915_private *dev_priv = dev->dev_private; u32 limits; + WARN_ON(!mutex_is_locked(&dev->struct_mutex)); + limits = 0; if (val >= dev_priv->max_delay) val = dev_priv->max_delay;
- Take the dev->struct_mutex around access the corresponding state (and adjusting the rps hw state). - Add an assert to gen6_set_rps to ensure we don't forget about this in the future. - Don't set up the min/max_freq files if it doesn't apply to the hw. And do the same for the gen6+ cache sharing file while at it. v2: Move the gen6+ checks into the read/write callbacks. Thanks to the awesome drm midlayer we can't check this when registering the debugfs files, because the driver is not yet fully set up, specifically the ->load callback hasn't run yet. Oh how I despise & loathe this disaster ... Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_debugfs.c | 27 +++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_pm.c | 2 ++ 2 files changed, 29 insertions(+)