[Intel-gfx] [PATCH 1/2] i965/gen9: Pass alignment as function parameter in drm_intel_gem_bo_alloc_internal()

[Anuj Phogat]

On Mon, Jun 22, 2015 at 10:01 AM, Ben Widawsky <ben at bwidawsk.net> wrote:
> On Fri, Jun 19, 2015 at 03:50:44PM -0700, Anuj Phogat wrote:
>> +Ben.
>>
>> On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
>> > and use it to initialize the align variable in drm_intel_bo.
>> >
>> > In case of YF/YS tiled buffers libdrm need not know about the tiling
>> > format because these buffers don't have hardware support to be tiled
>> > or detiled through a fenced region. But, libdrm still need to know
>> > about buffer alignment restrictions because kernel uses it when
>> > resolving the relocation.
>> >
>> > Mesa uses drm_intel_gem_bo_alloc_for_render() to allocate Yf/Ys buffers.
>> > So, use the passed alignment value in this function. Note that we continue
>> > ignoring the alignment value passed to drm_intel_gem_bo_alloc() to follow
>> > the previous behavior.
>
> I think there is a problem here if you're getting the BO from the cache. If you
> allocate out of the cache, and the alignment is incorrect, I don't think
> anything will fix it for you.
>
Right. Thanks for catching this. I'll send out v2 with suggested changes.
>> >
>> > Cc: Kristian Høgsberg <krh at bitplanet.net>
>> > Cc: Damien Lespiau <damien.lespiau at intel.com>
>> > Cc: Daniel Vetter <daniel at ffwll.ch>
>> > Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> > ---
>> >  intel/intel_bufmgr_gem.c | 11 +++++++----
>> >  1 file changed, 7 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
>> > index 5a67f53..51d87ae 100644
>> > --- a/intel/intel_bufmgr_gem.c
>> > +++ b/intel/intel_bufmgr_gem.c
>> > @@ -655,7 +655,8 @@ drm_intel_gem_bo_alloc_internal(drm_intel_bufmgr *bufmgr,
>> >                                 unsigned long size,
>> >                                 unsigned long flags,
>> >                                 uint32_t tiling_mode,
>> > -                               unsigned long stride)
>> > +                               unsigned long stride,
>> > +                               unsigned int alignment)
>> >  {
>> >         drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bufmgr;
>> >         drm_intel_bo_gem *bo_gem;
>
> I think you need a check somewhere like:
> if (alignment &&
>     bo_gem->bo.align != alignment)
>         alloc_from_cache = false;
>
>> > @@ -754,6 +755,7 @@ retry:
>> >                         return NULL;
>> >                 }
>> >                 bo_gem->bo.bufmgr = bufmgr;
>> > +               bo_gem->bo.align = alignment;
>> >
>> >                 bo_gem->tiling_mode = I915_TILING_NONE;
>> >                 bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE;
>> > @@ -797,7 +799,8 @@ drm_intel_gem_bo_alloc_for_render(drm_intel_bufmgr *bufmgr,
>> >  {
>> >         return drm_intel_gem_bo_alloc_internal(bufmgr, name, size,
>> >                                                BO_ALLOC_FOR_RENDER,
>> > -                                              I915_TILING_NONE, 0);
>> > +                                              I915_TILING_NONE, 0,
>> > +                                              alignment);
>> >  }
>> >
>> >  static drm_intel_bo *
>> > @@ -807,7 +810,7 @@ drm_intel_gem_bo_alloc(drm_intel_bufmgr *bufmgr,
>> >                        unsigned int alignment)
>> >  {
>> >         return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, 0,
>> > -                                              I915_TILING_NONE, 0);
>> > +                                              I915_TILING_NONE, 0, 0);
>> >  }
>> >
>> >  static drm_intel_bo *
>> > @@ -859,7 +862,7 @@ drm_intel_gem_bo_alloc_tiled(drm_intel_bufmgr *bufmgr, const char *name,
>> >                 stride = 0;
>> >
>> >         return drm_intel_gem_bo_alloc_internal(bufmgr, name, size, flags,
>> > -                                              tiling, stride);
>> > +                                              tiling, stride, 0);
>> >  }
>> >
>> >  static drm_intel_bo *
>> > --
>> > 2.3.4
>> >