Skip to contentSkip to conversation

drm/etnaviv: add binding for the gc320 found in ti socs

RFC New Robert Nelson 6 participants 25 messages Nov 18, 2016 – Nov 20, 2016

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

RFC 1/6 drm/etnaviv: add binding for the gc320 found in ti socs

CC Julien <[email protected]>
CC Christian Gmeiner <[email protected]>
CC Russell King <[email protected]>
CC Lucas Stach <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 1 +
 drivers/gpu/drm/etnaviv/etnaviv_drv.c                             | 1 +
 2 files changed, 2 insertions(+)
Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt +1 -0
index ed5e0a7..9fa259d 100644
@@ -8,6 +8,7 @@ Required properties:
 - compatible: Should be one of
     "fsl,imx-gpu-subsystem"
     "marvell,dove-gpu-subsystem"
+    "ti,gc320-gpu-subsystem"
 - cores: Should contain a list of phandles pointing to Vivante GPU devices
 
 example:
drivers/gpu/drm/etnaviv/etnaviv_drv.c +1 -0
index a6799b0..ce51270 100644
@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev)
 static const struct of_device_id dt_match[] = {
 	{ .compatible = "fsl,imx-gpu-subsystem" },
 	{ .compatible = "marvell,dove-gpu-subsystem" },
+	{ .compatible = "ti,gc320-gpu-subsystem" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
Signature · 1 line
2.10.2
#1 lore raw

RFC 1/6 drm/etnaviv: add binding for the gc320 found in ti socs

Sorry, messed up the cover-header, patches 3 & 4 in this series where cherry picked form ti's v4.4.x tree:

http://git.ti.com/cgit/cgit.cgi/ti-linux-kernel/ti-linux-kernel.git/commit/arch/arm/boot/dts/dra7.dtsi?h=ti-linux-4.4.y&id=8067f5a5619ce45657d3729ab3adb9e5b1294f0d

http://git.ti.com/cgit/cgit.cgi/ti-linux-kernel/ti-linux-kernel.git/commit/Documentation/devicetree/bindings/gpu/ti-bb2d.txt?h=ti-linux-4.4.y&id=ddadad0828f8e5ad7e89b11dd243249228ff2997

"ti,gc320-gpu-subsystem" seems like the best middle ground option, the gc320 is found on one omap4770, omap5, dra7..

"ti,omap-gpu-subsystem" might clash with the naming for the sgx544 3D core found on these devices, then some day, ti could use "ti,sgx<num>-gpu-subsystem"

For BeagleBoard.org & BeagleBoard-x15 users we have this working in the updated images.

For people looking to build the packages, i have it up here:

https://gist.github.com/RobertCNelson/fc6d07157b0fcc13b9c28c5832fdc74b

Regards,

Quoted 40 lines · Robert Nelson
CC Julien <[email protected]>
CC Christian Gmeiner <[email protected]>
CC Russell King <[email protected]>
CC Lucas Stach <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 1 +
 drivers/gpu/drm/etnaviv/etnaviv_drv.c                             | 1 +
 2 files changed, 2 insertions(+)
Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt +1 -0
index ed5e0a7..9fa259d 100644
@@ -8,6 +8,7 @@ Required properties:
 - compatible: Should be one of
     "fsl,imx-gpu-subsystem"
     "marvell,dove-gpu-subsystem"
+    "ti,gc320-gpu-subsystem"
 - cores: Should contain a list of phandles pointing to Vivante GPU devices
 
 example:
drivers/gpu/drm/etnaviv/etnaviv_drv.c +1 -1
index a6799b0..ce51270 100644
@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev)
 static const struct of_device_id dt_match[] = {
        { .compatible = "fsl,imx-gpu-subsystem" },
        { .compatible = "marvell,dove-gpu-subsystem" },
+       { .compatible = "ti,gc320-gpu-subsystem" },
        {}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
--

2.10.2

Signature · 2 lines
Robert Nelson
https://rcn-ee.com/
#2 lore raw

RFC 2/6 drm/etnaviv: allow building etnaviv on omap devices

In reply to Robert Nelson
CC Christian Gmeiner <[email protected]>
CC Russell King <[email protected]>
CC Lucas Stach <[email protected]>
---
 drivers/gpu/drm/etnaviv/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/etnaviv/Kconfig +1 -1
index 2cde7a5..b776f41 100644
@@ -2,7 +2,7 @@
 config DRM_ETNAVIV
 	tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
 	depends on DRM
-	depends on ARCH_MXC || ARCH_DOVE
+	depends on ARCH_MXC || ARCH_DOVE || ARCH_OMAP2PLUS
 	select SHMEM
 	select TMPFS
 	select IOMMU_API
Signature · 1 line
2.10.2
#3 lore raw

RFC 2/6 drm/etnaviv: allow building etnaviv on omap devices

On 11/17/2016 08:44 PM, Robert Nelson wrote:

Could we write at least a oneline commit message? :)

Quoted 22 lines
CC Christian Gmeiner <[email protected]>
CC Russell King <[email protected]>
CC Lucas Stach <[email protected]>
---
 drivers/gpu/drm/etnaviv/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/etnaviv/Kconfig +1 -1
index 2cde7a5..b776f41 100644
@@ -2,7 +2,7 @@
 config DRM_ETNAVIV
 	tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
 	depends on DRM
-	depends on ARCH_MXC || ARCH_DOVE
+	depends on ARCH_MXC || ARCH_DOVE || ARCH_OMAP2PLUS
 	select SHMEM
 	select TMPFS
 	select IOMMU_API
Signature · 2 lines
Regards,
Nishanth Menon
maintainer
#4 lore raw

RFC 2/6 drm/etnaviv: allow building etnaviv on omap devices

In reply to Robert Nelson
Quoted 13 lines · Robert Nelson
CC Christian Gmeiner <[email protected]>
CC Russell King <[email protected]>
CC Lucas Stach <[email protected]>
---
 drivers/gpu/drm/etnaviv/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/etnaviv/Kconfig +0 -0
index 2cde7a5..b776f41 100644

drivers/gpu/drm/etnaviv/Kconfig line 5

Quoted 5 lines
drivers/gpu/drm/etnaviv/Kconfig +1 -1
@@ -2,7 +2,7 @@
 config DRM_ETNAVIV
 	tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
 	depends on DRM
-	depends on ARCH_MXC || ARCH_DOVE
+	depends on ARCH_MXC || ARCH_DOVE || ARCH_OMAP2PLUS

Why not just drop this line. Then it will get better build testing, too.

Quoted 10 lines
drivers/gpu/drm/etnaviv/Kconfig +0 -0
 	select SHMEM
 	select TMPFS
 	select IOMMU_API
Signature · 6 lines
2.10.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#5 lore raw

RFC 2/6 drm/etnaviv: allow building etnaviv on omap devices

Quoted 22 lines · Rob Herring
Quoted 13 lines · Robert Nelson
CC Christian Gmeiner <[email protected]>
CC Russell King <[email protected]>
CC Lucas Stach <[email protected]>
---
 drivers/gpu/drm/etnaviv/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/etnaviv/Kconfig +0 -0
index 2cde7a5..b776f41 100644

drivers/gpu/drm/etnaviv/Kconfig line 5

Quoted 5 lines
drivers/gpu/drm/etnaviv/Kconfig +1 -1
@@ -2,7 +2,7 @@
 config DRM_ETNAVIV
 	tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
 	depends on DRM
-	depends on ARCH_MXC || ARCH_DOVE
+	depends on ARCH_MXC || ARCH_DOVE || ARCH_OMAP2PLUS

Why not just drop this line. Then it will get better build testing, too.

Quoted 8 lines

drivers/gpu/drm/etnaviv/Kconfig line 8

Quoted 3 lines
drivers/gpu/drm/etnaviv/Kconfig +0 -0
 	select SHMEM
 	select TMPFS
 	select IOMMU_API

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

...building with ARCH=x86_64

            ^
drivers/gpu/drm/etnaviv/etnaviv_gpu.c: In function ?etnaviv_gpu_init?:
drivers/gpu/drm/etnaviv/etnaviv_gpu.c:633:18: error: ?PHYS_OFFSET? undeclared (first use in this function)
   if (dma_mask < PHYS_OFFSET + SZ_2G)
                  ^

drivers/gpu/drm/etnaviv/etnaviv_gpu.c:633:18: note: each undeclared identifier is reported only once for each function it appears in make[4]: *** [scripts/Makefile.build:290: drivers/gpu/drm/etnaviv/etnaviv_gpu.o] Error 1
make[3]: *** [scripts/Makefile.build:440: drivers/gpu/drm/etnaviv] Error 2
make[2]: *** [scripts/Makefile.build:440: drivers/gpu/drm] Error 2
make[1]: *** [scripts/Makefile.build:440: drivers/gpu] Error 2
make: *** [Makefile:968: drivers] Error 2

...looks like this snippit is the only (compile time) problem:

622 /*

 623          * Set the GPU linear window to be@the end of the DMA window, where
 624          * the CMA area is likely to reside. This ensures that we are able to
 625          * map the command buffers while having the linear window overlap as
 626          * much RAM as possible, so we can optimize mappings for other buffers.
 627          *
 628          * For 3D cores only do this if MC2.0 is present, as with MC1.0 it leads
 629          * to different views of the memory on the individual engines.
 630          */

631 if (!(gpu->identity.features & chipFeatures_PIPE_3D) ||
632 (gpu->identity.minor_features0 & chipMinorFeatures0_MC20)) {
633 u32 dma_mask = (u32)dma_get_required_mask(gpu->dev);
634 if (dma_mask < PHYS_OFFSET + SZ_2G)
635 gpu->memory_base = PHYS_OFFSET;
636 else
637 gpu->memory_base = dma_mask - SZ_2G + 1;
638 }

Joshua

#6 lore raw

RFC 2/6 drm/etnaviv: allow building etnaviv on omap devices

Quoted 31 lines · Joshua Clayton
Quoted 22 lines · Rob Herring
Quoted 13 lines · Robert Nelson
CC Christian Gmeiner <[email protected]>
CC Russell King <[email protected]>
CC Lucas Stach <[email protected]>
---
 drivers/gpu/drm/etnaviv/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/etnaviv/Kconfig +0 -0
index 2cde7a5..b776f41 100644

drivers/gpu/drm/etnaviv/Kconfig line 5

Quoted 5 lines
drivers/gpu/drm/etnaviv/Kconfig +1 -1
@@ -2,7 +2,7 @@
 config DRM_ETNAVIV
 	tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
 	depends on DRM
-	depends on ARCH_MXC || ARCH_DOVE
+	depends on ARCH_MXC || ARCH_DOVE || ARCH_OMAP2PLUS

Why not just drop this line. Then it will get better build testing, too.

Quoted 8 lines

drivers/gpu/drm/etnaviv/Kconfig line 8

Quoted 3 lines
drivers/gpu/drm/etnaviv/Kconfig +0 -0
 	select SHMEM
 	select TMPFS
 	select IOMMU_API

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 7 lines

...building with ARCH=x86_64

            ^
drivers/gpu/drm/etnaviv/etnaviv_gpu.c: In function ?etnaviv_gpu_init?:
drivers/gpu/drm/etnaviv/etnaviv_gpu.c:633:18: error: ?PHYS_OFFSET? undeclared (first use in this function)
   if (dma_mask < PHYS_OFFSET + SZ_2G)
                  ^

drivers/gpu/drm/etnaviv/etnaviv_gpu.c:633:18: note: each undeclared identifier is reported only once for each function it appears in

... and that's why we don't drop the dependency, although changing it to "depends on ARM" will open it up to wider build testing on other ARM platforms, I don't think that gains very much.

The issue is that the command buffers must be located within the first 2GB of GPU virtual address space, and that space can only be translated (offset). What the code here is trying to do is to locate the 2GB of virtual address space such that it overlaps the CMA region if RAM is bigger than 2GB, or covers all of the RAM if not.

For that, we need to know where physical RAM starts.

I guess we could replace PHYS_OFFSET with virt_to_phys(PAGE_OFFSET) here instead, which would be more arch-portable - although that would be assuming there's no (system?) IOMMU between memory and the GPU.

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
#7 lore raw

RFC 3/6 Documentation: dt: add bindings for ti bb2d

In reply to Robert Nelson

From: Gowtham Tammana <[email protected]>

Add documentation for device tree bindings description for 2D GPU blitter module present in Texas Instruments family of SoCs.

---
 Documentation/devicetree/bindings/gpu/ti-bb2d.txt | 27 +++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/ti-bb2d.txt
Documentation/devicetree/bindings/gpu/ti-bb2d.txt new file +27 -0
new file mode 100644
index 0000000..af47488
@@ -0,0 +1,27 @@
+* Texas Instruments BB2D blitter module
+
+This binding describes the 2D BitBlit (BB2D) graphics accelerator
+subsystem based on the GC320 core from Vivante Corporation available
+in Texas Instruments SoCs.
+
+Required properties:
+  - compatible: value should take the following format:
+        "ti,<soc>-bb2d", "vivante,<gpuversion>"
+    accepted values:
+     (a) "ti,dra7-bb2d", "vivante,gc320" for TI DRA7xx / AM57x
+
+  - reg : base address and length of BB2D IP registers
+  - interrupts : BB2D interrupt line number
+  - ti,hwmods : name of the hwmod associated with BB2D module
+  - clocks : handle to BB2D functional clock
+  - clock-names : fclk
+
+Example for DRA7x SoC:
+        bb2d: bb2d at 59000000 {
+            compatible = "ti,dra7-bb2d", "vivante,gc320";
+            reg = <0x59000000 0x0700>;
+            interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+            ti,hwmods = "bb2d";
+            clocks = <&dpll_core_h24x2_ck>;
+            clock-names = "fclk";
+        };
Signature · 1 line
2.10.2
#8 lore raw

RFC 3/6 Documentation: dt: add bindings for ti bb2d

Quoted 8 lines · Robert Nelson

From: Gowtham Tammana <[email protected]>

Add documentation for device tree bindings description for 2D GPU blitter module present in Texas Instruments family of SoCs.

---

Might want to point at the source of the patch -> just for the record, this came from TI vendor kernel..

Quoted 38 lines
 Documentation/devicetree/bindings/gpu/ti-bb2d.txt | 27 +++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpu/ti-bb2d.txt
Documentation/devicetree/bindings/gpu/ti-bb2d.txt new file +27 -0
new file mode 100644
index 0000000..af47488
@@ -0,0 +1,27 @@
+* Texas Instruments BB2D blitter module
+
+This binding describes the 2D BitBlit (BB2D) graphics accelerator
+subsystem based on the GC320 core from Vivante Corporation available
+in Texas Instruments SoCs.
+
+Required properties:
+  - compatible: value should take the following format:
+        "ti,<soc>-bb2d", "vivante,<gpuversion>"
+    accepted values:
+     (a) "ti,dra7-bb2d", "vivante,gc320" for TI DRA7xx / AM57x
+
+  - reg : base address and length of BB2D IP registers
+  - interrupts : BB2D interrupt line number
+  - ti,hwmods : name of the hwmod associated with BB2D module
+  - clocks : handle to BB2D functional clock
+  - clock-names : fclk
+
+Example for DRA7x SoC:
+        bb2d: bb2d at 59000000 {
+            compatible = "ti,dra7-bb2d", "vivante,gc320";
+            reg = <0x59000000 0x0700>;
+            interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+            ti,hwmods = "bb2d";
+            clocks = <&dpll_core_h24x2_ck>;
+            clock-names = "fclk";
+        };
Signature · 2 lines
Regards,
Nishanth Menon
#9 lore raw

RFC 4/6 ARM: dts: dra7: add entry for bb2d module

In reply to Robert Nelson

From: Gowtham Tammana <[email protected]>

BB2D entry is added to the dts file. Crossbar index number is used for interrupt mapping.

---
 arch/arm/boot/dts/dra7.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)
arch/arm/boot/dts/dra7.dtsi +10 -0
index addb753..43488b6 100644
@@ -959,6 +959,16 @@
 			ti,hwmods = "dmm";
 		};
 
+		bb2d: bb2d at 59000000 {
+			compatible = "ti,dra7-bb2d";
+			reg = <0x59000000 0x0700>;
+			interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "bb2d";
+			clocks = <&dpll_core_h24x2_ck>;
+			clock-names = "fclk";
+			status = "disabled";
+		};
+
 		i2c1: i2c at 48070000 {
 			compatible = "ti,omap4-i2c";
 			reg = <0x48070000 0x100>;
Signature · 1 line
2.10.2
maintainer
#10 lore raw

RFC 4/6 ARM: dts: dra7: add entry for bb2d module

Am Donnerstag, den 17.11.2016, 20:44 -0600 schrieb Robert Nelson:

Quoted 16 lines

From: Gowtham Tammana <[email protected]>

BB2D entry is added to the dts file. Crossbar index number is used for interrupt mapping.

---
 arch/arm/boot/dts/dra7.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)
arch/arm/boot/dts/dra7.dtsi +0 -0
index addb753..43488b6 100644

arch/arm/boot/dts/dra7.dtsi line 968

Quoted 10 lines
arch/arm/boot/dts/dra7.dtsi +7 -0
@@ -959,6 +959,16 @@
 			ti,hwmods = "dmm";
 		};
 
+		bb2d: bb2d at 59000000 {
+			compatible = "ti,dra7-bb2d";
+			reg = <0x59000000 0x0700>;
+			interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "bb2d";
+			clocks = <&dpll_core_h24x2_ck>;
+			clock-names = "fclk";

"fclk" is not an accepted clock name for the etnaviv driver. It supports up to 3 clocks: "bus", "core" and "shader". If there is only one clock required in your design it would probably be the "core" clock.

Regards,
Lucas

#11 lore raw

RFC 4/6 ARM: dts: dra7: add entry for bb2d module

Quoted 20 lines · Lucas Stach

Am Donnerstag, den 17.11.2016, 20:44 -0600 schrieb Robert Nelson:

Quoted 19 lines

From: Gowtham Tammana <[email protected]>

BB2D entry is added to the dts file. Crossbar index number is used for interrupt mapping.

---
 arch/arm/boot/dts/dra7.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)
arch/arm/boot/dts/dra7.dtsi +0 -0
index addb753..43488b6 100644
@@ -959,6 +959,16 @@
 			ti,hwmods = "dmm";
 		};
 
Quoted 11 lines

arch/arm/boot/dts/dra7.dtsi line 968

Quoted 7 lines
arch/arm/boot/dts/dra7.dtsi +7 -0
+		bb2d: bb2d at 59000000 {
+			compatible = "ti,dra7-bb2d";
+			reg = <0x59000000 0x0700>;
+			interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+			ti,hwmods = "bb2d";
+			clocks = <&dpll_core_h24x2_ck>;
+			clock-names = "fclk";

"fclk" is not an accepted clock name for the etnaviv driver. It supports up to 3 clocks: "bus", "core" and "shader". If there is only one clock required in your design it would probably be the "core" clock.

fclk is used as a standard for all hwmod (SoC level control library that existed prior to ARM DT-fication, and now slowly being converted to DT). I think the core and bus clock is the same here.. I wonder if something duplicated like the following will fly?

clocks = <&dpll_core_h24x2_ck>, <&dpll_core_h24x2_ck> ; clock-names = "fclk", "core";

It might have been better if hwmod clocks were'nt assumed from clock-names.. instead it could have been something like:
ti,hwmod-fck-clk-names = "core"; ti,hwmod-ick-clk-names = "bus"; ti,hwmod-ock-clk-names = "shader";
clocks = <&dpll_core_h24x2_ck>, <&clk2>, <&clk3>; clock-names = "core", "bus", "shader";

Tero: ?

Signature · 2 lines
Regards,
Nishanth Menon
#12 lore raw

RFC 5/6 ARM: dts: dra7: add vivante for bb2d module

In reply to Robert Nelson
CC Julien <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 arch/arm/boot/dts/dra7.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/dra7.dtsi +1 -1
index 43488b6..22bd0a5 100644
@@ -960,7 +960,7 @@
 		};
 
 		bb2d: bb2d at 59000000 {
-			compatible = "ti,dra7-bb2d";
+			compatible = "ti,dra7-bb2d","vivante,gc";
 			reg = <0x59000000 0x0700>;
 			interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
 			ti,hwmods = "bb2d";
Signature · 1 line
2.10.2
maintainer
#13 lore raw

RFC 5/6 ARM: dts: dra7: add vivante for bb2d module

Am Donnerstag, den 17.11.2016, 20:44 -0600 schrieb Robert Nelson:

Quoted 14 lines
CC Julien <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 arch/arm/boot/dts/dra7.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/dra7.dtsi +0 -0
index 43488b6..22bd0a5 100644

arch/arm/boot/dts/dra7.dtsi line 963

Quoted 5 lines
arch/arm/boot/dts/dra7.dtsi +1 -1
@@ -960,7 +960,7 @@
 		};
 
 		bb2d: bb2d at 59000000 {
-			compatible = "ti,dra7-bb2d";
+			compatible = "ti,dra7-bb2d","vivante,gc";

This is what the driver expects as a compatible, but it's not consistent with the DT documentation patch you sent. As the driver can work out the HW version from the identification registers I would say fix your DT binding to only require "vivante,gc"

Regards,
Lucas

#14 lore raw

RFC 6/6 ARM: dts: am57xx-beagle-x15-common: enable etnaviv

In reply to Robert Nelson
CC Julien <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +11 -0
index 6df7829..3bc47be 100644
@@ -97,6 +97,12 @@
 		#cooling-cells = <2>;
 	};
 
+	gpu-subsystem {
+		compatible = "ti,gc320-gpu-subsystem";
+		cores = <&bb2d>;
+		status = "okay";
+	};
+
 	hdmi0: connector {
 		compatible = "hdmi-connector";
 		label = "hdmi";
@@ -190,6 +196,11 @@
 		>;
 	};
 };
+
+&bb2d {
+	status = "okay";
+};
+
 &i2c1 {
 	status = "okay";
 	clock-frequency = <400000>;
Signature · 1 line
2.10.2
#15 lore raw

RFC 6/6 ARM: dts: am57xx-beagle-x15-common: enable etnaviv

On 11/17/2016 08:44 PM, Robert Nelson wrote:
again.. a short commit message at least please? :)

Quoted 14 lines
CC Julien <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +0 -0
index 6df7829..3bc47be 100644

arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi line 100

Quoted 4 lines
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +1 -0
@@ -97,6 +97,12 @@
 		#cooling-cells = <2>;
 	};
 
+	gpu-subsystem {

A) do we want to make things clear that this is gpu subsystem for gc320?
B) How about other platforms that could equally reuse?

Quoted 21 lines
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +10 -0
+		compatible = "ti,gc320-gpu-subsystem";
+		cores = <&bb2d>;
+		status = "okay";
+	};
+
 	hdmi0: connector {
 		compatible = "hdmi-connector";
 		label = "hdmi";
@@ -190,6 +196,11 @@
 		>;
 	};
 };
+
+&bb2d {
+	status = "okay";
+};
+
 &i2c1 {
 	status = "okay";
 	clock-frequency = <400000>;
Signature · 2 lines
Regards,
Nishanth Menon
#16 lore raw

RFC 6/6 ARM: dts: am57xx-beagle-x15-common: enable etnaviv

Quoted 2 lines · Nishanth Menon

On 11/17/2016 08:44 PM, Robert Nelson wrote:
again.. a short commit message at least please? :)

yeah, i'll fix all those. ;)

Quoted 19 lines
Quoted 19 lines
CC Julien <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +0 -0
index 6df7829..3bc47be 100644
@@ -97,6 +97,12 @@
                #cooling-cells = <2>;
        };
 
Quoted 4 lines

arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi line 100

Quoted 1 line
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +1 -0
+       gpu-subsystem {

A) do we want to make things clear that this is gpu subsystem for gc320?
B) How about other platforms that could equally reuse?

so the 'gpu-subsystem' comes from etnaviv:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt?id=refs/tags/v4.9-rc5

For a generic name, it's currently only tied to the etnaviv driver:

gpu-subsystem {
compatible = "fsl,imx-gpu-subsystem";
cores = <&gpu_2d>, <&gpu_3d>;
};

it would make sense to make that more generic, so you could tie a 2d vivante and a imgtec/sgx 3d core.. <sad laugh> but that would require adding a imgtec/sgx driver/bindings to the kernel mainline... </sad laugh>

Quoted 14 lines
Quoted 14 lines
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +6 -0
+               compatible = "ti,gc320-gpu-subsystem";
+               cores = <&bb2d>;
+               status = "okay";
+       };
+
        hdmi0: connector {
                compatible = "hdmi-connector";
                label = "hdmi";
@@ -190,6 +196,11 @@
                >;
        };
 };
+
Quoted 8 lines

arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi line 206

Quoted 8 lines
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +4 -0
+&bb2d {
+       status = "okay";
+};
+
 &i2c1 {
        status = "okay";
        clock-frequency = <400000>;

Regards,

Signature · 2 lines
Robert Nelson
https://rcn-ee.com/
#17 lore raw

RFC 6/6 ARM: dts: am57xx-beagle-x15-common: enable etnaviv

Quoted 30 lines · Robert Nelson
Quoted 2 lines · Nishanth Menon

On 11/17/2016 08:44 PM, Robert Nelson wrote:
again.. a short commit message at least please? :)

yeah, i'll fix all those. ;)

Quoted 19 lines
Quoted 19 lines
CC Julien <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +0 -0
index 6df7829..3bc47be 100644
@@ -97,6 +97,12 @@
                #cooling-cells = <2>;
        };
 
Quoted 4 lines

arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi line 100

Quoted 1 line
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +1 -0
+       gpu-subsystem {

A) do we want to make things clear that this is gpu subsystem for gc320?
B) How about other platforms that could equally reuse?

Quoted 6 lines

so the 'gpu-subsystem' comes from etnaviv:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt?id=refs/tags/v4.9-rc5

For a generic name, it's currently only tied to the etnaviv driver:

I was only complaining about "gpu-subsystem {", not the compatible. it is not the only gpu subsystem on the SoC. either "gpu-subsystem0 {" or something like gpu-subsystem-gc320 might be helpful to clarify.

Quoted 10 lines

gpu-subsystem {
compatible = "fsl,imx-gpu-subsystem";
cores = <&gpu_2d>, <&gpu_3d>;
};

it would make sense to make that more generic, so you could tie a 2d vivante and a imgtec/sgx 3d core.. <sad laugh> but that would require adding a imgtec/sgx driver/bindings to the kernel mainline... </sad laugh>

I should have clarified... I meant other dra7 devices to reuse the same definitions. this definition is not by any means constrained to EVM - it is a SoC definition, it should be moved to appropriate place (convention for dra7 is to mark them as disabled by default in SoC.dtsi to prevent proliferation of paper spin dtsi and just do "status = okay" in board file to indicate presence in the variation for the board).

Yes - I guess some day there might be a bunch of folks like etnaviv who might make an community driver possible..

Signature · 2 lines
Regards,
Nishanth Menon
#18 lore raw

RFC 6/6 ARM: dts: am57xx-beagle-x15-common: enable etnaviv

Quoted 59 lines · Nishanth Menon
Quoted 35 lines · Robert Nelson
Quoted 3 lines · Nishanth Menon

On 11/17/2016 08:44 PM, Robert Nelson wrote:
again.. a short commit message at least please? :)

yeah, i'll fix all those. ;)

Quoted 19 lines
Quoted 19 lines
CC Julien <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +0 -0
index 6df7829..3bc47be 100644
@@ -97,6 +97,12 @@
                #cooling-cells = <2>;
        };
 
Quoted 5 lines

arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi line 100

Quoted 1 line
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +1 -0
+       gpu-subsystem {

A) do we want to make things clear that this is gpu subsystem for gc320?
B) How about other platforms that could equally reuse?

Quoted 7 lines

so the 'gpu-subsystem' comes from etnaviv:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt?id=refs/tags/v4.9-rc5

For a generic name, it's currently only tied to the etnaviv driver:

I was only complaining about "gpu-subsystem {", not the compatible. it is not the only gpu subsystem on the SoC. either "gpu-subsystem0 {" or something like gpu-subsystem-gc320 might be helpful to clarify.

Quoted 10 lines

gpu-subsystem {
compatible = "fsl,imx-gpu-subsystem";
cores = <&gpu_2d>, <&gpu_3d>;
};

it would make sense to make that more generic, so you could tie a 2d vivante and a imgtec/sgx 3d core.. <sad laugh> but that would require adding a imgtec/sgx driver/bindings to the kernel mainline... </sad laugh>

Quoted 6 lines

I should have clarified... I meant other dra7 devices to reuse the same definitions. this definition is not by any means constrained to EVM - it is a SoC definition, it should be moved to appropriate place (convention for dra7 is to mark them as disabled by default in SoC.dtsi to prevent proliferation of paper spin dtsi and just do "status = okay" in board file to indicate presence in the variation for the board).

Oh yeah, defintely, we can move gpu-subsystem to the base dra7.dtsi, as the whole dra.dtsi family has a gc320 and then the board device tree can enable it via:

&bb2d {
       status = "okay";
};

Regards,

Signature · 2 lines
Robert Nelson
https://rcn-ee.com/
#19 lore raw

RFC 6/6 ARM: dts: am57xx-beagle-x15-common: enable etnaviv

On 11/17/2016 10:26 PM, Robert Nelson wrote:
[...]

Quoted 7 lines

Oh yeah, defintely, we can move gpu-subsystem to the base dra7.dtsi, as the whole dra.dtsi family has a gc320 and then the board device tree can enable it via:

&bb2d {
       status = "okay";
};

Yep, thanks.

Signature · 2 lines
Regards,
Nishanth Menon
maintainer
#20 lore raw

RFC 6/6 ARM: dts: am57xx-beagle-x15-common: enable etnaviv

In reply to Nishanth Menon
Quoted 20 lines · Nishanth Menon

On 11/17/2016 08:44 PM, Robert Nelson wrote:
again.. a short commit message at least please? :)

Quoted 17 lines
CC Julien <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +0 -0
index 6df7829..3bc47be 100644
@@ -97,6 +97,12 @@
 		#cooling-cells = <2>;
 	};
 
Quoted 2 lines

arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi line 100

Quoted 1 line
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +1 -0
+	gpu-subsystem {

A) do we want to make things clear that this is gpu subsystem for gc320?

No.

Quoted 1 line

B) How about other platforms that could equally reuse?

Better yet, get rid of this nonsense. You are defining a wrapper to group 1 nodes. Yes, I know this is what the driver wants, but you should be able to make it deal with this situation and only have a gc320 node.

Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi +5 -0
+		compatible = "ti,gc320-gpu-subsystem";
+		cores = <&bb2d>;
+		status = "okay";
+	};
+
#21 lore raw

RFC 1/6 drm/etnaviv: add binding for the gc320 found in ti socs

In reply to Robert Nelson

On 11/17/2016 08:44 PM, Robert Nelson wrote:

Could we write at least a oneline commit message? :)

Might want to state that since the TI gpu systems are a mixed bunch and certain SoCs may have more than 1 GPU integrated, we indicate clearly the rev here?

Quoted 38 lines
CC Julien <[email protected]>
CC Christian Gmeiner <[email protected]>
CC Russell King <[email protected]>
CC Lucas Stach <[email protected]>
CC Nishanth Menon <[email protected]>
CC Tomi Valkeinen <[email protected]>
CC Tony Lindgren <[email protected]>
---
 Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 1 +
 drivers/gpu/drm/etnaviv/etnaviv_drv.c                             | 1 +
 2 files changed, 2 insertions(+)
Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt +1 -0
index ed5e0a7..9fa259d 100644
@@ -8,6 +8,7 @@ Required properties:
 - compatible: Should be one of
     "fsl,imx-gpu-subsystem"
     "marvell,dove-gpu-subsystem"
+    "ti,gc320-gpu-subsystem"
 - cores: Should contain a list of phandles pointing to Vivante GPU devices
 
 example:
drivers/gpu/drm/etnaviv/etnaviv_drv.c +1 -0
index a6799b0..ce51270 100644
@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev)
 static const struct of_device_id dt_match[] = {
 	{ .compatible = "fsl,imx-gpu-subsystem" },
 	{ .compatible = "marvell,dove-gpu-subsystem" },
+	{ .compatible = "ti,gc320-gpu-subsystem" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
Signature · 2 lines
Regards,
Nishanth Menon
#22 lore raw

RFC 1/6 drm/etnaviv: add binding for the gc320 found in ti socs

Quoted 9 lines · Nishanth Menon

drivers/gpu/drm/etnaviv/etnaviv_drv.c static int etnaviv_pdev_remove(struct platform_device *pdev) · line 656

Quoted 9 lines
drivers/gpu/drm/etnaviv/etnaviv_drv.c +1 -0
index a6799b0..ce51270 100644
@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev)
 static const struct of_device_id dt_match[] = {
 	{ .compatible = "fsl,imx-gpu-subsystem" },
 	{ .compatible = "marvell,dove-gpu-subsystem" },
+	{ .compatible = "ti,gc320-gpu-subsystem" },

We need to get away from this ever-increasing set of compatible strings here, as this is not long-term maintainable.

What we should have is a common compatible which describes that the node is compatible with this driver, and then use SoC specific compatible strings later if we need to (eg, because of some GPU subsystem SoC specifics.)

So, I'd suggest that we update the documentation and add:

"vivante,gc-gpu-subsystem"

as a common compatible now, and if necessary move on to more specific compatibles if we need to later.

Also, I'd strongly suggest that no compatibles should contain the ID number of the GPU core for exactly the same reason - Vivante GPU cores vary according to features, and we don't want to end up with a long list of specific compatibles (eg)

	"ti,gc2000-and-gc320-and-gc355-gpu-subsystem" because TI

decides to integrate a 3d, 2d and VG core.

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
maintainer
#23 lore raw

RFC 1/6 drm/etnaviv: add binding for the gc320 found in ti socs

Am Freitag, den 18.11.2016, 12:13 +0000 schrieb Russell King - ARM Linux:

Quoted 26 lines · Nishanth Menon
Quoted 9 lines

drivers/gpu/drm/etnaviv/etnaviv_drv.c static int etnaviv_pdev_remove(struct platform_device *pdev) · line 656

Quoted 9 lines
drivers/gpu/drm/etnaviv/etnaviv_drv.c +1 -0
index a6799b0..ce51270 100644
@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev)
 static const struct of_device_id dt_match[] = {
 	{ .compatible = "fsl,imx-gpu-subsystem" },
 	{ .compatible = "marvell,dove-gpu-subsystem" },
+	{ .compatible = "ti,gc320-gpu-subsystem" },

We need to get away from this ever-increasing set of compatible strings here, as this is not long-term maintainable.

What we should have is a common compatible which describes that the node is compatible with this driver, and then use SoC specific compatible strings later if we need to (eg, because of some GPU subsystem SoC specifics.)

So, I'd suggest that we update the documentation and add:

"vivante,gc-gpu-subsystem"

as a common compatible now, and if necessary move on to more specific compatibles if we need to later.

Quoted 7 lines

Also, I'd strongly suggest that no compatibles should contain the ID number of the GPU core for exactly the same reason - Vivante GPU cores vary according to features, and we don't want to end up with a long list of specific compatibles (eg)

	"ti,gc2000-and-gc320-and-gc355-gpu-subsystem" because TI

decides to integrate a 3d, 2d and VG core.

All of the above sounds sensible and I would prefer if the patches are reworked to take those things into account.

Regards,
Lucas

#24 lore raw

RFC 1/6 drm/etnaviv: add binding for the gc320 found in ti socs

Quoted 35 lines · Lucas Stach

Am Freitag, den 18.11.2016, 12:13 +0000 schrieb Russell King - ARM Linux:

Quoted 26 lines · Nishanth Menon
Quoted 9 lines

drivers/gpu/drm/etnaviv/etnaviv_drv.c static int etnaviv_pdev_remove(struct platform_device *pdev) · line 656

Quoted 9 lines
drivers/gpu/drm/etnaviv/etnaviv_drv.c +1 -0
index a6799b0..ce51270 100644
@@ -653,6 +653,7 @@ static int etnaviv_pdev_remove(struct platform_device *pdev)
 static const struct of_device_id dt_match[] = {
  { .compatible = "fsl,imx-gpu-subsystem" },
  { .compatible = "marvell,dove-gpu-subsystem" },
+  { .compatible = "ti,gc320-gpu-subsystem" },

We need to get away from this ever-increasing set of compatible strings here, as this is not long-term maintainable.

What we should have is a common compatible which describes that the node is compatible with this driver, and then use SoC specific compatible strings later if we need to (eg, because of some GPU subsystem SoC specifics.)

So, I'd suggest that we update the documentation and add:

"vivante,gc-gpu-subsystem"

as a common compatible now, and if necessary move on to more specific compatibles if we need to later.

Quoted 7 lines

Also, I'd strongly suggest that no compatibles should contain the ID number of the GPU core for exactly the same reason - Vivante GPU cores vary according to features, and we don't want to end up with a long list of specific compatibles (eg)

      "ti,gc2000-and-gc320-and-gc355-gpu-subsystem" because TI

decides to integrate a 3d, 2d and VG core.

Quoted 2 lines

All of the above sounds sensible and I would prefer if the patches are reworked to take those things into account.

Thanks for reviewing guys, the "vivante,gc-gpu-subsystem" will work perfectly fine for me, as the etnaviv driver does a great job at auto detecting the hardware..

Regards,

Signature · 2 lines
Robert Nelson
https://rcn-ee.com/