Linaro-mm-sig August 2026

linaro-mm-sig@lists.linaro.org

28 participants
82 discussions

Re: [PATCH v4 1/5] accel/amdxdna: refuse an I/O memory mapping of an imported BO

by Lizhi Hou

On 8/17/26 16:07, Taimuraz Kaitmazov wrote: > amdxdna_gem_vmap() flattens the iosys_map drm_gem_vmap() fills in down to > the void * in abo->mem.kva, and iosys_map is discriminated by is_iomem, so > an exporter answering with an I/O mapping leaves a void __iomem pointer > there, which amdxdna_cmd_set_error() memsets and memcpys through. > > amdxdna_drm_va_tbl takes a dmabuf_fd, so such a BO can be any exporter's > buffer. amdgpu cannot reach this: its .pin forces GTT for a non peer to > peer attachment like ours. An exporter on drm_gem_prime_dmabuf_ops has > no .pin, and drm_gem_ttm_vmap() answers iomem for a VRAM resident > object, so an NPU paired with nouveau or radeon does. > > Drop such a mapping and answer NULL. Checking here rather than in the > .vmap callback leaves that callback's iosys_map contract intact for a > caller equipped to read I/O memory, and covers everything that takes a > plain kernel address through this helper. vmw_gem_vmap() refuses the > same case; unlike that one this path is reachable from an unprivileged > ioctl, so it neither warns nor logs at error level. > > Signed-off-by: Taimuraz Kaitmazov <taimuraz(a)kaitmazov.com> > --- > drivers/accel/amdxdna/amdxdna_gem.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/accel/amdxdna/amdxdna_gem.c b/drivers/accel/amdxdna/amdxdna_gem.c > index cca84fa07e9d..f88b5349cd4b 100644 > --- a/drivers/accel/amdxdna/amdxdna_gem.c > +++ b/drivers/accel/amdxdna/amdxdna_gem.c > @@ -209,10 +209,15 @@ void *amdxdna_gem_vmap(struct amdxdna_gem_obj *abo) > > if (!abo->mem.kva) { > ret = drm_gem_vmap(to_gobj(abo), &map); > - if (ret) > + if (ret) { > XDNA_ERR(abo->client->xdna, "Vmap bo failed, ret %d", ret); > - else > + } else if (map.is_iomem) { > + /* Callers use the result as an ordinary kernel address. */ > + XDNA_DBG(abo->client->xdna, "Vmap bo returned I/O memory"); > + drm_gem_vunmap(to_gobj(abo), &map); > + } else { > abo->mem.kva = map.vaddr; > + } Reviewed-by: Lizhi Hou <lizhi.hou(a)amd.com> > } > return abo->mem.kva; > }

1 month, 3 weeks

Re: [PATCH v4 0/5] accel/amdxdna: SYNC_BO correctness fixes

by Lizhi Hou

On 8/18/26 14:40, Taimuraz Kaitmazov wrote: > Patch 1 needs a prerequisite. It makes amdxdna_gem_vmap() answer NULL > on an iomem exporter, and the eight amdxdna_cmd_get_payload() callers > in aie2_message.c check neither the pointer nor the length it leaves > unwritten there. A command BO can be an import, so patch 1 alone turns > a silent __iomem write into a NULL deref with an uninitialised length. > https://lore.kernel.org/all/20260818002459.377641-1-taimuraz@kaitmazov.com/ > amdxdna_cmd_get_op() is always called before amdxdna_cmd_get_payload() for the same BO, and since amdxdna_gem_vmap() caches its results, the vmap inside get_payload() is currently guaranteed to succeed. The patch is therefore defensive rather than fixing a currently reachable crash. Lizhi > > fixes the callers. Happy to respin on top if you prefer them together. > > Taimuraz > > On 8/18/26 02:07, Taimuraz Kaitmazov wrote: >> Five fixes in and around amdxdna_drm_sync_bo_ioctl(). >> >> Patch 1 refuses an I/O memory mapping the driver would otherwise >> store as >> if it were an ordinary kernel address. Patch 2 checks the device-BO >> range >> for overflow. Patch 4 refuses a flush of an imported BO, which is why >> patch 3 comes first: the ioctl answers a rejected flush with drm_WARN(), >> so without it an ordinary sync on an imported BO splats. Patch 5 stops >> the ioctl reporting failure for a flush that succeeded. >> >> v3's zero-length patch has left this series. On hardware it turns out to >> be a page fault in drm_clflush_virt_range() rather than the tidy-up its >> commit message described, so it is a -fixes patch now, sent >> separately as >> "accel/amdxdna: return early from a zero-length flush" with Fixes: and >> Cc: stable. Patch 4 here needs its hunk, so this series wants that one >> first. >> >> Changes in v4: >> - patch 1: the is_iomem check moved from the .vmap callback into >> amdxdna_gem_vmap(), per Lizhi, and it logs at debug level rather >> than >> error, since an unprivileged caller can repeat it. >> - new patch 3: an unprivileged SYNC_BO with an out-of-range offset >> already reaches that drm_WARN() today. Sashiko's review of v3 3/3 >> flagged the same thing. >> - new patch 4: refuses the flush for every imported BO, as asked. >> - new patch 5: the debug-BO sync I mentioned on the v2 thread. >> Only a BO >> attached with ATTACH_DEBUG_BO has an assigned hwctx, so every other >> FROM_DEVICE sync ends in -EINVAL with its flush already done. The >> -EINVAL reproduces on a Strix Point NPU. >> >> On patch 4, one consequence worth deciding before it lands. A heap BO is >> created through amdxdna_drm_create_share_bo(), so a device running on >> carveout memory reaches its heap through a cbuf, is_import_bo() is >> true of >> it, and SYNC_BO on every AMDXDNA_BO_DEV now answers -EOPNOTSUPP. Today >> that path flushes nothing anyway -- amdxdna_cbuf_map() fills in only the >> DMA address and length, so drm_clflush_sg() walks zero pages -- so the >> change is silent no-op to hard error, and XRT's dbg_buffer::sync() >> reaches >> it without Debug.force_driver_sync. If you would rather keep our own >> exporters working, I have the variant keyed on the exporter's ops, which >> confines the refusal to foreign buffers. Say which you prefer. >> >> v3: >> https://lore.kernel.org/all/20260813164700.43960-1-taimuraz@kaitmazov.com/ >> >> Built on drm-misc-next, each commit on its own: x86_64 with >> DRM_ACCEL_AMDXDNA=m, clang 22.1.8, no warnings. Patch 5's reproducer was >> run against 7.1.8's in-tree driver, where that call is unchanged. >> >> Taimuraz Kaitmazov (5): >> accel/amdxdna: refuse an I/O memory mapping of an imported BO >> accel/amdxdna: check the sync range for overflow on a device BO >> accel/amdxdna: do not warn when a sync request is rejected >> accel/amdxdna: refuse to flush an imported BO >> accel/amdxdna: do not fail a sync for a BO with no debug context >> >> drivers/accel/amdxdna/amdxdna_ctx.c | 3 ++- >> drivers/accel/amdxdna/amdxdna_gem.c | 27 +++++++++++++++++++-------- >> 2 files changed, 21 insertions(+), 9 deletions(-) >> > >

1 month, 3 weeks

Re: [PATCH v2 00/15] accel/qda: Qualcomm DSP Accelerator driver

by Krzysztof Kozlowski

On 19/08/2026 17:23, Rob Clark wrote: >>>> >>>> Growing old driver does not look complicated itself. The only a bit >>>> tricky thing is to present somehow exclusive interface to user-space, >>>> like usage of one disables the second etc. Depending on actual >>>> differences in that interface. >>>> >>>> But having a duplicated driver is a clear no go and it is well known >>>> upstream requirement. Nothing new here. >>> >>> Would it be acceptable as a first step to, by default (ie. when no >>> cmdline override/etc) for the new driver to bind to new compatibles >> >> You have one compatible. >> >>> and the existing driver to old? Ie. have both drivers but only one >>> binds on a given platform? >> >> I don't see how is it possible to write such DTS, because - repeating my >> question - how many hardware blocks is there? I believe only one per >> given DSP, so how could you have two device nodes? > > Maybe I'm misunderstanding something here.. I don't see any new > bindings with this series so my assumption is that the bindings are > the same. What I meant was something more like "qcom,glymur-fastrpc" > would bind to new driver but "qcom,fastrpc" (which seems to be what is > used on older platforms) would bind to the old driver. > > So single dts node, but different compatible strings picking which > driver is used. Glymur is already done, so imagining we talk about next/future SoC then we would be at point of duplicating drivers for the same hardware. So back to square one of my comments. The rule of usptream development is that we do not accept duplicated code, just because a vendor wants to write something new. This is basically the concept applied all over the drivers tree, where we pushed back against all sorts of duplications all over the vendors. What I miss in this thread is why would there be any exception here. We do not grant exceptions from standard practices on "I want" reasons. Best regards, Krzysztof

1 month, 3 weeks

Re: [PATCH v2 00/15] accel/qda: Qualcomm DSP Accelerator driver

by Krzysztof Kozlowski

On 19/08/2026 16:49, Rob Clark wrote: > On Wed, Aug 19, 2026 at 7:43 AM Krzysztof Kozlowski <krzk(a)kernel.org> wrote: >> >> On 19/08/2026 16:38, Rob Clark wrote: >>> On Wed, Aug 19, 2026 at 7:21 AM Krzysztof Kozlowski <krzk(a)kernel.org> wrote: >>>> >>>> On 19/08/2026 15:26, Ekansh Gupta wrote: >>>>> On 19-08-2026 00:43, Krzysztof Kozlowski wrote: >>>>>> On 17/08/2026 06:47, Ekansh Gupta wrote: >>>>>>> This patch series introduces the Qualcomm DSP Accelerator (QDA) driver, >>>>>>> a DRM-based accelerator driver for Qualcomm DSPs. The driver provides a >>>>>>> standardized interface for offloading computational tasks to DSPs found >>>>>>> on Qualcomm SoCs, supporting all DSP domains. >>>>>>> >>>>>>> The QDA driver implements the FastRPC protocol over the DRM accel >>>>>>> subsystem. It uses the same device-tree node structure as the existing >>>>>>> fastrpc driver in drivers/misc/. The approach for binding the QDA driver >>>>>>> to device-tree nodes while coexisting with the fastrpc driver is an open >>>>>>> item described below. >>>>>> >>>>>> No. Grow/replace/improve existing driver instead of coming with a duplicate. >>>>>> >>>>>> That's a standard upstream requirement, basically given on every >>>>>> upstreaming guide. >>>>>> >>>>>> Please watch old talk from Greg - "I Don’t Want Your Code!". >>>>> Posted discussion threads here[1]. Would seek comments from Dmitry, >>>>> Srini as well. >>>>> >>>>> [1] >>>>> https://lore.kernel.org/all/3476b5c3-7983-4994-a901-3d7d8bd75255@oss.qualco… >>>> >>>> The rest of the comments is still valid even if you did not acknowledge >>>> them. >>>> >>>> Anyway, regarding above - again, watch the talk from Greg. >>>> >>>> You have ONE driver. Not two. >>> >>> Long term, moving to the common driver framework (which did not exist >>> when fastrpc was first created) seems like a good thing. But does >>> that not allow for some transition period? How can we get from here >>> to there without otherwise breaking userspace? Is there some other >>> precedent elsewhere in other driver subsystems? >> >> Yes, Iris and Venus where we agreed for an exception (two drivers) as >> long as new driver supports old hardware / features. >> >> This is not the case here, right? >> >>> >>> I suppose drm exposing legacy fbdev on top of drm drivers is _sort of_ >>> a precedent if you squint a bit? I'm not really familiar enough to >>> say if that would be reasonable/possible in this case. >> >> Growing old driver does not look complicated itself. The only a bit >> tricky thing is to present somehow exclusive interface to user-space, >> like usage of one disables the second etc. Depending on actual >> differences in that interface. >> >> But having a duplicated driver is a clear no go and it is well known >> upstream requirement. Nothing new here. > > Would it be acceptable as a first step to, by default (ie. when no > cmdline override/etc) for the new driver to bind to new compatibles You have one compatible. > and the existing driver to old? Ie. have both drivers but only one > binds on a given platform? I don't see how is it possible to write such DTS, because - repeating my question - how many hardware blocks is there? I believe only one per given DSP, so how could you have two device nodes? Best regards, Krzysztof

1 month, 3 weeks

Re: [PATCH v2 00/15] accel/qda: Qualcomm DSP Accelerator driver

by Krzysztof Kozlowski

On 19/08/2026 16:38, Rob Clark wrote: > On Wed, Aug 19, 2026 at 7:21 AM Krzysztof Kozlowski <krzk(a)kernel.org> wrote: >> >> On 19/08/2026 15:26, Ekansh Gupta wrote: >>> On 19-08-2026 00:43, Krzysztof Kozlowski wrote: >>>> On 17/08/2026 06:47, Ekansh Gupta wrote: >>>>> This patch series introduces the Qualcomm DSP Accelerator (QDA) driver, >>>>> a DRM-based accelerator driver for Qualcomm DSPs. The driver provides a >>>>> standardized interface for offloading computational tasks to DSPs found >>>>> on Qualcomm SoCs, supporting all DSP domains. >>>>> >>>>> The QDA driver implements the FastRPC protocol over the DRM accel >>>>> subsystem. It uses the same device-tree node structure as the existing >>>>> fastrpc driver in drivers/misc/. The approach for binding the QDA driver >>>>> to device-tree nodes while coexisting with the fastrpc driver is an open >>>>> item described below. >>>> >>>> No. Grow/replace/improve existing driver instead of coming with a duplicate. >>>> >>>> That's a standard upstream requirement, basically given on every >>>> upstreaming guide. >>>> >>>> Please watch old talk from Greg - "I Don’t Want Your Code!". >>> Posted discussion threads here[1]. Would seek comments from Dmitry, >>> Srini as well. >>> >>> [1] >>> https://lore.kernel.org/all/3476b5c3-7983-4994-a901-3d7d8bd75255@oss.qualco… >> >> The rest of the comments is still valid even if you did not acknowledge >> them. >> >> Anyway, regarding above - again, watch the talk from Greg. >> >> You have ONE driver. Not two. > > Long term, moving to the common driver framework (which did not exist > when fastrpc was first created) seems like a good thing. But does > that not allow for some transition period? How can we get from here > to there without otherwise breaking userspace? Is there some other > precedent elsewhere in other driver subsystems? Yes, Iris and Venus where we agreed for an exception (two drivers) as long as new driver supports old hardware / features. This is not the case here, right? > > I suppose drm exposing legacy fbdev on top of drm drivers is _sort of_ > a precedent if you squint a bit? I'm not really familiar enough to > say if that would be reasonable/possible in this case. Growing old driver does not look complicated itself. The only a bit tricky thing is to present somehow exclusive interface to user-space, like usage of one disables the second etc. Depending on actual differences in that interface. But having a duplicated driver is a clear no go and it is well known upstream requirement. Nothing new here. Best regards, Krzysztof

1 month, 3 weeks

Re: [PATCH v2 00/15] accel/qda: Qualcomm DSP Accelerator driver

by Krzysztof Kozlowski

On 19/08/2026 15:26, Ekansh Gupta wrote: > On 19-08-2026 00:43, Krzysztof Kozlowski wrote: >> On 17/08/2026 06:47, Ekansh Gupta wrote: >>> This patch series introduces the Qualcomm DSP Accelerator (QDA) driver, >>> a DRM-based accelerator driver for Qualcomm DSPs. The driver provides a >>> standardized interface for offloading computational tasks to DSPs found >>> on Qualcomm SoCs, supporting all DSP domains. >>> >>> The QDA driver implements the FastRPC protocol over the DRM accel >>> subsystem. It uses the same device-tree node structure as the existing >>> fastrpc driver in drivers/misc/. The approach for binding the QDA driver >>> to device-tree nodes while coexisting with the fastrpc driver is an open >>> item described below. >> >> No. Grow/replace/improve existing driver instead of coming with a duplicate. >> >> That's a standard upstream requirement, basically given on every >> upstreaming guide. >> >> Please watch old talk from Greg - "I Don’t Want Your Code!". > Posted discussion threads here[1]. Would seek comments from Dmitry, > Srini as well. > > [1] > https://lore.kernel.org/all/3476b5c3-7983-4994-a901-3d7d8bd75255@oss.qualco… The rest of the comments is still valid even if you did not acknowledge them. Anyway, regarding above - again, watch the talk from Greg. You have ONE driver. Not two. Best regards, Krzysztof

1 month, 3 weeks

Re: [PATCH v2 00/15] accel/qda: Qualcomm DSP Accelerator driver

by Krzysztof Kozlowski

On 19/08/2026 15:32, Ekansh Gupta wrote: > On 19-08-2026 00:51, Krzysztof Kozlowski wrote: >> On 18/08/2026 21:13, Krzysztof Kozlowski wrote: >>> On 17/08/2026 06:47, Ekansh Gupta wrote: >>>> This patch series introduces the Qualcomm DSP Accelerator (QDA) driver, >>>> a DRM-based accelerator driver for Qualcomm DSPs. The driver provides a >>>> standardized interface for offloading computational tasks to DSPs found >>>> on Qualcomm SoCs, supporting all DSP domains. >>>> >>>> The QDA driver implements the FastRPC protocol over the DRM accel >>>> subsystem. It uses the same device-tree node structure as the existing >>>> fastrpc driver in drivers/misc/. The approach for binding the QDA driver >>>> to device-tree nodes while coexisting with the fastrpc driver is an open >>>> item described below. >>> >>> No. Grow/replace/improve existing driver instead of coming with a duplicate. >>> >>> That's a standard upstream requirement, basically given on every >>> upstreaming guide. >>> >>> Please watch old talk from Greg - "I Don’t Want Your Code!". >>> >>>> >>>> v1: https://lore.kernel.org/all/20260519-qda-series-v1-0-b2d984c297f8@oss.qualc… >>>> RFC: https://lore.kernel.org/dri-devel/20260224-qda-firstpost-v1-0-fe46a9c1a046@… >>>> >>>> Changes since v1 >>>> ================ >>>> >>>> The v1 review raised two architectural objections and one correctness >>>> issue; all three are resolved in v2: >>>> >>>> * Christian König (dma-buf maintainer) pointed out that the imported- >>>> buffer path silently assumed the IOMMU maps every buffer as a single >>>> contiguous range, which is not guaranteed. v2 walks the scatterlist >>>> and cleanly rejects non-contiguous imports; contiguous imports (e.g. >>>> CMA DMA-buf heap) are accepted. (patch 11) >>>> >>>> * Dmitry Baryshkov objected to three different buffer-passing formats >>>> in the invoke IOCTL (DMA-BUF fd, direct/inline, DMA handle). v2 >>>> passes only GEM handles; userspace imports any fd to a GEM handle >>>> with DRM_IOCTL_PRIME_FD_TO_HANDLE before invoking. Packing and >>>> overlap handling are left to userspace. (patch 12) >>>> >>>> * The memory manager (patch 07) used a fixed 16-entry array without >>>> justification and leaked the device descriptor on teardown. v2 >>>> allocates the array from the DT context-bank count (as Dmitry >>>> suggested) and frees it correctly. >>>> >>>> User-space staging branch >>>> ========================= >>>> https://github.com/qualcomm/fastrpc/tree/accel/staging >>>> >>>> Key Features >>>> ============ >>>> >>>> * Standard DRM accelerator interface via /dev/accel/accelN >>>> * GEM-based buffer management with DMA-BUF import (PRIME) >>>> * IOMMU-based memory isolation using per-process context banks >>>> * FastRPC protocol implementation for DSP communication >>>> * RPMsg transport layer for reliable message passing >>>> * Support for all DSP domains (ADSP, CDSP, SDSP, GDSP) >>>> * DRM IOCTL interface for DSP session management, buffer allocation, >>>> and remote procedure invocation >>>> >>>> Architecture >>>> ============ >>>> >>>> 1. DRM Accelerator Framework Integration >>>> The driver registers as a DRM accel device, exposing a standard >>>> /dev/accel/accelN character device node. This provides established >>>> DRM infrastructure for device management, file operations, and >>>> IOCTL dispatch. >>>> >>>> 2. Memory Management >>>> Buffers are managed as GEM objects with PRIME support for DMA-BUF >>>> import. This enables buffer sharing with other DRM drivers (GPU, >>>> camera, video) using standard kernel mechanisms. Only contiguous >>>> imports are accepted; the driver verifies contiguity at import time >>>> rather than assuming it. >>>> >>>> 3. IOMMU Context Bank Management >>>> IOMMU context banks (CBs) are represented as proper struct device >>>> instances on a custom virtual bus (qda-compute-cb). Each CB device >>>> is registered with the IOMMU subsystem and receives its own IOMMU >>>> domain, enabling per-session address space isolation. The custom >>>> bus was introduced because IOMMU context banks are synthetic >>>> constructs — not real platform devices — and to ensure CB device >>>> lifetime is strictly subordinate to the parent QDA device. >>>> See also: https://lore.kernel.org/all/245d602f-3037-4ae3-9af9-d98f37258aae@oss.qualco… >>>> >>>> 4. Memory Manager Architecture >>>> The memory manager maintains a registry of IOMMU devices in an >>>> array sized to the number of context banks described in the device >>>> tree, and coordinates per-process device assignment with reference- >>>> counted lifetime management. The DMA-coherent backend allocates >>>> buffers with SID-prefixed DMA addresses for DSP firmware >>>> compatibility. >>>> >>>> 5. Transport Layer >>>> RPMsg communication is handled in a dedicated transport layer >>>> (qda_rpmsg.c), separate from the core DRM driver logic. >>>> >>>> 6. Code Organization >>>> The driver is organized across multiple files (~4800 lines total): >>>> * qda_drv.c: Core driver and DRM integration >>>> * qda_rpmsg.c: RPMsg transport layer >>>> * qda_cb.c: Context bank device management >>>> * qda_compute_bus.c: Custom virtual bus for CB devices >>>> * qda_gem.c: GEM object management >>>> * qda_prime.c: DMA-BUF import (PRIME) >>>> * qda_memory_manager.c: IOMMU device registry and allocation >>>> * qda_memory_dma.c: DMA-coherent allocation backend >>>> * qda_fastrpc.c: FastRPC protocol implementation >>>> * qda_ioctl.c: IOCTL dispatch >>>> >>>> 7. UAPI Design >>>> The driver exposes DRM-style IOCTLs defined in >>>> include/uapi/drm/qda_accel.h, following DRM UAPI conventions >>>> (__u32/__u64 types, C++ guard, GPL-2.0-only WITH Linux-syscall-note). >>>> Buffer arguments are identified by GEM handles; the driver never >>>> accepts DMA-BUF fds directly in any IOCTL. >>>> >>>> Patch Series Organization >>>> ========================== >>>> >>>> Patch 01: MAINTAINERS entry >>>> Patch 02: Driver documentation (Documentation/accel/qda/) >>>> Patches 03-04: Core driver skeleton and compute bus >>>> Patch 05: iommu: Register qda-compute-cb bus with IOMMU subsystem >>>> Patches 06-07: CB device enumeration and memory manager >>>> Patch 08: QUERY IOCTL and UAPI header >>>> Patches 09-11: GEM buffer management and PRIME import >>>> Patches 12-15: FastRPC protocol (invoke, session create/release, >>>> map/unmap) >>>> >>>> Open Items >>>> =========== >>>> >>>> 1. Device-Tree Compatible String >>>> The QDA driver uses the same device-tree node structure and >>>> properties as the existing fastrpc driver in drivers/misc/. A >>>> mechanism is needed to allow the QDA driver to bind to its device >>>> node independently of the fastrpc driver. >>>> >>>> The intended coexistence model is: platforms that require the >>>> complete fastrpc feature set continue to use "qcom,fastrpc"; new >>>> platforms where QDA's feature set is sufficient use a QDA-specific >>>> compatible string. New feature development is directed toward QDA. >>>> >>>> The options under consideration are: >>>> >>>> a) Add a new "qcom,qda" compatible string to the existing >>>> qcom,fastrpc.yaml binding, since the DT node structure and >>>> properties are identical. >>> No >>> >>>> >>>> b) Introduce a separate qcom,qda.yaml binding that references or >>>> inherits the fastrpc binding properties. >>> >>> No >>> >>>> >>>> Seeking guidance from DT binding maintainers on the preferred >>>> approach. >>> >>> Grow existing driver. You do not get new driver, you do not get new >>> bindings. >>> >> >> And this was already questioned at v1 (the true v1, not v1+1) but you >> ignored the comment. > The discussion were around compat layers in v1 patch (which is not yet > concluded) and on whether this driver is going to be an alternative or a No, you got comment, from Trilok I think, asking what is the plan in respect of existing fastrpc driver. Best regards, Krzysztof

1 month, 3 weeks

Re: [PATCH v2 03/15] accel/qda: Add initial QDA DRM accelerator driver

by Krzysztof Kozlowski

On 19/08/2026 15:17, Ekansh Gupta wrote: > On 19-08-2026 00:40, Krzysztof Kozlowski wrote: >> On 17/08/2026 06:47, Ekansh Gupta wrote: >>> Add the skeleton of the Qualcomm DSP Accelerator (QDA) driver, a DRM >>> accel driver for the Hexagon DSPs found on Qualcomm SoCs. >>> >>> This patch registers a DRM accel device, exposing a /dev/accel/accelN >>> character device node, and binds it to the RPMsg channel used to reach >>> the DSP. Buffer management, IOMMU context banks and the FastRPC >>> protocol are added by later patches in this series. >>> >>> qda_drv.c / qda_drv.h define the drm_driver ops table, the per-file >>> private state (qda_file_priv) and the main device structure (qda_dev), >>> which embeds drm_device so that it can be recovered with container_of(). >>> >>> qda_rpmsg.c binds to the "qcom,fastrpc" compatible via >>> module_rpmsg_driver(), reads the DSP domain name from the "label" >>> device-tree property, and registers the DRM device. >>> >>> Assisted-by: Claude:claude-sonnet-5 >>> Signed-off-by: Ekansh Gupta <ekansh.gupta(a)oss.qualcomm.com> >>> --- >>> Changes in v2: >>> - Use module_rpmsg_driver() and drop the qda_rpmsg_register()/ >>> _unregister() wrappers, module_init()/module_exit() and >>> qda_rpmsg.h entirely (Dmitry Baryshkov) >>> - Read the "label" property directly into qdev->dsp_name (Dmitry Baryshkov) >>> - Drop the probe/remove/init log messages (Dmitry Baryshkov) >>> - Return the result of qda_register_device() directly (Dmitry Baryshkov) >>> - Clarify the Kconfig help text (Dmitry Baryshkov) >>> --- >>> drivers/accel/Kconfig | 1 + >>> drivers/accel/Makefile | 1 + >>> drivers/accel/qda/Kconfig | 30 ++++++++++++++++ >>> drivers/accel/qda/Makefile | 10 ++++++ >>> drivers/accel/qda/qda_drv.c | 71 ++++++++++++++++++++++++++++++++++++++ >>> drivers/accel/qda/qda_drv.h | 61 +++++++++++++++++++++++++++++++++ >>> drivers/accel/qda/qda_rpmsg.c | 79 +++++++++++++++++++++++++++++++++++++++++++ >>> 7 files changed, 253 insertions(+) >>> >>> diff --git a/drivers/accel/Kconfig b/drivers/accel/Kconfig >>> index bdf48ccafcf2..74ac0f71bc9d 100644 >>> --- a/drivers/accel/Kconfig >>> +++ b/drivers/accel/Kconfig >>> @@ -29,6 +29,7 @@ source "drivers/accel/ethosu/Kconfig" >>> source "drivers/accel/habanalabs/Kconfig" >>> source "drivers/accel/ivpu/Kconfig" >>> source "drivers/accel/qaic/Kconfig" >>> +source "drivers/accel/qda/Kconfig" >>> source "drivers/accel/rocket/Kconfig" >>> >>> endif >>> diff --git a/drivers/accel/Makefile b/drivers/accel/Makefile >>> index 1d3a7251b950..58c08dd5f389 100644 >>> --- a/drivers/accel/Makefile >>> +++ b/drivers/accel/Makefile >>> @@ -5,4 +5,5 @@ obj-$(CONFIG_DRM_ACCEL_ARM_ETHOSU) += ethosu/ >>> obj-$(CONFIG_DRM_ACCEL_HABANALABS) += habanalabs/ >>> obj-$(CONFIG_DRM_ACCEL_IVPU) += ivpu/ >>> obj-$(CONFIG_DRM_ACCEL_QAIC) += qaic/ >>> +obj-$(CONFIG_DRM_ACCEL_QDA) += qda/ >>> obj-$(CONFIG_DRM_ACCEL_ROCKET) += rocket/ >>> \ No newline at end of file >> >> You have trivial patch errors. > newline problem was already there, wasn't introduced as part of this > patch series, so I wasn't sure to fix it here. I can fix this in v3.> >> ... >> >>> +} >>> + >>> +static const struct of_device_id qda_rpmsg_id_table[] = { >>> + { .compatible = "qcom,fastrpc" }, >>> + {}, >> >> Device node with this compatible is already populated, so this looks >> simply wrong or you are adding a duplicated driver. >> >> That's a no-go, you are supposed to work with existing drivers and grow >> them. > I'll bring the discussion again here, there was a discussion to move the > driver to accel subsystem if we want to support new features/uAPI > changes. Please read [1],[2] threads. The intention is to replace > fastrpc driver with QDA eventually. None of them address the problem. You want to grow fastrpc into user of dmabuf? So you move it from misc to here. Again: you have one driver. > > Do you suggest adding a different compatible, say "qcom,qda" for this > driver? How would that even work? How much (or many?) hardware do you have in your SoC? > > Let me know if you have a better suggestion to proceed here. > > [1] https://lkml.org/lkml/2024/6/24/479 > [2] https://lkml.org/lkml/2024/6/21/1252> Please do not ever reference lkml.org, it's long time deprecated and barely working. Use lore. Best regards, Krzysztof

1 month, 3 weeks

Re: [PATCH v2 01/15] MAINTAINERS: Add entry for Qualcomm DSP Accelerator (QDA) driver

by Krzysztof Kozlowski

On 19/08/2026 15:05, Ekansh Gupta wrote: > On 19-08-2026 00:52, Krzysztof Kozlowski wrote: >> On 17/08/2026 06:47, Ekansh Gupta wrote: >>> Add a MAINTAINERS entry for the Qualcomm DSP Accelerator (QDA) driver, >>> covering the driver source under drivers/accel/qda, documentation under >>> Documentation/accel/qda, and the UAPI header >>> include/uapi/drm/qda_accel.h. The linux-arm-msm and dri-devel mailing >>> lists are listed as the relevant review lists. >>> >>> Signed-off-by: Ekansh Gupta <ekansh.gupta(a)oss.qualcomm.com> >>> --- >>> MAINTAINERS | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 912837dd4640..75fa90c137c7 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -22388,6 +22388,17 @@ S: Supported >>> F: Documentation/devicetree/bindings/crypto/qcom-qce.yaml >>> F: drivers/crypto/qce/ >>> >>> +QUALCOMM DSP ACCELERATOR (QDA) DRIVER >>> +M: Ekansh Gupta <ekansh.gupta(a)oss.qualcomm.com> >>> +L: linux-arm-msm(a)vger.kernel.org >>> +L: dri-devel(a)lists.freedesktop.org >>> +S: Supported >>> +T: git https://gitlab.freedesktop.org/drm/misc/kernel.git >>> +F: Documentation/accel/qda/ >> >> There is no such directory. > the directory got added in a later patch, added this patch at the start > following the comment here[1]. > This comment did not tell you to make it non-bisectable. This comment asked you to add that parts in your first patch. Why did you move patches instead of adding this into the first one? Best regards, Krzysztof

1 month, 3 weeks

Re: [PATCH 1/9] dma-buf: add dma_fence_was_initialized function v2

by Philipp Stanner

On Wed, 2026-08-19 at 10:59 +0200, Jiri Slaby wrote: > On 14. 07. 26, 10:43, Philipp Stanner wrote: > > On Tue, 2026-07-14 at 09:53 +0200, Philipp Stanner wrote: > > > On Mon, 2026-07-13 at 10:58 +0200, Jiri Slaby wrote: > > > > Hi, > > > > > > > > On 20. 01. 26, 11:54, Christian König wrote: > > > > > Some driver use fence->ops to test if a fence was initialized or not. > > > > > The problem is that this utilizes internal behavior of the dma_fence > > > > > implementation. > > > > > > > > > > So better abstract that into a function. > > > > > > > > > > v2: use a flag instead of testing fence->ops, rename the function, move > > > > > to the beginning of the patch set. > > > > ... > > > > > --- a/drivers/gpu/drm/qxl/qxl_release.c > > > > > +++ b/drivers/gpu/drm/qxl/qxl_release.c > > > > > @@ -146,7 +146,7 @@ qxl_release_free(struct qxl_device *qdev, > > > > > idr_remove(&qdev->release_idr, release->id); > > > > > spin_unlock(&qdev->release_idr_lock); > > > > > > > > > > - if (release->base.ops) { > > > > > + if (dma_fence_was_initialized(&release->base)) { > > > > > > Could you verify the cause with sth like > > > > > > if (release->base.ops && dma_fence_was_initialized(…)) { > > > > Forget about that, probably would not work or cause other issues > > because the NULL-setting on signal(). > > > > I'd then probably try to verify it with a separate boolean in struct > > release. Though I also don't get why the fence-initialized check does > > not do the trick. > > Hi, have you come up with something yet? Well, I was more like suggesting this as a debug option [to you] :D Regardless, looking at the code again, I would say that this might be a race, but I don't know enough about QXL to say for sure. dma_fence_init() is (of course) not ordered: static void __dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, spinlock_t *lock, u64 context, u64 seqno, unsigned long flags) { BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name); kref_init(&fence->refcount); /* * While it is counter intuitive to protect a constant function pointer * table by RCU it allows modules to wait for an RCU grace period * before they unload, to make sure that nobody is executing their * functions any more. */ RCU_INIT_POINTER(fence->ops, ops); INIT_LIST_HEAD(&fence->cb_list); fence->context = context; fence->seqno = seqno; fence->flags = flags | BIT(DMA_FENCE_FLAG_INITIALIZED_BIT); (Should this maybe be set_bit() btw?) The fact that QXL could run into qxl_release_free() with an uninitialized fence hints at the fact that this might race, so DMA_FENCE_FLAG_INITIALIZED_BIT could be set / read before kref_init() ran. Maybe one way to verify / debug that would be to move spin_unlock(&qdev->release_idr_lock) downwards so it also guards dma_fence_was_initialized(), and also lock the initialization of the fence (in qxl_release_fence_buffer_objects() ?) with said lock. If that's possible. Just brainstorming a bit for ways how to debug. QXL does a few tricky things with the release->base.ops pointer. qxl_release_alloc() sets it to NULL, and only qxl_release_fence_buffer_objects() then actually sets it. So this could be the race? Setting of the ops pointer got replaced by setting of the fence-flag. P.

1 month, 3 weeks

← Newer
1
2
3
4
5
6
7
8
9
Older →

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig August 2026