[igt-dev] [PATCH i-g-t v26 13/35] lib/intel_bufops: Add init with handle and size function

[Zbigniew Kempczyński]

On Wed, Mar 17, 2021 at 04:36:54PM -0500, Jason Ekstrand wrote:
> On Wed, Mar 17, 2021 at 9:46 AM Zbigniew Kempczyński
> <zbigniew.kempczynski at intel.com> wrote:
> >
> > For some cases (fb with compression) fb size is bigger than calculated
> > intel_buf what lead to execbuf failure when allocator is used
> > along with EXEC_OBJECT_PINNED flag set for all objects.
> >
> > We need to create intel_buf with size equal to fb so new function
> > in which we pass handle and size is required.
> >
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Cc: Dominik Grzegorzek <dominik.grzegorzek at intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  lib/intel_bufops.c | 33 ++++++++++++++++++++++++++++-----
> >  lib/intel_bufops.h |  7 +++++++
> >  2 files changed, 35 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c
> > index eb5ac4dad..d8eb64e3a 100644
> > --- a/lib/intel_bufops.c
> > +++ b/lib/intel_bufops.c
> > @@ -708,7 +708,8 @@ static void __intel_buf_init(struct buf_ops *bops,
> >                              uint32_t handle,
> >                              struct intel_buf *buf,
> >                              int width, int height, int bpp, int alignment,
> > -                            uint32_t req_tiling, uint32_t compression)
> > +                            uint32_t req_tiling, uint32_t compression,
> > +                            uint64_t bo_size)
> >  {
> >         uint32_t tiling = req_tiling;
> >         uint64_t size;
> > @@ -758,7 +759,7 @@ static void __intel_buf_init(struct buf_ops *bops,
> >                 buf->ccs[0].offset = buf->surface[0].stride * ALIGN(height, 32);
> >                 buf->ccs[0].stride = aux_width;
> >
> > -               size = buf->ccs[0].offset + aux_width * aux_height;
> > +               size = max(bo_size, buf->ccs[0].offset + aux_width * aux_height);
> >         } else {
> >                 if (tiling) {
> >                         devid =  intel_get_drm_devid(bops->fd);
> > @@ -773,7 +774,7 @@ static void __intel_buf_init(struct buf_ops *bops,
> >                 buf->tiling = tiling;
> >                 buf->bpp = bpp;
> >
> > -               size = buf->surface[0].stride * ALIGN(height, align_h);
> > +               size = max(bo_size, buf->surface[0].stride * ALIGN(height, align_h));
> 
> Do we want to take a max() here or do something like this afterwards:
> 
> if (bo_size > 0) {
>     igt_assert(bo_size >= size);
>     buf->size = bo_size;
> } else {
>     buf->size = size;
> }

Agree, if caller really want to pass bo_size we expect he will at least
provide valid size (according to w/h/bpp).

This is cosmetic change which provides better input data protection
so I dare to keep Chris r-b here. If you would complain I'll remove
this in next series.

--
Zbigniew


> 
> >         }
> >
> >         /* Store real bo size to avoid mistakes in calculating it again */
> > @@ -809,7 +810,7 @@ void intel_buf_init(struct buf_ops *bops,
> >                     uint32_t tiling, uint32_t compression)
> >  {
> >         __intel_buf_init(bops, 0, buf, width, height, bpp, alignment,
> > -                        tiling, compression);
> > +                        tiling, compression, 0);
> >
> >         intel_buf_set_ownership(buf, true);
> >  }
> > @@ -858,7 +859,7 @@ void intel_buf_init_using_handle(struct buf_ops *bops,
> >                                  uint32_t req_tiling, uint32_t compression)
> >  {
> >         __intel_buf_init(bops, handle, buf, width, height, bpp, alignment,
> > -                        req_tiling, compression);
> > +                        req_tiling, compression, 0);
> >  }
> >
> >  /**
> > @@ -927,6 +928,28 @@ struct intel_buf *intel_buf_create_using_handle(struct buf_ops *bops,
> >         return buf;
> >  }
> >
> > +struct intel_buf *intel_buf_create_using_handle_and_size(struct buf_ops *bops,
> > +                                                        uint32_t handle,
> > +                                                        int width, int height,
> > +                                                        int bpp, int alignment,
> > +                                                        uint32_t req_tiling,
> > +                                                        uint32_t compression,
> > +                                                        uint64_t size)
> > +{
> > +       struct intel_buf *buf;
> > +
> > +       igt_assert(bops);
> > +
> > +       buf = calloc(1, sizeof(*buf));
> > +       igt_assert(buf);
> > +
> > +       __intel_buf_init(bops, handle, buf, width, height, bpp, alignment,
> > +                        req_tiling, compression, size);
> > +
> > +       return buf;
> > +}
> > +
> > +
> >  /**
> >   * intel_buf_destroy
> >   * @buf: intel_buf
> > diff --git a/lib/intel_bufops.h b/lib/intel_bufops.h
> > index 5619fc6fa..54480bff6 100644
> > --- a/lib/intel_bufops.h
> > +++ b/lib/intel_bufops.h
> > @@ -139,6 +139,13 @@ struct intel_buf *intel_buf_create_using_handle(struct buf_ops *bops,
> >                                                 uint32_t req_tiling,
> >                                                 uint32_t compression);
> >
> > +struct intel_buf *intel_buf_create_using_handle_and_size(struct buf_ops *bops,
> > +                                                        uint32_t handle,
> > +                                                        int width, int height,
> > +                                                        int bpp, int alignment,
> > +                                                        uint32_t req_tiling,
> > +                                                        uint32_t compression,
> > +                                                        uint64_t size);
> >  void intel_buf_destroy(struct intel_buf *buf);
> >
> >  void *intel_buf_cpu_map(struct intel_buf *buf, bool write);
> > --
> > 2.26.0
> >