Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

15 participants
3025 discussions

Patch "drm/gem: Acquire references on GEM handles for framebuffers" has been added to the 6.12-stable tree

by gregkh＠linuxfoundation.org

This is a note to let you know that I've just added the patch titled drm/gem: Acquire references on GEM handles for framebuffers to the 6.12-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… The filename of the patch is: drm-gem-acquire-references-on-gem-handles-for-framebuffers.patch and it can be found in the queue-6.12 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable(a)vger.kernel.org> know about it. From 5307dce878d4126e1b375587318955bd019c3741 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann <tzimmermann(a)suse.de> Date: Mon, 30 Jun 2025 10:36:47 +0200 Subject: drm/gem: Acquire references on GEM handles for framebuffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Thomas Zimmermann <tzimmermann(a)suse.de> commit 5307dce878d4126e1b375587318955bd019c3741 upstream. A GEM handle can be released while the GEM buffer object is attached to a DRM framebuffer. This leads to the release of the dma-buf backing the buffer object, if any. [1] Trying to use the framebuffer in further mode-setting operations leads to a segmentation fault. Most easily happens with driver that use shadow planes for vmap-ing the dma-buf during a page flip. An example is shown below. [ 156.791968] ------------[ cut here ]------------ [ 156.796830] WARNING: CPU: 2 PID: 2255 at drivers/dma-buf/dma-buf.c:1527 dma_buf_vmap+0x224/0x430 [...] [ 156.942028] RIP: 0010:dma_buf_vmap+0x224/0x430 [ 157.043420] Call Trace: [ 157.045898] <TASK> [ 157.048030] ? show_trace_log_lvl+0x1af/0x2c0 [ 157.052436] ? show_trace_log_lvl+0x1af/0x2c0 [ 157.056836] ? show_trace_log_lvl+0x1af/0x2c0 [ 157.061253] ? drm_gem_shmem_vmap+0x74/0x710 [ 157.065567] ? dma_buf_vmap+0x224/0x430 [ 157.069446] ? __warn.cold+0x58/0xe4 [ 157.073061] ? dma_buf_vmap+0x224/0x430 [ 157.077111] ? report_bug+0x1dd/0x390 [ 157.080842] ? handle_bug+0x5e/0xa0 [ 157.084389] ? exc_invalid_op+0x14/0x50 [ 157.088291] ? asm_exc_invalid_op+0x16/0x20 [ 157.092548] ? dma_buf_vmap+0x224/0x430 [ 157.096663] ? dma_resv_get_singleton+0x6d/0x230 [ 157.101341] ? __pfx_dma_buf_vmap+0x10/0x10 [ 157.105588] ? __pfx_dma_resv_get_singleton+0x10/0x10 [ 157.110697] drm_gem_shmem_vmap+0x74/0x710 [ 157.114866] drm_gem_vmap+0xa9/0x1b0 [ 157.118763] drm_gem_vmap_unlocked+0x46/0xa0 [ 157.123086] drm_gem_fb_vmap+0xab/0x300 [ 157.126979] drm_atomic_helper_prepare_planes.part.0+0x487/0xb10 [ 157.133032] ? lockdep_init_map_type+0x19d/0x880 [ 157.137701] drm_atomic_helper_commit+0x13d/0x2e0 [ 157.142671] ? drm_atomic_nonblocking_commit+0xa0/0x180 [ 157.147988] drm_mode_atomic_ioctl+0x766/0xe40 [...] [ 157.346424] ---[ end trace 0000000000000000 ]--- Acquiring GEM handles for the framebuffer's GEM buffer objects prevents this from happening. The framebuffer's cleanup later puts the handle references. Commit 1a148af06000 ("drm/gem-shmem: Use dma_buf from GEM object instance") triggers the segmentation fault easily by using the dma-buf field more widely. The underlying issue with reference counting has been present before. v2: - acquire the handle instead of the BO (Christian) - fix comment style (Christian) - drop the Fixes tag (Christian) - rename err_ gotos - add missing Link tag Suggested-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Link: https://elixir.bootlin.com/linux/v6.15/source/drivers/gpu/drm/drm_gem.c#L241 # [1] Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: <stable(a)vger.kernel.org> Reviewed-by: Christian König <christian.koenig(a)amd.com> Link: https://lore.kernel.org/r/20250630084001.293053-1-tzimmermann@suse.de Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/gpu/drm/drm_gem.c | 44 ++++++++++++++++++++++++--- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 +++++---- drivers/gpu/drm/drm_internal.h | 2 + 3 files changed, 51 insertions(+), 11 deletions(-) --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -186,6 +186,35 @@ void drm_gem_private_object_fini(struct } EXPORT_SYMBOL(drm_gem_private_object_fini); +static void drm_gem_object_handle_get(struct drm_gem_object *obj) +{ + struct drm_device *dev = obj->dev; + + drm_WARN_ON(dev, !mutex_is_locked(&dev->object_name_lock)); + + if (obj->handle_count++ == 0) + drm_gem_object_get(obj); +} + +/** + * drm_gem_object_handle_get_unlocked - acquire reference on user-space handles + * @obj: GEM object + * + * Acquires a reference on the GEM buffer object's handle. Required + * to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked() + * to release the reference. + */ +void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj) +{ + struct drm_device *dev = obj->dev; + + guard(mutex)(&dev->object_name_lock); + + drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in create-tail helper */ + drm_gem_object_handle_get(obj); +} +EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked); + /** * drm_gem_object_handle_free - release resources bound to userspace handles * @obj: GEM object to clean up. @@ -216,8 +245,14 @@ static void drm_gem_object_exported_dma_ } } -static void -drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) +/** + * drm_gem_object_handle_put_unlocked - releases reference on user-space handles + * @obj: GEM object + * + * Releases a reference on the GEM buffer object's handle. Possibly releases + * the GEM buffer object and associated dma-buf objects. + */ +void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; bool final = false; @@ -242,6 +277,7 @@ drm_gem_object_handle_put_unlocked(struc if (final) drm_gem_object_put(obj); } +EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked); /* * Called at device or object close to release the file's @@ -363,8 +399,8 @@ drm_gem_handle_create_tail(struct drm_fi int ret; WARN_ON(!mutex_is_locked(&dev->object_name_lock)); - if (obj->handle_count++ == 0) - drm_gem_object_get(obj); + + drm_gem_object_handle_get(obj); /* * Get the user-visible handle using idr. Preload and perform --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -99,7 +99,7 @@ void drm_gem_fb_destroy(struct drm_frame unsigned int i; for (i = 0; i < fb->format->num_planes; i++) - drm_gem_object_put(fb->obj[i]); + drm_gem_object_handle_put_unlocked(fb->obj[i]); drm_framebuffer_cleanup(fb); kfree(fb); @@ -182,8 +182,10 @@ int drm_gem_fb_init_with_funcs(struct dr if (!objs[i]) { drm_dbg_kms(dev, "Failed to lookup GEM object\n"); ret = -ENOENT; - goto err_gem_object_put; + goto err_gem_object_handle_put_unlocked; } + drm_gem_object_handle_get_unlocked(objs[i]); + drm_gem_object_put(objs[i]); min_size = (height - 1) * mode_cmd->pitches[i] + drm_format_info_min_pitch(info, i, width) @@ -193,22 +195,22 @@ int drm_gem_fb_init_with_funcs(struct dr drm_dbg_kms(dev, "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n", objs[i]->size, min_size, i); - drm_gem_object_put(objs[i]); + drm_gem_object_handle_put_unlocked(objs[i]); ret = -EINVAL; - goto err_gem_object_put; + goto err_gem_object_handle_put_unlocked; } } ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs); if (ret) - goto err_gem_object_put; + goto err_gem_object_handle_put_unlocked; return 0; -err_gem_object_put: +err_gem_object_handle_put_unlocked: while (i > 0) { --i; - drm_gem_object_put(objs[i]); + drm_gem_object_handle_put_unlocked(objs[i]); } return ret; } --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -153,6 +153,8 @@ void drm_sysfs_lease_event(struct drm_de /* drm_gem.c */ int drm_gem_init(struct drm_device *dev); +void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj); +void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj); int drm_gem_handle_create_tail(struct drm_file *file_priv, struct drm_gem_object *obj, u32 *handlep); Patches currently in stable-queue which might be from tzimmermann(a)suse.de are queue-6.12/drm-gem-fix-race-in-drm_gem_handle_create_tail.patch queue-6.12/drm-gem-acquire-references-on-gem-handles-for-framebuffers.patch

2 months

Patch "drm/gem: Acquire references on GEM handles for framebuffers" has been added to the 6.6-stable tree

by gregkh＠linuxfoundation.org

This is a note to let you know that I've just added the patch titled drm/gem: Acquire references on GEM handles for framebuffers to the 6.6-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… The filename of the patch is: drm-gem-acquire-references-on-gem-handles-for-framebuffers.patch and it can be found in the queue-6.6 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable(a)vger.kernel.org> know about it. From 5307dce878d4126e1b375587318955bd019c3741 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann <tzimmermann(a)suse.de> Date: Mon, 30 Jun 2025 10:36:47 +0200 Subject: drm/gem: Acquire references on GEM handles for framebuffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Thomas Zimmermann <tzimmermann(a)suse.de> commit 5307dce878d4126e1b375587318955bd019c3741 upstream. A GEM handle can be released while the GEM buffer object is attached to a DRM framebuffer. This leads to the release of the dma-buf backing the buffer object, if any. [1] Trying to use the framebuffer in further mode-setting operations leads to a segmentation fault. Most easily happens with driver that use shadow planes for vmap-ing the dma-buf during a page flip. An example is shown below. [ 156.791968] ------------[ cut here ]------------ [ 156.796830] WARNING: CPU: 2 PID: 2255 at drivers/dma-buf/dma-buf.c:1527 dma_buf_vmap+0x224/0x430 [...] [ 156.942028] RIP: 0010:dma_buf_vmap+0x224/0x430 [ 157.043420] Call Trace: [ 157.045898] <TASK> [ 157.048030] ? show_trace_log_lvl+0x1af/0x2c0 [ 157.052436] ? show_trace_log_lvl+0x1af/0x2c0 [ 157.056836] ? show_trace_log_lvl+0x1af/0x2c0 [ 157.061253] ? drm_gem_shmem_vmap+0x74/0x710 [ 157.065567] ? dma_buf_vmap+0x224/0x430 [ 157.069446] ? __warn.cold+0x58/0xe4 [ 157.073061] ? dma_buf_vmap+0x224/0x430 [ 157.077111] ? report_bug+0x1dd/0x390 [ 157.080842] ? handle_bug+0x5e/0xa0 [ 157.084389] ? exc_invalid_op+0x14/0x50 [ 157.088291] ? asm_exc_invalid_op+0x16/0x20 [ 157.092548] ? dma_buf_vmap+0x224/0x430 [ 157.096663] ? dma_resv_get_singleton+0x6d/0x230 [ 157.101341] ? __pfx_dma_buf_vmap+0x10/0x10 [ 157.105588] ? __pfx_dma_resv_get_singleton+0x10/0x10 [ 157.110697] drm_gem_shmem_vmap+0x74/0x710 [ 157.114866] drm_gem_vmap+0xa9/0x1b0 [ 157.118763] drm_gem_vmap_unlocked+0x46/0xa0 [ 157.123086] drm_gem_fb_vmap+0xab/0x300 [ 157.126979] drm_atomic_helper_prepare_planes.part.0+0x487/0xb10 [ 157.133032] ? lockdep_init_map_type+0x19d/0x880 [ 157.137701] drm_atomic_helper_commit+0x13d/0x2e0 [ 157.142671] ? drm_atomic_nonblocking_commit+0xa0/0x180 [ 157.147988] drm_mode_atomic_ioctl+0x766/0xe40 [...] [ 157.346424] ---[ end trace 0000000000000000 ]--- Acquiring GEM handles for the framebuffer's GEM buffer objects prevents this from happening. The framebuffer's cleanup later puts the handle references. Commit 1a148af06000 ("drm/gem-shmem: Use dma_buf from GEM object instance") triggers the segmentation fault easily by using the dma-buf field more widely. The underlying issue with reference counting has been present before. v2: - acquire the handle instead of the BO (Christian) - fix comment style (Christian) - drop the Fixes tag (Christian) - rename err_ gotos - add missing Link tag Suggested-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Link: https://elixir.bootlin.com/linux/v6.15/source/drivers/gpu/drm/drm_gem.c#L241 # [1] Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: <stable(a)vger.kernel.org> Reviewed-by: Christian König <christian.koenig(a)amd.com> Link: https://lore.kernel.org/r/20250630084001.293053-1-tzimmermann@suse.de Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/gpu/drm/drm_gem.c | 44 ++++++++++++++++++++++++--- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 +++++---- drivers/gpu/drm/drm_internal.h | 2 + 3 files changed, 51 insertions(+), 11 deletions(-) --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -186,6 +186,35 @@ void drm_gem_private_object_fini(struct } EXPORT_SYMBOL(drm_gem_private_object_fini); +static void drm_gem_object_handle_get(struct drm_gem_object *obj) +{ + struct drm_device *dev = obj->dev; + + drm_WARN_ON(dev, !mutex_is_locked(&dev->object_name_lock)); + + if (obj->handle_count++ == 0) + drm_gem_object_get(obj); +} + +/** + * drm_gem_object_handle_get_unlocked - acquire reference on user-space handles + * @obj: GEM object + * + * Acquires a reference on the GEM buffer object's handle. Required + * to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked() + * to release the reference. + */ +void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj) +{ + struct drm_device *dev = obj->dev; + + guard(mutex)(&dev->object_name_lock); + + drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in create-tail helper */ + drm_gem_object_handle_get(obj); +} +EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked); + /** * drm_gem_object_handle_free - release resources bound to userspace handles * @obj: GEM object to clean up. @@ -216,8 +245,14 @@ static void drm_gem_object_exported_dma_ } } -static void -drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) +/** + * drm_gem_object_handle_put_unlocked - releases reference on user-space handles + * @obj: GEM object + * + * Releases a reference on the GEM buffer object's handle. Possibly releases + * the GEM buffer object and associated dma-buf objects. + */ +void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; bool final = false; @@ -242,6 +277,7 @@ drm_gem_object_handle_put_unlocked(struc if (final) drm_gem_object_put(obj); } +EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked); /* * Called at device or object close to release the file's @@ -363,8 +399,8 @@ drm_gem_handle_create_tail(struct drm_fi int ret; WARN_ON(!mutex_is_locked(&dev->object_name_lock)); - if (obj->handle_count++ == 0) - drm_gem_object_get(obj); + + drm_gem_object_handle_get(obj); /* * Get the user-visible handle using idr. Preload and perform --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c @@ -99,7 +99,7 @@ void drm_gem_fb_destroy(struct drm_frame unsigned int i; for (i = 0; i < fb->format->num_planes; i++) - drm_gem_object_put(fb->obj[i]); + drm_gem_object_handle_put_unlocked(fb->obj[i]); drm_framebuffer_cleanup(fb); kfree(fb); @@ -182,8 +182,10 @@ int drm_gem_fb_init_with_funcs(struct dr if (!objs[i]) { drm_dbg_kms(dev, "Failed to lookup GEM object\n"); ret = -ENOENT; - goto err_gem_object_put; + goto err_gem_object_handle_put_unlocked; } + drm_gem_object_handle_get_unlocked(objs[i]); + drm_gem_object_put(objs[i]); min_size = (height - 1) * mode_cmd->pitches[i] + drm_format_info_min_pitch(info, i, width) @@ -193,22 +195,22 @@ int drm_gem_fb_init_with_funcs(struct dr drm_dbg_kms(dev, "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n", objs[i]->size, min_size, i); - drm_gem_object_put(objs[i]); + drm_gem_object_handle_put_unlocked(objs[i]); ret = -EINVAL; - goto err_gem_object_put; + goto err_gem_object_handle_put_unlocked; } } ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs); if (ret) - goto err_gem_object_put; + goto err_gem_object_handle_put_unlocked; return 0; -err_gem_object_put: +err_gem_object_handle_put_unlocked: while (i > 0) { --i; - drm_gem_object_put(objs[i]); + drm_gem_object_handle_put_unlocked(objs[i]); } return ret; } --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -155,6 +155,8 @@ void drm_sysfs_lease_event(struct drm_de /* drm_gem.c */ int drm_gem_init(struct drm_device *dev); +void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj); +void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj); int drm_gem_handle_create_tail(struct drm_file *file_priv, struct drm_gem_object *obj, u32 *handlep); Patches currently in stable-queue which might be from tzimmermann(a)suse.de are queue-6.6/drm-gem-fix-race-in-drm_gem_handle_create_tail.patch queue-6.6/drm-gem-acquire-references-on-gem-handles-for-framebuffers.patch

2 months

Re: [RFC PATCH 00/30] Host side (KVM/VFIO/IOMMUFD) support for TDISP using TSM

by dan.j.williams＠intel.com

Xu Yilun wrote: > On Sat, Jun 21, 2025 at 11:07:24AM +1000, Alexey Kardashevskiy wrote: > > > > > > On 11/6/25 11:55, Alexey Kardashevskiy wrote: > > > Hi, > > > > > > Is there a QEMU tree using this somewhere? > > > > Ping? Thanks, > > Sorry for late. I've finally got a public tree. > > https://github.com/yiliu1765/qemu/tree/zhenzhong/devsec_tsm > > Again, I think the changes are far from good, just work for enabling. At some point I want to stage a merge tree QEMU bits here: https://git.kernel.org/pub/scm/linux/kernel/git/devsec/qemu.git/ (not created yet) ...unless Paolo or others in QEMU community are open to running a staging branch in qemu.git. At some point we need to collide all the QEMU POC branches, and I expect that needs to happen and show some success before the upstream projects start ingesting all these changes.

2 months

Re: [PATCH v7 03/10] accel/rocket: Add IOCTL for BO creation

by Andrew Davis

On 6/6/25 1:28 AM, Tomeu Vizoso wrote: > This uses the SHMEM DRM helpers and we map right away to the CPU and NPU > sides, as all buffers are expected to be accessed from both. > > v2: > - Sync the IOMMUs for the other cores when mapping and unmapping. > > v3: > - Make use of GPL-2.0-only for the copyright notice (Jeff Hugo) > > v6: > - Use mutexes guard (Markus Elfring) > > v7: > - Assign its own IOMMU domain to each client, for isolation (Daniel > Stone and Robin Murphy) > > Reviewed-by: Jeffrey Hugo <quic_jhugo(a)quicinc.com> > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > --- > drivers/accel/rocket/Makefile | 3 +- > drivers/accel/rocket/rocket_device.c | 4 ++ > drivers/accel/rocket/rocket_device.h | 2 + > drivers/accel/rocket/rocket_drv.c | 7 ++- > drivers/accel/rocket/rocket_gem.c | 115 +++++++++++++++++++++++++++++++++++ > drivers/accel/rocket/rocket_gem.h | 27 ++++++++ > include/uapi/drm/rocket_accel.h | 44 ++++++++++++++ > 7 files changed, 200 insertions(+), 2 deletions(-) > > diff --git a/drivers/accel/rocket/Makefile b/drivers/accel/rocket/Makefile > index abdd75f2492eaecf8bf5e78a2ac150ea19ac3e96..4deef267f9e1238c4d8bd108dcc8afd9dc8b2b8f 100644 > --- a/drivers/accel/rocket/Makefile > +++ b/drivers/accel/rocket/Makefile > @@ -5,4 +5,5 @@ obj-$(CONFIG_DRM_ACCEL_ROCKET) := rocket.o > rocket-y := \ > rocket_core.o \ > rocket_device.o \ > - rocket_drv.o > + rocket_drv.o \ > + rocket_gem.o > diff --git a/drivers/accel/rocket/rocket_device.c b/drivers/accel/rocket/rocket_device.c > index a05c103e117e3eaa6439884b7acb6e3483296edb..5e559104741af22c528914c96e44558323ab6c89 100644 > --- a/drivers/accel/rocket/rocket_device.c > +++ b/drivers/accel/rocket/rocket_device.c > @@ -4,6 +4,7 @@ > #include <linux/array_size.h> > #include <linux/clk.h> > #include <linux/dev_printk.h> > +#include <linux/mutex.h> > > #include "rocket_device.h" > > @@ -16,10 +17,13 @@ int rocket_device_init(struct rocket_device *rdev) > if (err) > return err; > > + mutex_init(&rdev->iommu_lock); devm_mutex_init() again keeps you from needing rocket_device_fini(). Same in the next patch even if you don't end up needing the iommu_lock. Andrew

2 months

Re: [PATCH v7 02/10] accel/rocket: Add a new driver for Rockchip's NPU

by Andrew Davis

On 6/6/25 1:28 AM, Tomeu Vizoso wrote: > This initial version supports the NPU as shipped in the RK3588 SoC and > described in the first part of its TRM, in Chapter 36. > > This NPU contains 3 independent cores that the driver can submit jobs > to. > > This commit adds just hardware initialization and power management. > > v2: > - Split cores and IOMMUs as independent devices (Sebastian Reichel) > - Add some documentation (Jeffrey Hugo) > - Be more explicit in the Kconfig documentation (Jeffrey Hugo) > - Remove resets, as these haven't been found useful so far (Zenghui Yu) > - Repack structs (Jeffrey Hugo) > - Use DEFINE_DRM_ACCEL_FOPS (Jeffrey Hugo) > - Use devm_drm_dev_alloc (Jeffrey Hugo) > - Use probe log helper (Jeffrey Hugo) > - Introduce UABI header in a later patch (Jeffrey Hugo) > > v3: > - Adapt to a split of the register block in the DT bindings (Nicolas > Frattaroli) > - Move registers header to its own commit (Thomas Zimmermann) > - Misc. cleanups (Thomas Zimmermann and Jeff Hugo) > - Make use of GPL-2.0-only for the copyright notice (Jeff Hugo) > - PM improvements (Nicolas Frattaroli) > > v4: > - Use bulk clk API (Krzysztof Kozlowski) > > v6: > - Remove mention to NVDLA, as the hardware is only incidentally related > (Kever Yang) > - Use calloc instead of GFP_ZERO (Jeff Hugo) > - Explicitly include linux/container_of.h (Jeff Hugo) > - pclk and npu clocks are now needed by all cores (Rob Herring) > > v7: > - Assign its own IOMMU domain to each client, for isolation (Daniel > Stone and Robin Murphy) > > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > --- > Documentation/accel/index.rst | 1 + > Documentation/accel/rocket/index.rst | 19 +++ > MAINTAINERS | 10 ++ > drivers/accel/Kconfig | 1 + > drivers/accel/Makefile | 1 + > drivers/accel/rocket/Kconfig | 25 ++++ > drivers/accel/rocket/Makefile | 8 + > drivers/accel/rocket/rocket_core.c | 70 +++++++++ > drivers/accel/rocket/rocket_core.h | 45 ++++++ > drivers/accel/rocket/rocket_device.c | 25 ++++ > drivers/accel/rocket/rocket_device.h | 26 ++++ > drivers/accel/rocket/rocket_drv.c | 279 +++++++++++++++++++++++++++++++++++ > drivers/accel/rocket/rocket_drv.h | 15 ++ > 13 files changed, 525 insertions(+) > > diff --git a/Documentation/accel/index.rst b/Documentation/accel/index.rst > index bc85f26533d88891dde482f91e26c99991b22869..d8fa332d60a890dbb617454d2a26d9b6f9b196aa 100644 > --- a/Documentation/accel/index.rst > +++ b/Documentation/accel/index.rst > @@ -10,6 +10,7 @@ Compute Accelerators > introduction > amdxdna/index > qaic/index > + rocket/index > > .. only:: subproject and html > > diff --git a/Documentation/accel/rocket/index.rst b/Documentation/accel/rocket/index.rst > new file mode 100644 > index 0000000000000000000000000000000000000000..300eb3aeab1d8c6514c65af4d216b2d5a1669131 > --- /dev/null > +++ b/Documentation/accel/rocket/index.rst > @@ -0,0 +1,19 @@ > +.. SPDX-License-Identifier: GPL-2.0-only > + > +===================================== > + accel/rocket Rockchip NPU driver > +===================================== > + > +The accel/rocket driver supports the Neural Processing Units (NPUs) inside some > +Rockchip SoCs such as the RK3588. Rockchip calls it RKNN and sometimes RKNPU. > + > +The hardware is described in chapter 36 in the RK3588 TRM. > + > +This driver just powers the hardware on and off, allocates and maps buffers to > +the device and submits jobs to the frontend unit. Everything else is done in > +userspace, as a Gallium driver (also called rocket) that is part of the Mesa3D > +project. > + > +Hardware currently supported: > + > +* RK3588 > \ No newline at end of file > diff --git a/MAINTAINERS b/MAINTAINERS > index 96b82704950184bd71623ff41fc4df31e4c7fe87..2d8833bf1f2db06ca624d703f19066adab2f9fde 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7263,6 +7263,16 @@ T: git https://gitlab.freedesktop.org/drm/misc/kernel.git > F: drivers/accel/ivpu/ > F: include/uapi/drm/ivpu_accel.h > > +DRM ACCEL DRIVER FOR ROCKCHIP NPU > +M: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > +L: dri-devel(a)lists.freedesktop.org > +S: Supported > +T: git https://gitlab.freedesktop.org/drm/misc/kernel.git > +F: Documentation/accel/rocket/ > +F: Documentation/devicetree/bindings/npu/rockchip,rknn-core.yaml > +F: drivers/accel/rocket/ > +F: include/uapi/drm/rocket_accel.h > + > DRM COMPUTE ACCELERATORS DRIVERS AND FRAMEWORK > M: Oded Gabbay <ogabbay(a)kernel.org> > L: dri-devel(a)lists.freedesktop.org > diff --git a/drivers/accel/Kconfig b/drivers/accel/Kconfig > index 5b9490367a39fd12d35a8d9021768aa186c09308..bb01cebc42bf16ebf02e938040f339ff94869e33 100644 > --- a/drivers/accel/Kconfig > +++ b/drivers/accel/Kconfig > @@ -28,5 +28,6 @@ source "drivers/accel/amdxdna/Kconfig" > source "drivers/accel/habanalabs/Kconfig" > source "drivers/accel/ivpu/Kconfig" > source "drivers/accel/qaic/Kconfig" > +source "drivers/accel/rocket/Kconfig" > > endif > diff --git a/drivers/accel/Makefile b/drivers/accel/Makefile > index a301fb6089d4c515430175c5e2ba9190f6dc9158..ffc3fa58866616d933184a7659573cd4d4780a8d 100644 > --- a/drivers/accel/Makefile > +++ b/drivers/accel/Makefile > @@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_ACCEL_AMDXDNA) += amdxdna/ > obj-$(CONFIG_DRM_ACCEL_HABANALABS) += habanalabs/ > obj-$(CONFIG_DRM_ACCEL_IVPU) += ivpu/ > obj-$(CONFIG_DRM_ACCEL_QAIC) += qaic/ > +obj-$(CONFIG_DRM_ACCEL_ROCKET) += rocket/ > \ No newline at end of file Couple of these no newline warnings > diff --git a/drivers/accel/rocket/Kconfig b/drivers/accel/rocket/Kconfig > new file mode 100644 > index 0000000000000000000000000000000000000000..9a59c6c61bf4d6460d8008b16331f001c97de67d > --- /dev/null > +++ b/drivers/accel/rocket/Kconfig > @@ -0,0 +1,25 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +config DRM_ACCEL_ROCKET > + tristate "Rocket (support for Rockchip NPUs)" > + depends on DRM > + depends on ARM64 || COMPILE_TEST Should this be more specific for now ARCH_ROCKCHIP? > + depends on MMU > + select DRM_SCHED > + select IOMMU_SUPPORT > + select IOMMU_IO_PGTABLE_LPAE > + select DRM_GEM_SHMEM_HELPER > + help > + Choose this option if you have a Rockchip SoC that contains a > + compatible Neural Processing Unit (NPU), such as the RK3588. Called by > + Rockchip either RKNN or RKNPU, it accelerates inference of neural > + networks. > + > + The interface exposed to userspace is described in > + include/uapi/drm/rocket_accel.h and is used by the Rocket userspace > + driver in Mesa3D. > + > + If unsure, say N. > + > + To compile this driver as a module, choose M here: the > + module will be called rocket. > diff --git a/drivers/accel/rocket/Makefile b/drivers/accel/rocket/Makefile > new file mode 100644 > index 0000000000000000000000000000000000000000..abdd75f2492eaecf8bf5e78a2ac150ea19ac3e96 > --- /dev/null > +++ b/drivers/accel/rocket/Makefile > @@ -0,0 +1,8 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +obj-$(CONFIG_DRM_ACCEL_ROCKET) := rocket.o > + > +rocket-y := \ > + rocket_core.o \ > + rocket_device.o \ > + rocket_drv.o > diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c > new file mode 100644 > index 0000000000000000000000000000000000000000..3a6f25f2b4103075102739588bcdad96510e2a4e > --- /dev/null > +++ b/drivers/accel/rocket/rocket_core.c > @@ -0,0 +1,70 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright 2024-2025 Tomeu Vizoso <tomeu(a)tomeuvizoso.net> */ > + > +#include <linux/clk.h> > +#include <linux/dev_printk.h> > +#include <linux/err.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > + > +#include "rocket_core.h" > + > +int rocket_core_init(struct rocket_core *core) > +{ > + struct device *dev = core->dev; > + struct platform_device *pdev = to_platform_device(dev); > + u32 version; > + int err = 0; > + > + err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks); > + if (err) > + return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index); > + > + core->pc_iomem = devm_platform_ioremap_resource_byname(pdev, "pc"); > + if (IS_ERR(core->pc_iomem)) { > + dev_err(dev, "couldn't find PC registers %ld\n", PTR_ERR(core->pc_iomem)); > + return PTR_ERR(core->pc_iomem); > + } > + > + core->cna_iomem = devm_platform_ioremap_resource_byname(pdev, "cna"); > + if (IS_ERR(core->cna_iomem)) { > + dev_err(dev, "couldn't find CNA registers %ld\n", PTR_ERR(core->cna_iomem)); > + return PTR_ERR(core->cna_iomem); > + } > + > + core->core_iomem = devm_platform_ioremap_resource_byname(pdev, "core"); > + if (IS_ERR(core->core_iomem)) { > + dev_err(dev, "couldn't find CORE registers %ld\n", PTR_ERR(core->core_iomem)); > + return PTR_ERR(core->core_iomem); > + } > + > + pm_runtime_use_autosuspend(dev); > + > + /* > + * As this NPU will be most often used as part of a media pipeline that > + * ends presenting in a display, choose 50 ms (~3 frames at 60Hz) as an > + * autosuspend delay as that will keep the device powered up while the > + * pipeline is running. > + */ > + pm_runtime_set_autosuspend_delay(dev, 50); > + > + pm_runtime_enable(dev); devm_pm_runtime_enable(dev) here would take care of both functions in rocket_core_fini() so you wouldn't need that and can cleanup some return paths here. Andrew > + > + err = pm_runtime_get_sync(dev); > + > + version = rocket_pc_readl(core, VERSION); > + version += rocket_pc_readl(core, VERSION_NUM) & 0xffff; > + > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > + > + dev_info(dev, "Rockchip NPU core %d version: %d\n", core->index, version); > + > + return 0; > +} > + > +void rocket_core_fini(struct rocket_core *core) > +{ > + pm_runtime_dont_use_autosuspend(core->dev); > + pm_runtime_disable(core->dev); > +} > diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h > new file mode 100644 > index 0000000000000000000000000000000000000000..1b1beb9798f03ec2ca325496a4d894674d0b798d > --- /dev/null > +++ b/drivers/accel/rocket/rocket_core.h > @@ -0,0 +1,45 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* Copyright 2024-2025 Tomeu Vizoso <tomeu(a)tomeuvizoso.net> */ > + > +#ifndef __ROCKET_CORE_H__ > +#define __ROCKET_CORE_H__ > + > +#include <drm/gpu_scheduler.h> > +#include <linux/clk.h> > +#include <linux/io.h> > +#include <linux/mutex_types.h> > + > +#include "rocket_registers.h" > + > +#define rocket_pc_readl(core, reg) \ > + readl((core)->pc_iomem + (REG_PC_##reg)) > +#define rocket_pc_writel(core, reg, value) \ > + writel(value, (core)->pc_iomem + (REG_PC_##reg)) > + > +#define rocket_cna_readl(core, reg) \ > + readl((core)->cna_iomem + (REG_CNA_##reg) - REG_CNA_S_STATUS) > +#define rocket_cna_writel(core, reg, value) \ > + writel(value, (core)->cna_iomem + (REG_CNA_##reg) - REG_CNA_S_STATUS) > + > +#define rocket_core_readl(core, reg) \ > + readl((core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS) > +#define rocket_core_writel(core, reg, value) \ > + writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS) > + > +struct rocket_core { > + struct device *dev; > + struct rocket_device *rdev; > + struct device_link *link; > + unsigned int index; > + > + int irq; > + void __iomem *pc_iomem; > + void __iomem *cna_iomem; > + void __iomem *core_iomem; > + struct clk_bulk_data clks[4]; > +}; > + > +int rocket_core_init(struct rocket_core *core); > +void rocket_core_fini(struct rocket_core *core); > + > +#endif > diff --git a/drivers/accel/rocket/rocket_device.c b/drivers/accel/rocket/rocket_device.c > new file mode 100644 > index 0000000000000000000000000000000000000000..a05c103e117e3eaa6439884b7acb6e3483296edb > --- /dev/null > +++ b/drivers/accel/rocket/rocket_device.c > @@ -0,0 +1,25 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright 2024-2025 Tomeu Vizoso <tomeu(a)tomeuvizoso.net> */ > + > +#include <linux/array_size.h> > +#include <linux/clk.h> > +#include <linux/dev_printk.h> > + > +#include "rocket_device.h" > + > +int rocket_device_init(struct rocket_device *rdev) > +{ > + int err; > + > + /* Initialize core 0 (top) */ > + err = rocket_core_init(&rdev->cores[0]); > + if (err) > + return err; > + > + return 0; > +} > + > +void rocket_device_fini(struct rocket_device *rdev) > +{ > + rocket_core_fini(&rdev->cores[0]); > +} > diff --git a/drivers/accel/rocket/rocket_device.h b/drivers/accel/rocket/rocket_device.h > new file mode 100644 > index 0000000000000000000000000000000000000000..b5d5f1479d56e2fde59bbcad9de2b58cef9a9a4d > --- /dev/null > +++ b/drivers/accel/rocket/rocket_device.h > @@ -0,0 +1,26 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* Copyright 2024-2025 Tomeu Vizoso <tomeu(a)tomeuvizoso.net> */ > + > +#ifndef __ROCKET_DEVICE_H__ > +#define __ROCKET_DEVICE_H__ > + > +#include <drm/drm_device.h> > +#include <linux/clk.h> > +#include <linux/container_of.h> > + > +#include "rocket_core.h" > + > +struct rocket_device { > + struct drm_device ddev; > + > + struct rocket_core *cores; > + unsigned int num_cores; > +}; > + > +int rocket_device_init(struct rocket_device *rdev); > +void rocket_device_fini(struct rocket_device *rdev); > + > +#define to_rocket_device(drm_dev) \ > + ((struct rocket_device *)container_of(drm_dev, struct rocket_device, ddev)) > + > +#endif > diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c > new file mode 100644 > index 0000000000000000000000000000000000000000..b38a5c6264cb4e74d5e381adaeba1426e576fa56 > --- /dev/null > +++ b/drivers/accel/rocket/rocket_drv.c > @@ -0,0 +1,279 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright 2024-2025 Tomeu Vizoso <tomeu(a)tomeuvizoso.net> */ > + > +#include <drm/drm_accel.h> > +#include <drm/drm_drv.h> > +#include <drm/drm_gem.h> > +#include <drm/drm_ioctl.h> > +#include <drm/drm_of.h> > +#include <linux/array_size.h> > +#include <linux/clk.h> > +#include <linux/component.h> > +#include <linux/dma-mapping.h> > +#include <linux/iommu.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/pm_runtime.h> > + > +#include "rocket_drv.h" > + > +static int > +rocket_open(struct drm_device *dev, struct drm_file *file) > +{ > + struct rocket_device *rdev = to_rocket_device(dev); > + struct rocket_file_priv *rocket_priv; > + > + rocket_priv = kzalloc(sizeof(*rocket_priv), GFP_KERNEL); > + if (!rocket_priv) > + return -ENOMEM; > + > + rocket_priv->rdev = rdev; > + rocket_priv->domain = iommu_paging_domain_alloc(dev->dev); > + file->driver_priv = rocket_priv; > + > + return 0; > +} > + > +static void > +rocket_postclose(struct drm_device *dev, struct drm_file *file) > +{ > + struct rocket_file_priv *rocket_priv = file->driver_priv; > + > + iommu_domain_free(rocket_priv->domain); > + kfree(rocket_priv); > +} > + > +static const struct drm_ioctl_desc rocket_drm_driver_ioctls[] = { > +#define ROCKET_IOCTL(n, func) \ > + DRM_IOCTL_DEF_DRV(ROCKET_##n, rocket_ioctl_##func, 0) > +}; > + > +DEFINE_DRM_ACCEL_FOPS(rocket_accel_driver_fops); > + > +/* > + * Rocket driver version: > + * - 1.0 - initial interface > + */ > +static const struct drm_driver rocket_drm_driver = { > + .driver_features = DRIVER_COMPUTE_ACCEL, > + .open = rocket_open, > + .postclose = rocket_postclose, > + .ioctls = rocket_drm_driver_ioctls, > + .num_ioctls = ARRAY_SIZE(rocket_drm_driver_ioctls), > + .fops = &rocket_accel_driver_fops, > + .name = "rocket", > + .desc = "rocket DRM", > +}; > + > +static int rocket_drm_bind(struct device *dev) > +{ > + struct device_node *core_node; > + struct rocket_device *rdev; > + struct drm_device *ddev; > + unsigned int num_cores = 1; > + int err; > + > + rdev = devm_drm_dev_alloc(dev, &rocket_drm_driver, struct rocket_device, ddev); > + if (IS_ERR(rdev)) > + return PTR_ERR(rdev); > + > + ddev = &rdev->ddev; > + dev_set_drvdata(dev, rdev); > + > + for_each_compatible_node(core_node, NULL, "rockchip,rk3588-rknn-core") > + if (of_device_is_available(core_node)) > + num_cores++; > + > + rdev->cores = devm_kcalloc(dev, num_cores, sizeof(*rdev->cores), GFP_KERNEL); > + if (IS_ERR(rdev->cores)) > + return PTR_ERR(rdev->cores); > + > + /* Add core 0, any other cores will be added later when they are bound */ > + rdev->cores[0].rdev = rdev; > + rdev->cores[0].dev = dev; > + rdev->cores[0].index = 0; > + rdev->num_cores = 1; > + > + err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(40)); > + if (err) > + return err; > + > + err = rocket_device_init(rdev); > + if (err) { > + dev_err_probe(dev, err, "Fatal error during NPU init\n"); > + goto err_device_fini; > + } > + > + err = component_bind_all(dev, rdev); > + if (err) > + goto err_device_fini; > + > + err = drm_dev_register(ddev, 0); > + if (err < 0) > + goto err_unbind; > + > + return 0; > + > +err_unbind: > + component_unbind_all(dev, rdev); > +err_device_fini: > + rocket_device_fini(rdev); > + return err; > +} > + > +static void rocket_drm_unbind(struct device *dev) > +{ > + struct rocket_device *rdev = dev_get_drvdata(dev); > + struct drm_device *ddev = &rdev->ddev; > + > + drm_dev_unregister(ddev); > + > + component_unbind_all(dev, rdev); > + > + rocket_device_fini(rdev); > +} > + > +const struct component_master_ops rocket_drm_ops = { > + .bind = rocket_drm_bind, > + .unbind = rocket_drm_unbind, > +}; > + > +static int rocket_core_bind(struct device *dev, struct device *master, void *data) > +{ > + struct rocket_device *rdev = data; > + unsigned int core = rdev->num_cores; > + int err; > + > + dev_set_drvdata(dev, rdev); > + > + rdev->cores[core].rdev = rdev; > + rdev->cores[core].dev = dev; > + rdev->cores[core].index = core; > + rdev->cores[core].link = device_link_add(dev, rdev->cores[0].dev, > + DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME); > + > + rdev->num_cores++; > + > + err = rocket_core_init(&rdev->cores[core]); > + if (err) { > + rocket_device_fini(rdev); > + return err; > + } > + > + return 0; > +} > + > +static void rocket_core_unbind(struct device *dev, struct device *master, void *data) > +{ > + struct rocket_device *rdev = data; > + > + for (unsigned int core = 1; core < rdev->num_cores; core++) { > + if (rdev->cores[core].dev == dev) { > + rocket_core_fini(&rdev->cores[core]); > + device_link_del(rdev->cores[core].link); > + break; > + } > + } > +} > + > +const struct component_ops rocket_core_ops = { > + .bind = rocket_core_bind, > + .unbind = rocket_core_unbind, > +}; > + > +static int rocket_probe(struct platform_device *pdev) > +{ > + struct component_match *match = NULL; > + struct device_node *core_node; > + > + if (fwnode_device_is_compatible(pdev->dev.fwnode, "rockchip,rk3588-rknn-core")) > + return component_add(&pdev->dev, &rocket_core_ops); > + > + for_each_compatible_node(core_node, NULL, "rockchip,rk3588-rknn-core") { > + if (!of_device_is_available(core_node)) > + continue; > + > + drm_of_component_match_add(&pdev->dev, &match, > + component_compare_of, core_node); > + } > + > + return component_master_add_with_match(&pdev->dev, &rocket_drm_ops, match); > +} > + > +static void rocket_remove(struct platform_device *pdev) > +{ > + if (fwnode_device_is_compatible(pdev->dev.fwnode, "rockchip,rk3588-rknn-core-top")) > + component_master_del(&pdev->dev, &rocket_drm_ops); > + else if (fwnode_device_is_compatible(pdev->dev.fwnode, "rockchip,rk3588-rknn-core")) > + component_del(&pdev->dev, &rocket_core_ops); > +} > + > +static const struct of_device_id dt_match[] = { > + { .compatible = "rockchip,rk3588-rknn-core-top" }, > + { .compatible = "rockchip,rk3588-rknn-core" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, dt_match); > + > +static int find_core_for_dev(struct device *dev) > +{ > + struct rocket_device *rdev = dev_get_drvdata(dev); > + > + for (unsigned int core = 0; core < rdev->num_cores; core++) { > + if (dev == rdev->cores[core].dev) > + return core; > + } > + > + return -1; > +} > + > +static int rocket_device_runtime_resume(struct device *dev) > +{ > + struct rocket_device *rdev = dev_get_drvdata(dev); > + int core = find_core_for_dev(dev); > + int err = 0; > + > + if (core < 0) > + return -ENODEV; > + > + err = clk_bulk_prepare_enable(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks); > + if (err) { > + dev_err(dev, "failed to enable (%d) clocks for core %d\n", err, core); > + return err; > + } > + > + return 0; > +} > + > +static int rocket_device_runtime_suspend(struct device *dev) > +{ > + struct rocket_device *rdev = dev_get_drvdata(dev); > + int core = find_core_for_dev(dev); > + > + if (core < 0) > + return -ENODEV; > + > + clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks); > + > + return 0; > +} > + > +EXPORT_GPL_DEV_PM_OPS(rocket_pm_ops) = { > + RUNTIME_PM_OPS(rocket_device_runtime_suspend, rocket_device_runtime_resume, NULL) > + SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume) > +}; > + > +static struct platform_driver rocket_driver = { > + .probe = rocket_probe, > + .remove = rocket_remove, > + .driver = { > + .name = "rocket", > + .pm = pm_ptr(&rocket_pm_ops), > + .of_match_table = dt_match, > + }, > +}; > +module_platform_driver(rocket_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_DESCRIPTION("DRM driver for the Rockchip NPU IP"); > +MODULE_AUTHOR("Tomeu Vizoso"); > diff --git a/drivers/accel/rocket/rocket_drv.h b/drivers/accel/rocket/rocket_drv.h > new file mode 100644 > index 0000000000000000000000000000000000000000..3219621afb72acdfa915c110e2ec3aacb66bd940 > --- /dev/null > +++ b/drivers/accel/rocket/rocket_drv.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* Copyright 2024-2025 Tomeu Vizoso <tomeu(a)tomeuvizoso.net> */ > + > +#ifndef __ROCKET_DRV_H__ > +#define __ROCKET_DRV_H__ > + > +#include "rocket_device.h" > + > +struct rocket_file_priv { > + struct rocket_device *rdev; > + > + struct iommu_domain *domain; > +}; > + > +#endif >

2 months

Re: [PATCH v7 04/10] accel/rocket: Add job submission IOCTL

by Robin Murphy

On 11/07/2025 5:00 pm, Tomeu Vizoso wrote: > On Tue, Jun 24, 2025 at 3:50 PM Robin Murphy <robin.murphy(a)arm.com> wrote: >> >> On 2025-06-06 7:28 am, Tomeu Vizoso wrote: >> [...] >>> diff --git a/drivers/accel/rocket/rocket_device.h b/drivers/accel/rocket/rocket_device.h >>> index 10acfe8534f00a7985d40a93f4b2f7f69d43caee..50e46f0516bd1615b5f826c5002a6c0ecbf9aed4 100644 >>> --- a/drivers/accel/rocket/rocket_device.h >>> +++ b/drivers/accel/rocket/rocket_device.h >>> @@ -13,6 +13,8 @@ >>> struct rocket_device { >>> struct drm_device ddev; >>> >>> + struct mutex sched_lock; >>> + >>> struct mutex iommu_lock; >> >> Just realised I missed this in the last patch, but iommu_lock appears to >> be completely unnecessary now. >> >>> struct rocket_core *cores; >> [...] >>> +static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *job) >>> +{ >>> + struct rocket_task *task; >>> + bool task_pp_en = 1; >>> + bool task_count = 1; >>> + >>> + /* GO ! */ >>> + >>> + /* Don't queue the job if a reset is in progress */ >>> + if (atomic_read(&core->reset.pending)) >>> + return; >>> + >>> + task = &job->tasks[job->next_task_idx]; >>> + job->next_task_idx++; >>> + >>> + rocket_pc_writel(core, BASE_ADDRESS, 0x1); >>> + >>> + rocket_cna_writel(core, S_POINTER, 0xe + 0x10000000 * core->index); >>> + rocket_core_writel(core, S_POINTER, 0xe + 0x10000000 * core->index); >> >> Those really look like bitfield operations rather than actual arithmetic >> to me. >> >>> + >>> + rocket_pc_writel(core, BASE_ADDRESS, task->regcmd); >> >> I don't see how regcmd is created (I guess that's in userspace?), but >> given that it's explicitly u64 all the way through - and especially >> since you claim to support 40-bit DMA addresses - it definitely seems >> suspicious that the upper 32 bits never seem to be consumed anywhere :/ > > Yeah, but there's no other register for BASE_ADDRESS address in the TRM. That only reaffirms the question then - if this value is only ever written verbatim to a 32-bit register, why is it 64-bit? Thanks, Robin.

2 months

[PATCH v6 0/2] dma-buf: heaps: Create a CMA heap for each CMA reserved region

by Maxime Ripard

Hi, Here's another attempt at supporting user-space allocations from a specific carved-out reserved memory region. The initial problem we were discussing was that I'm currently working on a platform which has a memory layout with ECC enabled. However, enabling the ECC has a number of drawbacks on that platform: lower performance, increased memory usage, etc. So for things like framebuffers, the trade-off isn't great and thus there's a memory region with ECC disabled to allocate from for such use cases. After a suggestion from John, I chose to first start using heap allocations flags to allow for userspace to ask for a particular ECC setup. This is then backed by a new heap type that runs from reserved memory chunks flagged as such, and the existing DT properties to specify the ECC properties. After further discussion, it was considered that flags were not the right solution, and relying on the names of the heaps would be enough to let userspace know the kind of buffer it deals with. Thus, even though the uAPI part of it had been dropped in this second version, we still needed a driver to create heaps out of carved-out memory regions. In addition to the original usecase, a similar driver can be found in BSPs from most vendors, so I believe it would be a useful addition to the kernel. Some extra discussion with Rob Herring [1] came to the conclusion that some specific compatible for this is not great either, and as such an new driver probably isn't called for either. Some other discussions we had with John [2] also dropped some hints that multiple CMA heaps might be a good idea, and some vendors seem to do that too. So here's another attempt that doesn't affect the device tree at all and will just create a heap for every CMA reserved memory region. It also falls nicely into the current plan we have to support cgroups in DRM/KMS and v4l2, which is an additional benefit. Let me know what you think, Maxime 1: https://lore.kernel.org/all/20250707-cobalt-dingo-of-serenity-dbf92c@houat/ 2: https://lore.kernel.org/all/CANDhNCroe6ZBtN_o=c71kzFFaWK-fF5rCdnr9P5h1sgPOW… Let me know what you think, Maxime Signed-off-by: Maxime Ripard <mripard(a)kernel.org> --- Changes in v6: - Drop the new driver and allocate a CMA heap for each region now - Dropped the binding - Rebased on 6.16-rc5 - Link to v5: https://lore.kernel.org/r/20250617-dma-buf-ecc-heap-v5-0-0abdc5863a4f@kerne… Changes in v5: - Rebased on 6.16-rc2 - Switch from property to dedicated binding - Link to v4: https://lore.kernel.org/r/20250520-dma-buf-ecc-heap-v4-1-bd2e1f1bb42c@kerne… Changes in v4: - Rebased on 6.15-rc7 - Map buffers only when map is actually called, not at allocation time - Deal with restricted-dma-pool and shared-dma-pool - Reword Kconfig options - Properly report dma_map_sgtable failures - Link to v3: https://lore.kernel.org/r/20250407-dma-buf-ecc-heap-v3-0-97cdd36a5f29@kerne… Changes in v3: - Reworked global variable patch - Link to v2: https://lore.kernel.org/r/20250401-dma-buf-ecc-heap-v2-0-043fd006a1af@kerne… Changes in v2: - Add vmap/vunmap operations - Drop ECC flags uapi - Rebase on top of 6.14 - Link to v1: https://lore.kernel.org/r/20240515-dma-buf-ecc-heap-v1-0-54cbbd049511@kerne… --- Maxime Ripard (2): dma/contiguous: Add helper to test reserved memory type dma-buf: heaps: cma: Create CMA heap for each CMA reserved region drivers/dma-buf/heaps/cma_heap.c | 52 +++++++++++++++++++++++++++++++++++++++- include/linux/dma-map-ops.h | 13 ++++++++++ kernel/dma/contiguous.c | 7 ++++++ 3 files changed, 71 insertions(+), 1 deletion(-) --- base-commit: 47633099a672fc7bfe604ef454e4f116e2c954b1 change-id: 20240515-dma-buf-ecc-heap-28a311d2c94e prerequisite-message-id: <20250610131231.1724627-1-jkangas(a)redhat.com> prerequisite-patch-id: bc44be5968feb187f2bc1b8074af7209462b18e7 prerequisite-patch-id: f02a91b723e5ec01fbfedf3c3905218b43d432da prerequisite-patch-id: e944d0a3e22f2cdf4d3b3906e5603af934696deb Best regards, -- Maxime Ripard <mripard(a)kernel.org>

2 months

Re: DMA-BUFs always uncached on arm64, causing poor camera performance on Librem 5

by Laurent Pinchart

On Thu, Jul 10, 2025 at 10:49:19AM +0200, Pavel Machek wrote: > Hi! > > > > memcpy() from normal memory is about 2msec/1MB. Unfortunately, for > > > DMA-BUFs it is 20msec/1MB, and that basically means I can't easily do > > > 760p video recording. Plus, copying full-resolution photo buffer takes > > > more than 200msec! > > > > > > There's possibility to do some processing on GPU, and its implemented here: > > > > > > https://gitlab.com/tui/tui/-/tree/master/icam?ref_type=heads > > > > > > but that hits the same problem in the end -- data is in DMA-BUF, > > > uncached, and takes way too long to copy out. > > > > > > And that's ... wrong. DMA ended seconds ago, complete cache flush > > > would be way cheaper than copying single frame out, and I still have > > > to deal with uncached frames. > > > > > > So I have two questions: > > > > > > 1) Is my analysis correct that, no matter how I get frame from v4l and > > > process it on GPU, I'll have to copy it from uncached memory in the > > > end? > > > > If you need to touch the buffers using the CPU then you are either > > stuck with uncached memory or you need to implement bracketed access to > > do the necessary cache maintenance. Be aware that completely flushing > > the cache is not really an option, as that would impact other > > workloads, so you have to flush the cache by walking the virtual > > address space of the buffer, which may take a significant amount of CPU > > time. > > What kind of "significant amount of CPU time" are we talking here? > Millisecond? It really depends on the platform, the type of cache, and the size of the buffer. I remember that back in the N900 days a selective cash clean of a large buffer for full resolution images took several dozens of milliseconds, possibly close to 100ms. We had to clean the whole D-cache to make it fast enough, but you can't always do that as Lucas mentioned. > Bracketed access is fine with me. > > Flushing a cache should be an option. I'm root, there's no other > significant workload, and copying out the buffer takes 200msec+. There > are lot of cache flushes that can be done in quarter a second! > > > However, if you are only going to use the buffer with the GPU I see no > > reason to touch it from the CPU side. Why would you even need to copy > > the content? After all dma-bufs are meant to enable zero-copy between > > DMA capable accelerators. You can simply import the V4L2 buffer into a > > GL texture using EGL_EXT_image_dma_buf_import. Using this path you > > don't need to bother with the cache at all, as the GPU will directly > > read the video buffers from RAM. > > Yes, so GPU will read video buffer from RAM, then debayer it, and then > what? Then I need to store a data into raw file, or use CPU to turn it > into JPEG file, or maybe run video encoder on it. That are all tasks > that are done on CPU... -- Regards, Laurent Pinchart

2 months

Re: DMA-BUFs always uncached on arm64, causing poor camera performance on Librem 5

by Nicolas Dufresne

Hi Pavel, Le jeudi 10 juillet 2025 à 10:24 +0200, Pavel Machek a écrit : > Hi! > > It seems that DMA-BUFs are always uncached on arm64... which is a > problem. > > I'm trying to get useful camera support on Librem 5, and that includes > recording vidos (and taking photos). > > memcpy() from normal memory is about 2msec/1MB. Unfortunately, for > DMA-BUFs it is 20msec/1MB, and that basically means I can't easily do > 760p video recording. Plus, copying full-resolution photo buffer takes > more than 200msec! > > There's possibility to do some processing on GPU, and its implemented here: > > https://gitlab.com/tui/tui/-/tree/master/icam?ref_type=heads > > but that hits the same problem in the end -- data is in DMA-BUF, > uncached, and takes way too long to copy out. > > And that's ... wrong. DMA ended seconds ago, complete cache flush > would be way cheaper than copying single frame out, and I still have > to deal with uncached frames. > > So I have two questions: > > 1) Is my analysis correct that, no matter how I get frame from v4l and > process it on GPU, I'll have to copy it from uncached memory in the > end? > > 2) Does anyone have patches / ideas / roadmap how to solve that? It > makes GPU unusable for computing, and camera basically unusable for > video. If CPU access is strictly required for your use case, the way forward is to implement V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINT in the capture driver. Very little drivers enable that. Once your driver have that capability, you will be able to set V4L2_MEMORY_FLAG_NON_COHERENT while doing REQBUFS or CREATE_BUFS ioctl. That gives you allocation with CPU cache working, but you'll get the invalidation (or flush) overhead by default. When capture data have not been read by CPU, you can always queue it back with the V4L2_BUF_FLAG_NO_CACHE_INVALIDATE. But for your use case, it seems that you want the invalidation to take place, otherwise your software will endup reading old cache data instead of the next frame data. Please note that the integration in the DMABuf SYNC ioctl was missing for a while, so make sure you have recent enough kernel or get ready for backports. The feature itself was commonly used with CPU only access, notably on ChromeOS using libyuv. No DMABuf was involved initially. regards, Nicolas [0] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/vidioc-reqbu… > > Best regards, > Pavel

2 months

[PATCH v2] Documentation: dma-buf: heaps: Add naming guidelines

by Maxime Ripard

We've discussed a number of times of how some heap names are bad, but not really what makes a good heap name. Let's document what we expect the heap names to look like. Reviewed-by: Bagas Sanjaya <bagasdotme(a)gmail.com> Signed-off-by: Maxime Ripard <mripard(a)kernel.org> --- Changes in v2: - Added justifications for each requirement / suggestions - Added a mention and example of buffer attributes - Link to v1: https://lore.kernel.org/r/20250520-dma-buf-heap-names-doc-v1-1-ab31f74809ee… --- Documentation/userspace-api/dma-buf-heaps.rst | 38 +++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst index 535f49047ce6450796bf4380c989e109355efc05..835ad1c3a65bc07b6f41d387d85c57162909e859 100644 --- a/Documentation/userspace-api/dma-buf-heaps.rst +++ b/Documentation/userspace-api/dma-buf-heaps.rst @@ -21,5 +21,43 @@ following heaps: usually created either through the kernel commandline through the `cma` parameter, a memory region Device-Tree node with the `linux,cma-default` property set, or through the `CMA_SIZE_MBYTES` or `CMA_SIZE_PERCENTAGE` Kconfig options. Depending on the platform, it might be called ``reserved``, ``linux,cma``, or ``default-pool``. + +Naming Convention +================= + +``dma-buf`` heaps name should meet a number of constraints: + +- That name must be stable, and must not change from one version to the + other. Userspace identifies heaps by their name, so if the names ever + changes, we would be likely to introduce regressions. + +- That name must describe the memory region the heap will allocate from, + and must uniquely identify it in a given platform. Since userspace + applications use the heap name as the discriminant, it must be able to + tell which heap it wants to use reliably if there's multiple heaps. + +- That name must not mention implementation details, such as the + allocator. The heap driver will change over time, and implementation + details when it was introduced might not be relevant in the future. + +- The name should describe properties of the buffers that would be + allocated. Doing so will make heap identification easier for + userspace. Such properties are: + + - ``cacheable`` / ``uncacheable`` for buffers with CPU caches enabled + or disabled; + + - ``contiguous`` for physically contiguous buffers; + + - ``protected`` for encrypted buffers not accessible the OS; + +- The name may describe intended usage. Doing so will make heap + identification easier for userspace applications and users. + +For example, assuming a platform with a reserved memory region located +at the RAM address 0x42000000, intended to allocate video framebuffers, +physically contiguous, and backed by the CMA kernel allocator. Good +names would be ``memory@42000000-cacheable-contiguous`` or +``video@42000000``, but ``cma-video`` wouldn't. --- base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 change-id: 20250520-dma-buf-heap-names-doc-31261aa0cfe6 Best regards, -- Maxime Ripard <mripard(a)kernel.org>

2 months

← Newer
1
...
6
7
8
9
10
11
12
...
303
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig