Re: [Intel-gfx] [PATCH 5/7] drm/i915: Error checks in gen6_set_rps
On Sun, Sep 09, 2012 at 07:25:40PM +0100, Chris Wilson wrote: On Fri, 7 Sep 2012 19:43:42 -0700, Ben Widawsky b...@bwidawsk.net wrote: With the new standardized sysfs interfaces we need to be a bit more careful about setting the RPS values. Because the sysfs code and the rps workqueue can run at the same time, if the sysfs setter wins the race to the mutex, the workqueue can come in and set a value which is out of range (ie. we're no longer protecting by RPINTLIM). I was not able to actually make this error occur in testing. Signed-off-by: Ben Widawsky b...@bwidawsk.net Good catch, care to squeeze the comment into a single line ;-) Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Applied all patches to dinq up to this one here, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Error checks in gen6_set_rps
On Fri, 7 Sep 2012 19:43:42 -0700, Ben Widawsky b...@bwidawsk.net wrote: With the new standardized sysfs interfaces we need to be a bit more careful about setting the RPS values. Because the sysfs code and the rps workqueue can run at the same time, if the sysfs setter wins the race to the mutex, the workqueue can come in and set a value which is out of range (ie. we're no longer protecting by RPINTLIM). I was not able to actually make this error occur in testing. Signed-off-by: Ben Widawsky b...@bwidawsk.net Good catch, care to squeeze the comment into a single line ;-) Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/7] drm/i915: Error checks in gen6_set_rps
With the new standardized sysfs interfaces we need to be a bit more
careful about setting the RPS values.
Because the sysfs code and the rps workqueue can run at the same time,
if the sysfs setter wins the race to the mutex, the workqueue can come
in and set a value which is out of range (ie. we're no longer protecting
by RPINTLIM).
I was not able to actually make this error occur in testing.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_irq.c | 8 +++-
drivers/gpu/drm/i915/intel_pm.c | 2 ++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d601013..e34b7d4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -382,7 +382,13 @@ static void gen6_pm_rps_work(struct work_struct *work)
else
new_delay = dev_priv-rps.cur_delay - 1;
- gen6_set_rps(dev_priv-dev, new_delay);
+ /* sysfs frequency interfaces may have snuck in while servicing the
+* interrupt
+*/
+ if (!(new_delay dev_priv-rps.max_delay ||
+ new_delay dev_priv-rps.min_delay)) {
+ gen6_set_rps(dev_priv-dev, new_delay);
+ }
mutex_unlock(dev_priv-dev-struct_mutex);
}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4e86037..82ca172 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2324,6 +2324,8 @@ void gen6_set_rps(struct drm_device *dev, u8 val)
u32 limits = gen6_rps_limits(dev_priv, val);
WARN_ON(!mutex_is_locked(dev-struct_mutex));
+ WARN_ON(val dev_priv-rps.max_delay);
+ WARN_ON(val dev_priv-rps.min_delay);
if (val == dev_priv-rps.cur_delay)
return;
--
1.7.12
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
