This adds a DRM driver that implements communication between the CPU and an
APU. The driver target embedded device that usually run inference using some
prebuilt models. The goal is to provide common infrastructure that could be
re-used to support many accelerators. Both kernel, userspace and firmware tries
to use standard and existing to leverage the development and maintenance effort.
The series implements two platform drivers, one for simulation and another one for
the mt8183 (compatible with mt8365).
For the people interested by the firmware or userspace library,
the sources are available here:
https://gitlab.baylibre.com/baylibre/libapu/libapu
The support of APU has to be upstreamed to libdrm.
Until this is done, you could find the source here:
https://gitlab.baylibre.com/baylibre/libapu/libdrm/-/tree/abailon/main
The driver for mt8183 depends on this series (which is currently blocked):
https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=620429
Alexandre Bailon (5):
drm: Add support of AI Processor Unit (APU)
drm/apu: Add memory allocator
drm/apu: Add support of requests
drm/apu: Add support of IOMMU
dt-bindings: Add bidings for mtk,apu-drm
Julien Stephan (2):
drm/apu: allow platform driver to implement their own mmap function
drm/apu: Add support for a simulated APU
.../devicetree/bindings/gpu/mtk,apu-drm.yaml | 38 ++
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/apu/Kconfig | 22 +
drivers/gpu/drm/apu/Makefile | 10 +
drivers/gpu/drm/apu/apu_drv.c | 282 +++++++++
drivers/gpu/drm/apu/apu_gem.c | 230 +++++++
drivers/gpu/drm/apu/apu_internal.h | 205 ++++++
drivers/gpu/drm/apu/apu_sched.c | 592 ++++++++++++++++++
drivers/gpu/drm/apu/simu_apu.c | 313 +++++++++
include/uapi/drm/apu_drm.h | 81 +++
11 files changed, 1776 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpu/mtk,apu-drm.yaml
create mode 100644 drivers/gpu/drm/apu/Kconfig
create mode 100644 drivers/gpu/drm/apu/Makefile
create mode 100644 drivers/gpu/drm/apu/apu_drv.c
create mode 100644 drivers/gpu/drm/apu/apu_gem.c
create mode 100644 drivers/gpu/drm/apu/apu_internal.h
create mode 100644 drivers/gpu/drm/apu/apu_sched.c
create mode 100644 drivers/gpu/drm/apu/simu_apu.c
create mode 100644 include/uapi/drm/apu_drm.h
--
2.39.2
@@ -9,6 +9,18 @@extern"C"{#endif+/*+*Pleasenotethatmodificationstoallstructsdefinedhereare+*subjecttobackwards-compatibilityconstraints.+*/++structdrm_apu_gem_new{+__u32size;/* in */+__u32flags;/* in */+__u32handle;/* out */+__u64offset;/* out */+};+#define APU_ONLINE BIT(0)structdrm_apu_state{
Many AI Processur Unit (APU) have a similar architecture.
This driver intends helping supporting them.
This relies on DRM and provides some abstractions useful
for AI accelerators.
Currently, this provides the infrastructure to alloc an APU
device and register one or many cores.
The driver will takes care to register itself to DRM.
Signed-off-by: Alexandre Bailon <redacted>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
---
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/apu/Kconfig | 12 ++
drivers/gpu/drm/apu/Makefile | 5 +
drivers/gpu/drm/apu/apu_drv.c | 272 +++++++++++++++++++++++++++++
drivers/gpu/drm/apu/apu_internal.h | 68 ++++++++
include/uapi/drm/apu_drm.h | 28 +++
7 files changed, 388 insertions(+)
create mode 100644 drivers/gpu/drm/apu/Kconfig
create mode 100644 drivers/gpu/drm/apu/Makefile
create mode 100644 drivers/gpu/drm/apu/apu_drv.c
create mode 100644 drivers/gpu/drm/apu/apu_internal.h
create mode 100644 include/uapi/drm/apu_drm.h
@@ -371,6 +371,8 @@ source "drivers/gpu/drm/solomon/Kconfig"source"drivers/gpu/drm/sprd/Kconfig"+source"drivers/gpu/drm/apu/Kconfig"+configDRM_HYPERVtristate"DRM Support for Hyper-V synthetic video device"depends onDRM&&PCI&&MMU&&HYPERV
Some APU devices are behind an IOMMU.
For some of these devices, we can't use DMA API because
they use static addresses so we have to manually use
IOMMU API to correctly map the buffers.
This adds support of IOMMU.
Signed-off-by: Alexandre Bailon <redacted>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
---
drivers/gpu/drm/apu/apu_drv.c | 4 +
drivers/gpu/drm/apu/apu_gem.c | 174 +++++++++++++++++++++++++++++
drivers/gpu/drm/apu/apu_internal.h | 16 +++
drivers/gpu/drm/apu/apu_sched.c | 28 +++++
include/uapi/drm/apu_drm.h | 12 +-
5 files changed, 233 insertions(+), 1 deletion(-)
From: Robin Murphy <robin.murphy@arm.com> Date: 2023-05-18 13:25:08
On 2023-05-17 15:52, Alexandre Bailon wrote:
Some APU devices are behind an IOMMU.
For some of these devices, we can't use DMA API because
they use static addresses so we have to manually use
IOMMU API to correctly map the buffers.
Except you still need to use the DMA for the sake of cache coherency and
any other aspects :(
@@ -42,6 +45,7 @@ int ioctl_gem_new(struct drm_device *dev, void *data,*/apu_obj->size=args->size;apu_obj->offset=0;+apu_obj->iommu_refcount=0;mutex_init(&apu_obj->mutex);ret=drm_gem_handle_create(file_priv,gem_obj,&args->handle);
@@ -54,3 +58,173 @@ int ioctl_gem_new(struct drm_device *dev, void *data,return0;}++voidapu_bo_iommu_unmap(structapu_drm*apu_drm,structapu_gem_object*obj)+{+intiova_pfn;+inti;++if(!obj->iommu_sgt)+return;++mutex_lock(&obj->mutex);+obj->iommu_refcount--;+if(obj->iommu_refcount){+mutex_unlock(&obj->mutex);+return;+}++iova_pfn=PHYS_PFN(obj->iova);
Using mm layer operations on IOVAs looks wrong. In practice I don't
think it's ultimately harmful, other than potentially making less
efficient use of IOVA space if the CPU page size is larger than the
IOMMU page size, but it's still a bad code smell when you're using an
IOVA abstraction that is deliberately decoupled from CPU pages.
+ for (i = 0; i < obj->iommu_sgt->nents; i++) {
+ iommu_unmap(apu_drm->domain, PFN_PHYS(iova_pfn),
+ PAGE_ALIGN(obj->iommu_sgt->sgl[i].length));
+ iova_pfn += PHYS_PFN(PAGE_ALIGN(obj->iommu_sgt->sgl[i].length));
You can unmap a set of IOVA-contiguous mappings as a single range with
one call.
If you need things mapped at specific addresses like the commit message
claims, the DMA IOVA allocator is a terrible tool for the job. DRM
already has its own more flexible abstraction for address space
management in the form of drm_mm, so as a DRM driver it would seem a lot
more sensible to use one of those.
And even if you could justify using this allocator, I can't imagine
there's any way you'd need the _fast version (further illustrated by the
fact that you're freeing the IOVAs wrongly for that).
This looks a lot like it should just be iommu_map_sg(). Also it makes me
suspicious of the relationship between obj->size and the sgtable - if
the size is already pre-calculated to include any required padding then
why can't the caller just provide aligned SG segments in the first
place? Conversely if it's the original un-padded size, then any padding
you *do* add at this point means you're going to overrun the allocated
IOVA space.
If the domain *did* ever exist, but could suddenly disappear at any
point after you've decided to go ahead and start mapping things into it,
then there is a heck of a lot of sychronisation missing from this whole
infrastructure.
Thanks,
Robin.
From: Julien Stephan <jstephan@baylibre.com>
By default we will call drm_gem_mmap() unless the apu driver has
declared it's own mmap handler.
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
---
drivers/gpu/drm/apu/apu_drv.c | 38 +++++++++++++++++++++++++++++-
drivers/gpu/drm/apu/apu_internal.h | 2 ++
2 files changed, 39 insertions(+), 1 deletion(-)
From: Krzysztof Kozlowski <hidden> Date: 2023-05-17 19:45:29
On 17/05/2023 16:52, Alexandre Bailon wrote:
From: Julien Stephan <jstephan@baylibre.com>
By default we will call drm_gem_mmap() unless the apu driver has
declared it's own mmap handler.
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
One does not have to review own code. We all assume that we send good
code which we do not have to review by ourselves (by the author). We
also assume we make mistakes, which we cannot find, thus other person's
review is important.
Adding own review tag suggests you added them mechanically, so I doubt
that they really happened.
Anyway, your SoB is missing.
Best regards,
Krzysztof
From: Julien Stephan <jstephan@baylibre.com>
By default we will call drm_gem_mmap() unless the apu driver has
declared it's own mmap handler.
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
One does not have to review own code. We all assume that we send good
code which we do not have to review by ourselves (by the author). We
also assume we make mistakes, which we cannot find, thus other person's
review is important.
I am sorry, I am the one who made the misstake.
I squashed this patch with another one I made, lost my signedof and left
the reviewed by which indeed doesn't make any sense.
Best Regards,
Alexandre
Adding own review tag suggests you added them mechanically, so I doubt
that they really happened.
Anyway, your SoB is missing.
Best regards,
Krzysztof
This updates the APU driver to with two new ioctls
to queue and dequeue requests.
This uses DRM scheduler to manage the requests.
The requests allocation and send and receive operations
are platform specifics and must be implemented as callback.
Signed-off-by: Alexandre Bailon <redacted>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
---
drivers/gpu/drm/apu/Kconfig | 1 +
drivers/gpu/drm/apu/Makefile | 1 +
drivers/gpu/drm/apu/apu_drv.c | 52 +--
drivers/gpu/drm/apu/apu_internal.h | 93 ++++-
drivers/gpu/drm/apu/apu_sched.c | 564 +++++++++++++++++++++++++++++
include/uapi/drm/apu_drm.h | 31 +-
6 files changed, 697 insertions(+), 45 deletions(-)
create mode 100644 drivers/gpu/drm/apu/apu_sched.c
@@ -35,6 +41,43 @@ struct apu_drm {structidaida;};+structapu_job{+structdrm_sched_jobbase;++structkrefrefcount;++structapu_core*core;+structapu_drm*apu;++/* Fence to be signaled by IRQ handler when the job is complete. */+structdma_fence*done_fence;++__u32cmd;++/* Exclusive fences we have taken from the BOs to wait for */+structdma_fence**implicit_fences;+structdrm_gem_object**bos;+u32bo_count;++/* Fence to be signaled by drm-sched once its done with the job */+structdma_fence*render_done_fence;++void*data_in;+uint16_tsize_in;+void*data_out;+uint16_tsize_out;+uint16_tresult;+uint16_tid;++structlist_headnode;+structdrm_syncobj*sync_out;++structapu_event*event;++void*request_data;+intrequest_len;+};+/***@apu_core_ops:Providesplatformspecificcallbacks*/
@@ -0,0 +1,564 @@+// SPDX-License-Identifier: GPL-2.0+//+// Copyright 2020 BayLibre SAS++#include<drm/apu_drm.h>+#include<drm/drm_syncobj.h>+#include<drm/gpu_scheduler.h>++#include"apu_internal.h"++structapu_queue_state{+structdrm_gpu_schedulersched;+u64fence_context;+u64seqno;+};++structapu_request{+structlist_headnode;+structapu_job*job;+};++structapu_sched{+structapu_queue_stateapu_queue;+spinlock_tjob_lock;+structdrm_sched_entitysched_entity;+};++structapu_event{+structdrm_pending_eventpending_event;+union{+structdrm_eventbase;+structapu_job_eventjob_event;+};+};++staticDEFINE_IDA(req_ida);+staticLIST_HEAD(complete_node);++staticvoidapu_core_update_state(structapu_core*core)+{+if(!core->ops->is_ready(core))+core->flags&=~APU_ONLINE;+}++staticintapu_core_is_running(structapu_core*core)+{+apu_core_update_state(core);++returncore->flags&APU_ONLINE;+}++staticvoidapu_set_online(structapu_core*core)+{+core->flags|=APU_ONLINE;+}++staticvoidapu_set_offline(structapu_core*core)+{+core->flags&=~APU_ONLINE;+}++/*+*apu_drm_callback()-Handlethedatacomingfromaccelerator+*+*@core:ThepointertotheAPUcore+*@job_id:Thejobid+*@data:Thedatacomingfromtheaccelerator+*@len:Thesizeofthedata+*+*Returns:Zeroonsuccess,non-zerovalueonfailure.+*/+intapu_drm_callback(structapu_core*core,uint16_tjob_id,void*data,intlen)+{+structapu_request*apu_req,*tmp;+unsignedlongflags;+intret=-EINVAL;++spin_lock_irqsave(&core->ctx_lock,flags);+list_for_each_entry_safe(apu_req,tmp,&core->requests,node){+structapu_job*job=apu_req->job;++if(job&&job_id==job->id){+kref_get(&job->refcount);+ret=core->ops->handle_request(job,data,len);+list_add(&job->node,&complete_node);+list_del(&apu_req->node);+ida_simple_remove(&req_ida,job->id);+kfree(apu_req);+drm_send_event(&job->apu->base,+&job->event->pending_event);+dma_fence_signal_locked(job->done_fence);+break;+}+}+spin_unlock_irqrestore(&core->ctx_lock,flags);++returnret;+}+EXPORT_SYMBOL_GPL(apu_drm_callback);++staticvoidapu_job_cleanup(structkref*ref)+{+structapu_job*job=container_of(ref,structapu_job,+refcount);+unsignedinti;++if(job->implicit_fences){+for(i=0;i<job->bo_count;i++)+dma_fence_put(job->implicit_fences[i]);+kvfree(job->implicit_fences);+}+dma_fence_put(job->done_fence);+dma_fence_put(job->render_done_fence);++if(job->bos){+for(i=0;i<job->bo_count;i++){+structapu_gem_object*apu_obj;++apu_obj=to_apu_bo(job->bos[i]);+drm_gem_object_put(job->bos[i]);+}++kvfree(job->bos);+}++kfree(job->data_out);+kfree(job->data_in);+kfree(job);+}++staticvoidapu_job_put(structapu_job*job)+{+kref_put(&job->refcount,apu_job_cleanup);+}++staticvoidapu_acquire_object_fences(structdrm_gem_object**bos,+intbo_count,+structdma_fence**implicit_fences)+{+inti;++for(i=0;i<bo_count;i++)+dma_resv_get_singleton(bos[i]->resv,DMA_RESV_USAGE_KERNEL,+&implicit_fences[i]);+}++staticvoidapu_attach_object_fences(structdrm_gem_object**bos,+intbo_count,structdma_fence*fence)+{+inti;++for(i=0;i<bo_count;i++){+dma_resv_reserve_fences(bos[i]->resv,1);+dma_resv_add_fence(bos[i]->resv,fence,DMA_RESV_USAGE_KERNEL);+}+}++staticintapu_job_push(structapu_job*job)+{+structdrm_sched_entity*entity=&job->core->sched->sched_entity;+structww_acquire_ctxacquire_ctx;+intret=0;+inti;++ret=drm_gem_lock_reservations(job->bos,job->bo_count,&acquire_ctx);+if(ret)+returnret;++ret=drm_sched_job_init(&job->base,entity,NULL);+if(ret)+gotounlock;++drm_sched_job_arm(&job->base);+job->render_done_fence=dma_fence_get(&job->base.s_fence->finished);++kref_get(&job->refcount);/* put by scheduler job completion */+apu_acquire_object_fences(job->bos,job->bo_count,+job->implicit_fences);++drm_sched_entity_push_job(&job->base);++apu_attach_object_fences(job->bos,job->bo_count,+job->render_done_fence);++for(i=0;i<job->bo_count;i++)+ret=drm_sched_job_add_implicit_dependencies(&job->base,job->bos[i],+true);+unlock:+drm_gem_unlock_reservations(job->bos,job->bo_count,&acquire_ctx);++returnret;+}++staticconstchar*apu_fence_get_driver_name(structdma_fence*fence)+{+return"apu";+}++staticconstchar*apu_fence_get_timeline_name(structdma_fence*fence)+{+return"apu-0";+}++staticvoidapu_fence_release(structdma_fence*f)+{+kfree(f);+}++staticconststructdma_fence_opsapu_fence_ops={+.get_driver_name=apu_fence_get_driver_name,+.get_timeline_name=apu_fence_get_timeline_name,+.release=apu_fence_release,+};++staticstructdma_fence*apu_fence_create(structapu_sched*sched)+{+structdma_fence*fence;+structapu_queue_state*apu_queue=&sched->apu_queue;++fence=kzalloc(sizeof(*fence),GFP_KERNEL);+if(!fence)+returnERR_PTR(-ENOMEM);++dma_fence_init(fence,&apu_fence_ops,&sched->job_lock,+apu_queue->fence_context,apu_queue->seqno++);++returnfence;+}++staticstructapu_job*to_apu_job(structdrm_sched_job*sched_job)+{+returncontainer_of(sched_job,structapu_job,base);+}++staticintapu_job_hw_submit(structapu_job*job)+{+intret;+structapu_core*core=job->core;+structapu_request*apu_req;+unsignedlongflags;++ret=ida_simple_get(&req_ida,0,0xffff,GFP_KERNEL);+if(ret<0)+returnret;+job->id=ret;++ret=core->ops->alloc_prepare_request(job);+if(ret||!job->request_data||!job->request_len){+ret=-ENOMEM;+gotoerr_ida_remove;+}++apu_req=kzalloc(sizeof(*apu_req),GFP_KERNEL);+if(!apu_req){+ret=-ENOMEM;+gotoerr_free_data;+}++apu_req->job=job;+spin_lock_irqsave(&core->ctx_lock,flags);+list_add(&apu_req->node,&core->requests);+spin_unlock_irqrestore(&core->ctx_lock,flags);+ret=core->ops->send_request(job);+if(ret<0)+gotoerr;+kfree(job->request_data);++return0;++err:+list_del(&apu_req->node);+kfree(apu_req);+err_free_data:+kfree(job->request_data);+err_ida_remove:+ida_simple_remove(&req_ida,job->id);++returnret;+}++staticstructdma_fence*apu_job_run(structdrm_sched_job*sched_job)+{+structapu_job*job=to_apu_job(sched_job);+structdma_fence*fence=NULL;++if(unlikely(job->base.s_fence->finished.error))+returnNULL;++fence=apu_fence_create(job->core->sched);+if(IS_ERR(fence))+returnNULL;++job->done_fence=dma_fence_get(fence);++apu_job_hw_submit(job);++returnfence;+}++staticenumdrm_gpu_sched_statapu_job_timedout(structdrm_sched_job*sched_job)+{+structapu_request*apu_req,*tmp;+structapu_job*job=to_apu_job(sched_job);++if(dma_fence_is_signaled(job->done_fence))+returnDRM_GPU_SCHED_STAT_NOMINAL;++list_for_each_entry_safe(apu_req,tmp,&job->core->requests,node){+/* Remove the request and notify user about timeout */+if(apu_req->job==job){+kref_get(&job->refcount);+job->result=ETIMEDOUT;+list_add(&job->node,&complete_node);+list_del(&apu_req->node);+ida_simple_remove(&req_ida,job->id);+kfree(apu_req);+drm_send_event(&job->apu->base,+&job->event->pending_event);+dma_fence_signal_locked(job->done_fence);+}+}++returnDRM_GPU_SCHED_STAT_NOMINAL;+}++staticvoidapu_job_free(structdrm_sched_job*sched_job)+{+structapu_job*job=to_apu_job(sched_job);++drm_sched_job_cleanup(sched_job);++apu_job_put(job);+}++staticconststructdrm_sched_backend_opsapu_sched_ops={+.run_job=apu_job_run,+.timedout_job=apu_job_timedout,+.free_job=apu_job_free+};++intapu_drm_job_init(structapu_core*core)+{+intret;+structapu_sched*apu_sched;+structdrm_gpu_scheduler*sched;++apu_sched=devm_kzalloc(core->dev,sizeof(*apu_sched),GFP_KERNEL);+if(!apu_sched)+return-ENOMEM;++sched=&apu_sched->apu_queue.sched;+apu_sched->apu_queue.fence_context=dma_fence_context_alloc(1);+ret=drm_sched_init(sched,&apu_sched_ops,+1,0,msecs_to_jiffies(500),+NULL,NULL,"apu_js",core->dev);+if(ret){+dev_err(core->dev,"Failed to create scheduler");+returnret;+}++ret=drm_sched_entity_init(&apu_sched->sched_entity,+DRM_SCHED_PRIORITY_NORMAL,+&sched,1,NULL);+if(ret){+dev_err(core->dev,"Failed to initialize scheduler entity");+drm_sched_fini(&core->sched->apu_queue.sched);+returnret;+}++core->sched=apu_sched;+apu_set_online(core);++return0;+}++voidapu_sched_fini(structapu_core*core)+{+apu_set_offline(core);+drm_sched_fini(&core->sched->apu_queue.sched);+devm_kfree(core->dev,core->sched);+core->sched=NULL;+}++staticstructapu_core*get_apu_core(structapu_drm*apu,intdevice_id)+{+structapu_core*core;++list_for_each_entry(core,&apu->cores,node){+if(core->device_id==device_id)+returncore;+}++returnNULL;+}++staticintapu_lookup_bos(structdrm_device*dev,structdrm_file*file_priv,+structdrm_apu_gem_queue*args,structapu_job*job)+{+void__user*bo_handles;+intret;++job->bo_count=args->bo_handle_count;++if(!job->bo_count)+return0;++job->implicit_fences=kvmalloc_array(job->bo_count,+sizeof(structdma_fence*),+GFP_KERNEL|__GFP_ZERO);+if(!job->implicit_fences)+return-ENOMEM;++bo_handles=(void__user*)(uintptr_t)args->bo_handles;+ret=drm_gem_objects_lookup(file_priv,bo_handles,+job->bo_count,&job->bos);++returnret;+}++intioctl_gem_queue(structdrm_device*dev,void*data,+structdrm_file*file_priv)+{+structapu_drm*apu=dev->dev_private;+structdrm_apu_gem_queue*args=data;+structapu_event*event;+structapu_core*core;+structdrm_syncobj*sync_out=NULL;+structapu_job*job;+intret=0;++core=get_apu_core(apu,args->device);+if(!apu_core_is_running(core))+return-ENODEV;++if(args->out_sync>0){+sync_out=drm_syncobj_find(file_priv,args->out_sync);+if(!sync_out)+return-ENODEV;+}++job=kzalloc(sizeof(*job),GFP_KERNEL);+if(!job){+ret=-ENOMEM;+gotofail_out_sync;+}++kref_init(&job->refcount);++job->apu=apu;+job->core=core;+job->cmd=args->cmd;+job->size_in=args->size_in;+job->size_out=args->size_out;+job->sync_out=sync_out;+if(job->size_in){+job->data_in=kmalloc(job->size_in,GFP_KERNEL);+if(!job->data_in){+ret=-ENOMEM;+gotofail_job;+}++ret=+copy_from_user(job->data_in,+(void__user*)(uintptr_t)args->data,+job->size_in);+if(ret)+gotofail_job;+}++if(job->size_out){+job->data_out=kmalloc(job->size_out,GFP_KERNEL);+if(!job->data_out){+ret=-ENOMEM;+gotofail_job;+}+}++ret=apu_lookup_bos(dev,file_priv,args,job);+if(ret)+gotofail_job;++event=kzalloc(sizeof(*event),GFP_KERNEL);+event->base.length=sizeof(structapu_job_event);+event->base.type=APU_JOB_COMPLETED;+event->job_event.out_sync=args->out_sync;+job->event=event;+ret=drm_event_reserve_init(dev,file_priv,&job->event->pending_event,+&job->event->base);+if(ret)+gotofail_job;++ret=apu_job_push(job);+if(ret){+drm_event_cancel_free(dev,&job->event->pending_event);+gotofail_job;+}++if(sync_out)+drm_syncobj_replace_fence(sync_out,job->render_done_fence);++fail_job:+apu_job_put(job);+fail_out_sync:+if(sync_out)+drm_syncobj_put(sync_out);++returnret;+}++intioctl_gem_dequeue(structdrm_device*dev,void*data,+structdrm_file*file_priv)+{+structdrm_apu_gem_dequeue*args=data;+structdrm_syncobj*sync_out=NULL;+structapu_job*job;+intret=0;++if(args->out_sync>0){+sync_out=drm_syncobj_find(file_priv,args->out_sync);+if(!sync_out)+return-ENODEV;+}++list_for_each_entry(job,&complete_node,node){+if(job->sync_out==sync_out){+if(job->data_out){+ret=copy_to_user((void__user*)(uintptr_t)+args->data,job->data_out,+job->size_out);+args->size=job->size_out;+}+args->result=job->result;+list_del(&job->node);+apu_job_put(job);+drm_syncobj_put(sync_out);++returnret;+}+}++if(sync_out)+drm_syncobj_put(sync_out);++return0;+}++intioctl_apu_state(structdrm_device*dev,void*data,+structdrm_file*file_priv)+{+structapu_drm*apu=dev->dev_private;+structdrm_apu_state*args=data;+structapu_core*core;++args->flags=0;++core=get_apu_core(apu,args->device);+if(!core)+return-ENODEV;++apu_core_update_state(core);+args->flags|=core->flags;++return0;+}
@@ -0,0 +1,38 @@+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause+%YAML1.2+---+$id:http://devicetree.org/schemas/gpu/mediatek,apu-drm.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:AI Processor Unit DRM++properties:+compatible:+const:mediatek,apu-drm++remoteproc:+maxItems:2+description:+Handle to remoteproc devices controlling the APU++iova:+maxItems:1+description:+Address and size of virtual memory that could used by the APU++required:+-compatible+-remoteproc+-iova++additionalProperties:false++examples:+-|+apu@0 {+compatible = "mediatek,apu-drm";+remoteproc = <&vpu0>, <&vpu1>;+iova = <0 0x60000000 0 0x10000000>;+};++...
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> Date: 2023-05-17 15:05:47
Il 17/05/23 16:52, Alexandre Bailon ha scritto:
This adds the device tree bindings for the APU DRM driver.
Signed-off-by: Alexandre Bailon <redacted>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
---
.../devicetree/bindings/gpu/mtk,apu-drm.yaml | 38 +++++++++++++++++++
@@ -0,0 +1,38 @@+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause+%YAML1.2+---+$id:http://devicetree.org/schemas/gpu/mediatek,apu-drm.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:AI Processor Unit DRM++properties:+compatible:+const:mediatek,apu-drm
const: mediatek,mt8195-apu (or whatever else).
...besides, I don't think that this patch even belongs to this series? :-)
Spoiler alert! :-)
Cheers,
Angelo
On Wed, May 17, 2023 at 05:04:00PM +0200, AngeloGioacchino Del Regno wrote:
Il 17/05/23 16:52, Alexandre Bailon ha scritto:
quoted
This adds the device tree bindings for the APU DRM driver.
Signed-off-by: Alexandre Bailon <redacted>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
---
.../devicetree/bindings/gpu/mtk,apu-drm.yaml | 38 +++++++++++++++++++
@@ -0,0 +1,38 @@+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause+%YAML1.2+---+$id:http://devicetree.org/schemas/gpu/mediatek,apu-drm.yaml#+$schema:http://devicetree.org/meta-schemas/core.yaml#++title:AI Processor Unit DRM++properties:+compatible:+const:mediatek,apu-drm
const: mediatek,mt8195-apu (or whatever else).
Aye, and drop the references to DRM in the title field too (and add the
vendor name?).
...besides, I don't think that this patch even belongs to this series? :-)
Spoiler alert! :-)
Well, I do not know what this means - but if it is being respun as part
of some other work, a description field should be added to the binding.
Cheers,
Conor.
On 5/17/23 17:04, AngeloGioacchino Del Regno wrote:
Il 17/05/23 16:52, Alexandre Bailon ha scritto:
quoted
This adds the device tree bindings for the APU DRM driver.
Signed-off-by: Alexandre Bailon <redacted>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
---
.../devicetree/bindings/gpu/mtk,apu-drm.yaml | 38 +++++++++++++++++++
From: Rob Herring <robh@kernel.org> Date: 2023-05-17 15:31:55
On Wed, 17 May 2023 16:52:37 +0200, Alexandre Bailon wrote:
This adds the device tree bindings for the APU DRM driver.
Signed-off-by: Alexandre Bailon <redacted>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
---
.../devicetree/bindings/gpu/mtk,apu-drm.yaml | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpu/mtk,apu-drm.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpu/mtk,apu-drm.yaml: 'maintainers' is a required property
hint: Metaschema for devicetree binding documentation
from schema $id: http://devicetree.org/meta-schemas/base.yaml#
./Documentation/devicetree/bindings/gpu/mtk,apu-drm.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/gpu/mtk,apu-drm.yaml#
Documentation/devicetree/bindings/gpu/mtk,apu-drm.example.dts:18.15-22.11: Warning (unit_address_vs_reg): /example-0/apu@0: node has a unit name, but no reg or ranges property
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpu/mtk,apu-drm.example.dtb: apu@0: remoteproc: [[4294967295, 4294967295]] is too short
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpu/mtk,apu-drm.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230517145237.295461-8-abailon@baylibre.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
From: Krzysztof Kozlowski <hidden> Date: 2023-05-17 16:54:46
On Wed, 17 May 2023 16:52:37 +0200, Alexandre Bailon wrote:
This adds the device tree bindings for the APU DRM driver.
Signed-off-by: Alexandre Bailon <redacted>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
---
.../devicetree/bindings/gpu/mtk,apu-drm.yaml | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpu/mtk,apu-drm.yaml
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpu/mtk,apu-drm.yaml: 'maintainers' is a required property
hint: Metaschema for devicetree binding documentation
from schema $id: http://devicetree.org/meta-schemas/base.yaml#
./Documentation/devicetree/bindings/gpu/mtk,apu-drm.yaml: $id: relative path/filename doesn't match actual path or filename
expected: http://devicetree.org/schemas/gpu/mtk,apu-drm.yaml#
Documentation/devicetree/bindings/gpu/mtk,apu-drm.example.dts:18.15-22.11: Warning (unit_address_vs_reg): /example-0/apu@0: node has a unit name, but no reg or ranges property
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpu/mtk,apu-drm.example.dtb: apu@0: remoteproc: [[4294967295, 4294967295]] is too short
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/gpu/mtk,apu-drm.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1782720
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
From: Krzysztof Kozlowski <hidden> Date: 2023-05-17 19:38:44
On 17/05/2023 16:52, Alexandre Bailon wrote:
This adds the device tree bindings for the APU DRM driver.
Signed-off-by: Alexandre Bailon <redacted>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
There are so many errors in this patch... that for sure it was not
tested. Reduced review, except what was already said:
Why would you store virtual address, not real, in DT? Let's say you have
some randomization like KASLR. How is it going to work? Drop, it is not
hardware property.
Best regards,
Krzysztof
From: Krzysztof Kozlowski <hidden> Date: 2023-05-17 19:41:56
On 17/05/2023 21:38, Krzysztof Kozlowski wrote:
On 17/05/2023 16:52, Alexandre Bailon wrote:
quoted
This adds the device tree bindings for the APU DRM driver.
Signed-off-by: Alexandre Bailon <redacted>
Reviewed-by: Julien Stephan <jstephan@baylibre.com>
There are so many errors in this patch... that for sure it was not
tested. Reduced review, except what was already said:
Why would you store virtual address, not real, in DT? Let's say you have
some randomization like KASLR. How is it going to work? Drop, it is not
hardware property.
Actually RANDOMIZE_BASE. KASLR randomizes the physical.
Best regards,
Krzysztof
From: Julien Stephan <jstephan@baylibre.com>
This implements a driver to use with a simulation APU.
This is useful for testing purpose and can be used as a basis to
implement real platform driver.
Communication between the simulated APU and the driver is done
using netlink socket.
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
---
drivers/gpu/drm/apu/Kconfig | 9 +
drivers/gpu/drm/apu/Makefile | 3 +
drivers/gpu/drm/apu/simu_apu.c | 313 +++++++++++++++++++++++++++++++++
3 files changed, 325 insertions(+)
create mode 100644 drivers/gpu/drm/apu/simu_apu.c
@@ -0,0 +1,313 @@+// SPDX-License-Identifier: GPL-2.0+//+// Copyright 2023 BayLibre SAS++#include<linux/module.h>+#include<linux/netlink.h>+#include<linux/platform_device.h>+#include<linux/skbuff.h>++#include<net/sock.h>++#include<drm/apu_drm.h>++#include"apu_internal.h"+++#define MYPROTO 17+#define MYGRP 17++#define DRIVER_NAME "SIMU APU DRIVER"++/*+*Firmwarerequest,mustbealignedwiththeonedefinedinfirmware.+*@id:Requestid,usedinthecaseofreply,tofindthependingrequest+*@cmd:Thecommandidtoexecuteinthefirmware+*@result:Theresultofthecommandexecutedonthefirmware+*@size:Thesizeofthedataavailableinthisrequest+*@count:Thenumberofsharedbuffer+*@data:Containsthedataattachedwiththerequestifsizeisgreaterthan+*zero,andtheaddressesofsharedbuffersifcountisgreaterthan+*zero.Boththedataandthesharedbuffercouldbereadandwrite+*bytheAPU.+*/+structapu_dev_request{+u16id;+u16cmd;+u16result;+u16size_in;+u16size_out;+u16count;+u8data[0];+}__packed;++structplatform_device*platform;+structapu_core*apu_core;+staticintpid=-1;+structsock*nl_sock;++staticintapu_netlink_read(structsk_buff*skb,structapu_dev_request**msg_ptr,int*pid)+{+structnlmsghdr*nlh;++nlh=(structnlmsghdr*)skb->data;+*pid=nlh->nlmsg_pid;/* pid of sending process */+*msg_ptr=nlmsg_data(nlh);++returnnlh->nlmsg_len-NLMSG_HDRLEN;+}++staticintapu_netlink_write(void*msg_ptr,intmsg_size,intpid)+{+structsk_buff*skb_out;+structnlmsghdr*nlh;+intres;++skb_out=nlmsg_new(msg_size,0);+if(!skb_out)+return-ENOMEM;++nlh=nlmsg_put(skb_out,0,0,NLMSG_DONE,msg_size,0);+NETLINK_CB(skb_out).dst_group=0;/* not in multicast group */+memcpy(nlmsg_data(nlh),msg_ptr,msg_size);++res=nlmsg_unicast(nl_sock,skb_out,pid);++if(res<0)+returnres;+else+returnnlh->nlmsg_len-NLMSG_HDRLEN;+}++staticvoidnetlink_recv_msg(structsk_buff*skb)+{+intmsg_size;+structapu_dev_request*hdr;+intnlmsg_pid;++msg_size=apu_netlink_read(skb,&hdr,&nlmsg_pid);++if(pid==-1){+// No device registered yet, the first message should be+// "READY"+if(!strncmp((char*)hdr,"READY",strlen("READY"))){++pid=nlmsg_pid;+if(apu_core_register(&platform->dev,apu_core,apu_core->apu))+pr_err("cannot register SIMU APU\n");+}+}elseif(pid==nlmsg_pid){+if(!strncmp((char*)hdr,"STOP",strlen("STOP"))){+pid=-1;+apu_core_remove(apu_core);+}else+apu_drm_callback(apu_core,hdr->id,hdr,msg_size);+}else{+pr_err("%s: Only one core is supported for now\n",DRIVER_NAME);+}+}++staticintnetlink_setup(void)+{+intret=0;+structnetlink_kernel_cfgcfg={+.input=netlink_recv_msg,+};++nl_sock=netlink_kernel_create(&init_net,MYPROTO,&cfg);+if(!nl_sock)+ret=-ENOMEM;++returnret;+}++staticintsimu_apu_send(structapu_job*job)+{+returnapu_netlink_write((void*)(job->request_data),job->request_len,pid);+}++staticintsimu_apu_handle_request(structapu_job*job,void*data,intlen)+{+structapu_dev_request*hdr=data;++job->result=hdr->result;+if(job->size_out)+memcpy(job->data_out,hdr->data+job->size_in,+min(job->size_out,hdr->size_out));+job->size_out=hdr->size_out;+return0;+}++staticintsimu_apu_alloc_request(structapu_job*job)+{+structapu_dev_request*dev_req;++intsize;+u64*dev_req_da;+u32*dev_req_buffer_size;+inti;++size=sizeof(*dev_req)+(sizeof(u64)+sizeof(u32))*job->bo_count*2++job->size_in+job->size_out;+dev_req=kmalloc(size,GFP_KERNEL);+if(!dev_req)+return-ENOMEM;++dev_req->cmd=job->cmd;+dev_req->size_in=job->size_in;+dev_req->size_out=job->size_out;+dev_req->count=job->bo_count;+dev_req_da=+(u64*)(dev_req->data+dev_req->size_in+dev_req->size_out);+dev_req_buffer_size=(u32*)(dev_req_da+dev_req->count);+memcpy(dev_req->data,job->data_in,job->size_in);++for(i=0;i<job->bo_count;i++){+structapu_gem_object*obj=to_apu_bo(job->bos[i]);++dev_req_da[i]=drm_vma_node_offset_addr(&obj->base.base.vma_node);+dev_req_buffer_size[i]=obj->size;+}++dev_req->id=job->id;++job->request_data=dev_req;+job->request_len=size;+return0;+}++staticintsimu_apu_ready(structapu_core*core)+{+if(pid==-1)+return0;++return1;+}++/**+*simu_apu_gem_mmap+*+*thisisdirectlybasedondrm_gem_mmap()functionbutremovingthepermission+*checkbeforemappingabuffer.Thisisusefulheretobeabletoeasily+*sharebuffersbetweenlibapuhostapplicationandlibapudeviceapplication+*(simulationusecase)+*+*/+staticintsimu_apu_gem_mmap(structfile*filp,structvm_area_struct*vma)+{+structdrm_file*priv=filp->private_data;+structdrm_device*dev=priv->minor->dev;+structdrm_gem_object*obj=NULL;+structdrm_vma_offset_node*node;+intret;++if(drm_dev_is_unplugged(dev))+return-ENODEV;++drm_vma_offset_lock_lookup(dev->vma_offset_manager);+node=drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,+vma->vm_pgoff,+vma_pages(vma));+if(likely(node)){+obj=container_of(node,structdrm_gem_object,vma_node);+/*+*Whentheobjectisbeingfreed,afterithits0-refcntit+*proceedstoteardowntheobject.Intheprocessitwill+*attempttoremovetheVMAoffsetandsoacquirethis+*mgr->vm_lock.Thereforeifwefindanobjectwitha0-refcnt+*thatmatchesourrange,weknowitisintheprocessofbeing+*destroyedandwillbefreedassoonaswereleasethelock-+*sowehavetocheckforthe0-refcntedobjectandtreatitas+*invalid.+*/+if(!kref_get_unless_zero(&obj->refcount)){+obj=NULL;+pr_err("DTC: %s: %d\n",__func__,__LINE__);+}+}+drm_vma_offset_unlock_lookup(dev->vma_offset_manager);++if(!obj)+return-EINVAL;++ret=drm_gem_mmap_obj(obj,drm_vma_node_size(node)<<PAGE_SHIFT,+vma);++drm_gem_object_put(obj);++returnret;+}++staticstructapu_core_opssimu_apu_ops={+.alloc_prepare_request=simu_apu_alloc_request,+.send_request=simu_apu_send,+.handle_request=simu_apu_handle_request,+.is_ready=simu_apu_ready,+};++staticint__initapu_platform_init(void)+{+intret;+structapu_drm*apu;++platform=platform_device_register_simple(DRIVER_NAME,-1,NULL,0);+if(IS_ERR(platform))+returnPTR_ERR(platform);++if(!devres_open_group(&platform->dev,NULL,GFP_KERNEL)){+ret=-ENOMEM;+gotoout_unregister;+}++apu=apu_dev_alloc(&platform->dev);+if(!apu){+ret=-ENOMEM;+gotoout_devres;+}++apu_core=apu_core_alloc(apu,&simu_apu_ops,apu);+if(!apu_core){+ret=-ENOMEM;+gotoout_devres;+}++ret=apu_dev_register(apu);+if(ret)+gotoout_apu_core_free;++apu->mmap=simu_apu_gem_mmap;++ret=netlink_setup();+if(ret)+gotoout_apu_dev_unregister;++return0;++out_apu_dev_unregister:+apu_dev_unregister(apu);+out_apu_core_free:+apu_core_free(apu_core);+out_devres:+devres_release_group(&platform->dev,NULL);+out_unregister:+platform_device_unregister(platform);+returnret;+}++staticvoid__exitapu_platform_exit(void)+{+netlink_kernel_release(nl_sock);+apu_core_remove(apu_core);+apu_core_free(apu_core);+apu_dev_unregister((structapu_drm*)apu_core->apu);+devres_release_group(&platform->dev,NULL);+platform_device_unregister(platform);+}+++module_init(apu_platform_init);+module_exit(apu_platform_exit);++MODULE_LICENSE("GPL");+MODULE_AUTHOR("Julien Stephan");+MODULE_DESCRIPTION(DRIVER_NAME);
From: Thomas Zimmermann <tzimmermann@suse.de> Date: 2023-05-17 15:06:51
Hi,
it looks like this driver belongs into driver/accel.
Best regards
Thomas
Am 17.05.23 um 16:52 schrieb Alexandre Bailon:
This adds a DRM driver that implements communication between the CPU and an
APU. The driver target embedded device that usually run inference using some
prebuilt models. The goal is to provide common infrastructure that could be
re-used to support many accelerators. Both kernel, userspace and firmware tries
to use standard and existing to leverage the development and maintenance effort.
The series implements two platform drivers, one for simulation and another one for
the mt8183 (compatible with mt8365).
For the people interested by the firmware or userspace library,
the sources are available here:
https://gitlab.baylibre.com/baylibre/libapu/libapu
The support of APU has to be upstreamed to libdrm.
Until this is done, you could find the source here:
https://gitlab.baylibre.com/baylibre/libapu/libdrm/-/tree/abailon/main
The driver for mt8183 depends on this series (which is currently blocked):
https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=620429
Alexandre Bailon (5):
drm: Add support of AI Processor Unit (APU)
drm/apu: Add memory allocator
drm/apu: Add support of requests
drm/apu: Add support of IOMMU
dt-bindings: Add bidings for mtk,apu-drm
Julien Stephan (2):
drm/apu: allow platform driver to implement their own mmap function
drm/apu: Add support for a simulated APU
.../devicetree/bindings/gpu/mtk,apu-drm.yaml | 38 ++
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/apu/Kconfig | 22 +
drivers/gpu/drm/apu/Makefile | 10 +
drivers/gpu/drm/apu/apu_drv.c | 282 +++++++++
drivers/gpu/drm/apu/apu_gem.c | 230 +++++++
drivers/gpu/drm/apu/apu_internal.h | 205 ++++++
drivers/gpu/drm/apu/apu_sched.c | 592 ++++++++++++++++++
drivers/gpu/drm/apu/simu_apu.c | 313 +++++++++
include/uapi/drm/apu_drm.h | 81 +++
11 files changed, 1776 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpu/mtk,apu-drm.yaml
create mode 100644 drivers/gpu/drm/apu/Kconfig
create mode 100644 drivers/gpu/drm/apu/Makefile
create mode 100644 drivers/gpu/drm/apu/apu_drv.c
create mode 100644 drivers/gpu/drm/apu/apu_gem.c
create mode 100644 drivers/gpu/drm/apu/apu_internal.h
create mode 100644 drivers/gpu/drm/apu/apu_sched.c
create mode 100644 drivers/gpu/drm/apu/simu_apu.c
create mode 100644 include/uapi/drm/apu_drm.h
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
From: Jeffrey Hugo <hidden> Date: 2023-05-17 15:12:48
On 5/17/2023 8:52 AM, Alexandre Bailon wrote:
This adds a DRM driver that implements communication between the CPU and an
APU. The driver target embedded device that usually run inference using some
prebuilt models. The goal is to provide common infrastructure that could be
re-used to support many accelerators. Both kernel, userspace and firmware tries
to use standard and existing to leverage the development and maintenance effort.
The series implements two platform drivers, one for simulation and another one for
the mt8183 (compatible with mt8365).
This looks like the 3 existing Accel drivers. Why is this in DRM?
The support of APU has to be upstreamed to libdrm.
Until this is done, you could find the source here:
https://gitlab.baylibre.com/baylibre/libapu/libdrm/-/tree/abailon/main
The driver for mt8183 depends on this series (which is currently blocked):
https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=620429
Alexandre Bailon (5):
drm: Add support of AI Processor Unit (APU)
drm/apu: Add memory allocator
drm/apu: Add support of requests
drm/apu: Add support of IOMMU
dt-bindings: Add bidings for mtk,apu-drm
Julien Stephan (2):
drm/apu: allow platform driver to implement their own mmap function
drm/apu: Add support for a simulated APU
.../devicetree/bindings/gpu/mtk,apu-drm.yaml | 38 ++
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/apu/Kconfig | 22 +
drivers/gpu/drm/apu/Makefile | 10 +
drivers/gpu/drm/apu/apu_drv.c | 282 +++++++++
drivers/gpu/drm/apu/apu_gem.c | 230 +++++++
drivers/gpu/drm/apu/apu_internal.h | 205 ++++++
drivers/gpu/drm/apu/apu_sched.c | 592 ++++++++++++++++++
drivers/gpu/drm/apu/simu_apu.c | 313 +++++++++
include/uapi/drm/apu_drm.h | 81 +++
"apu" seems too generic. We already have 3 "AI processing units" over
in drivers/accel already...
From: Kevin Hilman <khilman@baylibre.com> Date: 2023-05-23 23:34:40
Jeffrey Hugo [off-list ref] writes:
On 5/17/2023 8:52 AM, Alexandre Bailon wrote:
quoted
This adds a DRM driver that implements communication between the CPU and an
APU. The driver target embedded device that usually run inference using some
prebuilt models. The goal is to provide common infrastructure that could be
re-used to support many accelerators. Both kernel, userspace and firmware tries
to use standard and existing to leverage the development and maintenance effort.
The series implements two platform drivers, one for simulation and another one for
the mt8183 (compatible with mt8365).
This looks like the 3 existing Accel drivers. Why is this in DRM?
Yes, this belongs in accel. I think Alex had some issues around the
infra in accel with device nodes not appearing/opening properly, but
I'll let him comment there. But either way, the right approach should
be to fix any issues in accel and move it there.
[...]
"apu" seems too generic. We already have 3 "AI processing units" over
in drivers/accel already...
Indeed, it is generic, but that's kind of the point for this driver
since it's targetted at generalizing the interface with "AI processing
units" on a growing number of embedded SoCs (ARM, RISC-V, etc.) In
addition, the generic naming is intentional because the goal is bigger
than the kernel and is working towards a generic, shared "libAPU"
userspace[1], but also common firmware for DSP-style inference engines
(e.g. analgous Sound Open Firmware for audio DSPs.)
As usual, the various SoC vendors use different names (APU, NPU, NN
unit, etc.) but we'd like a generic name for the class of devices
targetted by this driver. And unfortunately, it looks like the equally
generic "Versatile processing unit" is already taken Intel's
drivers/accel/ivpu. :)
Maybe since this is more about generalizing the interface between the
CPU running linux and the APU, what about the name apu_if? But I guess
that applies to the other 3 drivers in drivers/accell also. Hmmm...
Naming things is hard[2], so we're definitly open to other ideas. Any
suggestions?
Kevin
[1] https://gitlab.baylibre.com/baylibre/libapu/libapu
[2]
"There are 2 hard problems in computer science: cache invalidation,
naming things and off-by-1 errors."
-- https://twitter.com/secretGeek/status/7269997868
On Wed, May 24, 2023 at 2:34 AM Kevin Hilman [off-list ref] wrote:
Jeffrey Hugo [off-list ref] writes:
quoted
On 5/17/2023 8:52 AM, Alexandre Bailon wrote:
quoted
This adds a DRM driver that implements communication between the CPU and an
APU. The driver target embedded device that usually run inference using some
prebuilt models. The goal is to provide common infrastructure that could be
re-used to support many accelerators. Both kernel, userspace and firmware tries
to use standard and existing to leverage the development and maintenance effort.
The series implements two platform drivers, one for simulation and another one for
the mt8183 (compatible with mt8365).
This looks like the 3 existing Accel drivers. Why is this in DRM?
Yes, this belongs in accel. I think Alex had some issues around the
infra in accel with device nodes not appearing/opening properly, but
I'll let him comment there. But either way, the right approach should
be to fix any issues in accel and move it there.
[...]
"apu" seems too generic. We already have 3 "AI processing units" over
in drivers/accel already...
Indeed, it is generic, but that's kind of the point for this driver
since it's targetted at generalizing the interface with "AI processing
units" on a growing number of embedded SoCs (ARM, RISC-V, etc.) In
addition, the generic naming is intentional because the goal is bigger
than the kernel and is working towards a generic, shared "libAPU"
userspace[1], but also common firmware for DSP-style inference engines
(e.g. analgous Sound Open Firmware for audio DSPs.)
As usual, the various SoC vendors use different names (APU, NPU, NN
unit, etc.) but we'd like a generic name for the class of devices
targetted by this driver. And unfortunately, it looks like the equally
generic "Versatile processing unit" is already taken Intel's
drivers/accel/ivpu. :)
Maybe since this is more about generalizing the interface between the
CPU running linux and the APU, what about the name apu_if? But I guess
that applies to the other 3 drivers in drivers/accell also. Hmmm...
Naming things is hard[2], so we're definitly open to other ideas. Any
suggestions?
Maybe model it according to the tiny driver in drm display ? You can
then call it tiny_apu :-)
Disclosure: It was Daniel's suggestion, he can chime in with more
details on the tiny driver concept.
Oded
From: Daniel Vetter <hidden> Date: 2023-05-24 10:40:44
On Wed, May 24, 2023 at 01:27:00PM +0300, Oded Gabbay wrote:
On Wed, May 24, 2023 at 2:34 AM Kevin Hilman [off-list ref] wrote:
quoted
Jeffrey Hugo [off-list ref] writes:
quoted
On 5/17/2023 8:52 AM, Alexandre Bailon wrote:
quoted
This adds a DRM driver that implements communication between the CPU and an
APU. The driver target embedded device that usually run inference using some
prebuilt models. The goal is to provide common infrastructure that could be
re-used to support many accelerators. Both kernel, userspace and firmware tries
to use standard and existing to leverage the development and maintenance effort.
The series implements two platform drivers, one for simulation and another one for
the mt8183 (compatible with mt8365).
This looks like the 3 existing Accel drivers. Why is this in DRM?
Yes, this belongs in accel. I think Alex had some issues around the
infra in accel with device nodes not appearing/opening properly, but
I'll let him comment there. But either way, the right approach should
be to fix any issues in accel and move it there.
[...]
"apu" seems too generic. We already have 3 "AI processing units" over
in drivers/accel already...
Indeed, it is generic, but that's kind of the point for this driver
since it's targetted at generalizing the interface with "AI processing
units" on a growing number of embedded SoCs (ARM, RISC-V, etc.) In
addition, the generic naming is intentional because the goal is bigger
than the kernel and is working towards a generic, shared "libAPU"
userspace[1], but also common firmware for DSP-style inference engines
(e.g. analgous Sound Open Firmware for audio DSPs.)
As usual, the various SoC vendors use different names (APU, NPU, NN
unit, etc.) but we'd like a generic name for the class of devices
targetted by this driver. And unfortunately, it looks like the equally
generic "Versatile processing unit" is already taken Intel's
drivers/accel/ivpu. :)
Maybe since this is more about generalizing the interface between the
CPU running linux and the APU, what about the name apu_if? But I guess
that applies to the other 3 drivers in drivers/accell also. Hmmm...
Naming things is hard[2], so we're definitly open to other ideas. Any
suggestions?
Maybe model it according to the tiny driver in drm display ? You can
then call it tiny_apu :-)
Disclosure: It was Daniel's suggestion, he can chime in with more
details on the tiny driver concept.
Yeah so maybe a bit more detail on my thoughts:
First this smells like a need bypass of the entire "we want open userspace
for accel drivers" rule. The rule isn't quite a strict as for drm gpu
drivers (not sure we ended up documenting exactly what, but iirc the
consensus was that for build-time only dependencies we're ok with
downstream compilers), but it's still there.
And at least from a quick look apu.ko and libapu just look like a generic
accel interface, and that's not enough.
For the big training engines it's more or less "enough to run pytorch, but
it can be really slow", not sure what the right standard for these
inference-only drivers should be.
So that's the first reason why I don't like this.
The other is that I think if we do end up with a pile of tiny accel
drivers, we should probably look into something like simmpledrm for the
tiny display drivers. Probably still IP specific ioctls (at least most) so
that IP specific job knows and all that are easy, but then just pass to a
framework that simplifies a drm gem driver to "write ptes" and "run job"
callback, maybe with an optional "create/destroy vm/ctx" for hw which can
do that.
So maybe we end up with a drivers/accel/tiny and a bunch more helpers
around the existing gem ones. The rule we have for drm/tiny is "1 file,
less than 1kloc", and there's a bunch of them. I do think we can achieve
the same for tiny accel inference engines (but it's still a bit a road).
Maybe tiny accel is more like "less than 5kloc" since you need a bit more
glue for the driver specific ioctl stuff - maybe that's only needed for
the submit ioctl, maybe also for buffer map/unmap and creation.
Also note that there's an entire pile of in-flight work for adding new
helpers to the gem world to make this all easier. Once we have gpuva and
exec helpers there not much glue left to tie it all together with the
scheduler.
But the real crux is that an accel inference driver really needs to have
enough userspace to do an actual inference job with some
android/cros/whatever framework for inference (there's just too many).
-Daniel
On Wed, May 24, 2023 at 01:27:00PM +0300, Oded Gabbay wrote:
quoted
On Wed, May 24, 2023 at 2:34 AM Kevin Hilman [off-list ref] wrote:
quoted
Jeffrey Hugo [off-list ref] writes:
quoted
On 5/17/2023 8:52 AM, Alexandre Bailon wrote:
quoted
This adds a DRM driver that implements communication between the CPU and an
APU. The driver target embedded device that usually run inference using some
prebuilt models. The goal is to provide common infrastructure that could be
re-used to support many accelerators. Both kernel, userspace and firmware tries
to use standard and existing to leverage the development and maintenance effort.
The series implements two platform drivers, one for simulation and another one for
the mt8183 (compatible with mt8365).
This looks like the 3 existing Accel drivers. Why is this in DRM?
Yes, this belongs in accel. I think Alex had some issues around the
infra in accel with device nodes not appearing/opening properly, but
I'll let him comment there. But either way, the right approach should
be to fix any issues in accel and move it there.
[...]
"apu" seems too generic. We already have 3 "AI processing units" over
in drivers/accel already...
Indeed, it is generic, but that's kind of the point for this driver
since it's targetted at generalizing the interface with "AI processing
units" on a growing number of embedded SoCs (ARM, RISC-V, etc.) In
addition, the generic naming is intentional because the goal is bigger
than the kernel and is working towards a generic, shared "libAPU"
userspace[1], but also common firmware for DSP-style inference engines
(e.g. analgous Sound Open Firmware for audio DSPs.)
As usual, the various SoC vendors use different names (APU, NPU, NN
unit, etc.) but we'd like a generic name for the class of devices
targetted by this driver. And unfortunately, it looks like the equally
generic "Versatile processing unit" is already taken Intel's
drivers/accel/ivpu. :)
Maybe since this is more about generalizing the interface between the
CPU running linux and the APU, what about the name apu_if? But I guess
that applies to the other 3 drivers in drivers/accell also. Hmmm...
Naming things is hard[2], so we're definitly open to other ideas. Any
suggestions?
Maybe model it according to the tiny driver in drm display ? You can
then call it tiny_apu :-)
Disclosure: It was Daniel's suggestion, he can chime in with more
details on the tiny driver concept.
Yeah so maybe a bit more detail on my thoughts:
First this smells like a need bypass of the entire "we want open userspace
for accel drivers" rule. The rule isn't quite a strict as for drm gpu
drivers (not sure we ended up documenting exactly what, but iirc the
consensus was that for build-time only dependencies we're ok with
downstream compilers), but it's still there.
What is letting you think that we want to bypass open source requirements ?
Although the neural network firmware and userspace application are not yet
opensource, our intention is to develop a full open source stack.
Currently, we only support Mediatek APU (an Xtensa VP6) and we have to
use closed source sotfware to execute inferences on the accelerator.
As far I know, there software stack similar to mesa where we could add
support of a new accelerator (this is also true for firmware).
That is actually what we would like to do. But this will take a lot of
time and we consider this driver as a first (small) step.
And at least from a quick look apu.ko and libapu just look like a generic
accel interface, and that's not enough.
For the big training engines it's more or less "enough to run pytorch, but
it can be really slow", not sure what the right standard for these
inference-only drivers should be.
To be honest, I don't know what would be required for training engines.
We only target accelerators for embedded device that usually only run
inferences. In my opinion, this is 2 different use cases and I don't
think we could address them in the same way.
So that's the first reason why I don't like this.
The other is that I think if we do end up with a pile of tiny accel
drivers, we should probably look into something like simmpledrm for the
tiny display drivers. Probably still IP specific ioctls (at least most) so
that IP specific job knows and all that are easy, but then just pass to a
framework that simplifies a drm gem driver to "write ptes" and "run job"
callback, maybe with an optional "create/destroy vm/ctx" for hw which can
do that.
So maybe we end up with a drivers/accel/tiny and a bunch more helpers
around the existing gem ones. The rule we have for drm/tiny is "1 file,
less than 1kloc", and there's a bunch of them. I do think we can achieve
the same for tiny accel inference engines (but it's still a bit a road).
Maybe tiny accel is more like "less than 5kloc" since you need a bit more
glue for the driver specific ioctl stuff - maybe that's only needed for
the submit ioctl, maybe also for buffer map/unmap and creation.
This makes sense to me.
Also note that there's an entire pile of in-flight work for adding new
helpers to the gem world to make this all easier. Once we have gpuva and
exec helpers there not much glue left to tie it all together with the
scheduler.
I wrote this series a long time ago and just rebased it recently.
I will take some time to see the in-flight work and see if that
something I could start using.
But the real crux is that an accel inference driver really needs to have
enough userspace to do an actual inference job with some
android/cros/whatever framework for inference (there's just too many).
We are currently stuck with closed source fimrware, userspace
applications and toolchains (works with android and linux).
We are looking for a solution but implementing something will take some
time.
Alexandre