Re: [PATCH] drm/i915/skl: Use correct use counters for force wakes

[Damien Lespiau <damien.lespiau@intel.com>]

On Tue, Sep 30, 2014 at 03:08:02PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Write and reads following the block changed use engine specific use counters
> and unless that is matched here force wake use counting goes bad. Same
> force wake is attempted to be taken twice which leads to at least time outs.
> 
> NOTE: Depending on feedback from hardware designers it may not be necessary
> to grab force wakes on Gen9 here. But for Gen8 it is needed due to a race
> between RC6 and ELSP writes.
> 
> v2: Added blitter force wake engine and made more future proof.
>     Added commit note.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> ---

This still has the issue of taking every fw engine, not looking at the
ring we're queuing the work for. Also I'll add the note in a comment
above the whole block. It does solve at least an error in current
kernels so:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

>  drivers/gpu/drm/i915/intel_lrc.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 3048d78..0792d7a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -276,7 +276,8 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
>  				 struct drm_i915_gem_object *ctx_obj0,
>  				 struct drm_i915_gem_object *ctx_obj1)
>  {
> -	struct drm_i915_private *dev_priv = ring->dev->dev_private;
> +	struct drm_device *dev = ring->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	uint64_t temp = 0;
>  	uint32_t desc[4];
>  	unsigned long flags;
> @@ -301,13 +302,18 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
>  	 * Instead, we do the runtime_pm_get/put when creating/destroying requests.
>  	 */
>  	spin_lock_irqsave(&dev_priv->uncore.lock, flags);
> -	if (IS_CHERRYVIEW(dev_priv->dev)) {
> +	if (IS_CHERRYVIEW(dev) || INTEL_INFO(dev)->gen >= 9) {
>  		if (dev_priv->uncore.fw_rendercount++ == 0)
>  			dev_priv->uncore.funcs.force_wake_get(dev_priv,
>  							      FORCEWAKE_RENDER);
>  		if (dev_priv->uncore.fw_mediacount++ == 0)
>  			dev_priv->uncore.funcs.force_wake_get(dev_priv,
>  							      FORCEWAKE_MEDIA);
> +		if (INTEL_INFO(dev)->gen >= 9) {
> +			if (dev_priv->uncore.fw_blittercount++ == 0)
> +				dev_priv->uncore.funcs.force_wake_get(dev_priv,
> +							FORCEWAKE_BLITTER);
> +		}
>  	} else {
>  		if (dev_priv->uncore.forcewake_count++ == 0)
>  			dev_priv->uncore.funcs.force_wake_get(dev_priv,
> @@ -326,13 +332,18 @@ static void execlists_elsp_write(struct intel_engine_cs *ring,
>  
>  	/* Release Force Wakeup (see the big comment above). */
>  	spin_lock_irqsave(&dev_priv->uncore.lock, flags);
> -	if (IS_CHERRYVIEW(dev_priv->dev)) {
> +	if (IS_CHERRYVIEW(dev) || INTEL_INFO(dev)->gen >= 9) {
>  		if (--dev_priv->uncore.fw_rendercount == 0)
>  			dev_priv->uncore.funcs.force_wake_put(dev_priv,
>  							      FORCEWAKE_RENDER);
>  		if (--dev_priv->uncore.fw_mediacount == 0)
>  			dev_priv->uncore.funcs.force_wake_put(dev_priv,
>  							      FORCEWAKE_MEDIA);
> +		if (INTEL_INFO(dev)->gen >= 9) {
> +			if (--dev_priv->uncore.fw_blittercount == 0)
> +				dev_priv->uncore.funcs.force_wake_put(dev_priv,
> +							FORCEWAKE_BLITTER);
> +		}
>  	} else {
>  		if (--dev_priv->uncore.forcewake_count == 0)
>  			dev_priv->uncore.funcs.force_wake_put(dev_priv,
> -- 
> 2.1.0
>