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 ? > 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> 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 ? [...] > + > + /* 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? 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. I guess is OK because is_vmalloc_addr() is always true for this case ? 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 ? -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
