[RFC 1/3] drm/driver: Add defaults for .gem_prime_export/import callbacks
Daniel Vetter
daniel at ffwll.ch
Mon Sep 24 14:47:16 UTC 2018
On Sat, Sep 22, 2018 at 05:58:30PM +0200, Noralf Trønnes wrote:
>
> Den 22.09.2018 12.10, skrev Daniel Vetter:
> > On Fri, Sep 21, 2018 at 06:42:28PM +0200, Noralf Trønnes wrote:
> > > The majority of drivers use drm_gem_prime_export() and
> > > drm_gem_prime_import() for these callbacks so let's make them the
> > > default.
> > >
> > > Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
> > > ---
> > > Documentation/gpu/todo.rst | 7 +++++++
> > > drivers/gpu/drm/drm_prime.c | 10 ++++++++--
> > > include/drm/drm_drv.h | 4 ++++
> > > 3 files changed, 19 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> > > index 77c2b3c25565..39748e22dea8 100644
> > > --- a/Documentation/gpu/todo.rst
> > > +++ b/Documentation/gpu/todo.rst
> > > @@ -234,6 +234,13 @@ efficient.
> > > Contact: Daniel Vetter
> > > +Defaults for .gem_prime_import and export
> > > +-----------------------------------------
> > > +
> > > +Most drivers don't need to set drm_driver->gem_prime_import and
> > > +->gem_prime_export now that drm_gem_prime_import() and drm_gem_prime_export()
> > > +are the default.
> > > +
> > > Core refactorings
> > > =================
> > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > > index 3f0205fc0a1a..6721571749ff 100644
> > > --- a/drivers/gpu/drm/drm_prime.c
> > > +++ b/drivers/gpu/drm/drm_prime.c
> > > @@ -559,7 +559,10 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev,
> > > return dmabuf;
> > > }
> > > - dmabuf = dev->driver->gem_prime_export(dev, obj, flags);
> > > + if (dev->driver->gem_prime_export)
> > > + dmabuf = dev->driver->gem_prime_export(dev, obj, flags);
> > > + else
> > > + dmabuf = drm_gem_prime_export(dev, obj, flags);
> > Hm, this one here operates on a drm_gem_object (the dev parameter is kinda
> > redundant). I think this would look neat in the callback structure you're
> > adding in the next patch. There's also a bunch of gem drivers overwriting
> > this one, so would make your callbacks structure more generally useful.
>
> I'm not following you here.
> What I do is to add a default for the 29 drivers that have this set to
> drm_gem_prime_export(). For the 8 drivers that have their own thing, I
> have the funcs->export callback that they can use.
>
> Combined with the next patch it looks like this:
>
> if (dev->driver->gem_prime_export)
> dmabuf = dev->driver->gem_prime_export(dev, obj, flags);
> else if (obj->funcs && obj->funcs->export)
> dmabuf = obj->funcs->export(obj, flags);
> else
> dmabuf = drm_gem_prime_export(dev, obj, flags);
Oh, I missed that you're adding the funcs->export thing already, that's
what I wanted to suggested.
-Daniel
>
> Noralf.
>
> > Otherwise lgtm.
> > -Daniel
> >
> > > if (IS_ERR(dmabuf)) {
> > > /* normally the created dma-buf takes ownership of the ref,
> > > * but if that fails then drop the ref
> > > @@ -792,7 +795,10 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
> > > /* never seen this one, need to import */
> > > mutex_lock(&dev->object_name_lock);
> > > - obj = dev->driver->gem_prime_import(dev, dma_buf);
> > > + if (dev->driver->gem_prime_import)
> > > + obj = dev->driver->gem_prime_import(dev, dma_buf);
> > > + else
> > > + obj = drm_gem_prime_import(dev, dma_buf);
> > > if (IS_ERR(obj)) {
> > > ret = PTR_ERR(obj);
> > > goto out_unlock;
> > > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> > > index 8830e3de3a86..1162145f7f68 100644
> > > --- a/include/drm/drm_drv.h
> > > +++ b/include/drm/drm_drv.h
> > > @@ -471,6 +471,8 @@ struct drm_driver {
> > > * @gem_prime_export:
> > > *
> > > * export GEM -> dmabuf
> > > + *
> > > + * This defaults to drm_gem_prime_export() if not set.
> > > */
> > > struct dma_buf * (*gem_prime_export)(struct drm_device *dev,
> > > struct drm_gem_object *obj, int flags);
> > > @@ -478,6 +480,8 @@ struct drm_driver {
> > > * @gem_prime_import:
> > > *
> > > * import dmabuf -> GEM
> > > + *
> > > + * This defaults to drm_gem_prime_import() if not set.
> > > */
> > > struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev,
> > > struct dma_buf *dma_buf);
> > > --
> > > 2.15.1
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list