Re: [PATCH 2/2] drm/omapdrm: Fix console with deferred ops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 20.02.24 um 09:56 schrieb Tony Lindgren:
* Thomas Zimmermann <tzimmermann@xxxxxxx> [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)




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux