On ke, 2015-12-09 at 17:10 +0000, Chris Wilson wrote:
> Make sure that the RPS bottom-half is flushed before we set the idle
> frequency when we decide the GPU is idle. This should prevent any
> races
> with the bottom-half and setting the idle frequency, and ensures that
> the bottom-half is bounded by the GPU's rpm reference taken for when
> it
> is active (i.e. between gen6_rps_busy() and gen6_rps_idle()).
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index e655321385e2..bb796d4e9a3a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4401,11 +4401,15 @@ void gen6_rps_busy(struct drm_i915_private
> *dev_priv)
>
> void gen6_rps_idle(struct drm_i915_private *dev_priv)
> {
> - struct drm_device *dev = dev_priv->dev;
> + /* Flush our bottom-half so that it does not race with us
> + * setting the idle frequency and so that it is bounded by
> + * our rpm wakeref.
> + */
> + flush_work(&dev_priv->rps.work);
A (spurious) RPS interrupt could still reschedule the work, so could we
also explicitly disable the interrupts? Meaning to use
gen6_{disable,enable}_rps_interrupts() in gen6_rps_{idle,busy} and
making sure vlv_set_rps_idle(), gen6_set_rps() would not re-enable the
interrupts.
That would also make it possible to
remove gen6_{disable,enable}_rps_interrupts() from the
suspend/resume path.
> mutex_lock(&dev_priv->rps.hw_lock);
> if (dev_priv->rps.enabled) {
> - if (IS_VALLEYVIEW(dev))
> + if (IS_VALLEYVIEW(dev_priv))
> vlv_set_rps_idle(dev_priv);
> else
> gen6_set_rps(dev_priv->dev, dev_priv-
> >rps.idle_freq);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
