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 12:05:11 +0000, Chris Wilson wrote:
> Quoting Ramalingam C (2020-02-05 12:02:50)
> > 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.
> 
> It for doing tricks with I915_GGTT_VIEW_REMAPPED (intel_remap_pages) so
> that a large framebuffer can be cut into CRTC-sized chunks and not go
> over the HW limits.

Sure. I will discard this change. Thanks for explaining that to me.

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