Re: [PATCH v5] drm/i915: Cache LRC state page in the context

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v5] drm/i915: Cache LRC state page in the context
Date: Fri, 15 Jan 2016 17:02:20 +0000	[thread overview]
Message-ID: <20160115170220.GC27877@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <1452875969-20246-1-git-send-email-tvrtko.ursulin@linux.intel.com>

On Fri, Jan 15, 2016 at 04:39:29PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> LRC lifetime is well defined so we can cache the page pointing
> to the object backing store in the context in order to avoid
> walking over the object SG page list from the interrupt context
> without the big lock held.
> 
> v2: Also cache the mapping. (Chris Wilson)
> v3: Unmap on the error path.
> v4: No need to cache the page. (Chris Wilson)
> v5: No need to dirty the page on unpin. (Chris Wilson)

Sorry.

> @@ -1076,12 +1069,25 @@ static int intel_lr_context_do_pin(struct intel_engine_cs *ring,
>  	if (ret)
>  		return ret;
>  
> +	lrc_state_page = i915_gem_object_get_dirty_page(ctx_obj, LRC_STATE_PN);
> +	if (WARN_ON(!lrc_state_page)) {
> +		ret = -ENODEV;
> +		goto unpin_ctx_obj;
> +	}
> +
> +	lrc_reg_state = kmap(lrc_state_page);

kmap() cannot fail.

> +	if (!lrc_reg_state) {
> +		ret = -ENOMEM;
> +		goto unpin_ctx_obj;
> +	}
> +	if (--rq->ctx->engine[ring->id].pin_count == 0) {

I was wondering if we should just use

kunmap(kmap_to_page(ce->lrc_reg_state));

Except until we get struct intel_context_engine that line will exceed
80cols!
-Chris

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