Re: [PATCH v3 3/3] drm/i915: Only grab correct forcewake for the engine with execlists

[Chris Wilson <chris@chris-wilson.co.uk>]

On Thu, Apr 07, 2016 at 04:56:00PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Rather than blindly waking up all forcewake domains on command
> submission, we can teach each engine what is (or are) the correct
> one to take.
> 
> On platforms with multiple forcewake domains like VLV, CHV, SKL
> and BXT, this has the potential of lowering the GPU and CPU
> power use and submission latency.
> 
> To implement it we add a function named
> intel_uncore_forcewake_for_reg whose purpose is to query which
> forcewake domains need to be taken to read or write a specific
> register with raw mmio accessors.
> 
> These enables the execlists engine setup  to query which
> forcewake domains are relevant per engine on the currently
> running platform.
> 
> v2:
>   * Kerneldoc.
>   * Split from intel_uncore.c macro extraction, WARN_ON,
>     no warns on old platforms. (Chris Wilson)
> 
> v3:
>   * Single domain per engine, mention all registers,
>     bi-directional function and a new name, fix handling
>     of gen6 and gen7 writes. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

> +/**
> + * intel_uncore_forcewake_for_reg - which forcewake domains are needed to access
> + * 				    a register
> + * @dev_priv: pointer to struct drm_i915_private
> + * @reg: register in question
> + * @op: operation bitmask of FW_REG_READ and/or FW_REG_WRITE
> + *
> + * Returns a set of forcewake domains required to be taken with for example
> + * intel_uncore_forcewake_get for the specified register to be accessible in the
> + * specified mode (read, write or read/write) with raw mmio accessors.
> + *
> + * NOTE: On Gen6 and Gen7 write forcewake domain (FORCEWAKE_RENDER) requires the
> + * callers to do FIFO management on their own or risk losing writes.
> + */
> +enum forcewake_domains
> +intel_uncore_forcewake_for_reg(struct drm_i915_private *dev_priv,
> +			       i915_reg_t reg, unsigned int op)
> +{
> +	enum forcewake_domains fw_domains = 0;
> +
> +	WARN_ON(!op);
> +
> +	if (op & FW_REG_READ)
> +		fw_domains = intel_uncore_forcewake_for_read(dev_priv, reg);
> +
> +	if (op & FW_REG_WRITE)
> +		fw_domains |= intel_uncore_forcewake_for_write(dev_priv, reg);
> +
> +	return fw_domains;
> +}

Like it, like it a lot.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx