[PATCH 1/3] drm/etnaviv: add generic compatible for the GPU subsystem node

[PATCH 1/3] drm/etnaviv: add generic compatible for the GPU subsystem node

[Lucas Stach]

With different SoCs gaining support for etnaviv it doesn't make much sense
to add specific compatibles for the generic GPU subsystem node, which is
only used to find all GPU core nodes.

Add a generic compatible, that can be used by all new implementations.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 1 +
 drivers/gpu/drm/etnaviv/etnaviv_drv.c                             | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
index 05176f1ae108..c6f4e023c34a 100644
--- a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
+++ b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
@@ -8,6 +8,7 @@ Required properties:
 - compatible: Should be one of
     "fsl,imx-gpu-subsystem"
     "marvell,dove-gpu-subsystem"
+    "vivante,gpu-subsystem"
 - cores: Should contain a list of phandles pointing to Vivante GPU devices
 
 example:
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 491eddf9b150..c9fb94f6b976 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -694,6 +694,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 = "vivante,gpu-subsystem" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] dt-bindings: etnaviv: change clocks to be optional

[Lucas Stach]

While the clocks were documented as required, the driver always treated them
as optional and there are existing Marvell Dove DTs, which would break if
changed to required. Accept reality and document the clocks as optional.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 .../devicetree/bindings/display/etnaviv/etnaviv-drm.txt        | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
index c6f4e023c34a..f28aa5735f4f 100644
--- a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
+++ b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
@@ -30,16 +30,16 @@ Required properties:
 - reg: should be register base and length as documented in the
   datasheet
 - interrupts: Should contain the cores interrupt line
+
+Optional properties:
+- power-domains: a power domain consumer specifier according to
+  Documentation/devicetree/bindings/power/power_domain.txt
 - clocks: should contain one clock for entry in clock-names
   see Documentation/devicetree/bindings/clock/clock-bindings.txt
 - clock-names:
    - "bus":    AXI/register clock
    - "core":   GPU core clock
-   - "shader": Shader clock (only required if GPU has feature PIPE_3D)
-
-Optional properties:
-- power-domains: a power domain consumer specifier according to
-  Documentation/devicetree/bindings/power/power_domain.txt
+   - "shader": Shader clock (only if GPU has feature PIPE_3D)
 
 example:
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] dt-bindings: etnaviv: change clocks to be optional

[Rob Herring]

On Fri, Jan 19, 2018 at 6:06 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> While the clocks were documented as required, the driver always treated them
> as optional and there are existing Marvell Dove DTs, which would break if
> changed to required. Accept reality and document the clocks as optional.

The fact that clocks are optional for a driver doesn't mean they are
optional for the binding. Now they could be optional because you have
power-domains and all clock management is done within the power domain
(though that too is driver specifics leaking into the binding because
power domain has come to mean power management domain).

>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  .../devicetree/bindings/display/etnaviv/etnaviv-drm.txt        | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
> index c6f4e023c34a..f28aa5735f4f 100644
> --- a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
> +++ b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
> @@ -30,16 +30,16 @@ Required properties:
>  - reg: should be register base and length as documented in the
>    datasheet
>  - interrupts: Should contain the cores interrupt line
> +
> +Optional properties:
> +- power-domains: a power domain consumer specifier according to
> +  Documentation/devicetree/bindings/power/power_domain.txt
>  - clocks: should contain one clock for entry in clock-names
>    see Documentation/devicetree/bindings/clock/clock-bindings.txt
>  - clock-names:
>     - "bus":    AXI/register clock
>     - "core":   GPU core clock
> -   - "shader": Shader clock (only required if GPU has feature PIPE_3D)
> -
> -Optional properties:
> -- power-domains: a power domain consumer specifier according to
> -  Documentation/devicetree/bindings/power/power_domain.txt
> +   - "shader": Shader clock (only if GPU has feature PIPE_3D)

"required" is still appropriate here because it is "if you have
optional clocks, then it is required if gpu has PIPE_3D").

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] dt-bindings: etnaviv: change clocks to be optional

[Lucas Stach]

Am Montag, den 22.01.2018, 10:28 -0600 schrieb Rob Herring:
> On Fri, Jan 19, 2018 at 6:06 AM, Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> wrote:
> > While the clocks were documented as required, the driver always
> > treated them
> > as optional and there are existing Marvell Dove DTs, which would
> > break if
> > changed to required. Accept reality and document the clocks as
> > optional.
> 
> The fact that clocks are optional for a driver doesn't mean they are
> optional for the binding. Now they could be optional because you have
> power-domains and all clock management is done within the power
> domain
> (though that too is driver specifics leaking into the binding because
> power domain has come to mean power management domain).

So you prefer the binding to stay as-is and accept the Dove DTs as non-
compliant to the bindings, but still okay? That's fine with me and I'll
drop that patch in this case.

Regards,
Lucas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] dt-bindings: etnaviv: add slave interface clock

[Lucas Stach]

Newer GPU cores added a new clock input, which allows to gate the slave (AHB)
interface independently from other parts of the GPU. Add it to the supported
clocks.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
index f28aa5735f4f..5daee0a3da45 100644
--- a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
+++ b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
@@ -37,7 +37,8 @@ Optional properties:
 - clocks: should contain one clock for entry in clock-names
   see Documentation/devicetree/bindings/clock/clock-bindings.txt
 - clock-names:
-   - "bus":    AXI/register clock
+   - "bus":    AXI/master interface clock
+   - "reg":    AHB/slave interface clock
    - "core":   GPU core clock
    - "shader": Shader clock (only if GPU has feature PIPE_3D)
 
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] drm/etnaviv: add generic compatible for the GPU subsystem node

[Rob Herring]

On Fri, Jan 19, 2018 at 01:06:32PM +0100, Lucas Stach wrote:
> With different SoCs gaining support for etnaviv it doesn't make much sense
> to add specific compatibles for the generic GPU subsystem node, which is
> only used to find all GPU core nodes.

How many other SoC families? Is it really more than a couple? The 
strings are already pretty generic. Plus the real GPU nodes already have 
completely generic compatibles. If you ever need to do something SoC 
specific, that leaves you with looking at the top-level compatible or 
SOL.

> 
> Add a generic compatible, that can be used by all new implementations.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt | 1 +
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c                             | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
> index 05176f1ae108..c6f4e023c34a 100644
> --- a/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
> +++ b/Documentation/devicetree/bindings/display/etnaviv/etnaviv-drm.txt
> @@ -8,6 +8,7 @@ Required properties:
>  - compatible: Should be one of
>      "fsl,imx-gpu-subsystem"
>      "marvell,dove-gpu-subsystem"
> +    "vivante,gpu-subsystem"

I'd prefer to get rid of this fake node altogether. It exists 
entirely because somewhere in the stack wants 2D and 3D gpus 
exposed as a single DRM device. That's not really my problem 
from a DT perspective. 

Something like the patch below would solve the problem. It does break 
module auto loading, but that could be fixed by moving the device 
creation to the platforms (sometimes you just need platform specific 
code).

Note, I probably don't have node ref counting right and the 
of_device_is_available checking should really be in the core code.

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c 
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 491eddf9b150..fc62827055a8 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -662,16 +662,11 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
 
 	if (node) {
 		struct device_node *core_node;
-		int i;
 
-		for (i = 0; ; i++) {
-			core_node = of_parse_phandle(node, "cores", i);
-			if (!core_node)
-				break;
-
-			drm_of_component_match_add(&pdev->dev, &match,
-						   compare_of, core_node);
-			of_node_put(core_node);
+		for_each_compatible_node(core_node, NULL, "vivante,gc") {
+			if (of_device_is_available(core_node))
+				drm_of_component_match_add(&pdev->dev, &match,
+							   compare_of, core_node);
 		}
 	} else if (dev->platform_data) {
 		char **names = dev->platform_data;
@@ -691,28 +686,27 @@ static int etnaviv_pdev_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id dt_match[] = {
-	{ .compatible = "fsl,imx-gpu-subsystem" },
-	{ .compatible = "marvell,dove-gpu-subsystem" },
-	{}
-};
-MODULE_DEVICE_TABLE(of, dt_match);
-
 static struct platform_driver etnaviv_platform_driver = {
 	.probe      = etnaviv_pdev_probe,
 	.remove     = etnaviv_pdev_remove,
 	.driver     = {
 		.name   = "etnaviv",
-		.of_match_table = dt_match,
 	},
 };
 
 static int __init etnaviv_init(void)
 {
 	int ret;
+	struct device_node *node;
 
 	etnaviv_validate_init();
 
+	node = of_find_compatible_node(NULL, "vivante,gc");
+	if (node && of_device_is_available(node)) {
+		platform_device_register_simple("etnaviv", -1, NULL, 0);
+		of_node_put(node);
+	}
+
 	ret = platform_driver_register(&etnaviv_gpu_driver);
 	if (ret != 0)
 		return ret;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel