Re: [EARLY RFC][PATCH] dma-buf: Add dma-buf heaps framework

[John Stultz <john.stultz@linaro.org>]

On Wed, Feb 27, 2019 at 3:22 PM Laura Abbott <labbott@redhat.com> wrote:
> On 2/25/19 6:36 AM, Andrew F. Davis wrote:
> > +
> > +dev_t dma_heap_devt;
> > +struct class *dma_heap_class;
>
> Can we make sure this gets reviewed by Greg sooner rather than
> later when we drop the RFC? I think the use of this here
> is fine with the device model but I'd rather not find out at
> the end.

Agreed!

> > +/**
> > + * struct dma_heap_allocation_data - metadata passed from userspace for
> > + *                                      allocations
> > + * @len:             size of the allocation
> > + * @flags:           flags passed to pool
> > + * @fd:                      will be populated with a fd which provdes the
> > + *                   handle to the allocated dma-buf
> > + *
> > + * Provided by userspace as an argument to the ioctl
> > + */
> > +struct dma_heap_allocation_data {
> > +     __u64 len;
> > +     __u32 flags;
> > +     __u32 fd;
> > +     __u32 reserved0;
> > +     __u32 reserved1;
> > +};
> > +
>
> Did you have anything in particular for the reserved fields?
> I don't object to having them, was just curious.

As we have thinned down some of the ion fields, I'm mostly suspecting
some additions may be needed as we start integrating more vendor
implemented heaps. But I don't have any plans myself.

> It might be worth considering bumping up the flags to u64
> and/or adding an align field back in. I originally removed
> the align field because nothing was actually using it and
> it seemed to cause more confusion than anything. I suggested
> if heaps really wanted an alignment they could pass it in
> the flags.

Yea, I think a u64 for flags is a good idea. If we don't have an
immediate use for an alignment field, it might be good to just hold
one of the reserved fields for that for now before complicating the
interface w/o users.

> Did you have thoughts on declaring which bits of the flags
> can be used by heaps for their own private use? That was
> a request I got several times.

Hrm.  I sort of feel like if folks can create their own heaps, we
shouldn't need too many options on how to allocate from those heaps.
I'd rather try to force a more standard interface so that userland
allocators can be more easily switched to use different heaps (just by
changing the pathname) w/o extra considerations.

But maybe I'm being a short-sighted. I guess if we really need this,
I'd probably just add an explicit heap_private field instead? I guess
that's another potential use for the reserved fields?

thanks
-john
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel