Hi Javier
Am 16.04.24 um 13:25 schrieb Javier Martinez Canillas:
Thomas Zimmermann <tzimmermann@xxxxxxx> writes:
Hello Thomas,
Add an fbdev emulation for SHMEM-based memory managers. The code is
similar to fbdev-generic, but does not require an addition shadow
"additional" I think ?
Yes.
buffer for mmap(). Fbdev-shmem operates directly on the buffer object's
SHMEM pages. Fbdev's deferred-I/O mechanism updates the hardware state
on write operations.
v2:
- use drm_driver_legacy_fb_format() (Geert)
Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
Patch looks good to me.
Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
Thanks for reviewing.
Just a couple of questions below:
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/drm_fbdev_shmem.c | 316 ++++++++++++++++++++++++++++++
Should fbdev-generic then be renamed to fbdev_shmem_shadow or something
like that ?
We'll do that in patch 42. :)
[...]
+
+ /* screen */
+ info->flags |= FBINFO_VIRTFB; /* system memory */
+ if (!shmem->map_wc)
+ info->flags |= FBINFO_READS_FAST; /* signal caching */
+ info->screen_size = sizes->surface_height * fb->pitches[0];
+ info->screen_buffer = map.vaddr;
+ info->fix.smem_len = info->screen_size;
+
Do I understand correctly that info->fix.smem_start doesn't have to be set
because that's only used for I/O memory?
It's the start of the framebuffer memory in physical memory. Setting
smem_start only makes sense if the framebuffer is physically continuous,
which isn't the case here.
Since drm_fbdev_shmem_fb_mmap() calls fb_deferred_io_mmap() which in turn
sets vma->vm_ops = &fb_deferred_io_vm_ops and struct vm_operations_struct
fb_deferred_io_vm_ops .fault function handler is fb_deferred_io_fault()
that calls fb_deferred_io_page() which uses info->fix.smem_start value.
Right, except in this case, which we don't trigger. Patch 5 adds an
additional check to ensure this.
I guess is OK because is_vmalloc_addr() is always true for this case ?
It's not a vmalloc'ed address, but see patch 7. Fbdev-shmem uses a new
get_page callback in fb_defio. It provides the necessary page directly
to fb_defio.
This also made me think why info->fix.smem_len is really needed. Can't we
make the fbdev core to only look at that if info->screen_size is not set ?
The fbdev core doesn't use smem_len AFAICT. But smem_len is part of the
fbdev UAPI, so I set it. I assume that programs use it to go to the end
of the framebuffer memory.
Best regards
Thomas
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
