Re: [PATCH v3 21/43] drm/fbdev-dma: Implement damage handling and deferred I/O

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

 



Hi Linus, Alexander,

Attached is a patch to mitigate the problem and go back to the old behavior for pl111. Can you please test it on affected and unaffected systems and report the results?

Best regards
Thomas

Am 04.09.24 um 00:53 schrieb Linus Walleij:
On Fri, Apr 19, 2024 at 10:35 AM Thomas Zimmermann <tzimmermann@xxxxxxx> wrote:

Add support for damage handling and deferred I/O to fbdev-dma. This
enables fbdev-dma to support all DMA-memory-based DRM drivers, even
such with a dirty callback in their framebuffers.

The patch adds the code for deferred I/O and also sets a dedicated
helper for struct fb_ops.fb_mmap that support coherent mappings.

v3:
- init fb_ops with FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS() (Javier)

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
For some reason this makes the Versatile Express crash in QEMU
(I can retest on real hardware if necessary):

8<--- cut here ---
Unable to handle kernel paging request at virtual address 18095000 when write
[18095000] *pgd=00000000
Internal error: Oops: 805 [#1] SMP ARM
CPU: 0 PID: 75 Comm: S08_ledflash Not tainted 6.9.0-rc6+ #44
Hardware name: ARM-Versatile Express
PC is at mmioset+0x34/0xac
LR is at 0x0
pc : [<809c2a54>]    lr : [<00000000>]    psr: 20000013
sp : f8dddc38  ip : 18095000  fp : 00000000
r10: 81109a18  r9 : 81238b04  r8 : 00000000
r7 : 00440dc0  r6 : ed4f32a0  r5 : ed4f32a0  r4 : 00000001
r3 : 00000000  r2 : 00000fc0  r1 : 00000000  r0 : 18095000
Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 81d6406a  DAC: 00000051
Register r0 information: non-paged memory
Register r1 information: NULL pointer
Register r2 information: non-paged memory
Register r3 information: NULL pointer
Register r4 information: non-paged memory
Register r5 information: non-slab/vmalloc memory
Register r6 information: non-slab/vmalloc memory
Register r7 information: non-paged memory
Register r8 information: NULL pointer
Register r9 information: non-slab/vmalloc memory
Register r10 information: non-slab/vmalloc memory
Register r11 information: NULL pointer
Register r12 information: non-paged memory
Process S08_ledflash (pid: 75, stack limit = 0x(ptrval))
Stack: (0xf8dddc38 to 0xf8dde000)
dc20:                                                       00000801 802af888
dc40: 80a0fb80 00440dc0 00000801 811fad80 00000000 f8dddd28 ed4f32a0 811fa680
dc60: 00000000 802b0c30 ee1d2140 ee1d215c 95ff4385 00000001 00000001 00000001
dc80: 00000000 00000000 ffffffff 8196a70b 000000b0 00000064 0000006c 00000000
dca0: 00000001 00000002 00000001 811fa798 00000801 811fa940 80a0fb80 00000cc0
dcc0: 00000001 00000001 00000000 f8dddd28 811fa93c 811fa680 00000002 802b0c30
dce0: ee1d2140 00440dc0 00000000 00000000 00000001 00000000 811fad80 00440dc0
dd00: 00000001 00000000 00000000 83201100 00440dc0 00000000 00000000 802afa18
dd20: 00000bdc 00000bd8 811fad80 00000000 811fad80 00000000 00000000 00000000
dd40: adf00ec5 816d5fa8 816d5fa8 821c3c00 00000000 7ebc2000 7ebe3000 00000000
dd60: f8ddde44 8028cfa0 816d5fa8 8288df50 8169cf00 821c3c00 7ebc2000 7ebe3000
dd80: f8ddde44 8028e1e8 00000001 80bbe932 f8ddde44 f8dddde8 f8ddde44 8288df50
dda0: 83201100 809cf474 f8ddde44 8169cf00 00000020 8169cf94 821c3c94 7ebe3000
ddc0: 7ebe2fff fffffff4 00000000 816d5fa8 821c3c00 8169cf00 81d65fa8 809cdbb0
dde0: 8288df50 81ffa910 7ebe3000 82890b00 00000000 00000000 00000000 00000000
de00: 00000000 00000000 adf00ec5 00000000 00000000 8169cf00 8288df50 83201100
de20: 821c3c00 81ffa910 f8ddde44 8011f6e4 00000000 00000000 821c3c7c 8169cf7c
de40: 83200880 821c3c40 7ebc2000 7ebe2fff 82890b0c 76ea4000 ffffffff 00000000
de60: 00000000 0f000a01 f8ddde68 f8ddde68 adf00ec5 00000000 00000000 00000000
de80: 00000000 01200000 83201100 00000000 00000000 8011e2ac 00000000 83200d7c
dea0: 83201020 83200e70 83200e80 00000000 f8dddf30 83200880 00000000 00000000
dec0: 00000000 00000000 00000000 82182f38 adf00ec5 00000000 01200000 f8dddf30
dee0: 00000000 00000000 00000000 00000078 000cb21c 8011fe24 00000020 00000000
df00: 828a5d80 80a10140 adf00ec5 01200011 00000000 00000000 00000000 f8dddf30
df20: 00000000 00000078 000cb21c 801203d4 01200000 00000000 00000000 76f46468
df40: 00000000 00000000 00000011 00000000 00000000 00000000 00000000 00000000
df60: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
df80: 00000003 adf00ec5 76f46468 00000000 76f468c0 00000078 80100290 83201100
dfa0: 000cb21c 80100060 76f46468 00000000 01200011 00000000 00000000 00000000
dfc0: 76f46468 00000000 76f468c0 00000078 00000000 76e81000 76f46400 000cb21c
dfe0: 00000078 7ebe2938 76e0854d 76dae7e6 20000030 01200011 00000000 00000000
Call trace:
  mmioset from prep_new_page+0x160/0x194
  prep_new_page from get_page_from_freelist+0x10f4/0x110c
  get_page_from_freelist from __alloc_pages+0x15c/0x280
  __alloc_pages from __pte_alloc+0x28/0x1bc
  __pte_alloc from copy_page_range+0xc10/0xd28
  copy_page_range from copy_mm+0x5cc/0x850
  copy_mm from copy_process+0x508/0xd8c
  copy_process from kernel_clone+0x94/0x338
  kernel_clone from sys_clone+0x94/0xb0
  sys_clone from ret_fast_syscall+0x0/0x1c
Exception stack(0xf8dddfa8 to 0xf8dddff0)
dfa0:                   76f46468 00000000 01200011 00000000 00000000 00000000
dfc0: 76f46468 00000000 76f468c0 00000078 00000000 76e81000 76f46400 000cb21c
dfe0: 00000078 7ebe2938 76e0854d 76dae7e6
Code: e92d4100 e1a08001 e1a0e003 e2522040 (a8ac410a)
---[ end trace 0000000000000000 ]---

I bisected down to this commit and reverting the commit solves the issue.

What is special about the Versatile Express graphics is that it uses a special
dedicated video RAM, looks like this in the device tree


         reserved-memory {
                 #address-cells = <1>;
                 #size-cells = <1>;
                 ranges;

                 /* Chipselect 3 is physically at 0x4c000000 */
                 vram: vram@4c000000 {
                         /* 8 MB of designated video RAM */
                         compatible = "shared-dma-pool";
                         reg = <0x4c000000 0x00800000>;
                         no-map;
                 };
         };
(...)
          clcd@1f000 {
                       compatible = "arm,pl111", "arm,primecell";
(...)
                       memory-region = <&vram>;

This gets picked up in the driver
drivers/gpu/drm/pl111/pl111_drv.c
like this:

         ret = of_reserved_mem_device_init(dev);
         if (!ret) {
                 dev_info(dev, "using device-specific reserved memory\n");
                 priv->use_device_memory = true;
         }

Yours,
Linus Walleij

--
--
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)
From 64de70dc23a66de8b7fd3e341136ac274da01e65 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@xxxxxxx>
Date: Wed, 4 Sep 2024 09:29:41 +0200
Subject: [PATCH] drm/fbdev-dma: Only install deferred I/O if necessary

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.

A reported bug affected the pl111 drivers, which has video memory in
either Normal or HighMem zones

[    0.000000] Zone ranges:
[    0.000000]   Normal   [mem 0x0000000010000000-0x000000003fffffff]
[    0.000000]   HighMem  [mem 0x0000000040000000-0x000000004fffffff]

where deferred I/O only works correctly with HighMem. See the Closes
tags for bug reports.

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Fixes: 808a40b69468 ("drm/fbdev-dma: Implement damage handling and deferred I/O")
Reported-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>
Closes: https://lore.kernel.org/all/23636953.6Emhk5qWAg@steina-w/
Reported-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
Closes: https://lore.kernel.org/dri-devel/CACRpkdb+hb9AGavbWpY-=uQQ0apY9en_tWJioPKf_fAbXMP4Hg@xxxxxxxxxxxxxx/
Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
Cc: Javier Martinez Canillas <javierm@xxxxxxxxxx>
Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
Cc: Maxime Ripard <mripard@xxxxxxxxxx>
---
 drivers/gpu/drm/drm_fbdev_dma.c | 71 ++++++++++++++++++++++++---------
 1 file changed, 52 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
index 7ef5a48c8029..455dc48d17a7 100644
--- a/drivers/gpu/drm/drm_fbdev_dma.c
+++ b/drivers/gpu/drm/drm_fbdev_dma.c
@@ -36,20 +36,11 @@ static int drm_fbdev_dma_fb_release(struct fb_info *info, int user)
 	return 0;
 }
 
-FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(drm_fbdev_dma,
-				   drm_fb_helper_damage_range,
-				   drm_fb_helper_damage_area);
-
 static int drm_fbdev_dma_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	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);
 
-	if (!dma->map_noncoherent)
-		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
-
-	return fb_deferred_io_mmap(info, vma);
+	return drm_gem_prime_mmap(fb_helper->buffer->gem, vma);
 }
 
 static void drm_fbdev_dma_fb_destroy(struct fb_info *info)
@@ -70,13 +61,40 @@ static void drm_fbdev_dma_fb_destroy(struct fb_info *info)
 }
 
 static const struct fb_ops drm_fbdev_dma_fb_ops = {
+	.owner = THIS_MODULE,
+	.fb_open = drm_fbdev_dma_fb_open,
+	.fb_release = drm_fbdev_dma_fb_release,
+	__FB_DEFAULT_DMAMEM_OPS_RDWR,
+	DRM_FB_HELPER_DEFAULT_OPS,
+	__FB_DEFAULT_DMAMEM_OPS_DRAW,
+	.fb_mmap = drm_fbdev_dma_fb_mmap,
+	.fb_destroy = drm_fbdev_dma_fb_destroy,
+};
+
+FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(drm_fbdev_dma,
+				   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)
+{
+	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);
+
+	if (!dma->map_noncoherent)
+		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+
+	return fb_deferred_io_mmap(info, vma);
+}
+
+static const struct fb_ops drm_fbdev_dma_deferred_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),
 	DRM_FB_HELPER_DEFAULT_OPS,
 	__FB_DEFAULT_DEFERRED_OPS_DRAW(drm_fbdev_dma),
-	.fb_mmap = drm_fbdev_dma_fb_mmap,
+	.fb_mmap = drm_fbdev_dma_deferred_fb_mmap,
 	.fb_destroy = drm_fbdev_dma_fb_destroy,
 };
 
@@ -89,6 +107,7 @@ static int drm_fbdev_dma_helper_fb_probe(struct drm_fb_helper *fb_helper,
 {
 	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;
@@ -111,6 +130,15 @@ static int drm_fbdev_dma_helper_fb_probe(struct drm_fb_helper *fb_helper,
 
 	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;
@@ -130,7 +158,10 @@ static int drm_fbdev_dma_helper_fb_probe(struct drm_fb_helper *fb_helper,
 
 	drm_fb_helper_fill_info(info, fb_helper, sizes);
 
-	info->fbops = &drm_fbdev_dma_fb_ops;
+	if (use_deferred_io)
+		info->fbops = &drm_fbdev_dma_deferred_fb_ops;
+	else
+		info->fbops = &drm_fbdev_dma_fb_ops;
 
 	/* screen */
 	info->flags |= FBINFO_VIRTFB; /* system memory */
@@ -145,13 +176,15 @@ static int drm_fbdev_dma_helper_fb_probe(struct drm_fb_helper *fb_helper,
 	info->fix.smem_len = info->screen_size;
 
 	/* deferred I/O */
-	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;
+	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;
+	}
 
 	return 0;
 
-- 
2.46.0


[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