[Intel-gfx] [PATCH 2/8] drm/i915: Introduce i915_gem_object_create_stolen_for_preallocated

[Ben Widawsky]

On Thu, Feb 07, 2013 at 02:32:58PM +0000, Chris Wilson wrote:
> On Wed, Feb 06, 2013 at 09:09:34PM -0800, Ben Widawsky wrote:
> > On Wed, Feb 06, 2013 at 11:10:22AM +0000, Chris Wilson wrote:
> > > Wrap a preallocated region of stolen memory within an ordinary GEM
> > > object, for example the BIOS framebuffer.
> > > 
> > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h        |    5 +++
> > >  drivers/gpu/drm/i915/i915_gem_stolen.c |   65 ++++++++++++++++++++++++++++++++
> > >  2 files changed, 70 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 08c5def..fd8a495 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1720,6 +1720,11 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
> > >  void i915_gem_cleanup_stolen(struct drm_device *dev);
> > >  struct drm_i915_gem_object *
> > >  i915_gem_object_create_stolen(struct drm_device *dev, u32 size);
> > > +struct drm_i915_gem_object *
> > > +i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
> > > +					       u32 stolen_offset,
> > > +					       u32 gtt_offset,
> > > +					       u32 size);
> > 
> > Can we default to u64 for things from now on. Not that I know anything,
> > or anything. At least gtt_offset.
> 
> We could. Do we envison getting more than 4GiB of stolen memory? Do you
> worry that the BIOS may not use a 1:1 mapping?

I suppose we can always change it later actually, so nevermind. I
wouldn't have envisioned the stolen size would grow as large as it has
today, so I am not the right person to ask anyway.

> 
> > > +	/* To simplify the initialisation sequence between KMS and GTT,
> > > +	 * we allow construction of the stolen object prior to
> > > +	 * setting up the GTT space. The actual reservation will occur
> > > +	 * later.
> > > +	 */
> > > +	if (drm_mm_initialized(&dev_priv->mm.gtt_space)) {
> > > +		obj->gtt_space = drm_mm_create_block(&dev_priv->mm.gtt_space,
> > > +						     gtt_offset, size,
> > > +						     false);
> > > +		if (obj->gtt_space == NULL) {
> > > +			DRM_DEBUG_KMS("failed to allocate stolen GTT space\n");
> > > +			drm_gem_object_unreference(&obj->base);
> > > +			return NULL;
> > > +		}
> > > +	} else
> > > +		obj->gtt_space = I915_GTT_RESERVED;
> > 
> > Could you explain how/why we'd do this before the mm is initialized.
> 
> It's all due to having to wrap the BIOS structures prior to us
> initialising the mm. If we don't, we overwrite those objects whilst
> setting up the GTT. Note that the display is currently scaning out from
> those objects...
> -Chris

For some reason, I thought the code that had already landed took care of
this by not initializing those PTEs. My bad.

-- 
Ben Widawsky, Intel Open Source Technology Center