diff mbox series

[08/11] ARM: dts: DRA7xx: Add device tree entry for SGX GPU

Message ID 20240109171950.31010-9-afd@ti.com (mailing list archive)
State New
Headers show
Series Device tree support for Imagination Series5 GPU | expand

Commit Message

Andrew Davis Jan. 9, 2024, 5:19 p.m. UTC 
Add SGX GPU device entry to base DRA7x dtsi file.

Signed-off-by: Andrew Davis <afd@ti.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 arch/arm/boot/dts/ti/omap/dra7.dtsi | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

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
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/ti/omap/dra7.dtsi b/arch/arm/boot/dts/ti/omap/dra7.dtsi
index 6509c742fb58c..8527643cb69a8 100644
--- 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>;
 			clocks = <&gpu_clkctrl DRA7_GPU_CLKCTRL 0>;
 			clock-names = "fck";
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges = <0 0x56000000 0x2000000>;
+
+			gpu@0 {
+				compatible = "ti,am5728-gpu", "img,powervr-sgx544";
+				reg = <0x0 0x10000>; /* 64kB */
+				interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
+			};
 		};
 
 		crossbar_mpu: crossbar@4a002a48 {