[Intel-gfx] [PATCH 2/2] drm/i915: Use new i915_gem_object_pin_map for LRC

[Chris Wilson]

On Tue, Apr 12, 2016 at 03:40:42PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> We can use the new pin/lazy unpin API for simplicity
> and more performance in the execlist submission paths.
> 
> v2:
>   * Fix error handling and convert more users.
>   * Compact some names for readability.
> 
> v3:
>   * intel_lr_context_free was not unpinning.
>   * Special case for GPU reset which otherwise unbalances
>     the HWS object pages pin count by running the engine
>     initialization only (not destructors).
> 
> v4:
>   * Rebased on top of hws setup/init split.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>

Minor comments,
both Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 3fd2ae6ce8e7..b61f8da5d6f3 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1091,8 +1091,8 @@ static int intel_lr_context_do_pin(struct intel_context *ctx,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state;
>  	struct intel_ringbuffer *ringbuf = ctx->engine[engine->id].ringbuf;
> -	struct page *lrc_state_page;
> -	uint32_t *lrc_reg_state;
> +	void *obj_addr;

obj_addr, harking back to a time when it was an unsigned long? Even then
it would be more traditionally be vaddr.

map, base, vaddr, obj_*

>  	/* And setup the hardware status page. */
> -	lrc_setup_hws(engine, dctx->engine[engine->id].state);
> +	ret = lrc_setup_hws(engine, dctx->engine[engine->id].state);
> +	if (ret) {
> +		DRM_ERROR("Failed to set up hwd %s: %d\n", engine->name, ret);

s/hwd/hws/

I would have put this set of chunks in the previous patch for less churn.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre