All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: "Ekansh Gupta" <ekansh.gupta@oss.qualcomm.com>,
	"Oded Gabbay" <ogabbay@kernel.org>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Shuah Khan" <skhan@linuxfoundation.org>,
	"Randy Dunlap" <rdunlap@infradead.org>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nick Desaulniers" <ndesaulniers@google.com>,
	"Bill Wendling" <morbo@google.com>,
	"Justin Stitt" <justinstitt@google.com>,
	"Joerg Roedel (AMD)" <joro@8bytes.org>,
	"Will Deacon" <will@kernel.org>,
	"Robin Murphy" <robin.murphy@arm.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Christian König" <christian.koenig@amd.com>
Cc: Bharath Kumar <quic_bkumar@quicinc.com>,
	Chenna Kesava Raju <quic_chennak@quicinc.com>,
	srinivas.kandagatla@oss.qualcomm.com,
	dmitry.baryshkov@oss.qualcomm.com, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linux-doc@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, llvm@lists.linux.dev,
	iommu@lists.linux.dev, linux-media@vger.kernel.org,
	linaro-mm-sig@lists.linaro.org
Subject: Re: [PATCH v2 00/15] accel/qda: Qualcomm DSP Accelerator driver
Date: Tue, 18 Aug 2026 21:21:47 +0200	[thread overview]
Message-ID: <3b235b4c-0c1c-400a-b081-0df881cab06d@kernel.org> (raw)
In-Reply-To: <3322b42b-3755-45ec-ad55-345125f0d488@kernel.org>

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.qualcomm.com/
>> RFC: https://lore.kernel.org/dri-devel/20260224-qda-firstpost-v1-0-fe46a9c1a046@oss.qualcomm.com/T/
>>
>> 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.qualcomm.com/
>>
>> 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.

Great, so here goes away trust.

NAK

Best regards,
Krzysztof

  reply	other threads:[~2026-08-18 19:22 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17  4:47 [PATCH v2 00/15] accel/qda: Qualcomm DSP Accelerator driver Ekansh Gupta
2026-08-17  4:47 ` [PATCH v2 01/15] MAINTAINERS: Add entry for Qualcomm DSP Accelerator (QDA) driver Ekansh Gupta
2026-08-18 19:22   ` Krzysztof Kozlowski
2026-08-17  4:47 ` [PATCH v2 02/15] accel/qda: Add QDA driver documentation Ekansh Gupta
2026-08-17  4:47 ` [PATCH v2 03/15] accel/qda: Add initial QDA DRM accelerator driver Ekansh Gupta
2026-08-18 19:10   ` Krzysztof Kozlowski
2026-08-17  4:47 ` [PATCH v2 04/15] accel/qda: Add compute bus for QDA context banks Ekansh Gupta
2026-08-17  4:47 ` [PATCH v2 05/15] iommu: Add QDA compute context bank bus to iommu_buses Ekansh Gupta
2026-08-17  7:01   ` Joerg Roedel (AMD)
2026-08-17 13:44   ` Jason Gunthorpe
2026-08-18  5:10     ` Ekansh Gupta
2026-08-18 13:07       ` Jason Gunthorpe
2026-08-17  4:47 ` [PATCH v2 06/15] accel/qda: Create compute context bank devices on QDA compute bus Ekansh Gupta
2026-08-17  4:47 ` [PATCH v2 07/15] accel/qda: Add memory manager for CB devices Ekansh Gupta
2026-08-17  6:52   ` Dmitry Baryshkov
2026-08-18  4:33     ` Ekansh Gupta
2026-08-18  4:44       ` Dmitry Baryshkov
2026-08-17  4:47 ` [PATCH v2 08/15] accel/qda: Add QUERY IOCTL and QDA UAPI header Ekansh Gupta
2026-08-17  6:58   ` Dmitry Baryshkov
2026-08-18  4:40     ` Ekansh Gupta
2026-08-18  4:45       ` Dmitry Baryshkov
2026-08-17  4:47 ` [PATCH v2 09/15] accel/qda: Add DMA-backed GEM objects and memory manager integration Ekansh Gupta
2026-08-18  3:42   ` Dmitry Baryshkov
2026-08-18  5:51     ` Ekansh Gupta
2026-08-18  3:48   ` Dmitry Baryshkov
2026-08-18  5:52     ` Ekansh Gupta
2026-08-17  4:47 ` [PATCH v2 10/15] accel/qda: Add GEM_CREATE and GEM_MMAP_OFFSET IOCTLs Ekansh Gupta
2026-08-18  3:49   ` Dmitry Baryshkov
2026-08-18  5:54     ` Ekansh Gupta
2026-08-17  4:47 ` [PATCH v2 11/15] accel/qda: Add PRIME DMA-BUF import support Ekansh Gupta
2026-08-18  3:55   ` Dmitry Baryshkov
2026-08-18  6:25     ` Ekansh Gupta
2026-08-17  4:47 ` [PATCH v2 12/15] accel/qda: Add FastRPC invocation support Ekansh Gupta
2026-08-18  4:19   ` Dmitry Baryshkov
2026-08-17  4:47 ` [PATCH v2 13/15] accel/qda: Add DSP process creation and release Ekansh Gupta
2026-08-17  4:47 ` [PATCH v2 14/15] accel/qda: Add remote memory mapping to DSP address space Ekansh Gupta
2026-08-18 19:13 ` [PATCH v2 00/15] accel/qda: Qualcomm DSP Accelerator driver Krzysztof Kozlowski
2026-08-18 19:21   ` Krzysztof Kozlowski [this message]
2026-08-18 19:18 ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3b235b4c-0c1c-400a-b081-0df881cab06d@kernel.org \
    --to=krzk@kernel.org \
    --cc=airlied@gmail.com \
    --cc=christian.koenig@amd.com \
    --cc=corbet@lwn.net \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ekansh.gupta@oss.qualcomm.com \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=justinstitt@google.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=morbo@google.com \
    --cc=mripard@kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=ogabbay@kernel.org \
    --cc=quic_bkumar@quicinc.com \
    --cc=quic_chennak@quicinc.com \
    --cc=rdunlap@infradead.org \
    --cc=robin.murphy@arm.com \
    --cc=simona@ffwll.ch \
    --cc=skhan@linuxfoundation.org \
    --cc=srinivas.kandagatla@oss.qualcomm.com \
    --cc=sumit.semwal@linaro.org \
    --cc=tzimmermann@suse.de \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.