diff mbox

[2/8] drm/i915: Introduce i915_gem_object_create_stolen_for_preallocated

Message ID 1360149028-13531-2-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Feb. 6, 2013, 11:10 a.m. UTC
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(+)

Comments

Ben Widawsky Feb. 7, 2013, 5:09 a.m. UTC | #1
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
Chris Wilson Feb. 7, 2013, 2:32 p.m. UTC | #2
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
Ben Widawsky Feb. 7, 2013, 6:13 p.m. UTC | #3
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 mbox

Patch

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)
 {