On Wed, Dec 11, 2024 at 10:06:28AM +0100, Thomas Zimmermann wrote: > DMA areas are not necessarily backed by struct page, so we cannot > rely on it for deferred I/O. Allocate a shadow buffer for drivers > that require deferred I/O and use it as framebuffer memory. > > Fixes driver errors about being "Unable to handle kernel NULL pointer > dereference at virtual address" or "Unable to handle kernel paging > request at virtual address". > > The patch splits drm_fbdev_dma_driver_fbdev_probe() in an initial > allocation, which creates the DMA-backed buffer object, and a tail > that sets up the fbdev data structures. There is a tail function for > direct memory mappings and a tail function for deferred I/O with > the shadow buffer. > > It is no longer possible to use deferred I/O without shadow buffer. > It can be re-added if there exists a reliably test for usable struct > page in the allocated DMA-backed buffer object. > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > Reported-by: Nuno Gonçalves <nunojpg@xxxxxxxxx> > CLoses: https://lore.kernel.org/dri-devel/CAEXMXLR55DziAMbv_+2hmLeH-jP96pmit6nhs6siB22cpQFr9w@xxxxxxxxxxxxxx/ > Tested-by: Nuno Gonçalves <nunojpg@xxxxxxxxx> > Fixes: 5ab91447aa13 ("drm/tiny/ili9225: Use fbdev-dma") > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> # v6.11+ fbdev code scares me, but I at least tried to check a few things and looks all good. Reviewed-by: Simona Vetter <simona.vetter@xxxxxxxx> > --- > drivers/gpu/drm/drm_fbdev_dma.c | 217 +++++++++++++++++++++++--------- > 1 file changed, 155 insertions(+), 62 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c > index b14b581c059d..02a516e77192 100644 > --- a/drivers/gpu/drm/drm_fbdev_dma.c > +++ b/drivers/gpu/drm/drm_fbdev_dma.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: MIT > > #include <linux/fb.h> > +#include <linux/vmalloc.h> > > #include <drm/drm_drv.h> > #include <drm/drm_fbdev_dma.h> > @@ -70,37 +71,102 @@ static const struct fb_ops drm_fbdev_dma_fb_ops = { > .fb_destroy = drm_fbdev_dma_fb_destroy, > }; > > -FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(drm_fbdev_dma, > +FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(drm_fbdev_dma_shadowed, > drm_fb_helper_damage_range, > drm_fb_helper_damage_area); > > -static int drm_fbdev_dma_deferred_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) > +static void drm_fbdev_dma_shadowed_fb_destroy(struct fb_info *info) > { > struct drm_fb_helper *fb_helper = info->par; > - struct drm_framebuffer *fb = fb_helper->fb; > - struct drm_gem_dma_object *dma = drm_fb_dma_get_gem_obj(fb, 0); > + void *shadow = info->screen_buffer; > + > + if (!fb_helper->dev) > + return; > > - if (!dma->map_noncoherent) > - vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); > + if (info->fbdefio) > + fb_deferred_io_cleanup(info); > + drm_fb_helper_fini(fb_helper); > + vfree(shadow); > > - return fb_deferred_io_mmap(info, vma); > + drm_client_buffer_vunmap(fb_helper->buffer); > + drm_client_framebuffer_delete(fb_helper->buffer); > + drm_client_release(&fb_helper->client); > + drm_fb_helper_unprepare(fb_helper); > + kfree(fb_helper); > } > > -static const struct fb_ops drm_fbdev_dma_deferred_fb_ops = { > +static const struct fb_ops drm_fbdev_dma_shadowed_fb_ops = { > .owner = THIS_MODULE, > .fb_open = drm_fbdev_dma_fb_open, > .fb_release = drm_fbdev_dma_fb_release, > - __FB_DEFAULT_DEFERRED_OPS_RDWR(drm_fbdev_dma), > + FB_DEFAULT_DEFERRED_OPS(drm_fbdev_dma_shadowed), > DRM_FB_HELPER_DEFAULT_OPS, > - __FB_DEFAULT_DEFERRED_OPS_DRAW(drm_fbdev_dma), > - .fb_mmap = drm_fbdev_dma_deferred_fb_mmap, > - .fb_destroy = drm_fbdev_dma_fb_destroy, > + .fb_destroy = drm_fbdev_dma_shadowed_fb_destroy, > }; > > /* > * struct drm_fb_helper > */ > > +static void drm_fbdev_dma_damage_blit_real(struct drm_fb_helper *fb_helper, > + struct drm_clip_rect *clip, > + struct iosys_map *dst) > +{ > + struct drm_framebuffer *fb = fb_helper->fb; > + size_t offset = clip->y1 * fb->pitches[0]; > + size_t len = clip->x2 - clip->x1; > + unsigned int y; > + void *src; > + > + switch (drm_format_info_bpp(fb->format, 0)) { > + case 1: > + offset += clip->x1 / 8; > + len = DIV_ROUND_UP(len + clip->x1 % 8, 8); > + break; > + case 2: > + offset += clip->x1 / 4; > + len = DIV_ROUND_UP(len + clip->x1 % 4, 4); > + break; > + case 4: > + offset += clip->x1 / 2; > + len = DIV_ROUND_UP(len + clip->x1 % 2, 2); > + break; > + default: > + offset += clip->x1 * fb->format->cpp[0]; > + len *= fb->format->cpp[0]; > + break; > + } > + > + src = fb_helper->info->screen_buffer + offset; > + iosys_map_incr(dst, offset); /* go to first pixel within clip rect */ > + > + for (y = clip->y1; y < clip->y2; y++) { > + iosys_map_memcpy_to(dst, 0, src, len); > + iosys_map_incr(dst, fb->pitches[0]); > + src += fb->pitches[0]; > + } > +} > + > +static int drm_fbdev_dma_damage_blit(struct drm_fb_helper *fb_helper, > + struct drm_clip_rect *clip) > +{ > + struct drm_client_buffer *buffer = fb_helper->buffer; > + struct iosys_map dst; > + > + /* > + * For fbdev emulation, we only have to protect against fbdev modeset > + * operations. Nothing else will involve the client buffer's BO. So it > + * is sufficient to acquire struct drm_fb_helper.lock here. > + */ > + mutex_lock(&fb_helper->lock); > + > + dst = buffer->map; > + drm_fbdev_dma_damage_blit_real(fb_helper, clip, &dst); > + > + mutex_unlock(&fb_helper->lock); > + > + return 0; > +} > static int drm_fbdev_dma_helper_fb_dirty(struct drm_fb_helper *helper, > struct drm_clip_rect *clip) > { > @@ -112,6 +178,10 @@ static int drm_fbdev_dma_helper_fb_dirty(struct drm_fb_helper *helper, > return 0; > > if (helper->fb->funcs->dirty) { > + ret = drm_fbdev_dma_damage_blit(helper, clip); > + if (drm_WARN_ONCE(dev, ret, "Damage blitter failed: ret=%d\n", ret)) > + return ret; > + > ret = helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, clip, 1); > if (drm_WARN_ONCE(dev, ret, "Dirty helper failed: ret=%d\n", ret)) > return ret; > @@ -128,14 +198,80 @@ static const struct drm_fb_helper_funcs drm_fbdev_dma_helper_funcs = { > * struct drm_fb_helper > */ > > +static int drm_fbdev_dma_driver_fbdev_probe_tail(struct drm_fb_helper *fb_helper, > + struct drm_fb_helper_surface_size *sizes) > +{ > + struct drm_device *dev = fb_helper->dev; > + struct drm_client_buffer *buffer = fb_helper->buffer; > + struct drm_gem_dma_object *dma_obj = to_drm_gem_dma_obj(buffer->gem); > + struct drm_framebuffer *fb = fb_helper->fb; > + struct fb_info *info = fb_helper->info; > + struct iosys_map map = buffer->map; > + > + info->fbops = &drm_fbdev_dma_fb_ops; > + > + /* screen */ > + info->flags |= FBINFO_VIRTFB; /* system memory */ > + if (dma_obj->map_noncoherent) > + info->flags |= FBINFO_READS_FAST; /* signal caching */ > + info->screen_size = sizes->surface_height * fb->pitches[0]; > + info->screen_buffer = map.vaddr; > + if (!(info->flags & FBINFO_HIDE_SMEM_START)) { > + if (!drm_WARN_ON(dev, is_vmalloc_addr(info->screen_buffer))) > + info->fix.smem_start = page_to_phys(virt_to_page(info->screen_buffer)); > + } > + info->fix.smem_len = info->screen_size; > + > + return 0; > +} > + > +static int drm_fbdev_dma_driver_fbdev_probe_tail_shadowed(struct drm_fb_helper *fb_helper, > + struct drm_fb_helper_surface_size *sizes) > +{ > + struct drm_client_buffer *buffer = fb_helper->buffer; > + struct fb_info *info = fb_helper->info; > + size_t screen_size = buffer->gem->size; > + void *screen_buffer; > + int ret; > + > + /* > + * Deferred I/O requires struct page for framebuffer memory, > + * which is not guaranteed for all DMA ranges. We thus create > + * a shadow buffer in system memory. > + */ > + screen_buffer = vzalloc(screen_size); > + if (!screen_buffer) > + return -ENOMEM; > + > + info->fbops = &drm_fbdev_dma_shadowed_fb_ops; > + > + /* screen */ > + info->flags |= FBINFO_VIRTFB; /* system memory */ > + info->flags |= FBINFO_READS_FAST; /* signal caching */ > + info->screen_buffer = screen_buffer; > + info->fix.smem_len = screen_size; > + > + fb_helper->fbdefio.delay = HZ / 20; > + fb_helper->fbdefio.deferred_io = drm_fb_helper_deferred_io; > + > + info->fbdefio = &fb_helper->fbdefio; > + ret = fb_deferred_io_init(info); > + if (ret) > + goto err_vfree; > + > + return 0; > + > +err_vfree: > + vfree(screen_buffer); > + return ret; > +} > + > int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper, > struct drm_fb_helper_surface_size *sizes) > { > struct drm_client_dev *client = &fb_helper->client; > struct drm_device *dev = fb_helper->dev; > - bool use_deferred_io = false; > struct drm_client_buffer *buffer; > - struct drm_gem_dma_object *dma_obj; > struct drm_framebuffer *fb; > struct fb_info *info; > u32 format; > @@ -152,19 +288,9 @@ int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper, > sizes->surface_height, format); > if (IS_ERR(buffer)) > return PTR_ERR(buffer); > - dma_obj = to_drm_gem_dma_obj(buffer->gem); > > fb = buffer->fb; > > - /* > - * Deferred I/O requires struct page for framebuffer memory, > - * which is not guaranteed for all DMA ranges. We thus only > - * install deferred I/O if we have a framebuffer that requires > - * it. > - */ > - if (fb->funcs->dirty) > - use_deferred_io = true; > - > ret = drm_client_buffer_vmap(buffer, &map); > if (ret) { > goto err_drm_client_buffer_delete; > @@ -185,45 +311,12 @@ int drm_fbdev_dma_driver_fbdev_probe(struct drm_fb_helper *fb_helper, > > drm_fb_helper_fill_info(info, fb_helper, sizes); > > - if (use_deferred_io) > - info->fbops = &drm_fbdev_dma_deferred_fb_ops; > + if (fb->funcs->dirty) > + ret = drm_fbdev_dma_driver_fbdev_probe_tail_shadowed(fb_helper, sizes); > else > - info->fbops = &drm_fbdev_dma_fb_ops; > - > - /* screen */ > - info->flags |= FBINFO_VIRTFB; /* system memory */ > - if (dma_obj->map_noncoherent) > - info->flags |= FBINFO_READS_FAST; /* signal caching */ > - info->screen_size = sizes->surface_height * fb->pitches[0]; > - info->screen_buffer = map.vaddr; > - if (!(info->flags & FBINFO_HIDE_SMEM_START)) { > - if (!drm_WARN_ON(dev, is_vmalloc_addr(info->screen_buffer))) > - info->fix.smem_start = page_to_phys(virt_to_page(info->screen_buffer)); > - } > - info->fix.smem_len = info->screen_size; > - > - /* > - * Only set up deferred I/O if the screen buffer supports > - * it. If this disagrees with the previous test for ->dirty, > - * mmap on the /dev/fb file might not work correctly. > - */ > - if (!is_vmalloc_addr(info->screen_buffer) && info->fix.smem_start) { > - unsigned long pfn = info->fix.smem_start >> PAGE_SHIFT; > - > - if (drm_WARN_ON(dev, !pfn_to_page(pfn))) > - use_deferred_io = false; > - } > - > - /* deferred I/O */ > - if (use_deferred_io) { > - fb_helper->fbdefio.delay = HZ / 20; > - fb_helper->fbdefio.deferred_io = drm_fb_helper_deferred_io; > - > - info->fbdefio = &fb_helper->fbdefio; > - ret = fb_deferred_io_init(info); > - if (ret) > - goto err_drm_fb_helper_release_info; > - } > + ret = drm_fbdev_dma_driver_fbdev_probe_tail(fb_helper, sizes); > + if (ret) > + goto err_drm_fb_helper_release_info; > > return 0; > > -- > 2.47.1 > -- Simona Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
