Re: [Intel-gfx] [PATCH 3/3] drm/i915/dumb: return the allocated memory size

[Ramalingam C <ramalingam.c@intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx