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

[snip]

quoted

quoted

  
+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

quoted

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat