Linux Media Controller development
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox