[Intel-gfx] [PATCH 2/2] drm/i915: Execlist irq handler micro optimisations

Chris Wilson chris at chris-wilson.co.uk
Thu Feb 11 20:59:16 UTC 2016


On Thu, Feb 11, 2016 at 06:03:10PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Assorted changes most likely without any practical effect
> apart from a tiny reduction in generated code for the interrupt
> handler and request submission.
> 
>  * Remove needless initialization.
>  * Improve cache locality by reorganizing code and/or using
>    branch hints to keep unexpected or error conditions out
>    of line.
>  * Favor busy submit path vs. empty queue.
>  * Less branching in hot-paths.

Sadly there are panics here to be taken care of before optimisations. :|

> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c        | 91 ++++++++++++++++-----------------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
>  2 files changed, 44 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 951f1e6af947..589d6404b5ca 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -269,6 +269,9 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *ring)
>  {
>  	struct drm_device *dev = ring->dev;
>  
> +	if (IS_GEN8(dev) || IS_GEN9(dev))
> +		ring->idle_lite_restore_wa = ~0;
> +
>  	ring->disable_lite_restore_wa = (IS_SKL_REVID(dev, 0, SKL_REVID_B0) ||
>  					IS_BXT_REVID(dev, 0, BXT_REVID_A1)) &&
>  					(ring->id == VCS || ring->id == VCS2);
> @@ -424,7 +427,7 @@ static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
>  static void execlists_context_unqueue(struct intel_engine_cs *ring)
>  {
>  	struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
> -	struct drm_i915_gem_request *cursor = NULL, *tmp = NULL;
> +	struct drm_i915_gem_request *cursor, *tmp;
>  
>  	assert_spin_locked(&ring->execlist_lock);
>  
> @@ -434,9 +437,6 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
>  	 */
>  	WARN_ON(!intel_irqs_enabled(ring->dev->dev_private));
>  
> -	if (list_empty(&ring->execlist_queue))
> -		return;
> -
>  	/* Try to read in pairs */
>  	list_for_each_entry_safe(cursor, tmp, &ring->execlist_queue,
>  				 execlist_link) {
> @@ -451,37 +451,35 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
>  			req0 = cursor;
>  		} else {
>  			req1 = cursor;
> +			WARN_ON(req1->elsp_submitted);
>  			break;
>  		}
>  	}
>  
> -	if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
> +	if (unlikely(!req0))
> +		return;
> +
> +	if (req0->elsp_submitted & ring->idle_lite_restore_wa) {

If you are going for the microptimsation, just do

	if (req->elsp_submitted)

We unconditionally prepare the ringbuffer for the wa and currently apply
it everywhere (and future hw will not be taking this path, aiui at least).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the Intel-gfx mailing list