Re: [PATCH 09/18] drm/i915: Add functions to allocate / release gem obj for GuC

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Mar 27, 2015 at 09:48:20AM +0100, Daniel Vetter wrote:
> On Thu, Mar 26, 2015 at 12:41:16PM -0700, yu.dai@intel.com wrote:
> > From: Alex Dai <yu.dai@intel.com>
> > 
> > All gem objects used by GuC are pinned to ggtt space out of range
> > [0, WOPCM size]. In GuC address space mapping, [0, WPOCM size] is
> > used internally for its Boot ROM, SRAM etc. Currently this WPOCM
> > size is 512K. This is done by using of PIN_OFFSET_BIAS.
> > 
> > Issue: VIZ-4884
> > Signed-off-by: Alex Dai <yu.dai@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_guc.h        |  3 +++
> >  drivers/gpu/drm/i915/intel_guc_loader.c | 34 +++++++++++++++++++++++++++++++++
> >  2 files changed, 37 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> > index d8262cf..ae14c57 100644
> > --- a/drivers/gpu/drm/i915/intel_guc.h
> > +++ b/drivers/gpu/drm/i915/intel_guc.h
> > @@ -92,5 +92,8 @@ struct intel_guc {
> >  extern int intel_guc_load_ucode(struct drm_device *dev, bool wait);
> >  extern void intel_guc_ucode_fini(struct drm_device *dev);
> >  extern void intel_guc_ucode_init(struct drm_device *dev);
> > +struct drm_i915_gem_object *
> > +intel_guc_allocate_gem_obj(struct drm_device *dev, u32 size);
> > +void intel_guc_release_gem_obj(struct drm_i915_gem_object *obj);
> >  
> >  #endif
> > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> > index 315e5d9..0500a53 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> > @@ -30,6 +30,40 @@
> >  MODULE_FIRMWARE(I915_GUC_UCODE_GEN8);
> >  MODULE_FIRMWARE(I915_GUC_UCODE_GEN9);
> >  
> > +struct drm_i915_gem_object *
> > +intel_guc_allocate_gem_obj(struct drm_device *dev, u32 size)
> > +{
> > +	struct drm_i915_gem_object *obj;
> > +
> > +	obj = i915_gem_alloc_object(dev, size);
> > +	if (!obj)
> > +		return NULL;
> > +
> > +	if (i915_gem_object_get_pages(obj)) {
> > +		drm_gem_object_unreference(&obj->base);
> > +		return NULL;
> > +	}
> > +
> > +	if (i915_gem_obj_ggtt_pin(obj, PAGE_SIZE,
> > +			PIN_OFFSET_BIAS | GUC_WOPCM_SIZE_VALUE)) {
> 
> Please add a comment here about how much pin bias you exactly need and
> why. Otherwise this will be extremely arcane knowledge lost when someone
> reworks the pin bias stuff or changes the default offset.
> 
> Or well just use the right offset directly, since atm PIN_OFFSET_BIAS is
> 256k and you claim to need 512k.

Strike that all, I'm blind and your code is all fine.

Sorry for the mess.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx