* [PATCH 1/6] accel/qaic: Centralize execution info
2026-10-03 2:38 [PATCH 0/6] accel/qaic: Use DRM Scheduler for submitting BO reqs Carl Vanderlip
@ 2026-10-03 2:38 ` Carl Vanderlip
2026-10-03 2:49 ` sashiko-bot
2026-10-03 2:38 ` [PATCH 2/6] accel/qaic: Replace transfer completion with dma-fence Carl Vanderlip
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Carl Vanderlip @ 2026-10-03 2:38 UTC (permalink / raw)
To: jeff.hugo, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel,
Sashiko, Pranjal Ramajor Asha Kanojiya
Move per-exec ioctl information into common data structure in
preparation for adding dma-resv fences which require all BO's associated
with a fence to be locked with drm_gem_lock_reservations().
Co-developed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Signed-off-by: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
---
drivers/accel/qaic/qaic_data.c | 261 ++++++++++++++++++++++++---------
1 file changed, 190 insertions(+), 71 deletions(-)
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 4d55531bf1c9..64bc3cc50d1f 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -143,6 +143,47 @@ struct dbc_rsp {
__le16 status;
} __packed;
+/* Common data structure for all the variants of execute IOCTL */
+struct execute_info {
+ /* Number of entries present in arrays */
+ u32 count;
+ struct {
+ u64 received_ts;
+ u64 submit_ts;
+ u32 queue_level;
+ } perf_stats;
+ /* Array of pointers to BOs for this execution */
+ struct qaic_bo **bo_arr;
+ /* Array of handles to above BOs, helpful for identifying failures */
+ u32 *handle_arr;
+ /* Array of new sizes for BO transfer if request is_partial */
+ u64 *resize_arr;
+ /* True if execution is of resized BO */
+ bool is_partial;
+};
+
+/* Helper structure for retrieving info about execution entries */
+struct qaic_exec_data_ent {
+ /* Pointer to requested gem object */
+ struct drm_gem_object *obj;
+ /* New size for BO transfer if request is partial execute */
+ u64 resize;
+ /* Per-file handle for BO, helpful for identifying failures */
+ u32 handle;
+};
+
+/* Helper structure for requesting info about execution entries */
+struct qaic_exec_data_req {
+ /* Pointer to drm file */
+ struct drm_file *drm_file;
+ /* Pointer to beginning of array of execution entries from user */
+ void *exec_data;
+ /* Is exec_data an array of partial execution entries? */
+ bool is_partial;
+ /* Index of exec_data array this request is acting upon */
+ int idx;
+};
+
static inline bool bo_queued(struct qaic_bo *bo)
{
return !list_empty(&bo->xfer_list);
@@ -1222,41 +1263,43 @@ static inline int copy_partial_exec_reqs(struct qaic_device *qdev, struct bo_sli
}
static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *file_priv,
- struct qaic_execute_entry *exec, unsigned int count,
- bool is_partial, struct dma_bridge_chan *dbc, u32 head,
- u32 *tail)
+ struct execute_info *exec, struct dma_bridge_chan *dbc,
+ u32 head, u32 *tail)
{
- struct qaic_partial_execute_entry *pexec = (struct qaic_partial_execute_entry *)exec;
- struct drm_gem_object *obj;
+ struct ww_acquire_ctx acquire_ctx;
struct bo_slice *slice;
unsigned long flags;
struct qaic_bo *bo;
+ u64 resize;
+ u32 handle;
int i, j;
int ret;
- for (i = 0; i < count; i++) {
- /*
- * ref count will be decremented when the transfer of this
- * buffer is complete. It is inside dbc_irq_threaded_fn().
- */
- obj = drm_gem_object_lookup(file_priv,
- is_partial ? pexec[i].handle : exec[i].handle);
- if (!obj) {
- ret = -ENOENT;
- goto failed_to_send_bo;
- }
+ ret = drm_gem_lock_reservations((struct drm_gem_object **)exec->bo_arr,
+ exec->count, &acquire_ctx);
+ if (ret)
+ return ret;
- bo = to_qaic_bo(obj);
+ for (i = 0; i < exec->count; i++) {
+ resize = exec->is_partial ? exec->resize_arr[i] : 0;
+ bo = exec->bo_arr[i];
+ handle = exec->handle_arr[i];
ret = mutex_lock_interruptible(&bo->lock);
if (ret)
goto failed_to_send_bo;
+ /*
+ * Take reference to object before sending to device,
+ * released when transfer is complete (in dbc_irq_threaded_fn()).
+ */
+ drm_gem_object_get(&bo->base);
+
if (!bo->sliced) {
ret = -EINVAL;
goto unlock_bo;
}
- if (is_partial && pexec[i].resize > bo->base.size) {
+ if (exec->is_partial && resize > bo->base.size) {
ret = -EINVAL;
goto unlock_bo;
}
@@ -1274,13 +1317,13 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
for (j = 0; j < slice->nents; j++)
slice->reqs[j].req_id = cpu_to_le16(bo->req_id);
- if (is_partial && (!pexec[i].resize || pexec[i].resize <= slice->offset))
+ if (exec->is_partial && (!resize || resize <= slice->offset))
/* Configure the slice for no DMA transfer */
ret = copy_partial_exec_reqs(qdev, slice, 0, dbc, head, tail);
- else if (is_partial && pexec[i].resize < slice->offset + slice->size)
+ else if (exec->is_partial && resize < slice->offset + slice->size)
/* Configure the slice to be partially DMA transferred */
ret = copy_partial_exec_reqs(qdev, slice,
- pexec[i].resize - slice->offset, dbc,
+ resize - slice->offset, dbc,
head, tail);
else
ret = copy_exec_reqs(qdev, slice, dbc->id, head, tail);
@@ -1296,82 +1339,124 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
mutex_unlock(&bo->lock);
}
- return 0;
+ goto unlock_resv;
unlock_bo:
+ drm_gem_object_put(&bo->base);
mutex_unlock(&bo->lock);
failed_to_send_bo:
- if (likely(obj))
- drm_gem_object_put(obj);
for (j = 0; j < i; j++) {
+ drm_gem_object_put(&exec->bo_arr[j]->base);
spin_lock_irqsave(&dbc->xfer_lock, flags);
bo = list_last_entry(&dbc->xfer_list, struct qaic_bo, xfer_list);
- obj = &bo->base;
list_del_init(&bo->xfer_list);
spin_unlock_irqrestore(&dbc->xfer_lock, flags);
dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, bo->dir);
- drm_gem_object_put(obj);
}
+unlock_resv:
+ drm_gem_unlock_reservations((struct drm_gem_object **)exec->bo_arr,
+ exec->count, &acquire_ctx);
return ret;
}
-static void update_profiling_data(struct drm_file *file_priv,
- struct qaic_execute_entry *exec, unsigned int count,
- bool is_partial, u64 received_ts, u64 submit_ts, u32 queue_level)
+static void update_profiling_data(struct execute_info *exec)
{
- struct qaic_partial_execute_entry *pexec = (struct qaic_partial_execute_entry *)exec;
- struct drm_gem_object *obj;
+ u32 queue_level = exec->perf_stats.queue_level;
struct qaic_bo *bo;
int i;
- for (i = 0; i < count; i++) {
- /*
- * Since we already committed the BO to hardware, the only way
- * this should fail is a pending signal. We can't cancel the
- * submit to hardware, so we have to just skip the profiling
- * data. In case the signal is not fatal to the process, we
- * return success so that the user doesn't try to resubmit.
- */
- obj = drm_gem_object_lookup(file_priv,
- is_partial ? pexec[i].handle : exec[i].handle);
- if (!obj)
- break;
- bo = to_qaic_bo(obj);
- bo->perf_stats.req_received_ts = received_ts;
- bo->perf_stats.req_submit_ts = submit_ts;
+ for (i = 0; i < exec->count; i++) {
+ bo = exec->bo_arr[i];
+ bo->perf_stats.req_received_ts = exec->perf_stats.received_ts;
+ bo->perf_stats.req_submit_ts = exec->perf_stats.submit_ts;
bo->perf_stats.queue_level_before = queue_level;
queue_level += bo->total_slice_nents;
- drm_gem_object_put(obj);
}
}
+static int exec_index_to_gem_info(struct qaic_exec_data_req req, struct qaic_exec_data_ent *ent)
+{
+ int idx = req.idx;
+ int ret = 0;
+ u32 handle;
+
+ if (req.is_partial) {
+ struct qaic_partial_execute_entry *exec_ent_part = req.exec_data;
+
+ handle = exec_ent_part[idx].handle;
+ ent->resize = exec_ent_part[idx].resize;
+ } else {
+ struct qaic_execute_entry *exec_ent = req.exec_data;
+
+ handle = exec_ent[idx].handle;
+ }
+
+ ent->obj = drm_gem_object_lookup(req.drm_file, handle);
+ if (!ent->obj)
+ ret = -ENOENT;
+
+ ent->handle = handle;
+
+ return ret;
+}
+
+static int lookup_exec_data(struct drm_file *file_priv, void *exec_data,
+ struct execute_info *exec)
+{
+ struct qaic_exec_data_req req;
+ struct qaic_exec_data_ent ent;
+ int i, ret = 0;
+
+ req.drm_file = file_priv;
+ req.exec_data = exec_data;
+ req.is_partial = exec->is_partial;
+
+ for (i = 0; i < exec->count; i++) {
+ req.idx = i;
+ ret = exec_index_to_gem_info(req, &ent);
+ if (ret)
+ goto put_obj;
+
+ exec->bo_arr[i] = to_qaic_bo(ent.obj);
+ exec->handle_arr[i] = ent.handle;
+ if (exec->resize_arr)
+ exec->resize_arr[i] = ent.resize;
+ }
+
+ return ret;
+put_obj:
+ for (i--; i >= 0; i--)
+ drm_gem_object_put(&exec->bo_arr[i]->base);
+ return ret;
+}
+
static int __qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv,
bool is_partial)
{
+ size_t usr_ent_size = is_partial ? sizeof(struct qaic_partial_execute_entry) :
+ sizeof(struct qaic_execute_entry);
+ int usr_rcu_id, qdev_rcu_id, ch_rcu_id;
struct qaic_execute *args = data;
- struct qaic_execute_entry *exec;
struct dma_bridge_chan *dbc;
- int usr_rcu_id, qdev_rcu_id;
struct qaic_device *qdev;
+ struct execute_info exec;
struct qaic_user *usr;
- u64 received_ts;
- u32 queue_level;
- u64 submit_ts;
- int rcu_id;
- u32 head;
- u32 tail;
- u64 size;
- int ret;
+ size_t elem_size_sum;
+ void *usr_exec_ent;
+ u32 head, tail;
+ int i, ret;
- received_ts = ktime_get_ns();
+ exec.perf_stats.received_ts = ktime_get_ns();
+ exec.is_partial = is_partial;
- size = is_partial ? sizeof(struct qaic_partial_execute_entry) : sizeof(*exec);
if (args->hdr.count == 0)
return -EINVAL;
- exec = memdup_array_user(u64_to_user_ptr(args->data), args->hdr.count, size);
- if (IS_ERR(exec))
- return PTR_ERR(exec);
+ exec.count = args->hdr.count;
+
+ usr_exec_ent = memdup_array_user(u64_to_user_ptr(args->data), exec.count, usr_ent_size);
+ if (IS_ERR(usr_exec_ent))
+ return PTR_ERR(usr_exec_ent);
usr = file_priv->driver_priv;
usr_rcu_id = srcu_read_lock(&usr->qddev_lock);
@@ -1394,7 +1479,39 @@ static int __qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct dr
dbc = &qdev->dbc[args->hdr.dbc_id];
- rcu_id = srcu_read_lock(&dbc->ch_lock);
+ /*
+ * Going to allocate one large array, and then create pointers to
+ * within it for sub-arrays that all have exec.count elements in them.
+ *
+ * bo_arr is an array of pointers to struct qaic_bo, but is used as an
+ * argument for drm_gem_lock_reservations().
+ *
+ * handle_arr is an array of handles to each drm_gem_object embedded in
+ * the respective bo_arr element. It is used to help debugging by
+ * identifying particular buffers.
+ *
+ * resize_arr is an array of u64 sizes for partial executions. It is
+ * only defined if a partial execution is underway.
+ */
+ elem_size_sum = sizeof(*exec.bo_arr) + sizeof(*exec.handle_arr);
+ elem_size_sum += is_partial ? sizeof(*exec.resize_arr) : 0;
+ exec.bo_arr = kcalloc(exec.count, elem_size_sum, GFP_KERNEL);
+ if (!exec.bo_arr) {
+ ret = -ENOMEM;
+ goto unlock_dev_srcu;
+ }
+ exec.handle_arr = ((void *)exec.bo_arr) + (sizeof(*exec.bo_arr) * exec.count);
+ if (is_partial)
+ exec.resize_arr = ((void *)exec.bo_arr) +
+ ((sizeof(*exec.bo_arr) + sizeof(*exec.handle_arr)) * exec.count);
+ else
+ exec.resize_arr = NULL;
+
+ ret = lookup_exec_data(file_priv, usr_exec_ent, &exec);
+ if (ret)
+ goto free_bo_arr;
+
+ ch_rcu_id = srcu_read_lock(&dbc->ch_lock);
if (!dbc->usr || dbc->usr->handle != usr->handle) {
ret = -EPERM;
goto release_ch_rcu;
@@ -1418,20 +1535,18 @@ static int __qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct dr
goto unlock_req_lock;
}
- queue_level = head <= tail ? tail - head : dbc->nelem - (head - tail);
+ exec.perf_stats.queue_level = head <= tail ? tail - head : dbc->nelem - (head - tail);
- ret = send_bo_list_to_device(qdev, file_priv, exec, args->hdr.count, is_partial, dbc,
- head, &tail);
+ ret = send_bo_list_to_device(qdev, file_priv, &exec, dbc, head, &tail);
if (ret)
goto unlock_req_lock;
/* Finalize commit to hardware */
- submit_ts = ktime_get_ns();
+ exec.perf_stats.submit_ts = ktime_get_ns();
writel(tail, dbc->dbc_base + REQTP_OFF);
mutex_unlock(&dbc->req_lock);
- update_profiling_data(file_priv, exec, args->hdr.count, is_partial, received_ts,
- submit_ts, queue_level);
+ update_profiling_data(&exec);
if (datapath_polling)
schedule_work(&dbc->poll_work);
@@ -1440,12 +1555,16 @@ static int __qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct dr
if (ret)
mutex_unlock(&dbc->req_lock);
release_ch_rcu:
- srcu_read_unlock(&dbc->ch_lock, rcu_id);
+ srcu_read_unlock(&dbc->ch_lock, ch_rcu_id);
+ for (i = 0; i < exec.count; i++)
+ drm_gem_object_put(&exec.bo_arr[i]->base);
+free_bo_arr:
+ kfree(exec.bo_arr);
unlock_dev_srcu:
srcu_read_unlock(&qdev->dev_lock, qdev_rcu_id);
unlock_usr_srcu:
srcu_read_unlock(&usr->qddev_lock, usr_rcu_id);
- kfree(exec);
+ kfree(usr_exec_ent);
return ret;
}
base-commit: 0915fb19e08f7f14a03df78c67d081bbfa8ff1fa
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/6] accel/qaic: Centralize execution info
2026-10-03 2:38 ` [PATCH 1/6] accel/qaic: Centralize execution info Carl Vanderlip
@ 2026-10-03 2:49 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-10-03 2:49 UTC (permalink / raw)
To: Carl Vanderlip; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] mm: unaligned memory access in exec.resize_arr array allocation
- [Medium] drm/qaic: failure to deduplicate resv pointers causes -EALREADY
--
commit 564e37a5c5a191292a9c140ea89ad4c33346feeb
Author: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
accel/qaic: Centralize execution info
This commit moves per-exec ioctl information into a common data structure
to prepare for adding dma-resv fences, which require all buffer objects
associated with a fence to be locked using drm_gem_lock_reservations.
[Severity: Medium]
In drivers/accel/qaic/qaic_data.c, the function __qaic_execute_bo_ioctl
calculates the offset for exec.resize_arr around line 1502. The driver
allocates a single buffer for three arrays: bo_arr at 8 bytes per element,
handle_arr at 4 bytes per element, and resize_arr at 8 bytes per element.
The offset for resize_arr is calculated as (8 + 4) * count.
If count is an odd number, for example 1, the byte offset becomes an odd
multiple of 12, resulting in a 4-byte aligned pointer. When this is later
written to in lookup_exec_data around line 1426, it performs an unaligned
64-bit memory access.
Can this lead to alignment faults on platforms with strict alignment
enforcement, or trigger undefined C behavior? Would reordering the array
layout or padding the calculation prevent this regression?
[Severity: Medium]
Also in drivers/accel/qaic/qaic_data.c, the function send_bo_list_to_device
calls drm_gem_lock_reservations directly on the raw, non-deduplicated
exec->bo_arr array around line 1280.
What happens if userspace submits distinct imported buffer objects that
share the same dma_resv pointer, such as from identical or shared dma-bufs?
It appears this attempts to lock the same reservation multiple times within
the same ww_acquire_ctx, causing the underlying ww_mutex to return -EALREADY
and fail the ioctl.
Since the driver previously used a per-BO mutex for locking that allowed
executing distinct BOs with shared reservations, does this code need to
deduplicate the reservation pointers before passing them to the locking
helper to avoid this regression?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261003023818.1161666-1-carl.vanderlip@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/6] accel/qaic: Replace transfer completion with dma-fence
2026-10-03 2:38 [PATCH 0/6] accel/qaic: Use DRM Scheduler for submitting BO reqs Carl Vanderlip
2026-10-03 2:38 ` [PATCH 1/6] accel/qaic: Centralize execution info Carl Vanderlip
@ 2026-10-03 2:38 ` Carl Vanderlip
2026-10-03 2:54 ` sashiko-bot
2026-10-03 2:38 ` [PATCH 3/6] accel/qaic: Isolate hw submission Carl Vanderlip
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Carl Vanderlip @ 2026-10-03 2:38 UTC (permalink / raw)
To: jeff.hugo, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel,
Sashiko, Pranjal Ramajor Asha Kanojiya
Currently, finished irq work is signaled by a completion. This is
lightweight but doesn't integrate with other kernel buffer
synchronization.
Using dma-fences instead of completions allows for the fences to be
added to their respective DMA-BUFs which enables polling on the buffer
instead of relying on the ~_WAIT_BO IOCTL for synchronization.
This is also useful in preparation for the addition of drm_sched
support. Fences can be linked together to create more complex
synchronization structures.
Co-developed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Signed-off-by: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
---
drivers/accel/qaic/Makefile | 1 +
drivers/accel/qaic/qaic.h | 26 +++++-
drivers/accel/qaic/qaic_data.c | 84 +++++++++++--------
drivers/accel/qaic/qaic_drv.c | 2 +-
drivers/accel/qaic/qaic_fence.c | 144 ++++++++++++++++++++++++++++++++
5 files changed, 220 insertions(+), 37 deletions(-)
create mode 100644 drivers/accel/qaic/qaic_fence.c
diff --git a/drivers/accel/qaic/Makefile b/drivers/accel/qaic/Makefile
index 71f727b74da3..90582b0693da 100644
--- a/drivers/accel/qaic/Makefile
+++ b/drivers/accel/qaic/Makefile
@@ -10,6 +10,7 @@ qaic-y := \
qaic_control.o \
qaic_data.o \
qaic_drv.o \
+ qaic_fence.o \
qaic_ras.o \
qaic_ssr.o \
qaic_sysfs.o \
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 02ca99000e2a..4cdd6579aa89 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -6,6 +6,7 @@
#ifndef _QAIC_H_
#define _QAIC_H_
+#include <linux/dma-fence.h>
#include <linux/interrupt.h>
#include <linux/kref.h>
#include <linux/mhi.h>
@@ -18,9 +19,11 @@
#include <drm/drm_device.h>
#include <drm/drm_gem.h>
+#define QAIC_NAME "qaic"
#define QAIC_DBC_BASE SZ_128K
#define QAIC_DBC_SIZE SZ_4K
#define QAIC_SSR_DBC_SENTINEL U32_MAX /* No ongoing SSR sentinel */
+#define QAIC_TIMELINE_LEN 32 /* Should fit max sized DBC id/BO handle */
#define QAIC_NO_PARTITION -1
@@ -227,6 +230,16 @@ struct qaic_drm_device {
void *sysfs_attrs;
};
+struct qaic_fence {
+ struct dma_fence base;
+ /* Spinlock used for initializing this fence */
+ spinlock_t lock;
+ /* Name of fence context */
+ char timeline_name[QAIC_TIMELINE_LEN];
+ /* Cached value of dbc_id for use in populating timeline_name */
+ u32 dbc_id;
+};
+
struct qaic_bo {
struct drm_gem_object base;
/* Scatter/gather table for allocate/imported BO */
@@ -253,8 +266,10 @@ struct qaic_bo {
bool sliced;
/* Request ID of this BO if it is queued for execution */
u16 req_id;
- /* Wait on this for completion of DMA transfer of this BO */
- struct completion xfer_done;
+ /* Wait on this fence for DMA transfer of this BO */
+ struct qaic_fence *fence;
+ /* Current fence context for this BO */
+ u64 fence_context;
/*
* Node in linked list where head is dbc->xfer_list.
* This link list contain BO's that are queued for DMA transfer.
@@ -290,6 +305,8 @@ struct qaic_bo {
} perf_stats;
/* Synchronizes BO operations */
struct mutex lock;
+ /* Callback to sync BO with CPU */
+ struct dma_fence_cb cb;
};
struct bo_slice {
@@ -351,6 +368,11 @@ void qaic_irq_polling_work(struct work_struct *work);
void qaic_dbc_enter_ssr(struct qaic_device *qdev, u32 dbc_id);
void qaic_dbc_exit_ssr(struct qaic_device *qdev);
+/* qaic_fence.c */
+int qaic_fence_wait(struct dma_fence *fence, signed long timeout);
+void qaic_remove_bo_fence(struct qaic_bo *bo);
+int qaic_acquire_bo_fence(struct qaic_bo *bo);
+
/* qaic_sysfs.c */
int qaic_sysfs_init(struct qaic_drm_device *qddev);
void qaic_sysfs_remove(struct qaic_drm_device *qddev);
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 64bc3cc50d1f..3326d9da0e1e 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -184,9 +184,12 @@ struct qaic_exec_data_req {
int idx;
};
+/* Caller should be holding bo->lock */
static inline bool bo_queued(struct qaic_bo *bo)
{
- return !list_empty(&bo->xfer_list);
+ if (bo->fence)
+ return !dma_fence_is_signaled(&bo->fence->base);
+ return false;
}
inline int get_dbc_req_elem_size(void)
@@ -745,16 +748,8 @@ static const struct drm_gem_object_funcs qaic_gem_funcs = {
.vm_ops = &drm_vm_ops,
};
-static void qaic_init_bo(struct qaic_bo *bo, bool reinit)
+static void qaic_init_bo_lists(struct qaic_bo *bo)
{
- if (reinit) {
- bo->sliced = false;
- reinit_completion(&bo->xfer_done);
- } else {
- mutex_init(&bo->lock);
- init_completion(&bo->xfer_done);
- }
- complete_all(&bo->xfer_done);
INIT_LIST_HEAD(&bo->slices);
INIT_LIST_HEAD(&bo->xfer_list);
}
@@ -767,7 +762,9 @@ static struct qaic_bo *qaic_alloc_init_bo(void)
if (!bo)
return ERR_PTR(-ENOMEM);
- qaic_init_bo(bo, false);
+ mutex_init(&bo->lock);
+ bo->fence_context = dma_fence_context_alloc(1);
+ qaic_init_bo_lists(bo);
return bo;
}
@@ -996,6 +993,7 @@ static void qaic_unprepare_bo(struct qaic_device *qdev, struct qaic_bo *bo)
bo->dir = 0;
bo->dbc = NULL;
bo->nr_slice = 0;
+ bo->sliced = false;
}
static void qaic_free_slices_bo(struct qaic_bo *bo)
@@ -1304,9 +1302,7 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
goto unlock_bo;
}
- spin_lock_irqsave(&dbc->xfer_lock, flags);
if (bo_queued(bo)) {
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
ret = -EINVAL;
goto unlock_bo;
}
@@ -1327,12 +1323,15 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
head, tail);
else
ret = copy_exec_reqs(qdev, slice, dbc->id, head, tail);
- if (ret) {
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+ if (ret)
goto unlock_bo;
- }
}
- reinit_completion(&bo->xfer_done);
+
+ ret = qaic_acquire_bo_fence(bo);
+ if (ret)
+ goto unlock_bo;
+
+ spin_lock_irqsave(&dbc->xfer_lock, flags);
list_add_tail(&bo->xfer_list, &dbc->xfer_list);
spin_unlock_irqrestore(&dbc->xfer_lock, flags);
dma_sync_sgtable_for_device(&qdev->pdev->dev, bo->sgt, bo->dir);
@@ -1787,11 +1786,13 @@ irqreturn_t dbc_irq_threaded_fn(int irq, void *data)
* At this point we have received all the interrupts for
* BO, which means BO execution is complete.
*/
- dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, bo->dir);
bo->nr_slice_xfer_done = 0;
list_del_init(&bo->xfer_list);
- bo->perf_stats.req_processed_ts = ktime_get_ns();
- complete_all(&bo->xfer_done);
+ /*
+ * fence is put when it is executed next, during xfer_list
+ * cleanup, or when slice is detached
+ */
+ dma_fence_signal(&bo->fence->base);
drm_gem_object_put(&bo->base);
break;
}
@@ -1840,6 +1841,7 @@ int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file
struct dma_bridge_chan *dbc;
struct drm_gem_object *obj;
struct qaic_device *qdev;
+ struct dma_fence *fence;
unsigned long timeout;
struct qaic_user *usr;
struct qaic_bo *bo;
@@ -1881,6 +1883,9 @@ int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file
goto unlock_ch_srcu;
}
+ timeout = args->timeout ? args->timeout : wait_exec_default_timeout_ms;
+ timeout = msecs_to_jiffies(timeout);
+
obj = drm_gem_object_lookup(file_priv, args->handle);
if (!obj) {
ret = -ENOENT;
@@ -1888,15 +1893,24 @@ int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file
}
bo = to_qaic_bo(obj);
- timeout = args->timeout ? args->timeout : wait_exec_default_timeout_ms;
- timeout = msecs_to_jiffies(timeout);
- ret = wait_for_completion_interruptible_timeout(&bo->xfer_done, timeout);
- if (!ret) {
- ret = -ETIMEDOUT;
+ ret = mutex_lock_interruptible(&bo->lock);
+ if (ret)
+ goto put_obj;
+
+ if (!bo->fence) {
+ ret = -ENOENT;
+ mutex_unlock(&bo->lock);
goto put_obj;
}
- if (ret > 0)
- ret = 0;
+
+ fence = &bo->fence->base;
+ dma_fence_get(fence);
+ mutex_unlock(&bo->lock);
+
+ ret = qaic_fence_wait(fence, timeout);
+ dma_fence_put(fence);
+ if (ret)
+ goto put_obj;
if (!dbc->usr)
ret = -EPERM;
@@ -2000,9 +2014,10 @@ int qaic_perf_stats_bo_ioctl(struct drm_device *dev, void *data, struct drm_file
static void detach_slice_bo(struct qaic_device *qdev, struct qaic_bo *bo)
{
+ qaic_remove_bo_fence(bo);
qaic_free_slices_bo(bo);
qaic_unprepare_bo(qdev, bo);
- qaic_init_bo(bo, true);
+ qaic_init_bo_lists(bo);
list_del(&bo->bo_list);
drm_gem_object_put(&bo->base);
}
@@ -2015,7 +2030,6 @@ int qaic_detach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
struct drm_gem_object *obj;
struct qaic_device *qdev;
struct qaic_user *usr;
- unsigned long flags;
struct qaic_bo *bo;
int ret;
@@ -2060,13 +2074,10 @@ int qaic_detach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
}
/* Check if BO is committed to H/W for DMA */
- spin_lock_irqsave(&dbc->xfer_lock, flags);
if (bo_queued(bo)) {
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
ret = -EBUSY;
goto unlock_ch_srcu;
}
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
detach_slice_bo(qdev, bo);
@@ -2093,14 +2104,19 @@ static void empty_xfer_list(struct qaic_device *qdev, struct dma_bridge_chan *db
bo = list_first_entry(&dbc->xfer_list, typeof(*bo), xfer_list);
list_del_init(&bo->xfer_list);
spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+ mutex_lock(&bo->lock);
bo->nr_slice_xfer_done = 0;
bo->req_id = 0;
bo->perf_stats.req_received_ts = 0;
bo->perf_stats.req_submit_ts = 0;
bo->perf_stats.req_processed_ts = 0;
bo->perf_stats.queue_level_before = 0;
- dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, bo->dir);
- complete_all(&bo->xfer_done);
+ dma_fence_get(&bo->fence->base);
+ dma_fence_set_error(&bo->fence->base, -ECANCELED);
+ dma_fence_signal(&bo->fence->base);
+ dma_fence_put(&bo->fence->base);
+ qaic_remove_bo_fence(bo);
+ mutex_unlock(&bo->lock);
drm_gem_object_put(&bo->base);
spin_lock_irqsave(&dbc->xfer_lock, flags);
}
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index c1bb6db78d12..219f58fdfde8 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -40,7 +40,7 @@ MODULE_IMPORT_NS("DMA_BUF");
#define PCI_DEVICE_ID_QCOM_AIC080 0xa080
#define PCI_DEVICE_ID_QCOM_AIC100 0xa100
#define PCI_DEVICE_ID_QCOM_AIC200 0xa110
-#define QAIC_NAME "qaic"
+#define PCI_DEVICE_ID_QCOM_AIC200VF 0xa111
#define QAIC_DESC "Qualcomm Cloud AI Accelerators"
#define CNTL_MAJOR 5
#define CNTL_MINOR 0
diff --git a/drivers/accel/qaic/qaic_fence.c b/drivers/accel/qaic/qaic_fence.c
new file mode 100644
index 000000000000..ec7e555c454b
--- /dev/null
+++ b/drivers/accel/qaic/qaic_fence.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. */
+
+#include <linux/dma-fence.h>
+#include <linux/dma-resv.h>
+#include <linux/ktime.h>
+#include <linux/spinlock.h>
+#include <linux/sprintf.h>
+
+#include "qaic.h"
+
+static const char *qaic_fence_get_driver_name(struct dma_fence *f)
+{
+ return QAIC_NAME;
+}
+
+static const char *qaic_fence_get_timeline_name(struct dma_fence *f)
+{
+ struct qaic_fence *qfence = container_of(f, struct qaic_fence, base);
+
+ if (!qfence->timeline_name[0])
+ scnprintf(qfence->timeline_name, ARRAY_SIZE(qfence->timeline_name), "D%02u",
+ qfence->dbc_id);
+ return qfence->timeline_name;
+}
+
+static const struct dma_fence_ops qaic_fence_ops = {
+ .get_driver_name = qaic_fence_get_driver_name,
+ .get_timeline_name = qaic_fence_get_timeline_name,
+};
+
+static void qaic_fence_sync_cb_func(struct dma_fence *f, struct dma_fence_cb *cb)
+{
+ struct qaic_bo *bo = container_of(cb, struct qaic_bo, cb);
+ struct qaic_device *qdev;
+
+ if (f->error)
+ return;
+
+ qdev = bo->dbc->qdev;
+ dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, bo->dir);
+ bo->perf_stats.req_processed_ts = ktime_to_ns(f->timestamp);
+}
+
+int qaic_fence_wait(struct dma_fence *fence, signed long timeout)
+{
+ int ret;
+
+ timeout = dma_fence_wait_timeout(fence, true, timeout);
+ if (!timeout) { /* Wait timed out */
+ ret = -ETIMEDOUT;
+ } else if (timeout < 0) { /* Wait interrupted */
+ ret = -EINTR;
+ } else { /* Wait completed */
+ ret = dma_fence_get_status(fence);
+ WARN_ON_ONCE(ret == 0);
+ if (!ret) /* Fence not signaled yet */
+ ret = -EBUSY;
+ else if (ret > 0) /* Fence is signaled successfully */
+ ret = 0;
+ /* otherwise ret already contains the error code of the fence */
+ }
+ return ret;
+}
+
+static struct qaic_fence *qaic_create_fence(struct qaic_bo *bo)
+{
+ struct qaic_fence *fence;
+
+ fence = kzalloc_obj(*fence);
+ if (!fence)
+ return ERR_PTR(-ENOMEM);
+
+ spin_lock_init(&fence->lock);
+ fence->dbc_id = bo->dbc->id;
+ dma_fence_init(&fence->base, &qaic_fence_ops, &fence->lock, bo->fence_context, 1);
+
+ return fence;
+}
+
+/* Caller should be holding bo->lock */
+static int qaic_set_bo_resv_fence(struct qaic_bo *bo, struct qaic_fence *fence)
+{
+ int ret;
+
+ ret = dma_resv_reserve_fences(bo->base.resv, 1);
+ if (ret)
+ goto err_out_fence;
+
+ /*
+ * Add a fence in dma-buf reservation object for this BO,
+ * enables buffer sharing across devices.
+ */
+ dma_resv_add_fence(bo->base.resv, &fence->base,
+ dma_resv_usage_rw(bo->dir == DMA_TO_DEVICE));
+
+ ret = dma_fence_add_callback(&fence->base, &bo->cb, qaic_fence_sync_cb_func);
+ if (ret)
+ goto err_out_fence;
+
+ return ret;
+
+err_out_fence:
+ dma_fence_put(&fence->base);
+ bo->fence = NULL;
+ return ret;
+}
+
+/* Caller should be holding bo->lock */
+void qaic_remove_bo_fence(struct qaic_bo *bo)
+{
+ if (bo->fence) {
+ if (!dma_fence_is_signaled(&bo->fence->base)) {
+ dma_fence_set_error(&bo->fence->base, -ECANCELED);
+ dma_fence_signal(&bo->fence->base);
+ }
+ dma_fence_put(&bo->fence->base);
+ }
+ bo->fence = NULL;
+}
+
+/* Caller should be holding bo->lock */
+static void qaic_replace_bo_fence(struct qaic_bo *bo, struct qaic_fence *fence)
+{
+ qaic_remove_bo_fence(bo);
+ bo->fence = fence;
+}
+
+/* Caller should be holding bo->lock */
+int qaic_acquire_bo_fence(struct qaic_bo *bo)
+{
+ struct qaic_fence *fence;
+ int ret = 0;
+
+ fence = qaic_create_fence(bo);
+ if (IS_ERR(fence))
+ return PTR_ERR(fence);
+
+ qaic_replace_bo_fence(bo, fence);
+ ret = qaic_set_bo_resv_fence(bo, fence);
+
+ return ret;
+}
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/6] accel/qaic: Replace transfer completion with dma-fence
2026-10-03 2:38 ` [PATCH 2/6] accel/qaic: Replace transfer completion with dma-fence Carl Vanderlip
@ 2026-10-03 2:54 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-10-03 2:54 UTC (permalink / raw)
To: Carl Vanderlip; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [Critical] dma-fence: Unsignaled dma-fences and leaked callbacks on the error cleanup path
- [Critical] qaic: Race condition in qaic_fence_sync_cb_func causes NULL pointer dereference
- [Critical] dma-fence: Reusing embedded dma_fence_cb corrupts callback list pointers
- [High] dma-mapping: Concurrent CPU and device SGT synchronization due to fence signaling race
- [High] uapi: QAIC_WAIT_BO ioctl returns -ENOENT for idle buffers
- [High] dma-fence: Reused sequence numbers violate monotonically increasing API contract
- [Medium] dma-mapping: Cancelled BO executions skip CPU DMA synchronization
- [Medium] uapi: Overriding -ERESTARTSYS with -EINTR breaks syscall restart ABI
--
commit cb1752759fe062100cc452aaddf96c2432a9f99c
Author: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
accel/qaic: Replace transfer completion with dma-fence
This commit replaces completion mechanisms with dma-fences to signal finished
IRQ work. It integrates buffer synchronization with dma_resv and prepares for
drm_sched support.
[Severity: Critical]
In drivers/accel/qaic/qaic_data.c, within the send_bo_list_to_device function
around line 1357, the error cleanup path removes previously queued buffer
objects from the transfer list without calling dma_fence_signal or
qaic_remove_bo_fence. Can this leak pending unsignaled fences into the
dma_resv object? Because qaic_acquire_bo_fence already added the fence to the
reservation object, it appears the unsignaled fence could remain there
permanently. This might cause a deadlock for kernel or userspace components
waiting on the buffer implicit sync barriers, and the embedded cb might
remain registered to the leaked fence, resulting in a use-after-free when the
buffer is destroyed.
[Severity: High]
Looking at qaic_fence_sync_cb_func in drivers/accel/qaic/qaic_fence.c around
line 42, the CPU DMA sync is performed asynchronously when the fence signals.
However, in send_bo_list_to_device in drivers/accel/qaic/qaic_data.c around
line 1338, the device DMA sync runs upon resubmission. Could this race if the
previous fence callback has not finished? A concurrent qaic_execute_bo_ioctl
could see bo_queued as false, acquire the lock, and resubmit the transfer.
This would allow the scheduler to run dma_sync_sgtable_for_device while the
IRQ thread is still executing dma_sync_sgtable_for_cpu, potentially violating
DMA API ownership rules and causing data corruption.
[Severity: High]
In the qaic_wait_bo_ioctl function in drivers/accel/qaic/qaic_data.c around
line 1899, the code now returns -ENOENT if the fence is missing. Does this
change break the userspace API behavior? Previously, a buffer with no pending
operations returned 0 immediately because it waited on a pre-completed
completion. Applications expecting success when waiting on an already-idle or
newly-mapped buffer might now fail when receiving this error code instead.
[Severity: Medium]
When operations are aborted, such as in empty_xfer_list in
drivers/accel/qaic/qaic_data.c around line 2115, the fences are signaled with
-ECANCELED. Then, in qaic_fence_sync_cb_func in drivers/accel/qaic/qaic_fence.c
around line 38, the callback explicitly checks the error status and returns
early. Does this permanently skip the required CPU DMA sync for cancelled
operations? If dma_sync_sgtable_for_cpu is skipped, the memory might be left
indefinitely mapped in the device DMA domain, violating API ownership rules.
[Severity: High]
In drivers/accel/qaic/qaic_fence.c around line 77, qaic_create_fence
initializes the dma_fence with a hardcoded sequence number of 1. The context
is allocated once per buffer object in qaic_alloc_init_bo, located in
drivers/accel/qaic/qaic_data.c around line 766. Is it correct to reuse the
same sequence number on the same context for every submission? The dma-fence
API requires sequence numbers on a given context to monotonically increase.
Reusing sequence numbers could cause dma_fence_is_later to evaluate timelines
incorrectly, potentially bypassing synchronization when buffers are shared.
[Severity: Critical]
There appears to be a race condition involving qaic_fence_sync_cb_func in
drivers/accel/qaic/qaic_fence.c around line 41 and qaic_unprepare_bo in
drivers/accel/qaic/qaic_data.c around line 994. The callback executes
locklessly and accesses the device pointer and scatter-gather table.
Concurrently, a thread calling qaic_detach_slice_bo_ioctl could see
bo_queued return false, acquire the lock, and execute qaic_unprepare_bo.
This tears down the DMA mappings and clears the channel pointer. Could the
fence callback on another CPU dereference the newly cleared channel pointer
and access the freed memory, leading to a null pointer dereference?
[Severity: Critical]
In qaic_set_bo_resv_fence in drivers/accel/qaic/qaic_fence.c around line 98,
dma_fence_add_callback adds the embedded callback node to the new fence. Is
it safe to reuse the embedded callback node for subsequent submissions? The
bo_queued check evaluates to false immediately when the signaling thread sets
the signaled bit, which could happen while it is still iterating over the old
fence callback list. If a concurrent submission thread allocates a new fence
and adds the callback, it could overwrite the node pointers, corrupting the
kernel linked lists.
[Severity: Medium]
In qaic_fence_wait in drivers/accel/qaic/qaic_fence.c around line 54, the
function checks if timeout is less than 0 and sets the return value to -EINTR.
Since dma_fence_wait_timeout typically returns -ERESTARTSYS when interrupted
by a signal, does mapping this internal restart instruction to an explicit
failure break the syscall restart ABI? This change seems to disable the
kernel automatic VFS syscall restart mechanism for the ioctl, forcing
userspace to handle interrupted waits manually.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261003023818.1161666-1-carl.vanderlip@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/6] accel/qaic: Isolate hw submission
2026-10-03 2:38 [PATCH 0/6] accel/qaic: Use DRM Scheduler for submitting BO reqs Carl Vanderlip
2026-10-03 2:38 ` [PATCH 1/6] accel/qaic: Centralize execution info Carl Vanderlip
2026-10-03 2:38 ` [PATCH 2/6] accel/qaic: Replace transfer completion with dma-fence Carl Vanderlip
@ 2026-10-03 2:38 ` Carl Vanderlip
2026-10-03 2:49 ` sashiko-bot
2026-10-03 2:38 ` [PATCH 4/6] accel/qaic: Use DRM_SCHED to handle FIFO submission Carl Vanderlip
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Carl Vanderlip @ 2026-10-03 2:38 UTC (permalink / raw)
To: jeff.hugo, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel,
Sashiko, Pranjal Ramajor Asha Kanojiya
Currently there are two pathways used to submit a request to the DBC
FIFO. In preparation for using the DRM scheduler, merge these two into
one so that job submission logic is simpler.
Co-developed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Signed-off-by: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
---
drivers/accel/qaic/qaic_data.c | 171 +++++++++++++++++----------------
1 file changed, 89 insertions(+), 82 deletions(-)
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 3326d9da0e1e..0208400d2636 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -1164,47 +1164,88 @@ static inline u32 fifo_space_avail(u32 head, u32 tail, u32 q_size)
return avail;
}
-static inline int copy_exec_reqs(struct qaic_device *qdev, struct bo_slice *slice, u32 dbc_id,
- u32 head, u32 *ptail)
+static void submit_reqs_to_fifo(struct dma_bridge_chan *dbc, struct dbc_req *reqs,
+ u32 num_req, u32 head, u32 tail)
{
- struct dma_bridge_chan *dbc = &qdev->dbc[dbc_id];
+ u32 num_submit;
+
+ if (tail + num_req > dbc->nelem) {
+ num_submit = dbc->nelem - tail;
+ memcpy(fifo_at(dbc->req_q_base, tail), reqs, sizeof(*reqs) * num_submit);
+ reqs += num_submit;
+ num_submit = num_req - num_submit;
+ memcpy(dbc->req_q_base, reqs, sizeof(*reqs) * num_submit);
+ } else {
+ memcpy(fifo_at(dbc->req_q_base, tail), reqs, sizeof(*reqs) * num_req);
+ }
+}
+
+/* Caller should be holding dbc->req_lock */
+static inline int qaic_submit_reqs_to_hw(struct bo_slice *slice,
+ unsigned int num_req,
+ bool is_partial, u32 partial_size)
+{
+ struct dma_bridge_chan *dbc = slice->bo->dbc;
struct dbc_req *reqs = slice->reqs;
- u32 tail = *ptail;
- u32 avail;
+ struct qaic_bo *bo = slice->bo;
+ struct dbc_req *last_req;
+ u32 head, tail, avail;
+ qaic_data_get_fifo_info(dbc, &head, &tail);
avail = fifo_space_avail(head, tail, dbc->nelem);
- if (avail < slice->nents)
+
+ /* PCI link error */
+ if (head == U32_MAX || tail == U32_MAX)
+ return -ENODEV;
+
+ if (avail < num_req)
return -EAGAIN;
- if (tail + slice->nents > dbc->nelem) {
- avail = dbc->nelem - tail;
- avail = min_t(u32, avail, slice->nents);
- memcpy(fifo_at(dbc->req_q_base, tail), reqs, sizeof(*reqs) * avail);
- reqs += avail;
- avail = slice->nents - avail;
- if (avail)
- memcpy(dbc->req_q_base, reqs, sizeof(*reqs) * avail);
- } else {
- memcpy(fifo_at(dbc->req_q_base, tail), reqs, sizeof(*reqs) * slice->nents);
+ submit_reqs_to_fifo(dbc, reqs, num_req, head, tail);
+
+ if (is_partial) {
+ /*
+ * Copy over the last entry. Here we need to adjust len to the left over
+ * size, and set src and dst to the entry it is copied to.
+ */
+ last_req = fifo_at(dbc->req_q_base, (tail + num_req - 1) % dbc->nelem);
+ memcpy(last_req, &slice->reqs[slice->nents - 1], sizeof(*last_req));
+ last_req->len = cpu_to_le32(partial_size);
+ last_req->src_addr = reqs[num_req - 1].src_addr;
+ last_req->dest_addr = reqs[num_req - 1].dest_addr;
+ last_req->req_id = reqs[num_req - 1].req_id;
+ /* Disable DMA transfer */
+ if (!last_req->len)
+ last_req->cmd &= ~GENMASK(1, 0);
}
- *ptail = (tail + slice->nents) % dbc->nelem;
+ tail = (tail + num_req) % dbc->nelem;
+ /* Submit_ts will be taken for the last job of this BO */
+ bo->perf_stats.req_submit_ts = ktime_get_ns();
+
+ /* Finalize commit to hardware */
+ dma_sync_sgtable_for_device(&dbc->qdev->pdev->dev, bo->sgt, bo->dir);
+ writel(tail, dbc->dbc_base + REQTP_OFF);
return 0;
}
-static inline int copy_partial_exec_reqs(struct qaic_device *qdev, struct bo_slice *slice,
- u64 resize, struct dma_bridge_chan *dbc, u32 head,
- u32 *ptail)
+static inline int copy_exec_reqs(struct bo_slice *slice)
+{
+ int ret;
+
+ ret = qaic_submit_reqs_to_hw(slice, slice->nents, false, 0);
+
+ return ret;
+}
+
+static inline int copy_partial_exec_reqs(struct bo_slice *slice, u64 resize)
{
struct dbc_req *reqs = slice->reqs;
- struct dbc_req *last_req;
- u32 tail = *ptail;
+ unsigned int nents_xfer;
u64 last_bytes;
u32 first_n;
- u32 avail;
-
- avail = fifo_space_avail(head, tail, dbc->nelem);
+ int ret;
/*
* After this for loop is complete, first_n represents the index
@@ -1219,50 +1260,15 @@ static inline int copy_partial_exec_reqs(struct qaic_device *qdev, struct bo_sli
else
break;
- if (avail < (first_n + 1))
- return -EAGAIN;
-
- if (first_n) {
- if (tail + first_n > dbc->nelem) {
- avail = dbc->nelem - tail;
- avail = min_t(u32, avail, first_n);
- memcpy(fifo_at(dbc->req_q_base, tail), reqs, sizeof(*reqs) * avail);
- last_req = reqs + avail;
- avail = first_n - avail;
- if (avail)
- memcpy(dbc->req_q_base, last_req, sizeof(*reqs) * avail);
- } else {
- memcpy(fifo_at(dbc->req_q_base, tail), reqs, sizeof(*reqs) * first_n);
- }
- }
-
- /*
- * Copy over the last entry. Here we need to adjust len to the left over
- * size, and set src and dst to the entry it is copied to.
- */
- last_req = fifo_at(dbc->req_q_base, (tail + first_n) % dbc->nelem);
- memcpy(last_req, reqs + slice->nents - 1, sizeof(*reqs));
-
- /*
- * last_bytes holds size of a DMA segment, maximum DMA segment size is
- * set to UINT_MAX by qaic and hence last_bytes can never exceed u32
- * range. So, by down sizing we are not corrupting the value.
- */
- last_req->len = cpu_to_le32((u32)last_bytes);
- last_req->src_addr = reqs[first_n].src_addr;
- last_req->dest_addr = reqs[first_n].dest_addr;
- if (!last_bytes)
- /* Disable DMA transfer */
- last_req->cmd = GENMASK(7, 2) & reqs[first_n].cmd;
+ nents_xfer = first_n + 1;
- *ptail = (tail + first_n + 1) % dbc->nelem;
+ ret = qaic_submit_reqs_to_hw(slice, nents_xfer, true, last_bytes);
- return 0;
+ return ret;
}
static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *file_priv,
- struct execute_info *exec, struct dma_bridge_chan *dbc,
- u32 head, u32 *tail)
+ struct execute_info *exec, struct dma_bridge_chan *dbc)
{
struct ww_acquire_ctx acquire_ctx;
struct bo_slice *slice;
@@ -1309,37 +1315,40 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
bo->req_id = dbc->next_req_id++;
+ spin_lock_irqsave(&dbc->xfer_lock, flags);
+ list_add_tail(&bo->xfer_list, &dbc->xfer_list);
+ spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+
+ ret = qaic_acquire_bo_fence(bo);
+ if (ret)
+ goto fence_failed;
+
list_for_each_entry(slice, &bo->slices, slice) {
for (j = 0; j < slice->nents; j++)
slice->reqs[j].req_id = cpu_to_le16(bo->req_id);
if (exec->is_partial && (!resize || resize <= slice->offset))
/* Configure the slice for no DMA transfer */
- ret = copy_partial_exec_reqs(qdev, slice, 0, dbc, head, tail);
+ ret = copy_partial_exec_reqs(slice, 0);
else if (exec->is_partial && resize < slice->offset + slice->size)
/* Configure the slice to be partially DMA transferred */
- ret = copy_partial_exec_reqs(qdev, slice,
- resize - slice->offset, dbc,
- head, tail);
+ ret = copy_partial_exec_reqs(slice, resize - slice->offset);
else
- ret = copy_exec_reqs(qdev, slice, dbc->id, head, tail);
+ ret = copy_exec_reqs(slice);
if (ret)
- goto unlock_bo;
+ goto submit_failed;
}
-
- ret = qaic_acquire_bo_fence(bo);
- if (ret)
- goto unlock_bo;
-
- spin_lock_irqsave(&dbc->xfer_lock, flags);
- list_add_tail(&bo->xfer_list, &dbc->xfer_list);
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
- dma_sync_sgtable_for_device(&qdev->pdev->dev, bo->sgt, bo->dir);
mutex_unlock(&bo->lock);
}
goto unlock_resv;
+submit_failed:
+ qaic_remove_bo_fence(bo);
+fence_failed:
+ spin_lock_irqsave(&dbc->xfer_lock, flags);
+ list_del_init(&bo->xfer_list);
+ spin_unlock_irqrestore(&dbc->xfer_lock, flags);
unlock_bo:
drm_gem_object_put(&bo->base);
mutex_unlock(&bo->lock);
@@ -1536,13 +1545,11 @@ static int __qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct dr
exec.perf_stats.queue_level = head <= tail ? tail - head : dbc->nelem - (head - tail);
- ret = send_bo_list_to_device(qdev, file_priv, &exec, dbc, head, &tail);
+ ret = send_bo_list_to_device(qdev, file_priv, &exec, dbc);
if (ret)
goto unlock_req_lock;
- /* Finalize commit to hardware */
exec.perf_stats.submit_ts = ktime_get_ns();
- writel(tail, dbc->dbc_base + REQTP_OFF);
mutex_unlock(&dbc->req_lock);
update_profiling_data(&exec);
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/6] accel/qaic: Isolate hw submission
2026-10-03 2:38 ` [PATCH 3/6] accel/qaic: Isolate hw submission Carl Vanderlip
@ 2026-10-03 2:49 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-10-03 2:49 UTC (permalink / raw)
To: Carl Vanderlip; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] qaic: redundant req_submit_ts assignment causes incorrect stats
--
commit 832ae6a6a52a6bd91f900e3b436da51c50e676e9
Author: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
accel/qaic: Isolate hw submission
This commit merges two pathways used to submit a request to the DBC FIFO
into a single function. This simplifies the job submission logic in
preparation for using the DRM scheduler.
> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
> index 3326d9da0e1e4..0208400d2636f 100644
> --- a/drivers/accel/qaic/qaic_data.c
> +++ b/drivers/accel/qaic/qaic_data.c
[ ... ]
> +/* Caller should be holding dbc->req_lock */
> +static inline int qaic_submit_reqs_to_hw(struct bo_slice *slice,
> + unsigned int num_req,
> + bool is_partial, u32 partial_size)
> +{
[ ... ]
> + tail = (tail + num_req) % dbc->nelem;
> +
> + /* Submit_ts will be taken for the last job of this BO */
> + bo->perf_stats.req_submit_ts = ktime_get_ns();
> +
> + /* Finalize commit to hardware */
> + dma_sync_sgtable_for_device(&dbc->qdev->pdev->dev, bo->sgt, bo->dir);
> + writel(tail, dbc->dbc_base + REQTP_OFF);
> return 0;
> }
[Severity: Medium]
Does update_profiling_data() unconditionally overwrite this new timestamp?
In the IOCTL path, __qaic_execute_bo_ioctl() calls send_bo_list_to_device(),
which eventually reaches this code and records the per-job timestamp.
However, immediately after send_bo_list_to_device() returns,
__qaic_execute_bo_ioctl() calls update_profiling_data(&exec):
static void update_profiling_data(struct execute_info *exec)
{
...
for (i = 0; i < exec->count; i++) {
bo = exec->bo_arr[i];
...
bo->perf_stats.req_submit_ts = exec->perf_stats.submit_ts;
...
}
Could this render the newly added per-job timestamp dead code? Since this
refactoring prepares for the DRM scheduler, might this overwrite also cause
a data race or incorrect profiling statistics when submissions become
asynchronous?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261003023818.1161666-1-carl.vanderlip@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/6] accel/qaic: Use DRM_SCHED to handle FIFO submission
2026-10-03 2:38 [PATCH 0/6] accel/qaic: Use DRM Scheduler for submitting BO reqs Carl Vanderlip
` (2 preceding siblings ...)
2026-10-03 2:38 ` [PATCH 3/6] accel/qaic: Isolate hw submission Carl Vanderlip
@ 2026-10-03 2:38 ` Carl Vanderlip
2026-10-03 2:51 ` sashiko-bot
2026-10-03 2:38 ` [PATCH 5/6] accel/qaic: Split oversized BO requests into multiple jobs Carl Vanderlip
2026-10-03 2:38 ` [PATCH 6/6] accel/qaic: Use reduced size allocation for FIFOs Carl Vanderlip
5 siblings, 1 reply; 13+ messages in thread
From: Carl Vanderlip @ 2026-10-03 2:38 UTC (permalink / raw)
To: jeff.hugo, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel,
Sashiko, Pranjal Ramajor Asha Kanojiya, Youssef Samir
Replace current FIFO submission process with DRM scheduler. Leverage
scheduler entity credit system to prevent overflow.
Since the network's FIFO size is unknown at device/scheduler creation, a
large fixed credit value is used (65536). During network activation, a
scaling ratio is calculated to map the real FIFO size to the fixed
credit limit. If the ratio would be a fraction, it is rounded up;
limiting FIFO utilization, but preventing overuse.
Co-developed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Co-developed-by: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
Signed-off-by: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
Signed-off-by: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
---
drivers/accel/qaic/Kconfig | 1 +
drivers/accel/qaic/Makefile | 1 +
drivers/accel/qaic/qaic.h | 159 +++++++++++++--
drivers/accel/qaic/qaic_control.c | 5 +
drivers/accel/qaic/qaic_data.c | 315 ++++++++++--------------------
drivers/accel/qaic/qaic_debugfs.c | 4 +-
drivers/accel/qaic/qaic_drv.c | 14 +-
drivers/accel/qaic/qaic_fence.c | 101 ++++++----
drivers/accel/qaic/qaic_sched.c | 236 ++++++++++++++++++++++
include/uapi/drm/qaic_accel.h | 2 +-
10 files changed, 563 insertions(+), 275 deletions(-)
create mode 100644 drivers/accel/qaic/qaic_sched.c
diff --git a/drivers/accel/qaic/Kconfig b/drivers/accel/qaic/Kconfig
index 116e42d152ca..d43c06394bb0 100644
--- a/drivers/accel/qaic/Kconfig
+++ b/drivers/accel/qaic/Kconfig
@@ -10,6 +10,7 @@ config DRM_ACCEL_QAIC
depends on MHI_BUS
select CRC32
select WANT_DEV_COREDUMP
+ select DRM_SCHED
help
Enables driver for Qualcomm's Cloud AI accelerator PCIe cards that are
designed to accelerate Deep Learning inference workloads.
diff --git a/drivers/accel/qaic/Makefile b/drivers/accel/qaic/Makefile
index 90582b0693da..092f962e71d3 100644
--- a/drivers/accel/qaic/Makefile
+++ b/drivers/accel/qaic/Makefile
@@ -12,6 +12,7 @@ qaic-y := \
qaic_drv.o \
qaic_fence.o \
qaic_ras.o \
+ qaic_sched.o \
qaic_ssr.o \
qaic_sysfs.o \
qaic_timesync.o \
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 4cdd6579aa89..40c7492ea121 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -18,12 +18,14 @@
#include <linux/workqueue.h>
#include <drm/drm_device.h>
#include <drm/drm_gem.h>
+#include <drm/gpu_scheduler.h>
#define QAIC_NAME "qaic"
#define QAIC_DBC_BASE SZ_128K
#define QAIC_DBC_SIZE SZ_4K
#define QAIC_SSR_DBC_SENTINEL U32_MAX /* No ongoing SSR sentinel */
-#define QAIC_TIMELINE_LEN 32 /* Should fit max sized DBC id/BO handle */
+#define QAIC_TIMELINE_LEN 32 /* Should fit max sized DBC id/Job id */
+#define QAIC_DBC_NAME_LEN 8
#define QAIC_NO_PARTITION -1
@@ -34,6 +36,7 @@
#define to_drm(qddev) (&(qddev)->drm)
#define to_accel_kdev(qddev) (to_drm(qddev)->accel->kdev) /* Return Linux device of accel node */
#define to_qaic_device(dev) (to_qaic_drm_device((dev))->qdev)
+#define to_qaic_job(job) container_of((job), struct qaic_job, base)
enum aic_families {
FAMILY_AIC100,
@@ -68,6 +71,32 @@ enum dbc_states {
extern bool datapath_polling;
+struct qaic_job {
+ /*
+ * Base job structure, internal to DRM.
+ * Do not update any fields in base directly in this driver
+ */
+ struct drm_sched_job base;
+ /* QAIC GEM object this job is referencing */
+ struct qaic_bo *bo;
+ /* Slice this job is executing */
+ struct bo_slice *slice;
+ /* DBC associated with this job */
+ struct dma_bridge_chan *dbc;
+ /* QAIC will signal this fence after execution of this job */
+ struct qaic_fence *irq_fence;
+ /* Node of list of jobs associated with a DBC */
+ struct list_head queue;
+ struct kref ref_count;
+ /* Number of requests this job executes */
+ unsigned int num_req;
+ /* True if this job is partially executed */
+ bool partial;
+ u32 partial_size;
+ /* Identifier for this job */
+ u16 id;
+};
+
struct qaic_user {
/* Uniquely identifies this user for the device */
int handle;
@@ -86,8 +115,8 @@ struct dma_bridge_chan {
struct qaic_device *qdev;
/* ID of this DMA bridge channel(DBC) */
unsigned int id;
- /* Synchronizes access to xfer_list */
- spinlock_t xfer_lock;
+ /* Synchronizes access to job queue */
+ spinlock_t job_q_lock;
/* Base address of request queue */
void *req_q_base;
/* Base address of response queue */
@@ -108,7 +137,7 @@ struct dma_bridge_chan {
* memory handle can enqueue more than one request elements, all
* this requests that belong to same memory handle have same request ID
*/
- u16 next_req_id;
+ atomic_t next_req_id;
/* true: DBC is in use; false: DBC not in use */
bool in_use;
/*
@@ -118,8 +147,6 @@ struct dma_bridge_chan {
void __iomem *dbc_base;
/* Synchronizes access to Request queue's head and tail pointer */
struct mutex req_lock;
- /* Head of list where each node is a memory handle queued in request queue */
- struct list_head xfer_list;
/* Synchronizes DBC readers during cleanup */
struct srcu_struct ch_lock;
/*
@@ -135,6 +162,23 @@ struct dma_bridge_chan {
struct work_struct poll_work;
/* Represents various states of this DBC from enum dbc_states */
unsigned int state;
+ /*
+ * Name of this DBC. Format is "dbcXXX" where XXX is DBC id
+ * with leading 0's
+ */
+ char name[QAIC_DBC_NAME_LEN];
+ /* Job scheduler for this DBC */
+ struct drm_gpu_scheduler sched;
+ /* Custom DRM scheduler workqueue for multi-device performance */
+ struct workqueue_struct *submit_wq;
+ /* Scheduler entity for this DBC */
+ struct drm_sched_entity sched_entity;
+ /* List head for jobs scheduled for execution */
+ struct list_head job_queue;
+ /* Ratio to scale FIFO length to scheduler credit constant */
+ u32 credit_ratio;
+ /* Total number of requests that have been queued to this DBC */
+ u32 pending_reqs;
};
struct qaic_device {
@@ -238,6 +282,8 @@ struct qaic_fence {
char timeline_name[QAIC_TIMELINE_LEN];
/* Cached value of dbc_id for use in populating timeline_name */
u32 dbc_id;
+ /* ID of associated Job */
+ u16 job_id;
};
struct qaic_bo {
@@ -257,24 +303,15 @@ struct qaic_bo {
struct dma_bridge_chan *dbc;
/* Number of slice that belongs to this buffer */
u32 nr_slice;
- /* Number of slice that have been transferred by DMA engine */
- u32 nr_slice_xfer_done;
/*
* If true then user has attached slicing information to this BO by
* calling DRM_IOCTL_QAIC_ATTACH_SLICE_BO ioctl.
*/
bool sliced;
- /* Request ID of this BO if it is queued for execution */
- u16 req_id;
/* Wait on this fence for DMA transfer of this BO */
- struct qaic_fence *fence;
+ struct dma_fence *fence;
/* Current fence context for this BO */
u64 fence_context;
- /*
- * Node in linked list where head is dbc->xfer_list.
- * This link list contain BO's that are queued for DMA transfer.
- */
- struct list_head xfer_list;
/*
* Node in linked list where head is dbc->bo_lists.
* This link list contain BO's that are associated with the DBC it is
@@ -307,6 +344,8 @@ struct qaic_bo {
struct mutex lock;
/* Callback to sync BO with CPU */
struct dma_fence_cb cb;
+ /* True if BO needs to be sync'd with device; otherwise no need */
+ bool need_dev_sync;
};
struct bo_slice {
@@ -331,6 +370,78 @@ struct bo_slice {
u64 offset;
};
+struct dbc_req {
+ /*
+ * A request ID is assigned to each memory handle going in DMA queue.
+ * As a single memory handle can enqueue multiple elements in DMA queue
+ * all of them will have the same request ID.
+ */
+ __le16 req_id;
+ /* Future use */
+ __u8 seq_id;
+ /*
+ * Special encoded variable
+ * 7 0 - Do not force to generate MSI after DMA is completed
+ * 1 - Force to generate MSI after DMA is completed
+ * 6:5 Reserved
+ * 4 1 - Generate completion element in the response queue
+ * 0 - No Completion Code
+ * 3 0 - DMA request is a Link list transfer
+ * 1 - DMA request is a Bulk transfer
+ * 2 Reserved
+ * 1:0 00 - No DMA transfer involved
+ * 01 - DMA transfer is part of inbound transfer
+ * 10 - DMA transfer has outbound transfer
+ * 11 - NA
+ */
+ __u8 cmd;
+ __le32 resv;
+ /* Source address for the transfer */
+ __le64 src_addr;
+ /* Destination address for the transfer */
+ __le64 dest_addr;
+ /* Length of transfer request */
+ __le32 len;
+ __le32 resv2;
+ /* Doorbell address */
+ __le64 db_addr;
+ /*
+ * Special encoded variable
+ * 7 1 - Doorbell(db) write
+ * 0 - No doorbell write
+ * 6:2 Reserved
+ * 1:0 00 - 32 bit access, db address must be aligned to 32bit-boundary
+ * 01 - 16 bit access, db address must be aligned to 16bit-boundary
+ * 10 - 8 bit access, db address must be aligned to 8bit-boundary
+ * 11 - Reserved
+ */
+ __u8 db_len;
+ __u8 resv3;
+ __le16 resv4;
+ /* 32 bit data written to doorbell address */
+ __le32 db_data;
+ /*
+ * Special encoded variable
+ * All the fields of sem_cmdX are passed from user and all are ORed
+ * together to form sem_cmd.
+ * 0:11 Semaphore value
+ * 15:12 Reserved
+ * 20:16 Semaphore index
+ * 21 Reserved
+ * 22 Semaphore Sync
+ * 23 Reserved
+ * 26:24 Semaphore command
+ * 28:27 Reserved
+ * 29 Semaphore DMA out bound sync fence
+ * 30 Semaphore DMA in bound sync fence
+ * 31 Enable semaphore command
+ */
+ __le32 sem_cmd0;
+ __le32 sem_cmd1;
+ __le32 sem_cmd2;
+ __le32 sem_cmd3;
+} __packed;
+
int get_dbc_req_elem_size(void);
int get_dbc_rsp_elem_size(void);
int get_cntl_version(struct qaic_device *qdev, struct qaic_user *usr, u16 *major, u16 *minor);
@@ -350,6 +461,8 @@ void enable_dbc(struct qaic_device *qdev, u32 dbc_id, struct qaic_user *usr);
void wakeup_dbc(struct qaic_device *qdev, u32 dbc_id);
void release_dbc(struct qaic_device *qdev, u32 dbc_id);
void qaic_data_get_fifo_info(struct dma_bridge_chan *dbc, u32 *head, u32 *tail);
+int qaic_submit_reqs_to_hw(struct bo_slice *slice, unsigned int num_req,
+ bool is_partial, u32 partial_size, u16 job_id);
void wake_all_cntl(struct qaic_device *qdev);
void qaic_dev_reset_clean_local_state(struct qaic_device *qdev);
@@ -370,8 +483,20 @@ void qaic_dbc_exit_ssr(struct qaic_device *qdev);
/* qaic_fence.c */
int qaic_fence_wait(struct dma_fence *fence, signed long timeout);
+struct qaic_fence *qaic_create_irq_fence(struct qaic_bo *bo, u64 seqno, u16 job_id);
+int qaic_acquire_bo_fence(struct qaic_bo *bo, int job_count, struct list_head *job_list);
void qaic_remove_bo_fence(struct qaic_bo *bo);
-int qaic_acquire_bo_fence(struct qaic_bo *bo);
+
+/* qaic_sched.c */
+int qaicm_sched_init(struct drm_device *drm, struct dma_bridge_chan *dbc);
+void qaic_sched_entity_init(struct dma_bridge_chan *dbc);
+struct qaic_job *qaic_create_job(struct drm_file *file_priv, struct bo_slice *slice,
+ unsigned int num_req, u64 seq_no, struct list_head *tmp_list);
+void qaic_submit_job(struct dma_bridge_chan *dbc, struct qaic_job *job);
+void qaic_free_job(struct kref *ref);
+void qaic_put_job(struct qaic_job *job);
+void qaic_cleanup_job(struct qaic_job *job);
+void set_dbc_scaling_ratio(struct dma_bridge_chan *dbc, u32 nelem);
/* qaic_sysfs.c */
int qaic_sysfs_init(struct qaic_drm_device *qddev);
diff --git a/drivers/accel/qaic/qaic_control.c b/drivers/accel/qaic/qaic_control.c
index 2ccc55486aac..9c4400c0fd17 100644
--- a/drivers/accel/qaic/qaic_control.c
+++ b/drivers/accel/qaic/qaic_control.c
@@ -919,7 +919,12 @@ static int decode_deactivate(struct qaic_device *qdev, void *trans, u32 *msg_len
* Releasing resources failed on the device side, which puts
* us in a bind since they may still be in use, so enable the
* dbc. User is expected to retry deactivation.
+ *
+ * Scheduler entity must be destroyed before it's reinitialized
+ * in enable_dbc, otherwise entity list element points to itself
+ * and causes a cycle in any list it was a node of.
*/
+ drm_sched_entity_destroy(&qdev->dbc[dbc_id].sched_entity);
enable_dbc(qdev, dbc_id, usr);
return -ECANCELED;
}
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 0208400d2636..63ca4a489a9a 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -64,78 +64,6 @@ module_param(datapath_poll_interval_us, uint, 0600);
MODULE_PARM_DESC(datapath_poll_interval_us,
"Amount of time to sleep between activity when datapath polling is enabled");
-struct dbc_req {
- /*
- * A request ID is assigned to each memory handle going in DMA queue.
- * As a single memory handle can enqueue multiple elements in DMA queue
- * all of them will have the same request ID.
- */
- __le16 req_id;
- /* Future use */
- __u8 seq_id;
- /*
- * Special encoded variable
- * 7 0 - Do not force to generate MSI after DMA is completed
- * 1 - Force to generate MSI after DMA is completed
- * 6:5 Reserved
- * 4 1 - Generate completion element in the response queue
- * 0 - No Completion Code
- * 3 0 - DMA request is a Link list transfer
- * 1 - DMA request is a Bulk transfer
- * 2 Reserved
- * 1:0 00 - No DMA transfer involved
- * 01 - DMA transfer is part of inbound transfer
- * 10 - DMA transfer has outbound transfer
- * 11 - NA
- */
- __u8 cmd;
- __le32 resv;
- /* Source address for the transfer */
- __le64 src_addr;
- /* Destination address for the transfer */
- __le64 dest_addr;
- /* Length of transfer request */
- __le32 len;
- __le32 resv2;
- /* Doorbell address */
- __le64 db_addr;
- /*
- * Special encoded variable
- * 7 1 - Doorbell(db) write
- * 0 - No doorbell write
- * 6:2 Reserved
- * 1:0 00 - 32 bit access, db address must be aligned to 32bit-boundary
- * 01 - 16 bit access, db address must be aligned to 16bit-boundary
- * 10 - 8 bit access, db address must be aligned to 8bit-boundary
- * 11 - Reserved
- */
- __u8 db_len;
- __u8 resv3;
- __le16 resv4;
- /* 32 bit data written to doorbell address */
- __le32 db_data;
- /*
- * Special encoded variable
- * All the fields of sem_cmdX are passed from user and all are ORed
- * together to form sem_cmd.
- * 0:11 Semaphore value
- * 15:12 Reserved
- * 20:16 Semaphore index
- * 21 Reserved
- * 22 Semaphore Sync
- * 23 Reserved
- * 26:24 Semaphore command
- * 28:27 Reserved
- * 29 Semaphore DMA out bound sync fence
- * 30 Semaphore DMA in bound sync fence
- * 31 Enable semaphore command
- */
- __le32 sem_cmd0;
- __le32 sem_cmd1;
- __le32 sem_cmd2;
- __le32 sem_cmd3;
-} __packed;
-
struct dbc_rsp {
/* Request ID of the memory handle whose DMA transaction is completed */
__le16 req_id;
@@ -188,7 +116,7 @@ struct qaic_exec_data_req {
static inline bool bo_queued(struct qaic_bo *bo)
{
if (bo->fence)
- return !dma_fence_is_signaled(&bo->fence->base);
+ return !dma_fence_is_signaled(bo->fence);
return false;
}
@@ -751,7 +679,6 @@ static const struct drm_gem_object_funcs qaic_gem_funcs = {
static void qaic_init_bo_lists(struct qaic_bo *bo)
{
INIT_LIST_HEAD(&bo->slices);
- INIT_LIST_HEAD(&bo->xfer_list);
}
static struct qaic_bo *qaic_alloc_init_bo(void)
@@ -1181,9 +1108,8 @@ static void submit_reqs_to_fifo(struct dma_bridge_chan *dbc, struct dbc_req *req
}
/* Caller should be holding dbc->req_lock */
-static inline int qaic_submit_reqs_to_hw(struct bo_slice *slice,
- unsigned int num_req,
- bool is_partial, u32 partial_size)
+int qaic_submit_reqs_to_hw(struct bo_slice *slice, unsigned int num_req,
+ bool is_partial, u32 partial_size, u16 job_id)
{
struct dma_bridge_chan *dbc = slice->bo->dbc;
struct dbc_req *reqs = slice->reqs;
@@ -1225,27 +1151,30 @@ static inline int qaic_submit_reqs_to_hw(struct bo_slice *slice,
bo->perf_stats.req_submit_ts = ktime_get_ns();
/* Finalize commit to hardware */
- dma_sync_sgtable_for_device(&dbc->qdev->pdev->dev, bo->sgt, bo->dir);
+ if (bo->need_dev_sync) {
+ bo->need_dev_sync = false;
+ dma_sync_sgtable_for_device(&dbc->qdev->pdev->dev, bo->sgt, bo->dir);
+ }
writel(tail, dbc->dbc_base + REQTP_OFF);
return 0;
}
-static inline int copy_exec_reqs(struct bo_slice *slice)
+static inline void qaic_dbc_put_job(struct dma_bridge_chan *dbc, struct qaic_job *job)
{
- int ret;
-
- ret = qaic_submit_reqs_to_hw(slice, slice->nents, false, 0);
-
- return ret;
+ list_del_init(&job->queue);
+ dbc->pending_reqs -= job->num_req;
+ qaic_put_job(job);
}
-static inline int copy_partial_exec_reqs(struct bo_slice *slice, u64 resize)
+static int create_slice_jobs(struct drm_file *file_priv, struct bo_slice *slice, bool partial,
+ u64 resize, struct list_head *tmp_list)
{
struct dbc_req *reqs = slice->reqs;
+ struct qaic_job *job = NULL;
+ unsigned int job_count = 0;
unsigned int nents_xfer;
u64 last_bytes;
u32 first_n;
- int ret;
/*
* After this for loop is complete, first_n represents the index
@@ -1260,20 +1189,33 @@ static inline int copy_partial_exec_reqs(struct bo_slice *slice, u64 resize)
else
break;
- nents_xfer = first_n + 1;
+ if (partial)
+ nents_xfer = first_n + 1;
+ else
+ nents_xfer = slice->nents;
- ret = qaic_submit_reqs_to_hw(slice, nents_xfer, true, last_bytes);
+ job = qaic_create_job(file_priv, slice, nents_xfer, 0, tmp_list);
+ if (IS_ERR(job))
+ return PTR_ERR(job);
+ job_count++;
- return ret;
+ /* job points to the last job for this slice, only valid for partial execute ioctl */
+ job->partial_size = last_bytes;
+ job->partial = partial;
+
+ return job_count;
}
-static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *file_priv,
- struct execute_info *exec, struct dma_bridge_chan *dbc)
+static int send_bo_list_to_sched(struct qaic_device *qdev, struct drm_file *file_priv,
+ struct execute_info *exec, struct dma_bridge_chan *dbc)
{
+ struct qaic_job *job, *job_i, *tmp;
struct ww_acquire_ctx acquire_ctx;
struct bo_slice *slice;
unsigned long flags;
+ LIST_HEAD(tmp_list);
struct qaic_bo *bo;
+ int job_count;
u64 resize;
u32 handle;
int i, j;
@@ -1290,13 +1232,7 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
handle = exec->handle_arr[i];
ret = mutex_lock_interruptible(&bo->lock);
if (ret)
- goto failed_to_send_bo;
-
- /*
- * Take reference to object before sending to device,
- * released when transfer is complete (in dbc_irq_threaded_fn()).
- */
- drm_gem_object_get(&bo->base);
+ goto free_jobs;
if (!bo->sliced) {
ret = -EINVAL;
@@ -1313,54 +1249,54 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
goto unlock_bo;
}
- bo->req_id = dbc->next_req_id++;
-
- spin_lock_irqsave(&dbc->xfer_lock, flags);
- list_add_tail(&bo->xfer_list, &dbc->xfer_list);
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
-
- ret = qaic_acquire_bo_fence(bo);
- if (ret)
- goto fence_failed;
-
+ job_count = 0;
list_for_each_entry(slice, &bo->slices, slice) {
- for (j = 0; j < slice->nents; j++)
- slice->reqs[j].req_id = cpu_to_le16(bo->req_id);
-
if (exec->is_partial && (!resize || resize <= slice->offset))
/* Configure the slice for no DMA transfer */
- ret = copy_partial_exec_reqs(slice, 0);
+ ret = create_slice_jobs(file_priv, slice, true, 0, &tmp_list);
else if (exec->is_partial && resize < slice->offset + slice->size)
/* Configure the slice to be partially DMA transferred */
- ret = copy_partial_exec_reqs(slice, resize - slice->offset);
+ ret = create_slice_jobs(file_priv, slice, true,
+ resize - slice->offset, &tmp_list);
else
- ret = copy_exec_reqs(slice);
- if (ret)
- goto submit_failed;
+ ret = create_slice_jobs(file_priv, slice, false, 0, &tmp_list);
+ if (ret <= 0)
+ goto unlock_bo;
+ job_count += ret;
}
+
+ ret = qaic_acquire_bo_fence(bo, job_count, &tmp_list);
+ if (ret)
+ goto unlock_bo;
+
+ bo->need_dev_sync = true;
mutex_unlock(&bo->lock);
}
+ spin_lock_irqsave(&dbc->job_q_lock, flags);
+ job = list_last_entry(&dbc->job_queue, typeof(*job), queue);
+ list_splice_tail_init(&tmp_list, &dbc->job_queue);
+ list_for_each_entry_continue(job, &dbc->job_queue, queue)
+ qaic_submit_job(dbc, job);
+ spin_unlock_irqrestore(&dbc->job_q_lock, flags);
+
goto unlock_resv;
-submit_failed:
- qaic_remove_bo_fence(bo);
-fence_failed:
- spin_lock_irqsave(&dbc->xfer_lock, flags);
- list_del_init(&bo->xfer_list);
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
unlock_bo:
- drm_gem_object_put(&bo->base);
mutex_unlock(&bo->lock);
-failed_to_send_bo:
- for (j = 0; j < i; j++) {
- drm_gem_object_put(&exec->bo_arr[j]->base);
- spin_lock_irqsave(&dbc->xfer_lock, flags);
- bo = list_last_entry(&dbc->xfer_list, struct qaic_bo, xfer_list);
- list_del_init(&bo->xfer_list);
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
- dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, bo->dir);
+free_jobs:
+ for (j = i - 1; i > 0 && j >= 0; j--) {
+ bo = exec->bo_arr[j];
+ ret = mutex_lock_interruptible(&bo->lock);
+ if (!ret) {
+ bo->need_dev_sync = false;
+ dma_fence_put(bo->fence);
+ bo->fence = NULL;
+ mutex_unlock(&bo->lock);
+ }
}
+ list_for_each_entry_safe_reverse(job_i, tmp, &tmp_list, queue)
+ qaic_cleanup_job(job_i);
unlock_resv:
drm_gem_unlock_reservations((struct drm_gem_object **)exec->bo_arr,
exec->count, &acquire_ctx);
@@ -1451,7 +1387,6 @@ static int __qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct dr
struct qaic_user *usr;
size_t elem_size_sum;
void *usr_exec_ent;
- u32 head, tail;
int i, ret;
exec.perf_stats.received_ts = ktime_get_ns();
@@ -1530,36 +1465,20 @@ static int __qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct dr
goto release_ch_rcu;
}
- ret = mutex_lock_interruptible(&dbc->req_lock);
- if (ret)
- goto release_ch_rcu;
-
- head = readl(dbc->dbc_base + REQHP_OFF);
- tail = readl(dbc->dbc_base + REQTP_OFF);
-
- if (head == U32_MAX || tail == U32_MAX) {
- /* PCI link error */
- ret = -ENODEV;
- goto unlock_req_lock;
- }
+ /* Not locking to read pending_reqs since exact value not necessary */
+ exec.perf_stats.queue_level = dbc->pending_reqs;
- exec.perf_stats.queue_level = head <= tail ? tail - head : dbc->nelem - (head - tail);
-
- ret = send_bo_list_to_device(qdev, file_priv, &exec, dbc);
+ ret = send_bo_list_to_sched(qdev, file_priv, &exec, dbc);
if (ret)
- goto unlock_req_lock;
+ goto release_ch_rcu;
exec.perf_stats.submit_ts = ktime_get_ns();
- mutex_unlock(&dbc->req_lock);
update_profiling_data(&exec);
if (datapath_polling)
schedule_work(&dbc->poll_work);
-unlock_req_lock:
- if (ret)
- mutex_unlock(&dbc->req_lock);
release_ch_rcu:
srcu_read_unlock(&dbc->ch_lock, ch_rcu_id);
for (i = 0; i < exec.count; i++)
@@ -1683,13 +1602,13 @@ void qaic_irq_polling_work(struct work_struct *work)
srcu_read_unlock(&dbc->ch_lock, rcu_id);
return;
}
- spin_lock_irqsave(&dbc->xfer_lock, flags);
- if (list_empty(&dbc->xfer_list)) {
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+ spin_lock_irqsave(&dbc->job_q_lock, flags);
+ if (list_empty(&dbc->job_queue)) {
+ spin_unlock_irqrestore(&dbc->job_q_lock, flags);
srcu_read_unlock(&dbc->ch_lock, rcu_id);
return;
}
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+ spin_unlock_irqrestore(&dbc->job_q_lock, flags);
head = readl(dbc->dbc_base + RSPHP_OFF);
if (head == U32_MAX) { /* PCI link error */
@@ -1719,8 +1638,8 @@ irqreturn_t dbc_irq_threaded_fn(int irq, void *data)
struct dma_bridge_chan *dbc = data;
int event_count = NUM_EVENTS;
int delay_count = NUM_DELAYS;
+ struct qaic_job *job, *tmp;
struct qaic_device *qdev;
- struct qaic_bo *bo, *i;
struct dbc_rsp *rsp;
unsigned long flags;
int rcu_id;
@@ -1773,37 +1692,17 @@ irqreturn_t dbc_irq_threaded_fn(int irq, void *data)
status = le16_to_cpu(rsp->status);
if (status)
pci_dbg(qdev->pdev, "req_id %d failed with status %d\n", req_id, status);
- spin_lock_irqsave(&dbc->xfer_lock, flags);
- /*
- * A BO can receive multiple interrupts, since a BO can be
- * divided into multiple slices and a buffer receives as many
- * interrupts as slices. So until it receives interrupts for
- * all the slices we cannot mark that buffer complete.
- */
- list_for_each_entry_safe(bo, i, &dbc->xfer_list, xfer_list) {
- if (bo->req_id == req_id)
- bo->nr_slice_xfer_done++;
- else
- continue;
- if (bo->nr_slice_xfer_done < bo->nr_slice)
+ spin_lock_irqsave(&dbc->job_q_lock, flags);
+ list_for_each_entry_safe(job, tmp, &dbc->job_queue, queue) {
+ if (job->id == req_id) {
+ dma_fence_signal(&job->irq_fence->base);
+ qaic_dbc_put_job(dbc, job);
break;
-
- /*
- * At this point we have received all the interrupts for
- * BO, which means BO execution is complete.
- */
- bo->nr_slice_xfer_done = 0;
- list_del_init(&bo->xfer_list);
- /*
- * fence is put when it is executed next, during xfer_list
- * cleanup, or when slice is detached
- */
- dma_fence_signal(&bo->fence->base);
- drm_gem_object_put(&bo->base);
- break;
+ }
}
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+ spin_unlock_irqrestore(&dbc->job_q_lock, flags);
+
head = (head + 1) % dbc->nelem;
}
@@ -1910,7 +1809,7 @@ int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file
goto put_obj;
}
- fence = &bo->fence->base;
+ fence = bo->fence;
dma_fence_get(fence);
mutex_unlock(&bo->lock);
@@ -2101,44 +2000,31 @@ int qaic_detach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
return ret;
}
-static void empty_xfer_list(struct qaic_device *qdev, struct dma_bridge_chan *dbc)
+static void empty_xfer_list(struct dma_bridge_chan *dbc)
{
+ struct qaic_job *job, *tmp;
unsigned long flags;
- struct qaic_bo *bo;
- spin_lock_irqsave(&dbc->xfer_lock, flags);
- while (!list_empty(&dbc->xfer_list)) {
- bo = list_first_entry(&dbc->xfer_list, typeof(*bo), xfer_list);
- list_del_init(&bo->xfer_list);
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
- mutex_lock(&bo->lock);
- bo->nr_slice_xfer_done = 0;
- bo->req_id = 0;
- bo->perf_stats.req_received_ts = 0;
- bo->perf_stats.req_submit_ts = 0;
- bo->perf_stats.req_processed_ts = 0;
- bo->perf_stats.queue_level_before = 0;
- dma_fence_get(&bo->fence->base);
- dma_fence_set_error(&bo->fence->base, -ECANCELED);
- dma_fence_signal(&bo->fence->base);
- dma_fence_put(&bo->fence->base);
- qaic_remove_bo_fence(bo);
- mutex_unlock(&bo->lock);
- drm_gem_object_put(&bo->base);
- spin_lock_irqsave(&dbc->xfer_lock, flags);
+ spin_lock_irqsave(&dbc->job_q_lock, flags);
+ list_for_each_entry_safe_reverse(job, tmp, &dbc->job_queue, queue) {
+ dma_fence_get(&job->irq_fence->base);
+ dma_fence_set_error(&job->irq_fence->base, -ECANCELED);
+ dma_fence_signal(&job->irq_fence->base);
+ dma_fence_put(&job->irq_fence->base);
+ qaic_dbc_put_job(dbc, job);
}
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+ spin_unlock_irqrestore(&dbc->job_q_lock, flags);
}
-static void sync_empty_xfer_list(struct qaic_device *qdev, struct dma_bridge_chan *dbc)
+static void sync_empty_xfer_list(struct dma_bridge_chan *dbc)
{
- empty_xfer_list(qdev, dbc);
+ empty_xfer_list(dbc);
synchronize_srcu(&dbc->ch_lock);
/*
* Threads holding channel lock, may add more elements in the xfer_list.
* Flush out these elements from xfer_list.
*/
- empty_xfer_list(qdev, dbc);
+ empty_xfer_list(dbc);
}
int disable_dbc(struct qaic_device *qdev, u32 dbc_id, struct qaic_user *usr)
@@ -2161,6 +2047,8 @@ int disable_dbc(struct qaic_device *qdev, u32 dbc_id, struct qaic_user *usr)
*/
void enable_dbc(struct qaic_device *qdev, u32 dbc_id, struct qaic_user *usr)
{
+ qaic_sched_entity_init(&qdev->dbc[dbc_id]);
+ set_dbc_scaling_ratio(&qdev->dbc[dbc_id], qdev->dbc[dbc_id].nelem);
qdev->dbc[dbc_id].usr = usr;
}
@@ -2169,7 +2057,7 @@ void wakeup_dbc(struct qaic_device *qdev, u32 dbc_id)
struct dma_bridge_chan *dbc = &qdev->dbc[dbc_id];
dbc->usr = NULL;
- sync_empty_xfer_list(qdev, dbc);
+ sync_empty_xfer_list(dbc);
}
void release_dbc(struct qaic_device *qdev, u32 dbc_id)
@@ -2182,6 +2070,7 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
return;
wakeup_dbc(qdev, dbc_id);
+ drm_sched_entity_destroy(&dbc->sched_entity);
dma_free_coherent(&qdev->pdev->dev, dbc->total_size, dbc->req_q_base, dbc->dma_addr);
dbc->total_size = 0;
diff --git a/drivers/accel/qaic/qaic_debugfs.c b/drivers/accel/qaic/qaic_debugfs.c
index 909a4d774d90..2fac928d5406 100644
--- a/drivers/accel/qaic/qaic_debugfs.c
+++ b/drivers/accel/qaic/qaic_debugfs.c
@@ -100,7 +100,6 @@ void qaic_debugfs_init(struct qaic_drm_device *qddev)
struct qaic_device *qdev = qddev->qdev;
struct dentry *debugfs_root;
struct dentry *debugfs_dir;
- char name[QAIC_DBC_DIR_NAME];
u32 i;
debugfs_root = to_drm(qddev)->debugfs_root;
@@ -111,8 +110,7 @@ void qaic_debugfs_init(struct qaic_drm_device *qddev)
* reasonable range.
*/
for (i = 0; i < qdev->num_dbc && i < 256; ++i) {
- snprintf(name, QAIC_DBC_DIR_NAME, "dbc%03u", i);
- debugfs_dir = debugfs_create_dir(name, debugfs_root);
+ debugfs_dir = debugfs_create_dir(qdev->dbc[i].name, debugfs_root);
debugfs_create_file("fifo_size", 0400, debugfs_dir, &qdev->dbc[i], &fifo_size_fops);
debugfs_create_file("queued", 0400, debugfs_dir, &qdev->dbc[i], &queued_fops);
}
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index 219f58fdfde8..46b86b3655ca 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -461,10 +461,11 @@ static struct qaic_device *create_qdev(struct pci_dev *pdev,
INIT_LIST_HEAD(&qddev->users);
for (i = 0; i < qdev->num_dbc; ++i) {
- spin_lock_init(&qdev->dbc[i].xfer_lock);
+ spin_lock_init(&qdev->dbc[i].job_q_lock);
qdev->dbc[i].qdev = qdev;
qdev->dbc[i].id = i;
- INIT_LIST_HEAD(&qdev->dbc[i].xfer_list);
+ INIT_LIST_HEAD(&qdev->dbc[i].job_queue);
+ scnprintf(qdev->dbc[i].name, ARRAY_SIZE(qdev->dbc[i].name), "dbc%03u", i);
ret = qaicm_srcu_init(drm, &qdev->dbc[i].ch_lock);
if (ret)
return NULL;
@@ -473,6 +474,9 @@ static struct qaic_device *create_qdev(struct pci_dev *pdev,
ret = drmm_mutex_init(drm, &qdev->dbc[i].req_lock);
if (ret)
return NULL;
+ ret = qaicm_sched_init(drm, &qdev->dbc[i]);
+ if (ret)
+ return NULL;
}
return qdev;
@@ -708,9 +712,9 @@ static bool qaic_data_path_busy(struct qaic_device *qdev)
srcu_read_unlock(&dbc->ch_lock, ch_rcu_id);
continue;
}
- spin_lock_irqsave(&dbc->xfer_lock, flags);
- ret = !list_empty(&dbc->xfer_list);
- spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+ spin_lock_irqsave(&dbc->job_q_lock, flags);
+ ret = !list_empty(&dbc->job_queue);
+ spin_unlock_irqrestore(&dbc->job_q_lock, flags);
srcu_read_unlock(&dbc->ch_lock, ch_rcu_id);
if (ret)
break;
diff --git a/drivers/accel/qaic/qaic_fence.c b/drivers/accel/qaic/qaic_fence.c
index ec7e555c454b..ed4c7f6a7768 100644
--- a/drivers/accel/qaic/qaic_fence.c
+++ b/drivers/accel/qaic/qaic_fence.c
@@ -2,9 +2,12 @@
/* Copyright (c) 2024-2025 Qualcomm Innovation Center, Inc. All rights reserved. */
+#include <linux/dma-fence-array.h>
#include <linux/dma-fence.h>
#include <linux/dma-resv.h>
+#include <linux/err.h>
#include <linux/ktime.h>
+#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/sprintf.h>
@@ -20,8 +23,8 @@ static const char *qaic_fence_get_timeline_name(struct dma_fence *f)
struct qaic_fence *qfence = container_of(f, struct qaic_fence, base);
if (!qfence->timeline_name[0])
- scnprintf(qfence->timeline_name, ARRAY_SIZE(qfence->timeline_name), "D%02u",
- qfence->dbc_id);
+ scnprintf(qfence->timeline_name, ARRAY_SIZE(qfence->timeline_name),
+ "D%02u-JOB%05x", qfence->dbc_id, qfence->job_id);
return qfence->timeline_name;
}
@@ -64,7 +67,7 @@ int qaic_fence_wait(struct dma_fence *fence, signed long timeout)
return ret;
}
-static struct qaic_fence *qaic_create_fence(struct qaic_bo *bo)
+struct qaic_fence *qaic_create_irq_fence(struct qaic_bo *bo, u64 seqno, u16 job_id)
{
struct qaic_fence *fence;
@@ -74,71 +77,97 @@ static struct qaic_fence *qaic_create_fence(struct qaic_bo *bo)
spin_lock_init(&fence->lock);
fence->dbc_id = bo->dbc->id;
- dma_fence_init(&fence->base, &qaic_fence_ops, &fence->lock, bo->fence_context, 1);
+ fence->job_id = job_id;
+ dma_fence_init(&fence->base, &qaic_fence_ops, &fence->lock, bo->fence_context, seqno);
return fence;
}
-/* Caller should be holding bo->lock */
-static int qaic_set_bo_resv_fence(struct qaic_bo *bo, struct qaic_fence *fence)
+static int qaic_create_bo_fence(struct qaic_bo *bo, struct dma_fence **output_fence,
+ int job_count, struct list_head *job_list)
{
- int ret;
-
- ret = dma_resv_reserve_fences(bo->base.resv, 1);
- if (ret)
- goto err_out_fence;
+ struct dma_fence_array *fence_arr;
+ struct dma_fence **job_fences;
+ struct qaic_job *job;
+ int ret, i = 0;
- /*
- * Add a fence in dma-buf reservation object for this BO,
- * enables buffer sharing across devices.
- */
- dma_resv_add_fence(bo->base.resv, &fence->base,
- dma_resv_usage_rw(bo->dir == DMA_TO_DEVICE));
+ job_fences = kcalloc(job_count, sizeof(*job_fences), GFP_KERNEL);
+ if (!job_fences)
+ return -ENOMEM;
- ret = dma_fence_add_callback(&fence->base, &bo->cb, qaic_fence_sync_cb_func);
+ ret = dma_resv_reserve_fences(bo->base.resv, job_count);
if (ret)
- goto err_out_fence;
+ goto job_err;
+
+ /* Add all the fences that we need to wait on to resv */
+ list_for_each_entry_reverse(job, job_list, queue) {
+ if (i < job_count) {
+ job_fences[i] = dma_fence_get(&job->irq_fence->base);
+ dma_resv_add_fence(bo->base.resv, job_fences[i],
+ dma_resv_usage_rw(bo->dir == DMA_TO_DEVICE));
+ i++;
+ } else {
+ break;
+ }
+ }
- return ret;
+ /* When successful, job_fences and the fences it contains are now owned by fence_arr */
+ fence_arr = dma_fence_array_create(job_count, job_fences, bo->fence_context, 1);
+ if (!fence_arr) {
+ ret = -ENOMEM;
+ goto fence_err;
+ }
-err_out_fence:
- dma_fence_put(&fence->base);
- bo->fence = NULL;
+ /* Output fence array so bo will only have one fence to wait on */
+ *output_fence = &fence_arr->base;
+ return 0;
+fence_err:
+ for (i = 0; i < job_count; i++)
+ dma_fence_put(job_fences[i]);
+job_err:
+ kfree(job_fences);
return ret;
+
}
/* Caller should be holding bo->lock */
void qaic_remove_bo_fence(struct qaic_bo *bo)
{
if (bo->fence) {
- if (!dma_fence_is_signaled(&bo->fence->base)) {
- dma_fence_set_error(&bo->fence->base, -ECANCELED);
- dma_fence_signal(&bo->fence->base);
+ if (!dma_fence_is_signaled(bo->fence)) {
+ dma_fence_set_error(bo->fence, -ECANCELED);
+ dma_fence_signal(bo->fence);
}
- dma_fence_put(&bo->fence->base);
+ dma_fence_put(bo->fence);
}
bo->fence = NULL;
}
/* Caller should be holding bo->lock */
-static void qaic_replace_bo_fence(struct qaic_bo *bo, struct qaic_fence *fence)
+static void qaic_replace_bo_fence(struct qaic_bo *bo, struct dma_fence *fence)
{
qaic_remove_bo_fence(bo);
bo->fence = fence;
}
/* Caller should be holding bo->lock */
-int qaic_acquire_bo_fence(struct qaic_bo *bo)
+int qaic_acquire_bo_fence(struct qaic_bo *bo, int job_count, struct list_head *job_list)
{
- struct qaic_fence *fence;
- int ret = 0;
+ struct dma_fence *fence;
+ int ret;
- fence = qaic_create_fence(bo);
- if (IS_ERR(fence))
- return PTR_ERR(fence);
+ ret = qaic_create_bo_fence(bo, &fence, job_count, job_list);
+ if (ret)
+ return ret;
+
+ ret = dma_fence_add_callback(fence, &bo->cb, qaic_fence_sync_cb_func);
+ if (ret) {
+ dma_fence_put(fence);
+ bo->fence = NULL;
+ return ret;
+ }
qaic_replace_bo_fence(bo, fence);
- ret = qaic_set_bo_resv_fence(bo, fence);
- return ret;
+ return 0;
}
diff --git a/drivers/accel/qaic/qaic_sched.c b/drivers/accel/qaic/qaic_sched.c
new file mode 100644
index 000000000000..b0992d63c2b6
--- /dev/null
+++ b/drivers/accel/qaic/qaic_sched.c
@@ -0,0 +1,236 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/* Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries. */
+
+#include <drm/drm_device.h>
+#include <drm/drm_file.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_managed.h>
+#include <drm/gpu_scheduler.h>
+#include <linux/dma-fence.h>
+#include <linux/workqueue.h>
+
+#include "qaic.h"
+
+#define QAIC_CREDITS SZ_64K
+
+static void qaic_job_fini(struct drm_sched_job *sched_job)
+{
+ struct qaic_job *job = to_qaic_job(sched_job);
+
+ qaic_cleanup_job(job);
+}
+
+static struct dma_fence *qaic_job_run(struct drm_sched_job *sched_job)
+{
+ struct qaic_job *job = to_qaic_job(sched_job);
+ struct dma_bridge_chan *dbc = job->dbc;
+ struct bo_slice *slice = job->slice;
+ struct dma_fence *fence;
+ int ret;
+
+ fence = dma_fence_get(&job->irq_fence->base);
+
+ /* Job has been cancelled */
+ if (fence->error) {
+ ret = fence->error;
+ goto exit;
+ }
+
+ ret = mutex_lock_interruptible(&dbc->req_lock);
+ if (ret)
+ goto exit;
+
+ ret = qaic_submit_reqs_to_hw(slice, job->num_req, job->partial,
+ job->partial_size, job->id);
+ if (ret)
+ goto unlock_dbc;
+ mutex_unlock(&dbc->req_lock);
+
+ return fence;
+
+unlock_dbc:
+ mutex_unlock(&dbc->req_lock);
+exit:
+ dma_fence_put(fence);
+ return ERR_PTR(ret);
+}
+
+static const struct drm_sched_backend_ops qaic_sched_ops = {
+ .run_job = qaic_job_run,
+ .free_job = qaic_job_fini,
+};
+
+static void qaicm_sched_fini(struct drm_device *dev, void *res)
+{
+ struct dma_bridge_chan *dbc = res;
+
+ drm_sched_fini(&dbc->sched);
+ destroy_workqueue(dbc->submit_wq);
+}
+
+/*
+ * Currently, drm_sched is configured to always have credits equal to 64k, but
+ * the network can configure different HW queue sizes. Create a scaling ratio to
+ * convert sizes of HW queue to size in credits.
+ *
+ * The ratio expresses the cost of each queue element. For example, if nelem=10,
+ * then the ratio is 6554 credits consumed per queue element. This cost,
+ * together with the queue credit balance, limits the number of items in the
+ * queue at any time.
+ *
+ * The credit limit should be greater than the typical HW queue so that the
+ * scaling ratio is never less than 1. 'nelem' is bounds checked in
+ * encode_activate() to guarantee that it is less than QAIC_CREDITS. Round up
+ * the scaling ratio if it isn't perfectly divisible.
+ */
+void set_dbc_scaling_ratio(struct dma_bridge_chan *dbc, u32 nelem)
+{
+ if (!nelem) {
+ /*
+ * There is a chance of enable_dbc() being called after a failed deactivation of the
+ * dbc. In that case, set the credit ratio to a value that shouldn't let any further
+ * work be scheduled.
+ */
+ dbc->credit_ratio = QAIC_CREDITS;
+ return;
+ }
+ if (QAIC_CREDITS % nelem != 0)
+ pr_debug("Credits not evenly divisible by queue size: size = %d\n", nelem);
+ dbc->credit_ratio = DIV_ROUND_UP(QAIC_CREDITS, nelem);
+}
+
+int qaicm_sched_init(struct drm_device *drm, struct dma_bridge_chan *dbc)
+{
+ int ret;
+
+ /*
+ * Using out own workqueue here due to performance regression with
+ * large numbers of devices and jobs (32+ and 16+ respectively).
+ *
+ * Workqueues with WQ_UNBOUND + max_active > 1 also prevented this
+ * regression, but introduced the possibility of unordered
+ * submission to the HW queue. Under testing that did not seem to
+ * cause any issues, though eventually decided to go with this to
+ * minimize changes to existing HW interaction patterns.
+ */
+ dbc->submit_wq = alloc_workqueue("%s", WQ_MEM_RECLAIM | WQ_PERCPU, 1, dbc->name);
+ if (!dbc->submit_wq)
+ return -EINVAL;
+
+ const struct drm_sched_init_args args = {
+ .ops = &qaic_sched_ops,
+ .submit_wq = dbc->submit_wq,
+ .num_rqs = 1,
+ .credit_limit = (QAIC_CREDITS - 1),
+ .hang_limit = 1,
+ .timeout = MAX_SCHEDULE_TIMEOUT,
+ .name = dbc->name,
+ .dev = drm->dev,
+ };
+
+ ret = drm_sched_init(&dbc->sched, &args);
+ if (ret) {
+ destroy_workqueue(dbc->submit_wq);
+ return ret;
+ }
+
+ return drmm_add_action_or_reset(drm, qaicm_sched_fini, dbc);
+}
+
+void qaic_sched_entity_init(struct dma_bridge_chan *dbc)
+{
+ struct drm_gpu_scheduler *sched = &dbc->sched;
+
+ drm_sched_entity_init(&dbc->sched_entity, DRM_SCHED_PRIORITY_KERNEL, &sched, 1, NULL);
+}
+
+struct qaic_job *qaic_create_job(struct drm_file *file_priv, struct bo_slice *slice,
+ unsigned int num_req, u64 seq_no, struct list_head *tmp_list)
+{
+ struct dma_bridge_chan *dbc = slice->bo->dbc;
+ struct qaic_bo *bo = slice->bo;
+ struct qaic_job *job;
+ u32 credits;
+ u16 req_id;
+ int ret, i;
+
+ if (check_mul_overflow((u32)num_req, dbc->credit_ratio, &credits))
+ return ERR_PTR(-EINVAL);
+
+ job = kzalloc_obj(*job);
+ if (!job)
+ return ERR_PTR(-ENOMEM);
+
+ req_id = (u16) atomic_inc_return(&dbc->next_req_id);
+
+ job->irq_fence = qaic_create_irq_fence(bo, seq_no, req_id);
+ if (IS_ERR(job->irq_fence)) {
+ ret = PTR_ERR(job->irq_fence);
+ goto free_job;
+ }
+
+ ret = drm_sched_job_init(&job->base, &dbc->sched_entity, credits,
+ dbc, file_priv->client_id);
+ if (ret)
+ goto put_fence;
+
+ /*
+ * Make new job depend on the pre-existing fences in BO reservation.
+ * DRM scheduler will wait on every fence in the dependency list
+ * before running this job.
+ */
+ ret = drm_sched_job_add_implicit_dependencies(&job->base, &bo->base, true);
+ if (ret)
+ goto job_cleanup;
+
+ job->slice = slice;
+ job->num_req = num_req;
+ job->dbc = dbc;
+ INIT_LIST_HEAD(&job->queue);
+ job->id = req_id;
+ kref_init(&job->ref_count);
+ for (i = 0; i < job->num_req; i++)
+ slice->reqs[i].req_id = cpu_to_le16(job->id);
+
+ list_add_tail(&job->queue, tmp_list);
+ drm_gem_object_get(&bo->base);
+ job->bo = bo;
+ return job;
+
+job_cleanup:
+ drm_sched_job_cleanup(&job->base);
+put_fence:
+ dma_fence_put(&job->irq_fence->base);
+free_job:
+ kfree(job);
+ return ERR_PTR(ret);
+}
+
+void qaic_submit_job(struct dma_bridge_chan *dbc, struct qaic_job *job)
+{
+ kref_get(&job->ref_count);
+ drm_sched_job_arm(&job->base);
+ drm_sched_entity_push_job(&job->base);
+ dbc->pending_reqs += job->num_req;
+}
+
+void qaic_free_job(struct kref *ref)
+{
+ struct qaic_job *job = container_of(ref, struct qaic_job, ref_count);
+
+ dma_fence_put(&job->irq_fence->base);
+ drm_gem_object_put(&job->bo->base);
+ kfree(job);
+}
+
+void qaic_put_job(struct qaic_job *job)
+{
+ kref_put(&job->ref_count, qaic_free_job);
+}
+
+void qaic_cleanup_job(struct qaic_job *job)
+{
+ drm_sched_job_cleanup(&job->base);
+ qaic_put_job(job);
+}
diff --git a/include/uapi/drm/qaic_accel.h b/include/uapi/drm/qaic_accel.h
index c92d0309d583..5271bf66f5bd 100644
--- a/include/uapi/drm/qaic_accel.h
+++ b/include/uapi/drm/qaic_accel.h
@@ -346,7 +346,7 @@ struct qaic_perf_stats {
* struct qaic_perf_stats_entry - Defines a BO perf info.
* @handle: In. GEM handle of the BO to get perf stats for.
* @queue_level_before: Out. Number of elements in the queue before this BO
- * was submitted.
+ * was submitted, including those not yet scheduled to HW.
* @num_queue_element: Out. Number of elements added to the queue to submit
* this BO.
* @submit_latency_us: Out. Time taken by the driver to submit this BO.
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/6] accel/qaic: Use DRM_SCHED to handle FIFO submission
2026-10-03 2:38 ` [PATCH 4/6] accel/qaic: Use DRM_SCHED to handle FIFO submission Carl Vanderlip
@ 2026-10-03 2:51 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-10-03 2:51 UTC (permalink / raw)
To: Carl Vanderlip; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] accel/qaic: Aborted jobs leave unsignaled hardware fences in dma_resv
- [Critical] accel/qaic: Resetting sequence numbers across slices overwrites fences
- [Critical] accel/qaic: Deadlock in decode_deactivate due to ignored interrupts
- [High] accel/qaic: Error rollback with mutex_lock_interruptible hangs BO
- [High] accel/qaic: Job credit overflow triggers DRM scheduler WARN_ON_ONCE
--
commit 188a88e259ba9f726c46ba938ac70127cfee6105
Author: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
accel/qaic: Use DRM_SCHED to handle FIFO submission
This commit replaces the custom FIFO submission process with the DRM
scheduler, introducing a scheduler entity credit system to prevent overflow.
It calculates a scaling ratio to map variable FIFO sizes to a fixed credit
limit.
> diff --git a/drivers/accel/qaic/qaic_control.c b/drivers/accel/qaic/qaic_control.c
> --- a/drivers/accel/qaic/qaic_control.c
> +++ b/drivers/accel/qaic/qaic_control.c
[ ... ]
> @@ -919,7 +919,12 @@ static int decode_deactivate(struct qaic_device *qdev, void *trans, u32 *msg_len
> * Releasing resources failed on the device side, which puts
> * us in a bind since they may still be in use, so enable the
> * dbc. User is expected to retry deactivation.
> + *
> + * Scheduler entity must be destroyed before it's reinitialized
> + * in enable_dbc, otherwise entity list element points to itself
> + * and causes a cycle in any list it was a node of.
> */
> + drm_sched_entity_destroy(&qdev->dbc[dbc_id].sched_entity);
> enable_dbc(qdev, dbc_id, usr);
> return -ECANCELED;
[Severity: Critical]
Can this synchronous destroy call cause a deadlock?
When shutting down a user context, the driver first calls disable_dbc(),
which sets dbc->usr = NULL. If the device replies with an error status here,
we call drm_sched_entity_destroy() which blocks waiting for all pending jobs
to complete.
However, since dbc->usr is already NULL, the hardware interrupt handler
dbc_irq_threaded_fn() will skip processing and ignore all completions, which
deadlocks the control workqueue forever.
> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
> --- a/drivers/accel/qaic/qaic_data.c
> +++ b/drivers/accel/qaic/qaic_data.c
[ ... ]
> -static inline int copy_partial_exec_reqs(struct bo_slice *slice, u64 resize)
> +static int create_slice_jobs(struct drm_file *file_priv, struct bo_slice *slice, bool partial,
> + u64 resize, struct list_head *tmp_list)
> {
[ ... ]
> - ret = qaic_submit_reqs_to_hw(slice, nents_xfer, true, last_bytes);
> + if (partial)
> + nents_xfer = first_n + 1;
> + else
> + nents_xfer = slice->nents;
>
> - return ret;
> + job = qaic_create_job(file_priv, slice, nents_xfer, 0, tmp_list);
[Severity: Critical]
Is it safe to hardcode 0 for the sequence number across multiple slices?
Because all slices of a BO share the exact same bo->fence_context,
dma_resv_add_fence() sees subsequent slice fences as having the same context
and an overlapping sequence number (e.g., 0 >= 0). The dma_resv logic
therefore assumes they are on the same timeline and overwrites the earlier
slice's fence, which could allow userspace or other drivers to access the
buffer while earlier slices are still undergoing DMA.
[ ... ]
> @@ -1290,13 +1232,7 @@ static int send_bo_list_to_sched(...)
[ ... ]
> unlock_bo:
> - drm_gem_object_put(&bo->base);
> mutex_unlock(&bo->lock);
> -failed_to_send_bo:
> - for (j = 0; j < i; j++) {
> - drm_gem_object_put(&exec->bo_arr[j]->base);
> - spin_lock_irqsave(&dbc->xfer_lock, flags);
> - bo = list_last_entry(&dbc->xfer_list, struct qaic_bo, xfer_list);
> - list_del_init(&bo->xfer_list);
> - spin_unlock_irqrestore(&dbc->xfer_lock, flags);
> - dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, bo->dir);
> +free_jobs:
> + for (j = i - 1; i > 0 && j >= 0; j--) {
> + bo = exec->bo_arr[j];
> + ret = mutex_lock_interruptible(&bo->lock);
> + if (!ret) {
> + bo->need_dev_sync = false;
> + dma_fence_put(bo->fence);
> + bo->fence = NULL;
> + mutex_unlock(&bo->lock);
> + }
[Severity: High]
Does using an interruptible lock in this error rollback path risk permanently
deadlocking the buffer object?
If a thread has a pending signal, the lock acquisition fails and this
critical cleanup block is skipped. Failing to perform this rollback leaves
the BO permanently waiting on jobs that will never be scheduled.
> }
> + list_for_each_entry_safe_reverse(job_i, tmp, &tmp_list, queue)
> + qaic_cleanup_job(job_i);
> unlock_resv:
> drm_gem_unlock_reservations((struct drm_gem_object **)exec->bo_arr,
> exec->count, &acquire_ctx);
[Severity: Critical]
Does this error path leak unsignaled hardware fences into the global
dma_resv object?
The internal hardware irq_fence is extracted from these jobs and added
directly to the BO's dma_resv during qaic_create_bo_fence(). If we jump to
free_jobs and drop these jobs here using qaic_cleanup_job(), their
irq_fence is never signaled, which could permanently deadlock the buffer
object.
Similarly, if the DRM scheduler fails to execute the job, it signals its
software s_fence->finished but leaves the hardware irq_fence unsignaled.
> diff --git a/drivers/accel/qaic/qaic_sched.c b/drivers/accel/qaic/qaic_sched.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/accel/qaic/qaic_sched.c
[ ... ]
> +void set_dbc_scaling_ratio(struct dma_bridge_chan *dbc, u32 nelem)
> +{
> + if (!nelem) {
[ ... ]
> + dbc->credit_ratio = QAIC_CREDITS;
> + return;
> + }
> + if (QAIC_CREDITS % nelem != 0)
> + pr_debug("Credits not evenly divisible by queue size: size = %d\n", nelem);
> + dbc->credit_ratio = DIV_ROUND_UP(QAIC_CREDITS, nelem);
> +}
[ ... ]
> +struct qaic_job *qaic_create_job(struct drm_file *file_priv, struct bo_slice *slice,
> + unsigned int num_req, u64 seq_no, struct list_head *tmp_list)
> +{
[ ... ]
> + u32 credits;
[ ... ]
> +
> + if (check_mul_overflow((u32)num_req, dbc->credit_ratio, &credits))
> + return ERR_PTR(-EINVAL);
[Severity: High]
Can this math result in credits exceeding the DRM scheduler's limit?
By rounding up the scaling ratio, (nelem - 1) * credit_ratio can exceed
QAIC_CREDITS - 1. For example, if nelem = 5456, ratio = 13, and num_req =
5455, the required credits evaluate to 70915, exceeding the 65535 limit.
While check_mul_overflow() prevents 32-bit integer overflow, it does not
verify if credits exceeds the DRM scheduler's static credit_limit.
Exceeding this limit will trip the WARN_ON_ONCE in drm_sched_can_queue()
and permanently stall the entity queue.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261003023818.1161666-1-carl.vanderlip@oss.qualcomm.com?part=4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/6] accel/qaic: Split oversized BO requests into multiple jobs
2026-10-03 2:38 [PATCH 0/6] accel/qaic: Use DRM Scheduler for submitting BO reqs Carl Vanderlip
` (3 preceding siblings ...)
2026-10-03 2:38 ` [PATCH 4/6] accel/qaic: Use DRM_SCHED to handle FIFO submission Carl Vanderlip
@ 2026-10-03 2:38 ` Carl Vanderlip
2026-10-03 2:51 ` sashiko-bot
2026-10-03 2:38 ` [PATCH 6/6] accel/qaic: Use reduced size allocation for FIFOs Carl Vanderlip
5 siblings, 1 reply; 13+ messages in thread
From: Carl Vanderlip @ 2026-10-03 2:38 UTC (permalink / raw)
To: jeff.hugo, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel,
Sashiko, Pranjal Ramajor Asha Kanojiya
When a BO's requests would exceed FIFO size and credit balance, divide
the requests into groups to be handled by multiple jobs. This addresses
limitations where large or fragmented buffers could not be submitted due
to FIFO capacity.
Co-developed-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Signed-off-by: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
---
drivers/accel/qaic/qaic.h | 9 ++++---
drivers/accel/qaic/qaic_data.c | 45 +++++++++++++++++++++------------
drivers/accel/qaic/qaic_sched.c | 25 ++++++++++++++----
3 files changed, 55 insertions(+), 24 deletions(-)
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 40c7492ea121..53c71584edf9 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -88,6 +88,8 @@ struct qaic_job {
/* Node of list of jobs associated with a DBC */
struct list_head queue;
struct kref ref_count;
+ /* Offset of requests this job needs to skip within the slice */
+ unsigned int req_offset;
/* Number of requests this job executes */
unsigned int num_req;
/* True if this job is partially executed */
@@ -461,8 +463,8 @@ void enable_dbc(struct qaic_device *qdev, u32 dbc_id, struct qaic_user *usr);
void wakeup_dbc(struct qaic_device *qdev, u32 dbc_id);
void release_dbc(struct qaic_device *qdev, u32 dbc_id);
void qaic_data_get_fifo_info(struct dma_bridge_chan *dbc, u32 *head, u32 *tail);
-int qaic_submit_reqs_to_hw(struct bo_slice *slice, unsigned int num_req,
- bool is_partial, u32 partial_size, u16 job_id);
+int qaic_submit_reqs_to_hw(struct bo_slice *slice, struct dbc_req *reqs,
+ unsigned int num_req, bool is_partial, u32 partial_size, u16 job_id);
void wake_all_cntl(struct qaic_device *qdev);
void qaic_dev_reset_clean_local_state(struct qaic_device *qdev);
@@ -491,7 +493,8 @@ void qaic_remove_bo_fence(struct qaic_bo *bo);
int qaicm_sched_init(struct drm_device *drm, struct dma_bridge_chan *dbc);
void qaic_sched_entity_init(struct dma_bridge_chan *dbc);
struct qaic_job *qaic_create_job(struct drm_file *file_priv, struct bo_slice *slice,
- unsigned int num_req, u64 seq_no, struct list_head *tmp_list);
+ unsigned int req_offset, unsigned int num_full_reqs, u64 seq_no,
+ struct list_head *tmp_list);
void qaic_submit_job(struct dma_bridge_chan *dbc, struct qaic_job *job);
void qaic_free_job(struct kref *ref);
void qaic_put_job(struct qaic_job *job);
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 63ca4a489a9a..7287370e4fab 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -948,11 +948,6 @@ static int qaic_attach_slicing_bo(struct qaic_device *qdev, struct qaic_bo *bo,
}
}
- if (bo->total_slice_nents > bo->dbc->nelem) {
- qaic_free_slices_bo(bo);
- return -ENOSPC;
- }
-
return 0;
}
@@ -1108,11 +1103,11 @@ static void submit_reqs_to_fifo(struct dma_bridge_chan *dbc, struct dbc_req *req
}
/* Caller should be holding dbc->req_lock */
-int qaic_submit_reqs_to_hw(struct bo_slice *slice, unsigned int num_req,
- bool is_partial, u32 partial_size, u16 job_id)
+int qaic_submit_reqs_to_hw(struct bo_slice *slice, struct dbc_req *reqs,
+ unsigned int num_req, bool is_partial,
+ u32 partial_size, u16 job_id)
{
struct dma_bridge_chan *dbc = slice->bo->dbc;
- struct dbc_req *reqs = slice->reqs;
struct qaic_bo *bo = slice->bo;
struct dbc_req *last_req;
u32 head, tail, avail;
@@ -1129,12 +1124,12 @@ int qaic_submit_reqs_to_hw(struct bo_slice *slice, unsigned int num_req,
submit_reqs_to_fifo(dbc, reqs, num_req, head, tail);
+ last_req = fifo_at(dbc->req_q_base, (tail + num_req - 1) % dbc->nelem);
if (is_partial) {
/*
* Copy over the last entry. Here we need to adjust len to the left over
* size, and set src and dst to the entry it is copied to.
*/
- last_req = fifo_at(dbc->req_q_base, (tail + num_req - 1) % dbc->nelem);
memcpy(last_req, &slice->reqs[slice->nents - 1], sizeof(*last_req));
last_req->len = cpu_to_le32(partial_size);
last_req->src_addr = reqs[num_req - 1].src_addr;
@@ -1144,6 +1139,8 @@ int qaic_submit_reqs_to_hw(struct bo_slice *slice, unsigned int num_req,
if (!last_req->len)
last_req->cmd &= ~GENMASK(1, 0);
}
+ /* When a BO has been split into multiple jobs, each job needs to signal its completion */
+ last_req->cmd |= GEN_COMPLETION;
tail = (tail + num_req) % dbc->nelem;
@@ -1169,11 +1166,11 @@ static inline void qaic_dbc_put_job(struct dma_bridge_chan *dbc, struct qaic_job
static int create_slice_jobs(struct drm_file *file_priv, struct bo_slice *slice, bool partial,
u64 resize, struct list_head *tmp_list)
{
+ unsigned int nents_offset, nents_xfer;
struct dbc_req *reqs = slice->reqs;
- struct qaic_job *job = NULL;
+ struct qaic_job *job, *job_i;
unsigned int job_count = 0;
- unsigned int nents_xfer;
- u64 last_bytes;
+ u64 last_bytes, seq_no;
u32 first_n;
/*
@@ -1194,16 +1191,32 @@ static int create_slice_jobs(struct drm_file *file_priv, struct bo_slice *slice,
else
nents_xfer = slice->nents;
- job = qaic_create_job(file_priv, slice, nents_xfer, 0, tmp_list);
- if (IS_ERR(job))
- return PTR_ERR(job);
- job_count++;
+ nents_offset = 0;
+ seq_no = 0;
+ job = NULL;
+ while (nents_xfer > nents_offset) {
+ job = qaic_create_job(file_priv, slice, nents_offset, nents_xfer,
+ seq_no++, tmp_list);
+ if (IS_ERR(job))
+ goto free_jobs;
+
+ job_count++;
+ nents_offset += job->num_req;
+ }
/* job points to the last job for this slice, only valid for partial execute ioctl */
job->partial_size = last_bytes;
job->partial = partial;
return job_count;
+
+free_jobs:
+ list_for_each_entry_safe_reverse(job, job_i, tmp_list, queue)
+ if (job_count--)
+ qaic_cleanup_job(job);
+ else
+ break;
+ return job_count;
}
static int send_bo_list_to_sched(struct qaic_device *qdev, struct drm_file *file_priv,
diff --git a/drivers/accel/qaic/qaic_sched.c b/drivers/accel/qaic/qaic_sched.c
index b0992d63c2b6..ded0bdccdfdb 100644
--- a/drivers/accel/qaic/qaic_sched.c
+++ b/drivers/accel/qaic/qaic_sched.c
@@ -41,8 +41,8 @@ static struct dma_fence *qaic_job_run(struct drm_sched_job *sched_job)
if (ret)
goto exit;
- ret = qaic_submit_reqs_to_hw(slice, job->num_req, job->partial,
- job->partial_size, job->id);
+ ret = qaic_submit_reqs_to_hw(slice, &slice->reqs[job->req_offset], job->num_req,
+ job->partial, job->partial_size, job->id);
if (ret)
goto unlock_dbc;
mutex_unlock(&dbc->req_lock);
@@ -146,15 +146,19 @@ void qaic_sched_entity_init(struct dma_bridge_chan *dbc)
}
struct qaic_job *qaic_create_job(struct drm_file *file_priv, struct bo_slice *slice,
- unsigned int num_req, u64 seq_no, struct list_head *tmp_list)
+ unsigned int req_offset, unsigned int num_full_reqs, u64 seq_no,
+ struct list_head *tmp_list)
{
struct dma_bridge_chan *dbc = slice->bo->dbc;
struct qaic_bo *bo = slice->bo;
- struct qaic_job *job;
+ struct qaic_job *job, *dep_job;
+ unsigned int num_req;
u32 credits;
u16 req_id;
int ret, i;
+ num_req = min(slice->bo->dbc->nelem - 1, num_full_reqs - req_offset);
+
if (check_mul_overflow((u32)num_req, dbc->credit_ratio, &credits))
return ERR_PTR(-EINVAL);
@@ -185,13 +189,24 @@ struct qaic_job *qaic_create_job(struct drm_file *file_priv, struct bo_slice *sl
goto job_cleanup;
job->slice = slice;
+ job->req_offset = req_offset;
job->num_req = num_req;
job->dbc = dbc;
INIT_LIST_HEAD(&job->queue);
job->id = req_id;
kref_init(&job->ref_count);
for (i = 0; i < job->num_req; i++)
- slice->reqs[i].req_id = cpu_to_le16(job->id);
+ slice->reqs[i + job->req_offset].req_id = cpu_to_le16(job->id);
+
+ /* Handle BOs that have been split into many jobs */
+ if (seq_no) {
+ dep_job = list_last_entry(tmp_list, struct qaic_job, queue);
+ ret = drm_sched_job_add_dependency(&job->base,
+ dma_fence_get(&dep_job->irq_fence->base));
+ /* add_dependency() will put() the fence on error */
+ if (ret)
+ goto job_cleanup;
+ }
list_add_tail(&job->queue, tmp_list);
drm_gem_object_get(&bo->base);
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 5/6] accel/qaic: Split oversized BO requests into multiple jobs
2026-10-03 2:38 ` [PATCH 5/6] accel/qaic: Split oversized BO requests into multiple jobs Carl Vanderlip
@ 2026-10-03 2:51 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-10-03 2:51 UTC (permalink / raw)
To: Carl Vanderlip; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] drm/qaic: UAF and double free in create_slice_jobs error path
- [Medium] drm/qaic: sequence number collision breaks DRM scheduler dependencies
- [Medium] drm/qaic: silent failure propagation due to clobbered error code
--
commit 0016fc8438fc80359a88bc33d3c9d6dd42769622
Author: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
accel/qaic: Split oversized BO requests into multiple jobs
This commit updates the job submission logic to handle oversized buffer
objects by dividing them into multiple jobs that fit within the hardware
FIFO. It modifies job creation and scheduling to accommodate multiple
chained requests for a single slice.
> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
> index 63ca4a489a9a3..7287370e4fab7 100644
> --- a/drivers/accel/qaic/qaic_data.c
> +++ b/drivers/accel/qaic/qaic_data.c
[ ... ]
> @@ -1169,11 +1166,11 @@ static inline void qaic_dbc_put_job(struct dma_bridge_chan *dbc, struct qaic_job
> static int create_slice_jobs(struct drm_file *file_priv, struct bo_slice *slice, bool partial,
> u64 resize, struct list_head *tmp_list)
> {
> + unsigned int nents_offset, nents_xfer;
> struct dbc_req *reqs = slice->reqs;
> - struct qaic_job *job = NULL;
> + struct qaic_job *job, *job_i;
> unsigned int job_count = 0;
> - unsigned int nents_xfer;
> - u64 last_bytes;
> + u64 last_bytes, seq_no;
> u32 first_n;
>
> /*
> @@ -1194,16 +1191,32 @@ static int create_slice_jobs(struct drm_file *file_priv, struct bo_slice *slice,
> else
> nents_xfer = slice->nents;
>
> - job = qaic_create_job(file_priv, slice, nents_xfer, 0, tmp_list);
> - if (IS_ERR(job))
> - return PTR_ERR(job);
> - job_count++;
> + nents_offset = 0;
> + seq_no = 0;
> + job = NULL;
> + while (nents_xfer > nents_offset) {
> + job = qaic_create_job(file_priv, slice, nents_offset, nents_xfer,
> + seq_no++, tmp_list);
[Severity: Medium]
When a BO has multiple slices, does this reset the sequence number for every
slice? This appears to create duplicate sequence numbers on the same shared BO
fence context.
Because drm_sched_job_add_dependency() deduplicates fences using
dma_fence_is_later(), the explicit intra-BO job dependencies might be silently
dropped if the new seq_no is not greater than older sequence numbers on the
same context.
> + if (IS_ERR(job))
> + goto free_jobs;
> +
> + job_count++;
> + nents_offset += job->num_req;
> + }
>
> /* job points to the last job for this slice, only valid for partial execute ioctl */
> job->partial_size = last_bytes;
> job->partial = partial;
>
> return job_count;
> +
> +free_jobs:
> + list_for_each_entry_safe_reverse(job, job_i, tmp_list, queue)
> + if (job_count--)
> + qaic_cleanup_job(job);
[Severity: High]
Does this error path leave dangling pointers in tmp_list? By calling
qaic_cleanup_job() without removing the job from tmp_list (e.g., via
list_del()), the freed jobs remain in the list.
If send_bo_list_to_sched() is the caller and this returns an error (or 0),
the error path there iterates over tmp_list again:
drivers/accel/qaic/qaic_data.c:send_bo_list_to_sched() {
...
list_for_each_entry_safe_reverse(job_i, tmp, &tmp_list, queue)
qaic_cleanup_job(job_i);
...
}
Would this result in a double free of the jobs?
[Severity: Medium]
Will this error path clobber the original error code from qaic_create_job()?
The job iterator here shadows the job variable containing the ERR_PTR, and the
function returns the post-decremented job_count (which is 0 or -1).
When this propagates up to send_bo_list_to_sched(), it sees a return of 0:
drivers/accel/qaic/qaic_data.c:send_bo_list_to_sched() {
...
ret = create_slice_jobs(file_priv, slice, false, 0, &tmp_list);
if (ret <= 0)
goto unlock_bo;
...
return ret;
}
Could this mask the failure and return 0 (success) to userspace, causing a hang
when userspace waits for BO fences that will never be signaled?
> + else
> + break;
> + return job_count;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261003023818.1161666-1-carl.vanderlip@oss.qualcomm.com?part=5
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/6] accel/qaic: Use reduced size allocation for FIFOs
2026-10-03 2:38 [PATCH 0/6] accel/qaic: Use DRM Scheduler for submitting BO reqs Carl Vanderlip
` (4 preceding siblings ...)
2026-10-03 2:38 ` [PATCH 5/6] accel/qaic: Split oversized BO requests into multiple jobs Carl Vanderlip
@ 2026-10-03 2:38 ` Carl Vanderlip
2026-10-03 2:51 ` sashiko-bot
5 siblings, 1 reply; 13+ messages in thread
From: Carl Vanderlip @ 2026-10-03 2:38 UTC (permalink / raw)
To: jeff.hugo, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel,
Sashiko
On memory limited systems, large allocations from the DMA coherent pool are
likely to fail (especially as memory becomes framgented). Leverage the DRM
Scheduler to be able to break up submissions and make them fit within
available allocation.
Signed-off-by: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
---
drivers/accel/qaic/qaic_control.c | 31 ++++++++++++++++---------------
drivers/accel/qaic/qaic_sched.c | 2 --
2 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/accel/qaic/qaic_control.c b/drivers/accel/qaic/qaic_control.c
index 9c4400c0fd17..7619f87043d2 100644
--- a/drivers/accel/qaic/qaic_control.c
+++ b/drivers/accel/qaic/qaic_control.c
@@ -30,7 +30,12 @@
#define MANAGE_MAGIC_NUMBER ((__force __le32)0x43494151) /* "QAIC" in little endian */
#define QAIC_DBC_Q_GAP SZ_256
-#define QAIC_DBC_Q_BUF_ALIGN SZ_4K
+#define QAIC_DBC_Q_MAX_SIZE SZ_256K
+#define QAIC_DBC_Q_MIN_SIZE (ALIGN(2 * (get_dbc_req_elem_size() + \
+ get_dbc_rsp_elem_size()) + \
+ QAIC_DBC_Q_GAP, SZ_4K))
+#define QAIC_DBC_NELEM(size) (((size) - QAIC_DBC_Q_GAP) / \
+ (get_dbc_req_elem_size() + get_dbc_rsp_elem_size()))
#define QAIC_MANAGE_WIRE_MSG_LENGTH SZ_64K /* Max DMA message length */
#define QAIC_WRAPPER_MAX_SIZE SZ_4K
#define QAIC_MHI_RETRY_WAIT_MS 100
@@ -635,13 +640,13 @@ static int encode_activate(struct qaic_device *qdev, void *trans, struct wrapper
struct qaic_manage_trans_activate_to_dev *in_trans = trans;
struct wire_trans_activate_to_dev *out_trans;
struct wrapper_msg *trans_wrapper;
+ u32 size = QAIC_DBC_Q_MAX_SIZE;
struct wrapper_msg *wrapper;
struct wire_msg *msg;
dma_addr_t dma_addr;
u32 msg_hdr_len;
- void *buf;
u32 nelem;
- u32 size;
+ void *buf;
int ret;
wrapper = list_first_entry(&wrappers->list, struct wrapper_msg, list);
@@ -657,20 +662,16 @@ static int encode_activate(struct qaic_device *qdev, void *trans, struct wrapper
if (in_trans->pad)
return -EINVAL;
- nelem = in_trans->queue_size;
- if (check_mul_overflow((u32)(get_dbc_req_elem_size() + get_dbc_rsp_elem_size()),
- nelem,
- &size))
- return -EINVAL;
-
- if (size + QAIC_DBC_Q_GAP + QAIC_DBC_Q_BUF_ALIGN < size)
- return -EINVAL;
-
- size = ALIGN((size + QAIC_DBC_Q_GAP), QAIC_DBC_Q_BUF_ALIGN);
-
- buf = dma_alloc_coherent(&qdev->pdev->dev, size, &dma_addr, GFP_KERNEL);
+ while (size >= QAIC_DBC_Q_MIN_SIZE) {
+ buf = dma_alloc_coherent(&qdev->pdev->dev, size, &dma_addr, GFP_KERNEL);
+ if (!buf)
+ size >>= 1;
+ else
+ break;
+ }
if (!buf)
return -ENOMEM;
+ nelem = QAIC_DBC_NELEM(size);
trans_wrapper = add_wrapper(wrappers,
offsetof(struct wrapper_msg, trans) + sizeof(*out_trans));
diff --git a/drivers/accel/qaic/qaic_sched.c b/drivers/accel/qaic/qaic_sched.c
index ded0bdccdfdb..20ad8c13cf26 100644
--- a/drivers/accel/qaic/qaic_sched.c
+++ b/drivers/accel/qaic/qaic_sched.c
@@ -95,8 +95,6 @@ void set_dbc_scaling_ratio(struct dma_bridge_chan *dbc, u32 nelem)
dbc->credit_ratio = QAIC_CREDITS;
return;
}
- if (QAIC_CREDITS % nelem != 0)
- pr_debug("Credits not evenly divisible by queue size: size = %d\n", nelem);
dbc->credit_ratio = DIV_ROUND_UP(QAIC_CREDITS, nelem);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 6/6] accel/qaic: Use reduced size allocation for FIFOs
2026-10-03 2:38 ` [PATCH 6/6] accel/qaic: Use reduced size allocation for FIFOs Carl Vanderlip
@ 2026-10-03 2:51 ` sashiko-bot
0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-10-03 2:51 UTC (permalink / raw)
To: Carl Vanderlip; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] accel/qaic: job credits can exceed DRM scheduler credit limit
- [Medium] accel/qaic: missing __GFP_NOWARN in fallback allocation causes log spam
--
commit 71c6cfc1950c4572f2fdf94ea37059d815f2f378
Author: Carl Vanderlip <carl.vanderlip@oss.qualcomm.com>
Subject: accel/qaic: Use reduced size allocation for FIFOs
This commit modifies the allocation strategy for FIFOs in the qaic driver.
Instead of failing when a large DMA coherent pool allocation cannot be
fulfilled, it iteratively halves the requested size until the allocation
succeeds, allowing the DRM scheduler to break up submissions to fit available
memory.
[Severity: Medium]
In drivers/accel/qaic/qaic_control.c inside encode_activate around line 663,
there is a fallback allocation loop that calls dma_alloc_coherent with
GFP_KERNEL. Because the loop starts with a large allocation size and expects
it to potentially fail on fragmented systems before halving the size, will
this cause the page allocator to print stack traces and spam dmesg on each
expected failure?
Could the __GFP_NOWARN flag be added to the allocation request to prevent
this log spam when the higher order allocations predictably fail?
[Severity: High]
In drivers/accel/qaic/qaic_control.c inside encode_activate around line 671,
nelem is calculated based on the dynamically reduced size using the
QAIC_DBC_NELEM macro. This can result in a non-power-of-two queue size.
Later, in drivers/accel/qaic/qaic_sched.c inside set_dbc_scaling_ratio at
line 98, the credit_ratio is computed using DIV_ROUND_UP(QAIC_CREDITS, nelem).
If nelem is not a power of two, the rounding up means that a job requesting
the maximum number of elements in qaic_create_job (around line 156) will
calculate its total credits as (nelem - 1) multiplied by credit_ratio. Can
this total routinely exceed the QAIC_CREDITS limit and the DRM scheduler
maximum credit limit?
If a user submits a large execution request that triggers this, it appears
it would result in a dev_WARN and credit truncation within the DRM scheduler,
preventing the job from executing properly. How should this credit overflow
be prevented when dealing with non-power-of-two sizes?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261003023818.1161666-1-carl.vanderlip@oss.qualcomm.com?part=6
^ permalink raw reply [flat|nested] 13+ messages in thread