[Mesa-dev] [PATCH 05/10] i965: Introduce a "memory zone" concept on BO allocation.

[Kenneth Graunke]

On Friday, May 4, 2018 2:18:11 PM PDT Chris Wilson wrote:
> Quoting Kenneth Graunke (2018-05-04 02:12:35)
> > diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > index 66f30a1637f..66828f319be 100644
> > --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > @@ -268,6 +268,7 @@ static struct brw_bo *
> >  bo_alloc_internal(struct brw_bufmgr *bufmgr,
> >                    const char *name,
> >                    uint64_t size,
> > +                  enum brw_memory_zone memzone,
> >                    unsigned flags,
> >                    uint32_t tiling_mode,
> >                    uint32_t stride)
> 
> Did you consider carving out some space from flags for the zone?
> 
> > @@ -426,23 +427,27 @@ err:
> >  
> >  struct brw_bo *
> >  brw_bo_alloc(struct brw_bufmgr *bufmgr,
> > -             const char *name, uint64_t size)
> > +             const char *name, uint64_t size,
> > +             enum brw_memory_zone memzone)
> >  {
> > -   return bo_alloc_internal(bufmgr, name, size, 0, I915_TILING_NONE, 0);
> > +   return bo_alloc_internal(bufmgr, name, size, memzone,
> > +                            0, I915_TILING_NONE, 0);
> >  }
> >  
> >  struct brw_bo *
> >  brw_bo_alloc_tiled(struct brw_bufmgr *bufmgr, const char *name,
> > -                   uint64_t size, uint32_t tiling_mode, uint32_t pitch,
> > +                   uint64_t size, enum brw_memory_zone memzone,
> > +                   uint32_t tiling_mode, uint32_t pitch,
> >                     unsigned flags)
> >  {
> > -   return bo_alloc_internal(bufmgr, name, size, flags, tiling_mode, pitch);
> > +   return bo_alloc_internal(bufmgr, name, size, memzone,
> > +                            flags, tiling_mode, pitch);
> >  }
> >  
> >  struct brw_bo *
> >  brw_bo_alloc_tiled_2d(struct brw_bufmgr *bufmgr, const char *name,
> > -                      int x, int y, int cpp, uint32_t tiling,
> > -                      uint32_t *pitch, unsigned flags)
> > +                      int x, int y, int cpp, enum brw_memory_zone memzone,
> > +                      uint32_t tiling, uint32_t *pitch, unsigned flags)
> 
> The main benefit will be one less parameter here; but whether that is
> more readable / less error prone?
> -Chris

I had considered it.  I liked the idea of having an enum, and using an
explicit enum parameter which forces you to make a choice...but it could
work either way.  We could make BRW_MEMZONE_OTHER the default option and
the others flags.

I'm not sure it would be cleaner though.  We also don't store it, as it
can be inferred from the address, so having it separate won't waste any
storage.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180507/31d030ed/attachment.sig>