Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

22 participants
3666 discussions

[PATCH RFC v2 0/8] memcg: dma-buf cgroup accounting and charge transfer

by Albert Esteve

This series adds memory cgroup accounting for dma-buf allocations, building on T.J. Mercier's earlier series [1] and incorporating feedback from the v1 and LPC discussions. Patch 1 introduces the MEMCG_DMABUF stat and charge/uncharge helpers. Rather than charging at the page level via __GFP_ACCOUNT (which would require every dma-buf exporter to implement charging, and makes cross-cgroup transfer impractical without access to backing pages), we track charges externally on struct dma_buf via a MEMCG_DMABUF stat counter and route all accounting through a single path, removing the __GFP_ACCOUNT flag from system-heap allocations to avoid double counting. Patch 2 adds a charge_fd field to DMA_HEAP_IOCTL_ALLOC, accepting either a pidfd or a cgroupfd to identify the target cgroup at allocation time. If the target cgroup cannot accommodate the charge, the allocation returns -ENOMEM. Patches 3-4 add LSM hooks for the two charge_fd flavours: security_dma_buf_charge_task() gates the pidfd path (target identified by task credentials) and security_dma_buf_charge_cgroup() gates the cgroupfd path (target identified by cgroup directory label). Patches 5-6 implement the SELinux side under a dma_buf object class with charge_task and charge_cgroup permissions. For charge_cgroup, the target SID is derived from the cgroup directory's SELinux label. Patch 7 adds DMA_BUF_IOCTL_XFER_CHARGE, a general ioctl on any dma-buf fd to transfer charges (or charge) post-allocation. This covers the double-hop allocator case (SurfaceFlinger -> gralloc) and works for buffers from any exporter (DRM, V4L2, etc.). If the receiving cgroup lacks capacity for the charge, the ioctl returns -ENOMEM and the original charge is preserved. Patch 8 adds selftests. Known limitations: - Single-owner model: a dma-buf carries one memcg charge regardless of how many processes share it. - Only memcg accounting. CMA heaps would additionally need dmem controller integration. - Charging CMA-backed heaps to memcg charges unreclaimable memory: when a cgroup hits its limit, try_charge_memcg() may trigger a reclaim pass that cannot free CMA pages. A future version could move charging into per-heap callbacks. - If a transfer of an uncharged buffer fails, the buffer remains uncharged. The allocator (userspace) is expected to handle this (e.g., retry, free, ...). - The LSM hook is intentionally narrow. Broader cgroup access controls could be a separate effort. [1] https://lore.kernel.org/cgroups/20230109213809.418135-1-tjmercier@google.co… Signed-off-by: Albert Esteve <aesteve(a)redhat.com> --- Changes in v2: - Move charge_pid_fd to charge_fd, accepting cgroupfd in addition to pidfd per Christian König. - New DMA_BUF_IOCTL_XFER_CHARGE ioctl for post-allocation charge transfer on any dma-buf, not just dma-heap buffers per Christian König and T.J. Mercier. - Moved __GFP_ACCOUNT removal to patch 1 per Barry Song. - dma-buf charge/uncharge helpers take struct dma_buf * directly to avoid the need for caller to manage size calculations. - Added test_xfer_charge_cgroupfd_self to dmabuf-heap selftests. - Added LSM and SELinux hooks for cgroupfd-based charging. - Rebased to v7.3-rc7. - Link to v1: https://lore.kernel.org/r/20260512-v2_20230123_tjmercier_google_com-v1-0-63… --- Albert Esteve (7): dma-heap: charge dma-buf memory via explicit memcg security: dma-buf: Add dma_buf_charge_task LSM hook security: dma-buf: Add dma_buf_charge_cgroup LSM hook selinux: Restrict cross-cgroup dma-buf charging selinux: Restrict dma-buf direct cgroup charging dma-buf: transfer memcg charges via DMA_BUF_IOCTL_XFER_CHARGE selftests/dmabuf-heaps: Add dma-buf memcg accounting tests T.J. Mercier (1): memcg: Add MEMCG_DMABUF accounting infrastructure Documentation/admin-guide/cgroup-v2.rst | 5 + drivers/dma-buf/dma-buf.c | 80 ++++++++ drivers/dma-buf/dma-heap.c | 72 ++++++- drivers/dma-buf/heaps/system_heap.c | 2 - include/linux/dma-buf.h | 4 + include/linux/lsm_hook_defs.h | 2 + include/linux/memcontrol.h | 29 +++ include/linux/security.h | 14 ++ include/uapi/linux/dma-buf.h | 26 +++ include/uapi/linux/dma-heap.h | 7 + mm/memcontrol.c | 94 +++++++++ security/security.c | 32 +++ security/selinux/hooks.c | 40 ++++ security/selinux/include/classmap.h | 1 + tools/testing/selftests/cgroup/Makefile | 2 +- tools/testing/selftests/cgroup/test_memcontrol.c | 145 +++++++++++++- tools/testing/selftests/dmabuf-heaps/config | 1 + tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c | 222 ++++++++++++++++++++- tools/testing/selftests/dmabuf-heaps/vmtest.sh | 205 +++++++++++++++++++ 19 files changed, 974 insertions(+), 9 deletions(-) --- base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907 change-id: 20260508-v2_20230123_tjmercier_google_com-f44fcfb16530 Best regards, -- Albert Esteve <aesteve(a)redhat.com>

2 hours, 2 minutes

Re: [PATCH v3 09/16] accel/thames: Add IOCTLs for job creation and submission

by Christian König

On 10/23/26 08:39, Tomeu Vizoso wrote: > A Job is created once from a program, its parameters and its transfers, > checked then, and submitted any number of times; the params buffer's > contents may change between submissions. A submission pushes the Job > onto a core's Ring and rings the doorbell; the firmware moves the Fence > when the Job is done. Each core has a drm-gpu-scheduler instance, with > one credit per Ring entry. A submit call takes an array of Jobs, the > buffers they use for implicit synchronisation, and sync objects to > signal; it waits on none. > > The job submission code was initially based on Panfrost. I only skimmed over this, but what I saw looks sane to me from the dma_fence requirement side. Regards, Christian. > Assisted-by: LLM > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > > --- > > v3: > - Add Job create/destroy, for validating transfers. > - Add sync objects to the submission IOCTL. > - Allocate the done fence with the Job; run_job allocates nothing > (Christian König). > v2: > - Add thames_accel.h UAPI header (Robert Nelson). > --- > drivers/accel/thames/Kconfig | 1 + > drivers/accel/thames/Makefile | 3 + > drivers/accel/thames/thames_core.c | 36 +- > drivers/accel/thames/thames_core.h | 14 + > drivers/accel/thames/thames_device.h | 6 + > drivers/accel/thames/thames_drv.c | 43 ++ > drivers/accel/thames/thames_drv.h | 7 + > drivers/accel/thames/thames_job.c | 869 +++++++++++++++++++++++++++++++++++ > drivers/accel/thames/thames_job.h | 73 +++ > drivers/accel/thames/thames_trace.h | 77 ++++ > include/uapi/drm/thames_accel.h | 259 ++++++++++- > 11 files changed, 1381 insertions(+), 7 deletions(-) > > diff --git a/drivers/accel/thames/Kconfig b/drivers/accel/thames/Kconfig > index 47a2986fcabd..f2c32ce8d228 100644 > --- a/drivers/accel/thames/Kconfig > +++ b/drivers/accel/thames/Kconfig > @@ -6,6 +6,7 @@ config DRM_ACCEL_THAMES > depends on TI_K3_DSP_REMOTEPROC > depends on REMOTEPROC > depends on MAILBOX > + select DRM_SCHED > select DRM_GEM_DMA_HELPER > select DRM_GEM_SHMEM_HELPER > select IOMMU_IO_PGTABLE_LPAE > diff --git a/drivers/accel/thames/Makefile b/drivers/accel/thames/Makefile > index 143a829f24cf..7bd652795cf5 100644 > --- a/drivers/accel/thames/Makefile > +++ b/drivers/accel/thames/Makefile > @@ -6,4 +6,7 @@ thames-y := \ > thames_core.o \ > thames_drv.o \ > thames_gem.o \ > + thames_job.o \ > thames_transfers.o > + > +CFLAGS_thames_job.o := -I$(src) > diff --git a/drivers/accel/thames/thames_core.c b/drivers/accel/thames/thames_core.c > index dff76abd8951..85e8f48aec60 100644 > --- a/drivers/accel/thames/thames_core.c > +++ b/drivers/accel/thames/thames_core.c > @@ -25,6 +25,7 @@ > #include "thames_core.h" > #include "thames_device.h" > #include "thames_gem.h" > +#include "thames_job.h" > #include "thames_transfers.h" > > #define THAMES_RESTART_TIMEOUT_MS 5000 > @@ -327,6 +328,7 @@ static void thames_mbox_rx(struct mbox_client *cl, void *mssg) > > dev_dbg(core->dev, "Doorbell on " THAMES_CORE_FMT ": 0x%x\n", THAMES_CORE_ARG(core), > (u32)(uintptr_t)mssg); > + thames_job_complete(core); > } > > /* Embedded in struct thames_core: nothing to free. */ > @@ -515,10 +517,18 @@ int thames_core_init(struct thames_core *core) > if (err) > goto err_timers; > > - err = thames_core_request_doorbell(core); > + /* > + * Before the doorbell: requesting it delivers at once a word the core > + * wrote while no driver was loaded. > + */ > + err = thames_job_init(core); > if (err) > goto err_unmap; > > + err = thames_core_request_doorbell(core); > + if (err) > + goto err_job; > + > if (rproc->state == RPROC_OFFLINE) { > err = rproc_boot(rproc); > if (err) > @@ -540,6 +550,8 @@ int thames_core_init(struct thames_core *core) > > err_doorbell: > thames_core_free_doorbell(core); > +err_job: > + thames_job_fini(core); > err_unmap: > thames_core_unmap_ipc(core); > err_timers: > @@ -554,6 +566,7 @@ void thames_core_fini(struct thames_core *core) > { > if (core->mbox_chan) > thames_core_free_doorbell(core); > + thames_job_fini(core); > /* > * Drop only our reference and keep the boot count: the core runs on > * for the next load to reattach to; only a reset or a suspend, which > @@ -567,6 +580,21 @@ void thames_core_fini(struct thames_core *core) > core->of_node = NULL; > } > > +/* The system firmware accepts a stop only of a core in IDLE. */ > +bool thames_core_idles(struct thames_core *core) > +{ > + u32 head = READ_ONCE(core->ipc->ring.head); > + u32 slot = head % THAMES_RING_DEPTH; > + struct thames_fault_record *record = &core->ipc->fault_records[slot]; > + > + if (head == core->ring_tail) > + return true; > + > + /* Read the fault record after the head that says it is there. */ > + rmb(); > + return READ_ONCE(record->kind) == DRM_THAMES_FAULT_FIRMWARE; > +} > + > /* > * rproc_shutdown() reaches the system firmware because the core has one power > * count, remoteproc's auto-boot or this driver's boot: the probe defers while > @@ -577,6 +605,10 @@ int thames_core_suspend(struct thames_core *core) > struct rproc *rproc = core->rproc; > int err; > > + err = thames_job_suspend(core); > + if (err) > + return err; > + > if (core->suspended) > return 0; > > @@ -589,6 +621,7 @@ int thames_core_suspend(struct thames_core *core) > err = -EBUSY; > } > if (err) { > + thames_job_resume(core); > return err; > } > > @@ -640,5 +673,6 @@ int thames_core_resume(struct thames_core *core) > > offline: > restart: > + thames_job_resume(core); > return err; > } > diff --git a/drivers/accel/thames/thames_core.h b/drivers/accel/thames/thames_core.h > index 590c5716f7f5..a78414d18ad3 100644 > --- a/drivers/accel/thames/thames_core.h > +++ b/drivers/accel/thames/thames_core.h > @@ -10,6 +10,7 @@ > #include <linux/mutex_types.h> > #include <linux/remoteproc.h> > > +#include <drm/gpu_scheduler.h> > > #include "thames_abi.h" > > @@ -44,13 +45,25 @@ struct thames_core { > struct mbox_chan *mbox_chan; > > u32 ring_tail; > + struct list_head submitted; > + struct list_head held; > + > + struct mutex job_lock; > + > + spinlock_t fence_lock; > + > /* > * @rproc_caller is the task inside the driver's own rproc_boot() or > * rproc_shutdown(), whose subdevice callbacks then do nothing. > */ > struct task_struct *rproc_caller; > > + struct drm_gpu_scheduler sched; > + u64 fence_context; > + u64 emit_seqno; > + > bool suspended; > + unsigned long timeout_left; > > struct resource l2sram; > struct resource l2aux; > @@ -81,6 +94,7 @@ static inline const struct resource *thames_core_window(const struct thames_core > > int thames_core_init(struct thames_core *core); > void thames_core_fini(struct thames_core *core); > +bool thames_core_idles(struct thames_core *core); > int thames_core_suspend(struct thames_core *core); > int thames_core_resume(struct thames_core *core); > > diff --git a/drivers/accel/thames/thames_device.h b/drivers/accel/thames/thames_device.h > index e183c78e2acf..0a34764885df 100644 > --- a/drivers/accel/thames/thames_device.h > +++ b/drivers/accel/thames/thames_device.h > @@ -36,7 +36,10 @@ struct thames_caps { > struct thames_device { > struct drm_device ddev; > > + struct mutex sched_lock; > + > struct thames_core cores[THAMES_MAX_CORES]; > + struct drm_gpu_scheduler *sched_list[THAMES_MAX_CORES]; > unsigned int num_cores; > > struct thames_caps caps; > @@ -72,6 +75,9 @@ struct thames_device { > */ > struct mutex bindings_lock; > > + /* Whoever takes a file off the list closes its Jobs. */ > + struct mutex files_lock; > + struct list_head files; > }; > > #define to_thames_device(drm_dev) \ > diff --git a/drivers/accel/thames/thames_drv.c b/drivers/accel/thames/thames_drv.c > index 030c60963a0a..fc3328834162 100644 > --- a/drivers/accel/thames/thames_drv.c > +++ b/drivers/accel/thames/thames_drv.c > @@ -23,6 +23,7 @@ > #include "thames_core.h" > #include "thames_drv.h" > #include "thames_gem.h" > +#include "thames_job.h" > #include "thames_transfers.h" > > static struct platform_device *drm_dev; > @@ -54,14 +55,23 @@ static int thames_open(struct drm_device *dev, struct drm_file *file) > > file->driver_priv = thames_priv; > > + ret = thames_job_open(thames_priv); > + if (ret) > + goto err_free; > + > ret = thames_context_create(thames_priv); > if (ret) > goto err_close_job; > > + scoped_guard(mutex, &tdev->files_lock) > + list_add_tail(&thames_priv->file_node, &tdev->files); > + > drm_dev_exit(idx); > return 0; > > err_close_job: > + thames_job_close(thames_priv); > +err_free: > kfree(thames_priv); > err_exit: > drm_dev_exit(idx); > @@ -87,6 +97,14 @@ void thames_file_priv_put(struct thames_file_priv *priv) > static void thames_postclose(struct drm_device *dev, struct drm_file *file) > { > struct thames_file_priv *thames_priv = file->driver_priv; > + struct thames_device *tdev = thames_priv->tdev; > + > + scoped_guard(mutex, &tdev->files_lock) { > + if (!list_empty(&thames_priv->file_node)) { > + list_del_init(&thames_priv->file_node); > + thames_job_close(thames_priv); > + } > + } > > dev_dbg(dev->dev, "Context %u: file closed, %u references left\n", > thames_priv->context_id, kref_read(&thames_priv->refcount) - 1); > @@ -208,6 +226,9 @@ static int thames_ioctl_##func##_entered(struct drm_device *dev, void *data, \ > THAMES_IOCTL_ENTERED(bo_create) > THAMES_IOCTL_ENTERED(bo_mmap_offset) > THAMES_IOCTL_ENTERED(dev_query) > +THAMES_IOCTL_ENTERED(job_create) > +THAMES_IOCTL_ENTERED(job_submit) > +THAMES_IOCTL_ENTERED(job_destroy) > THAMES_IOCTL_ENTERED(bo_bind) > > static const struct drm_ioctl_desc thames_drm_driver_ioctls[] = { > @@ -215,6 +236,9 @@ static const struct drm_ioctl_desc thames_drm_driver_ioctls[] = { > THAMES_IOCTL(BO_CREATE, bo_create_entered), > THAMES_IOCTL(BO_MMAP_OFFSET, bo_mmap_offset_entered), > THAMES_IOCTL(DEV_QUERY, dev_query_entered), > + THAMES_IOCTL(JOB_CREATE, job_create_entered), > + THAMES_IOCTL(JOB_SUBMIT, job_submit_entered), > + THAMES_IOCTL(JOB_DESTROY, job_destroy_entered), > THAMES_IOCTL(BO_BIND, bo_bind_entered), > }; > > @@ -272,6 +296,10 @@ static struct thames_device *thames_device_init(struct platform_device *pdev, > if (err) > return ERR_PTR(err); > > + err = drmm_mutex_init(&tdev->ddev, &tdev->sched_lock); > + if (err) > + return ERR_PTR(err); > + > err = drmm_mutex_init(&tdev->ddev, &tdev->transfer_table_pool_lock); > if (err) > return ERR_PTR(err); > @@ -288,8 +316,13 @@ static struct thames_device *thames_device_init(struct platform_device *pdev, > if (err) > return ERR_PTR(err); > > + err = drmm_mutex_init(&tdev->ddev, &tdev->files_lock); > + if (err) > + return ERR_PTR(err); > + > INIT_LIST_HEAD(&tdev->transfer_tables); > INIT_LIST_HEAD(&tdev->contexts); > + INIT_LIST_HEAD(&tdev->files); > > tdev->asid_next = 1; > /* > @@ -347,6 +380,7 @@ static int thames_platform_probe(struct platform_device *pdev) > continue; > } > > + tdev->sched_list[core] = &tdev->cores[core].sched; > tdev->num_cores++; > core++; > } > @@ -391,6 +425,15 @@ static void thames_platform_remove(struct platform_device *pdev) > > drm_dev_unplug(&tdev->ddev); > > + scoped_guard(mutex, &tdev->files_lock) { > + struct thames_file_priv *priv, *tmp; > + > + list_for_each_entry_safe(priv, tmp, &tdev->files, file_node) { > + list_del_init(&priv->file_node); > + thames_job_close(priv); > + } > + } > + > for (core = 0; core < tdev->num_cores; core++) > thames_core_fini(&tdev->cores[core]); > tdev->num_cores = 0; > diff --git a/drivers/accel/thames/thames_drv.h b/drivers/accel/thames/thames_drv.h > index 7f9031b63780..2115f7eae589 100644 > --- a/drivers/accel/thames/thames_drv.h > +++ b/drivers/accel/thames/thames_drv.h > @@ -11,6 +11,7 @@ > #include <linux/xarray.h> > > #include <drm/drm_mm.h> > +#include <drm/gpu_scheduler.h> > > #include "thames_device.h" > > @@ -26,6 +27,12 @@ struct thames_file_priv { > */ > struct kref refcount; > > + struct list_head file_node; > + > + struct drm_sched_entity sched_entity; > + > + struct xarray jobs; > + > struct io_pgtable_ops *pgtbl_ops[THAMES_MAX_CORES]; > u64 page_table_base[THAMES_MAX_CORES]; > u32 asid; > diff --git a/drivers/accel/thames/thames_job.c b/drivers/accel/thames/thames_job.c > new file mode 100644 > index 000000000000..6560106d47a8 > --- /dev/null > +++ b/drivers/accel/thames/thames_job.c > @@ -0,0 +1,869 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright 2019 Linaro, Ltd, Rob Herring <robh(a)kernel.org> */ > +/* Copyright 2019 Collabora ltd. */ > +/* Copyright 2024-2025 Tomeu Vizoso <tomeu(a)tomeuvizoso.net> */ > +/* Copyright 2026 Texas Instruments Incorporated - https://www.ti.com/ */ > + > +#include <clocksource/arm_arch_timer.h> > +#include <linux/dev_printk.h> > +#include <linux/dma-fence-chain.h> > +#include <linux/iopoll.h> > +#include <linux/mailbox_client.h> > +#include <linux/platform_device.h> > +#include <linux/xarray.h> > + > +#include <drm/drm_drv.h> > +#include <drm/drm_file.h> > +#include <drm/drm_gem.h> > +#include <drm/drm_print.h> > +#include <drm/drm_syncobj.h> > +#include <drm/thames_accel.h> > + > +#include "thames_core.h" > +#include "thames_device.h" > +#include "thames_drv.h" > +#include "thames_gem.h" > +#include "thames_job.h" > + > +#define CREATE_TRACE_POINTS > +#include "thames_trace.h" > + > +static struct thames_sched_job *to_thames_sched_job(struct drm_sched_job *sched_job) > +{ > + return container_of(sched_job, struct thames_sched_job, base); > +} > + > +static void thames_sched_job_put(struct thames_sched_job *job); > +static void thames_job_put(struct thames_job *job); > + > +#define THAMES_MAX_JOBS_PER_SUBMIT 256 > +#define THAMES_MAX_SUBMIT_BOS 4096 > +#define THAMES_MAX_SUBMIT_SYNCS 64 > + > +/* > + * The host's timeout is longer than the device's: a Job still on the Ring when > + * it fires is on a stalled core. > + */ > +#define THAMES_DEVICE_TIMEOUT_MS 500 > + > +struct thames_submit_bos { > + struct kref refcount; > + u32 read_count; > + u32 count; > + struct drm_gem_object *bos[] __counted_by(count); > +}; > + > +static void thames_submit_bos_free(struct kref *ref) > +{ > + struct thames_submit_bos *bos = container_of(ref, struct thames_submit_bos, refcount); > + u32 i; > + > + for (i = 0; i < bos->count; i++) { > + if (bos->bos[i]) > + drm_gem_object_put(bos->bos[i]); > + } > + > + kvfree(bos); > +} > + > +static void thames_submit_bos_put(struct thames_submit_bos *bos) > +{ > + if (bos) > + kref_put(&bos->refcount, thames_submit_bos_free); > +} > + > +static const char *thames_fence_get_driver_name(struct dma_fence *fence) > +{ > + return "thames"; > +} > + > +static const char *thames_fence_get_timeline_name(struct dma_fence *fence) > +{ > + return "thames"; > +} > + > +static const struct dma_fence_ops thames_fence_ops = { > + .get_driver_name = thames_fence_get_driver_name, > + .get_timeline_name = thames_fence_get_timeline_name, > +}; > + > +static void thames_job_fail_done_fence(struct thames_sched_job *job, int error) > +{ > + if (!dma_fence_was_initialized(job->done_fence) || dma_fence_is_signaled(job->done_fence)) > + return; > + > + dma_fence_set_error(job->done_fence, error); > + dma_fence_signal(job->done_fence); > +} > + > +static int thames_job_hw_submit(struct thames_core *core, struct thames_sched_job *sched_job) > +{ > + struct thames_ring *ring = &core->ipc->ring; > + struct thames_ring_host *host; > + u32 tail = core->ring_tail; > + u32 index = tail % THAMES_RING_DEPTH; > + int ret; > + > + if (tail - READ_ONCE(ring->head) >= THAMES_RING_DEPTH) { > + dev_err(core->dev, "Ring full on " THAMES_CORE_FMT "\n", THAMES_CORE_ARG(core)); > + return -ENOSPC; > + } > + > + sched_job->stamps[0] = arch_timer_read_counter(); > + sched_job->ring_index = index; > + > + host = &ring->entries[index].host; > + host->context_id = sched_job->file_priv->context_id; > + host->transfer_count = sched_job->job->transfers.count; > + host->transfer_group_count = sched_job->job->transfers.group_count; > + host->transfer_table_offset = sched_job->job->transfers.offset; > + host->seqno = sched_job->seqno; > + host->program_dsp_va = sched_job->job->program_dsp_va; > + host->params_dsp_va = sched_job->job->params_dsp_va; > + host->context_serial = sched_job->file_priv->context_serial; > + host->timeout_ms = THAMES_DEVICE_TIMEOUT_MS; > + > + scoped_guard(mutex, &core->job_lock) { > + kref_get(&sched_job->refcount); /* Put by thames_job_complete(). */ > + list_add_tail(&sched_job->node, &core->submitted); > + > + /* Order the host half before the tail. */ > + wmb(); > + WRITE_ONCE(ring->tail, tail + 1); > + core->ring_tail = tail + 1; > + /* Order the tail before the doorbell. */ > + wmb(); > + } > + > + /* > + * The Job is on the Ring whatever the send returns; it completes > + * through the Fence or the timeout. > + */ > + ret = mbox_send_message(core->mbox_chan, (void *)(uintptr_t)THAMES_DOORBELL); > + if (ret < 0) > + dev_warn_ratelimited(core->dev, "Doorbell not sent: %d\n", ret); > + > + return 0; > +} > + > +void thames_job_complete(struct thames_core *core) > +{ > + struct thames_sched_job *job, *tmp; > + LIST_HEAD(done); > + u64 completed, signalled; > + > + completed = READ_ONCE(core->ipc->fence.seqno); > + /* Read the entries after the Fence that covers them. */ > + rmb(); > + > + scoped_guard(mutex, &core->job_lock) { > + list_for_each_entry_safe(job, tmp, &core->submitted, node) { > + if (job->seqno > completed) > + break; > + list_move_tail(&job->node, &done); > + } > + } > + > + list_for_each_entry_safe(job, tmp, &done, node) { > + struct thames_ring_fw *fw = &core->ipc->ring.entries[job->ring_index].fw; > + > + job->stamps[1] = fw->doorbell_seen_stamp; > + job->stamps[2] = fw->run_start_stamp; > + job->stamps[3] = fw->run_end_stamp; > + job->stamps[4] = fw->kernel_entry_stamp; > + job->stamps[5] = fw->kernel_exit_stamp; > + > + if (fw->fence_value == THAMES_FENCE_ERROR) { > + dma_fence_set_error(job->done_fence, -EIO); > + } > + > + list_del_init(&job->node); > + signalled = arch_timer_read_counter(); > + dma_fence_signal(job->done_fence); > + trace_thames_job_done(core, job, signalled); > + thames_sched_job_put(job); > + } > +} > + > +static int thames_acquire_object_fences(struct drm_gem_object **bos, int bo_count, > + struct drm_sched_job *job, bool is_write) > +{ > + int i, ret; > + > + for (i = 0; i < bo_count; i++) { > + ret = dma_resv_reserve_fences(bos[i]->resv, 1); > + if (ret) > + return ret; > + > + ret = drm_sched_job_add_implicit_dependencies(job, bos[i], is_write); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static void thames_attach_object_fences(struct drm_gem_object **bos, int bo_count, > + struct dma_fence *fence, enum dma_resv_usage usage) > +{ > + int i; > + > + for (i = 0; i < bo_count; i++) > + dma_resv_add_fence(bos[i]->resv, fence, usage); > +} > + > +/* Nothing here fails, so a submit call pushes every Job or none. */ > +static void thames_job_push(struct thames_sched_job *sched_job) > +{ > + scoped_guard(mutex, &sched_job->tdev->sched_lock) { > + drm_sched_job_arm(&sched_job->base); > + > + sched_job->inference_done_fence = dma_fence_get(&sched_job->base.s_fence->finished); > + > + kref_get(&sched_job->refcount); /* put by scheduler job completion */ > + > + drm_sched_entity_push_job(&sched_job->base); > + } > +} > + > +static void thames_sched_job_cleanup(struct kref *ref) > +{ > + struct thames_sched_job *sched_job = container_of(ref, struct thames_sched_job, refcount); > + > + if (dma_fence_was_initialized(sched_job->done_fence)) > + dma_fence_put(sched_job->done_fence); > + else > + dma_fence_free(sched_job->done_fence); > + dma_fence_put(sched_job->inference_done_fence); > + > + thames_submit_bos_put(sched_job->bos); > + > + if (sched_job->file_priv) > + thames_file_priv_put(sched_job->file_priv); > + > + if (sched_job->job) > + thames_job_put(sched_job->job); > + > + kfree(sched_job); > +} > + > +static void thames_sched_job_put(struct thames_sched_job *job) > +{ > + kref_put(&job->refcount, thames_sched_job_cleanup); > +} > + > +static void thames_sched_job_free(struct drm_sched_job *sched_job) > +{ > + struct thames_sched_job *job = to_thames_sched_job(sched_job); > + > + drm_sched_job_cleanup(sched_job); > + > + thames_sched_job_put(job); > +} > + > +static struct dma_fence *thames_job_run(struct drm_sched_job *sched_job) > +{ > + struct thames_sched_job *job = to_thames_sched_job(sched_job); > + struct thames_core *core = container_of(sched_job->sched, struct thames_core, sched); > + int ret; > + > + if (unlikely(job->base.s_fence->finished.error)) > + return NULL; > + > + /* The done fence keeps the seqno of the Job's first push. */ > + job->seqno = ++core->emit_seqno; > + if (!dma_fence_was_initialized(job->done_fence)) > + dma_fence_init(job->done_fence, &thames_fence_ops, &core->fence_lock, > + core->fence_context, job->seqno); > + > + ret = thames_job_hw_submit(core, job); > + if (ret) { > + thames_job_fail_done_fence(job, ret); > + return ERR_PTR(ret); > + } > + > + return dma_fence_get(job->done_fence); > +} > + > +/* > + * A done fence not signalled here is a Job on a live core's Ring that may still > + * run. > + */ > +static void thames_job_cancel(struct drm_sched_job *sched_job) > +{ > + struct thames_sched_job *job = to_thames_sched_job(sched_job); > + struct thames_core *core = container_of(sched_job->sched, struct thames_core, sched); > + > + if (!dma_fence_was_initialized(job->done_fence) || dma_fence_is_signaled(job->done_fence)) > + return; > + > + scoped_guard(mutex, &core->job_lock) > + list_move_tail(&job->node, &core->held); > + > + thames_job_fail_done_fence(job, -ECANCELED); > +} > + > +static const struct drm_sched_backend_ops thames_sched_ops = { > + .run_job = thames_job_run, > + .free_job = thames_sched_job_free, > + .cancel_job = thames_job_cancel, > +}; > + > +int thames_job_init(struct thames_core *core) > +{ > + /* One credit per Ring entry, so the tail never overruns the head. */ > + struct drm_sched_init_args args = { > + .ops = &thames_sched_ops, > + .num_rqs = 1, > + .credit_limit = THAMES_RING_DEPTH, > + .timeout = MAX_SCHEDULE_TIMEOUT, > + .name = dev_name(core->dev), > + .dev = core->dev, > + }; > + u32 head, tail, before; > + u64 fence, before_seqno, last; > + int ret; > + > + spin_lock_init(&core->fence_lock); > + mutex_init(&core->job_lock); > + > + INIT_LIST_HEAD(&core->submitted); > + INIT_LIST_HEAD(&core->held); > + > + /* > + * Both indices run free, so only a tail not at the head is wrong: the > + * previous instance's entries are still pending. The Fence may trail > + * the head by one entry, the firmware moving the head first. > + */ > + head = READ_ONCE(core->ipc->ring.head); > + tail = READ_ONCE(core->ipc->ring.tail); > + before = (head - 1) % THAMES_RING_DEPTH; > + > + if (tail != head) { > + dev_err(core->dev, "Ring on " THAMES_CORE_FMT > + " has %u pending entries (tail %u, head %u); not attaching\n", > + THAMES_CORE_ARG(core), tail - head, tail, head); > + return -EBUSY; > + } > + > + /* Read the Fence and the entry after the head and the tail. */ > + rmb(); > + fence = READ_ONCE(core->ipc->fence.seqno); > + before_seqno = core->ipc->ring.entries[before].host.seqno; > + last = max(fence, before_seqno); > + > + core->ring_tail = head; > + core->emit_seqno = last; > + > + core->fence_context = dma_fence_context_alloc(1); > + > + ret = drm_sched_init(&core->sched, &args); > + if (ret) { > + dev_err(core->dev, "Failed to create scheduler: %d.", ret); > + return ret; > + } > + > + return 0; > +} > + > +/* > + * These Jobs' core may still run their Kernels: the Jobs, Contexts and buffers > + * stay allocated. > + */ > +void thames_job_fini(struct thames_core *core) > +{ > + struct thames_sched_job *job; > + u64 first = U64_MAX, last = 0; > + size_t kept; > + > + drm_sched_fini(&core->sched); > + > + guard(mutex)(&core->job_lock); > + > + kept = list_count_nodes(&core->held); > + if (!kept) > + return; > + > + list_for_each_entry(job, &core->held, node) { > + first = min(first, job->seqno); > + last = max(last, job->seqno); > + } > + > + drm_dev_get(&core->tdev->ddev); > + dev_warn(core->dev, THAMES_CORE_FMT > + " may still run %zu Jobs (seqno %llu to %llu); keeping them and their buffers until reboot\n", > + THAMES_CORE_ARG(core), kept, first, last); > +} > + > +/* > + * Above the device's timeout, so the firmware ends a runaway Kernel within the > + * wait. > + */ > +#define THAMES_SUSPEND_TIMEOUT_MS (THAMES_DEVICE_TIMEOUT_MS + 100) > + > +int thames_job_suspend(struct thames_core *core) > +{ > + struct drm_gpu_scheduler *sched = &core->sched; > + unsigned int wait_ms; > + size_t pending; > + bool idle; > + int err; > + > + drm_sched_wqueue_stop(sched); > + core->timeout_left = drm_sched_suspend_timeout(sched); > + cancel_delayed_work_sync(&sched->work_tdr); > + drm_sched_wqueue_stop(sched); > + > + scoped_guard(mutex, &core->job_lock) > + pending = list_count_nodes(&core->submitted); > + wait_ms = max_t(unsigned int, pending, 1) * THAMES_SUSPEND_TIMEOUT_MS; > + > + err = read_poll_timeout(thames_core_idles, idle, idle, USEC_PER_MSEC, > + wait_ms * USEC_PER_MSEC, false, core); > + > + if (!err) { > + thames_job_complete(core); > + scoped_guard(mutex, &core->job_lock) { > + if (!list_empty(&core->submitted)) > + err = -EBUSY; > + } > + } > + > + if (err) { > + dev_err(core->dev, THAMES_CORE_FMT > + " is not idle with its Jobs completed after %u ms; refusing the suspend\n", > + THAMES_CORE_ARG(core), wait_ms); > + thames_job_resume(core); > + return -EBUSY; > + } > + > + return 0; > +} > + > +void thames_job_resume(struct thames_core *core) > +{ > + drm_sched_resume_timeout(&core->sched, core->timeout_left); > + drm_sched_wqueue_start(&core->sched); > +} > + > +int thames_job_open(struct thames_file_priv *thames_priv) > +{ > + struct thames_device *tdev = thames_priv->tdev; > + int ret; > + > + ret = drm_sched_entity_init(&thames_priv->sched_entity, 0, tdev->sched_list, > + tdev->num_cores, NULL); > + if (WARN_ON(ret)) > + return ret; > + > + xa_init_flags(&thames_priv->jobs, XA_FLAGS_ALLOC1); > + > + return 0; > +} > + > +void thames_job_close(struct thames_file_priv *thames_priv) > +{ > + struct thames_job *job; > + unsigned long handle; > + > + drm_sched_entity_destroy(&thames_priv->sched_entity); > + > + xa_for_each(&thames_priv->jobs, handle, job) > + thames_job_put(job); > + xa_destroy(&thames_priv->jobs); > +} > + > +static void thames_job_free(struct kref *ref) > +{ > + struct thames_job *job = container_of(ref, struct thames_job, refcount); > + > + if (job->program) > + drm_gem_object_put(job->program); > + if (job->params) > + drm_gem_object_put(job->params); > + > + thames_transfers_free(job->tdev, &job->transfers); > + mutex_destroy(&job->lock); > + kfree(job); > +} > + > +static void thames_job_put(struct thames_job *job) > +{ > + kref_put(&job->refcount, thames_job_free); > +} > + > +int thames_ioctl_job_create(struct drm_device *dev, void *data, struct drm_file *file) > +{ > + struct drm_thames_job_create *args = data; > + struct thames_file_priv *file_priv = file->driver_priv; > + struct thames_device *tdev = to_thames_device(dev); > + struct thames_job *job; > + u32 handle; > + int ret; > + > + if (args->flags || args->pad) > + return -EINVAL; > + > + if (args->type != DRM_THAMES_JOB_DISPATCH) > + return -EINVAL; > + > + if (!args->program || !args->params) > + return -EINVAL; > + > + job = kzalloc_obj(*job, GFP_KERNEL); > + if (!job) > + return -ENOMEM; > + > + kref_init(&job->refcount); > + mutex_init(&job->lock); > + job->tdev = tdev; > + > + job->program = drm_gem_object_lookup(file, args->program); > + if (!job->program) { > + ret = -ENOENT; > + goto err_put; > + } > + > + if (!(to_thames_bo(job->program)->flags & DRM_THAMES_BO_EXECUTABLE)) { > + ret = -EINVAL; > + goto err_put; > + } > + > + job->program_dsp_va = thames_bo_dsp_va(to_thames_bo(job->program), file_priv); > + if (!job->program_dsp_va) { > + ret = -EINVAL; > + goto err_put; > + } > + > + job->params = drm_gem_object_lookup(file, args->params); > + if (!job->params) { > + ret = -ENOENT; > + goto err_put; > + } > + > + job->params_dsp_va = thames_bo_dsp_va(to_thames_bo(job->params), file_priv); > + if (!job->params_dsp_va) { > + ret = -EINVAL; > + goto err_put; > + } > + > + ret = thames_transfers_build(file, &args->transfers, &job->transfers); > + if (ret) > + goto err_put; > + > + ret = xa_alloc(&file_priv->jobs, &handle, job, xa_limit_32b, GFP_KERNEL); > + if (ret) > + goto err_put; > + > + job->handle = handle; > + args->handle = handle; > + > + return 0; > + > +err_put: > + thames_job_put(job); > + return ret; > +} > + > +static struct thames_sched_job *thames_sched_job_new(struct drm_file *file, u32 handle, > + struct thames_submit_bos *bos) > +{ > + struct thames_file_priv *file_priv = file->driver_priv; > + struct thames_job *job; > + struct thames_sched_job *sched_job; > + int ret; > + > + xa_lock(&file_priv->jobs); > + job = xa_load(&file_priv->jobs, handle); > + if (job) > + kref_get(&job->refcount); > + xa_unlock(&file_priv->jobs); > + > + if (!job) > + return ERR_PTR(-ENOENT); > + > + sched_job = kzalloc_obj(*sched_job, GFP_KERNEL); > + if (!sched_job) { > + thames_job_put(job); > + return ERR_PTR(-ENOMEM); > + } > + > + kref_init(&sched_job->refcount); > + INIT_LIST_HEAD(&sched_job->node); > + sched_job->tdev = job->tdev; > + sched_job->job = job; /* The lookup's reference. */ > + /* Put by the completion path once it has counted the Job for the ban. */ > + kref_get(&file_priv->refcount); > + sched_job->file_priv = file_priv; > + kref_get(&bos->refcount); > + sched_job->bos = bos; > + > + sched_job->done_fence = kzalloc_obj(*sched_job->done_fence); > + if (!sched_job->done_fence) { > + thames_sched_job_put(sched_job); > + return ERR_PTR(-ENOMEM); > + } > + > + ret = drm_sched_job_init(&sched_job->base, &file_priv->sched_entity, 1, NULL, > + file->client_id); > + if (ret) { > + thames_sched_job_put(sched_job); > + return ERR_PTR(ret); > + } > + > + return sched_job; > +} > + > +static int thames_submit_bos_lookup(struct drm_file *file, > + const struct drm_thames_job_submit *args, > + struct thames_submit_bos **out) > +{ > + const struct drm_thames_obj_array *lists[2] = { &args->read_handles, > + &args->write_handles }; > + struct thames_submit_bos *bos; > + u32 total = 0, n = 0, l, i; > + > + for (l = 0; l < ARRAY_SIZE(lists); l++) { > + if (lists[l]->count > THAMES_MAX_SUBMIT_BOS - total) > + return -EINVAL; > + total += lists[l]->count; > + } > + > + bos = kvzalloc(struct_size(bos, bos, total), GFP_KERNEL); > + if (!bos) > + return -ENOMEM; > + > + kref_init(&bos->refcount); > + bos->count = total; > + bos->read_count = args->read_handles.count; > + > + for (l = 0; l < ARRAY_SIZE(lists); l++) { > + u32 *handles; > + > + if (!lists[l]->count) > + continue; > + > + handles = thames_get_uobj_array(lists[l], sizeof(*handles), sizeof(*handles)); > + if (IS_ERR(handles)) { > + thames_submit_bos_put(bos); > + return PTR_ERR(handles); > + } > + > + for (i = 0; i < lists[l]->count; i++, n++) { > + bos->bos[n] = drm_gem_object_lookup(file, handles[i]); > + if (!bos->bos[n]) { > + kvfree(handles); > + thames_submit_bos_put(bos); > + return -ENOENT; > + } > + } > + > + kvfree(handles); > + } > + > + *out = bos; > + return 0; > +} > + > +/* > + * Allocated before the first push, so signalling cannot fail once the Jobs run. > + */ > +struct thames_submit_syncs { > + u32 count; > + struct drm_thames_sync_op *ops; > + struct thames_sync_signal { > + struct drm_syncobj *syncobj; > + struct dma_fence_chain *chain; > + } *signals; > +}; > + > +static int thames_sync_op_check(const struct drm_thames_sync_op *op) > +{ > + if (op->flags != (DRM_THAMES_SYNC_OP_HANDLE_TYPE_TIMELINE_SYNCOBJ | > + DRM_THAMES_SYNC_OP_SIGNAL)) > + return -EINVAL; > + > + return op->timeline_value ? 0 : -EINVAL; > +} > + > +static void thames_submit_syncs_free(struct thames_submit_syncs *syncs) > +{ > + u32 i; > + > + for (i = 0; i < syncs->count; i++) { > + if (syncs->signals[i].syncobj) > + drm_syncobj_put(syncs->signals[i].syncobj); > + dma_fence_chain_free(syncs->signals[i].chain); > + } > + > + kvfree(syncs->signals); > + kvfree(syncs->ops); > +} > + > +static int thames_submit_syncs_get(struct drm_file *file, const struct drm_thames_obj_array *in, > + struct thames_submit_syncs *syncs) > +{ > + struct drm_thames_sync_op *ops; > + u32 i; > + int ret; > + > + if (!in->count) > + return 0; > + > + if (in->count > THAMES_MAX_SUBMIT_SYNCS) > + return -EINVAL; > + > + ops = thames_get_uobj_array(in, offsetofend(struct drm_thames_sync_op, timeline_value), > + sizeof(*ops)); > + if (IS_ERR(ops)) > + return PTR_ERR(ops); > + syncs->ops = ops; > + > + syncs->signals = kvcalloc(in->count, sizeof(*syncs->signals), GFP_KERNEL); > + if (!syncs->signals) > + return -ENOMEM; > + syncs->count = in->count; > + > + for (i = 0; i < syncs->count; i++) { > + struct thames_sync_signal *signal = &syncs->signals[i]; > + > + ret = thames_sync_op_check(&ops[i]); > + if (ret) > + return ret; > + > + signal->syncobj = drm_syncobj_find(file, ops[i].handle); > + if (!signal->syncobj) > + return -ENOENT; > + > + signal->chain = dma_fence_chain_alloc(); > + if (!signal->chain) > + return -ENOMEM; > + } > + > + return 0; > +} > + > +static void thames_submit_syncs_signal(struct thames_submit_syncs *syncs, struct dma_fence *fence) > +{ > + u32 i; > + > + for (i = 0; i < syncs->count; i++) { > + struct thames_sync_signal *signal = &syncs->signals[i]; > + > + drm_syncobj_add_point(signal->syncobj, signal->chain, fence, > + syncs->ops[i].timeline_value); > + signal->chain = NULL; > + } > +} > + > +int thames_ioctl_job_submit(struct drm_device *dev, void *data, struct drm_file *file) > +{ > + struct drm_thames_job_submit *args = data; > + struct thames_submit_syncs syncs = {}; > + struct thames_submit_bos *bos = NULL; > + struct ww_acquire_ctx acquire_ctx; > + struct thames_sched_job **subs = NULL; > + struct dma_fence *last; > + u32 count = args->jobs.count; > + u32 pushed = 0; > + u32 *handles; > + unsigned int i; > + int ret = 0; > + > + if (args->flags || args->pad) > + return -EINVAL; > + > + if (!count || count > THAMES_MAX_JOBS_PER_SUBMIT) > + return -EINVAL; > + > + handles = thames_get_uobj_array(&args->jobs, sizeof(u32), sizeof(u32)); > + if (IS_ERR(handles)) > + return PTR_ERR(handles); > + > + ret = thames_submit_syncs_get(file, &args->syncs, &syncs); > + if (ret) > + goto out_syncs; > + > + ret = thames_submit_bos_lookup(file, args, &bos); > + if (ret) > + goto out_syncs; > + > + subs = kvcalloc(count, sizeof(*subs), GFP_KERNEL); > + if (!subs) { > + ret = -ENOMEM; > + goto out_put; > + } > + > + for (i = 0; i < count; i++) { > + subs[i] = thames_sched_job_new(file, handles[i], bos); > + if (IS_ERR(subs[i])) { > + ret = PTR_ERR(subs[i]); > + subs[i] = NULL; > + goto out_subs; > + } > + } > + > + ret = drm_gem_lock_reservations(bos->bos, bos->count, &acquire_ctx); > + if (ret) > + goto out_subs; > + > + /* > + * Only the first Job waits: the entity runs the call's Jobs in order. > + */ > + ret = thames_acquire_object_fences(bos->bos, bos->read_count, &subs[0]->base, false); > + if (ret) > + goto out_unlock; > + > + ret = thames_acquire_object_fences(&bos->bos[bos->read_count], bos->count - bos->read_count, > + &subs[0]->base, true); > + if (ret) > + goto out_unlock; > + > + for (pushed = 0; pushed < count; pushed++) > + thames_job_push(subs[pushed]); > + > + last = subs[count - 1]->inference_done_fence; > + thames_attach_object_fences(bos->bos, bos->read_count, last, DMA_RESV_USAGE_READ); > + thames_attach_object_fences(&bos->bos[bos->read_count], bos->count - bos->read_count, last, > + DMA_RESV_USAGE_WRITE); > + thames_submit_syncs_signal(&syncs, last); > + > +out_unlock: > + drm_gem_unlock_reservations(bos->bos, bos->count, &acquire_ctx); > +out_subs: > + for (i = 0; i < count; i++) { > + if (!subs[i]) > + continue; > + if (i >= pushed) > + drm_sched_job_cleanup(&subs[i]->base); > + /* The creation reference; the scheduler holds its own. */ > + thames_sched_job_put(subs[i]); > + } > + kvfree(subs); > +out_put: > + thames_submit_bos_put(bos); > +out_syncs: > + thames_submit_syncs_free(&syncs); > + kvfree(handles); > + return ret; > +} > + > +int thames_ioctl_job_destroy(struct drm_device *dev, void *data, struct drm_file *file) > +{ > + struct drm_thames_job_destroy *args = data; > + struct thames_file_priv *file_priv = file->driver_priv; > + struct thames_job *job; > + > + if (args->pad) > + return -EINVAL; > + > + job = xa_erase(&file_priv->jobs, args->handle); > + if (!job) > + return -ENOENT; > + > + /* A submission still running holds its own reference. */ > + thames_job_put(job); > + > + return 0; > +} > + > diff --git a/drivers/accel/thames/thames_job.h b/drivers/accel/thames/thames_job.h > new file mode 100644 > index 000000000000..a851febe8f98 > --- /dev/null > +++ b/drivers/accel/thames/thames_job.h > @@ -0,0 +1,73 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* Copyright 2024-2025 Tomeu Vizoso <tomeu(a)tomeuvizoso.net> */ > +/* Copyright 2026 Texas Instruments Incorporated - https://www.ti.com/ */ > + > +#ifndef __THAMES_JOB_H__ > +#define __THAMES_JOB_H__ > + > +#include <drm/drm_drv.h> > +#include <drm/gpu_scheduler.h> > +#include <drm/thames_accel.h> > + > +#include "thames_core.h" > +#include "thames_drv.h" > +#include "thames_transfers.h" > + > +struct thames_submit_bos; > + > +struct thames_sched_job { > + struct drm_sched_job base; > + > + struct thames_device *tdev; > + struct thames_file_priv *file_priv; > + > + struct dma_fence *inference_done_fence; > + struct dma_fence *done_fence; > + > + /* @seqno moves at every push; the done fence keeps the first push's. */ > + u64 seqno; > + u32 ring_index; > + u64 stamps[6]; > + struct list_head node; > + > + struct thames_job *job; > + > + struct thames_submit_bos *bos; > + > + struct kref refcount; > +}; > + > +struct thames_job { > + struct kref refcount; > + struct thames_device *tdev; > + u32 handle; > + > + struct drm_gem_object *program; > + struct drm_gem_object *params; > + > + /* > + * Taken at create: after an unbind the Job faults in its Context rather > + * than read a binding that is gone. > + */ > + u64 program_dsp_va; > + u64 params_dsp_va; > + > + struct thames_transfer_table transfers; > + > + struct mutex lock; > +}; > + > +int thames_ioctl_job_create(struct drm_device *dev, void *data, struct drm_file *file); > +int thames_ioctl_job_submit(struct drm_device *dev, void *data, struct drm_file *file); > +int thames_ioctl_job_destroy(struct drm_device *dev, void *data, struct drm_file *file); > + > +void thames_job_complete(struct thames_core *core); > + > +int thames_job_init(struct thames_core *core); > +int thames_job_suspend(struct thames_core *core); > +void thames_job_resume(struct thames_core *core); > +void thames_job_fini(struct thames_core *core); > +int thames_job_open(struct thames_file_priv *thames_priv); > +void thames_job_close(struct thames_file_priv *thames_priv); > + > +#endif > diff --git a/drivers/accel/thames/thames_trace.h b/drivers/accel/thames/thames_trace.h > new file mode 100644 > index 000000000000..3d1e59708869 > --- /dev/null > +++ b/drivers/accel/thames/thames_trace.h > @@ -0,0 +1,77 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* Copyright 2026 Texas Instruments Incorporated - https://www.ti.com/ */ > + > +#undef TRACE_SYSTEM > +#define TRACE_SYSTEM thames > + > +#if !defined(__THAMES_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ) > +#define __THAMES_TRACE_H__ > + > +#include <linux/math64.h> > +#include <linux/time64.h> > +#include <linux/tracepoint.h> > +#include <linux/types.h> > + > +#include "thames_core.h" > +#include "thames_device.h" > +#include "thames_drv.h" > +#include "thames_job.h" > + > +/* Every stamp is the SoC's system counter in microseconds, so any two subtract. */ > +TRACE_EVENT(thames_job_done, > + TP_PROTO(const struct thames_core *core, const struct thames_sched_job *job, > + u64 signalled), > + > + TP_ARGS(core, job, signalled), > + > + TP_STRUCT__entry(__string(dev_name, dev_name(core->dev)) > + __field(u32, core) > + __field(u32, context) > + __field(u64, seqno) > + __field(int, error) > + __field(u64, submit_us) > + __field(u64, doorbell_seen_us) > + __field(u64, run_start_us) > + __field(u64, kernel_entry_us) > + __field(u64, kernel_exit_us) > + __field(u64, run_end_us) > + __field(u64, signalled_us)), > + > + TP_fast_assign(u32 hz = core->tdev->caps.timestamp_hz; > + > + __assign_str(dev_name); > + __entry->core = core->index; > + __entry->context = job->file_priv->context_id; > + __entry->seqno = job->seqno; > + __entry->error = job->done_fence->error; > + __entry->submit_us = > + mul_u64_u32_div(job->stamps[0], USEC_PER_SEC, hz); > + __entry->doorbell_seen_us = > + mul_u64_u32_div(job->stamps[1], USEC_PER_SEC, hz); > + __entry->run_start_us = > + mul_u64_u32_div(job->stamps[2], USEC_PER_SEC, hz); > + __entry->run_end_us = > + mul_u64_u32_div(job->stamps[3], USEC_PER_SEC, hz); > + __entry->kernel_entry_us = > + mul_u64_u32_div(job->stamps[4], USEC_PER_SEC, hz); > + __entry->kernel_exit_us = > + mul_u64_u32_div(job->stamps[5], USEC_PER_SEC, hz); > + __entry->signalled_us = > + mul_u64_u32_div(signalled, USEC_PER_SEC, hz);), > + > + TP_printk("%s: core=%u context=%u seqno=%llu error=%d submit_us=%llu doorbell_seen_us=%llu run_start_us=%llu kernel_entry_us=%llu kernel_exit_us=%llu run_end_us=%llu signalled_us=%llu", > + __get_str(dev_name), __entry->core, __entry->context, > + __entry->seqno, __entry->error, __entry->submit_us, > + __entry->doorbell_seen_us, __entry->run_start_us, > + __entry->kernel_entry_us, __entry->kernel_exit_us, > + __entry->run_end_us, __entry->signalled_us) > +); > + > +#endif /* __THAMES_TRACE_H__ */ > + > +#undef TRACE_INCLUDE_PATH > +#define TRACE_INCLUDE_PATH . > +#undef TRACE_INCLUDE_FILE > +#define TRACE_INCLUDE_FILE thames_trace > + > +#include <trace/define_trace.h> > diff --git a/include/uapi/drm/thames_accel.h b/include/uapi/drm/thames_accel.h > index 5671cffaf8f9..bd2b4271b896 100644 > --- a/include/uapi/drm/thames_accel.h > +++ b/include/uapi/drm/thames_accel.h > @@ -57,6 +57,15 @@ enum drm_thames_ioctl_id { > /** @DRM_THAMES_DEV_QUERY: Query the device's capabilities. */ > DRM_THAMES_DEV_QUERY, > > + /** @DRM_THAMES_JOB_CREATE: Validate a Dispatch once into a Job. */ > + DRM_THAMES_JOB_CREATE, > + > + /** @DRM_THAMES_JOB_SUBMIT: Run created Jobs once, in order. */ > + DRM_THAMES_JOB_SUBMIT, > + > + /** @DRM_THAMES_JOB_DESTROY: Release a Job and the buffers it holds. */ > + DRM_THAMES_JOB_DESTROY, > + > /** > * @DRM_THAMES_BO_BIND: Bind buffers into the file's Context at the > * addresses the client chooses, or unbind them. > @@ -244,6 +253,21 @@ enum drm_thames_dev_query_type { > > }; > > +/** > + * enum drm_thames_job_type - What a Job is > + * > + * A Job is one Dispatch, and a submission is an ordered array of Jobs. A Job > + * whose transfers are not K3 DMA records would be a new type, not a new > + * operation, and so are the copies, fills, timestamps and the list-carrying > + * command buffer a Vulkan compute driver will add. The query grows a field > + * naming the types beyond the Dispatch when the first one comes; an absent type > + * is refused at Job creation. > + */ > +enum drm_thames_job_type { > + /** @DRM_THAMES_JOB_DISPATCH: Run a Kernel with its transfers. */ > + DRM_THAMES_JOB_DISPATCH = 0, > +}; > + > /** > * enum drm_thames_hardware_id - &drm_thames_caps.hardware_id > */ > @@ -255,8 +279,9 @@ enum drm_thames_hardware_id { > /** > * struct drm_thames_caps - The capability query > * > - * Each window sits at one address whatever core runs the Job and reaches that > - * core's own memory. > + * The submit's limits are stated on &struct drm_thames_job_submit. Each window > + * sits at one address whatever core runs the Job and reaches that core's own > + * memory. > */ > struct drm_thames_caps { > /** @l2_size: Size in bytes of each core's L2 SRAM. */ > @@ -328,6 +353,35 @@ struct drm_thames_caps { > __u64 dru_capabilities; > }; > > +/** > + * enum drm_thames_fault_kind - The kind of a Job's fault > + * > + * The kind is what the firmware knows without decoding a register: what stopped > + * the Job and whose mode it stopped in. > + */ > +enum drm_thames_fault_kind { > + /** > + * @DRM_THAMES_FAULT_KERNEL: A Kernel took an exception in user mode. > + * The Context continues; the registers say why. > + */ > + DRM_THAMES_FAULT_KERNEL = 1, > + > + /** > + * @DRM_THAMES_FAULT_FIRMWARE: The firmware took an exception in > + * supervisor mode while running this Job. The core was reset. > + */ > + DRM_THAMES_FAULT_FIRMWARE, > + > + /** > + * @DRM_THAMES_FAULT_TIMEOUT: The device's Job timeout fired: the Kernel > + * ran longer than the driver lets a Kernel run and the device ended it, > + * or the Kernel returned with transfers still running and they did not > + * drain. The Context continues; NRP and NTSR say where the Kernel was, > + * and are 0 for transfers that did not drain. > + */ > + DRM_THAMES_FAULT_TIMEOUT, > +}; > + > /** > * struct drm_thames_dev_query - Arguments passed to DRM_IOCTL_THAMES_DEV_QUERY > */ > @@ -353,9 +407,9 @@ struct drm_thames_dev_query { > * order: the first group starts at the first transfer, each later group at a > * transfer carrying this flag. The firmware submits the first group before it > * enters the Kernel, and each later group, whole, when the Kernel asks for it > - * with system call 2. No group may hold more transfers on one channel than > - * &drm_thames_caps.dru_queue_depth. Never on the first transfer. A Job > - * without the flag is one group. > + * with system call 2 (see &struct drm_thames_job_create). No group may hold > + * more transfers on one channel than &drm_thames_caps.dru_queue_depth. Never > + * on the first transfer. A Job without the flag is one group. > */ > #define DRM_THAMES_TRANSFER_GROUP (1 << 0) > > @@ -393,7 +447,9 @@ struct drm_thames_dev_query { > * auxiliary aperture, at &drm_thames_caps.l2_main_dsp_va and l2_aux, which the > * kernel translates to the physical aperture of the core the Job runs on. The > * Job holds a reference on each buffer a transfer resolves to until it is > - * destroyed and its last submission has completed. > + * destroyed and its last submission has completed. No list names these > + * buffers: the read and write lists of &drm_thames_job_submit are for > + * synchronisation alone. > */ > struct drm_thames_transfer { > /** > @@ -409,6 +465,191 @@ struct drm_thames_transfer { > __u32 words[16]; > }; > > +/** > + * struct drm_thames_job_create - Arguments passed to DRM_IOCTL_THAMES_JOB_CREATE > + * > + * Once per Dispatch: the transfers are copied, each checked and translated, a > + * reference taken on the program, the params and every buffer a transfer > + * resolves to so the translation holds for the Job's life, and a handle > + * returned. The buffers the Kernel reads and writes are named at each > + * submit, in &drm_thames_job_submit. > + * > + * The firmware enters the Kernel at the program's start as a C function > + * taking one argument, the params' address, and returning a status word: > + * > + * int kernel(void *params); > + * > + * That is the whole of what the kernel and the firmware know. The Kernel finds > + * everything else, its inputs, outputs, constants, grid and status buffer, > + * through params, whose layout is private to the userspace driver that emitted > + * the Kernel. The program must have been created DRM_THAMES_BO_EXECUTABLE. > + * The program and the params must be bound in the Context. > + * The Job keeps the two addresses they are bound at when it is created, and > + * every submission runs the Kernel at them. > + * > + * A Kernel asks for its Job's next transfer group with system call 2 (the > + * SYSCALL instruction with 2, no argument). The call returns to the > + * instruction after it with every register kept but A0-A3 and RP. The firmware > + * submits the group only if the Job has one left and every channel the group > + * uses has no transfer queued or running; otherwise it fails the Job, writing > + * none of the group's transfers. A Kernel waits for the tile event of its > + * channels' DRU records with system call 3. > + */ > +struct drm_thames_job_create { > + /** @type: One of &enum drm_thames_job_type. */ > + __u32 type; > + > + /** @flags: MBZ. */ > + __u32 flags; > + > + /** @program: Handle of the buffer holding the Kernel. */ > + __u32 program; > + > + /** @params: Handle of the params buffer. */ > + __u32 params; > + > + /** > + * @transfers: Array of &struct drm_thames_transfer, the DRU records > + * the compiler scheduled for this Kernel; may be empty. > + */ > + struct drm_thames_obj_array transfers; > + > + /** @handle: Returned handle of the Job. Job handles are nonzero. */ > + __u32 handle; > + > + /** @pad: MBZ. */ > + __u32 pad; > +}; > + > +/** > + * DOC: Sync operation flags > + * > + * The low byte of &drm_thames_sync_op.flags is the handle type, bit 31 the > + * direction; every other bit is MBZ. > + * > + * The one op accepted is a timeline sync object signalled, > + * %DRM_THAMES_SYNC_OP_HANDLE_TYPE_TIMELINE_SYNCOBJ | > + * %DRM_THAMES_SYNC_OP_SIGNAL. Handle type 0 and a clear bit 31, a binary sync > + * object and a wait, are left for the client that will use them. > + * > + * - %DRM_THAMES_SYNC_OP_HANDLE_TYPE_MASK: The bits of the handle type. > + * - %DRM_THAMES_SYNC_OP_HANDLE_TYPE_TIMELINE_SYNCOBJ: A timeline sync > + * object, at the point &drm_thames_sync_op.timeline_value. > + * - %DRM_THAMES_SYNC_OP_SIGNAL: The call's last Job's finished fence is > + * added to the timeline at the point. > + */ > +#define DRM_THAMES_SYNC_OP_HANDLE_TYPE_MASK 0xff > +#define DRM_THAMES_SYNC_OP_HANDLE_TYPE_TIMELINE_SYNCOBJ 1 > +#define DRM_THAMES_SYNC_OP_SIGNAL (1u << 31) > + > +/** > + * struct drm_thames_sync_op - One signal of a submit call > + */ > +struct drm_thames_sync_op { > + /** > + * @flags: %DRM_THAMES_SYNC_OP_HANDLE_TYPE_TIMELINE_SYNCOBJ | > + * %DRM_THAMES_SYNC_OP_SIGNAL. > + */ > + __u32 flags; > + > + /** @handle: A timeline sync object of the client. */ > + __u32 handle; > + > + /** > + * @timeline_value: The point on the timeline sync object, nonzero. > + */ > + __u64 timeline_value; > +}; > + > +/** > + * struct drm_thames_job_submit - Arguments passed to DRM_IOCTL_THAMES_JOB_SUBMIT > + * > + * A submission is an ordered array of Jobs, one Ring entry each, so a sequence > + * of Dispatches is one call. Per run the kernel re-checks only the Context's > + * current state, waits on the implicit fences of the buffers the call names and > + * pushes the Ring entries. A submission of a Job while an earlier > + * submission of the same Job is in flight waits for it, which the Ring's order > + * gives on its own. One Ring completes its Jobs in submission order; there is > + * no order between the two cores' Rings, but the Jobs of one call run in the > + * order named. > + * > + * The buffers the call's Jobs read and write are named here, one pair of lists > + * per call. They are for synchronisation alone: the first Job waits on the > + * fences already on them, and the last Job's fence is set on them once the Jobs > + * are pushed, for read usage on @read_handles and write usage on > + * @write_handles, so DRM_IOCTL_THAMES_BO_PREP on a written buffer waits for the > + * call's Jobs. No flag turns either side off. The call holds a reference on > + * each buffer named until its last Job completes. A buffer may be named once > + * in the two lists together. > + * > + * The sync objects the call signals are named in @syncs. Every op receives the > + * call's last Job's finished fence, the same fence the listed buffers get; the > + * call waits on no sync object. The call is all or nothing: every Job handle > + * is looked up and every sync op resolved before the first Job is pushed, so a > + * call that fails has submitted nothing and signals nothing. An opt-out of the > + * implicit synchronisation would be a bit of @flags, appended under a minor > + * bump. > + * > + * Every Job the call submits has its fault record cleared; a refused call > + * clears none. > + * > + * A Job of the call that fails does not stop the ones after it: each later Job > + * runs, and completes or fails on its own merits. Only a Context banned after > + * its faults has its Jobs not yet run cancelled, their fences signalled with > + * -ECANCELED, and its later submit calls refused with -ECANCELED. > + */ > +struct drm_thames_job_submit { > + /** @flags: MBZ. */ > + __u32 flags; > + > + /** @pad: MBZ. */ > + __u32 pad; > + > + /** > + * @jobs: Array of __u32 Job handles, run in order. > + * > + * At least 1 and at most 256 Jobs. > + */ > + struct drm_thames_obj_array jobs; > + > + /** > + * @read_handles: Array of __u32 handles of the buffers the Jobs read; > + * may be empty. > + * > + * At most 4096 handles in @read_handles and @write_handles together. > + */ > + struct drm_thames_obj_array read_handles; > + > + /** > + * @write_handles: Array of __u32 handles of the buffers the Jobs write; > + * may be empty. Counted with @read_handles. > + */ > + struct drm_thames_obj_array write_handles; > + > + /** > + * @syncs: Array of &struct drm_thames_sync_op; may be empty. > + * > + * At most 64 ops. > + */ > + struct drm_thames_obj_array syncs; > +}; > + > +/** > + * struct drm_thames_job_destroy - Arguments passed to DRM_IOCTL_THAMES_JOB_DESTROY > + * > + * Releases the handle and returns at once; it does not wait for the Job's > + * submissions. A submission still in flight keeps the Job, its validated copy > + * and the references on its buffers until it completes, and its fence signals > + * as it would have. The handle names nothing afterwards. > + */ > +struct drm_thames_job_destroy { > + /** @handle: The Job. */ > + __u32 handle; > + > + /** @pad: MBZ. */ > + __u32 pad; > +}; > + > /** > * DRM_IOCTL_THAMES() - Build a thames IOCTL number > * @__access: Access type. Must be R, W or RW. > @@ -431,6 +672,12 @@ enum { > DRM_IOCTL_THAMES(WR, BO_MMAP_OFFSET, bo_mmap_offset), > DRM_IOCTL_THAMES_DEV_QUERY = > DRM_IOCTL_THAMES(WR, DEV_QUERY, dev_query), > + DRM_IOCTL_THAMES_JOB_CREATE = > + DRM_IOCTL_THAMES(WR, JOB_CREATE, job_create), > + DRM_IOCTL_THAMES_JOB_SUBMIT = > + DRM_IOCTL_THAMES(WR, JOB_SUBMIT, job_submit), > + DRM_IOCTL_THAMES_JOB_DESTROY = > + DRM_IOCTL_THAMES(WR, JOB_DESTROY, job_destroy), > DRM_IOCTL_THAMES_BO_BIND = > DRM_IOCTL_THAMES(WR, BO_BIND, bo_bind), > }; > > -- > 2.54.0 >

6 hours, 45 minutes

Re: [PATCH v3 07/16] accel/thames: Add IOCTLs for BO creation and mapping

by Christian König

On 10/23/26 08:39, Tomeu Vizoso wrote: > Creation returns a handle and maps nothing. The client maps a buffer > with DRM_IOCTL_THAMES_BO_BIND at a DSP address of its choice, as a > Vulkan driver would; the kernel enters the mapping in the page table > of every core, because we cannot know upfront which core will run a > given job. > > We use the SHMEM helpers for non-contiguous memory. Contiguous memory > comes from the driver's own allocator, which aligns a buffer of 2 MiB or > more to 2 MiB so it can be mapped as blocks; only the GEM DMA object > and its mmap and vmap helpers are used for it. How exactly do you do that? IIRC the SHMEM helpers can only do 2MiB allocations on a best effort basis. In other words you don't get a guarantee that this always works and when memory is fragmented the 4k fallback must work as well. Regards, Christian. > > Contiguous memory is needed for buffers that are going to be read from > or written into by the DRU, as it doesn't resolve addresses via the MMU. > > Binding at an address the client chooses lets a program be compiled, > with its addresses fixed, on a machine without the hardware, and is what > a Vulkan driver needs. > > A buffer is bound anywhere from 4 GiB up to the user region at 1 TiB; a > buffer shared through a dma-buf may be bound in several Contexts, so > the bindings are kept under one device-wide lock. The IOCTLs take their > arrays as versioned objects whose stride is the size userspace was > built against, so a structure can grow under a minor bump. > > Assisted-by: LLM > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > > --- > > v3: > - Added read-only, executable and contiguous flags. > - Added BO_BIND IOCTL. > - Copy the IOCTLs' arrays with vmemdup_array_user(), through one helper > for versioned arrays (Tvrtko Ursulin). > > v2: > - Add thames_accel.h UAPI header (Robert Nelson). > --- > drivers/accel/thames/Kconfig | 2 + > drivers/accel/thames/thames_device.h | 6 + > drivers/accel/thames/thames_drv.c | 47 +++++ > drivers/accel/thames/thames_drv.h | 5 + > drivers/accel/thames/thames_gem.c | 398 +++++++++++++++++++++++++++++++++++ > drivers/accel/thames/thames_gem.h | 21 ++ > include/uapi/drm/thames_accel.h | 183 +++++++++++++++- > 7 files changed, 661 insertions(+), 1 deletion(-) > > diff --git a/drivers/accel/thames/Kconfig b/drivers/accel/thames/Kconfig > index 9362b9274075..47a2986fcabd 100644 > --- a/drivers/accel/thames/Kconfig > +++ b/drivers/accel/thames/Kconfig > @@ -6,6 +6,8 @@ config DRM_ACCEL_THAMES > depends on TI_K3_DSP_REMOTEPROC > depends on REMOTEPROC > depends on MAILBOX > + select DRM_GEM_DMA_HELPER > + select DRM_GEM_SHMEM_HELPER > select IOMMU_IO_PGTABLE_LPAE > help > Choose this option if you have a Texas Instruments J722S and want to > diff --git a/drivers/accel/thames/thames_device.h b/drivers/accel/thames/thames_device.h > index e89e0dfd89b8..a1a54d974359 100644 > --- a/drivers/accel/thames/thames_device.h > +++ b/drivers/accel/thames/thames_device.h > @@ -58,6 +58,12 @@ struct thames_device { > struct list_head contexts; > struct mutex contexts_lock; > > + /* > + * One lock for the device: a dma-buf shared buffer is bound from > + * several files. > + */ > + struct mutex bindings_lock; > + > }; > > #define to_thames_device(drm_dev) \ > diff --git a/drivers/accel/thames/thames_drv.c b/drivers/accel/thames/thames_drv.c > index cb0287430d25..a17e7a90f7c1 100644 > --- a/drivers/accel/thames/thames_drv.c > +++ b/drivers/accel/thames/thames_drv.c > @@ -93,6 +93,41 @@ static void thames_postclose(struct drm_device *dev, struct drm_file *file) > thames_file_priv_put(thames_priv); > } > > +void *thames_get_uobj_array(const struct drm_thames_obj_array *in, u32 min_stride, u32 obj_size) > +{ > + void __user *in_ptr = u64_to_user_ptr(in->array); > + void *out, *out_ptr; > + int ret; > + u32 i; > + > + if (!in->count) > + return NULL; > + > + if (in->stride < min_stride) > + return ERR_PTR(-EINVAL); > + > + if (in->stride == obj_size) > + return vmemdup_array_user(in_ptr, in->count, obj_size); > + > + out = kvmalloc_array(in->count, obj_size, GFP_KERNEL); > + if (!out) > + return ERR_PTR(-ENOMEM); > + > + out_ptr = out; > + for (i = 0; i < in->count; i++) { > + ret = copy_struct_from_user(out_ptr, obj_size, in_ptr, in->stride); > + if (ret) { > + kvfree(out); > + return ERR_PTR(ret); > + } > + > + out_ptr += obj_size; > + in_ptr += in->stride; > + } > + > + return out; > +} > + > #define THAMES_DRIVER_MAJOR 1 > #define THAMES_DRIVER_MINOR 0 > > @@ -121,6 +156,8 @@ static int thames_query_caps(struct drm_device *dev, struct drm_thames_dev_query > .l2_aux.dsp_va = thames_window_va(tdev, THAMES_WINDOW_L2AUX), > .l2_aux.size = reported->window_sizes[THAMES_WINDOW_L2AUX], > .dru_realtime_dsp_va = thames_window_va(tdev, THAMES_WINDOW_DRU), > + .dsp_va_range.dsp_va = THAMES_BIND_VA_START, > + .dsp_va_range.size = THAMES_BIND_VA_END - THAMES_BIND_VA_START, > .user_region.dsp_va = THAMES_USER_REGION_BASE, > .user_region.size = THAMES_USER_REGION_SIZE, > .hardware_id = reported->hardware_id, > @@ -165,11 +202,17 @@ static int thames_ioctl_##func##_entered(struct drm_device *dev, void *data, \ > return ret; \ > } > > +THAMES_IOCTL_ENTERED(bo_create) > +THAMES_IOCTL_ENTERED(bo_mmap_offset) > THAMES_IOCTL_ENTERED(dev_query) > +THAMES_IOCTL_ENTERED(bo_bind) > > static const struct drm_ioctl_desc thames_drm_driver_ioctls[] = { > #define THAMES_IOCTL(n, func) DRM_IOCTL_DEF_DRV(THAMES_##n, thames_ioctl_##func, 0) > + THAMES_IOCTL(BO_CREATE, bo_create_entered), > + THAMES_IOCTL(BO_MMAP_OFFSET, bo_mmap_offset_entered), > THAMES_IOCTL(DEV_QUERY, dev_query_entered), > + THAMES_IOCTL(BO_BIND, bo_bind_entered), > }; > > DEFINE_DRM_ACCEL_FOPS(thames_accel_driver_fops); > @@ -234,6 +277,10 @@ static struct thames_device *thames_device_init(struct platform_device *pdev, > if (err) > return ERR_PTR(err); > > + err = drmm_mutex_init(&tdev->ddev, &tdev->bindings_lock); > + if (err) > + return ERR_PTR(err); > + > INIT_LIST_HEAD(&tdev->contexts); > > tdev->asid_next = 1; > diff --git a/drivers/accel/thames/thames_drv.h b/drivers/accel/thames/thames_drv.h > index f4536078e02f..7f9031b63780 100644 > --- a/drivers/accel/thames/thames_drv.h > +++ b/drivers/accel/thames/thames_drv.h > @@ -15,6 +15,7 @@ > #include "thames_device.h" > > struct drm_gem_dma_object; > +struct drm_thames_obj_array; > > struct thames_file_priv { > struct thames_device *tdev; > @@ -37,6 +38,8 @@ struct thames_file_priv { > struct drm_mm_node user_region; > struct drm_mm_node window_nodes[THAMES_NUM_WINDOWS]; > > + struct list_head bindings; > + > u32 context_id; > > u64 context_serial; > @@ -47,4 +50,6 @@ struct thames_file_priv { > > void thames_file_priv_put(struct thames_file_priv *priv); > > +void *thames_get_uobj_array(const struct drm_thames_obj_array *in, u32 min_stride, u32 obj_size); > + > #endif > diff --git a/drivers/accel/thames/thames_gem.c b/drivers/accel/thames/thames_gem.c > index b98e0fe203b0..c9fd08c7aaf4 100644 > --- a/drivers/accel/thames/thames_gem.c > +++ b/drivers/accel/thames/thames_gem.c > @@ -14,6 +14,7 @@ > #include <drm/drm_device.h> > #include <drm/drm_drv.h> > #include <drm/drm_gem_dma_helper.h> > +#include <drm/drm_gem_shmem_helper.h> > #include <drm/drm_print.h> > #include <drm/drm_timeout.h> > #include <drm/thames_accel.h> > @@ -23,6 +24,30 @@ > #include "thames_drv.h" > #include "thames_gem.h" > > +/* > + * References neither the buffer nor the Context: whichever goes first removes > + * it. > + */ > +struct thames_binding { > + struct thames_gem_object *bo; > + struct thames_file_priv *priv; > + struct drm_mm_node mm; > + struct list_head bo_link; > + struct list_head context_link; > +}; > + > +static int thames_bo_flags_to_prot(u32 flags) > +{ > + int prot = IOMMU_READ | IOMMU_CACHE; > + > + if (!(flags & DRM_THAMES_BO_EXECUTABLE)) > + prot |= IOMMU_NOEXEC; > + if (!(flags & (DRM_THAMES_BO_READ_ONLY | DRM_THAMES_BO_EXECUTABLE))) > + prot |= IOMMU_WRITE; > + > + return prot; > +} > + > static size_t thames_pgsize(u64 addr, size_t size, size_t *count) > { > size_t blk_offset = -addr % SZ_2M; > @@ -95,6 +120,72 @@ static int thames_context_map(struct thames_file_priv *priv, u64 dsp_va, int pro > return 0; > } > > +static void thames_context_unmap(struct thames_file_priv *priv, u64 dsp_va, u64 size) > +{ > + unsigned int c; > + > + for (c = 0; c < priv->tdev->num_cores; c++) > + thames_pgtable_unmap(priv->pgtbl_ops[c], dsp_va, size); > +} > + > +struct thames_gem_object *thames_context_find_bo(struct thames_file_priv *priv, u64 start, > + u64 end, u64 *offset) > +{ > + struct thames_binding *binding; > + struct thames_gem_object *bo; > + struct drm_mm_node *node; > + unsigned int i; > + > + guard(mutex)(&priv->mm_lock); > + > + node = __drm_mm_interval_first(&priv->mm, start, start); > + if (node == &priv->mm.head_node || node == &priv->first_page || > + node == &priv->user_region) > + return NULL; > + > + for (i = 0; i < THAMES_NUM_WINDOWS; i++) { > + if (node == &priv->window_nodes[i]) > + return NULL; > + } > + > + binding = container_of(node, struct thames_binding, mm); > + bo = binding->bo; > + if (end > node->start + bo->base.base.size) > + return NULL; > + > + if (!kref_get_unless_zero(&bo->base.base.refcount)) > + return NULL; > + > + *offset = start - node->start; > + > + return bo; > +} > + > +static struct thames_binding *thames_bo_find_binding(struct thames_gem_object *bo, > + struct thames_file_priv *priv) > +{ > + struct thames_binding *binding; > + > + lockdep_assert_held(&priv->tdev->bindings_lock); > + > + list_for_each_entry(binding, &bo->bindings, bo_link) { > + if (binding->priv == priv) > + return binding; > + } > + > + return NULL; > +} > + > +u64 thames_bo_dsp_va(struct thames_gem_object *bo, struct thames_file_priv *priv) > +{ > + struct thames_binding *binding; > + > + guard(mutex)(&priv->tdev->bindings_lock); > + binding = thames_bo_find_binding(bo, priv); > + > + return binding ? binding->mm.start : 0; > +} > + > /* Cyclic, so a freed ASID is not handed out again at once. */ > static int thames_asid_alloc(struct thames_device *tdev) > { > @@ -215,6 +306,7 @@ static int thames_context_map_core_regions(struct thames_file_priv *priv) > } > > static const struct drm_gem_object_funcs thames_gem_funcs; > +static const struct drm_gem_object_funcs thames_gem_shmem_funcs; > > /* > * The allocator zeroes through the cache: clean it before the DSP can reach the > @@ -234,6 +326,7 @@ static struct drm_gem_dma_object *thames_gem_create(struct drm_device *ddev, siz > if (!bo) > return ERR_PTR(-ENOMEM); > > + INIT_LIST_HEAD(&bo->bindings); > obj = &bo->base.base; > obj->funcs = &thames_gem_funcs; > bo->base.map_noncoherent = true; > @@ -271,6 +364,38 @@ static struct drm_gem_dma_object *thames_gem_create(struct drm_device *ddev, siz > return ERR_PTR(ret); > } > > +/* > + * The DSP reaches these pages only through the page tables, never by a DRU > + * transfer. > + */ > +static struct thames_gem_object *thames_gem_create_shmem(struct drm_device *ddev, size_t size) > +{ > + struct thames_gem_object *bo; > + struct sg_table *sgt; > + int ret; > + > + bo = kzalloc_obj(*bo, GFP_KERNEL); > + if (!bo) > + return ERR_PTR(-ENOMEM); > + > + INIT_LIST_HEAD(&bo->bindings); > + bo->shmem.base.funcs = &thames_gem_shmem_funcs; > + > + ret = drm_gem_shmem_init(ddev, &bo->shmem, PAGE_ALIGN(size)); > + if (ret) { > + kfree(bo); > + return ERR_PTR(ret); > + } > + > + sgt = drm_gem_shmem_get_pages_sgt(&bo->shmem); > + if (IS_ERR(sgt)) { > + drm_gem_object_put(&bo->shmem.base); > + return ERR_CAST(sgt); > + } > + > + return bo; > +} > + > static struct drm_gem_dma_object *thames_context_alloc_user_pages(struct thames_file_priv *priv, > u64 dsp_va, size_t size) > { > @@ -459,6 +584,7 @@ int thames_context_create(struct thames_file_priv *priv) > > priv->context_id = ret; > priv->context_serial = atomic64_inc_return(&tdev->last_context_serial); > + INIT_LIST_HEAD(&priv->bindings); > > ret = thames_context_va_init(priv); > if (ret) > @@ -500,14 +626,37 @@ int thames_context_create(struct thames_file_priv *priv) > return ret; > } > > +static void thames_binding_remove(struct thames_binding *binding) > +{ > + struct thames_file_priv *priv = binding->priv; > + > + lockdep_assert_held(&priv->tdev->bindings_lock); > + > + thames_context_unmap(priv, binding->mm.start, binding->mm.size); > + thames_context_bump_generation(priv); > + > + scoped_guard(mutex, &priv->mm_lock) > + drm_mm_remove_node(&binding->mm); > + > + list_del(&binding->bo_link); > + list_del(&binding->context_link); > + kfree(binding); > +} > + > void thames_context_destroy(struct thames_file_priv *priv) > { > struct thames_device *tdev = priv->tdev; > + struct thames_binding *binding, *tmp; > > /* > * The Context goes after its last pushed Job completed, so no Job runs > * on mappings removed here. > */ > + scoped_guard(mutex, &tdev->bindings_lock) { > + list_for_each_entry_safe(binding, tmp, &priv->bindings, context_link) > + thames_binding_remove(binding); > + } > + > scoped_guard(mutex, &tdev->contexts_lock) > list_del(&priv->context_node); > > @@ -522,10 +671,124 @@ void thames_context_destroy(struct thames_file_priv *priv) > priv->context_id, priv->asid); > } > > +static int thames_context_map_sgt(struct thames_file_priv *priv, u64 dsp_va, int prot, > + struct sg_table *sgt) > +{ > + struct scatterlist *sg; > + u64 offset = 0; > + unsigned int i; > + int ret; > + > + for_each_sgtable_dma_sg(sgt, sg, i) { > + ret = thames_context_map(priv, dsp_va + offset, prot, sg_dma_address(sg), > + sg_dma_len(sg)); > + if (ret) { > + thames_context_unmap(priv, dsp_va, offset); > + return ret; > + } > + > + offset += sg_dma_len(sg); > + } > + > + return 0; > +} > + > +static int thames_bo_bind(struct thames_file_priv *priv, struct thames_gem_object *bo, > + u64 dsp_va) > +{ > + size_t size = bo->base.base.size; > + int prot = thames_bo_flags_to_prot(bo->flags); > + struct thames_binding *binding; > + int ret; > + > + if (!IS_ALIGNED(dsp_va, PAGE_SIZE)) > + return -EINVAL; > + > + if (dsp_va < THAMES_BIND_VA_START || dsp_va > THAMES_BIND_VA_END || > + size > THAMES_BIND_VA_END - dsp_va) > + return -EINVAL; > + > + binding = kzalloc_obj(*binding); > + if (!binding) > + return -ENOMEM; > + > + binding->bo = bo; > + binding->priv = priv; > + binding->mm.start = dsp_va; > + binding->mm.size = size; > + > + ret = mutex_lock_interruptible(&priv->tdev->bindings_lock); > + if (ret) > + goto err_free; > + > + if (thames_bo_find_binding(bo, priv)) { > + ret = -EBUSY; > + goto err_unlock; > + } > + > + scoped_guard(mutex, &priv->mm_lock) > + ret = drm_mm_reserve_node(&priv->mm, &binding->mm); > + if (ret) > + goto err_unlock; > + > + if (bo->flags & DRM_THAMES_BO_CONTIGUOUS) > + ret = thames_context_map(priv, dsp_va, prot, bo->base.dma_addr, size); > + else > + ret = thames_context_map_sgt(priv, dsp_va, prot, bo->shmem.sgt); > + if (ret) { > + scoped_guard(mutex, &priv->mm_lock) > + drm_mm_remove_node(&binding->mm); > + goto err_unlock; > + } > + > + list_add_tail(&binding->bo_link, &bo->bindings); > + list_add_tail(&binding->context_link, &priv->bindings); > + mutex_unlock(&priv->tdev->bindings_lock); > + > + return 0; > + > +err_unlock: > + mutex_unlock(&priv->tdev->bindings_lock); > +err_free: > + kfree(binding); > + return ret; > +} > + > +static int thames_bo_unbind(struct thames_file_priv *priv, struct thames_gem_object *bo) > +{ > + struct thames_binding *binding; > + int ret; > + > + ret = mutex_lock_interruptible(&priv->tdev->bindings_lock); > + if (ret) > + return ret; > + > + binding = thames_bo_find_binding(bo, priv); > + if (binding) > + thames_binding_remove(binding); > + else > + ret = -EINVAL; > + > + mutex_unlock(&priv->tdev->bindings_lock); > + return ret; > +} > + > +static void thames_bo_unmap_from_device(struct thames_gem_object *bo) > +{ > + struct thames_device *tdev = to_thames_device(bo->base.base.dev); > + struct thames_binding *binding, *tmp; > + > + guard(mutex)(&tdev->bindings_lock); > + list_for_each_entry_safe(binding, tmp, &bo->bindings, bo_link) > + thames_binding_remove(binding); > +} > + > static void thames_gem_bo_free(struct drm_gem_object *obj) > { > struct thames_gem_object *bo = to_thames_bo(obj); > > + thames_bo_unmap_from_device(bo); > + > if (bo->alloc_vaddr) > dma_free_noncoherent(drm_dev_dma_dev(obj->dev), bo->alloc_size, bo->alloc_vaddr, > bo->alloc_dma, DMA_TO_DEVICE); > @@ -535,6 +798,16 @@ static void thames_gem_bo_free(struct drm_gem_object *obj) > kfree(bo); > } > > +static void thames_gem_shmem_bo_free(struct drm_gem_object *obj) > +{ > + struct thames_gem_object *bo = to_thames_bo(obj); > + > + thames_bo_unmap_from_device(bo); > + > + drm_gem_free_mmap_offset(obj); > + drm_gem_shmem_free(&bo->shmem); > +} > + > static const struct drm_gem_object_funcs thames_gem_funcs = { > .free = thames_gem_bo_free, > .print_info = drm_gem_dma_object_print_info, > @@ -544,3 +817,128 @@ static const struct drm_gem_object_funcs thames_gem_funcs = { > .vm_ops = &drm_gem_dma_vm_ops, > }; > > +static const struct drm_gem_object_funcs thames_gem_shmem_funcs = { > + .free = thames_gem_shmem_bo_free, > + .print_info = drm_gem_shmem_object_print_info, > + .pin = drm_gem_shmem_object_pin, > + .unpin = drm_gem_shmem_object_unpin, > + .get_sg_table = drm_gem_shmem_object_get_sg_table, > + .vmap = drm_gem_shmem_object_vmap, > + .vunmap = drm_gem_shmem_object_vunmap, > + .mmap = drm_gem_shmem_object_mmap, > + .vm_ops = &drm_gem_shmem_vm_ops, > +}; > + > +int thames_ioctl_bo_create(struct drm_device *ddev, void *data, struct drm_file *file) > +{ > + struct drm_thames_bo_create *args = data; > + struct thames_gem_object *bo; > + struct drm_gem_dma_object *mem; > + int ret; > + > + if (args->handle) > + return -EINVAL; > + > + if (!args->size) > + return -EINVAL; > + > + if (args->flags & ~(DRM_THAMES_BO_READ_ONLY | DRM_THAMES_BO_EXECUTABLE | > + DRM_THAMES_BO_CONTIGUOUS)) > + return -EINVAL; > + > + if (args->flags & DRM_THAMES_BO_CONTIGUOUS) { > + mem = thames_gem_create(ddev, args->size); > + if (IS_ERR(mem)) > + return PTR_ERR(mem); > + bo = to_thames_bo(&mem->base); > + } else { > + bo = thames_gem_create_shmem(ddev, args->size); > + if (IS_ERR(bo)) > + return PTR_ERR(bo); > + } > + > + bo->flags = args->flags; > + > + ret = drm_gem_handle_create(file, &bo->base.base, &args->handle); > + drm_gem_object_put(&bo->base.base); > + if (ret) { > + dev_err(ddev->dev, "Failed to create gem handle: %d", ret); > + return ret; > + } > + > + args->size = bo->base.base.size; > + > + return 0; > +} > + > +static int thames_bind_exec_op(struct drm_file *file, const struct drm_thames_bind_op *op) > +{ > + struct thames_file_priv *priv = file->driver_priv; > + struct drm_gem_object *obj; > + int ret; > + > + if (op->flags & ~DRM_THAMES_BIND_OP_UNBIND) > + return -EINVAL; > + > + if ((op->flags & DRM_THAMES_BIND_OP_UNBIND) && op->dsp_va) > + return -EINVAL; > + > + obj = drm_gem_object_lookup(file, op->handle); > + if (!obj) > + return -ENOENT; > + > + if (op->flags & DRM_THAMES_BIND_OP_UNBIND) > + ret = thames_bo_unbind(priv, to_thames_bo(obj)); > + else > + ret = thames_bo_bind(priv, to_thames_bo(obj), op->dsp_va); > + > + drm_gem_object_put(obj); > + > + return ret; > +} > + > +int thames_ioctl_bo_bind(struct drm_device *ddev, void *data, struct drm_file *file) > +{ > + struct drm_thames_bo_bind *args = data; > + struct drm_thames_bind_op *ops; > + int ret = 0; > + u32 i; > + > + if (args->flags || args->pad) > + return -EINVAL; > + > + ops = thames_get_uobj_array(&args->ops, sizeof(*ops), sizeof(*ops)); > + if (IS_ERR(ops)) > + return PTR_ERR(ops); > + > + for (i = 0; i < args->ops.count; i++) { > + ret = thames_bind_exec_op(file, &ops[i]); > + if (ret) { > + args->ops.count = i; > + break; > + } > + } > + > + kvfree(ops); > + > + return ret; > +} > + > +int thames_ioctl_bo_mmap_offset(struct drm_device *ddev, void *data, struct drm_file *file) > +{ > + struct drm_thames_bo_mmap_offset *args = data; > + struct drm_gem_object *obj; > + > + if (args->pad) > + return -EINVAL; > + > + obj = drm_gem_object_lookup(file, args->handle); > + if (!obj) > + return -ENOENT; > + > + args->offset = drm_vma_node_offset_addr(&obj->vma_node); > + drm_gem_object_put(obj); > + > + return 0; > +} > + > diff --git a/drivers/accel/thames/thames_gem.h b/drivers/accel/thames/thames_gem.h > index 09ac1b50d21a..c9d0e170dad3 100644 > --- a/drivers/accel/thames/thames_gem.h > +++ b/drivers/accel/thames/thames_gem.h > @@ -12,6 +12,13 @@ > > #include "thames_abi.h" > > +/* > + * The windows sit below 4 GiB because Kernels load their addresses with 32-bit > + * moves. > + */ > +#define THAMES_BIND_VA_START SZ_4G > +#define THAMES_BIND_VA_END THAMES_USER_REGION_BASE > + > struct thames_core; > struct thames_device; > struct thames_file_priv; > @@ -34,9 +41,18 @@ struct thames_gem_object { > dma_addr_t alloc_dma; > size_t alloc_size; > > + struct list_head bindings; > + > + u32 flags; > }; > > > +int thames_ioctl_bo_create(struct drm_device *ddev, void *data, struct drm_file *file); > + > +int thames_ioctl_bo_mmap_offset(struct drm_device *ddev, void *data, struct drm_file *file); > + > +int thames_ioctl_bo_bind(struct drm_device *ddev, void *data, struct drm_file *file); > + > int thames_context_create(struct thames_file_priv *priv); > > void thames_context_destroy(struct thames_file_priv *priv); > @@ -44,6 +60,11 @@ void thames_context_rewrite_entries(struct thames_core *core); > > void thames_context_va_fini(struct thames_file_priv *priv); > > +struct thames_gem_object *thames_context_find_bo(struct thames_file_priv *priv, u64 start, > + u64 end, u64 *offset); > + > +u64 thames_bo_dsp_va(struct thames_gem_object *bo, struct thames_file_priv *priv); > + > static inline struct thames_gem_object *to_thames_bo(struct drm_gem_object *obj) > { > return container_of(obj, struct thames_gem_object, base.base); > diff --git a/include/uapi/drm/thames_accel.h b/include/uapi/drm/thames_accel.h > index af427a29b585..ea556b48f1d7 100644 > --- a/include/uapi/drm/thames_accel.h > +++ b/include/uapi/drm/thames_accel.h > @@ -45,15 +45,116 @@ extern "C" { > * definitions instead. > */ > enum drm_thames_ioctl_id { > + /** @DRM_THAMES_BO_CREATE: Create a buffer object. */ > + DRM_THAMES_BO_CREATE, > + > + /** > + * @DRM_THAMES_BO_MMAP_OFFSET: Get the file offset to pass to > + * mmap to map a GEM object. > + */ > + DRM_THAMES_BO_MMAP_OFFSET, > + > /** @DRM_THAMES_DEV_QUERY: Query the device's capabilities. */ > DRM_THAMES_DEV_QUERY, > > + /** > + * @DRM_THAMES_BO_BIND: Bind buffers into the file's Context at the > + * addresses the client chooses, or unbind them. > + */ > + DRM_THAMES_BO_BIND, > }; > > /** > * DOC: IOCTL arguments > */ > > +/** > + * enum drm_thames_bo_flags - Buffer object flags, passed at creation time. > + * > + * A buffer with neither permission flag is user read-write and execute-never; > + * DRM_THAMES_BO_CONTIGUOUS chooses the backing, not the permissions. > + */ > +enum drm_thames_bo_flags { > + /** > + * @DRM_THAMES_BO_READ_ONLY: Mapped user read-only in the Context. For > + * the params, the coefficients and the constants, so that a Kernel > + * cannot alter the inputs of a Job's next run. > + */ > + DRM_THAMES_BO_READ_ONLY = 1 << 0, > + > + /** > + * @DRM_THAMES_BO_EXECUTABLE: Mapped user read-execute and not > + * writable. Required of a Job's program. > + */ > + DRM_THAMES_BO_EXECUTABLE = 1 << 1, > + > + /** > + * @DRM_THAMES_BO_CONTIGUOUS: Backed by one physically contiguous > + * allocation, so that a transfer of a Job can name it. For the buffers > + * a DRU transfer reads or writes; any other buffer is backed page by > + * page. > + */ > + DRM_THAMES_BO_CONTIGUOUS = 1 << 2, > +}; > + > +/** > + * struct drm_thames_bo_create - Arguments passed to DRM_IOCTL_THAMES_BO_CREATE. > + * > + * Returns a handle and no address; the buffer is bound in no Context until > + * DRM_IOCTL_THAMES_BO_BIND. > + */ > +struct drm_thames_bo_create { > + /** > + * @size: Requested size for the object > + * > + * The (page-aligned) allocated size for the object will be returned. > + */ > + __u64 size; > + > + /** > + * @handle: MBZ on input; returns the object's handle, which is nonzero. > + */ > + __u32 handle; > + > + /** @flags: Combination of &enum drm_thames_bo_flags. */ > + __u32 flags; > +}; > + > +/** > + * struct drm_thames_bo_mmap_offset - Arguments passed to DRM_IOCTL_THAMES_BO_MMAP_OFFSET. > + */ > +struct drm_thames_bo_mmap_offset { > + /** @handle: Handle of the object we want an mmap offset for. */ > + __u32 handle; > + > + /** @pad: MBZ. */ > + __u32 pad; > + > + /** @offset: The fake offset to use for subsequent mmap calls. */ > + __u64 offset; > +}; > + > +/** > + * struct drm_thames_obj_array - An array of versioned objects passed by pointer. > + * > + * The stride is the size of the object userspace was built against, so a > + * structure can grow at its end under a minor bump. > + * The kernel reads each element with copy_struct_from_user(): a stride smaller > + * than its own size has the rest read as zero, down to the size the structure > + * had when it entered the uAPI and no smaller; a larger stride is accepted only > + * if the bytes the kernel does not know are zero. > + */ > +struct drm_thames_obj_array { > + /** @stride: Size of one element in bytes. */ > + __u32 stride; > + > + /** @count: Number of elements. */ > + __u32 count; > + > + /** @array: User pointer to the first element. */ > + __u64 array; > +}; > + > /** > * struct drm_thames_range - A range of the Context's address space. > */ > @@ -65,6 +166,73 @@ struct drm_thames_range { > __u64 size; > }; > > +/** > + * DOC: Bind operation flags > + * > + * - %DRM_THAMES_BIND_OP_UNBIND: Unbind the buffer from the Context instead > + * of binding it; &drm_thames_bind_op.dsp_va must be 0. > + */ > +#define DRM_THAMES_BIND_OP_UNBIND (1 << 0) > + > +/** > + * struct drm_thames_bind_op - One operation of DRM_IOCTL_THAMES_BO_BIND > + * > + * A bind maps the whole buffer at @dsp_va in every page table of the file's > + * Context, with the permissions of its creation flags; an unbind removes the > + * mapping. A buffer is bound at most once in a Context, at one address, over > + * no range another buffer is bound in. A buffer shared through a dma-buf may > + * be bound in each importing file's Context, each at its own address. The > + * kernel maps 2 MiB blocks where both the address and the buffer's pages are > + * 2 MiB aligned, so a client aligns a buffer of 2 MiB or more to 2 MiB. > + * > + * Unbinding a buffer a submitted Job still reaches through the MMU is the > + * client's error: the mapping goes at once, and the Job faults in the client's > + * own Context as on any other fault. A Job's transfers keep the buffers they > + * reach, whether bound or not, until the Job is destroyed. Freeing a bound > + * buffer unbinds it. > + */ > +struct drm_thames_bind_op { > + /** @handle: The buffer. */ > + __u32 handle; > + > + /** > + * @flags: 0 to bind the buffer at @dsp_va, > + * %DRM_THAMES_BIND_OP_UNBIND to unbind it. > + */ > + __u32 flags; > + > + /** > + * @dsp_va: Where the buffer is bound in the DSP's address space, > + * page-aligned, the buffer wholly inside > + * &drm_thames_caps.dsp_va_range; 0 for an unbind. > + * > + * The address the core's MMU translates through the Context's page > + * table, the same on every core. It is the number userspace embeds > + * in params and Kernel code. It is neither the bus address the page > + * table entries hold nor the CPU address mmap returns. > + */ > + __u64 dsp_va; > +}; > + > +/** > + * struct drm_thames_bo_bind - Arguments passed to DRM_IOCTL_THAMES_BO_BIND > + * > + * The operations are applied in order and each is done when the call > + * returns. The call stops at the first operation that fails and returns its > + * error; the operations before it hold, and @ops.count is set to their > + * number. > + */ > +struct drm_thames_bo_bind { > + /** @flags: MBZ. */ > + __u32 flags; > + > + /** @pad: MBZ. */ > + __u32 pad; > + > + /** @ops: Array of &struct drm_thames_bind_op, applied in order. */ > + struct drm_thames_obj_array ops; > +}; > + > /** > * enum drm_thames_dev_query_type - Query type > * > @@ -126,9 +294,16 @@ struct drm_thames_caps { > */ > __u64 dru_realtime_dsp_va; > > + /** > + * @dsp_va_range: The span of the Context's address space the client > + * binds its buffers in with DRM_IOCTL_THAMES_BO_BIND. The windows > + * above sit below it and @user_region above it. > + */ > + struct drm_thames_range dsp_va_range; > + > /** > * @user_region: Where the kernel driver maps the Context's user stack > - * and save page. > + * and save page, outside @dsp_va_range. > */ > struct drm_thames_range user_region; > > @@ -189,8 +364,14 @@ struct drm_thames_dev_query { > struct drm_thames_ ## __type) > > enum { > + DRM_IOCTL_THAMES_BO_CREATE = > + DRM_IOCTL_THAMES(WR, BO_CREATE, bo_create), > + DRM_IOCTL_THAMES_BO_MMAP_OFFSET = > + DRM_IOCTL_THAMES(WR, BO_MMAP_OFFSET, bo_mmap_offset), > DRM_IOCTL_THAMES_DEV_QUERY = > DRM_IOCTL_THAMES(WR, DEV_QUERY, dev_query), > + DRM_IOCTL_THAMES_BO_BIND = > + DRM_IOCTL_THAMES(WR, BO_BIND, bo_bind), > }; > > #if defined(__cplusplus) > > -- > 2.54.0 >

6 hours, 48 minutes

Re: [PATCH] dma-buf: Add DMA_BUF_IOCTL_SET_RELEASE_EVENTFD

by Christian König

On 10/23/26 02:36, Val Packett wrote: > On 10/22/26 9:26 AM, Christian König wrote: >> On 10/21/26 22:16, Val Packett wrote: >>> On 10/21/26 10:43 AM, Christian König wrote: >>>> [..] >>>>> The buffer lifecycle coordination we need this for applies to KVM as well >>>>> and it generally doesn't make sense to have this as a provider-specific >>>>> thing since it's not related to the provider in any way, it's just >>>>> something we want from dmabufs in general. >>>> I wouldn't put that on the DMA-buf file descriptor where basically anybody can mess with it. That is just an recipe for trouble. >>>> >>>> The better approach seems to be putting it into the udmabuf_ioctl(), so that from the very beginning you add the eventfd which signals when the udmabuf is released. >>>> >>>> If we need that for more implementations than just udmabuf, we can potentially add it to dma_buf_export_info so that exporters can provide the eventfd file descriptor directly on creation. >>> Well, we already need it with both udmabuf and gntdev-dmabuf.. >>> >>> Involving the exporter sounds really annoying when it's not something exporter dependent really.. What if the policy is just "only one eventfd ever" i.e. the first call of the ioctl succeeds but later calls on the same dmabuf return an error? >> No, exactly that is what I'm trying to avoid. >> >> Nobody except the original creater should be able to insert an eventfd here. > > If there's no replacement allowed anymore, what could possibly be harmful/undesirable about "someone else" being able to do this? > > With a "first caller only" policy, the original creator already can be 100% sure that no one else is able to insert an eventfd: Ok that sounds like you my argumentation wasn't fully clear: That approach is exactly what I try to avoid! > If the original creator calls this API before sending the fd to anyone else, well, the original creator *is* the first caller, that's it. > > Otherwise, i.e. if the original creator is not interested in this API.. it's unlikely a different process would use it, but if one does for some reason, I don't see any negative consequences from that? First of all if this gets added as common DMA-buf API the functionality suddenly becomes available to every exporter, not just the ones who explicitly implement it. That in turn would be rather questionable because lifetime is something only the exporter of a buffer can fully deal with. For example it is very common that the exporter delegates cleanup from the ->release callback to a work item and your whole approach with the eventfd on the DMA-buf then doesn't work any more. Then second when the add this as common DMA-buf functionality then it should be usable by everybody and not just the first one who asks. Otherwise you can run into all kind mess with that when you give a DMA-buf where the eventfd slot is already used up to some other process which expects to use that functionality. For now that functionality only seems to make sense for the original creator and a limited amount of exporters where it actually works. So my suggestion is to either add some functionality to the exporter(s) or add some common functionality to dma_buf_export_info to aid exporters with their implementation. Apart from that the approach with the eventfd looks perfectly fine to me. Regards, Christian. > > > > Thanks, > ~val

7 hours, 39 minutes

Re: [PATCH v3 03/16] dt-bindings: remoteproc: k3-dsp: Allow referencing the timers' power domains

by Krzysztof Kozlowski

On Fri, Oct 23, 2026 at 08:39:28AM +0200, Tomeu Vizoso wrote: > A firmware may drive the core's timers, which need their power domains > held; the remoteproc driver does not hold them, so let the DSP node > reference them for a driver that attaches to the running core to hold. > > Assisted-by: LLM > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > > --- > > v3: > - Initial version > --- > .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml > index d6319c8e96f8..80eaef0a19b8 100644 > --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml > +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml > @@ -76,6 +76,23 @@ properties: > # Optional properties: > # -------------------- > > + power-domains: > + description: | > + Power domains of the SoC timers the running firmware ticks on and > + times its work out with. The firmware programs these timers itself > + and makes no power request for them, and the timers' own nodes are > + reserved for it, so nothing else requests their power; a client > + driver of the firmware holds them on while it drives the processor. > + The processor's own power is managed through ti,sci-dev-id, not here. > + items: > + - description: power domain of the first timer first timer > + - description: power domain of the second timer second timer No need to say that power domain is a power domain. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski(a)oss.qualcomm.com> Best regards, Krzysztof

9 hours, 4 minutes

Re: [PATCH v3 02/16] dt-bindings: remoteproc: k3-dsp: Allow a second sub-mailbox

by Krzysztof Kozlowski

On Fri, Oct 23, 2026 at 08:39:27AM +0200, Tomeu Vizoso wrote: > Allow specifying a second sub-mailbox that could be used by the firmware > in addition to the first one, which is taken by remoteproc. > > This is required by firmware that doesn't use rpmsg for their > communication with the Linux kernel. > > Assisted-by: LLM Drop > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> DCO mismatch. > > --- > > v3: > - Initial version > --- > .../devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml > index 278b88b7f906..d6319c8e96f8 100644 > --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml > +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml > @@ -51,10 +51,12 @@ properties: > > mboxes: > description: | > - OMAP Mailbox specifier denoting the sub-mailbox, to be used for > - communication with the remote processor. This property should match > - with the sub-mailbox node used in the firmware image. > - maxItems: 1 > + OMAP Mailbox specifiers denoting the sub-mailboxes allotted to > + communication with the remote processor. The first is the one the > + remoteproc core uses and should match the sub-mailbox node used in the > + firmware image. The second is optional. Instead list the items with description and minItems: 1. Don't repeat constraints in free form text. > + minItems: 1 Best regards, Krzysztof

9 hours, 6 minutes

[PATCH v9 00/18] PCI/P2PDMA: Route peer-to-peer DMA by TLP class

by Leon Romanovsky

PCI P2PDMA applies Request and Completion Redirect throughout both paths. This misclassifies asymmetric and nested switches, and reports one answer for every kind of TLP. Three ACS controls act on TLP attributes the client chooses rather than on the topology: Translation Blocking and Direct Translated P2P act on a Request's Address Type, and Completion Redirect skips Completions carrying Relaxed Ordering. Evaluate each direction at the path divergence, decide every class from the one walk, and treat a client with ATS enabled as translating unless its driver declares per-mapping ATS. This completes the P2PDMA side; dma-buf and mlx5 follow separately. Signed-off-by: Leon Romanovsky <leonro(a)nvidia.com> --- Changes in v9: - Removed dmabuf patches for now. - Removed mlx5 per-mapping ATS declaration for now; it comes back with the dmabuf patches. - Link to v8: https://patch.msgid.link/20260928-fix-p2p-acs-v4-0-v8-0-404453b9c435@nvidia… Changes in v8: - Added extra Reviewed-by from Logan. - Added patches to take care of ATS per-device vs. per-mapping option - Link to v7: https://patch.msgid.link/20260920-fix-p2p-acs-v4-0-v7-0-ca0828ab697c@nvidia… Changes in v7: - Removed "Document pdev->p2pdma lifetime rules" patch, it gives nothing after p2pmem fix. - Split dmabuf patch. - Tushar retested the series, so added his Tested-by. - Added Logan's ROB tags and fixed minor documentation issues pointed by him. - Link to v6: https://lore.kernel.org/all/20260914-fix-p2p-acs-v4-0-v6-0-5ef07ec9ef06@nvi… Changes in v6: - Changed DMABUF to use callbacks and not directly stored pointer. - Link to v5: https://patch.msgid.link/20260910-fix-p2p-acs-v4-0-v5-0-856087f63c0d@nvidia… Changes in v5: - Rebase on the posted fixes. - Dropped tags from changed patches. - Remove Egress Control Vector interpretation and coverage. - Keep enabled Egress Control conservative as a Request redirect. - Use pci_dbg()/dev_dbg() for diagnostics and drop the "debug" prefix. - Removed code comments from "Document the pdev->p2pdma lifetime and RCU rules" patch and reduced description to actual lifetime explanation. - Added note that Linux assumes that TLPs are in strict-ordering and untranslated. - Added code to calculate p2p paths per-TLP type. - Converted mlx5 to use that new proposed API. - Link to https://patch.msgid.link/20260821-fix-p2p-acs-v4-0-v4-0-94426b96de73@nvidia… Changes in v4: - Reject ACS Violations and unreadable routing state instead of treating them as host-bridge redirects - Added Tested-by tags from Tushar Dave - Added support to asymmetric ACS routing - Limited redirect checks to the two ports at the path divergence - Added standalone ACS routing diagnostics for hardware retesting - Dropped " PCI: Account for Direct Translated P2P in ACS isolation checks" patch - Link to v3: https://patch.msgid.link/20260811-fix-p2p-acs-v3-0-efc488ee7c03@nvidia.com Changes in v3: - Fixed pci_p2pdma_add_resource() error unwinding - Made pdev->p2pdma teardown wait unconditionally for RCU readers - Restricted pci_p2pmem_find_many() to pool-backed providers - Documented the pdev->p2pdma lifetime and RCU rules - Fixed calc_map_type_and_dist() handling of the verbose argument - Required the ACS port and target to share a bus before indexing the Egress Control Vector - Gave pci_acs_enabled() and pci_acs_path_enabled() a scope, so the ACS Direct Translated P2P rule no longer stops pci_enable_pasid() from enabling PASID - Dropped "Report ACS ports when the paths share no upstream bridge": the mapping type cannot change without a shared upstream bridge, so the pci=disable_acs_redir= hint was not actionable there and the ACS walk only cost config space reads - Folded the Request Redirect rule into pci_acs_rr_ineffective(), so pci_acs_flags_enabled() and the Intel SPT PCH quirk share one copy - Renamed pci_acs_egress_ctrl_set() to pci_acs_egress_ctrl_is_set(), it reads the bit rather than setting it - Reworded the blocked-path warning: ACS may also leave the direct route indeterminate rather than blocked - Added KUnit coverage for the shared-bus guard, a device with no ACS capability and an unreadable ACS Control register - Added the missing Fixes: tags, a second one on the pci_p2pdma_add_resource() unwinding fix (the dangling devres action dates to f58ef9d1d135) and one on the Egress Control isolation change - Link to v2: https://patch.msgid.link/20260806-fix-p2p-acs-v2-0-0cec14812965@nvidia.com Changes in v2: - Added Logan's ROB tags - Added commas in Documentation patch - Link to v1: https://patch.msgid.link/20260802-fix-p2p-acs-v1-0-a7c5eb64fff6@nvidia.com --- Leon Romanovsky (18): PCI/P2PDMA: Document the TLP attribute assumptions PCI/P2PDMA: Derive routing from directional ACS controls PCI: Reject unreadable ACS controls in isolation checks PCI/P2PDMA: Evaluate ACS controls at the path divergence PCI/P2PDMA: Document directional ACS routing PCI/P2PDMA: Collect the path's ACS controls before deciding PCI/P2PDMA: Answer routing per TLP class PCI/P2PDMA: Route Relaxed Ordering Completions directly PCI/P2PDMA: Reject Translated Requests blocked by Translation Blocking PCI/P2PDMA: Route Translated Requests under Direct Translated P2P PCI/P2PDMA: Log detailed ACS routing diagnostics PCI/P2PDMA: Add KUnit tests for the ACS routing decisions PCI/P2PDMA: Test the ACS P2P routing walk PCI: Add KUnit coverage for ACS isolation checks PCI/P2PDMA: Document TLP-class routing PCI/P2PDMA: Let a client declare that it selects ATS per mapping PCI/P2PDMA: Evaluate the ATS path for clients with ATS enabled PCI/P2PDMA: Test the routing of clients with ATS enabled Documentation/admin-guide/kernel-parameters.txt | 15 +- Documentation/driver-api/pci/p2pdma.rst | 80 +++ drivers/pci/Kconfig | 15 + drivers/pci/Makefile | 1 + drivers/pci/p2pdma.c | 719 ++++++++++++++++++-- drivers/pci/pci.c | 7 +- drivers/pci/pci.h | 58 ++ drivers/pci/pci_acs_test.c | 860 ++++++++++++++++++++++++ drivers/pci/quirks.c | 6 +- include/linux/pci-p2pdma.h | 12 +- 10 files changed, 1687 insertions(+), 86 deletions(-) --- base-commit: 08dbfad3f5040f5bdb6c529da20d6d4e81fefd72 change-id: 20260821-fix-p2p-acs-v4-0-e72455e3a261 prerequisite-message-id: <20260830-batch-p2p-fixes-v1-0-5044e8dfbe2e(a)nvidia.com> prerequisite-patch-id: 6b25c7fcf164cdfc14e9fac5b908d97fcf6509d7 prerequisite-patch-id: 0d083c281001365aae4b35544cf28891a6ab9a96 prerequisite-patch-id: bfd9dabf271f3cc9a3a61f46387d20c20311363d prerequisite-patch-id: fad0275efc722830fc591509506c0a5e4f581073 prerequisite-patch-id: 0c83bee688fec1f6d1564654df7c630fa6a4a978 Best regards, -- Leon Romanovsky <leonro(a)nvidia.com>

2 days, 18 hours

Re: [PATCH 01/13] dt-bindings: display: msm: Add Qualcomm DCP bindings

by Krzysztof Kozlowski

On 23/10/2026 10:10, Jayant Shekhar wrote: >>> +properties: >>> + reg: >>> + maxItems: 1 >>> + description: Index of this DSI host among its siblings. >>> + >>> + status: true >> I see slop, I finished here. Mind this maintainers summit words - if you >> use AI, you must use good one, not the shloppy slop. > > > Will remove this in v2. And please also remove other sloppy-code produced by your AI as well. Best regards, Krzysztof

3 days, 8 hours

Re: [PATCH RFC 01/26] dma-buf: Introduce DMA-buf mapping types

by Christian König

On 10/22/26 08:52, Alistair Popple wrote: > On 2026-10-21 at 23:37 +1100, Jason Gunthorpe <jgg(a)nvidia.com> wrote... >> On Wed, Oct 21, 2026 at 08:24:40AM +0200, Thomas Hellström wrote: >>>> struct dma_buf_mapping_exp_ops >>>> Base type for the per-mapping type exporter provided functions. >>>> This >>>> would be the map/unmap callbacks. Each mapping type can provide >>>> its own >>>> functions for map/unmap type operations with optimal type >>>> signatures. >>> >>> Does it make sense to try to land this patch first, plus a conversion >>> of pcie_p2p so we could agree on what the contract should look like? >> >> I think it would be good to focus on this patch as the main 'crux' of >> the series. The API contract is the main agreement. I'm relucant to >> merge code that has no users, so it woudl be ebtter to go up to at >> least through SGT. >> >>> s/mapping/interconnect/? My gut feeling is that "interconnect" best >>> describes what's typically going on, but "mapping" perhaps better >>> describes all cases. >> >> My intention was to cover more than just the phyiscal transport. We do >> really want to get away from scatterlist, so having PCIe map with >> scatterlist and PCIe map with <something else> still ends up with two >> mapping types in the system. >> >> This is why I choose the name over interconnect. >> >>> Location of common protocols which typically carry cross-subsystem >>> dependencies? >>> drivers/dma-buf/mapping_types? drivers/dma-buf/interconnects? >> >> I'd encourage people to design more like PCI where the actual >> *interconnect* has its own home and there is just some minor call outs >> from dmabuf to help the matching logic. >> >>> As discussed previously: >>> Agree on theoretically possible matches at attach time, Match at >>> mapping time and guarantee the match is valid until >>> invalidate_mappings(). >> >> I was thinking about this some more, and I'm less sure now. >> >> If I think about mlx5, in a multi-path PCIe configuration, it can move >> the translation around within a single PCIe path, but it cannot move >> it to another path without creating disruption for the >> application. This is a HW limitation in how the internal routing >> works.. >> >> I strongly suspect this will be the norm for any hardware that doesn't >> rely on decorated page tables to select the IO path.. >> >> So, while I agree we should support hardware that can fully change the >> path on each and every map, we also need to suport hardware that must >> pin the path during attach. >> >> Probably this points to needing a 'pin' operation for the mapping >> selection after attach. > > If I'm understanding things correctly we're expecting a change in path between > attach and map time to be rare in practice. Therefore the need to handle this > case is mostly as a fallback to not break things. So instead of pinning could > new attachments just invalidate all existing incompatible attachments on the > list by setting eg. attach->valid = 0? No, that would result in ping/pong behavior which is something we explicitly try to avoid. This is also not something uncommon, it happens multiple times a second on a standard X11 Linux desktop when a new buffer is started to be scanned out on an A+A or A+I laptop. Regards, Christian. > > Attachment could also scan existing attachments to preferentially pick matching > mapping_types that are compatible with existing attachments, thus avoiding > invalidation where possible. > > Map time functions (eg. dma_buf_sgt_map_attachment) would check attach->valid > and return a failure if a subsequent attachment/event had invalidated the > mapping type. > > Importers with invalidated attachments would then have to retry creating the > attachment, probably getting a different mapping_type. Would this be acceptable? > Or do some importers absolutely require the mapping_type to never change once > the intial match/attachment has happened? > > - Alistair > >>>> +/* >>>> + * Match items are generated by the importer using the >>>> DMA_BUF_IMAPPING_*() and >>>> + * the exporter using the DMA_BUF_EMAPPING_*() functions. Each >>>> mapping type >>>> + * defines its own signature with its own data to make the match and >>>> attachment. >>>> + */ >>>> +struct dma_buf_mapping_match { >>>> + const struct dma_buf_mapping_type *type; >>>> + const struct dma_buf_mapping_exp_ops *exp_ops; >>>> + union { >>>> + /* Each mapping_type has unique match parameters >>>> here */ >>>> + }; >>> >>> Do we really need to add all global mapping types here? Then we'd need >>> something for driver-private mapping types as well. >> >> IIRC it is fixed size up front like this because of how the >> structure's memory is managed in this design.. >> >> If you want a driver private thing then I'd add a 'u64 >> driver_private[2]' to the union and the driver can cast over it. >> >> But this is the kernel, if you want to add an xe_mapping_type, then >> just add it. I don't know we need to do driver private things. >> >> Jason

4 days, 2 hours

Re: [PATCH RFC 01/26] dma-buf: Introduce DMA-buf mapping types

by Thomas Hellström

* On Thu, 2026-10-22 at 22:17 +1100, Alistair Popple wrote: > On 2026-10-22 at 18:25 +1100, Thomas Hellström > <thomas.hellstrom(a)linux.intel.com> wrote... > > On Thu, 2026-10-22 at 17:52 +1100, Alistair Popple wrote: > > > On 2026-10-21 at 23:37 +1100, Jason Gunthorpe <jgg(a)nvidia.com> > > > wrote... > > > > On Wed, Oct 21, 2026 at 08:24:40AM +0200, Thomas Hellström > > > > wrote: > > > > > > struct dma_buf_mapping_exp_ops > > > > > > Base type for the per-mapping type exporter provided > > > > > > functions. > > > > > > This > > > > > > would be the map/unmap callbacks. Each mapping type can > > > > > > provide > > > > > > its own > > > > > > functions for map/unmap type operations with optimal > > > > > > type > > > > > > signatures. > > > > > > > > > > Does it make sense to try to land this patch first, plus a > > > > > conversion > > > > > of pcie_p2p so we could agree on what the contract should > > > > > look > > > > > like? > > > > > > > > I think it would be good to focus on this patch as the main > > > > 'crux' > > > > of > > > > the series. The API contract is the main agreement. I'm > > > > relucant to > > > > merge code that has no users, so it woudl be ebtter to go up to > > > > at > > > > least through SGT. > > > > > > > > > s/mapping/interconnect/? My gut feeling is that > > > > > "interconnect" > > > > > best > > > > > describes what's typically going on, but "mapping" perhaps > > > > > better > > > > > describes all cases. > > > > > > > > My intention was to cover more than just the phyiscal > > > > transport. We > > > > do > > > > really want to get away from scatterlist, so having PCIe map > > > > with > > > > scatterlist and PCIe map with <something else> still ends up > > > > with > > > > two > > > > mapping types in the system. > > > > > > > > This is why I choose the name over interconnect. > > > > > > > > > Location of common protocols which typically carry cross- > > > > > subsystem > > > > > dependencies? > > > > > drivers/dma-buf/mapping_types? drivers/dma-buf/interconnects? > > > > > > > > I'd encourage people to design more like PCI where the actual > > > > *interconnect* has its own home and there is just some minor > > > > call > > > > outs > > > > from dmabuf to help the matching logic. > > > > > > > > > As discussed previously: > > > > > Agree on theoretically possible matches at attach time, Match > > > > > at > > > > > mapping time and guarantee the match is valid until > > > > > invalidate_mappings(). > > > > > > > > I was thinking about this some more, and I'm less sure now. > > > > > > > > If I think about mlx5, in a multi-path PCIe configuration, it > > > > can > > > > move > > > > the translation around within a single PCIe path, but it cannot > > > > move > > > > it to another path without creating disruption for the > > > > application. This is a HW limitation in how the internal > > > > routing > > > > works.. > > > > > > > > I strongly suspect this will be the norm for any hardware that > > > > doesn't > > > > rely on decorated page tables to select the IO path.. > > > > > > > > So, while I agree we should support hardware that can fully > > > > change > > > > the > > > > path on each and every map, we also need to suport hardware > > > > that > > > > must > > > > pin the path during attach. > > > > > > > > Probably this points to needing a 'pin' operation for the > > > > mapping > > > > selection after attach. > > > > > > If I'm understanding things correctly we're expecting a change in > > > path between > > > attach and map time to be rare in practice. Therefore the need to > > > handle this > > > case is mostly as a fallback to not break things. So instead of > > > pinning could > > > new attachments just invalidate all existing incompatible > > > attachments > > > on the > > > list by setting eg. attach->valid = 0? > > > > I agree, (although I think the "invalidate_attachment" needs to be > > discussed). In the end the importer should be given a choice, > > either > > pin and accept that subsequent attachers might see errors or > > accept > > app disruption. > > > > > > > > Attachment could also scan existing attachments to preferentially > > > pick matching > > > mapping_types that are compatible with existing attachments, thus > > > avoiding > > > invalidation where possible. > > > > Current design appears to be that attach only determines the set of > > theoretically possible mapping types, disregarding other > > attachments. > > Taking other attachments into account is also problematic because > > the > > dma-resv lock is not held. > > Oh I see. Do you foresee any problems just taking the dma-resv lock > to walk the > current attachments? If so we could probably resolve it with RCU, but > I'd rather > keep it simple if we can. It looks like dma_buf_dynamic_attach() actually takes the dma_resv lock to update the attachment list, so that should work. Provided we want to take existing attachments into account already at this stage. Need to take care, though, not to have the resv lock held over the ops- >attach callback into the driver. > > > Looking at current GPU drivers, what you describe above is > > typically > > done at pin() time, but if some importers want an early very likely > > decision on what mapping_type to use, one could introduce a > > separate > > function that match the mapping_types, but that would be very > > similar > > to the map() function but without returning a sgt/mapping, or a > > pin() > > without actually pinning. One could reuse the invalidate_mappings() > > callback to indicate the mapping type is no longer valid? > > I did wonder about that, but a callback for attachment invalidation > seems like > it might be a pain for importers to syncrhonize with. What I'm > thinking, which I > think is similar to what you describe, is something like the > following: > > 1. Importer gets the likley attachment by passing a list of mapping > types in > prefered order to dma_buf_mapping_attach() which returns the > dma_buf_attachment > that would work based on current attachments and what the exporter > provides: > > Something like this sounds OK, although given Jason's use-case, I added some comments: > attach = dma_buf_mapping_attach(...) > Just filter out impossible mappings. > 2. Takes dmabuf->resv lock. dma_buf_match_mapping_type(); Determines a mapping that is valid until the next invalidate_mappings(). NOP if a valid mapping already exists. Allows masking mapping_types. This allows Jason's use-case while keeping the attachment as a callback anchor. > > 3. Pins the attachment (if required): dma_buf_match_mapping_type(); Determines a mapping that is valid until the next invalidate_mappings(). NOP if a valid mapping already exists. Allows masking mapping_types. This allows Jason's use-case while keeping the attachment as a callback anchor. > > rc = dma_buf_pin(attach) This would implicitly pin also the mapping_type, but question is what happens if a link is severed? Then we'd want an invalidate_mappings() to re-negotiate the mapping type, but a pinned dma-buf that receives an invalidate_mappings() is revoked by definition. > > DMA-BUF core revalidates the attachment is still valid (ie. by > checking the > match again or via some other mechanism to detect the attachment > path/location > has changed such as a sequence/generation number). Since the resv is locked, nothing could have changed since dma_buf_match_mapping_type(); /Thomas > > On error the importer can either fail or try to create a new > attachment. > > 4. Map > > rc = dma_buf_[sgt|pcie_p2p]_map(attach, ...) > > Follows the same revalidation process as dma_buf_pin(). > > Thoughts? > > - Alistair > > > /Thomas > > > > > > > > > > Map time functions (eg. dma_buf_sgt_map_attachment) would check > > > attach->valid > > > and return a failure if a subsequent attachment/event had > > > invalidated > > > the > > > mapping type. > > > > > > Importers with invalidated attachments would then have to retry > > > creating the > > > attachment, probably getting a different mapping_type. Would this > > > be > > > acceptable? > > > Or do some importers absolutely require the mapping_type to never > > > change once > > > the intial match/attachment has happened? > > > > > > - Alistair > > > > > > > > > +/* > > > > > > + * Match items are generated by the importer using the > > > > > > DMA_BUF_IMAPPING_*() and > > > > > > + * the exporter using the DMA_BUF_EMAPPING_*() functions. > > > > > > Each > > > > > > mapping type > > > > > > + * defines its own signature with its own data to make the > > > > > > match and > > > > > > attachment. > > > > > > + */ > > > > > > +struct dma_buf_mapping_match { > > > > > > + const struct dma_buf_mapping_type *type; > > > > > > + const struct dma_buf_mapping_exp_ops *exp_ops; > > > > > > + union { > > > > > > + /* Each mapping_type has unique match > > > > > > parameters > > > > > > here */ > > > > > > + }; > > > > > > > > > > Do we really need to add all global mapping types here? Then > > > > > we'd > > > > > need > > > > > something for driver-private mapping types as well. > > > > > > > > IIRC it is fixed size up front like this because of how the > > > > structure's memory is managed in this design.. > > > > > > > > If you want a driver private thing then I'd add a 'u64 > > > > driver_private[2]' to the union and the driver can cast over > > > > it. > > > > > > > > But this is the kernel, if you want to add an xe_mapping_type, > > > > then > > > > just add it. I don't know we need to do driver private things. > > > > > > > > Jason

4 days, 2 hours

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig