| Message ID | 20180921164230.51838-2-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/gem: Add drm_gem_object_funcs | expand |
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@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. 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@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); 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@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@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@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
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); 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);
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@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(-)