[3/3] drm/i915/dumb: return the allocated memory size
diff mbox series

Message ID 20191230132351.17487-4-ramalingam.c@intel.com
State New
Headers show
Series
  • dumb buffer patches
Related show

Commit Message

Ramalingam C Dec. 30, 2019, 1:23 p.m. UTC
On successful allocation, instead returning the requested size
return the total size of allocated pages.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Chris Wilson Dec. 31, 2019, 11:50 p.m. UTC | #1
Quoting Ramalingam C (2019-12-30 13:23:51)
> On successful allocation, instead returning the requested size
> return the total size of allocated pages.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 7f39df3fab7f..5a53de797852 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -241,7 +241,9 @@ i915_gem_dumb_create(struct drm_file *file,
>  {
>         enum intel_memory_type mem_type = INTEL_MEMORY_SYSTEM;
>         int cpp = DIV_ROUND_UP(args->bpp, 8);
> +       struct intel_memory_region *mr;
>         u32 format;
> +       int ret;
>  
>         switch (cpp) {
>         case 1:
> @@ -270,8 +272,15 @@ i915_gem_dumb_create(struct drm_file *file,
>         if (HAS_LMEM(to_i915(dev)))
>                 mem_type = INTEL_MEMORY_LOCAL;
>  
> -       return i915_gem_create(file, to_i915(dev), mem_type,
> -                              &args->size, &args->handle);
> +       ret = i915_gem_create(file, to_i915(dev), mem_type,
> +                             &args->size, &args->handle);
> +       if (ret)
> +               goto out;
> +
> +       mr = intel_memory_region_by_type(to_i915(dev), mem_type);
> +       args->size = ALIGN(args->size, mr->min_page_size);

How? How did we create an object that was not a multiple of the minimum
page size?

(Besides you should be using obj->mm.region here rather than assuming,
so more rearrangement if this is the right approach.)
-Chris
Ramalingam C Jan. 6, 2020, 6:49 a.m. UTC | #2
On 2019-12-31 at 23:50:27 +0000, Chris Wilson wrote:
> Quoting Ramalingam C (2019-12-30 13:23:51)
> > On successful allocation, instead returning the requested size
> > return the total size of allocated pages.
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 7f39df3fab7f..5a53de797852 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -241,7 +241,9 @@ i915_gem_dumb_create(struct drm_file *file,
> >  {
> >         enum intel_memory_type mem_type = INTEL_MEMORY_SYSTEM;
> >         int cpp = DIV_ROUND_UP(args->bpp, 8);
> > +       struct intel_memory_region *mr;
> >         u32 format;
> > +       int ret;
> >  
> >         switch (cpp) {
> >         case 1:
> > @@ -270,8 +272,15 @@ i915_gem_dumb_create(struct drm_file *file,
> >         if (HAS_LMEM(to_i915(dev)))
> >                 mem_type = INTEL_MEMORY_LOCAL;
> >  
> > -       return i915_gem_create(file, to_i915(dev), mem_type,
> > -                              &args->size, &args->handle);
> > +       ret = i915_gem_create(file, to_i915(dev), mem_type,
> > +                             &args->size, &args->handle);
> > +       if (ret)
> > +               goto out;
> > +
> > +       mr = intel_memory_region_by_type(to_i915(dev), mem_type);
> > +       args->size = ALIGN(args->size, mr->min_page_size);
> 
> How? How did we create an object that was not a multiple of the minimum
> page size?
We round_up to PAGE_SIZE at i915_gem_create. But page size varies
between memory regions. So we might need to round up the size requested at
i915_gem_object_create_region!? So that we can consider the page size of
the region used for allocation?
> 
> (Besides you should be using obj->mm.region here rather than assuming,
> so more rearrangement if this is the right approach.)
Yes this i will fix it. This is just to report the right size to dumb
buffer. If we move the round up of the size requested at i915_gem_object_create_region
this change can be removed.

-Ram
> -Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7f39df3fab7f..5a53de797852 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -241,7 +241,9 @@  i915_gem_dumb_create(struct drm_file *file,
 {
 	enum intel_memory_type mem_type = INTEL_MEMORY_SYSTEM;
 	int cpp = DIV_ROUND_UP(args->bpp, 8);
+	struct intel_memory_region *mr;
 	u32 format;
+	int ret;
 
 	switch (cpp) {
 	case 1:
@@ -270,8 +272,15 @@  i915_gem_dumb_create(struct drm_file *file,
 	if (HAS_LMEM(to_i915(dev)))
 		mem_type = INTEL_MEMORY_LOCAL;
 
-	return i915_gem_create(file, to_i915(dev), mem_type,
-			       &args->size, &args->handle);
+	ret = i915_gem_create(file, to_i915(dev), mem_type,
+			      &args->size, &args->handle);
+	if (ret)
+		goto out;
+
+	mr = intel_memory_region_by_type(to_i915(dev), mem_type);
+	args->size = ALIGN(args->size, mr->min_page_size);
+out:
+	return ret;
 }
 
 /**