mbox series

[v3,00/17] Imagination Technologies PowerVR DRM driver

Message ID 20230613144800.52657-1-sarah.walker@imgtec.com (mailing list archive)
Headers show
Series Imagination Technologies PowerVR DRM driver | expand

Message

Sarah Walker June 13, 2023, 2:47 p.m. UTC
This patch series adds the initial DRM driver for Imagination Technologies PowerVR
GPUs, starting with those based on our Rogue architecture. It's worth pointing
out that this is a new driver, written from the ground up, rather than a
refactored version of our existing downstream driver (pvrsrvkm).

This new DRM driver supports:
- GEM shmem allocations
- dma-buf / PRIME
- Per-context userspace managed virtual address space
- DRM sync objects (binary and timeline)
- Power management suspend / resume
- GPU job submission (geometry, fragment, compute, transfer)
- META firmware processor
- MIPS firmware processor
- GPU hang detection

Currently our main focus is on our GX6250, AXE-1-16M and BXS-4-64 GPUs. Testing
so far has been done using an Acer Chromebook R13 (GX6250 GPU) and a TI SK-AM62
board (AXE-1-16M GPU). Firmware for the GX6250 and AXE-1-16M can be found here:
https://gitlab.freedesktop.org/frankbinns/linux-firmware/-/tree/powervr

A Vulkan driver that works with our downstream kernel driver has already been
merged into Mesa [1][2]. Support for this new DRM driver is being maintained in
a draft merge request [3], with the branch located here:
https://gitlab.freedesktop.org/frankbinns/mesa/-/tree/powervr-winsys

Job stream formats are documented at:
https://gitlab.freedesktop.org/mesa/mesa/-/blob/73fe6db819d951c02ce57eefcbd9b31b85900f33/src/imagination/csbgen/rogue_kmd_stream.xml

The Vulkan driver is progressing towards Vulkan 1.0. We're code complete, and
are working towards passing conformance. The current combination of this kernel
driver with the Mesa Vulkan driver achieves 71.8% conformance.

The code in this patch series, along with some of its history, can also be found here:
https://gitlab.freedesktop.org/frankbinns/powervr/-/tree/powervr-next

This patch series has dependencies on a number of patches not yet merged. They
are listed below :

maple_tree: split up MA_STATE() macro:https://lists.freedesktop.org/archives/dri-devel/2023-June/407927.html
drm: manager to keep track of GPUs VA mappings: https://lists.freedesktop.org/archives/dri-devel/2023-June/407928.html
drm/sched: Convert drm scheduler to use a work queue rather than kthread: https://lists.freedesktop.org/archives/dri-devel/2023-April/398458.html
drm/sched: Move schedule policy to scheduler / entity: https://lists.freedesktop.org/archives/dri-devel/2023-April/398461.html
drm/sched: Add DRM_SCHED_POLICY_SINGLE_ENTITY scheduling policy: https://lists.freedesktop.org/archives/dri-devel/2023-April/398460.html
drm/sched: Start run wq before TDR in drm_sched_start: https://lists.freedesktop.org/archives/dri-devel/2023-April/398462.html
drm/sched: Submit job before starting TDR: https://lists.freedesktop.org/archives/dri-devel/2023-April/398466.html
drm/sched: Add helper to set TDR timeout: https://lists.freedesktop.org/archives/dri-devel/2023-April/398464.html
drm: fix drmm_mutex_init(): https://lists.freedesktop.org/archives/dri-devel/2023-May/404863.html
drm/sched: Make sure we wait for all dependencies in kill_jobs_cb(): https://lists.freedesktop.org/archives/dri-devel/2023-June/408901.html
drm/sched: Call drm_sched_fence_set_parent() from drm_sched_fence_scheduled(): https://lists.freedesktop.org/archives/dri-devel/2023-June/408904.html

[1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15243
[2] https://gitlab.freedesktop.org/mesa/mesa/-/tree/main/src/imagination/vulkan
[3] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15507

v3:
* Use drm_sched for scheduling
* Use GPU VA manager
* Use runtime PM
* Use drm_gem_shmem
* GPU watchdog and device loss handling

v2:
* Redesigned and simplified UAPI based on RFC feedback from XDC 2022
* Support for transfer and partial render jobs
* Support for timeline sync objects

RFC v1: https://lists.freedesktop.org/archives/dri-devel/2022-August/367814.html

RFC v2: https://lists.freedesktop.org/archives/dri-devel/2023-April/400149.html

Matt Coster (1):
  sizes.h: Add entries between 32G and 64T

Sarah Walker (16):
  dt-bindings: gpu: Add Imagination Technologies PowerVR GPU
  drm/imagination/uapi: Add PowerVR driver UAPI
  drm/imagination: Add skeleton PowerVR driver
  drm/imagination: Get GPU resources
  drm/imagination: Add GPU register and FWIF headers
  drm/imagination: Add GPU ID parsing and firmware loading
  drm/imagination: Add GEM and VM related code
  drm/imagination: Implement power management
  drm/imagination: Implement firmware infrastructure and META FW support
  drm/imagination: Implement MIPS firmware processor and MMU support
  drm/imagination: Implement free list and HWRT create and destroy
    ioctls
  drm/imagination: Implement context creation/destruction ioctls
  drm/imagination: Implement job submission and scheduling
  drm/imagination: Add firmware trace to debugfs
  drm/imagination: Add driver documentation
  arm64: dts: ti: k3-am62-main: Add GPU device node [DO NOT MERGE]

 .../devicetree/bindings/gpu/img,powervr.yaml  |   71 +
 Documentation/gpu/drivers.rst                 |    2 +
 Documentation/gpu/imagination/index.rst       |   14 +
 Documentation/gpu/imagination/uapi.rst        |  174 +
 .../gpu/imagination/virtual_memory.rst        |  462 ++
 MAINTAINERS                                   |   10 +
 arch/arm64/boot/dts/ti/k3-am62-main.dtsi      |   13 +
 drivers/gpu/drm/Kconfig                       |    2 +
 drivers/gpu/drm/Makefile                      |    1 +
 drivers/gpu/drm/imagination/Kconfig           |   15 +
 drivers/gpu/drm/imagination/Makefile          |   33 +
 drivers/gpu/drm/imagination/pvr_ccb.c         |  660 ++
 drivers/gpu/drm/imagination/pvr_ccb.h         |   62 +
 drivers/gpu/drm/imagination/pvr_cccb.c        |  230 +
 drivers/gpu/drm/imagination/pvr_cccb.h        |  102 +
 drivers/gpu/drm/imagination/pvr_context.c     |  450 ++
 drivers/gpu/drm/imagination/pvr_context.h     |  190 +
 drivers/gpu/drm/imagination/pvr_debugfs.c     |   53 +
 drivers/gpu/drm/imagination/pvr_debugfs.h     |   29 +
 drivers/gpu/drm/imagination/pvr_device.c      |  761 ++
 drivers/gpu/drm/imagination/pvr_device.h      |  795 +++
 drivers/gpu/drm/imagination/pvr_device_info.c |  223 +
 drivers/gpu/drm/imagination/pvr_device_info.h |  133 +
 drivers/gpu/drm/imagination/pvr_drv.c         | 1520 ++++
 drivers/gpu/drm/imagination/pvr_drv.h         |  130 +
 drivers/gpu/drm/imagination/pvr_free_list.c   |  577 ++
 drivers/gpu/drm/imagination/pvr_free_list.h   |  185 +
 drivers/gpu/drm/imagination/pvr_fw.c          | 1388 ++++
 drivers/gpu/drm/imagination/pvr_fw.h          |  425 ++
 drivers/gpu/drm/imagination/pvr_fw_info.h     |  115 +
 drivers/gpu/drm/imagination/pvr_fw_meta.c     |  610 ++
 drivers/gpu/drm/imagination/pvr_fw_meta.h     |   14 +
 drivers/gpu/drm/imagination/pvr_fw_mips.c     |  280 +
 drivers/gpu/drm/imagination/pvr_fw_mips.h     |   38 +
 .../gpu/drm/imagination/pvr_fw_startstop.c    |  280 +
 .../gpu/drm/imagination/pvr_fw_startstop.h    |   13 +
 drivers/gpu/drm/imagination/pvr_fw_trace.c    |  490 ++
 drivers/gpu/drm/imagination/pvr_fw_trace.h    |   78 +
 drivers/gpu/drm/imagination/pvr_gem.c         |  397 ++
 drivers/gpu/drm/imagination/pvr_gem.h         |  201 +
 drivers/gpu/drm/imagination/pvr_hwrt.c        |  559 ++
 drivers/gpu/drm/imagination/pvr_hwrt.h        |  163 +
 drivers/gpu/drm/imagination/pvr_job.c         |  834 +++
 drivers/gpu/drm/imagination/pvr_job.h         |  147 +
 drivers/gpu/drm/imagination/pvr_params.c      |  147 +
 drivers/gpu/drm/imagination/pvr_params.h      |   72 +
 drivers/gpu/drm/imagination/pvr_power.c       |  332 +
 drivers/gpu/drm/imagination/pvr_power.h       |   41 +
 drivers/gpu/drm/imagination/pvr_queue.c       | 1203 ++++
 drivers/gpu/drm/imagination/pvr_queue.h       |  157 +
 .../gpu/drm/imagination/pvr_rogue_cr_defs.h   | 6193 +++++++++++++++++
 .../imagination/pvr_rogue_cr_defs_client.h    |  159 +
 drivers/gpu/drm/imagination/pvr_rogue_defs.h  |  179 +
 drivers/gpu/drm/imagination/pvr_rogue_fwif.h  | 2208 ++++++
 .../drm/imagination/pvr_rogue_fwif_check.h    |  491 ++
 .../drm/imagination/pvr_rogue_fwif_client.h   |  369 +
 .../imagination/pvr_rogue_fwif_client_check.h |  133 +
 .../drm/imagination/pvr_rogue_fwif_common.h   |   60 +
 .../pvr_rogue_fwif_resetframework.h           |   28 +
 .../gpu/drm/imagination/pvr_rogue_fwif_sf.h   | 1648 +++++
 .../drm/imagination/pvr_rogue_fwif_shared.h   |  258 +
 .../imagination/pvr_rogue_fwif_shared_check.h |  108 +
 .../drm/imagination/pvr_rogue_fwif_stream.h   |   78 +
 .../drm/imagination/pvr_rogue_heap_config.h   |  113 +
 drivers/gpu/drm/imagination/pvr_rogue_meta.h  |  356 +
 drivers/gpu/drm/imagination/pvr_rogue_mips.h  |  335 +
 .../drm/imagination/pvr_rogue_mips_check.h    |   58 +
 .../gpu/drm/imagination/pvr_rogue_mmu_defs.h  |  136 +
 drivers/gpu/drm/imagination/pvr_stream.c      |  309 +
 drivers/gpu/drm/imagination/pvr_stream.h      |   75 +
 drivers/gpu/drm/imagination/pvr_stream_defs.c |  351 +
 drivers/gpu/drm/imagination/pvr_stream_defs.h |   16 +
 drivers/gpu/drm/imagination/pvr_vm.c          | 3417 +++++++++
 drivers/gpu/drm/imagination/pvr_vm.h          |   99 +
 drivers/gpu/drm/imagination/pvr_vm_mips.c     |  222 +
 drivers/gpu/drm/imagination/pvr_vm_mips.h     |   22 +
 include/linux/sizes.h                         |    9 +
 include/uapi/drm/pvr_drm.h                    | 1333 ++++
 78 files changed, 33644 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/img,powervr.yaml
 create mode 100644 Documentation/gpu/imagination/index.rst
 create mode 100644 Documentation/gpu/imagination/uapi.rst
 create mode 100644 Documentation/gpu/imagination/virtual_memory.rst
 create mode 100644 drivers/gpu/drm/imagination/Kconfig
 create mode 100644 drivers/gpu/drm/imagination/Makefile
 create mode 100644 drivers/gpu/drm/imagination/pvr_ccb.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_ccb.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_cccb.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_cccb.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_context.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_context.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_debugfs.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_debugfs.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_device.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_device.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_device_info.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_device_info.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_drv.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_drv.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_free_list.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_free_list.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw_info.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw_meta.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw_meta.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw_mips.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw_mips.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw_startstop.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw_startstop.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw_trace.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_fw_trace.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_gem.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_gem.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_hwrt.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_hwrt.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_job.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_job.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_params.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_params.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_power.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_power.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_queue.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_queue.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_cr_defs.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_cr_defs_client.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_defs.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_fwif.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_fwif_check.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_fwif_client.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_fwif_client_check.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_fwif_common.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_fwif_resetframework.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_fwif_sf.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_fwif_shared.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_fwif_shared_check.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_fwif_stream.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_heap_config.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_meta.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_mips.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_mips_check.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_rogue_mmu_defs.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_stream.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_stream.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_stream_defs.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_stream_defs.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_vm.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_vm.h
 create mode 100644 drivers/gpu/drm/imagination/pvr_vm_mips.c
 create mode 100644 drivers/gpu/drm/imagination/pvr_vm_mips.h
 create mode 100644 include/uapi/drm/pvr_drm.h

Comments

Krzysztof Kozlowski June 13, 2023, 6:26 p.m. UTC | #1
On 13/06/2023 16:47, Sarah Walker wrote:
> This patch series adds the initial DRM driver for Imagination Technologies PowerVR
> GPUs, starting with those based on our Rogue architecture. It's worth pointing
> out that this is a new driver, written from the ground up, rather than a
> refactored version of our existing downstream driver (pvrsrvkm).
> 

...

> 
> maple_tree: split up MA_STATE() macro:https://lists.freedesktop.org/archives/dri-devel/2023-June/407927.html
> drm: manager to keep track of GPUs VA mappings: https://lists.freedesktop.org/archives/dri-devel/2023-June/407928.html
> drm/sched: Convert drm scheduler to use a work queue rather than kthread: https://lists.freedesktop.org/archives/dri-devel/2023-April/398458.html
> drm/sched: Move schedule policy to scheduler / entity: https://lists.freedesktop.org/archives/dri-devel/2023-April/398461.html
> drm/sched: Add DRM_SCHED_POLICY_SINGLE_ENTITY scheduling policy: https://lists.freedesktop.org/archives/dri-devel/2023-April/398460.html
> drm/sched: Start run wq before TDR in drm_sched_start: https://lists.freedesktop.org/archives/dri-devel/2023-April/398462.html
> drm/sched: Submit job before starting TDR: https://lists.freedesktop.org/archives/dri-devel/2023-April/398466.html
> drm/sched: Add helper to set TDR timeout: https://lists.freedesktop.org/archives/dri-devel/2023-April/398464.html
> drm: fix drmm_mutex_init(): https://lists.freedesktop.org/archives/dri-devel/2023-May/404863.html
> drm/sched: Make sure we wait for all dependencies in kill_jobs_cb(): https://lists.freedesktop.org/archives/dri-devel/2023-June/408901.html
> drm/sched: Call drm_sched_fence_set_parent() from drm_sched_fence_scheduled(): https://lists.freedesktop.org/archives/dri-devel/2023-June/408904.html
> 
> [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15243
> [2] https://gitlab.freedesktop.org/mesa/mesa/-/tree/main/src/imagination/vulkan
> [3] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15507
> 
> v3:
> * Use drm_sched for scheduling
> * Use GPU VA manager
> * Use runtime PM
> * Use drm_gem_shmem
> * GPU watchdog and device loss handling

No changes in the bindings? So you decided to just ignore the comments?

Sorry, that's not a good approach. Implement all the comments or respond
to them.

Best regards,
Krzysztof
Linus Walleij June 16, 2023, 12:29 p.m. UTC | #2
Hi Sarah,

thanks for starting this long awaited work!

On Tue, Jun 13, 2023 at 5:20 PM Sarah Walker <sarah.walker@imgtec.com> wrote:

> This patch series adds the initial DRM driver for Imagination Technologies PowerVR
> GPUs, starting with those based on our Rogue architecture. It's worth pointing
> out that this is a new driver, written from the ground up, rather than a
> refactored version of our existing downstream driver (pvrsrvkm).

This seems to be a fairly good starting point, a bit of trade-off
between latest-and-greatest
and recent enough devices that need aftermarket support.

I assume you are aware of the community existing around Series 5
(should be the immediate
predecessor to Rogue?):
https://github.com/openpvrsgx-devgroup/linux_openpvrsgx

I don't know how active those people are these days, but I can see that a branch
was updated for v6.4-rc3 just three weeks ago.
https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/tree/pvrsrvkm-6.4-rc3

I think it would be good for community building to make sure that you get these
people involved in reviewing, especially neutral stuff like device tree bindings
but also to make sure no architectural choices are done that will make it hard
to retrofit a proper driver for the older engines if this community
decide to work
on it.

Specifically I would ask that the DT bindings include all old and new PowerVR
hardware in one go, unless they have very specific hardware definition needs,
which I doubt.

Also I think they could use your help to get the proper firmware for the older
hardware licensed properly from Imagination and included into linux-firmware
so they do not need to ship files on the side.

Yours,
Linus Walleij
H. Nikolaus Schaller June 16, 2023, 2:06 p.m. UTC | #3
Hi Linus,
thanks for sharing this conversation with me.

> Am 16.06.2023 um 14:29 schrieb Linus Walleij <linus.walleij@linaro.org>:
> 
> Hi Sarah,
> 
> thanks for starting this long awaited work!
> 
> On Tue, Jun 13, 2023 at 5:20 PM Sarah Walker <sarah.walker@imgtec.com> wrote:
> 
>> This patch series adds the initial DRM driver for Imagination Technologies PowerVR
>> GPUs, starting with those based on our Rogue architecture. It's worth pointing
>> out that this is a new driver, written from the ground up, rather than a
>> refactored version of our existing downstream driver (pvrsrvkm).

Great!

> 
> This seems to be a fairly good starting point, a bit of trade-off
> between latest-and-greatest
> and recent enough devices that need aftermarket support.
> 
> I assume you are aware of the community existing around Series 5
> (should be the immediate
> predecessor to Rogue?):
> https://github.com/openpvrsgx-devgroup/linux_openpvrsgx
> 
> I don't know how active those people are these days, but I can see that a branch

Well, there hasn't been much progress due to lack of documentation and compatibility
issues of user-space code. Just keeping the code compatible to latest upstream kernels.

But at least for OMAP3 and OMAP5 processors people are actively using this work.

There is even a gaming console (www.pyra-handheld.com) in active production
with a strong need for an up-to-date SGX544 driver running on OMAP5.

> was updated for v6.4-rc3 just three weeks ago.
> https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/tree/pvrsrvkm-6.4-rc3
> 
> I think it would be good for community building to make sure that you get these
> people involved in reviewing, especially neutral stuff like device tree bindings
> but also to make sure no architectural choices are done that will make it hard
> to retrofit a proper driver for the older engines if this community
> decide to work
> on it.

Some questions to the author of the new driver:
- are there plans to support SGX5 (the predecessor of Rogue6)?
- will this be able to run the existing firmware and user-space code of pvrsrvkm?
- or will it have new firmware and user-space code for these older chips?
- or will there be open user-space code for SGX (and Rogue)?

> 
> Specifically I would ask that the DT bindings include all old and new PowerVR
> hardware in one go, unless they have very specific hardware definition needs,
> which I doubt.

Our current bindings for all SoC with a SGX5 GPU inside and which have at least
some official Linux support are here:

https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/blob/letux/omap-pvr-soc-glue-v8/Documentation/devicetree/bindings/gpu/img%2Cpvrsgx.yaml

There was an attempt to upstream at least this plus glue code (i.e. device tree
sources) some years ago but there was no consensus about the number and names of
clocks that should be included in such a bindings document.

> 
> Also I think they could use your help to get the proper firmware for the older
> hardware licensed properly from Imagination and included into linux-firmware
> so they do not need to ship files on the side.

Indeed, this and some "universal" user-space code would help a lot. Currently
we have collected a lot of binaries for several architectures (e.g. Intel, OMAP, JZ4780),
but all from different DDK versions and very different assumptions about system
library versions.

> 
> Yours,
> Linus Walleij

Best regards,
Nikolaus Schaller
H. Nikolaus Schaller June 16, 2023, 2:08 p.m. UTC | #4
Hi Linus,
thanks for sharing this conversation with me.

> Am 16.06.2023 um 14:29 schrieb Linus Walleij <linus.walleij@linaro.org>:
> 
> Hi Sarah,
> 
> thanks for starting this long awaited work!
> 
> On Tue, Jun 13, 2023 at 5:20 PM Sarah Walker <sarah.walker@imgtec.com> wrote:
> 
>> This patch series adds the initial DRM driver for Imagination Technologies PowerVR
>> GPUs, starting with those based on our Rogue architecture. It's worth pointing
>> out that this is a new driver, written from the ground up, rather than a
>> refactored version of our existing downstream driver (pvrsrvkm).

Great!

> 
> This seems to be a fairly good starting point, a bit of trade-off
> between latest-and-greatest
> and recent enough devices that need aftermarket support.
> 
> I assume you are aware of the community existing around Series 5
> (should be the immediate
> predecessor to Rogue?):
> https://github.com/openpvrsgx-devgroup/linux_openpvrsgx
> 
> I don't know how active those people are these days, but I can see that a branch

Well, there hasn't been much progress due to lack of documentation and compatibility
issues of user-space code. Just keeping the code compatible to latest upstream kernels.

But at least for OMAP3 and OMAP5 processors people are actively using this work.

There is even a gaming console (www.pyra-handheld.com) in active production
with a strong need for an up-to-date SGX544 driver running on OMAP5.

> was updated for v6.4-rc3 just three weeks ago.
> https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/tree/pvrsrvkm-6.4-rc3
> 
> I think it would be good for community building to make sure that you get these
> people involved in reviewing, especially neutral stuff like device tree bindings
> but also to make sure no architectural choices are done that will make it hard
> to retrofit a proper driver for the older engines if this community
> decide to work
> on it.

Some questions to the author of the new driver:
- are there plans to support SGX5 (the predecessor of Rogue6)?
- will this be able to run the existing firmware and user-space code of pvrsrvkm?
- or will it have new firmware and user-space code for these older chips?
- or will there be open user-space code for SGX (and Rogue)?

> 
> Specifically I would ask that the DT bindings include all old and new PowerVR
> hardware in one go, unless they have very specific hardware definition needs,
> which I doubt.

Our current bindings for all SoC with a SGX5 GPU inside and which have at least
some official Linux support are here:

https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/blob/letux/omap-pvr-soc-glue-v8/Documentation/devicetree/bindings/gpu/img%2Cpvrsgx.yaml

There was an attempt to upstream at least this plus glue code (i.e. device tree
sources) some years ago but there was no consensus about the number and names of
clocks that should be included in such a bindings document.

> 
> Also I think they could use your help to get the proper firmware for the older
> hardware licensed properly from Imagination and included into linux-firmware
> so they do not need to ship files on the side.

Indeed, this and some "universal" user-space code would help a lot. Currently
we have collected a lot of binaries for several architectures (e.g. Intel, OMAP, JZ4780),
but all from different DDK versions and very different assumptions about system
library versions.

> 
> Yours,
> Linus Walleij

Best regards,
Nikolaus Schaller
Frank Binns June 26, 2023, 1:31 p.m. UTC | #5
Hi Linus,

On Fri, 2023-06-16 at 14:29 +0200, Linus Walleij wrote:
> Hi Sarah,
> 
> thanks for starting this long awaited work!
> 
> On Tue, Jun 13, 2023 at 5:20 PM Sarah Walker <sarah.walker@imgtec.com> wrote:
> 
> > This patch series adds the initial DRM driver for Imagination Technologies PowerVR
> > GPUs, starting with those based on our Rogue architecture. It's worth pointing
> > out that this is a new driver, written from the ground up, rather than a
> > refactored version of our existing downstream driver (pvrsrvkm).
> 
> This seems to be a fairly good starting point, a bit of trade-off
> between latest-and-greatest
> and recent enough devices that need aftermarket support.
> 
> I assume you are aware of the community existing around Series 5
> (should be the immediate
> predecessor to Rogue?):
> https://github.com/openpvrsgx-devgroup/linux_openpvrsgx

Actually we were unaware of this community, so thank you for pointing it out.

> 
> I don't know how active those people are these days, but I can see that a branch
> was updated for v6.4-rc3 just three weeks ago.
> https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/tree/pvrsrvkm-6.4-rc3
> 
> I think it would be good for community building to make sure that you get these
> people involved in reviewing, especially neutral stuff like device tree bindings
> but also to make sure no architectural choices are done that will make it hard
> to retrofit a proper driver for the older engines if this community
> decide to work
> on it.

On the face of it, I'd imagine that it will make more sense for SGX to have its
own driver, just because it's different enough to require a different
design. For example, Series6 onwards uses a completely different firmware to
SGX/Series5. Another possible approach might be to share code between this
driver and a future SGX driver by extracting code out into a library. Of course,
we won't know what code to extract, if any, until someone starts working on
upstream SGX support.

> 
> Specifically I would ask that the DT bindings include all old and new PowerVR
> hardware in one go, unless they have very specific hardware definition needs,
> which I doubt.

I'll comment about this on the other thread.

> 
> Also I think they could use your help to get the proper firmware for the older
> hardware licensed properly from Imagination and included into linux-firmware
> so they do not need to ship files on the side.

Sure, we can do this. I've already got approval for the existing SGX firmware to
use the same license as the Rogue firmware, which can be found here:
https://gitlab.freedesktop.org/frankbinns/linux-firmware/-/blob/powervr/LICENSE.powervr

I'll speak to Nikolaus about next steps.

Thanks
Frank

> 
> Yours,
> Linus Walleij
Frank Binns June 26, 2023, 1:45 p.m. UTC | #6
Hi Nikolaus,

On Fri, 2023-06-16 at 16:06 +0200, H. Nikolaus Schaller wrote:
> Hi Linus,
> thanks for sharing this conversation with me.
> 
> > Am 16.06.2023 um 14:29 schrieb Linus Walleij <linus.walleij@linaro.org>:
> > 
> > Hi Sarah,
> > 
> > thanks for starting this long awaited work!
> > 
> > On Tue, Jun 13, 2023 at 5:20 PM Sarah Walker <sarah.walker@imgtec.com> wrote:
> > 
> > > This patch series adds the initial DRM driver for Imagination Technologies PowerVR
> > > GPUs, starting with those based on our Rogue architecture. It's worth pointing
> > > out that this is a new driver, written from the ground up, rather than a
> > > refactored version of our existing downstream driver (pvrsrvkm).
> 
> Great!
> 
> > This seems to be a fairly good starting point, a bit of trade-off
> > between latest-and-greatest
> > and recent enough devices that need aftermarket support.
> > 
> > I assume you are aware of the community existing around Series 5
> > (should be the immediate
> > predecessor to Rogue?):
> > https://github.com/openpvrsgx-devgroup/linux_openpvrsgx
> > 
> > I don't know how active those people are these days, but I can see that a branch
> 
> Well, there hasn't been much progress due to lack of documentation and compatibility
> issues of user-space code. Just keeping the code compatible to latest upstream kernels.
> 
> But at least for OMAP3 and OMAP5 processors people are actively using this work.
> 
> There is even a gaming console (www.pyra-handheld.com) in active production
> with a strong need for an up-to-date SGX544 driver running on OMAP5.
> 
> > was updated for v6.4-rc3 just three weeks ago.
> > https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/tree/pvrsrvkm-6.4-rc3
> > 
> > I think it would be good for community building to make sure that you get these
> > people involved in reviewing, especially neutral stuff like device tree bindings
> > but also to make sure no architectural choices are done that will make it hard
> > to retrofit a proper driver for the older engines if this community
> > decide to work
> > on it.
> 
> Some questions to the author of the new driver:
> - are there plans to support SGX5 (the predecessor of Rogue6)?

We don't currently have any plans to support SGX. Our main focus is currently on
Rogue and then we'll move onto Volcanic.

> - will this be able to run the existing firmware and user-space code of pvrsrvkm?

I'm afraid not. The interface for existing firmware and userspace code were
designed with different requirements in mind and don't cater to the kernel's
strict compatibility guarantees. As such, the uapi for this new driver is
very different to pvrsrvkm's, although naturally there are similarities:
https://gitlab.freedesktop.org/sarah-walker-imgtec/powervr/-/blob/dev/v3/include/uapi/drm/pvr_drm.h

We've also worked with our firmware team to make changes to the firmware
interface to better support this new driver. Specifically, parts of the firmware
interface are no longer conditional on the GPU feature set / hardware
workarounds, meaning we now have a single interface which can work across all
existing Rogue GPUs, which makes things a lot easier for this new kernel driver.

> - or will it have new firmware and user-space code for these older chips?
> - or will there be open user-space code for SGX (and Rogue)?

We're using the same Rogue firmware as our closed source driver, just with
modifications to the interface to cater for this new kernel driver. In terms of
userspace, we already have a Vulkan driver upstreamed to Mesa:
https://gitlab.freedesktop.org/mesa/mesa/-/tree/main/src/imagination/vulkan

and will be working to enable GLES support via Mesa's Zink GL(ES)-to-Vulkan
translation layer. This naturally limits support to Series 6 onwards, as
anything older isn't capable of supporting Vulkan.

> 
> > Specifically I would ask that the DT bindings include all old and new PowerVR
> > hardware in one go, unless they have very specific hardware definition needs,
> > which I doubt.
> 
> Our current bindings for all SoC with a SGX5 GPU inside and which have at least
> some official Linux support are here:
> 
> https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/blob/letux/omap-pvr-soc-glue-v8/Documentation/devicetree/bindings/gpu/img%2Cpvrsgx.yaml
> 
> There was an attempt to upstream at least this plus glue code (i.e. device tree
> sources) some years ago but there was no consensus about the number and names of
> clocks that should be included in such a bindings document.

I've taken a look and your bindings look very similar to the ones we've come up
with. If you decide to attempt to upstream these again, please feel free to CC
me, Sarah and Donald (all on this email chain) so we can provide some feedback.

> 
> > Also I think they could use your help to get the proper firmware for the older
> > hardware licensed properly from Imagination and included into linux-firmware
> > so they do not need to ship files on the side.
> 
> Indeed, this and some "universal" user-space code would help a lot. Currently
> we have collected a lot of binaries for several architectures (e.g. Intel, OMAP, JZ4780),
> but all from different DDK versions and very different assumptions about system
> library versions.

The way the SGX driver was designed means that it has to be built for a specific
GPU, the version of the firmware, userspace driver(s) & kernel driver have to
exactly match and the build configuration has to match as well. Essentially, we
don't have "universal" userspace code ourselves. With our focus being on Rogue
and beyond, we don't currently have any plans to work on this.

Thanks
Frank

> 
> > Yours,
> > Linus Walleij
> 
> Best regards,
> Nikolaus Schaller
>
H. Nikolaus Schaller June 26, 2023, 6:48 p.m. UTC | #7
Hi Frank,
I have added Merlijn who is doing a lot with PVRSGX for Maemo-Leste and the
phone-devel list since most SoC we find using a PVRSGX are smartphone processors.

> Am 26.06.2023 um 15:45 schrieb Frank Binns <Frank.Binns@imgtec.com>:
> 
> Hi Nikolaus,
> 
>> 
>> Some questions to the author of the new driver:
>> - are there plans to support SGX5 (the predecessor of Rogue6)?
> 
> We don't currently have any plans to support SGX. Our main focus is currently on
> Rogue and then we'll move onto Volcanic.

Okay, that's completely understandable from a commercial perspective.

> 
>> - will this be able to run the existing firmware and user-space code of pvrsrvkm?
> 
> I'm afraid not. The interface for existing firmware and userspace code were
> designed with different requirements in mind and don't cater to the kernel's
> strict compatibility guarantees. As such, the uapi for this new driver is
> very different to pvrsrvkm's, although naturally there are similarities:
> https://gitlab.freedesktop.org/sarah-walker-imgtec/powervr/-/blob/dev/v3/include/uapi/drm/pvr_drm.h

This makes sense. So the new Rogue/Volcanic and the older PVRSGX drivers should
be able to coexist (at least in source code as there is no hardware having both).

> We've also worked with our firmware team to make changes to the firmware
> interface to better support this new driver. Specifically, parts of the firmware
> interface are no longer conditional on the GPU feature set / hardware
> workarounds, meaning we now have a single interface which can work across all
> existing Rogue GPUs, which makes things a lot easier for this new kernel driver.

That is what I have dreamed of for SGX as well.

We could have replaced all the #if for specific versions and errata by some code
to runtime check with the device tree for the specific SGX version.

But this was never done because it is complex, difficult to automate and our means
for testing things are limited. And we could not decide which DDK version we
should build on as there is no common denominator for all SoC.

> 
>> - or will it have new firmware and user-space code for these older chips?
>> - or will there be open user-space code for SGX (and Rogue)?
> 
> We're using the same Rogue firmware as our closed source driver, just with
> modifications to the interface to cater for this new kernel driver. In terms of

Ok. Well, it was to be expected that SGX and Rogue firmware are quite different.

> userspace, we already have a Vulkan driver upstreamed to Mesa:
> https://gitlab.freedesktop.org/mesa/mesa/-/tree/main/src/imagination/vulkan

Nice!

> 
> and will be working to enable GLES support via Mesa's Zink GL(ES)-to-Vulkan
> translation layer. This naturally limits support to Series 6 onwards, as
> anything older isn't capable of supporting Vulkan.

I see.

> 
>> 
>>> Specifically I would ask that the DT bindings include all old and new PowerVR
>>> hardware in one go, unless they have very specific hardware definition needs,
>>> which I doubt.
>> 
>> Our current bindings for all SoC with a SGX5 GPU inside and which have at least
>> some official Linux support are here:
>> 
>> https://github.com/openpvrsgx-devgroup/linux_openpvrsgx/blob/letux/omap-pvr-soc-glue-v8/Documentation/devicetree/bindings/gpu/img%2Cpvrsgx.yaml
>> 
>> There was an attempt to upstream at least this plus glue code (i.e. device tree
>> sources) some years ago but there was no consensus about the number and names of
>> clocks that should be included in such a bindings document.
> 
> I've taken a look and your bindings look very similar to the ones we've come up
> with. If you decide to attempt to upstream these again, please feel free to CC
> me, Sarah and Donald (all on this email chain) so we can provide some feedback.

That is good!

It would be a good moment to give it another try as we can have even more
reviewers than before...

> 
>> 
>>> Also I think they could use your help to get the proper firmware for the older
>>> hardware licensed properly from Imagination and included into linux-firmware
>>> so they do not need to ship files on the side.
>> 
>> Indeed, this and some "universal" user-space code would help a lot. Currently
>> we have collected a lot of binaries for several architectures (e.g. Intel, OMAP, JZ4780),
>> but all from different DDK versions and very different assumptions about system
>> library versions.
> 
> The way the SGX driver was designed means that it has to be built for a specific
> GPU, the version of the firmware, userspace driver(s) & kernel driver have to
> exactly match and the build configuration has to match as well. Essentially, we
> don't have "universal" userspace code ourselves. With our focus being on Rogue
> and beyond, we don't currently have any plans to work on this.

Hm. This makes me wonder if it could be possible to open source the SGX code
since it is a different architecture than Rogue, is no longer your focus and
AFAIK the last SoC with SGX hit market almost a decade ago. This would enable
the community to make driver, firmware and user-space more generic (and more
compatible to modern distributions e.g. libc and other versions).

Perhaps others interested in PVRSGX can chime in.

> 
> Thanks
> Frank

Thanks as well,
Nikolaus