[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKGbVbtkN+asaDWUs4WKqtjXg6iX40Dgk4cy50ysJ5TrnPGY5A@mail.gmail.com>
Date: Mon, 18 Dec 2023 10:50:24 +0800
From: Qiang Yu <yuq825@...il.com>
To: Alban Browaeys <alban.browaeys@...il.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
dri-devel@...ts.freedesktop.org, lima@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] RFC: drm/lima: fix calling drm_mm_init with an empty range
Thanks for the fix. It could be done in a simpler way that swap the
va_start/va_end init/fini and empty_vm create/release.
On Thu, Dec 14, 2023 at 5:04 PM Alban Browaeys <alban.browaeys@...il.com> wrote:
>
> For the empty_vm initialization the range is empty which is not supported
> by drm_mm_init.
>
> With CONFIG_DRM_DEBUG_MM set, I get:
> ------------[ cut here ]------------
> kernel BUG at drivers/gpu/drm/drm_mm.c:965!
> Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
> Modules linked in: lima(+) drm_shmem_helper gpu_sched s5p_jpeg s5p_g2d
> videobuf2_dma_contig videobuf2_memops v4l2_mem2mem videobuf2_v4l2
> videobuf2_common s5p_cec tun fuse configfs auth_rpcgss sunrpc ip_tables
> x_tables autofs4 btrfs lzo_compress zlib_deflate raid10 raid456
> async_raid6_recov async_memcpy async_pq async_xor async_tx xor xor_neon
> raid6_pq libcrc32c raid1 raid0 linear md_mod dm_mirror dm_region_hash
> dm_log hid_logitech_hidpp hid_logitech_dj
> CPU: 0 PID: 1033 Comm: systemd-udevd Not tainted 6.4.0-rc1-debug+ #230
> Hardware name: Samsung Exynos (Flattened Device Tree)
> PC is at drm_mm_init+0x94/0x98
> LR is at 0x0
> Flags: nZCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment none
> drm_mm_init from lima_vm_create+0xcc/0x108 [lima]
> lima_vm_create [lima] from lima_device_init+0xd8/0x4a0 [lima]
> lima_device_init [lima] from lima_pdev_probe.part.0+0x6c/0x158 [lima]
> lima_pdev_probe.part.0 [lima] from platform_probe+0x64/0xc0
> platform_probe from call_driver_probe+0x2c/0x110
>
> The drm_mm.c line 965 is:
> drivers/gpu/drm/drm_mm.c
> void drm_mm_init(struct drm_mm *mm, u64 start, u64 size)
> {
> DRM_MM_BUG_ON(start + size <= start);
>
> lima_vm_create is called with va_start and va_end both unset
> in lima_device_init line 371:
> ldev->empty_vm = lima_vm_create(ldev);
>
> Signed-off-by: Alban Browaeys <alban.browaeys@...il.com>
> ---
> drivers/gpu/drm/lima/lima_device.c | 2 +-
> drivers/gpu/drm/lima/lima_drv.c | 2 +-
> drivers/gpu/drm/lima/lima_vm.c | 10 +++++++---
> drivers/gpu/drm/lima/lima_vm.h | 3 ++-
> 4 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/lima/lima_device.c b/drivers/gpu/drm/lima/lima_device.c
> index 02cef0cea657..bd3afff0f44a 100644
> --- a/drivers/gpu/drm/lima/lima_device.c
> +++ b/drivers/gpu/drm/lima/lima_device.c
> @@ -368,7 +368,7 @@ int lima_device_init(struct lima_device *ldev)
> if (err)
> goto err_out0;
>
> - ldev->empty_vm = lima_vm_create(ldev);
> + ldev->empty_vm = lima_vm_create(ldev, false);
> if (!ldev->empty_vm) {
> err = -ENOMEM;
> goto err_out1;
> diff --git a/drivers/gpu/drm/lima/lima_drv.c b/drivers/gpu/drm/lima/lima_drv.c
> index 10fd9154cc46..ca09142e0ac1 100644
> --- a/drivers/gpu/drm/lima/lima_drv.c
> +++ b/drivers/gpu/drm/lima/lima_drv.c
> @@ -219,7 +219,7 @@ static int lima_drm_driver_open(struct drm_device *dev, struct drm_file *file)
> if (!priv)
> return -ENOMEM;
>
> - priv->vm = lima_vm_create(ldev);
> + priv->vm = lima_vm_create(ldev, true);
> if (!priv->vm) {
> err = -ENOMEM;
> goto err_out0;
> diff --git a/drivers/gpu/drm/lima/lima_vm.c b/drivers/gpu/drm/lima/lima_vm.c
> index 2b2739adc7f5..7f9775eefd78 100644
> --- a/drivers/gpu/drm/lima/lima_vm.c
> +++ b/drivers/gpu/drm/lima/lima_vm.c
> @@ -197,7 +197,7 @@ u32 lima_vm_get_va(struct lima_vm *vm, struct lima_bo *bo)
> return ret;
> }
>
> -struct lima_vm *lima_vm_create(struct lima_device *dev)
> +struct lima_vm *lima_vm_create(struct lima_device *dev, bool has_drm_mm)
> {
> struct lima_vm *vm;
>
> @@ -221,7 +221,10 @@ struct lima_vm *lima_vm_create(struct lima_device *dev)
> goto err_out1;
> }
>
> - drm_mm_init(&vm->mm, dev->va_start, dev->va_end - dev->va_start);
> + if (has_drm_mm) {
> + vm->has_drm_mm = true;
> + drm_mm_init(&vm->mm, dev->va_start, dev->va_end - dev->va_start);
> + }
>
> return vm;
>
> @@ -237,7 +240,8 @@ void lima_vm_release(struct kref *kref)
> struct lima_vm *vm = container_of(kref, struct lima_vm, refcount);
> int i;
>
> - drm_mm_takedown(&vm->mm);
> + if (vm->has_drm_mm)
> + drm_mm_takedown(&vm->mm);
>
> for (i = 0; i < LIMA_VM_NUM_BT; i++) {
> if (vm->bts[i].cpu)
> diff --git a/drivers/gpu/drm/lima/lima_vm.h b/drivers/gpu/drm/lima/lima_vm.h
> index 3a7c74822d8b..e7443f410d6d 100644
> --- a/drivers/gpu/drm/lima/lima_vm.h
> +++ b/drivers/gpu/drm/lima/lima_vm.h
> @@ -30,6 +30,7 @@ struct lima_vm {
> struct mutex lock;
> struct kref refcount;
>
> + bool has_drm_mm;
> struct drm_mm mm;
>
> struct lima_device *dev;
> @@ -43,7 +44,7 @@ void lima_vm_bo_del(struct lima_vm *vm, struct lima_bo *bo);
>
> u32 lima_vm_get_va(struct lima_vm *vm, struct lima_bo *bo);
>
> -struct lima_vm *lima_vm_create(struct lima_device *dev);
> +struct lima_vm *lima_vm_create(struct lima_device *dev, bool has_drm_mm);
> void lima_vm_release(struct kref *kref);
>
> static inline struct lima_vm *lima_vm_get(struct lima_vm *vm)
> --
> 2.39.2
>
Powered by blists - more mailing lists