[PATCH 2/2] drm/omapdrm: Fix console with deferred ops
Thomas Zimmermann
tzimmermann at suse.de
Tue Feb 20 09:32:16 UTC 2024
Hi
Am 20.02.24 um 09:56 schrieb Tony Lindgren:
> * Thomas Zimmermann <tzimmermann at suse.de> [240219 16:43]:
>> Am 19.02.24 um 15:19 schrieb Tony Lindgren:
>>> --- a/drivers/gpu/drm/omapdrm/Kconfig
>>> +++ b/drivers/gpu/drm/omapdrm/Kconfig
>>> @@ -5,6 +5,7 @@ config DRM_OMAP
>>> depends on ARCH_OMAP2PLUS
>>> select DRM_KMS_HELPER
>>> select FB_DMAMEM_HELPERS if DRM_FBDEV_EMULATION
>>> + select FB_IOMEM_HELPERS if DRM_FBDEV_EMULATION
>> Anything named _IOMEM_ is for framebuffer's in I/O memory space. Just keep
>> DMAMEM_HELPERS with the few changes below.
> Oh right, yes omapdrm is operating on memory.
With the latest kernels, you should see a warning if helpers operate on
the wrong type of memory. [1][2]
[1]
https://elixir.bootlin.com/linux/v6.8-rc5/source/drivers/video/fbdev/core/fb_io_fops.c#L16
[2]
https://elixir.bootlin.com/linux/v6.8-rc5/source/drivers/video/fbdev/core/fb_sys_fops.c#L26
>
>>> select VIDEOMODE_HELPERS
>>> select HDMI
>>> default n
>>> diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
>>> --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
>>> +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
>>> @@ -51,6 +51,10 @@ static void pan_worker(struct work_struct *work)
>>> omap_gem_roll(bo, fbi->var.yoffset * npages);
>>> }
>>> +FB_GEN_DEFAULT_DEFERRED_IOMEM_OPS(omap_fbdev,
>>> + drm_fb_helper_damage_range,
>>> + drm_fb_helper_damage_area)
>>> +
>> Please create FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS() by duplicating
>> FB_GEN_DEFAULT_DEFERRED_SYSMEM_OPS() in <linux/fb.h>
> OK
>
>>> static int omap_fbdev_pan_display(struct fb_var_screeninfo *var,
>>> struct fb_info *fbi)
>>> {
>>> @@ -106,13 +110,13 @@ static void omap_fbdev_fb_destroy(struct fb_info *info)
>>> static const struct fb_ops omap_fb_ops = {
>>> .owner = THIS_MODULE,
>>> - __FB_DEFAULT_DMAMEM_OPS_RDWR,
>>> + __FB_DEFAULT_DEFERRED_OPS_RDWR(omap_fbdev),
>>> .fb_check_var = drm_fb_helper_check_var,
>>> .fb_set_par = drm_fb_helper_set_par,
>>> .fb_setcmap = drm_fb_helper_setcmap,
>>> .fb_blank = drm_fb_helper_blank,
>>> .fb_pan_display = omap_fbdev_pan_display,
>>> - __FB_DEFAULT_DMAMEM_OPS_DRAW,
>>> + __FB_DEFAULT_DEFERRED_OPS_DRAW(omap_fbdev),
>>> .fb_ioctl = drm_fb_helper_ioctl,
>>> .fb_mmap = omap_fbdev_fb_mmap,
>> The write and draw callbacks track the written pages and flush them to the
>> backbuffer. But mmap is a problem here, because mmap needs to do this as
>> well. You'd have to use fb_deferred_io_mmap() here and call
>> fb_deferred_io_init() in omap's fbdev init. See the generic fbdev in
>> drm_fbdev_generic() for a working example. But IDK whether that works easily
>> for omap's DMA memory. You have to mmap and track memory pages (i.e., struct
>> page).
> The following test patch works for me.. Not sure about the tracking though.
I know that i915 doesn't track mmap'ed pages correctly and I've see
systems that do not update the framebuffer. IDK how/why this works with
omapdrm.
> Do you mean that tracking needs to be implemented if fb_deferred_io_mmap()
> did not work?
omap_fbdev_fb_mmap() appears to mmap the DMA memory pages directly to
userspace. So the fb_dirty callback won't be invoked when userspace
writes to this framebuffer memory.
To implement tracking, you'd need to set fb_mmap to
fb_deferred_io_mmap(). If you init omap_fb_ops with
FB_DEFAULT_DEFERRED_OPS(), [3] you'd get that automatically.
fb_deferred_io_mmap() sets up the tracking whenever userspaces writes to
a mapped page. You also need to init the write-back mechanisms that
calls fb_dirty for the tracked pages. You should be able to duplicate
the code from [4] into omapdrm.
Best regards
Thomas
[3] https://elixir.bootlin.com/linux/latest/source/include/linux/fb.h#L704
[4]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_fbdev_generic.c#L119
>
>> The easy solution is to clear the fb_mmap callback and mmap() will thne not
>> be available to userspace.
> Sounds like that would break things for userspace.
Yes, probably.
>
> Regards,
>
> Tony
>
> 8< ------------------------
> diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
> @@ -51,6 +51,10 @@ static void pan_worker(struct work_struct *work)
> omap_gem_roll(bo, fbi->var.yoffset * npages);
> }
>
> +FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(omap_fbdev,
> + drm_fb_helper_damage_range,
> + drm_fb_helper_damage_area)
> +
> static int omap_fbdev_pan_display(struct fb_var_screeninfo *var,
> struct fb_info *fbi)
> {
> @@ -80,9 +84,13 @@ static int omap_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
> {
> struct drm_fb_helper *helper = info->par;
> struct drm_framebuffer *fb = helper->fb;
> - struct drm_gem_object *bo = drm_gem_fb_get_obj(fb, 0);
> + struct drm_gem_object *bo;
>
> - return drm_gem_mmap_obj(bo, omap_gem_mmap_size(bo), vma);
> + bo = drm_gem_fb_get_obj(fb, 0);
> + if (!bo)
> + return -EINVAL;
> +
> + return fb_deferred_io_mmap(info, vma);
> }
>
> static void omap_fbdev_fb_destroy(struct fb_info *info)
> @@ -94,6 +102,7 @@ static void omap_fbdev_fb_destroy(struct fb_info *info)
>
> DBG();
>
> + fb_deferred_io_cleanup(info);
> drm_fb_helper_fini(helper);
>
> omap_gem_unpin(bo);
> @@ -106,13 +115,13 @@ static void omap_fbdev_fb_destroy(struct fb_info *info)
>
> static const struct fb_ops omap_fb_ops = {
> .owner = THIS_MODULE,
> - __FB_DEFAULT_DMAMEM_OPS_RDWR,
> + __FB_DEFAULT_DEFERRED_OPS_RDWR(omap_fbdev),
> .fb_check_var = drm_fb_helper_check_var,
> .fb_set_par = drm_fb_helper_set_par,
> .fb_setcmap = drm_fb_helper_setcmap,
> .fb_blank = drm_fb_helper_blank,
> .fb_pan_display = omap_fbdev_pan_display,
> - __FB_DEFAULT_DMAMEM_OPS_DRAW,
> + __FB_DEFAULT_DEFERRED_OPS_DRAW(omap_fbdev),
> .fb_ioctl = drm_fb_helper_ioctl,
> .fb_mmap = omap_fbdev_fb_mmap,
> .fb_destroy = omap_fbdev_fb_destroy,
> @@ -213,6 +222,15 @@ static int omap_fbdev_create(struct drm_fb_helper *helper,
> fbi->fix.smem_start = dma_addr;
> fbi->fix.smem_len = bo->size;
>
> + /* deferred I/O */
> + helper->fbdefio.delay = HZ / 20;
> + helper->fbdefio.deferred_io = drm_fb_helper_deferred_io;
> +
> + fbi->fbdefio = &helper->fbdefio;
> + ret = fb_deferred_io_init(fbi);
> + if (ret)
> + goto fail;
> +
> /* if we have DMM, then we can use it for scrolling by just
> * shuffling pages around in DMM rather than doing sw blit.
> */
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -686,6 +686,10 @@ extern int fb_deferred_io_fsync(struct file *file, loff_t start,
> __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, sys) \
> __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, sys)
>
> +#define FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(__prefix, __damage_range, __damage_area) \
> + __FB_GEN_DEFAULT_DEFERRED_OPS_RDWR(__prefix, __damage_range, sys) \
> + __FB_GEN_DEFAULT_DEFERRED_OPS_DRAW(__prefix, __damage_area, sys)
> +
> /*
> * Initializes struct fb_ops for deferred I/O.
> */
--
--
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)
More information about the dri-devel
mailing list