diff mbox

[1/7] drm/i915: add simple wrappers for stolen node insertion/removal

Message ID 1435781726-7282-2-git-send-email-przanoni@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Zanoni July 1, 2015, 8:15 p.m. UTC
From: Paulo Zanoni <paulo.r.zanoni@intel.com>

We want to move the FBC code out of i915_gem_stolen.c, but that code
directly adds/removes stolen memory nodes. Let's create this
abstraction, so i915_gme_stolen.c is still in control of all the
stolen memory handling. These abstractions will also allow us to add
locking assertions later.

Requested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h        |  4 ++++
 drivers/gpu/drm/i915/i915_gem_stolen.c | 44 +++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 16 deletions(-)

Comments

Chris Wilson July 1, 2015, 8:38 p.m. UTC | #1
On Wed, Jul 01, 2015 at 05:15:20PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> We want to move the FBC code out of i915_gem_stolen.c, but that code
> directly adds/removes stolen memory nodes. Let's create this
> abstraction, so i915_gme_stolen.c is still in control of all the
> stolen memory handling. These abstractions will also allow us to add
> locking assertions later.
> 
> Requested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h        |  4 ++++
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 44 +++++++++++++++++++++-------------
>  2 files changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1dbd957..b9de374 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3109,6 +3109,10 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev)
>  }
>  
>  /* i915_gem_stolen.c */
> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> +				struct drm_mm_node *node, u64 size,
> +				unsigned alignment);
> +void i915_gem_stolen_remove_node(struct drm_mm_node *node);

Might as well pass in dev_priv now to save changing the interface later.


>  int i915_gem_init_stolen(struct drm_device *dev);
>  int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, int fb_cpp);
>  void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 348ed5a..6b43234 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -42,6 +42,22 @@
>   * for is a boon.
>   */
>  
> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> +				struct drm_mm_node *node, u64 size,
> +				unsigned alignment)
> +{
> +	if (!drm_mm_initialized(&dev_priv->mm.stolen))
> +		return -ENODEV;

Might as well take advantage of this test here to remove the same check
from i915_gem_object_create_stolen_for_preallocated and
i915_gem_object_create_stolen

Other than those minor, looks good.
-Chris
Paulo Zanoni July 2, 2015, 1:33 p.m. UTC | #2
2015-07-01 17:38 GMT-03:00 Chris Wilson <chris@chris-wilson.co.uk>:
> On Wed, Jul 01, 2015 at 05:15:20PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> We want to move the FBC code out of i915_gem_stolen.c, but that code
>> directly adds/removes stolen memory nodes. Let's create this
>> abstraction, so i915_gme_stolen.c is still in control of all the
>> stolen memory handling. These abstractions will also allow us to add
>> locking assertions later.
>>
>> Requested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h        |  4 ++++
>>  drivers/gpu/drm/i915/i915_gem_stolen.c | 44 +++++++++++++++++++++-------------
>>  2 files changed, 32 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 1dbd957..b9de374 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3109,6 +3109,10 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev)
>>  }
>>
>>  /* i915_gem_stolen.c */
>> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
>> +                             struct drm_mm_node *node, u64 size,
>> +                             unsigned alignment);
>> +void i915_gem_stolen_remove_node(struct drm_mm_node *node);
>
> Might as well pass in dev_priv now to save changing the interface later.

I thought about doing it, but dev_priv is just for the mutex WARN, and
I thought it could be rejected, so I didn't add dev_priv because of
the risk of having the WARN rejected, so dev_priv would be useless.
But I can change this, no problem.

>
>
>>  int i915_gem_init_stolen(struct drm_device *dev);
>>  int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, int fb_cpp);
>>  void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> index 348ed5a..6b43234 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> @@ -42,6 +42,22 @@
>>   * for is a boon.
>>   */
>>
>> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
>> +                             struct drm_mm_node *node, u64 size,
>> +                             unsigned alignment)
>> +{
>> +     if (!drm_mm_initialized(&dev_priv->mm.stolen))
>> +             return -ENODEV;
>
> Might as well take advantage of this test here to remove the same check
> from i915_gem_object_create_stolen_for_preallocated and
> i915_gem_object_create_stolen
>
> Other than those minor, looks good.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
Chris Wilson July 2, 2015, 1:36 p.m. UTC | #3
On Thu, Jul 02, 2015 at 10:33:27AM -0300, Paulo Zanoni wrote:
> 2015-07-01 17:38 GMT-03:00 Chris Wilson <chris@chris-wilson.co.uk>:
> > On Wed, Jul 01, 2015 at 05:15:20PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> We want to move the FBC code out of i915_gem_stolen.c, but that code
> >> directly adds/removes stolen memory nodes. Let's create this
> >> abstraction, so i915_gme_stolen.c is still in control of all the
> >> stolen memory handling. These abstractions will also allow us to add
> >> locking assertions later.
> >>
> >> Requested-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_drv.h        |  4 ++++
> >>  drivers/gpu/drm/i915/i915_gem_stolen.c | 44 +++++++++++++++++++++-------------
> >>  2 files changed, 32 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 1dbd957..b9de374 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -3109,6 +3109,10 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev)
> >>  }
> >>
> >>  /* i915_gem_stolen.c */
> >> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> >> +                             struct drm_mm_node *node, u64 size,
> >> +                             unsigned alignment);
> >> +void i915_gem_stolen_remove_node(struct drm_mm_node *node);
> >
> > Might as well pass in dev_priv now to save changing the interface later.
> 
> I thought about doing it, but dev_priv is just for the mutex WARN, and
> I thought it could be rejected, so I didn't add dev_priv because of
> the risk of having the WARN rejected, so dev_priv would be useless.
> But I can change this, no problem.

If you change it over to taking the stolen_mutex inside these functions
(which is the only place we need to), then it is required.
-Chris
Paulo Zanoni July 2, 2015, 7:07 p.m. UTC | #4
2015-07-01 17:38 GMT-03:00 Chris Wilson <chris@chris-wilson.co.uk>:
> On Wed, Jul 01, 2015 at 05:15:20PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> We want to move the FBC code out of i915_gem_stolen.c, but that code
>> directly adds/removes stolen memory nodes. Let's create this
>> abstraction, so i915_gme_stolen.c is still in control of all the
>> stolen memory handling. These abstractions will also allow us to add
>> locking assertions later.
>>
>> Requested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h        |  4 ++++
>>  drivers/gpu/drm/i915/i915_gem_stolen.c | 44 +++++++++++++++++++++-------------
>>  2 files changed, 32 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 1dbd957..b9de374 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3109,6 +3109,10 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev)
>>  }
>>
>>  /* i915_gem_stolen.c */
>> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
>> +                             struct drm_mm_node *node, u64 size,
>> +                             unsigned alignment);
>> +void i915_gem_stolen_remove_node(struct drm_mm_node *node);
>
> Might as well pass in dev_priv now to save changing the interface later.
>
>
>>  int i915_gem_init_stolen(struct drm_device *dev);
>>  int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, int fb_cpp);
>>  void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> index 348ed5a..6b43234 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> @@ -42,6 +42,22 @@
>>   * for is a boon.
>>   */
>>
>> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
>> +                             struct drm_mm_node *node, u64 size,
>> +                             unsigned alignment)
>> +{
>> +     if (!drm_mm_initialized(&dev_priv->mm.stolen))
>> +             return -ENODEV;
>
> Might as well take advantage of this test here to remove the same check
> from i915_gem_object_create_stolen_for_preallocated and
> i915_gem_object_create_stolen

If we do this, we'll start printing "creating stolen something" and
"failed to alloc stolen node" messages even when stolen is not
present, and we'll also do some useless alloc+free calls.

For the specific case of
i915_gem_object_create_stolen_for_preallocated(), we'll then run
drm_mm_reserve_node() without doing the check first, and a brief look
at the implementation suggests it would probably fail with a
non-initialized mm.stolen (it seems drm_mm_for_each_hole() assumes the
hole_stack list is initialized). So we'd need to reorganize the
function first, and the result would be a code that is a little more
fragile.

It looks like i915_gem_object_create_stolen() would survive without
the drm_mm_initialized(), but it would still have useless alloc/free
calls and debug messages.

Based on that, and on the fact that drm_mm_initialized() is a simple
inline pointer check, I'd vote to not remove the drm_mm_initialized()
calls.

>
> Other than those minor, looks good.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
Chris Wilson July 2, 2015, 7:14 p.m. UTC | #5
On Thu, Jul 02, 2015 at 04:07:08PM -0300, Paulo Zanoni wrote:
> 2015-07-01 17:38 GMT-03:00 Chris Wilson <chris@chris-wilson.co.uk>:
> > On Wed, Jul 01, 2015 at 05:15:20PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> We want to move the FBC code out of i915_gem_stolen.c, but that code
> >> directly adds/removes stolen memory nodes. Let's create this
> >> abstraction, so i915_gme_stolen.c is still in control of all the
> >> stolen memory handling. These abstractions will also allow us to add
> >> locking assertions later.
> >>
> >> Requested-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/i915_drv.h        |  4 ++++
> >>  drivers/gpu/drm/i915/i915_gem_stolen.c | 44 +++++++++++++++++++++-------------
> >>  2 files changed, 32 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 1dbd957..b9de374 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -3109,6 +3109,10 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev)
> >>  }
> >>
> >>  /* i915_gem_stolen.c */
> >> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> >> +                             struct drm_mm_node *node, u64 size,
> >> +                             unsigned alignment);
> >> +void i915_gem_stolen_remove_node(struct drm_mm_node *node);
> >
> > Might as well pass in dev_priv now to save changing the interface later.
> >
> >
> >>  int i915_gem_init_stolen(struct drm_device *dev);
> >>  int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, int fb_cpp);
> >>  void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
> >> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> >> index 348ed5a..6b43234 100644
> >> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> >> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> >> @@ -42,6 +42,22 @@
> >>   * for is a boon.
> >>   */
> >>
> >> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> >> +                             struct drm_mm_node *node, u64 size,
> >> +                             unsigned alignment)
> >> +{
> >> +     if (!drm_mm_initialized(&dev_priv->mm.stolen))
> >> +             return -ENODEV;
> >
> > Might as well take advantage of this test here to remove the same check
> > from i915_gem_object_create_stolen_for_preallocated and
> > i915_gem_object_create_stolen
> 
> If we do this, we'll start printing "creating stolen something" and
> "failed to alloc stolen node" messages even when stolen is not
> present, and we'll also do some useless alloc+free calls.

The messages we can delete if you think they are a nuisance, certainly
error ones here tend to be (since we often use stolen then failsafe).
 
> For the specific case of
> i915_gem_object_create_stolen_for_preallocated(), we'll then run
> drm_mm_reserve_node() without doing the check first, and a brief look
> at the implementation suggests it would probably fail with a
> non-initialized mm.stolen (it seems drm_mm_for_each_hole() assumes the
> hole_stack list is initialized). So we'd need to reorganize the
> function first, and the result would be a code that is a little more
> fragile.

Ok, that's a nuisance. I was just trying to think of a way to trim a few
lines of code.

> It looks like i915_gem_object_create_stolen() would survive without
> the drm_mm_initialized(), but it would still have useless alloc/free
> calls and debug messages.
> 
> Based on that, and on the fact that drm_mm_initialized() is a simple
> inline pointer check, I'd vote to not remove the drm_mm_initialized()
> calls.

Ok.
-Chris
Daniel Vetter July 6, 2015, 8:41 a.m. UTC | #6
On Wed, Jul 01, 2015 at 05:15:20PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> We want to move the FBC code out of i915_gem_stolen.c, but that code
> directly adds/removes stolen memory nodes. Let's create this
> abstraction, so i915_gme_stolen.c is still in control of all the
> stolen memory handling. These abstractions will also allow us to add
> locking assertions later.
> 
> Requested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I guess you'll follow up with a nice kerneldoc patch for i915_gem_stolen?
You're the expert on this code now ;-)

Cheers, Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h        |  4 ++++
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 44 +++++++++++++++++++++-------------
>  2 files changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1dbd957..b9de374 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3109,6 +3109,10 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev)
>  }
>  
>  /* i915_gem_stolen.c */
> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> +				struct drm_mm_node *node, u64 size,
> +				unsigned alignment);
> +void i915_gem_stolen_remove_node(struct drm_mm_node *node);
>  int i915_gem_init_stolen(struct drm_device *dev);
>  int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, int fb_cpp);
>  void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 348ed5a..6b43234 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -42,6 +42,22 @@
>   * for is a boon.
>   */
>  
> +int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
> +				struct drm_mm_node *node, u64 size,
> +				unsigned alignment)
> +{
> +	if (!drm_mm_initialized(&dev_priv->mm.stolen))
> +		return -ENODEV;
> +
> +	return drm_mm_insert_node(&dev_priv->mm.stolen, node, size, alignment,
> +				  DRM_MM_SEARCH_DEFAULT);
> +}
> +
> +void i915_gem_stolen_remove_node(struct drm_mm_node *node)
> +{
> +	drm_mm_remove_node(node);
> +}
> +
>  static unsigned long i915_stolen_to_physical(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -168,8 +184,7 @@ static int find_compression_threshold(struct drm_device *dev,
>  	 */
>  
>  	/* Try to over-allocate to reduce reallocations and fragmentation. */
> -	ret = drm_mm_insert_node(&dev_priv->mm.stolen, node,
> -				 size <<= 1, 4096, DRM_MM_SEARCH_DEFAULT);
> +	ret = i915_gem_stolen_insert_node(dev_priv, node, size <<= 1, 4096);
>  	if (ret == 0)
>  		return compression_threshold;
>  
> @@ -179,9 +194,7 @@ again:
>  	    (fb_cpp == 2 && compression_threshold == 2))
>  		return 0;
>  
> -	ret = drm_mm_insert_node(&dev_priv->mm.stolen, node,
> -				 size >>= 1, 4096,
> -				 DRM_MM_SEARCH_DEFAULT);
> +	ret = i915_gem_stolen_insert_node(dev_priv, node, size >>= 1, 4096);
>  	if (ret && INTEL_INFO(dev)->gen <= 4) {
>  		return 0;
>  	} else if (ret) {
> @@ -218,8 +231,8 @@ static int i915_setup_compression(struct drm_device *dev, int size, int fb_cpp)
>  		if (!compressed_llb)
>  			goto err_fb;
>  
> -		ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_llb,
> -					 4096, 4096, DRM_MM_SEARCH_DEFAULT);
> +		ret = i915_gem_stolen_insert_node(dev_priv, compressed_llb,
> +						  4096, 4096);
>  		if (ret)
>  			goto err_fb;
>  
> @@ -240,7 +253,7 @@ static int i915_setup_compression(struct drm_device *dev, int size, int fb_cpp)
>  
>  err_fb:
>  	kfree(compressed_llb);
> -	drm_mm_remove_node(&dev_priv->fbc.compressed_fb);
> +	i915_gem_stolen_remove_node(&dev_priv->fbc.compressed_fb);
>  err_llb:
>  	pr_info_once("drm: not enough stolen space for compressed buffer (need %d more bytes), disabling. Hint: you may be able to increase stolen memory size in the BIOS to avoid this.\n", size);
>  	return -ENOSPC;
> @@ -269,10 +282,10 @@ void i915_gem_stolen_cleanup_compression(struct drm_device *dev)
>  	if (dev_priv->fbc.uncompressed_size == 0)
>  		return;
>  
> -	drm_mm_remove_node(&dev_priv->fbc.compressed_fb);
> +	i915_gem_stolen_remove_node(&dev_priv->fbc.compressed_fb);
>  
>  	if (dev_priv->fbc.compressed_llb) {
> -		drm_mm_remove_node(dev_priv->fbc.compressed_llb);
> +		i915_gem_stolen_remove_node(dev_priv->fbc.compressed_llb);
>  		kfree(dev_priv->fbc.compressed_llb);
>  	}
>  
> @@ -387,7 +400,7 @@ static void
>  i915_gem_object_release_stolen(struct drm_i915_gem_object *obj)
>  {
>  	if (obj->stolen) {
> -		drm_mm_remove_node(obj->stolen);
> +		i915_gem_stolen_remove_node(obj->stolen);
>  		kfree(obj->stolen);
>  		obj->stolen = NULL;
>  	}
> @@ -449,8 +462,7 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
>  	if (!stolen)
>  		return NULL;
>  
> -	ret = drm_mm_insert_node(&dev_priv->mm.stolen, stolen, size,
> -				 4096, DRM_MM_SEARCH_DEFAULT);
> +	ret = i915_gem_stolen_insert_node(dev_priv, stolen, size, 4096);
>  	if (ret) {
>  		kfree(stolen);
>  		return NULL;
> @@ -460,7 +472,7 @@ i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
>  	if (obj)
>  		return obj;
>  
> -	drm_mm_remove_node(stolen);
> +	i915_gem_stolen_remove_node(stolen);
>  	kfree(stolen);
>  	return NULL;
>  }
> @@ -505,7 +517,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
>  	obj = _i915_gem_object_create_stolen(dev, stolen);
>  	if (obj == NULL) {
>  		DRM_DEBUG_KMS("failed to allocate stolen object\n");
> -		drm_mm_remove_node(stolen);
> +		i915_gem_stolen_remove_node(stolen);
>  		kfree(stolen);
>  		return NULL;
>  	}
> @@ -546,7 +558,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
>  err_vma:
>  	i915_gem_vma_destroy(vma);
>  err_out:
> -	drm_mm_remove_node(stolen);
> +	i915_gem_stolen_remove_node(stolen);
>  	kfree(stolen);
>  	drm_gem_object_unreference(&obj->base);
>  	return NULL;
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1dbd957..b9de374 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3109,6 +3109,10 @@  static inline void i915_gem_chipset_flush(struct drm_device *dev)
 }
 
 /* i915_gem_stolen.c */
+int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
+				struct drm_mm_node *node, u64 size,
+				unsigned alignment);
+void i915_gem_stolen_remove_node(struct drm_mm_node *node);
 int i915_gem_init_stolen(struct drm_device *dev);
 int i915_gem_stolen_setup_compression(struct drm_device *dev, int size, int fb_cpp);
 void i915_gem_stolen_cleanup_compression(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 348ed5a..6b43234 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -42,6 +42,22 @@ 
  * for is a boon.
  */
 
+int i915_gem_stolen_insert_node(struct drm_i915_private *dev_priv,
+				struct drm_mm_node *node, u64 size,
+				unsigned alignment)
+{
+	if (!drm_mm_initialized(&dev_priv->mm.stolen))
+		return -ENODEV;
+
+	return drm_mm_insert_node(&dev_priv->mm.stolen, node, size, alignment,
+				  DRM_MM_SEARCH_DEFAULT);
+}
+
+void i915_gem_stolen_remove_node(struct drm_mm_node *node)
+{
+	drm_mm_remove_node(node);
+}
+
 static unsigned long i915_stolen_to_physical(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -168,8 +184,7 @@  static int find_compression_threshold(struct drm_device *dev,
 	 */
 
 	/* Try to over-allocate to reduce reallocations and fragmentation. */
-	ret = drm_mm_insert_node(&dev_priv->mm.stolen, node,
-				 size <<= 1, 4096, DRM_MM_SEARCH_DEFAULT);
+	ret = i915_gem_stolen_insert_node(dev_priv, node, size <<= 1, 4096);
 	if (ret == 0)
 		return compression_threshold;
 
@@ -179,9 +194,7 @@  again:
 	    (fb_cpp == 2 && compression_threshold == 2))
 		return 0;
 
-	ret = drm_mm_insert_node(&dev_priv->mm.stolen, node,
-				 size >>= 1, 4096,
-				 DRM_MM_SEARCH_DEFAULT);
+	ret = i915_gem_stolen_insert_node(dev_priv, node, size >>= 1, 4096);
 	if (ret && INTEL_INFO(dev)->gen <= 4) {
 		return 0;
 	} else if (ret) {
@@ -218,8 +231,8 @@  static int i915_setup_compression(struct drm_device *dev, int size, int fb_cpp)
 		if (!compressed_llb)
 			goto err_fb;
 
-		ret = drm_mm_insert_node(&dev_priv->mm.stolen, compressed_llb,
-					 4096, 4096, DRM_MM_SEARCH_DEFAULT);
+		ret = i915_gem_stolen_insert_node(dev_priv, compressed_llb,
+						  4096, 4096);
 		if (ret)
 			goto err_fb;
 
@@ -240,7 +253,7 @@  static int i915_setup_compression(struct drm_device *dev, int size, int fb_cpp)
 
 err_fb:
 	kfree(compressed_llb);
-	drm_mm_remove_node(&dev_priv->fbc.compressed_fb);
+	i915_gem_stolen_remove_node(&dev_priv->fbc.compressed_fb);
 err_llb:
 	pr_info_once("drm: not enough stolen space for compressed buffer (need %d more bytes), disabling. Hint: you may be able to increase stolen memory size in the BIOS to avoid this.\n", size);
 	return -ENOSPC;
@@ -269,10 +282,10 @@  void i915_gem_stolen_cleanup_compression(struct drm_device *dev)
 	if (dev_priv->fbc.uncompressed_size == 0)
 		return;
 
-	drm_mm_remove_node(&dev_priv->fbc.compressed_fb);
+	i915_gem_stolen_remove_node(&dev_priv->fbc.compressed_fb);
 
 	if (dev_priv->fbc.compressed_llb) {
-		drm_mm_remove_node(dev_priv->fbc.compressed_llb);
+		i915_gem_stolen_remove_node(dev_priv->fbc.compressed_llb);
 		kfree(dev_priv->fbc.compressed_llb);
 	}
 
@@ -387,7 +400,7 @@  static void
 i915_gem_object_release_stolen(struct drm_i915_gem_object *obj)
 {
 	if (obj->stolen) {
-		drm_mm_remove_node(obj->stolen);
+		i915_gem_stolen_remove_node(obj->stolen);
 		kfree(obj->stolen);
 		obj->stolen = NULL;
 	}
@@ -449,8 +462,7 @@  i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
 	if (!stolen)
 		return NULL;
 
-	ret = drm_mm_insert_node(&dev_priv->mm.stolen, stolen, size,
-				 4096, DRM_MM_SEARCH_DEFAULT);
+	ret = i915_gem_stolen_insert_node(dev_priv, stolen, size, 4096);
 	if (ret) {
 		kfree(stolen);
 		return NULL;
@@ -460,7 +472,7 @@  i915_gem_object_create_stolen(struct drm_device *dev, u32 size)
 	if (obj)
 		return obj;
 
-	drm_mm_remove_node(stolen);
+	i915_gem_stolen_remove_node(stolen);
 	kfree(stolen);
 	return NULL;
 }
@@ -505,7 +517,7 @@  i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
 	obj = _i915_gem_object_create_stolen(dev, stolen);
 	if (obj == NULL) {
 		DRM_DEBUG_KMS("failed to allocate stolen object\n");
-		drm_mm_remove_node(stolen);
+		i915_gem_stolen_remove_node(stolen);
 		kfree(stolen);
 		return NULL;
 	}
@@ -546,7 +558,7 @@  i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
 err_vma:
 	i915_gem_vma_destroy(vma);
 err_out:
-	drm_mm_remove_node(stolen);
+	i915_gem_stolen_remove_node(stolen);
 	kfree(stolen);
 	drm_gem_object_unreference(&obj->base);
 	return NULL;