| Message ID | 20231204173313.2098733-1-boris.brezillon@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: Add a driver for CSF-based Mali GPUs | expand |
Hi Boris, On Mon, 4 Dec 2023 at 18:33, Boris Brezillon <boris.brezillon@collabora.com> wrote: > > Hello, > > This is the 3rd version of the kernel driver for Mali CSF-based GPUs. > > With all the DRM dependencies being merged (drm-sched single entity and > drm_gpuvm), I thought now was a good time to post a new version. Note > that the iommu series we depend on [1] has been merged recently. The > only remaining dependency that hasn't been merged yet is this rather > trival drm_gpuvm [2] patch. > > As for v2, I pushed a branch based on drm-misc-next and containing > all the dependencies that are not yet available in drm-misc-next > here[3], and another [4] containing extra patches to have things > working on rk3588. The CSF firmware binary can be found here[5], and > should be placed under /lib/firmware/arm/mali/arch10.8/mali_csffw.bin. > > The mesa MR adding v10 support on top of panthor is available here [6]. > > Regarding the GPL2+MIT relicensing, Liviu did an audit and found two > more people that I didn't spot initially: Clément Péron for the devfreq > code, and Alexey Sheplyakov for some bits in panthor_gpu.c. Both are > Cc-ed on the relevant patches. The rest of the code is either new, or > covered by the Linaro, Arm and Collabora acks. I only did some trivial cleaning, the relicensing is fine for me. Do you need me to ack some patches? Regards, Clement > > And here is a non-exhaustive changelog, check each commit for a detailed > changelog. > > v3; > - Quite a few changes at the MMU/sched level to make the fix some > race conditions and deadlocks > - Addition of the a sync-only VM_BIND operation (to support > vkQueueSparseBind with zero commands). > - Addition of a VM_GET_STATE ioctl > - Various cosmetic changes (see the commit changelogs for more details) > - Various fixes (see the commit changelogs for more details) > > v2: > - Rename the driver (pancsf -> panthor) > - Split the commit adding the driver to ease review > - Use drm_sched for dependency tracking/job submission > - Add a VM_BIND ioctl > - Add the concept of exclusive VM for BOs that are only ever mapped to a > single VM > - Document the code and uAPI > - Add a DT binding doc > > Regards, > > Boris > > [1]https://lore.kernel.org/linux-arm-kernel/20231124142434.1577550-1-boris.brezillon@collabora.com/T/ > [2]https://patchwork.freedesktop.org/patch/570380/ > [3]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v3 > [4]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v3+rk3588 > [5]https://gitlab.com/firefly-linux/external/libmali/-/raw/firefly/firmware/g610/mali_csffw.bin > [6]https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26358 > > Boris Brezillon (13): > drm/panthor: Add uAPI > drm/panthor: Add GPU register definitions > drm/panthor: Add the device logical block > drm/panthor: Add the GPU logical block > drm/panthor: Add GEM logical block > drm/panthor: Add the devfreq logical block > drm/panthor: Add the MMU/VM logical block > drm/panthor: Add the FW logical block > drm/panthor: Add the heap logical block > drm/panthor: Add the scheduler logical block > drm/panthor: Add the driver frontend block > drm/panthor: Allow driver compilation > drm/panthor: Add an entry to MAINTAINERS > > Liviu Dudau (1): > dt-bindings: gpu: mali-valhall-csf: Add support for Arm Mali CSF GPUs > > .../bindings/gpu/arm,mali-valhall-csf.yaml | 147 + > Documentation/gpu/driver-uapi.rst | 5 + > MAINTAINERS | 11 + > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/panthor/Kconfig | 23 + > drivers/gpu/drm/panthor/Makefile | 15 + > drivers/gpu/drm/panthor/panthor_devfreq.c | 283 ++ > drivers/gpu/drm/panthor/panthor_devfreq.h | 25 + > drivers/gpu/drm/panthor/panthor_device.c | 497 +++ > drivers/gpu/drm/panthor/panthor_device.h | 381 ++ > drivers/gpu/drm/panthor/panthor_drv.c | 1454 +++++++ > drivers/gpu/drm/panthor/panthor_fw.c | 1332 +++++++ > drivers/gpu/drm/panthor/panthor_fw.h | 504 +++ > drivers/gpu/drm/panthor/panthor_gem.c | 227 ++ > drivers/gpu/drm/panthor/panthor_gem.h | 144 + > drivers/gpu/drm/panthor/panthor_gpu.c | 481 +++ > drivers/gpu/drm/panthor/panthor_gpu.h | 52 + > drivers/gpu/drm/panthor/panthor_heap.c | 517 +++ > drivers/gpu/drm/panthor/panthor_heap.h | 36 + > drivers/gpu/drm/panthor/panthor_mmu.c | 2653 +++++++++++++ > drivers/gpu/drm/panthor/panthor_mmu.h | 101 + > drivers/gpu/drm/panthor/panthor_regs.h | 237 ++ > drivers/gpu/drm/panthor/panthor_sched.c | 3410 +++++++++++++++++ > drivers/gpu/drm/panthor/panthor_sched.h | 48 + > include/uapi/drm/panthor_drm.h | 892 +++++ > 26 files changed, 13478 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml > create mode 100644 drivers/gpu/drm/panthor/Kconfig > create mode 100644 drivers/gpu/drm/panthor/Makefile > create mode 100644 drivers/gpu/drm/panthor/panthor_devfreq.c > create mode 100644 drivers/gpu/drm/panthor/panthor_devfreq.h > create mode 100644 drivers/gpu/drm/panthor/panthor_device.c > create mode 100644 drivers/gpu/drm/panthor/panthor_device.h > create mode 100644 drivers/gpu/drm/panthor/panthor_drv.c > create mode 100644 drivers/gpu/drm/panthor/panthor_fw.c > create mode 100644 drivers/gpu/drm/panthor/panthor_fw.h > create mode 100644 drivers/gpu/drm/panthor/panthor_gem.c > create mode 100644 drivers/gpu/drm/panthor/panthor_gem.h > create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.c > create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.h > create mode 100644 drivers/gpu/drm/panthor/panthor_heap.c > create mode 100644 drivers/gpu/drm/panthor/panthor_heap.h > create mode 100644 drivers/gpu/drm/panthor/panthor_mmu.c > create mode 100644 drivers/gpu/drm/panthor/panthor_mmu.h > create mode 100644 drivers/gpu/drm/panthor/panthor_regs.h > create mode 100644 drivers/gpu/drm/panthor/panthor_sched.c > create mode 100644 drivers/gpu/drm/panthor/panthor_sched.h > create mode 100644 include/uapi/drm/panthor_drm.h > > -- > 2.43.0 >
Hi Clément, On Mon, 4 Dec 2023 19:09:54 +0100 Clément Péron <peron.clem@gmail.com> wrote: > Hi Boris, > > On Mon, 4 Dec 2023 at 18:33, Boris Brezillon > <boris.brezillon@collabora.com> wrote: > > > > Hello, > > > > This is the 3rd version of the kernel driver for Mali CSF-based GPUs. > > > > With all the DRM dependencies being merged (drm-sched single entity and > > drm_gpuvm), I thought now was a good time to post a new version. Note > > that the iommu series we depend on [1] has been merged recently. The > > only remaining dependency that hasn't been merged yet is this rather > > trival drm_gpuvm [2] patch. > > > > As for v2, I pushed a branch based on drm-misc-next and containing > > all the dependencies that are not yet available in drm-misc-next > > here[3], and another [4] containing extra patches to have things > > working on rk3588. The CSF firmware binary can be found here[5], and > > should be placed under /lib/firmware/arm/mali/arch10.8/mali_csffw.bin. > > > > The mesa MR adding v10 support on top of panthor is available here [6]. > > > > Regarding the GPL2+MIT relicensing, Liviu did an audit and found two > > more people that I didn't spot initially: Clément Péron for the devfreq > > code, and Alexey Sheplyakov for some bits in panthor_gpu.c. Both are > > Cc-ed on the relevant patches. The rest of the code is either new, or > > covered by the Linaro, Arm and Collabora acks. > > I only did some trivial cleaning, the relicensing is fine for me. > Do you need me to ack some patches? Yes, please. We identified that most of your contributions were related to the devfreq logic ("drm/panthor: Add the devfreq logical block" in this series), but if you remember contributing to other files in panfrost, feel free to give a general ack on all your panfrost contributions. Thanks, Boris
Hi Steve, I forgot to mention that I intentionally dropped your R-b, because there was a gazillion of changes all over the place, and I thought it deserved a fresh review. Regards, Boris On Mon, 4 Dec 2023 18:32:53 +0100 Boris Brezillon <boris.brezillon@collabora.com> wrote: > Hello, > > This is the 3rd version of the kernel driver for Mali CSF-based GPUs. > > With all the DRM dependencies being merged (drm-sched single entity and > drm_gpuvm), I thought now was a good time to post a new version. Note > that the iommu series we depend on [1] has been merged recently. The > only remaining dependency that hasn't been merged yet is this rather > trival drm_gpuvm [2] patch. > > As for v2, I pushed a branch based on drm-misc-next and containing > all the dependencies that are not yet available in drm-misc-next > here[3], and another [4] containing extra patches to have things > working on rk3588. The CSF firmware binary can be found here[5], and > should be placed under /lib/firmware/arm/mali/arch10.8/mali_csffw.bin. > > The mesa MR adding v10 support on top of panthor is available here [6]. > > Regarding the GPL2+MIT relicensing, Liviu did an audit and found two > more people that I didn't spot initially: Clément Péron for the devfreq > code, and Alexey Sheplyakov for some bits in panthor_gpu.c. Both are > Cc-ed on the relevant patches. The rest of the code is either new, or > covered by the Linaro, Arm and Collabora acks. > > And here is a non-exhaustive changelog, check each commit for a detailed > changelog. > > v3; > - Quite a few changes at the MMU/sched level to make the fix some > race conditions and deadlocks > - Addition of the a sync-only VM_BIND operation (to support > vkQueueSparseBind with zero commands). > - Addition of a VM_GET_STATE ioctl > - Various cosmetic changes (see the commit changelogs for more details) > - Various fixes (see the commit changelogs for more details) > > v2: > - Rename the driver (pancsf -> panthor) > - Split the commit adding the driver to ease review > - Use drm_sched for dependency tracking/job submission > - Add a VM_BIND ioctl > - Add the concept of exclusive VM for BOs that are only ever mapped to a > single VM > - Document the code and uAPI > - Add a DT binding doc > > Regards, > > Boris > > [1]https://lore.kernel.org/linux-arm-kernel/20231124142434.1577550-1-boris.brezillon@collabora.com/T/ > [2]https://patchwork.freedesktop.org/patch/570380/ > [3]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v3 > [4]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v3+rk3588 > [5]https://gitlab.com/firefly-linux/external/libmali/-/raw/firefly/firmware/g610/mali_csffw.bin > [6]https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26358 > > Boris Brezillon (13): > drm/panthor: Add uAPI > drm/panthor: Add GPU register definitions > drm/panthor: Add the device logical block > drm/panthor: Add the GPU logical block > drm/panthor: Add GEM logical block > drm/panthor: Add the devfreq logical block > drm/panthor: Add the MMU/VM logical block > drm/panthor: Add the FW logical block > drm/panthor: Add the heap logical block > drm/panthor: Add the scheduler logical block > drm/panthor: Add the driver frontend block > drm/panthor: Allow driver compilation > drm/panthor: Add an entry to MAINTAINERS > > Liviu Dudau (1): > dt-bindings: gpu: mali-valhall-csf: Add support for Arm Mali CSF GPUs > > .../bindings/gpu/arm,mali-valhall-csf.yaml | 147 + > Documentation/gpu/driver-uapi.rst | 5 + > MAINTAINERS | 11 + > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/panthor/Kconfig | 23 + > drivers/gpu/drm/panthor/Makefile | 15 + > drivers/gpu/drm/panthor/panthor_devfreq.c | 283 ++ > drivers/gpu/drm/panthor/panthor_devfreq.h | 25 + > drivers/gpu/drm/panthor/panthor_device.c | 497 +++ > drivers/gpu/drm/panthor/panthor_device.h | 381 ++ > drivers/gpu/drm/panthor/panthor_drv.c | 1454 +++++++ > drivers/gpu/drm/panthor/panthor_fw.c | 1332 +++++++ > drivers/gpu/drm/panthor/panthor_fw.h | 504 +++ > drivers/gpu/drm/panthor/panthor_gem.c | 227 ++ > drivers/gpu/drm/panthor/panthor_gem.h | 144 + > drivers/gpu/drm/panthor/panthor_gpu.c | 481 +++ > drivers/gpu/drm/panthor/panthor_gpu.h | 52 + > drivers/gpu/drm/panthor/panthor_heap.c | 517 +++ > drivers/gpu/drm/panthor/panthor_heap.h | 36 + > drivers/gpu/drm/panthor/panthor_mmu.c | 2653 +++++++++++++ > drivers/gpu/drm/panthor/panthor_mmu.h | 101 + > drivers/gpu/drm/panthor/panthor_regs.h | 237 ++ > drivers/gpu/drm/panthor/panthor_sched.c | 3410 +++++++++++++++++ > drivers/gpu/drm/panthor/panthor_sched.h | 48 + > include/uapi/drm/panthor_drm.h | 892 +++++ > 26 files changed, 13478 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml > create mode 100644 drivers/gpu/drm/panthor/Kconfig > create mode 100644 drivers/gpu/drm/panthor/Makefile > create mode 100644 drivers/gpu/drm/panthor/panthor_devfreq.c > create mode 100644 drivers/gpu/drm/panthor/panthor_devfreq.h > create mode 100644 drivers/gpu/drm/panthor/panthor_device.c > create mode 100644 drivers/gpu/drm/panthor/panthor_device.h > create mode 100644 drivers/gpu/drm/panthor/panthor_drv.c > create mode 100644 drivers/gpu/drm/panthor/panthor_fw.c > create mode 100644 drivers/gpu/drm/panthor/panthor_fw.h > create mode 100644 drivers/gpu/drm/panthor/panthor_gem.c > create mode 100644 drivers/gpu/drm/panthor/panthor_gem.h > create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.c > create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.h > create mode 100644 drivers/gpu/drm/panthor/panthor_heap.c > create mode 100644 drivers/gpu/drm/panthor/panthor_heap.h > create mode 100644 drivers/gpu/drm/panthor/panthor_mmu.c > create mode 100644 drivers/gpu/drm/panthor/panthor_mmu.h > create mode 100644 drivers/gpu/drm/panthor/panthor_regs.h > create mode 100644 drivers/gpu/drm/panthor/panthor_sched.c > create mode 100644 drivers/gpu/drm/panthor/panthor_sched.h > create mode 100644 include/uapi/drm/panthor_drm.h >
On 05/12/2023 08:48, Boris Brezillon wrote: > Hi Steve, > > I forgot to mention that I intentionally dropped your R-b, because > there was a gazillion of changes all over the place, and I thought it > deserved a fresh review. No problem, I'll re-review the patches. Thanks for getting the v3 out to review. <snip> >> [3]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v3 >> [4]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v3+rk3588 AFAICT neither of these trees match (exactly) with what you've posted. Not a big deal - I'll review the patches on the list, but it's a little confusing having a 'v3' branch which doesn't match the actual v3 posted ;) I also note you have similarly named branches in https://gitlab.freedesktop.org/bbrezillon/linux which are more up-to-date but also not 'v3'. Steve >> [5]https://gitlab.com/firefly-linux/external/libmali/-/raw/firefly/firmware/g610/mali_csffw.bin >> [6]https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26358 >> >> Boris Brezillon (13): >> drm/panthor: Add uAPI >> drm/panthor: Add GPU register definitions >> drm/panthor: Add the device logical block >> drm/panthor: Add the GPU logical block >> drm/panthor: Add GEM logical block >> drm/panthor: Add the devfreq logical block >> drm/panthor: Add the MMU/VM logical block >> drm/panthor: Add the FW logical block >> drm/panthor: Add the heap logical block >> drm/panthor: Add the scheduler logical block >> drm/panthor: Add the driver frontend block >> drm/panthor: Allow driver compilation >> drm/panthor: Add an entry to MAINTAINERS > > > >> >> Liviu Dudau (1): >> dt-bindings: gpu: mali-valhall-csf: Add support for Arm Mali CSF GPUs >> >> .../bindings/gpu/arm,mali-valhall-csf.yaml | 147 + >> Documentation/gpu/driver-uapi.rst | 5 + >> MAINTAINERS | 11 + >> drivers/gpu/drm/Kconfig | 2 + >> drivers/gpu/drm/Makefile | 1 + >> drivers/gpu/drm/panthor/Kconfig | 23 + >> drivers/gpu/drm/panthor/Makefile | 15 + >> drivers/gpu/drm/panthor/panthor_devfreq.c | 283 ++ >> drivers/gpu/drm/panthor/panthor_devfreq.h | 25 + >> drivers/gpu/drm/panthor/panthor_device.c | 497 +++ >> drivers/gpu/drm/panthor/panthor_device.h | 381 ++ >> drivers/gpu/drm/panthor/panthor_drv.c | 1454 +++++++ >> drivers/gpu/drm/panthor/panthor_fw.c | 1332 +++++++ >> drivers/gpu/drm/panthor/panthor_fw.h | 504 +++ >> drivers/gpu/drm/panthor/panthor_gem.c | 227 ++ >> drivers/gpu/drm/panthor/panthor_gem.h | 144 + >> drivers/gpu/drm/panthor/panthor_gpu.c | 481 +++ >> drivers/gpu/drm/panthor/panthor_gpu.h | 52 + >> drivers/gpu/drm/panthor/panthor_heap.c | 517 +++ >> drivers/gpu/drm/panthor/panthor_heap.h | 36 + >> drivers/gpu/drm/panthor/panthor_mmu.c | 2653 +++++++++++++ >> drivers/gpu/drm/panthor/panthor_mmu.h | 101 + >> drivers/gpu/drm/panthor/panthor_regs.h | 237 ++ >> drivers/gpu/drm/panthor/panthor_sched.c | 3410 +++++++++++++++++ >> drivers/gpu/drm/panthor/panthor_sched.h | 48 + >> include/uapi/drm/panthor_drm.h | 892 +++++ >> 26 files changed, 13478 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml >> create mode 100644 drivers/gpu/drm/panthor/Kconfig >> create mode 100644 drivers/gpu/drm/panthor/Makefile >> create mode 100644 drivers/gpu/drm/panthor/panthor_devfreq.c >> create mode 100644 drivers/gpu/drm/panthor/panthor_devfreq.h >> create mode 100644 drivers/gpu/drm/panthor/panthor_device.c >> create mode 100644 drivers/gpu/drm/panthor/panthor_device.h >> create mode 100644 drivers/gpu/drm/panthor/panthor_drv.c >> create mode 100644 drivers/gpu/drm/panthor/panthor_fw.c >> create mode 100644 drivers/gpu/drm/panthor/panthor_fw.h >> create mode 100644 drivers/gpu/drm/panthor/panthor_gem.c >> create mode 100644 drivers/gpu/drm/panthor/panthor_gem.h >> create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.c >> create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.h >> create mode 100644 drivers/gpu/drm/panthor/panthor_heap.c >> create mode 100644 drivers/gpu/drm/panthor/panthor_heap.h >> create mode 100644 drivers/gpu/drm/panthor/panthor_mmu.c >> create mode 100644 drivers/gpu/drm/panthor/panthor_mmu.h >> create mode 100644 drivers/gpu/drm/panthor/panthor_regs.h >> create mode 100644 drivers/gpu/drm/panthor/panthor_sched.c >> create mode 100644 drivers/gpu/drm/panthor/panthor_sched.h >> create mode 100644 include/uapi/drm/panthor_drm.h >> >
Hi Steve, On Wed, 6 Dec 2023 15:47:29 +0000 Steven Price <steven.price@arm.com> wrote: > On 05/12/2023 08:48, Boris Brezillon wrote: > > Hi Steve, > > > > I forgot to mention that I intentionally dropped your R-b, because > > there was a gazillion of changes all over the place, and I thought it > > deserved a fresh review. > > No problem, I'll re-review the patches. Thanks for getting the v3 out to > review. > > <snip> > > >> [3]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v3 > >> [4]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v3+rk3588 > > AFAICT neither of these trees match (exactly) with what you've posted. > Not a big deal - I'll review the patches on the list, but it's a little > confusing having a 'v3' branch which doesn't match the actual v3 posted > ;) I also note you have similarly named branches in > https://gitlab.freedesktop.org/bbrezillon/linux which are more > up-to-date but also not 'v3'. Oops, definitely forgot to push to the panfrost tree. I also messed things up with the VM_GET_STATE ioctl addition, which I intended to be part of this v3, but somehow forgot to regenerate the patches. v3 branches should now match this series, and I also pushed preliminary v4 branches containing !fixup commits in case you want to have a quick look (nothing fundamentally different there). Regards, Boris
> On Dec 5, 2023, at 2:32, Boris Brezillon <boris.brezillon@collabora.com> wrote: > > Hello, > > This is the 3rd version of the kernel driver for Mali CSF-based GPUs. > > With all the DRM dependencies being merged (drm-sched single entity and > drm_gpuvm), I thought now was a good time to post a new version. Note > that the iommu series we depend on [1] has been merged recently. The > only remaining dependency that hasn't been merged yet is this rather > trival drm_gpuvm [2] patch. > > As for v2, I pushed a branch based on drm-misc-next and containing > all the dependencies that are not yet available in drm-misc-next > here[3], and another [4] containing extra patches to have things > working on rk3588. The CSF firmware binary can be found here[5], and > should be placed under /lib/firmware/arm/mali/arch10.8/mali_csffw.bin. > > The mesa MR adding v10 support on top of panthor is available here [6]. > > Regarding the GPL2+MIT relicensing, Liviu did an audit and found two > more people that I didn't spot initially: Clément Péron for the devfreq > code, and Alexey Sheplyakov for some bits in panthor_gpu.c. Both are > Cc-ed on the relevant patches. The rest of the code is either new, or > covered by the Linaro, Arm and Collabora acks. > > And here is a non-exhaustive changelog, check each commit for a detailed > changelog. > > v3; > - Quite a few changes at the MMU/sched level to make the fix some > race conditions and deadlocks > - Addition of the a sync-only VM_BIND operation (to support > vkQueueSparseBind with zero commands). Hi Boris, Just wanted to point out that vkQueueBindSparse's semantics is rather different from vkQueueSubmit when it comes to synchronization. In short, vkQueueBindSparse does not operate on a particular timeline (aka scheduling queue / drm_sched_entity). The property of following a timeline order is known as “submission order” [1] in Vulkan, and applies to vkQueueSubmit only and not vkQueueBindSparse. Hence, an implementation that takes full advantage of Vulkan semantics would essentially have an infinite amount of VM_BIND contexts. It would also not need sync-only VM_BIND submissions, assuming that drmSyncobjTransfer works. I’m not saying that the driver should always be implemented that way; in particular, app developers are also confused by the semantics and native Vulkan games can be terribly wrong [2]. D3D12 has serializing semantics for UpdateTileMappings, so for D3D12 emulation one timeline is fine also. Tatsuyuki. [1]: https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#synchronization-submission-order [2]: https://gitlab.freedesktop.org/mesa/mesa/-/issues/10234 > - Addition of a VM_GET_STATE ioctl > - Various cosmetic changes (see the commit changelogs for more details) > - Various fixes (see the commit changelogs for more details) > > v2: > - Rename the driver (pancsf -> panthor) > - Split the commit adding the driver to ease review > - Use drm_sched for dependency tracking/job submission > - Add a VM_BIND ioctl > - Add the concept of exclusive VM for BOs that are only ever mapped to a > single VM > - Document the code and uAPI > - Add a DT binding doc > > Regards, > > Boris > > [1]https://lore.kernel.org/linux-arm-kernel/20231124142434.1577550-1-boris.brezillon@collabora.com/T/ > [2]https://patchwork.freedesktop.org/patch/570380/ > [3]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v3 > [4]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor-v3+rk3588 > [5]https://gitlab.com/firefly-linux/external/libmali/-/raw/firefly/firmware/g610/mali_csffw.bin > [6]https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26358 > > Boris Brezillon (13): > drm/panthor: Add uAPI > drm/panthor: Add GPU register definitions > drm/panthor: Add the device logical block > drm/panthor: Add the GPU logical block > drm/panthor: Add GEM logical block > drm/panthor: Add the devfreq logical block > drm/panthor: Add the MMU/VM logical block > drm/panthor: Add the FW logical block > drm/panthor: Add the heap logical block > drm/panthor: Add the scheduler logical block > drm/panthor: Add the driver frontend block > drm/panthor: Allow driver compilation > drm/panthor: Add an entry to MAINTAINERS > > Liviu Dudau (1): > dt-bindings: gpu: mali-valhall-csf: Add support for Arm Mali CSF GPUs > > .../bindings/gpu/arm,mali-valhall-csf.yaml | 147 + > Documentation/gpu/driver-uapi.rst | 5 + > MAINTAINERS | 11 + > drivers/gpu/drm/Kconfig | 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/panthor/Kconfig | 23 + > drivers/gpu/drm/panthor/Makefile | 15 + > drivers/gpu/drm/panthor/panthor_devfreq.c | 283 ++ > drivers/gpu/drm/panthor/panthor_devfreq.h | 25 + > drivers/gpu/drm/panthor/panthor_device.c | 497 +++ > drivers/gpu/drm/panthor/panthor_device.h | 381 ++ > drivers/gpu/drm/panthor/panthor_drv.c | 1454 +++++++ > drivers/gpu/drm/panthor/panthor_fw.c | 1332 +++++++ > drivers/gpu/drm/panthor/panthor_fw.h | 504 +++ > drivers/gpu/drm/panthor/panthor_gem.c | 227 ++ > drivers/gpu/drm/panthor/panthor_gem.h | 144 + > drivers/gpu/drm/panthor/panthor_gpu.c | 481 +++ > drivers/gpu/drm/panthor/panthor_gpu.h | 52 + > drivers/gpu/drm/panthor/panthor_heap.c | 517 +++ > drivers/gpu/drm/panthor/panthor_heap.h | 36 + > drivers/gpu/drm/panthor/panthor_mmu.c | 2653 +++++++++++++ > drivers/gpu/drm/panthor/panthor_mmu.h | 101 + > drivers/gpu/drm/panthor/panthor_regs.h | 237 ++ > drivers/gpu/drm/panthor/panthor_sched.c | 3410 +++++++++++++++++ > drivers/gpu/drm/panthor/panthor_sched.h | 48 + > include/uapi/drm/panthor_drm.h | 892 +++++ > 26 files changed, 13478 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml > create mode 100644 drivers/gpu/drm/panthor/Kconfig > create mode 100644 drivers/gpu/drm/panthor/Makefile > create mode 100644 drivers/gpu/drm/panthor/panthor_devfreq.c > create mode 100644 drivers/gpu/drm/panthor/panthor_devfreq.h > create mode 100644 drivers/gpu/drm/panthor/panthor_device.c > create mode 100644 drivers/gpu/drm/panthor/panthor_device.h > create mode 100644 drivers/gpu/drm/panthor/panthor_drv.c > create mode 100644 drivers/gpu/drm/panthor/panthor_fw.c > create mode 100644 drivers/gpu/drm/panthor/panthor_fw.h > create mode 100644 drivers/gpu/drm/panthor/panthor_gem.c > create mode 100644 drivers/gpu/drm/panthor/panthor_gem.h > create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.c > create mode 100644 drivers/gpu/drm/panthor/panthor_gpu.h > create mode 100644 drivers/gpu/drm/panthor/panthor_heap.c > create mode 100644 drivers/gpu/drm/panthor/panthor_heap.h > create mode 100644 drivers/gpu/drm/panthor/panthor_mmu.c > create mode 100644 drivers/gpu/drm/panthor/panthor_mmu.h > create mode 100644 drivers/gpu/drm/panthor/panthor_regs.h > create mode 100644 drivers/gpu/drm/panthor/panthor_sched.c > create mode 100644 drivers/gpu/drm/panthor/panthor_sched.h > create mode 100644 include/uapi/drm/panthor_drm.h > > -- > 2.43.0 > >
Hi, On Sun, 10 Dec 2023 13:58:51 +0900 Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote: > > On Dec 5, 2023, at 2:32, Boris Brezillon <boris.brezillon@collabora.com> wrote: > > > > Hello, > > > > This is the 3rd version of the kernel driver for Mali CSF-based GPUs. > > > > With all the DRM dependencies being merged (drm-sched single entity and > > drm_gpuvm), I thought now was a good time to post a new version. Note > > that the iommu series we depend on [1] has been merged recently. The > > only remaining dependency that hasn't been merged yet is this rather > > trival drm_gpuvm [2] patch. > > > > As for v2, I pushed a branch based on drm-misc-next and containing > > all the dependencies that are not yet available in drm-misc-next > > here[3], and another [4] containing extra patches to have things > > working on rk3588. The CSF firmware binary can be found here[5], and > > should be placed under /lib/firmware/arm/mali/arch10.8/mali_csffw.bin. > > > > The mesa MR adding v10 support on top of panthor is available here [6]. > > > > Regarding the GPL2+MIT relicensing, Liviu did an audit and found two > > more people that I didn't spot initially: Clément Péron for the devfreq > > code, and Alexey Sheplyakov for some bits in panthor_gpu.c. Both are > > Cc-ed on the relevant patches. The rest of the code is either new, or > > covered by the Linaro, Arm and Collabora acks. > > > > And here is a non-exhaustive changelog, check each commit for a detailed > > changelog. > > > > v3; > > - Quite a few changes at the MMU/sched level to make the fix some > > race conditions and deadlocks > > - Addition of the a sync-only VM_BIND operation (to support > > vkQueueSparseBind with zero commands). > > Hi Boris, > > Just wanted to point out that vkQueueBindSparse's semantics is rather different > from vkQueueSubmit when it comes to synchronization. In short, > vkQueueBindSparse does not operate on a particular timeline (aka scheduling > queue / drm_sched_entity). The property of following a timeline order is known > as “submission order” [1] in Vulkan, and applies to vkQueueSubmit only and not > vkQueueBindSparse. Hm, okay. I really though the same ordering guarantees applied to sparse binding queues too, as the spec [1] says " Batches begin execution in the order they appear in pBindInfo, but may complete out of order. " which means things are submitted in order inside a vkQueueSparseBind context, so I was expecting the submission ordering guarantee to apply across vkQueueSparseBind() calls on the same queue too. Just want to mention that all kernel implementations I have seen so far assume VM_BIND submissions on a given queue are serialized (that's how drm_sched works, and Xe, Nouveau and Panthor are basing their VM_BIND implemenation on drm_sched). Maybe Faith, or anyone deeply involved in the Vulkan specification, can confirm that submission ordering guarantees are relaxed on sparse binding queues. > > Hence, an implementation that takes full advantage of Vulkan semantics would > essentially have an infinite amount of VM_BIND contexts. Uh, that's definitely not how I understood it initially... > It would also not need > sync-only VM_BIND submissions, assuming that drmSyncobjTransfer works. Sure, if each vkQueueSparseBind() has its own timeline, an internal timeline-syncobj with a bunch of drmSyncobjTransfer() calls would do the trick (might require several ioctls() to merge input syncobjs, but that's probably not the end of the world). > > I’m not saying that the driver should always be implemented that way; in > particular, app developers are also confused by the semantics and native Vulkan > games can be terribly wrong [2]. Okay, thanks for the pointer. If I may, I find this semantics utterly confusing, to say the least. At the very least, the vkQueueBindSparse() doc could be update so it clearly reflects the facts submission order is not guaranteed across vkQueueBindSparse() calls, because right now it's only suggested in the [3], by the lack of vkQueueBindSparse() mention in the first bullet, which can be interpreted as a genuine omission rather than an expected behavior. Regards, Boris [1]https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkQueueBindSparse.html [2]https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkQueueBindSparse.html#VUID-vkQueueBindSparse-pBindInfo-parameter [3]https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#synchronization-submission-order
On Mon, 2023-12-11 at 09:52 +0100, Boris Brezillon wrote: > Hi, > > On Sun, 10 Dec 2023 13:58:51 +0900 > Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote: > > > > On Dec 5, 2023, at 2:32, Boris Brezillon > > > <boris.brezillon@collabora.com> wrote: > > > > > > Hello, > > > > > > This is the 3rd version of the kernel driver for Mali CSF-based > > > GPUs. > > > > > > With all the DRM dependencies being merged (drm-sched single > > > entity and > > > drm_gpuvm), I thought now was a good time to post a new version. > > > Note > > > that the iommu series we depend on [1] has been merged recently. > > > The > > > only remaining dependency that hasn't been merged yet is this > > > rather > > > trival drm_gpuvm [2] patch. > > > > > > As for v2, I pushed a branch based on drm-misc-next and > > > containing > > > all the dependencies that are not yet available in drm-misc-next > > > here[3], and another [4] containing extra patches to have things > > > working on rk3588. The CSF firmware binary can be found here[5], > > > and > > > should be placed under > > > /lib/firmware/arm/mali/arch10.8/mali_csffw.bin. > > > > > > The mesa MR adding v10 support on top of panthor is available > > > here [6]. > > > > > > Regarding the GPL2+MIT relicensing, Liviu did an audit and found > > > two > > > more people that I didn't spot initially: Clément Péron for the > > > devfreq > > > code, and Alexey Sheplyakov for some bits in panthor_gpu.c. Both > > > are > > > Cc-ed on the relevant patches. The rest of the code is either > > > new, or > > > covered by the Linaro, Arm and Collabora acks. > > > > > > And here is a non-exhaustive changelog, check each commit for a > > > detailed > > > changelog. > > > > > > v3; > > > - Quite a few changes at the MMU/sched level to make the fix some > > > race conditions and deadlocks > > > - Addition of the a sync-only VM_BIND operation (to support > > > vkQueueSparseBind with zero commands). > > > > Hi Boris, > > > > Just wanted to point out that vkQueueBindSparse's semantics is > > rather different > > from vkQueueSubmit when it comes to synchronization. In short, > > vkQueueBindSparse does not operate on a particular timeline (aka > > scheduling > > queue / drm_sched_entity). The property of following a timeline > > order is known > > as “submission order” [1] in Vulkan, and applies to vkQueueSubmit > > only and not > > vkQueueBindSparse. > > Hm, okay. I really though the same ordering guarantees applied to > sparse binding queues too, as the spec [1] says > > " > Batches begin execution in the order they appear in pBindInfo, but > may complete out of order. > " Right. So this is something where the Vulkan spec isn't terribly clear and I think I need to go file a spec bug. I'm fairly sure that the intent is that bind operations MAY complete out-of-order but are not required to complete out-of-order. In other words, I'm fairly sure that implementations are allowed but not required to insert extra dependencies that force some amount of ordering. We take advantage of this in Mesa today to properly implement vkQueueWaitIdle() on sparse binding queues. This is also a requirement of Windows WDDM2 as far as I can tell so I'd be very surprised if we disallowed it in the spec. That said, that's all very unclear and IDK where you'd get that from the spec. I'm going to go file a spec bug so we can get this sorted out. ~Faith > which means things are submitted in order inside a vkQueueSparseBind > context, so I was expecting the submission ordering guarantee to > apply > across vkQueueSparseBind() calls on the same queue too. Just want to > mention that all kernel implementations I have seen so far assume > VM_BIND submissions on a given queue are serialized (that's how > drm_sched works, and Xe, Nouveau and Panthor are basing their VM_BIND > implemenation on drm_sched). > > Maybe Faith, or anyone deeply involved in the Vulkan specification, > can > confirm that submission ordering guarantees are relaxed on sparse > binding queues. > > > > > Hence, an implementation that takes full advantage of Vulkan > > semantics would > > essentially have an infinite amount of VM_BIND contexts. > > Uh, that's definitely not how I understood it initially... > > > It would also not need > > sync-only VM_BIND submissions, assuming that drmSyncobjTransfer > > works. > > Sure, if each vkQueueSparseBind() has its own timeline, an internal > timeline-syncobj with a bunch of drmSyncobjTransfer() calls would do > the > trick (might require several ioctls() to merge input syncobjs, but > that's probably not the end of the world). > > > > > I’m not saying that the driver should always be implemented that > > way; in > > particular, app developers are also confused by the semantics and > > native Vulkan > > games can be terribly wrong [2]. > > Okay, thanks for the pointer. If I may, I find this semantics utterly > confusing, to say the least. At the very least, the > vkQueueBindSparse() > doc could be update so it clearly reflects the facts submission order > is not guaranteed across vkQueueBindSparse() calls, because right now > it's only suggested in the [3], by the lack of vkQueueBindSparse() > mention in the first bullet, which can be interpreted as a genuine > omission rather than an expected behavior. > > Regards, > > Boris > > [1] > https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkQ > ueueBindSparse.html > [2] > https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkQ > ueueBindSparse.html#VUID-vkQueueBindSparse-pBindInfo-parameter > [3] > https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec. > html#synchronization-submission-order
Hi Faith, Sorry for the late reply, I only got back to panthor very recently. On Mon, 11 Dec 2023 12:18:04 -0600 Faith Ekstrand <faith.ekstrand@collabora.com> wrote: > On Mon, 2023-12-11 at 09:52 +0100, Boris Brezillon wrote: > > Hi, > > > > On Sun, 10 Dec 2023 13:58:51 +0900 > > Tatsuyuki Ishi <ishitatsuyuki@gmail.com> wrote: > > > > > > On Dec 5, 2023, at 2:32, Boris Brezillon > > > > <boris.brezillon@collabora.com> wrote: > > > > > > > > Hello, > > > > > > > > This is the 3rd version of the kernel driver for Mali CSF-based > > > > GPUs. > > > > > > > > With all the DRM dependencies being merged (drm-sched single > > > > entity and > > > > drm_gpuvm), I thought now was a good time to post a new version. > > > > Note > > > > that the iommu series we depend on [1] has been merged recently. > > > > The > > > > only remaining dependency that hasn't been merged yet is this > > > > rather > > > > trival drm_gpuvm [2] patch. > > > > > > > > As for v2, I pushed a branch based on drm-misc-next and > > > > containing > > > > all the dependencies that are not yet available in drm-misc-next > > > > here[3], and another [4] containing extra patches to have things > > > > working on rk3588. The CSF firmware binary can be found here[5], > > > > and > > > > should be placed under > > > > /lib/firmware/arm/mali/arch10.8/mali_csffw.bin. > > > > > > > > The mesa MR adding v10 support on top of panthor is available > > > > here [6]. > > > > > > > > Regarding the GPL2+MIT relicensing, Liviu did an audit and found > > > > two > > > > more people that I didn't spot initially: Clément Péron for the > > > > devfreq > > > > code, and Alexey Sheplyakov for some bits in panthor_gpu.c. Both > > > > are > > > > Cc-ed on the relevant patches. The rest of the code is either > > > > new, or > > > > covered by the Linaro, Arm and Collabora acks. > > > > > > > > And here is a non-exhaustive changelog, check each commit for a > > > > detailed > > > > changelog. > > > > > > > > v3; > > > > - Quite a few changes at the MMU/sched level to make the fix some > > > > race conditions and deadlocks > > > > - Addition of the a sync-only VM_BIND operation (to support > > > > vkQueueSparseBind with zero commands). > > > > > > Hi Boris, > > > > > > Just wanted to point out that vkQueueBindSparse's semantics is > > > rather different > > > from vkQueueSubmit when it comes to synchronization. In short, > > > vkQueueBindSparse does not operate on a particular timeline (aka > > > scheduling > > > queue / drm_sched_entity). The property of following a timeline > > > order is known > > > as “submission order” [1] in Vulkan, and applies to vkQueueSubmit > > > only and not > > > vkQueueBindSparse. > > > > Hm, okay. I really though the same ordering guarantees applied to > > sparse binding queues too, as the spec [1] says > > > > " > > Batches begin execution in the order they appear in pBindInfo, but > > may complete out of order. > > " > > Right. So this is something where the Vulkan spec isn't terribly clear > and I think I need to go file a spec bug. I'm fairly sure that the > intent is that bind operations MAY complete out-of-order but are not > required to complete out-of-order. In other words, I'm fairly sure > that implementations are allowed but not required to insert extra > dependencies that force some amount of ordering. We take advantage of > this in Mesa today to properly implement vkQueueWaitIdle() on sparse > binding queues. Do I get it correctly that, for Mesa's generic vk_common_QueueWaitIdle() implementation to work correctly, we not only need to guarantee in-order submission, but also in-order completion on the queue. I mean, that's no problem for panvk/panthor, because that's how it's implemented anyway, but I didn't realize this constraint existed until you mentioned it. > This is also a requirement of Windows WDDM2 as far as > I can tell so I'd be very surprised if we disallowed it in the spec. > > That said, that's all very unclear and IDK where you'd get that from > the spec. I'm going to go file a spec bug so we can get this sorted > out. > > ~Faith > > > > which means things are submitted in order inside a vkQueueSparseBind > > context, so I was expecting the submission ordering guarantee to > > apply > > across vkQueueSparseBind() calls on the same queue too. Just want to > > mention that all kernel implementations I have seen so far assume > > VM_BIND submissions on a given queue are serialized (that's how > > drm_sched works, and Xe, Nouveau and Panthor are basing their VM_BIND > > implemenation on drm_sched). > > > > Maybe Faith, or anyone deeply involved in the Vulkan specification, > > can > > confirm that submission ordering guarantees are relaxed on sparse > > binding queues. > > > > > > > > Hence, an implementation that takes full advantage of Vulkan > > > semantics would > > > essentially have an infinite amount of VM_BIND contexts. > > > > Uh, that's definitely not how I understood it initially... > > > > > It would also not need > > > sync-only VM_BIND submissions, assuming that drmSyncobjTransfer > > > works. > > > > Sure, if each vkQueueSparseBind() has its own timeline, an internal > > timeline-syncobj with a bunch of drmSyncobjTransfer() calls would do > > the > > trick (might require several ioctls() to merge input syncobjs, but > > that's probably not the end of the world). Back to the kernel-side implementation. As Tatsuyuki pointed out, we can always replace the sync-only VM_BIND (no actual operation on the VM, just a bunch of wait/signal sync operations) with X calls to drmSyncobjTransfer() and a mesa-driver-specific timeline syncobj that's used to consolidate all the operations we have submitted so far. But given Nouveau supports this sync-only operation (at least that's my understanding of the nouveau_uvmm.c code and how VM_BIND is currently used in NVK), I guess there are good reasons to support that natively. Besides the potential optimization on the number of ioctl() calls, and the fact adding support for VM_BIND with zero VM ops is trivial enough that it's probably worth adding to simplify the UMD implementation, is there anything else I'm missing? Regards, Boris