| Message ID | 1360149028-13531-2-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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. > void i915_gem_object_release_stolen(struct drm_i915_gem_object *obj); > > /* i915_gem_tiling.c */ > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c > index 69d97cb..130d1db 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -312,6 +312,71 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size) > return NULL; > } > > +struct drm_i915_gem_object * > +i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, > + u32 stolen_offset, > + u32 gtt_offset, > + u32 size) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_i915_gem_object *obj; > + struct drm_mm_node *stolen; > + > + if (dev_priv->mm.stolen_base == 0) > + return NULL; > + > + DRM_DEBUG_KMS("creating preallocated stolen object: stolen_offset=%x, gtt_offset=%x, size=%x\n", > + stolen_offset, gtt_offset, size); > + > + /* KISS and expect everything to be page-aligned */ > + BUG_ON(stolen_offset & 4095); > + BUG_ON(gtt_offset & 4095); > + BUG_ON(size & 4095); > + > + if (WARN_ON(size == 0)) > + return NULL; > + > + stolen = drm_mm_create_block(&dev_priv->mm.stolen, > + stolen_offset, size, > + false); > + if (stolen == NULL) { > + DRM_DEBUG_KMS("failed to allocate stolen space\n"); > + return NULL; > + } > + > + obj = _i915_gem_object_create_stolen(dev, stolen); > + if (obj == NULL) { > + DRM_DEBUG_KMS("failed to allocate stolen object\n"); > + drm_mm_put_block(stolen); > + return NULL; > + } > + > + /* 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. > + > + obj->gtt_offset = gtt_offset; > + obj->has_global_gtt_mapping = 1; > + > + list_add_tail(&obj->gtt_list, &dev_priv->mm.bound_list); > + list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list); > + > + return obj; > +} > + > void > i915_gem_object_release_stolen(struct drm_i915_gem_object *obj) > { > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@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? > > + /* 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
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@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.
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); void i915_gem_object_release_stolen(struct drm_i915_gem_object *obj); /* i915_gem_tiling.c */ diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 69d97cb..130d1db 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -312,6 +312,71 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size) return NULL; } +struct drm_i915_gem_object * +i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, + u32 stolen_offset, + u32 gtt_offset, + u32 size) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_gem_object *obj; + struct drm_mm_node *stolen; + + if (dev_priv->mm.stolen_base == 0) + return NULL; + + DRM_DEBUG_KMS("creating preallocated stolen object: stolen_offset=%x, gtt_offset=%x, size=%x\n", + stolen_offset, gtt_offset, size); + + /* KISS and expect everything to be page-aligned */ + BUG_ON(stolen_offset & 4095); + BUG_ON(gtt_offset & 4095); + BUG_ON(size & 4095); + + if (WARN_ON(size == 0)) + return NULL; + + stolen = drm_mm_create_block(&dev_priv->mm.stolen, + stolen_offset, size, + false); + if (stolen == NULL) { + DRM_DEBUG_KMS("failed to allocate stolen space\n"); + return NULL; + } + + obj = _i915_gem_object_create_stolen(dev, stolen); + if (obj == NULL) { + DRM_DEBUG_KMS("failed to allocate stolen object\n"); + drm_mm_put_block(stolen); + return NULL; + } + + /* 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; + + obj->gtt_offset = gtt_offset; + obj->has_global_gtt_mapping = 1; + + list_add_tail(&obj->gtt_list, &dev_priv->mm.bound_list); + list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list); + + return obj; +} + void i915_gem_object_release_stolen(struct drm_i915_gem_object *obj) {
Wrap a preallocated region of stolen memory within an ordinary GEM object, for example the BIOS framebuffer. Signed-off-by: Chris Wilson <chris@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(+)