| Message ID | 20241118-sets-bxs-4-64-patch-v1-v2-1-3fd45d9fb0cf@imgtec.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Imagination BXS-4-64 MC1 GPU support | expand |
On Mon, Nov 18, 2024 at 01:01:53PM +0000, Matt Coster wrote: > The current compatible strings are not specific enough to constrain the No, they are specific enough. > hardware in devicetree. For example, the current "img,img-axe" string > refers to the entire family of Series AXE GPUs. The more specific > "img,img-axe-1-16m" string refers to the AXE-1-16M GPU which, unlike the > rest of its family, only uses a single power domain. > > While in this instance there is already "ti,am62-gpu" for the more That's the specific compatible. > specific case, the intent here is to draw a line between properties > inherent to the IP core and choices made by the silicon vendor at > integration time. For example, the number of power domains is a property > of the IP core, whereas the decision to use one or three clocks (see > next patch) is a vendor one. > > Work is currently underway to add support for volcanic-based Imagination > GPUs, for which bindings will be added in "img,powervr-volcanic.yaml". > The split between rogue and volcanic cores is non-obvious at times, so > add a generic top-level "img,img-rogue" compatible string here to allow > for simpler differentiation in devicetrees without referring back to the > bindings. > > Make these changes now before introducing more compatible strings to keep > the legacy versions to a minimum. > > Signed-off-by: Matt Coster <matt.coster@imgtec.com> > --- > Changes in v2: > - Clarified justification for compatible strings > - Link to v1: https://lore.kernel.org/r/20241105-sets-bxs-4-64-patch-v1-v1-1-4ed30e865892@imgtec.com > --- > .../devicetree/bindings/gpu/img,powervr-rogue.yaml | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > index 256e252f8087fa0d6081f771a01601d34b66fe19..ef7070daf213277d0190fe319e202fdc597337d4 100644 > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > @@ -12,10 +12,19 @@ maintainers: > > properties: > compatible: > - items: > - - enum: > - - ti,am62-gpu > - - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable > + oneOf: > + - items: > + - enum: > + - ti,am62-gpu > + - const: img,img-axe-1-16m > + - const: img,img-rogue NAK, that's ABI break. You are stuck with whatever you sent earlier. > + > + # This legacy combination of compatible strings was introduced early on before the more > + # specific GPU identifiers were used. Keep it around here for compatibility, but never use > + # "img,img-axe" in new devicetrees. Wrap according to Linux coding style. But anyway this is not needed, just deprecate things. > + - items: > + - const: ti,am62-gpu > + - const: img,img-axe So you want to use deprecated here? Anyway, entire change is not needed and without proper justification. Your marketing terms are not proper justification. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml index 256e252f8087fa0d6081f771a01601d34b66fe19..ef7070daf213277d0190fe319e202fdc597337d4 100644 --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml @@ -12,10 +12,19 @@ maintainers: properties: compatible: - items: - - enum: - - ti,am62-gpu - - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable + oneOf: + - items: + - enum: + - ti,am62-gpu + - const: img,img-axe-1-16m + - const: img,img-rogue + + # This legacy combination of compatible strings was introduced early on before the more + # specific GPU identifiers were used. Keep it around here for compatibility, but never use + # "img,img-axe" in new devicetrees. + - items: + - const: ti,am62-gpu + - const: img,img-axe reg: maxItems: 1 @@ -64,7 +73,7 @@ examples: #include <dt-bindings/soc/ti,sci_pm_domain.h> gpu@fd00000 { - compatible = "ti,am62-gpu", "img,img-axe"; + compatible = "ti,am62-gpu", "img,img-axe-1-16m", "img,img-rogue"; reg = <0x0fd00000 0x20000>; clocks = <&k3_clks 187 0>; clock-names = "core";
The current compatible strings are not specific enough to constrain the hardware in devicetree. For example, the current "img,img-axe" string refers to the entire family of Series AXE GPUs. The more specific "img,img-axe-1-16m" string refers to the AXE-1-16M GPU which, unlike the rest of its family, only uses a single power domain. While in this instance there is already "ti,am62-gpu" for the more specific case, the intent here is to draw a line between properties inherent to the IP core and choices made by the silicon vendor at integration time. For example, the number of power domains is a property of the IP core, whereas the decision to use one or three clocks (see next patch) is a vendor one. Work is currently underway to add support for volcanic-based Imagination GPUs, for which bindings will be added in "img,powervr-volcanic.yaml". The split between rogue and volcanic cores is non-obvious at times, so add a generic top-level "img,img-rogue" compatible string here to allow for simpler differentiation in devicetrees without referring back to the bindings. Make these changes now before introducing more compatible strings to keep the legacy versions to a minimum. Signed-off-by: Matt Coster <matt.coster@imgtec.com> --- Changes in v2: - Clarified justification for compatible strings - Link to v1: https://lore.kernel.org/r/20241105-sets-bxs-4-64-patch-v1-v1-1-4ed30e865892@imgtec.com --- .../devicetree/bindings/gpu/img,powervr-rogue.yaml | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)