Re: [Intel-gfx] [PATCH] drm/i915: align dumb buffer stride to page_sz of the region

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

On 2020-02-05 at 11:43:35 +0000, Chris Wilson wrote:
> Quoting Ramalingam C (2020-02-05 11:40:19)
> > If stride of the dumb buffer requested is greater than the primary
> > plane's max stride, then we align the stride to the page size. But the
> > page size was hard coded for 4096.
> > 
> > With the lmem addition, lets align the stride to the page size of the
> > memory region that will be used for dumb buffer.
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > cc: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 20 +++++++++-----------
> >  1 file changed, 9 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index a712e60b016a..0f01396ca24e 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -239,8 +239,9 @@ i915_gem_dumb_create(struct drm_file *file,
> >                      struct drm_device *dev,
> >                      struct drm_mode_create_dumb *args)
> >  {
> > -       enum intel_memory_type mem_type;
> >         int cpp = DIV_ROUND_UP(args->bpp, 8);
> > +       enum intel_memory_type mem_type;
> > +       struct intel_memory_region *mr;
> >         u32 format;
> >  
> >         switch (cpp) {
> > @@ -260,24 +261,21 @@ i915_gem_dumb_create(struct drm_file *file,
> >         /* have to work out size/pitch and return them */
> >         args->pitch = ALIGN(args->width * cpp, 64);
> >  
> > +       mem_type = INTEL_MEMORY_SYSTEM;
> > +       if (HAS_LMEM(to_i915(dev)))
> > +               mem_type = INTEL_MEMORY_LOCAL;
> > +       mr = intel_memory_region_by_type(to_i915(dev), mem_type);
> > +
> >         /* align stride to page size so that we can remap */
> >         if (args->pitch > intel_plane_fb_max_stride(to_i915(dev), format,
> >                                                     DRM_FORMAT_MOD_LINEAR))
> > -               args->pitch = ALIGN(args->pitch, 4096);
> > +               args->pitch = ALIGN(args->pitch, mr->min_page_size);
> 
> That should be ggtt-page size, different semantics, right?
Chris,

Sicne the purpose(remapping stride) is not clear, couldn't understand
which page size this is. I assumed this is of hw page size.

Btw, there is no issues found with 4096 on lmem too. May be this is
unwanted change, unless they meant hw page size here and luckily we are
not hitting the issue. I am not sure though.

-Ram
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx