[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191104183023.GT10326@phenom.ffwll.local>
Date: Mon, 4 Nov 2019 19:30:23 +0100
From: Daniel Vetter <daniel@...ll.ch>
To: Brian Starkey <Brian.Starkey@....com>
Cc: Dave Airlie <airlied@...il.com>, Hillf Danton <hdanton@...a.com>,
Sudipto Paul <Sudipto.Paul@....com>,
Sandeep Patil <sspatil@...gle.com>,
Alistair Strachan <astrachan@...gle.com>,
Vincent Donnefort <Vincent.Donnefort@....com>,
Chenbo Feng <fengc@...gle.com>,
lkml <linux-kernel@...r.kernel.org>,
Liam Mark <lmark@...eaurora.org>,
"Andrew F. Davis" <afd@...com>,
Christoph Hellwig <hch@...radead.org>, nd <nd@....com>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
Hridya Valsaraju <hridya@...gle.com>,
Pratik Patel <pratikp@...eaurora.org>
Subject: Re: [PATCH v14 1/5] dma-buf: Add dma-buf heaps framework
On Mon, Nov 04, 2019 at 05:43:51PM +0000, Brian Starkey wrote:
> Hi Dave,
>
> On Tue, Nov 05, 2019 at 02:58:17AM +1000, Dave Airlie wrote:
> > On Mon, 4 Nov 2019 at 20:24, Brian Starkey <Brian.Starkey@....com> wrote:
> > >
> > > Hi John,
> > >
> > > On Fri, Nov 01, 2019 at 09:42:34PM +0000, John Stultz wrote:
> > > > From: "Andrew F. Davis" <afd@...com>
> > > >
> > > > This framework allows a unified userspace interface for dma-buf
> > > > exporters, allowing userland to allocate specific types of memory
> > > > for use in dma-buf sharing.
> > > >
> > > > Each heap is given its own device node, which a user can allocate
> > > > a dma-buf fd from using the DMA_HEAP_IOC_ALLOC.
> > > >
> > > > Additionally should the interface grow in the future, we have a
> > > > DMA_HEAP_IOC_GET_FEATURES ioctl which can return future feature
> > > > flags.
> > >
> > > The userspace patch doesn't use this - and there's no indication of
> > > what it's intended to allow in the future. I missed the discussion
> > > about it, do you have a link?
> > >
> > > I thought the preferred approach was to add the new ioctl only when we
> > > need it, and new userspace on old kernels will get "ENOSYS" to know
> > > that the kernel doesn't support it.
> >
> > This works once, expand the interface 3 or 4 times with no features
> > ioctl, and you start being hostile to userspace, which feature does
> > ENOSYS mean isn't supported etc.
>
> Sorry, perhaps I wasn't clear (or I misunderstand what you're saying
> about working only once). I'm not against adding a get_features ioctl,
> I just don't see why we'd add it before we have any features?
>
> When we gain the first "feature", can't we add the get_features ioctl
> then? For Future SW that knows about Future features, "ENOSYS" will
> always mean "no features". If it doesn't get ENOSYS, then it can use
> the ioctl to find out what features it has.
>
> Otherwise we're adding an ioctl which doesn't do anything, based on
> the assumption that in the future it _will_ do something... but we
> don't know that that is yet... but we're pretty sure whatever it will
> do can be described with a u64?
>
> Why not wait until we know what we want it for, and then implement it
> with an interface which we know is appropriate at that time?
Yeah I'm with Brian, adding the get_feature ioctl when we need it.
Otherwise it's going to be broken somehow and we'll immediately ref to
get_features2 :-)
-Daniel
> > Next userspace starts to rely on kernel version, but then someone
> > backports a feature, down the rabbit hole we go.
> >
>
> I suppose that adding the feature ioctl later would mean that it might
> be possible to backport a feature without also backporting the ioctl,
> depending on how the patches are split up. I think it would be pretty
> hard to do accidentally though.
>
> Thanks,
> -Brian
>
> > Be nice to userspace give them a features ioctl they can use to figure
> > out in advance which of the 4 uapis the kernel supports.
> >
> > Dave.
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Powered by blists - more mailing lists