Re: [PATCH 8/9] drm/gem-shmem: Implement fbdev dumb buffer and mmap helpers

On 3/9/22 09:47, Thomas Zimmermann wrote:

[snip]

...

...

+static const struct drm_gem_object_funcs drm_gem_shmem_funcs_fbdev = {

• .free = drm_gem_shmem_object_free,
• .print_info = drm_gem_shmem_object_print_info,
• .pin = drm_gem_shmem_object_pin,
• .unpin = drm_gem_shmem_object_unpin,
• .get_sg_table = drm_gem_shmem_object_get_sg_table,
• .vmap = drm_gem_shmem_object_vmap,
• .vunmap = drm_gem_shmem_object_vunmap,
• .mmap = drm_gem_shmem_object_mmap_fbdev,
• .vm_ops = &drm_gem_shmem_vm_ops_fbdev,

The drm_gem_shmem_funcs_fbdev is the same than drm_gem_shmem_funcs but .mmap and .vm_ops callbacks. Maybe adding a macro to declare these two struct drm_gem_object_funcs could make easier to maintain it long term ?

I see you point, but I'm undecided about this. Such macros also add some amount of obfuscation. I'm not sure if it's worth it for an internal constant. And since the fbdev buffer is never exported, we could remove some of the callbacks. AFAICT we never call pin, unpin or get_sg_table.

Yeah, that's true too. It was just a suggestion, I'm OK with patch as is.

Best regards Thomas

...