On Mon, Apr 18, 2016 at 11:15:07AM +0100, Dave Gordon wrote: > With the new i915_gem_obj_pin_map() interface, it makes sense to keep > GuC objects (which are always pinned in memory and in the GGTT anyway) > mapped into kernel address space, rather than mapping and unmapping them > on each access. > > This preliminary patch sets up the pin-and-map for all GuC-specific > objects, and updates the various setup/shutdown functions to use these > long-term mappings rather than doing their own kmap_atomic() calls. > > v2: > Invent & use accessor function i915_object_mapped_address() rather > than accessing obj->mapping directly (Tvrtko Ursulin) > Use IS_ERR() rather than checking for NULL (Tvrtko Ursulin) > Added big comment about struct i915_guc_client & usage of the GEM > object that it describes > > Cc: <tvrtko.ursu...@intel.com> > Signed-off-by: Alex Dai <yu....@intel.com> > Signed-off-by: Dave Gordon <david.s.gor...@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 12 ++++++++++ > drivers/gpu/drm/i915/i915_guc_submission.c | 38 > +++++++++++------------------- > drivers/gpu/drm/i915/intel_guc.h | 24 +++++++++++++++++++ > 3 files changed, 50 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 85102ad..439f149 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3025,6 +3025,18 @@ static inline void i915_gem_object_unpin_pages(struct > drm_i915_gem_object *obj) > void *__must_check i915_gem_object_pin_map(struct drm_i915_gem_object *obj); > > /** > + * i915_object_mapped_address - address at which a GEM object is mapped > + * @obj - the object (presumably already mapped into kernel address space) > + * > + * Returns the address at which an object has been mapped by a call to > + * i915_gem_object_pin_map() above, or NULL if not currently mapped and > pinned > + */ > +static inline void *i915_object_mapped_address(struct drm_i915_gem_object > *obj) > +{ > + return obj->pages_pin_count == 0 ? NULL : obj->mapping; > +}
No. > @@ -649,6 +639,9 @@ static void gem_release_guc_obj(struct > drm_i915_gem_object *obj) > if (i915_gem_obj_is_pinned(obj)) > i915_gem_object_ggtt_unpin(obj); > > + if (i915_object_mapped_address(obj)) > + i915_gem_object_unpin_map(obj); Eh? If you aren't tracking your pin, you can't just randomly free someone elses. > drm_gem_object_unreference(&obj->base); > } > > @@ -729,6 +722,8 @@ static struct i915_guc_client *guc_client_alloc(struct > drm_device *dev, > goto err; > > client->client_obj = obj; > + client->client_base = i915_object_mapped_address(obj); > + WARN_ON(!client->client_base); > client->wq_offset = GUC_DB_SIZE; > client->wq_size = GUC_WQ_SIZE; > > @@ -841,7 +836,6 @@ static void guc_create_ads(struct intel_guc *guc) > struct guc_policies *policies; > struct guc_mmio_reg_state *reg_state; > struct intel_engine_cs *engine; > - struct page *page; > u32 size; > > /* The ads obj includes the struct itself and buffers passed to GuC */ > @@ -857,9 +851,7 @@ static void guc_create_ads(struct intel_guc *guc) > > guc->ads_obj = obj; > } > - > - page = i915_gem_object_get_page(obj, 0); > - ads = kmap(page); > + ads = i915_object_mapped_address(obj); You need to explicitly aknowlege that you are using the memory tracked by the object otherwise it *will* be removed by the shrinker. So what is wrong with using i915_gem_object_pin_map()? -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