| Message ID | 20250128194816.2185326-13-m.wilczynski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable drm/imagination BXM-4-64 Support for LicheePi 4A | expand |
On Tue, 28 Jan 2025 20:48:10 +0100, Michal Wilczynski wrote: > Add bindings for the PowerVR BXM-4-64 GPU integrated in the T-HEAD > TH1520 SoC. This GPU requires two clocks. > > Document the integration details including clock, reset, power domain > and interrupt assignments. Add a dt-bindings example showing the proper > usage of the compatible string "thead,th1520-gpu" along with > "img,img-bxm". > > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> > --- > .../bindings/gpu/img,powervr-rogue.yaml | 39 +++++++++++++++++-- > 1 file changed, 35 insertions(+), 4 deletions(-) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: Error: Documentation/devicetree/bindings/gpu/img,powervr-rogue.example.dts:46.28-29 syntax error FATAL ERROR: Unable to parse input tree make[2]: *** [scripts/Makefile.dtbs:131: Documentation/devicetree/bindings/gpu/img,powervr-rogue.example.dtb] Error 1 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1506: dt_binding_check] Error 2 make: *** [Makefile:251: __sub-make] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250128194816.2185326-13-m.wilczynski@samsung.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On 28/01/2025 19:48, Michal Wilczynski wrote: > Add bindings for the PowerVR BXM-4-64 GPU integrated in the T-HEAD > TH1520 SoC. This GPU requires two clocks. None of the IMG Rogue GPUs use two clocks; they're all either one or three. The TRM for the TH1520 I have shows the standard three (core, cfg and mem). I mentioned this on P2 ("clk: thead: Add clock support for VO subsystem in T-Head TH1520 SoC"); can you add the missing clock here too? > Document the integration details including clock, reset, power domain > and interrupt assignments. Add a dt-bindings example showing the proper > usage of the compatible string "thead,th1520-gpu" along with > "img,img-bxm". > > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> > --- > .../bindings/gpu/img,powervr-rogue.yaml | 39 +++++++++++++++++-- > 1 file changed, 35 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > index bb607d4b1e07..b0d9635704d8 100644 > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml > @@ -12,10 +12,15 @@ 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 # IMG AXE GPU model/revision is fully discoverable > + - items: > + - enum: > + - thead,th1520-gpu > + - const: img,img-bxm This is going to be the main conflict between this series and the other B-Series series I mentioned on the cover letter. One of the main changes in that series is to rework how our compatible strings are structured; that would make this "thead,th1520-gpu", "img,img-bxm-4-64", "img,img-rogue". Would you mind holding this change back until the other series lands so we can avoid carrying a second deprecated compatible string? > > reg: > maxItems: 1 > @@ -60,6 +65,17 @@ allOf: > clocks: > maxItems: 1 > > + - if: > + properties: > + compatible: > + contains: > + const: thead,th1520-gpu > + then: > + properties: > + clocks: > + minItems: 2 > + maxItems: 2 As mentioned before, this doesn't represent the hardware. Please bump to 3 and add the missing clock. > + > examples: > - | > #include <dt-bindings/interrupt-controller/irq.h> > @@ -74,3 +90,18 @@ examples: > interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>; > power-domains = <&k3_pds 187 TI_SCI_PD_EXCLUSIVE>; > }; > + > + #include <dt-bindings/clock/thead,th1520-clk-ap.h> > + #include <dt-bindings/power/thead,th1520-power.h> > + #include <dt-bindings/reset/thead,th1520-reset.h> > + > + gpu: gpu@fff0000 { > + compatible = "thead,th1520-gpu", "img,img-bxm"; > + reg = <0xfff0000 0x1000>; > + interrupt-parent = <&plic>; > + interrupts = <102 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk CLK_GPU_CORE>, <&clk CLK_GPU_CFG_ACLK>; > + clock-names = "core", "mem"; You have CFG mapped to "mem" here. Out of curiosity, was that mismatch required to make things work? Cheers, Matt > + power-domains = <&pd TH1520_GPU_PD>; > + resets = <&rst TH1520_RESET_ID_GPU>; > + };
On 1/31/25 16:39, Matt Coster wrote: > On 28/01/2025 19:48, Michal Wilczynski wrote: >> Add bindings for the PowerVR BXM-4-64 GPU integrated in the T-HEAD >> TH1520 SoC. This GPU requires two clocks. > > None of the IMG Rogue GPUs use two clocks; they're all either one or > three. The TRM for the TH1520 I have shows the standard three (core, > cfg and mem). I mentioned this on P2 ("clk: thead: Add clock support for > VO subsystem in T-Head TH1520 SoC"); can you add the missing clock here > too? > >> Document the integration details including clock, reset, power domain >> and interrupt assignments. Add a dt-bindings example showing the proper >> usage of the compatible string "thead,th1520-gpu" along with >> "img,img-bxm". >> >> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> >> --- >> .../bindings/gpu/img,powervr-rogue.yaml | 39 +++++++++++++++++-- >> 1 file changed, 35 insertions(+), 4 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> index bb607d4b1e07..b0d9635704d8 100644 >> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml >> @@ -12,10 +12,15 @@ 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 # IMG AXE GPU model/revision is fully discoverable >> + - items: >> + - enum: >> + - thead,th1520-gpu >> + - const: img,img-bxm > > This is going to be the main conflict between this series and the other > B-Series series I mentioned on the cover letter. One of the main changes > in that series is to rework how our compatible strings are structured; > that would make this "thead,th1520-gpu", "img,img-bxm-4-64", > "img,img-rogue". Would you mind holding this change back until the other > series lands so we can avoid carrying a second deprecated compatible > string? Sure that's completely fine ! > >> >> reg: >> maxItems: 1 >> @@ -60,6 +65,17 @@ allOf: >> clocks: >> maxItems: 1 >> >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: thead,th1520-gpu >> + then: >> + properties: >> + clocks: >> + minItems: 2 >> + maxItems: 2 > > As mentioned before, this doesn't represent the hardware. Please bump to > 3 and add the missing clock. > >> + >> examples: >> - | >> #include <dt-bindings/interrupt-controller/irq.h> >> @@ -74,3 +90,18 @@ examples: >> interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>; >> power-domains = <&k3_pds 187 TI_SCI_PD_EXCLUSIVE>; >> }; >> + >> + #include <dt-bindings/clock/thead,th1520-clk-ap.h> >> + #include <dt-bindings/power/thead,th1520-power.h> >> + #include <dt-bindings/reset/thead,th1520-reset.h> >> + >> + gpu: gpu@fff0000 { >> + compatible = "thead,th1520-gpu", "img,img-bxm"; >> + reg = <0xfff0000 0x1000>; >> + interrupt-parent = <&plic>; >> + interrupts = <102 IRQ_TYPE_LEVEL_HIGH>; >> + clocks = <&clk CLK_GPU_CORE>, <&clk CLK_GPU_CFG_ACLK>; >> + clock-names = "core", "mem"; > > You have CFG mapped to "mem" here. Out of curiosity, was that mismatch > required to make things work? Yeah exactly, I understand that from the GPU perspective there are three clocks, but only two are programmable from the SoC perspective. So maybe a placeholder clock should be added in the devicetree then, since the clock exists, but is reserved. > > Cheers, > Matt > >> + power-domains = <&pd TH1520_GPU_PD>; >> + resets = <&rst TH1520_RESET_ID_GPU>; >> + }; >
diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml index bb607d4b1e07..b0d9635704d8 100644 --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml @@ -12,10 +12,15 @@ 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 # IMG AXE GPU model/revision is fully discoverable + - items: + - enum: + - thead,th1520-gpu + - const: img,img-bxm reg: maxItems: 1 @@ -60,6 +65,17 @@ allOf: clocks: maxItems: 1 + - if: + properties: + compatible: + contains: + const: thead,th1520-gpu + then: + properties: + clocks: + minItems: 2 + maxItems: 2 + examples: - | #include <dt-bindings/interrupt-controller/irq.h> @@ -74,3 +90,18 @@ examples: interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>; power-domains = <&k3_pds 187 TI_SCI_PD_EXCLUSIVE>; }; + + #include <dt-bindings/clock/thead,th1520-clk-ap.h> + #include <dt-bindings/power/thead,th1520-power.h> + #include <dt-bindings/reset/thead,th1520-reset.h> + + gpu: gpu@fff0000 { + compatible = "thead,th1520-gpu", "img,img-bxm"; + reg = <0xfff0000 0x1000>; + interrupt-parent = <&plic>; + interrupts = <102 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk CLK_GPU_CORE>, <&clk CLK_GPU_CFG_ACLK>; + clock-names = "core", "mem"; + power-domains = <&pd TH1520_GPU_PD>; + resets = <&rst TH1520_RESET_ID_GPU>; + };
Add bindings for the PowerVR BXM-4-64 GPU integrated in the T-HEAD TH1520 SoC. This GPU requires two clocks. Document the integration details including clock, reset, power domain and interrupt assignments. Add a dt-bindings example showing the proper usage of the compatible string "thead,th1520-gpu" along with "img,img-bxm". Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com> --- .../bindings/gpu/img,powervr-rogue.yaml | 39 +++++++++++++++++-- 1 file changed, 35 insertions(+), 4 deletions(-)