Re: [PATCH v3] drm/i915: Use new i915_gem_object_pin_map for LRC

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 v3] drm/i915: Use new i915_gem_object_pin_map for LRC
Date: Tue, 12 Apr 2016 14:29:50 +0100	[thread overview]
Message-ID: <20160412132950.GR20240@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <570CF5FA.4020802@linux.intel.com>

On Tue, Apr 12, 2016 at 02:19:54PM +0100, Tvrtko Ursulin wrote:
> 
> On 12/04/16 14:12, Chris Wilson wrote:
> >On Tue, Apr 12, 2016 at 02:05:05PM +0100, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@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).
> >
> >Ah! Light dawns...
> >
> >Should we not just separate out the hws setup and hws hw_init?
> 
> Okay...
> 
> >>-static void lrc_setup_hardware_status_page(struct intel_engine_cs *engine,
> >>-					   struct drm_i915_gem_object *default_ctx_obj)
> >>+static int
> >>+lrc_setup_hws(struct intel_engine_cs *engine,
> >>+	      struct drm_i915_gem_object *def_ctx_obj)
> >>  {
> >>  	struct drm_i915_private *dev_priv = engine->dev->dev_private;
> >>-	struct page *page;
> >>+	void *hws;
> >>
> >>  	/* The HWSP is part of the default context object in LRC mode. */
> >>-	engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj)
> >>-			+ LRC_PPHWSP_PN * PAGE_SIZE;
> >>-	page = i915_gem_object_get_page(default_ctx_obj, LRC_PPHWSP_PN);
> >>-	engine->status_page.page_addr = kmap(page);
> >>-	engine->status_page.obj = default_ctx_obj;
> >>+	engine->status_page.gfx_addr = i915_gem_obj_ggtt_offset(def_ctx_obj) +
> >>+				       LRC_PPHWSP_PN * PAGE_SIZE;
> >>+	hws = i915_gem_object_pin_map(def_ctx_obj);
> >>+	if (IS_ERR(hws))
> >>+		return PTR_ERR(hws);
> >>+	engine->status_page.page_addr = hws + LRC_PPHWSP_PN * PAGE_SIZE;
> >>+	engine->status_page.obj = def_ctx_obj;
> 
> ... so above here is setup and below is init, correct?
> 

Yes, allocating the mapping is setup; writing the register is hw_init.
-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