On Mon, Jun 17, 2019 at 06:54:04PM +0200, Noralf Trønnes wrote:
>
>
> Den 17.06.2019 18.29, skrev Daniel Vetter:
> > On Mon, Jun 17, 2019 at 05:47:50PM +0200, Noralf Trønnes wrote:
> >>
> >>
> >> Den 14.06.2019 22.35, skrev Daniel Vetter:
> >>> We're kinda going in the wrong direction. Spotted while typing better
> >>> gem/prime docs.
> >>>
> >>> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> >>> Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> >>> Cc: Rob Herring <robh@xxxxxxxxxx>
> >>> Cc: Noralf Trønnes <noralf@xxxxxxxxxxx>
> >>> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> >>> ---
> >>> Documentation/gpu/todo.rst | 4 ++++
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> >>> index b4a76c2703e5..23583f0e3755 100644
> >>> --- a/Documentation/gpu/todo.rst
> >>> +++ b/Documentation/gpu/todo.rst
> >>> @@ -228,6 +228,10 @@ struct drm_gem_object_funcs
> >>> GEM objects can now have a function table instead of having the callbacks on the
> >>> DRM driver struct. This is now the preferred way and drivers can be moved over.
> >>>
> >>> +Unfortunately some of the recently added GEM helpers are going in the wrong
> >>> +direction by adding OPS macros that use the old, deprecated hooks. See
> >>> +DRM_GEM_CMA_VMAP_DRIVER_OPS, DRM_GEM_SHMEM_DRIVER_OPS, and DRM_GEM_VRAM_DRIVER_PRIME.
> >>> +
> >>
> >> Both DRM_GEM_CMA_VMAP_DRIVER_OPS and DRM_GEM_SHMEM_DRIVER_OPS use the
> >> GEM vtable. Or am I missing something here?
> >
> > gem vtable I mean drm_gem_object_funcs. Which these macros definitely
> > aren't useful for.
>
> #define DRM_GEM_CMA_VMAP_DRIVER_OPS \
> .gem_create_object = drm_cma_gem_create_object_default_funcs, \
> .dumb_create = drm_gem_cma_dumb_create, \
> .prime_handle_to_fd = drm_gem_prime_handle_to_fd, \
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle, \
> .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table_vmap, \
> .gem_prime_mmap = drm_gem_prime_mmap
>
> __drm_gem_cma_create() calls ->gem_create_object.
>
> drm_cma_gem_create_object_default_funcs() sets:
> cma_obj->base.funcs = &drm_cma_gem_default_funcs;
>
> static const struct drm_gem_object_funcs drm_cma_gem_default_funcs = {
> .free = drm_gem_cma_free_object,
> .print_info = drm_gem_cma_print_info,
> .get_sg_table = drm_gem_cma_prime_get_sg_table,
> .vmap = drm_gem_cma_prime_vmap,
> .vm_ops = &drm_gem_cma_vm_ops,
> };
>
> The GEM SHMEM helper was made after drm_gem_object_funcs came about so
> it sets the default vtable in drm_gem_shmem_create():
> obj->funcs = &drm_gem_shmem_funcs;
>
> static const struct drm_gem_object_funcs drm_gem_shmem_funcs = {
> .free = drm_gem_shmem_free_object,
> .print_info = drm_gem_shmem_print_info,
> .pin = drm_gem_shmem_pin,
> .unpin = drm_gem_shmem_unpin,
> .get_sg_table = drm_gem_shmem_get_sg_table,
> .vmap = drm_gem_shmem_vmap,
> .vunmap = drm_gem_shmem_vunmap,
> .vm_ops = &drm_gem_shmem_vm_ops,
> };
>
> #define DRM_GEM_SHMEM_DRIVER_OPS \
> .prime_handle_to_fd = drm_gem_prime_handle_to_fd, \
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle, \
> .gem_prime_import_sg_table = drm_gem_shmem_prime_import_sg_table, \
> .gem_prime_mmap = drm_gem_prime_mmap, \
> .dumb_create = drm_gem_shmem_dumb_create
>
> So the two driver ops macroes only set the necessary bits to enable
> prime import/export/mmap and dumb buffer creation, leaving the rest to
> drm_gem_object_funcs.
> Have we deprecated any of these hooks?
Uh I was blind :-/ Unfortunately I pushed that patch already, I'll follow
up with a patch to fix it. vram helpers are not following latest best
practices though, right?
Also I guess a lot more of the cma helper using drivers could be cut over
to the vmap ones?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
