On Sun, Aug 25, 2013 at 08:45:34PM +0100, Chris Wilson wrote:
> Obtaining the forcwake requires expensive and time consuming
> serialisation. And we often try to obtain the forcewake multiple times
> in very quick succession. We can reduce the overhead of these sequences
> by delaying the forcewake release, and so not hammer the hw quite so
> hard.
>
> I was hoping this would help with the spurious
> [drm:__gen6_gt_force_wake_mt_get] *ERROR* Timed out waiting for forcewake old ack to clear.
> found on Haswell. Alas not.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 2 ++
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/gpu/drm/i915/intel_uncore.c | 33 +++++++++++++++++++++++++++++++--
> 3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 883990f..c0fb23e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1800,6 +1800,8 @@ int i915_driver_unload(struct drm_device *dev)
>
> dev_priv->gtt.base.cleanup(&dev_priv->gtt.base);
>
> + intel_uncore_fini(dev);
> +
We've already unmapped the registers.
> if (dev_priv->slab)
> kmem_cache_destroy(dev_priv->slab);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a6354c3..8c93d93 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -406,6 +406,8 @@ struct intel_uncore {
>
> unsigned fifo_count;
> unsigned forcewake_count;
> +
> + struct delayed_work force_wake_work;
> };
>
> #define DEV_INFO_FOR_EACH_FLAG(func, sep) \
> @@ -1792,6 +1794,7 @@ extern void intel_uncore_early_sanitize(struct drm_device *dev);
> extern void intel_uncore_init(struct drm_device *dev);
> extern void intel_uncore_clear_errors(struct drm_device *dev);
> extern void intel_uncore_check_errors(struct drm_device *dev);
> +extern void intel_uncore_fini(struct drm_device *dev);
>
> void
> i915_enable_pipestat(drm_i915_private_t *dev_priv, int pipe, u32 mask);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 8f5bc86..50fdad7 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -204,6 +204,18 @@ static void vlv_force_wake_put(struct drm_i915_private *dev_priv)
> gen6_gt_check_fifodbg(dev_priv);
> }
>
> +static void gen6_force_wake_work(struct work_struct *work)
> +{
> + struct drm_i915_private *dev_priv =
> + container_of(work, typeof(*dev_priv), uncore.force_wake_work.work);
> + unsigned long irqflags;
> +
> + spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> + if (--dev_priv->uncore.forcewake_count == 0)
> + dev_priv->uncore.funcs.force_wake_put(dev_priv);
> + spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +}
> +
> void intel_uncore_early_sanitize(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -216,6 +228,9 @@ void intel_uncore_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> + INIT_DELAYED_WORK(&dev_priv->uncore.force_wake_work,
> + gen6_force_wake_work);
> +
> if (IS_VALLEYVIEW(dev)) {
> dev_priv->uncore.funcs.force_wake_get = vlv_force_wake_get;
> dev_priv->uncore.funcs.force_wake_put = vlv_force_wake_put;
> @@ -261,6 +276,13 @@ void intel_uncore_init(struct drm_device *dev)
> }
> }
>
> +void intel_uncore_fini(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + cancel_delayed_work_sync(&dev_priv->uncore.force_wake_work);
Aren't we potentially leaking a forcewake here? Or do we clear it
forcefully somewhere else?
> +}
> +
> void intel_uncore_sanitize(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -301,8 +323,15 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
> unsigned long irqflags;
>
> spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> - if (--dev_priv->uncore.forcewake_count == 0)
> - dev_priv->uncore.funcs.force_wake_put(dev_priv);
> + if (--dev_priv->uncore.forcewake_count == 0) {
> + if (dev_priv->uncore.force_wake_work.work.func) {
> + dev_priv->uncore.forcewake_count++;
> + mod_delayed_work(dev_priv->wq,
> + &dev_priv->uncore.force_wake_work,
> + 1);
> + } else
> + dev_priv->uncore.funcs.force_wake_put(dev_priv);
> + }
> spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> }
>
> --
> 1.8.4.rc3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
