| Message ID | 20170828211918.11573-15-tony@atomide.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Fix missing device tree hwmods and IO ranges omap variants | expand |
Hi, On Mon, Aug 28, 2017 at 02:19:15PM -0700, Tony Lindgren wrote: > On omap4 we're missing the PowerVR SGX GPU node with it's related > "ti,hwmods" property that the SoC interconnect code needs. > > Note that this will only show up as a bug with "doesn't have > mpu register target base" boot errors when the legacy platform > data is removed. > > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- I think OMAP3 & OMAP5 should also be documented and getting a node in this series? -- Sebastian > .../devicetree/bindings/gpu/ti-powervr-sgx.txt | 34 ++++++++++++++++++++++ > arch/arm/boot/dts/omap4.dtsi | 7 +++++ > 2 files changed, 41 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpu/ti-powervr-sgx.txt > > diff --git a/Documentation/devicetree/bindings/gpu/ti-powervr-sgx.txt b/Documentation/devicetree/bindings/gpu/ti-powervr-sgx.txt > new file mode 100644 > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpu/ti-powervr-sgx.txt > @@ -0,0 +1,34 @@ > +Texas Instruments PowevVR SGX binding > + > +SGX can be used for graphics acceleration on Texas Instruments SoCs. > + > +Note that the SGX binding is currently only used by the SoC interconnect > +code to idle the module on init and no open source driver is available > +for SGX. Please update this documentation if that changes. > + > +Required properties: > + > +compatible: Shall be one of the following: > + "ti,omap4-gpu" > + > +reg: Shall contain the device instance IO range > + > +interrupts: Shall contain the device instance interrupt > + > + > +Optional properties: > + > +reg-names: Shall contain the IO range names if multiple IO > + ranges are used by the SoC > + > +ti,hwmods: Shall contain the TI interconnect module name if needed > + by the SoC > + > + > +Example: > + gpu: gpu@56000000 { > + compatible = "ti,omap4-gpu"; > + reg = <0x56000000 0x10000>; > + interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>; > + ti,hwmods = "gpu"; > + }; > diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi > --- a/arch/arm/boot/dts/omap4.dtsi > +++ b/arch/arm/boot/dts/omap4.dtsi > @@ -1086,6 +1086,13 @@ > status = "disabled"; > }; > > + gpu: gpu@56000000 { > + compatible = "ti,omap4-gpu"; > + reg = <0x56000000 0x10000>; > + interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>; > + ti,hwmods = "gpu"; > + }; > + > dss: dss@58000000 { > compatible = "ti,omap4-dss"; > reg = <0x58000000 0x80>; > -- > 2.14.1 > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
This message contains a digitally signed email which can be read by opening the attachment. Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 29/08/17 12:00, Sebastian Reichel wrote: > Hi, > > On Mon, Aug 28, 2017 at 02:19:15PM -0700, Tony Lindgren wrote: >> On omap4 we're missing the PowerVR SGX GPU node with it's related >> "ti,hwmods" property that the SoC interconnect code needs. >> >> Note that this will only show up as a bug with "doesn't have >> mpu register target base" boot errors when the legacy platform >> data is removed. >> >> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> >> Signed-off-by: Tony Lindgren <tony@atomide.com> >> --- > > I think OMAP3 & OMAP5 should also be documented and getting a > node in this series? Do we even want to add SGX to the .dts? We don't have proper drivers for SGX. If we ever do, who knows what kind of DT data they need. I know the DT data for SGX in TI's kernel tree has changed at least once. Tomi
Hi, On Tue, Aug 29, 2017 at 02:35:09PM +0300, Tomi Valkeinen wrote: > This message contains a digitally signed email which can be read by opening the attachment. > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > Date: Tue, 29 Aug 2017 14:35:09 +0300 > From: Tomi Valkeinen <tomi.valkeinen@ti.com> > To: Sebastian Reichel <sebastian.reichel@collabora.co.uk>, Tony Lindgren > <tony@atomide.com> > CC: linux-omap@vger.kernel.org, Benoît Cousson <bcousson@baylibre.com>, > devicetree@vger.kernel.org > Subject: Re: [PATCH 14/17] ARM: dts: Add missing gpu node and binding for > omap4 > > On 29/08/17 12:00, Sebastian Reichel wrote: > > Hi, > > > > On Mon, Aug 28, 2017 at 02:19:15PM -0700, Tony Lindgren wrote: > >> On omap4 we're missing the PowerVR SGX GPU node with it's related > >> "ti,hwmods" property that the SoC interconnect code needs. > >> > >> Note that this will only show up as a bug with "doesn't have > >> mpu register target base" boot errors when the legacy platform > >> data is removed. > >> > >> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > >> Signed-off-by: Tony Lindgren <tony@atomide.com> > >> --- > > > > I think OMAP3 & OMAP5 should also be documented and getting a > > node in this series? > > Do we even want to add SGX to the .dts? We don't have proper drivers for > SGX. If we ever do, who knows what kind of DT data they need. I know the > DT data for SGX in TI's kernel tree has changed at least once. I don't think reg or interrupts will be removed, so the properties added by Tony look pretty safe?. I guess if we ever have a driver it would need some more properties and would bail out. Having no DT data is does not load at all, the result is the same. OTOH having the node means the kernel can properly send the module to idle. I think this patchset should Cc Rob and Mark. -- Sebastian
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 29/08/17 15:10, Sebastian Reichel wrote: >> Do we even want to add SGX to the .dts? We don't have proper drivers for >> SGX. If we ever do, who knows what kind of DT data they need. I know the >> DT data for SGX in TI's kernel tree has changed at least once. > > I don't think reg or interrupts will be removed, so the properties > added by Tony look pretty safe?. I guess if we ever have a driver Maybe. At the moment we have this in TI's tree for DRA7: gpu: gpu@56000000 { compatible = "ti,dra7-sgx544", "img,sgx544"; reg = <0x56000000 0x10000>; reg-names = "gpu_ocp_base"; interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>; ti,hwmods = "gpu"; clocks = <&l3_iclk_div>, <&gpu_core_gclk_mux>, <&gpu_hyd_gclk_mux>; clock-names = "iclk", "fclk1", "fclk2"; }; > it would need some more properties and would bail out. Having no > DT data is does not load at all, the result is the same. OTOH having > the node means the kernel can properly send the module to idle. I just get uneasy when adding DT data that we're not really sure if it's ok or not. I've been fighting with such data for ages. But, as you say, this probably won't matter if the driver will just reject DT data that doesn't have all the details. If we need the DT node to idle SGX, and we don't even mean to actually use an SGX driver with this data, it sounds fine to me. Tomi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki On 29/08/17 15:10, Sebastian Reichel wrote: >> Do we even want to add SGX to the .dts? We don't have proper drivers for >> SGX. If we ever do, who knows what kind of DT data they need. I know the >> DT data for SGX in TI's kernel tree has changed at least once. > > I don't think reg or interrupts will be removed, so the properties > added by Tony look pretty safe?. I guess if we ever have a driver Oh, and one more thing about the regs. I believe SGX consists of multiple register blocks. But as it's not documented in any public docs, there's just that single block in the DT data. I'm not sure if single block or multiple blocks would be the right approach, but then, I guess we can always live with just a single block and if needed split the blocks inside the driver. Tomi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Tomi Valkeinen <tomi.valkeinen@ti.com> [170829 05:25]: > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > On 29/08/17 15:10, Sebastian Reichel wrote: > > >> Do we even want to add SGX to the .dts? We don't have proper drivers for > >> SGX. If we ever do, who knows what kind of DT data they need. I know the > >> DT data for SGX in TI's kernel tree has changed at least once. > > > > I don't think reg or interrupts will be removed, so the properties > > added by Tony look pretty safe?. I guess if we ever have a driver > > Maybe. At the moment we have this in TI's tree for DRA7: > > gpu: gpu@56000000 { > compatible = "ti,dra7-sgx544", "img,sgx544"; > reg = <0x56000000 0x10000>; > reg-names = "gpu_ocp_base"; > interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>; > ti,hwmods = "gpu"; > clocks = <&l3_iclk_div>, <&gpu_core_gclk_mux>, > <&gpu_hyd_gclk_mux>; > clock-names = "iclk", "fclk1", "fclk2"; > }; > > > it would need some more properties and would bail out. Having no > > DT data is does not load at all, the result is the same. OTOH having > > the node means the kernel can properly send the module to idle. > > I just get uneasy when adding DT data that we're not really sure if it's > ok or not. I've been fighting with such data for ages. But, as you say, > this probably won't matter if the driver will just reject DT data that > doesn't have all the details. The above example looks OK to me, the interrupt is different for omap4. > If we need the DT node to idle SGX, and we don't even mean to actually > use an SGX driver with this data, it sounds fine to me. Well ideally the SGX driver will make use of it too. Let me know if you want to leave out some parts of the above example from TI tree. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Tomi Valkeinen <tomi.valkeinen@ti.com> [170829 05:28]: > > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > > On 29/08/17 15:10, Sebastian Reichel wrote: > > >> Do we even want to add SGX to the .dts? We don't have proper drivers for > >> SGX. If we ever do, who knows what kind of DT data they need. I know the > >> DT data for SGX in TI's kernel tree has changed at least once. > > > > I don't think reg or interrupts will be removed, so the properties > > added by Tony look pretty safe?. I guess if we ever have a driver > > Oh, and one more thing about the regs. I believe SGX consists of > multiple register blocks. But as it's not documented in any public docs, > there's just that single block in the DT data. I'm not sure if single > block or multiple blocks would be the right approach, but then, I guess > we can always live with just a single block and if needed split the > blocks inside the driver. OK, those register blocks can easily be child nodes of the module if needed. This is for the parent interconnect target module revc/sysc/syss registers that we already have mapped in legacy platform data for the interconnect code. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170829 02:01]: > Hi, > > On Mon, Aug 28, 2017 at 02:19:15PM -0700, Tony Lindgren wrote: > > On omap4 we're missing the PowerVR SGX GPU node with it's related > > "ti,hwmods" property that the SoC interconnect code needs. > > > > Note that this will only show up as a bug with "doesn't have > > mpu register target base" boot errors when the legacy platform > > data is removed. > > I think OMAP3 & OMAP5 should also be documented and getting a > node in this series? Looks like we don't currently have any interconnect data defined for those, but yeah sure I can add those. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 29, 2017 at 9:42 AM, Tony Lindgren <tony@atomide.com> wrote: > * Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170829 02:01]: >> Hi, >> >> On Mon, Aug 28, 2017 at 02:19:15PM -0700, Tony Lindgren wrote: >> > On omap4 we're missing the PowerVR SGX GPU node with it's related >> > "ti,hwmods" property that the SoC interconnect code needs. >> > >> > Note that this will only show up as a bug with "doesn't have >> > mpu register target base" boot errors when the legacy platform >> > data is removed. >> >> I think OMAP3 & OMAP5 should also be documented and getting a >> node in this series? > > Looks like we don't currently have any interconnect data defined > for those, but yeah sure I can add those. > If there is anything I can test, I had modified the SGX driver for the OMAP36, but abandoned my attempts since much of the device tree and reset stuff was missing. I'd love to see SGX work again. :-) adam > Regards, > > Tony > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Adam Ford <aford173@gmail.com> [170829 08:31]: > On Tue, Aug 29, 2017 at 9:42 AM, Tony Lindgren <tony@atomide.com> wrote: > > * Sebastian Reichel <sebastian.reichel@collabora.co.uk> [170829 02:01]: > >> Hi, > >> > >> On Mon, Aug 28, 2017 at 02:19:15PM -0700, Tony Lindgren wrote: > >> > On omap4 we're missing the PowerVR SGX GPU node with it's related > >> > "ti,hwmods" property that the SoC interconnect code needs. > >> > > >> > Note that this will only show up as a bug with "doesn't have > >> > mpu register target base" boot errors when the legacy platform > >> > data is removed. > >> > >> I think OMAP3 & OMAP5 should also be documented and getting a > >> node in this series? > > > > Looks like we don't currently have any interconnect data defined > > for those, but yeah sure I can add those. > > > > If there is anything I can test, I had modified the SGX driver for the > OMAP36, but abandoned my attempts since much of the device tree and > reset stuff was missing. I'd love to see SGX work again. :-) Well the resets can be handled using platform data callbacks for now with pdata-quirks.c. Then when the reset driver is available, we can just remove the platform data. And maybe push your changes to some tree so others can participate? I'm sure the n900 users would like to see it working too. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Tony Lindgren <tony@atomide.com> [170829 07:35]: > * Tomi Valkeinen <tomi.valkeinen@ti.com> [170829 05:25]: > > Maybe. At the moment we have this in TI's tree for DRA7: > > > > gpu: gpu@56000000 { > > compatible = "ti,dra7-sgx544", "img,sgx544"; I'll leave out "img,sgx544" as that's the generic component that's really a child of this interconnect target module. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Tony Lindgren <tony@atomide.com> [170829 08:58]: > * Tony Lindgren <tony@atomide.com> [170829 07:35]: > > * Tomi Valkeinen <tomi.valkeinen@ti.com> [170829 05:25]: > > > Maybe. At the moment we have this in TI's tree for DRA7: > > > > > > gpu: gpu@56000000 { > > > compatible = "ti,dra7-sgx544", "img,sgx544"; > > I'll leave out "img,sgx544" as that's the generic component > that's really a child of this interconnect target module. And for the gpu clock, on omap4, it seems that the clocks are just the module clktrl with a mux option. So the top level module driver implementing runtime PM should be enough there. And on dra7, there are two functional clocks maybe as the SGX instance is dual core. And the mainline kernel is using "fck" naming vs "fclk" naming. So more consideration is for the clocks and I'll leave them out for now. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/gpu/ti-powervr-sgx.txt b/Documentation/devicetree/bindings/gpu/ti-powervr-sgx.txt new file mode 100644 --- /dev/null +++ b/Documentation/devicetree/bindings/gpu/ti-powervr-sgx.txt @@ -0,0 +1,34 @@ +Texas Instruments PowevVR SGX binding + +SGX can be used for graphics acceleration on Texas Instruments SoCs. + +Note that the SGX binding is currently only used by the SoC interconnect +code to idle the module on init and no open source driver is available +for SGX. Please update this documentation if that changes. + +Required properties: + +compatible: Shall be one of the following: + "ti,omap4-gpu" + +reg: Shall contain the device instance IO range + +interrupts: Shall contain the device instance interrupt + + +Optional properties: + +reg-names: Shall contain the IO range names if multiple IO + ranges are used by the SoC + +ti,hwmods: Shall contain the TI interconnect module name if needed + by the SoC + + +Example: + gpu: gpu@56000000 { + compatible = "ti,omap4-gpu"; + reg = <0x56000000 0x10000>; + interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>; + ti,hwmods = "gpu"; + }; diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi --- a/arch/arm/boot/dts/omap4.dtsi +++ b/arch/arm/boot/dts/omap4.dtsi @@ -1086,6 +1086,13 @@ status = "disabled"; }; + gpu: gpu@56000000 { + compatible = "ti,omap4-gpu"; + reg = <0x56000000 0x10000>; + interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>; + ti,hwmods = "gpu"; + }; + dss: dss@58000000 { compatible = "ti,omap4-dss"; reg = <0x58000000 0x80>;
On omap4 we're missing the PowerVR SGX GPU node with it's related "ti,hwmods" property that the SoC interconnect code needs. Note that this will only show up as a bug with "doesn't have mpu register target base" boot errors when the legacy platform data is removed. Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> Signed-off-by: Tony Lindgren <tony@atomide.com> --- .../devicetree/bindings/gpu/ti-powervr-sgx.txt | 34 ++++++++++++++++++++++ arch/arm/boot/dts/omap4.dtsi | 7 +++++ 2 files changed, 41 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpu/ti-powervr-sgx.txt