[PATCH 0/8] ARM: sun8i: a33: Mali improvements

[PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Maxime Ripard]

Hi,

This serie is building on the recently merged bindings for the ARM Mali
Utgard GPU.

The two features that are supported with this serie are DVFS and the fbdev
support. The first one uses devfreq and is pretty standard, the only
addition being the generic OPP mechanism we have, plus some DT and Kconfig
patches.

Running on framebuffer is a bit more tedious, since we need to access the
CMA memory region size and base. This is quite trivial to do as well
through the memory-region bindings, but require to export a few symbols
along the way to make sure our module builds properly.

Let me know what you think,
Maxime

Maxime Ripard (8):
  ARM: sun8i: Fix the mali clock rate
  dt-bindings: gpu: mali: Add optional memory-region
  mm: cma: Export a few symbols
  drm/sun4i: Grab reserved memory region
  ARM: sun8i: a33: Add shared display memory pool
  dt-bindings: gpu: mali: Add optional OPPs
  ARM: sunxi: Select PM_OPP
  ARM: sun8i: a33: Add the Mali OPPs

 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt |  8 ++-
 arch/arm/boot/dts/sun8i-a23-a33.dtsi                      |  2 +-
 arch/arm/boot/dts/sun8i-a33.dtsi                          | 34 ++++++++-
 arch/arm/mach-sunxi/Kconfig                               |  1 +-
 drivers/base/dma-contiguous.c                             |  1 +-
 drivers/gpu/drm/sun4i/sun4i_drv.c                         | 19 ++--
 mm/cma.c                                                  |  2 +-
 7 files changed, 61 insertions(+), 6 deletions(-)

base-commit: a2138ce584d59571dd18a6cf3417cb90be7625d8
-- 
git-series 0.8.11

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/8] ARM: sun8i: Fix the mali clock rate

[Maxime Ripard]

The Mali clock rate was improperly assumed to be 408MHz, while it was
really 384Mhz, 408MHz being the "extreme" frequency, and definitely not
stable.

Switch for the stable, correct frequency for the GPU.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 35008b78d899..8a880ecc4dda 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -495,7 +495,7 @@
 			resets = <&ccu RST_BUS_GPU>;
 
 			assigned-clocks = <&ccu CLK_GPU>;
-			assigned-clock-rates = <408000000>;
+			assigned-clock-rates = <384000000>;
 		};
 
 		gic: interrupt-controller@01c81000 {
-- 
git-series 0.8.11

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/8] dt-bindings: gpu: mali: Add optional memory-region

[Maxime Ripard]

The reserved memory bindings allow us to specify which memory areas our
buffers can be allocated from.

Let's use it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt | 4 ++++
 1 file changed, 4 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
index 476f5ea6c627..88df4a276607 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
@@ -35,6 +35,10 @@ Optional properties:
   - interrupt-names and interrupts:
     * pmu: Power Management Unit interrupt, if implemented in hardware
 
+  - memory-region:
+    Memory region to allocate from, as defined in
+    Documentation/devicetree/bindi/reserved-memory/reserved-memory.txt
+
 Vendor-specific bindings
 ------------------------
 
-- 
git-series 0.8.11

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 3/8] mm: cma: Export a few symbols

[Maxime Ripard]

Modules might want to check their CMA pool size and address for debugging
and / or have additional checks.

The obvious way to do this would be through dev_get_cma_area and
cma_get_base and cma_get_size, that are currently not exported, which
results in a build failure.

Export them to prevent such a failure.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/base/dma-contiguous.c | 1 +
 mm/cma.c                      | 2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index e167a1e1bccb..60f5c2591ccd 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -35,6 +35,7 @@
 #endif
 
 struct cma *dma_contiguous_default_area;
+EXPORT_SYMBOL(dma_contiguous_default_area);
 
 /*
  * Default global CMA area size can be defined in kernel's .config.
diff --git a/mm/cma.c b/mm/cma.c
index c960459eda7e..b50245282a18 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -47,11 +47,13 @@ phys_addr_t cma_get_base(const struct cma *cma)
 {
 	return PFN_PHYS(cma->base_pfn);
 }
+EXPORT_SYMBOL(cma_get_base);
 
 unsigned long cma_get_size(const struct cma *cma)
 {
 	return cma->count << PAGE_SHIFT;
 }
+EXPORT_SYMBOL(cma_get_size);
 
 static unsigned long cma_bitmap_aligned_mask(const struct cma *cma,
 					     int align_order)
-- 
git-series 0.8.11

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 4/8] drm/sun4i: Grab reserved memory region

[Maxime Ripard]

Allow to provide an optional memory region to allocate from for our DRM
driver.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 4ce665349f6b..7ed7a159c9c1 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -12,6 +12,7 @@
 
 #include <linux/component.h>
 #include <linux/of_graph.h>
+#include <linux/of_reserved_mem.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
@@ -133,10 +134,16 @@ static int sun4i_drv_bind(struct device *dev)
 	drm_vblank_init(drm, 1);
 	drm_mode_config_init(drm);
 
+	ret = of_reserved_mem_device_init(dev);
+	if (ret && ret != -ENODEV) {
+		dev_err(drm->dev, "Couldn't claim our memory region\n");
+		goto free_drm;
+	}
+
 	ret = component_bind_all(drm->dev, drm);
 	if (ret) {
 		dev_err(drm->dev, "Couldn't bind all pipelines components\n");
-		goto free_drm;
+		goto free_mem_region;
 	}
 
 	/* Create our layers */
@@ -144,7 +151,7 @@ static int sun4i_drv_bind(struct device *dev)
 	if (IS_ERR(drv->layers)) {
 		dev_err(drm->dev, "Couldn't create the planes\n");
 		ret = PTR_ERR(drv->layers);
-		goto free_drm;
+		goto free_mem_region;
 	}
 
 	/* Create our CRTC */
@@ -152,7 +159,7 @@ static int sun4i_drv_bind(struct device *dev)
 	if (!drv->crtc) {
 		dev_err(drm->dev, "Couldn't create the CRTC\n");
 		ret = -EINVAL;
-		goto free_drm;
+		goto free_mem_region;
 	}
 	drm->irq_enabled = true;
 
@@ -164,7 +171,7 @@ static int sun4i_drv_bind(struct device *dev)
 	if (IS_ERR(drv->fbdev)) {
 		dev_err(drm->dev, "Couldn't create our framebuffer\n");
 		ret = PTR_ERR(drv->fbdev);
-		goto free_drm;
+		goto free_mem_region;
 	}
 
 	/* Enable connectors polling */
@@ -172,10 +179,12 @@ static int sun4i_drv_bind(struct device *dev)
 
 	ret = drm_dev_register(drm, 0);
 	if (ret)
-		goto free_drm;
+		goto free_mem_region;
 
 	return 0;
 
+free_mem_region:
+	of_reserved_mem_device_release(dev);
 free_drm:
 	drm_dev_unref(drm);
 	return ret;
-- 
git-series 0.8.11

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 5/8] ARM: sun8i: a33: Add shared display memory pool

[Maxime Ripard]

The memory buffers might need to be allocated and shared from both the
scanout and the GPU.

Create a memory region reserved for their own usage so that each can
allocate from it, and get the informations on the region that is going to
be used (size and offset).

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a33.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
index 5a9ba43ccb07..043b1b017276 100644
--- a/arch/arm/boot/dts/sun8i-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a33.dtsi
@@ -97,6 +97,7 @@
 	de: display-engine {
 		compatible = "allwinner,sun8i-a33-display-engine";
 		allwinner,pipelines = <&fe0>;
+		memory-region = <&display_pool>;
 		status = "disabled";
 	};
 
@@ -104,6 +105,18 @@
 		reg = <0x40000000 0x80000000>;
 	};
 
+	reserved-memory {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		display_pool: cma {
+			compatible = "shared-dma-pool";
+			size = <0x1000000>;
+			reusable;
+		};
+	};
+
 	soc@01c00000 {
 		tcon0: lcd-controller@01c0c000 {
 			compatible = "allwinner,sun8i-a33-tcon";
@@ -267,6 +280,10 @@
 	compatible = "allwinner,sun8i-a33-ccu";
 };
 
+&mali {
+	memory-region = <&display_pool>;
+};
+
 &pio {
 	compatible = "allwinner,sun8i-a33-pinctrl";
 	interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>,
-- 
git-series 0.8.11

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 6/8] dt-bindings: gpu: mali: Add optional OPPs

[Maxime Ripard]

The operating-points-v2 binding gives a way to provide the OPP of the GPU.
Let's use it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt | 4 ++++
 1 file changed, 4 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
index 88df4a276607..2b6243e730f6 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt
@@ -39,6 +39,10 @@ Optional properties:
     Memory region to allocate from, as defined in
     Documentation/devicetree/bindi/reserved-memory/reserved-memory.txt
 
+  - operating-points-v2:
+    Operating Points for the GPU, as defined in
+    Documentation/devicetree/bindings/opp/opp.txt
+
 Vendor-specific bindings
 ------------------------
 
-- 
git-series 0.8.11

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 7/8] ARM: sunxi: Select PM_OPP

[Maxime Ripard]

Device frequency scaling is implemented through devfreq in the kernel,
which requires CONFIG_PM_OPP.

Let's select it.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/mach-sunxi/Kconfig | 1 +
 1 file changed, 1 insertion(+), 0 deletions(-)

diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index b9863f9a35fa..58153cdf025b 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -6,6 +6,7 @@ menuconfig ARCH_SUNXI
 	select GENERIC_IRQ_CHIP
 	select GPIOLIB
 	select PINCTRL
+	select PM_OPP
 	select SUN4I_TIMER
 	select RESET_CONTROLLER
 
-- 
git-series 0.8.11

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 8/8] ARM: sun8i: a33: Add the Mali OPPs

[Maxime Ripard]

The Mali GPU in the A33 has various operating frequencies used in the
Allwinner BSP.

Add them to our DT.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a33.dtsi | 17 +++++++++++++++++
 1 file changed, 17 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
index 043b1b017276..e1b0abfee42f 100644
--- a/arch/arm/boot/dts/sun8i-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a33.dtsi
@@ -101,6 +101,22 @@
 		status = "disabled";
 	};
 
+	mali_opp_table: opp_table1 {
+		compatible = "operating-points-v2";
+
+		opp@144000000 {
+			opp-hz = /bits/ 64 <144000000>;
+		};
+
+		opp@240000000 {
+			opp-hz = /bits/ 64 <240000000>;
+		};
+
+		opp@384000000 {
+			opp-hz = /bits/ 64 <384000000>;
+		};
+	};
+
 	memory {
 		reg = <0x40000000 0x80000000>;
 	};
@@ -282,6 +298,7 @@
 
 &mali {
 	memory-region = <&display_pool>;
+	operating-points-v2 = <&mali_opp_table>;
 };
 
 &pio {
-- 
git-series 0.8.11

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/8] mm: cma: Export a few symbols

[Michal Hocko]

[CC CMA people]

On Thu 09-02-17 17:39:17, Maxime Ripard wrote:
> Modules might want to check their CMA pool size and address for debugging
> and / or have additional checks.
> 
> The obvious way to do this would be through dev_get_cma_area and
> cma_get_base and cma_get_size, that are currently not exported, which
> results in a build failure.
> 
> Export them to prevent such a failure.

Who actually uses those exports. None of the follow up patches does
AFAICS.

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/base/dma-contiguous.c | 1 +
>  mm/cma.c                      | 2 ++
>  2 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index e167a1e1bccb..60f5c2591ccd 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -35,6 +35,7 @@
>  #endif
>  
>  struct cma *dma_contiguous_default_area;
> +EXPORT_SYMBOL(dma_contiguous_default_area);
>  
>  /*
>   * Default global CMA area size can be defined in kernel's .config.
> diff --git a/mm/cma.c b/mm/cma.c
> index c960459eda7e..b50245282a18 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -47,11 +47,13 @@ phys_addr_t cma_get_base(const struct cma *cma)
>  {
>  	return PFN_PHYS(cma->base_pfn);
>  }
> +EXPORT_SYMBOL(cma_get_base);
>  
>  unsigned long cma_get_size(const struct cma *cma)
>  {
>  	return cma->count << PAGE_SHIFT;
>  }
> +EXPORT_SYMBOL(cma_get_size);
>  
>  static unsigned long cma_bitmap_aligned_mask(const struct cma *cma,
>  					     int align_order)
> -- 
> git-series 0.8.11
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/8] mm: cma: Export a few symbols

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]

Hi Michal,

On Thu, Feb 09, 2017 at 08:20:47PM +0100, Michal Hocko wrote:
> [CC CMA people]
> 
> On Thu 09-02-17 17:39:17, Maxime Ripard wrote:
> > Modules might want to check their CMA pool size and address for debugging
> > and / or have additional checks.
> > 
> > The obvious way to do this would be through dev_get_cma_area and
> > cma_get_base and cma_get_size, that are currently not exported, which
> > results in a build failure.
> > 
> > Export them to prevent such a failure.
> 
> Who actually uses those exports. None of the follow up patches does
> AFAICS.

This is for the ARM Mali GPU driver that is out of tree, unfortunately.

In one case (using the legacy fbdev API), the driver wants to (and
probably should) validate that the buffer as indeed been allocated
from the memory allocation pool.

Rob suggested that instead of hardcoding it to cover the whole RAM
(which defeats the purpose of that check in the first place), we used
the memory-region bindings in the DT and follow that, which does work
great, but we still have to retrieve the base address and size of that
region, hence why this patches are needed.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/8] dt-bindings: gpu: mali: Add optional memory-region

[Rob Herring]

On Thu, Feb 09, 2017 at 05:39:16PM +0100, Maxime Ripard wrote:
> The reserved memory bindings allow us to specify which memory areas our
> buffers can be allocated from.
> 
> Let's use it.

You didn't think of this when you just added Mali binding? 

> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt | 4 ++++
>  1 file changed, 4 insertions(+), 0 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 8/8] ARM: sun8i: a33: Add the Mali OPPs

[Rob Herring]

On Thu, Feb 09, 2017 at 05:39:22PM +0100, Maxime Ripard wrote:
> The Mali GPU in the A33 has various operating frequencies used in the
> Allwinner BSP.
> 
> Add them to our DT.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm/boot/dts/sun8i-a33.dtsi | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
> index 043b1b017276..e1b0abfee42f 100644
> --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> @@ -101,6 +101,22 @@
>  		status = "disabled";
>  	};
>  
> +	mali_opp_table: opp_table1 {

gpu-opp-table

> +		compatible = "operating-points-v2";
> +
> +		opp@144000000 {
> +			opp-hz = /bits/ 64 <144000000>;
> +		};
> +
> +		opp@240000000 {
> +			opp-hz = /bits/ 64 <240000000>;
> +		};
> +
> +		opp@384000000 {
> +			opp-hz = /bits/ 64 <384000000>;
> +		};
> +	};
> +
>  	memory {
>  		reg = <0x40000000 0x80000000>;
>  	};
> @@ -282,6 +298,7 @@
>  
>  &mali {
>  	memory-region = <&display_pool>;
> +	operating-points-v2 = <&mali_opp_table>;
>  };
>  
>  &pio {
> -- 
> git-series 0.8.11

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 6/8] dt-bindings: gpu: mali: Add optional OPPs

[Rob Herring]

On Thu, Feb 09, 2017 at 05:39:20PM +0100, Maxime Ripard wrote:
> The operating-points-v2 binding gives a way to provide the OPP of the GPU.
> Let's use it.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt | 4 ++++
>  1 file changed, 4 insertions(+), 0 deletions(-)

Bindings should not unnecessarily evolve. The h/w does not evolve in 
this way. This should have been part of the original binding given that 
it was just added.

Acked-by: Rob Herring <robh@kernel.org>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/8] mm: cma: Export a few symbols

[Michal Hocko]

On Mon 13-02-17 14:44:16, Maxime Ripard wrote:
> Hi Michal,
> 
> On Thu, Feb 09, 2017 at 08:20:47PM +0100, Michal Hocko wrote:
> > [CC CMA people]
> > 
> > On Thu 09-02-17 17:39:17, Maxime Ripard wrote:
> > > Modules might want to check their CMA pool size and address for debugging
> > > and / or have additional checks.
> > > 
> > > The obvious way to do this would be through dev_get_cma_area and
> > > cma_get_base and cma_get_size, that are currently not exported, which
> > > results in a build failure.
> > > 
> > > Export them to prevent such a failure.
> > 
> > Who actually uses those exports. None of the follow up patches does
> > AFAICS.
> 
> This is for the ARM Mali GPU driver that is out of tree, unfortunately.

We do not export symbols which do not have any in-tree users.

> In one case (using the legacy fbdev API), the driver wants to (and
> probably should) validate that the buffer as indeed been allocated
> from the memory allocation pool.
> 
> Rob suggested that instead of hardcoding it to cover the whole RAM
> (which defeats the purpose of that check in the first place), we used
> the memory-region bindings in the DT and follow that, which does work
> great, but we still have to retrieve the base address and size of that
> region, hence why this patches are needed.

Anyway I would suggest talking to CMA people to find a better API for
modules to use...

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 3/8] mm: cma: Export a few symbols

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]

On Mon, Feb 20, 2017 at 01:35:50PM +0100, Michal Hocko wrote:
> On Mon 13-02-17 14:44:16, Maxime Ripard wrote:
> > Hi Michal,
> > 
> > On Thu, Feb 09, 2017 at 08:20:47PM +0100, Michal Hocko wrote:
> > > [CC CMA people]
> > > 
> > > On Thu 09-02-17 17:39:17, Maxime Ripard wrote:
> > > > Modules might want to check their CMA pool size and address for debugging
> > > > and / or have additional checks.
> > > > 
> > > > The obvious way to do this would be through dev_get_cma_area and
> > > > cma_get_base and cma_get_size, that are currently not exported, which
> > > > results in a build failure.
> > > > 
> > > > Export them to prevent such a failure.
> > > 
> > > Who actually uses those exports. None of the follow up patches does
> > > AFAICS.
> > 
> > This is for the ARM Mali GPU driver that is out of tree, unfortunately.
> 
> We do not export symbols which do not have any in-tree users.

Ok, sorry for the noise.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 1/8] ARM: sun8i: Fix the mali clock rate

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 479 bytes --]

On Thu, Feb 09, 2017 at 05:39:15PM +0100, Maxime Ripard wrote:
> The Mali clock rate was improperly assumed to be 408MHz, while it was
> really 384Mhz, 408MHz being the "extreme" frequency, and definitely not
> stable.
> 
> Switch for the stable, correct frequency for the GPU.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied.
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/8] dt-bindings: gpu: mali: Add optional memory-region

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]

On Thu, Feb 09, 2017 at 05:39:16PM +0100, Maxime Ripard wrote:
> The reserved memory bindings allow us to specify which memory areas our
> buffers can be allocated from.
> 
> Let's use it.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied with Rob Acked-by.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 6/8] dt-bindings: gpu: mali: Add optional OPPs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]

On Thu, Feb 09, 2017 at 05:39:20PM +0100, Maxime Ripard wrote:
> The operating-points-v2 binding gives a way to provide the OPP of the GPU.
> Let's use it.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied with Rob Acked-by.
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 7/8] ARM: sunxi: Select PM_OPP

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

On Thu, Feb 09, 2017 at 05:39:21PM +0100, Maxime Ripard wrote:
> Device frequency scaling is implemented through devfreq in the kernel,
> which requires CONFIG_PM_OPP.
> 
> Let's select it.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Applied.
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 8/8] ARM: sun8i: a33: Add the Mali OPPs

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 958 bytes --]

On Wed, Feb 15, 2017 at 05:40:29PM -0600, Rob Herring wrote:
> On Thu, Feb 09, 2017 at 05:39:22PM +0100, Maxime Ripard wrote:
> > The Mali GPU in the A33 has various operating frequencies used in the
> > Allwinner BSP.
> > 
> > Add them to our DT.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  arch/arm/boot/dts/sun8i-a33.dtsi | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
> > index 043b1b017276..e1b0abfee42f 100644
> > --- a/arch/arm/boot/dts/sun8i-a33.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-a33.dtsi
> > @@ -101,6 +101,22 @@
> >  		status = "disabled";
> >  	};
> >  
> > +	mali_opp_table: opp_table1 {
> 
> gpu-opp-table

Applied with that change. Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 4/8] drm/sun4i: Grab reserved memory region

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

On Thu, Feb 09, 2017 at 05:39:18PM +0100, Maxime Ripard wrote:
> Allow to provide an optional memory region to allocate from for our DRM
> driver.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Fixed the conflicts and applied.
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Emil Velikov]

Hi Maxime,

Thanks for the links.

On 24 February 2017 at 00:19, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Fri, Feb 17, 2017 at 08:39:33PM +0000, Emil Velikov wrote:
>> As I feared things have taken a turn for the bitter end :-]
>>
>> It seems that this is a heated topic, so I'l kindly ask that we try
>> the following:
>>
>>  - For people such as myself/Tobias/others who feel that driver and DT
>> bindings should go hand in hand, prove them wrong.
>> But please, do so by pointing to the documentation (conclusion of a
>> previous discussion). This way you don't have to repeat yourself and
>> get [too] annoyed over silly suggestions.
>
> http://lxr.free-electrons.com/source/Documentation/devicetree/usage-model.txt#L13
>
> "The "Open Firmware Device Tree", or simply Device Tree (DT), is a
> data structure and language for describing hardware. More
> specifically, it is a description of hardware that is readable by an
> operating system so that the operating system doesn't need to hard
> code details of the machine"
>
> http://lxr.free-electrons.com/source/Documentation/devicetree/usage-model.txt#L79
>
> "What it does do is provide a language for decoupling the hardware
> configuration from the board and device driver support in the Linux
> kernel (or any other operating system for that matter)."
>
The above seems to imply that there is (merged) device driver support
in the Linux kernel (or other) that uses the bindings.

It's not my call to make any of the policy, so I'll just kindly
suggest improving the existing documentation:
 - Reword/elaborate if out of tree [Linux or in general?] drivers are
suitable counterpart.
 - Patches could/should reference the "other OS" driver, or the "other
OS" name at least ?

Rather than clumping the above in 2.1 a separate section would be better ?

> And like I said, we already had bindings for out of tree bindings,
> like this one:
> https://patchwork.kernel.org/patch/9275707/
>
> Which triggered no discussion at the time (but the technical one,
> hence a v2, that should always be done).
>
Needless to say, there's many of us waiting to see a Mali driver land
- hence the noise. It's not meant to belittle/sway the work you and
others do.

>> - The series has code changes which [seemingly] cater for out of tree
>> module(s).
>
> That patch was dropped, only DT changes remains now, and do not depend
> of that missing patch anyway.
>
>> Clearly state in the commit message who is the user, why it's save to
>> do so and get an Ack from more prominent [DRM] developers.
>
> DRM is really not important here. We could implement a driver using
> i2c as far as the DT is concerned.
>
What I meant to say is:

Please, provide clear expectations from the start - "Linux driver is
OOT with no ETA on landing" or "driver for $FOO OS is at $LINK".
Afaict Hans did the former in the patch mentioned. Perhaps you already
did - in which case pardon for missing it.

> FreeBSD for example uses a different, !DRM framework to support our
> display stack, and still uses the DT.
>
Interesting - do you have a link handy ? Does it use open-source usespace ?

Thanks
Emil

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Rob Herring]

On Fri, Feb 17, 2017 at 9:56 AM, Tobias Jakobi
<tjakobi@math.uni-bielefeld.de> wrote:
> Alexandre Belloni wrote:
>> On 17/02/2017 at 13:45:44 +0100, Tobias Jakobi wrote:
>>>> The device tree is a representation of the hardware itself. The state
>>>> of the driver support doesn't change the hardware you're running on,
>>>> just like your BIOS/UEFI on x86 won't change the device it reports to
>>>> Linux based on whether it has a driver for it.
>>> Like Emil already said, the new bindings and the DT entries are solely
>>> introduced to support a proprietary out-of-tree module.
>>>
>>
>> Because device tree describes the hardware, the added binding doesn't
>> support any particular module. The eventually upstreamed drvier will
>> share the same bindings.
> OK, can we then agree that we _only_ merge the bindings and the entries,
> once this driver is upstream?

Absolutely not.

> Driver upstreaming and DT work go hand-in-hand. It's usually after a lot
> of discussion that new bindings get finalised. And for that discussion
> to happen we need to know how the driver uses the information from the
> DT. Otherwise we have no way to evaluate if the description is in any
> way "appropriate".
>
> And no, I don't follow the "DT is a separate/independent thing" thought.
> It maybe is in an ideal world, but we've seen it now often enough that
> bindings turned out to be poorly designed, even though they looked fine
> at first.

Certainly, that happens (though arguably that was more often from lack
of review). But this one is self contained, using standard, existing
properties. I'm not worried about us getting it right. If this was
something new or different, then certainly yes I would want to see the
code.

Rob

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2114 bytes --]

Hi,

On Fri, Feb 17, 2017 at 08:39:33PM +0000, Emil Velikov wrote:
> As I feared things have taken a turn for the bitter end :-]
> 
> It seems that this is a heated topic, so I'l kindly ask that we try
> the following:
> 
>  - For people such as myself/Tobias/others who feel that driver and DT
> bindings should go hand in hand, prove them wrong.
> But please, do so by pointing to the documentation (conclusion of a
> previous discussion). This way you don't have to repeat yourself and
> get [too] annoyed over silly suggestions.

http://lxr.free-electrons.com/source/Documentation/devicetree/usage-model.txt#L13

"The "Open Firmware Device Tree", or simply Device Tree (DT), is a
data structure and language for describing hardware. More
specifically, it is a description of hardware that is readable by an
operating system so that the operating system doesn't need to hard
code details of the machine"

http://lxr.free-electrons.com/source/Documentation/devicetree/usage-model.txt#L79

"What it does do is provide a language for decoupling the hardware
configuration from the board and device driver support in the Linux
kernel (or any other operating system for that matter)."

And like I said, we already had bindings for out of tree bindings,
like this one:
https://patchwork.kernel.org/patch/9275707/

Which triggered no discussion at the time (but the technical one,
hence a v2, that should always be done).

> - The series has code changes which [seemingly] cater for out of tree
> module(s).

That patch was dropped, only DT changes remains now, and do not depend
of that missing patch anyway.

> Clearly state in the commit message who is the user, why it's save to
> do so and get an Ack from more prominent [DRM] developers.

DRM is really not important here. We could implement a driver using
i2c as far as the DT is concerned.

FreeBSD for example uses a different, !DRM framework to support our
display stack, and still uses the DT.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4916 bytes --]

Hi Thierry,

On Mon, Feb 20, 2017 at 05:49:26PM +0100, Thierry Reding wrote:
> On Fri, Feb 17, 2017 at 04:43:41PM +0100, Maxime Ripard wrote:
> > On Fri, Feb 17, 2017 at 01:45:44PM +0100, Tobias Jakobi wrote:
> > > Hello Maxime,
> > > 
> > > Maxime Ripard wrote:
> > > > Hi,
> > > > 
> > > > On Thu, Feb 16, 2017 at 01:43:06PM +0100, Tobias Jakobi wrote:
> > > >> I was wondering about the following. Wasn't there some strict
> > > >> requirement about code going upstream, which also included that there
> > > >> was a full open-source driver stack for it?
> > > >>
> > > >> I don't see how this is the case for Mali, neither in the kernel, nor in
> > > >> userspace. I'm aware that the Mali kernel driver is open-source. But it
> > > >> is not upstream, maintained out of tree, and won't land upstream in its
> > > >> current form (no resemblence to a DRM driver at all). And let's not talk
> > > >> about the userspace part.
> > > >>
> > > >> So, why should this be here?
> > > > 
> > > > The device tree is a representation of the hardware itself. The state
> > > > of the driver support doesn't change the hardware you're running on,
> > > > just like your BIOS/UEFI on x86 won't change the device it reports to
> > > > Linux based on whether it has a driver for it.
> > >
> > > Like Emil already said, the new bindings and the DT entries are solely
> > > introduced to support a proprietary out-of-tree module.
> > 
> > No. This new binding and the DT entries are solely introduced to
> > describe a device found in a number of SoCs, just like any other DT
> > binding we have.
> > 
> > > The current workflow when introducing new DT entries is the following:
> > > - upstream a driver that uses the entries
> > > - THEN add the new entries
> > 
> > And that's never been the preferred workflow, for *any* patches.
> 
> Actually it has. How else are you going to test that your driver really
> works? You've got to have both pieces before you can verify that they're
> both adequate. So the typical workflow is to:
> 
> 	1) define the bindings
> 	2) write a driver that implements the bindings
> 	3) add entries to device tree files
> 
> Usually it doesn't matter in which order you do the above because they
> are all part of the same patch series. But that's not what you're doing
> here. The more general problem here is that you're providing device tree
> content (and therefore ABI) that's based on a binding which has no
> upstream users. So you don't actually have a way of validating that what
> you merge is going to be an adequate description.
> 
> You're probably going to respond: "but DT describes hardware, so it must
> be known already, there won't be a need for changes". Unfortunately that
> is only partially true. We've had a number of occasions where it later
> turned out that a binding was in fact not an adequate description, and
> then we've had to jump through hoops in order to preserve backwards
> compatibility. That's already annoying enough if you've got in-tree
> users, but it's going to be even more painful if you start out with an
> out-of-tree user.
> 
> All of that said, you've got an Acked-by from Rob and that's about as
> good as it's going to get. So I'm not going to NAK this. But I will
> caution against this, because I don't think you're doing yourself any
> favours with this.
> 
> So perhaps the question that we should ask is this: what do you gain by
> merging this series? The fact remains that you don't have an upstream
> driver that implements this binding, so ultimately you're going to be
> carrying patches in some development tree anyway. Why not simply stash
> these patches into the same tree? That should be about the same amount
> of work for you and your users, but it has the advantage of not locking
> you into something that you may regret.

This is really a usability issue. I don't want the average Jane / Joe
to have to patch and recompile the DT in order to get the GPU running
on her / his distro of choice. The only thing that should be needed
would be to install an (or a couple of) extra package in order to get
everything running. Just like it's done for any other GPU out there,
or wifi, disregarding whether it's supported upstream or not.

Another nice thing is also that the mali bindings have been a huge
mess so far. If we have a common bindings, everything will just work
the same for all the platforms, and we can use the same driver there,
allowing us at least to consolidate the source code.

I'm not too afraid about getting something wrong. We already have code
working for two different vendors, and all the other GPUs already
described in DT (nvidia's, the Adreno) all have the same kind of
binding that we have.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3673 bytes --]

On Fri, Feb 17, 2017 at 04:43:41PM +0100, Maxime Ripard wrote:
> On Fri, Feb 17, 2017 at 01:45:44PM +0100, Tobias Jakobi wrote:
> > Hello Maxime,
> > 
> > Maxime Ripard wrote:
> > > Hi,
> > > 
> > > On Thu, Feb 16, 2017 at 01:43:06PM +0100, Tobias Jakobi wrote:
> > >> I was wondering about the following. Wasn't there some strict
> > >> requirement about code going upstream, which also included that there
> > >> was a full open-source driver stack for it?
> > >>
> > >> I don't see how this is the case for Mali, neither in the kernel, nor in
> > >> userspace. I'm aware that the Mali kernel driver is open-source. But it
> > >> is not upstream, maintained out of tree, and won't land upstream in its
> > >> current form (no resemblence to a DRM driver at all). And let's not talk
> > >> about the userspace part.
> > >>
> > >> So, why should this be here?
> > > 
> > > The device tree is a representation of the hardware itself. The state
> > > of the driver support doesn't change the hardware you're running on,
> > > just like your BIOS/UEFI on x86 won't change the device it reports to
> > > Linux based on whether it has a driver for it.
> >
> > Like Emil already said, the new bindings and the DT entries are solely
> > introduced to support a proprietary out-of-tree module.
> 
> No. This new binding and the DT entries are solely introduced to
> describe a device found in a number of SoCs, just like any other DT
> binding we have.
> 
> > The current workflow when introducing new DT entries is the following:
> > - upstream a driver that uses the entries
> > - THEN add the new entries
> 
> And that's never been the preferred workflow, for *any* patches.

Actually it has. How else are you going to test that your driver really
works? You've got to have both pieces before you can verify that they're
both adequate. So the typical workflow is to:

	1) define the bindings
	2) write a driver that implements the bindings
	3) add entries to device tree files

Usually it doesn't matter in which order you do the above because they
are all part of the same patch series. But that's not what you're doing
here. The more general problem here is that you're providing device tree
content (and therefore ABI) that's based on a binding which has no
upstream users. So you don't actually have a way of validating that what
you merge is going to be an adequate description.

You're probably going to respond: "but DT describes hardware, so it must
be known already, there won't be a need for changes". Unfortunately that
is only partially true. We've had a number of occasions where it later
turned out that a binding was in fact not an adequate description, and
then we've had to jump through hoops in order to preserve backwards
compatibility. That's already annoying enough if you've got in-tree
users, but it's going to be even more painful if you start out with an
out-of-tree user.

All of that said, you've got an Acked-by from Rob and that's about as
good as it's going to get. So I'm not going to NAK this. But I will
caution against this, because I don't think you're doing yourself any
favours with this.

So perhaps the question that we should ask is this: what do you gain by
merging this series? The fact remains that you don't have an upstream
driver that implements this binding, so ultimately you're going to be
carrying patches in some development tree anyway. Why not simply stash
these patches into the same tree? That should be about the same amount
of work for you and your users, but it has the advantage of not locking
you into something that you may regret.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Rask Ingemann Lambertsen]

On Fri, Feb 17, 2017 at 04:44:19PM +0100, Maxime Ripard wrote:
[...]
> We already have DT bindings for out of tree drivers, there's really
> nothing new here.

We have DT bindings for *hardware*, not for drivers. As stated in
Documentation/devicetree/usage-model.txt:

"The "Open Firmware Device Tree", or simply Device Tree (DT), is a data
structure and language for describing hardware.  More specifically, it
is a description of hardware that is readable by an operating system
so that the operating system doesn't need to hard code details of the
machine."

"2.1 High Level View
-------------------
The most important thing to understand is that the DT is simply a data
structure that describes the hardware."

-- 
Rask Ingemann Lambertsen

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Emil Velikov]

Hi Maxime,

As I feared things have taken a turn for the bitter end :-]

It seems that this is a heated topic, so I'l kindly ask that we try
the following:

 - For people such as myself/Tobias/others who feel that driver and DT
bindings should go hand in hand, prove them wrong.
But please, do so by pointing to the documentation (conclusion of a
previous discussion). This way you don't have to repeat yourself and
get [too] annoyed over silly suggestions.

 - The series has code changes which [seemingly] cater for out of tree
module(s).
Clearly state in the commit message who is the user, why it's save to
do so and get an Ack from more prominent [DRM] developers.

Please try to understand that I do not want to annoy/agitate you, I'm
merely pointing what seems [to me] as incorrect.
Nobody is perfect, so if I/others are wrong do point me/us to a
reading to educate ourselves.

Thanks
Emil

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Tobias Jakobi]

Alexandre Belloni wrote:
> On 17/02/2017 at 13:45:44 +0100, Tobias Jakobi wrote:
>>> The device tree is a representation of the hardware itself. The state
>>> of the driver support doesn't change the hardware you're running on,
>>> just like your BIOS/UEFI on x86 won't change the device it reports to
>>> Linux based on whether it has a driver for it.
>> Like Emil already said, the new bindings and the DT entries are solely
>> introduced to support a proprietary out-of-tree module.
>>
> 
> Because device tree describes the hardware, the added binding doesn't
> support any particular module. The eventually upstreamed drvier will
> share the same bindings.
OK, can we then agree that we _only_ merge the bindings and the entries,
once this driver is upstream?

Driver upstreaming and DT work go hand-in-hand. It's usually after a lot
of discussion that new bindings get finalised. And for that discussion
to happen we need to know how the driver uses the information from the
DT. Otherwise we have no way to evaluate if the description is in any
way "appropriate".

And no, I don't follow the "DT is a separate/independent thing" thought.
It maybe is in an ideal world, but we've seen it now often enough that
bindings turned out to be poorly designed, even though they looked fine
at first.

With best wishes,
Tobias


> 
>> The current workflow when introducing new DT entries is the following:
>> - upstream a driver that uses the entries
>> - THEN add the new entries
>>
> 
> Exactly not, if you do that, checkpatch will complain loudly. Because
> you must not add a driver using bindings that are not documented first.
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]

On Thu, Feb 16, 2017 at 04:54:45PM +0000, Emil Velikov wrote:
> On 16 February 2017 at 12:43, Tobias Jakobi
> <tjakobi@math.uni-bielefeld.de> wrote:
> > Hello,
> >
> > I was wondering about the following. Wasn't there some strict
> > requirement about code going upstream, which also included that there
> > was a full open-source driver stack for it?
> >
> > I don't see how this is the case for Mali, neither in the kernel, nor in
> > userspace. I'm aware that the Mali kernel driver is open-source. But it
> > is not upstream, maintained out of tree, and won't land upstream in its
> > current form (no resemblence to a DRM driver at all). And let's not talk
> > about the userspace part.
> >
> > So, why should this be here?
> >
> Have to agree with Tobias, here.
> 
> I can see the annoyance that Maxime and others have to go through to
> their systems working.
> At the same time, changing upstream kernel to suit out of tree
> module(s) is not how things work. Right ?
> 
> Not to mention that the series adds stable ABI exclusively(?) used by
> a module which does not seem to be in the process of getting merged.

It really doesn't have any relation to whether a particular component
is supported in Linux. Our git repo just happens to be the canonical
source of DT, but those DTs are also used in other systems and
projects that have *no* relation with Linux, and might have a
different view on things than we do.

There's been a long-running discussion about moving the DTs out of the
kernel and in a separate repo. Would you still be opposed to it if I
happened to contribute that binding to that repo, even if Linux didn't
have any in-tree support for it? I'm pretty sure you wouldn't, yet
this is the exact same case.

And taking the ACPI example once again, this doesn't seem to bother
you at all that ACPI reports that it has a device that is not
supported in-tree in Linux. Why is it any different in DT.

We already have DT bindings for out of tree drivers, there's really
nothing new here.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]

On Fri, Feb 17, 2017 at 01:45:44PM +0100, Tobias Jakobi wrote:
> Hello Maxime,
> 
> Maxime Ripard wrote:
> > Hi,
> > 
> > On Thu, Feb 16, 2017 at 01:43:06PM +0100, Tobias Jakobi wrote:
> >> I was wondering about the following. Wasn't there some strict
> >> requirement about code going upstream, which also included that there
> >> was a full open-source driver stack for it?
> >>
> >> I don't see how this is the case for Mali, neither in the kernel, nor in
> >> userspace. I'm aware that the Mali kernel driver is open-source. But it
> >> is not upstream, maintained out of tree, and won't land upstream in its
> >> current form (no resemblence to a DRM driver at all). And let's not talk
> >> about the userspace part.
> >>
> >> So, why should this be here?
> > 
> > The device tree is a representation of the hardware itself. The state
> > of the driver support doesn't change the hardware you're running on,
> > just like your BIOS/UEFI on x86 won't change the device it reports to
> > Linux based on whether it has a driver for it.
>
> Like Emil already said, the new bindings and the DT entries are solely
> introduced to support a proprietary out-of-tree module.

No. This new binding and the DT entries are solely introduced to
describe a device found in a number of SoCs, just like any other DT
binding we have.

> The current workflow when introducing new DT entries is the following:
> - upstream a driver that uses the entries
> - THEN add the new entries

And that's never been the preferred workflow, for *any* patches.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Alexandre Belloni]

On 17/02/2017 at 13:45:44 +0100, Tobias Jakobi wrote:
> > The device tree is a representation of the hardware itself. The state
> > of the driver support doesn't change the hardware you're running on,
> > just like your BIOS/UEFI on x86 won't change the device it reports to
> > Linux based on whether it has a driver for it.
> Like Emil already said, the new bindings and the DT entries are solely
> introduced to support a proprietary out-of-tree module.
> 

Because device tree describes the hardware, the added binding doesn't
support any particular module. The eventually upstreamed drvier will
share the same bindings.

> The current workflow when introducing new DT entries is the following:
> - upstream a driver that uses the entries
> - THEN add the new entries
> 

Exactly not, if you do that, checkpatch will complain loudly. Because
you must not add a driver using bindings that are not documented first.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Emil Velikov]

On 17 February 2017 at 12:45, Tobias Jakobi
<tjakobi@math.uni-bielefeld.de> wrote:
> Hello Maxime,
>
> Maxime Ripard wrote:
>> Hi,
>>
>> On Thu, Feb 16, 2017 at 01:43:06PM +0100, Tobias Jakobi wrote:
>>> I was wondering about the following. Wasn't there some strict
>>> requirement about code going upstream, which also included that there
>>> was a full open-source driver stack for it?
>>>
>>> I don't see how this is the case for Mali, neither in the kernel, nor in
>>> userspace. I'm aware that the Mali kernel driver is open-source. But it
>>> is not upstream, maintained out of tree, and won't land upstream in its
>>> current form (no resemblence to a DRM driver at all). And let's not talk
>>> about the userspace part.
>>>
>>> So, why should this be here?
>>
>> The device tree is a representation of the hardware itself. The state
>> of the driver support doesn't change the hardware you're running on,
>> just like your BIOS/UEFI on x86 won't change the device it reports to
>> Linux based on whether it has a driver for it.
> Like Emil already said, the new bindings and the DT entries are solely
> introduced to support a proprietary out-of-tree module.
>
> The current workflow when introducing new DT entries is the following:
> - upstream a driver that uses the entries
> - THEN add the new entries
>
That's the ideal route that I was thinking of.

At the same time, if prominent DRM people believe that we can/should
turn a blind eye, so be it.
I'm not trying to make Maxime's life hard, but point out that things
feel iffy IMHO.

Thanks
Emil

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Tobias Jakobi]

Hello Maxime,

Maxime Ripard wrote:
> Hi,
> 
> On Thu, Feb 16, 2017 at 01:43:06PM +0100, Tobias Jakobi wrote:
>> I was wondering about the following. Wasn't there some strict
>> requirement about code going upstream, which also included that there
>> was a full open-source driver stack for it?
>>
>> I don't see how this is the case for Mali, neither in the kernel, nor in
>> userspace. I'm aware that the Mali kernel driver is open-source. But it
>> is not upstream, maintained out of tree, and won't land upstream in its
>> current form (no resemblence to a DRM driver at all). And let's not talk
>> about the userspace part.
>>
>> So, why should this be here?
> 
> The device tree is a representation of the hardware itself. The state
> of the driver support doesn't change the hardware you're running on,
> just like your BIOS/UEFI on x86 won't change the device it reports to
> Linux based on whether it has a driver for it.
Like Emil already said, the new bindings and the DT entries are solely
introduced to support a proprietary out-of-tree module.

The current workflow when introducing new DT entries is the following:
- upstream a driver that uses the entries
- THEN add the new entries

I'm against adding such entries without having any upstream "consumer".


With best wishes,
Tobias


> So yes, unfortunately, we don't have a driver upstream at the
> moment. But that doesn't prevent us from describing the hardware
> accurately.
> 
> Maxime
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]

Hi,

On Thu, Feb 16, 2017 at 01:43:06PM +0100, Tobias Jakobi wrote:
> I was wondering about the following. Wasn't there some strict
> requirement about code going upstream, which also included that there
> was a full open-source driver stack for it?
> 
> I don't see how this is the case for Mali, neither in the kernel, nor in
> userspace. I'm aware that the Mali kernel driver is open-source. But it
> is not upstream, maintained out of tree, and won't land upstream in its
> current form (no resemblence to a DRM driver at all). And let's not talk
> about the userspace part.
> 
> So, why should this be here?

The device tree is a representation of the hardware itself. The state
of the driver support doesn't change the hardware you're running on,
just like your BIOS/UEFI on x86 won't change the device it reports to
Linux based on whether it has a driver for it.

So yes, unfortunately, we don't have a driver upstream at the
moment. But that doesn't prevent us from describing the hardware
accurately.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Emil Velikov]

On 16 February 2017 at 12:43, Tobias Jakobi
<tjakobi@math.uni-bielefeld.de> wrote:
> Hello,
>
> I was wondering about the following. Wasn't there some strict
> requirement about code going upstream, which also included that there
> was a full open-source driver stack for it?
>
> I don't see how this is the case for Mali, neither in the kernel, nor in
> userspace. I'm aware that the Mali kernel driver is open-source. But it
> is not upstream, maintained out of tree, and won't land upstream in its
> current form (no resemblence to a DRM driver at all). And let's not talk
> about the userspace part.
>
> So, why should this be here?
>
Have to agree with Tobias, here.

I can see the annoyance that Maxime and others have to go through to
their systems working.
At the same time, changing upstream kernel to suit out of tree
module(s) is not how things work. Right ?

Not to mention that the series adds stable ABI exclusively(?) used by
a module which does not seem to be in the process of getting merged.

Maxime, you're a great guy but I don't think this is suitable for
upstream... yet.

Regards,
Emil

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 0/8] ARM: sun8i: a33: Mali improvements

[Tobias Jakobi]

Hello,

I was wondering about the following. Wasn't there some strict
requirement about code going upstream, which also included that there
was a full open-source driver stack for it?

I don't see how this is the case for Mali, neither in the kernel, nor in
userspace. I'm aware that the Mali kernel driver is open-source. But it
is not upstream, maintained out of tree, and won't land upstream in its
current form (no resemblence to a DRM driver at all). And let's not talk
about the userspace part.

So, why should this be here?

With best wishes,
Tobias

P.S.: I'm signed up to dri-devel in digest mode, so sorry if this mail
doesn't properly show up in the corresponding ml thread.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>