mbox series

[00/11] Device tree support for Imagination Series5 GPU

Message ID 20240109171950.31010-1-afd@ti.com
Headers show
Series Device tree support for Imagination Series5 GPU | expand

Message

Andrew Davis Jan. 9, 2024, 5:19 p.m. UTC
Hello all,

I know this has been tried before[0], but given the recent upstreaming of
the Series6+ GPU bindings I figured it might be time to give the Series5
bindings another try.

While there is currently no mainline driver for these binding, there is an
open source out-of-tree kernel-side driver available[1]. Having a stable
and upstream binding for these devices allows us to describe this hardware
in device tree.

This is my vision for how these bindings should look, along with some
example uses in several SoC DT files. The compatible names have been
updated to match what was decided on for Series6+, but otherwise most
is the same as we have been using in our vendor tree for many years.

Thanks,
Andrew

Based on next-20240109.

[0]: https://lkml.org/lkml/2020/4/24/1222
[1]: https://github.com/openpvrsgx-devgroup

Changes for v1:
 - Added commit message to patch #1
 - Reworked Rogue binding title
 - Add TI copyright to new binding doc
 - Added default min/maxItems to clocks property
 - Moved "additionalProperties" to end
 - Flattened out allOf block logic
 - Added extra SGX binding example
 - Added Suggested/Reviewed tags

Changes for RFC v2:
 - Added patch to rename Rogue+ binding to img,powervr-rogue.yaml
 - Locked all property item counts
 - Removed nodename pattern check

Andrew Davis (11):
  dt-bindings: gpu: Rename img,powervr to img,powervr-rogue
  dt-bindings: gpu: Add PowerVR Series5 SGX GPUs
  ARM: dts: omap3: Add device tree entry for SGX GPU
  ARM: dts: omap4: Add device tree entry for SGX GPU
  ARM: dts: omap5: Add device tree entry for SGX GPU
  ARM: dts: AM33xx: Add device tree entry for SGX GPU
  ARM: dts: AM437x: Add device tree entry for SGX GPU
  ARM: dts: DRA7xx: Add device tree entry for SGX GPU
  arm64: dts: ti: k3-am654-main: Add device tree entry for SGX GPU
  ARM: dts: sun6i: Add device tree entry for SGX GPU
  MIPS: DTS: jz4780: Add device tree entry for SGX GPU

 ...mg,powervr.yaml => img,powervr-rogue.yaml} |   4 +-
 .../bindings/gpu/img,powervr-sgx.yaml         | 138 ++++++++++++++++++
 MAINTAINERS                                   |   3 +-
 arch/arm/boot/dts/allwinner/sun6i-a31.dtsi    |   9 ++
 arch/arm/boot/dts/ti/omap/am33xx.dtsi         |   9 +-
 arch/arm/boot/dts/ti/omap/am3517.dtsi         |  11 +-
 arch/arm/boot/dts/ti/omap/am4372.dtsi         |   6 +
 arch/arm/boot/dts/ti/omap/dra7.dtsi           |   9 +-
 arch/arm/boot/dts/ti/omap/omap34xx.dtsi       |  11 +-
 arch/arm/boot/dts/ti/omap/omap36xx.dtsi       |   9 +-
 arch/arm/boot/dts/ti/omap/omap4.dtsi          |   9 +-
 arch/arm/boot/dts/ti/omap/omap5.dtsi          |   9 +-
 arch/arm64/boot/dts/ti/k3-am65-main.dtsi      |   7 +
 arch/mips/boot/dts/ingenic/jz4780.dtsi        |  11 ++
 14 files changed, 215 insertions(+), 30 deletions(-)
 rename Documentation/devicetree/bindings/gpu/{img,powervr.yaml => img,powervr-rogue.yaml} (91%)
 create mode 100644 Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml

Comments

Tony Lindgren Jan. 10, 2024, 8:29 a.m. UTC | #1
* Andrew Davis <afd@ti.com> [240109 17:20]:
> --- a/arch/arm/boot/dts/ti/omap/dra7.dtsi
> +++ b/arch/arm/boot/dts/ti/omap/dra7.dtsi
> @@ -850,12 +850,19 @@ target-module@56000000 {
>  					<SYSC_IDLE_SMART>;
>  			ti,sysc-sidle = <SYSC_IDLE_FORCE>,
>  					<SYSC_IDLE_NO>,
> -					<SYSC_IDLE_SMART>;
> +					<SYSC_IDLE_SMART>,
> +					<SYSC_IDLE_SMART_WKUP>;

You probably checked this already.. But just in case, can you please
confirm this is intentional. The documentation lists the smart wakeup
capability bit as reserved for dra7, maybe the documentation is wrong.

Regards,

Tony
Andrew Davis Jan. 17, 2024, 3:52 p.m. UTC | #2
On 1/10/24 2:29 AM, Tony Lindgren wrote:
> * Andrew Davis <afd@ti.com> [240109 17:20]:
>> --- a/arch/arm/boot/dts/ti/omap/dra7.dtsi
>> +++ b/arch/arm/boot/dts/ti/omap/dra7.dtsi
>> @@ -850,12 +850,19 @@ target-module@56000000 {
>>   					<SYSC_IDLE_SMART>;
>>   			ti,sysc-sidle = <SYSC_IDLE_FORCE>,
>>   					<SYSC_IDLE_NO>,
>> -					<SYSC_IDLE_SMART>;
>> +					<SYSC_IDLE_SMART>,
>> +					<SYSC_IDLE_SMART_WKUP>;
> 
> You probably checked this already.. But just in case, can you please
> confirm this is intentional. The documentation lists the smart wakeup
> capability bit as reserved for dra7, maybe the documentation is wrong.
> 

It was an intentional change, although I'm not sure it is correct :)

This is how we had it in our "evil vendor tree" for years (back when it
was hwmod based), so when converting these nodes to use "ti,sysc" I noticed
this bit was set, but as you point out the documentation disagrees.

I'd rather go with what has worked before, but it doesn't seem to
break anything either way, so we could also break this change out into
its own patch if you would prefer.

Andrew

> Regards,
> 
> Tony
>
Tony Lindgren Jan. 18, 2024, 8:55 a.m. UTC | #3
* Andrew Davis <afd@ti.com> [240117 15:52]:
> On 1/10/24 2:29 AM, Tony Lindgren wrote:
> > * Andrew Davis <afd@ti.com> [240109 17:20]:
> > > --- a/arch/arm/boot/dts/ti/omap/dra7.dtsi
> > > +++ b/arch/arm/boot/dts/ti/omap/dra7.dtsi
> > > @@ -850,12 +850,19 @@ target-module@56000000 {
> > >   					<SYSC_IDLE_SMART>;
> > >   			ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> > >   					<SYSC_IDLE_NO>,
> > > -					<SYSC_IDLE_SMART>;
> > > +					<SYSC_IDLE_SMART>,
> > > +					<SYSC_IDLE_SMART_WKUP>;
> > 
> > You probably checked this already.. But just in case, can you please
> > confirm this is intentional. The documentation lists the smart wakeup
> > capability bit as reserved for dra7, maybe the documentation is wrong.
> > 
> 
> It was an intentional change, although I'm not sure it is correct :)
> 
> This is how we had it in our "evil vendor tree" for years (back when it
> was hwmod based), so when converting these nodes to use "ti,sysc" I noticed
> this bit was set, but as you point out the documentation disagrees.
> 
> I'd rather go with what has worked before, but it doesn't seem to
> break anything either way, so we could also break this change out into
> its own patch if you would prefer.

I agree it's best to stick what is known to work. How about let's add
the related information to the patch description?

Regards,

Tony
Tony Lindgren Jan. 26, 2024, 7:45 a.m. UTC | #4
* Tony Lindgren <tony@atomide.com> [240118 08:57]:
> * Andrew Davis <afd@ti.com> [240117 15:52]:
> > On 1/10/24 2:29 AM, Tony Lindgren wrote:
> > > * Andrew Davis <afd@ti.com> [240109 17:20]:
> > > > --- a/arch/arm/boot/dts/ti/omap/dra7.dtsi
> > > > +++ b/arch/arm/boot/dts/ti/omap/dra7.dtsi
> > > > @@ -850,12 +850,19 @@ target-module@56000000 {
> > > >   					<SYSC_IDLE_SMART>;
> > > >   			ti,sysc-sidle = <SYSC_IDLE_FORCE>,
> > > >   					<SYSC_IDLE_NO>,
> > > > -					<SYSC_IDLE_SMART>;
> > > > +					<SYSC_IDLE_SMART>,
> > > > +					<SYSC_IDLE_SMART_WKUP>;
> > > 
> > > You probably checked this already.. But just in case, can you please
> > > confirm this is intentional. The documentation lists the smart wakeup
> > > capability bit as reserved for dra7, maybe the documentation is wrong.
> > > 
> > 
> > It was an intentional change, although I'm not sure it is correct :)
> > 
> > This is how we had it in our "evil vendor tree" for years (back when it
> > was hwmod based), so when converting these nodes to use "ti,sysc" I noticed
> > this bit was set, but as you point out the documentation disagrees.
> > 
> > I'd rather go with what has worked before, but it doesn't seem to
> > break anything either way, so we could also break this change out into
> > its own patch if you would prefer.
> 
> I agree it's best to stick what is known to work. How about let's add
> the related information to the patch description?

I'll update the commit message for it and apply these, no need to repost.

Regards,

Tony