Re: [PATCH v4 4/7] dt-bindings: gpu: v3d: Add per-compatible register restrictions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Stefan,

On 15/03/25 06:52, Stefan Wahren wrote:
Hello,

Am 13.03.25 um 20:04 schrieb Maíra Canal:
+Cc Stefan

Hi Krzysztof,

On 13/03/25 12:03, Krzysztof Kozlowski wrote:
On 13/03/2025 15:43, Maíra Canal wrote:
In order to enforce per-SoC register rules, add per-compatible
restrictions. V3D 3.3 (represented by brcm,7268-v3d) has a cache
controller (GCA), which is not present in other V3D generations.
Declaring these differences helps ensure the DTB accurately reflect
the hardware design.

While not ideal, this commit keeps the register order flexible for
brcm,7268-v3d with the goal to keep the ABI backwards compatible.

Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx>
---
  .../devicetree/bindings/gpu/brcm,bcm-v3d.yaml      | 73
++++++++++++++++++----
  1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
index
dc078ceeca9ac3447ba54a7c8830821f0b2a7f9f..9867b617c60c6fe34a0f88a3ee2f581a94b69a5c
100644
--- a/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml
+++ b/Documentation/devicetree/bindings/gpu/brcm,bcm-v3d.yaml

[...]

+  - if:
+      properties:
+        compatible:
+          contains:
+            const: brcm,7268-v3d
+    then:
+      properties:
+        reg:
+          items:
+            - description: hub register
+            - description: core0 register
+            - description: GCA cache controller register
+            - description: bridge register (if no external reset
controller)
+          minItems: 3
+        reg-names:
+          items:
+            - const: hub
+            - const: core0
+            - enum: [ bridge, gca ]

> So GCA is always there? Then this should be just 'gca'. Your list for

GCA is always there for V3D 3.3, therefore it is always there for
brcm,7268-v3d.

'reg' already says that third item must be GCA. I understand that you do
not want to affect the ABI, but it already kind of is with enforcing GCA
in 'reg'.

I'm adding Stefan to the loop as he was the one that converted this DT
binding to YAML. Stefan, could you share your thoughts about breaking
the ABI for BCM7268? We would enforce the following order: hub, core0,
bridge, and gca.
Phew, that was over 4 years ago. To be honest, my only motivation back
then was to prepare support for the Raspberry Pi 4 (BCM2711). I did it
all in my spare time and never had access to any Broadcom documents. I
have no idea about all the other BCM chips, so a possible break of the
ABI for the BCM7268 was an accident. I don't know if Florian Fainelli or
Maxime Ripard can help here.

Thanks for providing your feedback! I did my diligence and now I know
which SoCs have each register bank. For BCM2711, BCM2712, and BCM7278,
the ABI will be preserved. As for BCM7268, I plan to enforce the order
specified in the current DT binding example: hub, core0, bridge, and
gca.

Florian, it would be great to hear your feedback about BCM7268.


By the way the two schema maintainers have not been active at V3D for a
long time, so it would be good if someone could take over.

I sent a patch [1] to step in as maintainer.

[1] https://lore.kernel.org/dri-devel/20250313-v3d-gpu-reset-fixes-v4-0-c1e780d8e096@xxxxxxxxxx/T/#mf8ffc3dc7a216efc1842d773787394c3506814cd

Best Regards,
- Maíra


Regards


I anyway do not understand why 'gca' or 'bridge' are supposed to be
optional. Does the given SoC differ between boards? What is the external
reset controller here? External like outside of SoC?

TBH I never saw BCM7268 or BCM7278 in the wild, but you are correct,
"bridge" shouldn't change for the same SoC. I'll do my diligence and
research more about those SoCs.

Best Regards,
- Maíra


Best regards,
Krzysztof






[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux