linux-kernel - Re: [PATCH 1/2] dt-bindings: Add Apple SoC GPU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <edf14a64-decc-4392-a038-08b5dd942f8d@kernel.org>
Date: Wed, 11 Jun 2025 20:44:08 +0200
From: Sven Peter <sven@...nel.org>
To: fnkl.kernel@...il.com
Cc: asahi@...ts.linux.dev,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Simona Vetter <simona@...ll.ch>, David Airlie <airlied@...il.com>,
 Neal Gompa <neal@...pa.dev>, Maxime Ripard <mripard@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Rob Herring <robh@...nel.org>,
 Thomas Zimmermann <tzimmermann@...e.de>,
 Alyssa Rosenzweig <alyssa@...enzweig.io>, Janne Grunau <j@...nau.net>,
 linux-arm-kernel@...ts.infradead.org, dri-devel@...ts.freedesktop.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: Add Apple SoC GPU

Hi,


I think there's a "gpu: " missing in the subject.

On 11.06.25 19:32, Sasha Finkelstein via B4 Relay wrote:
> From: Sasha Finkelstein <fnkl.kernel@...il.com>
> 
> Add bindings for the GPU present in Apple SoCs
> 
> Signed-off-by: Sasha Finkelstein <fnkl.kernel@...il.com>
> ---
>   Documentation/devicetree/bindings/gpu/apple,agx.yaml | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   MAINTAINERS                                          |  1 +
>   2 files changed, 96 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpu/apple,agx.yaml b/Documentation/devicetree/bindings/gpu/apple,agx.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..23df3ebd689b1e885eb99ca573343fe7f2d09dc4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/apple,agx.yaml
> @@ -0,0 +1,95 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpu/apple,agx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple SoC GPU
> +
> +maintainers:
> +  - Sasha Finkelstein <fnkl.kernel@...il.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: apple,agx-g13g
> +      - items:
> +          - enum:
> +              - apple,agx-g13s
> +              - apple,agx-g13c
> +              - apple,agx-g13d
> +          - const: apple,agx-g13x
> +      - items:
> +          - const: apple,agx-g14g
> +  reg:
> +    items:
> +      - description: ASC coprocessor control

Apple calls these co-processors ASC but I'm not sure that's really 
helpful to describe them. Maybe just "GPU coprocessor control registers"?


> +      - description: GPU block MMIO registers
> +
> +  reg-names:
> +    items:
> +      - const: asc
> +      - const: sgx
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  mboxes:
> +    maxItems: 1
> +
> +  memory-region:
> +    items:
> +      - description: Region containing GPU MMU TTBs
> +      - description: Region containing GPU MMU page tables
> +      - description:
> +          Region containing a shared handoff structure for VM
> +          management coordination
> +      - description: Driver-opaque calibration blob
> +      - description: Calibration blob

Like Alyssa mentioned, this description also raises more questions than 
it answers for me. Do we know what these two blobs contain or why they 
are two separate blobs?

> +      - description: Shared global variables with GPU firmware
> +
> +  memory-region-names:
> +    items:
> +      - const: ttbs
> +      - const: pagetables
> +      - const: handoff
> +      - const: hw-cal-a
> +      - const: hw-cal-b
> +      - const: globals
> +
> +  apple,firmware-compat:
Nit: I'd prefer something like apple,firmware-abi here.

> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 3
> +    description:
> +      MacOS version the current firmware is paired with, used to pick
> +      the version of firmware ABI to be used.
> +      Bootloader will overwrite this
> +
> +required:
> +  - compatible
> +  - reg
> +  - mboxes
> +  - memory-region
> +  - apple,firmware-compat
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/apple-aic.h>

No need for the include for this example.

> +    gpu@...0000 {
> +        compatible = "apple,agx-g13g";
> +        reg = <0x6400000 0x40000>,
> +              <0x4000000 0x1000000>;
> +        reg-names = "asc", "sgx";
> +        mboxes = <&agx_mbox>;
> +        power-domains = <&ps_gfx>;
> +        memory-region = <&uat_ttbs>, <&uat_pagetables>, <&uat_handoff>,
> +                        <&gpu_hw_cal_a>, <&gpu_hw_cal_b>, <&gpu_globals>;
> +        memory-region-names = "ttbs", "pagetables", "handoff",
> +                              "hw-cal-a", "hw-cal-b", "globals";
> +
> +        apple,firmware-compat = <0 0 0>;
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a92290fffa163f9fe8fe3f04bf66426f9a894409..2a32c9c4ee355a1109a3e2031ea3663c39cc8c68 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2331,6 +2331,7 @@ F:	Documentation/devicetree/bindings/arm/apple/*
>   F:	Documentation/devicetree/bindings/clock/apple,nco.yaml
>   F:	Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
>   F:	Documentation/devicetree/bindings/dma/apple,admac.yaml
> +F:	Documentation/devicetree/bindings/gpu/apple,agx.yaml
>   F:	Documentation/devicetree/bindings/i2c/apple,i2c.yaml
>   F:	Documentation/devicetree/bindings/input/touchscreen/apple,z2-multitouch.yaml
>   F:	Documentation/devicetree/bindings/interrupt-controller/apple,*
> 


Best,


Sven