| Message ID | 20230809165330.2451699-1-boris.brezillon@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: Add a driver for FW-based Mali GPUs | expand |
On Wed, Aug 9, 2023 at 10:53 AM Boris Brezillon <boris.brezillon@collabora.com> wrote: > > Hello, > > This is the second version of the kernel driver meant to support new Mali > GPUs which are delegating the scheduling to a firmware. > > The RFC has been dropped as the major blocking points have been addressed > (request to use drm_sched, request to implement a VM_BIND-like ioctl, > request to use drm_gpuva_mgr for the VM logic, lack of PM/devfreq support). > > This series is based on drm-misc-next and depends on some drm_sched [1] > and iommu [2] changes. > > A branch containing all those dependencies is available here[3], and > here [4] is another one containing all the patches needed to have > a working GPU on rk3588 on top. The CSF firmware binary can be found > here[5]. > > The mesa branch used to test this new driver is available here [6]. > It's still under development and it's just a gallium driver right now, > but we are working on that ;-). > > Here is a non-exaustive changelog, check each commit for a detailed > changelog. > > 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 > > I tried to Cc anyone that was involved in any development of the code > I picked from panfrost, so they can acknowledge the GPL2 -> MIT+GPL2 > change. If I missed someone, please let me know. Panfrost was largely based on etnaviv, vc4, v3d, and msm. Those are all GPL2 (or 2+) only. How is relicensing that code okay? Also, panfrost depends on drm_gem_shmem_helper.c (at least) which is GPL2. Does that get re-implemented in a MIT licensed environment? Maybe some drivers are enough of a silo to get away with MIT licensing, but I wouldn't be comfortable claiming it. Rob
Hello Rob, On Wed, 9 Aug 2023 14:22:59 -0600 Rob Herring <robh@kernel.org> wrote: > On Wed, Aug 9, 2023 at 10:53 AM Boris Brezillon > <boris.brezillon@collabora.com> wrote: > > > > I tried to Cc anyone that was involved in any development of the code > > I picked from panfrost, so they can acknowledge the GPL2 -> MIT+GPL2 > > change. If I missed someone, please let me know. > > Panfrost was largely based on etnaviv, vc4, v3d, and msm. Those are > all GPL2 (or 2+) only. Uh, I must have missed some copyright headers then. Note that not all panfrost files were taken as a base for panthor: - Makefile/Kconfig. I honestly hope there's nothing copyright-able in there, given there's no other way to define your driver and compilation rules. - panthor_device.{c,h} copied from panfrost_device.{c,h} with quite a few modifications in the process. This one has your copyright, and Marty's one. - a tiny part of panthor_drv.c was copied from panfrost_drv.c, but let's be honest, the part that was copied (ioctl wrappers, mostly), can't really be done differently. This one has your copyright, Marty's one, and Collabora's one. - panthor_regs.h copied from panfrost_regs.h. This one has your copyright, Marty's one and Arm's one (definitions extracted from kbase). But again, I'm not even sure register definitions are copyright-able, given there's no other way to define them. If that makes a difference, I changed the prefix, and dropped definition that do not exist on CSF HW. - panthor_gpu.{c,h} copied from panfrost_gpu.{c,h}. These files have your copyright, Marty's one, and Collabora's one. - panthor_{gem,mmu}.{c,h} copied from panfrost_{gem,mmu}.{c,h}. Those ones have your copyright only. - panthor_devfreq.{c,h} copied from panfrost_devfreq.{c,h}. Collabora's copyright only. - panthor_{heap,fw,sched}.{c,h}. Those are brand new files, that were written from scratch. I also git-blamed the lines I copies to Cc any contributors to the above files. I might have omitted someone, but I did my best to try and spot people that have a word in this decision. > How is relicensing that code okay? Sorry, the copyright headers of the files I copied didn't mention that :-/. If that's an omission, it would be good to have the headers updated to reflect the actual chain of copyrights. > Also, > panfrost depends on drm_gem_shmem_helper.c (at least) which is GPL2. > Does that get re-implemented in a MIT licensed environment? Not only drm_gem_shmem, but drm_gpuva_mgr and drm_sched too. And yes, any helper function/lib that's not GPL+MIT will have to be re-implemented or replaced by something else. > > Maybe some drivers are enough of a silo to get away with MIT > licensing, but I wouldn't be comfortable claiming it. Well, yes, re-using the code as-is is almost impossible, unless someone rewrites the various GPL components we depend on. But if someone wants to pick, say, the scheduling logic, and replace drm_sched by something else, they can. Not saying it's worth it, just saying it's possible. Regards, Boris
On Thu, Aug 10, 2023 at 10:44 AM Boris Brezillon <boris.brezillon@collabora.com> wrote: > > Hello Rob, > > On Wed, 9 Aug 2023 14:22:59 -0600 > Rob Herring <robh@kernel.org> wrote: > > > On Wed, Aug 9, 2023 at 10:53 AM Boris Brezillon > > <boris.brezillon@collabora.com> wrote: > > > > > > I tried to Cc anyone that was involved in any development of the code > > > I picked from panfrost, so they can acknowledge the GPL2 -> MIT+GPL2 > > > change. If I missed someone, please let me know. > > > > Panfrost was largely based on etnaviv, vc4, v3d, and msm. Those are > > all GPL2 (or 2+) only. > > Uh, I must have missed some copyright headers then. Note that not all > panfrost files were taken as a base for panthor: > > - Makefile/Kconfig. I honestly hope there's nothing copyright-able in > there, given there's no other way to define your driver and > compilation rules. > - panthor_device.{c,h} copied from panfrost_device.{c,h} with quite a > few modifications in the process. This one has your copyright, and > Marty's one. > - a tiny part of panthor_drv.c was copied from panfrost_drv.c, but let's > be honest, the part that was copied (ioctl wrappers, mostly), can't > really be done differently. This one has your copyright, Marty's one, > and Collabora's one. > - panthor_regs.h copied from panfrost_regs.h. This one has your > copyright, Marty's one and Arm's one (definitions extracted from > kbase). But again, I'm not even sure register definitions are > copyright-able, given there's no other way to define them. If that > makes a difference, I changed the prefix, and dropped definition that > do not exist on CSF HW. > - panthor_gpu.{c,h} copied from panfrost_gpu.{c,h}. These files have > your copyright, Marty's one, and Collabora's one. > - panthor_{gem,mmu}.{c,h} copied from panfrost_{gem,mmu}.{c,h}. Those > ones have your copyright only. > - panthor_devfreq.{c,h} copied from panfrost_devfreq.{c,h}. Collabora's > copyright only. > - panthor_{heap,fw,sched}.{c,h}. Those are brand new files, that were > written from scratch. > > I also git-blamed the lines I copies to Cc any contributors to the > above files. I might have omitted someone, but I did my best to > try and spot people that have a word in this decision. > > > How is relicensing that code okay? > > Sorry, the copyright headers of the files I copied didn't mention that > :-/. If that's an omission, it would be good to have the headers updated > to reflect the actual chain of copyrights. Yes, we probably should make it more explicit though at this point it would be fairly vague in terms of the exact sources. IMO, it should be assumed by default any driver is derived work. No one writes a new driver from scratch (unless they are really actively trying to avoid being derivative work). Then the question is what driver(s) were the source. I think it is safe to say no one copies the big 3 (Intel, AMD, NVIDIA) nor para-virt drivers as those are the MIT licensed ones. The ones left are pretty much *all* GPL. > > Also, > > panfrost depends on drm_gem_shmem_helper.c (at least) which is GPL2. > > Does that get re-implemented in a MIT licensed environment? > > Not only drm_gem_shmem, but drm_gpuva_mgr and drm_sched too. And yes, > any helper function/lib that's not GPL+MIT will have to be > re-implemented or replaced by something else. > > > > > Maybe some drivers are enough of a silo to get away with MIT > > licensing, but I wouldn't be comfortable claiming it. > > Well, yes, re-using the code as-is is almost impossible, unless > someone rewrites the various GPL components we depend on. But if > someone wants to pick, say, the scheduling logic, and replace drm_sched > by something else, they can. Not saying it's worth it, just saying it's > possible. Sure, it is possible. Seems like reimplementing all that would be more work than the driver. Maybe the BSDs already have? Rob
On 09/08/2023 17:53, Boris Brezillon wrote: > Hello, > > This is the second version of the kernel driver meant to support new Mali > GPUs which are delegating the scheduling to a firmware. [...] > I tried to Cc anyone that was involved in any development of the code > I picked from panfrost, so they can acknowledge the GPL2 -> MIT+GPL2 > change. If I missed someone, please let me know. Regarding the relicensing, Arm agrees to the relicensing of the parts they hold copyright on. Acked-by: Steven Price <steven.price@arm.com> Steve
Hello, This is the second version of the kernel driver meant to support new Mali GPUs which are delegating the scheduling to a firmware. The RFC has been dropped as the major blocking points have been addressed (request to use drm_sched, request to implement a VM_BIND-like ioctl, request to use drm_gpuva_mgr for the VM logic, lack of PM/devfreq support). This series is based on drm-misc-next and depends on some drm_sched [1] and iommu [2] changes. A branch containing all those dependencies is available here[3], and here [4] is another one containing all the patches needed to have a working GPU on rk3588 on top. The CSF firmware binary can be found here[5]. The mesa branch used to test this new driver is available here [6]. It's still under development and it's just a gallium driver right now, but we are working on that ;-). Here is a non-exaustive changelog, check each commit for a detailed changelog. 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 I tried to Cc anyone that was involved in any development of the code I picked from panfrost, so they can acknowledge the GPL2 -> MIT+GPL2 change. If I missed someone, please let me know. Best Regards, Boris [1]https://lore.kernel.org/dri-devel/20230801205103.627779-1-matthew.brost@intel.com/T/#t [2]https://lore.kernel.org/linux-iommu/20230809121744.2341454-1-boris.brezillon@collabora.com/T/#t [3]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor [4]https://gitlab.freedesktop.org/panfrost/linux/-/tree/panthor+rk3588-evb1 [5]https://gitlab.com/firefly-linux/external/libmali/-/raw/firefly/firmware/g610/mali_csffw.bin [6]https://gitlab.freedesktop.org/panfrost/mesa/-/tree/v10+panthor Boris Brezillon (14): drm/shmem-helper: Make pages_use_count an atomic_t 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 initial bindings for panthor driver .../bindings/gpu/arm,mali-valhall-csf.yaml | 148 + Documentation/gpu/driver-uapi.rst | 5 + MAINTAINERS | 8 + drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_gem_shmem_helper.c | 28 +- drivers/gpu/drm/lima/lima_gem.c | 2 +- drivers/gpu/drm/panfrost/panfrost_mmu.c | 2 +- drivers/gpu/drm/panthor/Kconfig | 16 + drivers/gpu/drm/panthor/Makefile | 15 + drivers/gpu/drm/panthor/panthor_devfreq.c | 281 ++ drivers/gpu/drm/panthor/panthor_devfreq.h | 25 + drivers/gpu/drm/panthor/panthor_device.c | 479 +++ drivers/gpu/drm/panthor/panthor_device.h | 354 ++ drivers/gpu/drm/panthor/panthor_drv.c | 1540 ++++++++ drivers/gpu/drm/panthor/panthor_fw.c | 1417 +++++++ drivers/gpu/drm/panthor/panthor_fw.h | 505 +++ drivers/gpu/drm/panthor/panthor_gem.c | 229 ++ drivers/gpu/drm/panthor/panthor_gem.h | 96 + drivers/gpu/drm/panthor/panthor_gpu.c | 463 +++ drivers/gpu/drm/panthor/panthor_gpu.h | 52 + drivers/gpu/drm/panthor/panthor_heap.c | 550 +++ drivers/gpu/drm/panthor/panthor_heap.h | 36 + drivers/gpu/drm/panthor/panthor_mmu.c | 2611 +++++++++++++ drivers/gpu/drm/panthor/panthor_mmu.h | 81 + drivers/gpu/drm/panthor/panthor_regs.h | 229 ++ drivers/gpu/drm/panthor/panthor_sched.c | 3272 +++++++++++++++++ drivers/gpu/drm/panthor/panthor_sched.h | 50 + include/drm/drm_gem_shmem_helper.h | 2 +- include/uapi/drm/panthor_drm.h | 862 +++++ 30 files changed, 13345 insertions(+), 16 deletions(-) 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