[Buildroot] [PATCH v2] kmscube: Add new package

[Buildroot] [PATCH v2] kmscube: Add new package

[Fabio Estevam]

Add support for kmscube application, which is helpful for testing
kms/drm drivers.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
Changes since v1:
- Use Rob Clark's tree instead (Gary).

 package/Config.in                    |  1 +
 package/kmscube/0001-addimxdrm.patch | 28 ++++++++++++++++++++++++++++
 package/kmscube/Config.in            |  5 +++++
 package/kmscube/kmscube.hash         |  2 ++
 package/kmscube/kmscube.mk           | 14 ++++++++++++++
 5 files changed, 50 insertions(+)
 create mode 100644 package/kmscube/0001-addimxdrm.patch
 create mode 100644 package/kmscube/Config.in
 create mode 100644 package/kmscube/kmscube.hash
 create mode 100644 package/kmscube/kmscube.mk

diff --git a/package/Config.in b/package/Config.in
index deff0fe..af52e1a 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -239,6 +239,7 @@ comment "Graphic applications"
 	source "package/glmark2/Config.in"
 	source "package/gnuplot/Config.in"
 	source "package/jhead/Config.in"
+	source "package/kmscube/Config.in"
 	source "package/mesa3d-demos/Config.in"
 	source "package/qt5cinex/Config.in"
 	source "package/rrdtool/Config.in"
diff --git a/package/kmscube/0001-addimxdrm.patch b/package/kmscube/0001-addimxdrm.patch
new file mode 100644
index 0000000..85e9a2a
--- /dev/null
+++ b/package/kmscube/0001-addimxdrm.patch
@@ -0,0 +1,28 @@
+From 23186149100dd9c871f65132edce67db0b1ce3c3 Mon Sep 17 00:00:00 2001
+From: XoD <xoddark@gmail.com>
+Date: Mon, 13 Feb 2017 16:43:35 -0200
+Subject: [PATCH] Add imx-drm management
+
+Add imx-drm in the list of kms modules.
+
+Signed-off-by: XoD <xoddark@gmail.com>
+Signed-off-by: Fabio Estevam <festevam@gmail.com>
+---
+ kmscube.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/kmscube.c b/kmscube.c
+index ca2c266..95bd77f 100644
+--- a/kmscube.c
++++ b/kmscube.c
+@@ -122,6 +122,7 @@ static int init_drm(void)
+ 	static const char *modules[] = {
+ 		"exynos",
+ 		"i915",
++		"imx-drm",
+ 		"msm",
+ 		"nouveau",
+ 		"omapdrm",
+-- 
+2.7.4
+
diff --git a/package/kmscube/Config.in b/package/kmscube/Config.in
new file mode 100644
index 0000000..b3c3c8d
--- /dev/null
+++ b/package/kmscube/Config.in
@@ -0,0 +1,5 @@
+config BR2_PACKAGE_KMSCUBE
+	bool "kmscube"
+	depends on BR2_PACKAGE_MESA3D && BR2_PACKAGE_LIBDRM
+	help
+	  kmscube is an application to test kms/drm drivers.  
diff --git a/package/kmscube/kmscube.hash b/package/kmscube/kmscube.hash
new file mode 100644
index 0000000..d3e9858
--- /dev/null
+++ b/package/kmscube/kmscube.hash
@@ -0,0 +1,2 @@
+# Locally computed
+sha256 73cf923f915f9f7ee46f9f03bd24254fc04b5f166270c0d37c6f2bc3881186ec kmscube-8c6a20901f95e1b465bbca127f9d47fcfb8762e6.tar.gz
diff --git a/package/kmscube/kmscube.mk b/package/kmscube/kmscube.mk
new file mode 100644
index 0000000..267fffa
--- /dev/null
+++ b/package/kmscube/kmscube.mk
@@ -0,0 +1,14 @@
+KMSCUBE_VERSION = 8c6a20901f95e1b465bbca127f9d47fcfb8762e6
+KMSCUBE_SITE = $(call github,robclark,kmscube,$(KMSCUBE_VERSION))
+KMSCUBE_DEPENDENCIES = host-pkgconf libdrm mesa3d
+KMSCUBE_INSTALL_TARGET = YES
+KMSCUBE_AUTORECONF = YES
+KMSCUBE_INSTALL_STAGING = Y
+
+# Autoreconf requires an existing m4 directory
+define KMSCUBE_PATCH_M4
+	mkdir -p $(@D)/m4
+endef
+KMSCUBE_POST_PATCH_HOOKS += KMSCUBE_PATCH_M4
+
+$(eval $(autotools-package))
-- 
2.7.4

[Buildroot] [PATCH v2] kmscube: Add new package

[Gary Bisson]

Hi Fabio, All,

On Mon, Feb 13, 2017 at 7:49 PM, Fabio Estevam <festevam@gmail.com> wrote:
>
> Add support for kmscube application, which is helpful for testing
> kms/drm drivers.
>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Reviewed-by: Gary Bisson <gary.bisson@boundarydevices.com>
Tested-by: Gary Bisson <gary.bisson@boundarydevices.com>

Tested on a i.MX6Q Nitrogen6x platform (drm-imx, etnaviv).

Regards,
Gary

[Buildroot] [PATCH v2] kmscube: Add new package

[Thomas Petazzoni]

Hello,

On Mon, 13 Feb 2017 16:49:44 -0200, Fabio Estevam wrote:
> Add support for kmscube application, which is helpful for testing
> kms/drm drivers.
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Thanks for this patch. I have a few comments, see below.

> diff --git a/package/kmscube/Config.in b/package/kmscube/Config.in
> new file mode 100644
> index 0000000..b3c3c8d
> --- /dev/null
> +++ b/package/kmscube/Config.in
> @@ -0,0 +1,5 @@
> +config BR2_PACKAGE_KMSCUBE
> +	bool "kmscube"
> +	depends on BR2_PACKAGE_MESA3D && BR2_PACKAGE_LIBDRM

kmscube apparently wants EGL and OpenGL ES:

PKG_CHECK_MODULES(EGL, egl)
PKG_CHECK_MODULES(GLES2, glesv2)

so probably this needs to be taken into account here by using the
BR2_PACKAGE_MESA3D_OPENGL_EGL and BR2_PACKAGE_MESA3D_OPENGL_ES symbols.

> +	help
> +	  kmscube is an application to test kms/drm drivers.  

Trailing space at the end of the line + missing upstream project URL.

> diff --git a/package/kmscube/kmscube.mk b/package/kmscube/kmscube.mk
> new file mode 100644
> index 0000000..267fffa
> --- /dev/null
> +++ b/package/kmscube/kmscube.mk
> @@ -0,0 +1,14 @@

Missing usual comment header.

> +KMSCUBE_VERSION = 8c6a20901f95e1b465bbca127f9d47fcfb8762e6
> +KMSCUBE_SITE = $(call github,robclark,kmscube,$(KMSCUBE_VERSION))
> +KMSCUBE_DEPENDENCIES = host-pkgconf libdrm mesa3d
> +KMSCUBE_INSTALL_TARGET = YES

This is useless, as it is the default.

> +KMSCUBE_AUTORECONF = YES
> +KMSCUBE_INSTALL_STAGING = Y

This line doesn't do anything because 'Y' is not a valid value. And
anyway, there's most likely no point in installing kmscube to staging.

License information is also missing, and you forgot to add yourself to
the DEVELOPERS file (should be done in the same patch).

Could you address those issues and submit an updated version?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v2] kmscube: Add new package

[Fabio Estevam]

Hi Thomas,

Thanks for your review.

On Mon, Feb 13, 2017 at 7:31 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

>> diff --git a/package/kmscube/Config.in b/package/kmscube/Config.in
>> new file mode 100644
>> index 0000000..b3c3c8d
>> --- /dev/null
>> +++ b/package/kmscube/Config.in
>> @@ -0,0 +1,5 @@
>> +config BR2_PACKAGE_KMSCUBE
>> +     bool "kmscube"
>> +     depends on BR2_PACKAGE_MESA3D && BR2_PACKAGE_LIBDRM
>
> kmscube apparently wants EGL and OpenGL ES:
>
> PKG_CHECK_MODULES(EGL, egl)
> PKG_CHECK_MODULES(GLES2, glesv2)
>
> so probably this needs to be taken into account here by using the
> BR2_PACKAGE_MESA3D_OPENGL_EGL and BR2_PACKAGE_MESA3D_OPENGL_ES symbols.

When building kmscube via Buildroot I see:

checking for DRM... yes
checking for GBM... yes
checking for EGL... yes
checking for GLES2... yes

BR2_PACKAGE_MESA3D_OPENGL_EGL is selected by the Etnaviv mesa driver:
BR2_PACKAGE_MESA3D_GALLIUM_DRIVER_ETNAVIV, which comes from a patch I
sent today.

I don't have BR2_PACKAGE_MESA3D_OPENGL_ES option selected, but I do
get "checking for GLES2... yes", so I am bit confused.

Is your suggestion to do something like this?

config BR2_PACKAGE_KMSCUBE
 bool "kmscube"
 depends on BR2_PACKAGE_MESA3D && BR2_PACKAGE_LIBDRM
 depends on BR2_PACKAGE_MESA3D_OPENGL_EGL
 select BR2_PACKAGE_MESA3D_OPENGL_ES

> License information is also missing, and you forgot to add yourself to
> the DEVELOPERS file (should be done in the same patch).

It is not clear by looking at the header files which license this
project uses. How should I do in a case like this?

Thanks

[Buildroot] [PATCH v2] kmscube: Add new package

[Fabio Estevam]

On Mon, Feb 13, 2017 at 8:03 PM, Fabio Estevam <festevam@gmail.com> wrote:

> It is not clear by looking at the header files which license this
> project uses. How should I do in a case like this?

It is MIT license.

Now my only question is what should I do with regards to
BR2_PACKAGE_MESA3D_OPENGL_EGL and BR2_PACKAGE_MESA3D_OPENGL_ES
symbols.

Thanks

[Buildroot] [PATCH v2] kmscube: Add new package

[Thomas Petazzoni]

Hello,

On Mon, 13 Feb 2017 20:03:29 -0200, Fabio Estevam wrote:

> > PKG_CHECK_MODULES(EGL, egl)
> > PKG_CHECK_MODULES(GLES2, glesv2)

This code means that kmscube will only build fine if the pkg-config
files egl.pc and glesv2.pc are available.

> checking for DRM... yes
> checking for GBM... yes
> checking for EGL... yes
> checking for GLES2... yes
> 
> BR2_PACKAGE_MESA3D_OPENGL_EGL is selected by the Etnaviv mesa driver:
> BR2_PACKAGE_MESA3D_GALLIUM_DRIVER_ETNAVIV, which comes from a patch I
> sent today.
> 
> I don't have BR2_PACKAGE_MESA3D_OPENGL_ES option selected, but I do
> get "checking for GLES2... yes", so I am bit confused.
> 
> Is your suggestion to do something like this?
> 
> config BR2_PACKAGE_KMSCUBE
>  bool "kmscube"
>  depends on BR2_PACKAGE_MESA3D && BR2_PACKAGE_LIBDRM
>  depends on BR2_PACKAGE_MESA3D_OPENGL_EGL
>  select BR2_PACKAGE_MESA3D_OPENGL_ES

I would do just:

	depends on BR2_PACKAGE_MESA3D_OPENGL_EGL
	depends on BR2_PACKAGE_MESA3D_OPENGL_ES

that is sufficient, and it's what glmark2/Config.in is doing (except
glmark2 is a bit more complicated since it also supports full OpenGL,
not only OpenGL ES).

The "depends on BR2_PACKAGE_MESA3D && BR2_PACKAGE_LIBDRM" are useless,
because if BR2_PACKAGE_MESA3D_OPENGL_EGL and
BR2_PACKAGE_MESA3D_OPENGL_ES are enabled, then surely mesa3d is
enabled. And mesa3d selects libdrm, so we're good.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v2] kmscube: Add new package

[Maxime Ripard]

Hi,

On Tue, Feb 14, 2017 at 09:18:03AM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 13 Feb 2017 20:03:29 -0200, Fabio Estevam wrote:
> 
> > > PKG_CHECK_MODULES(EGL, egl)
> > > PKG_CHECK_MODULES(GLES2, glesv2)
> 
> This code means that kmscube will only build fine if the pkg-config
> files egl.pc and glesv2.pc are available.
> 
> > checking for DRM... yes
> > checking for GBM... yes
> > checking for EGL... yes
> > checking for GLES2... yes
> > 
> > BR2_PACKAGE_MESA3D_OPENGL_EGL is selected by the Etnaviv mesa driver:
> > BR2_PACKAGE_MESA3D_GALLIUM_DRIVER_ETNAVIV, which comes from a patch I
> > sent today.
> > 
> > I don't have BR2_PACKAGE_MESA3D_OPENGL_ES option selected, but I do
> > get "checking for GLES2... yes", so I am bit confused.
> > 
> > Is your suggestion to do something like this?
> > 
> > config BR2_PACKAGE_KMSCUBE
> >  bool "kmscube"
> >  depends on BR2_PACKAGE_MESA3D && BR2_PACKAGE_LIBDRM
> >  depends on BR2_PACKAGE_MESA3D_OPENGL_EGL
> >  select BR2_PACKAGE_MESA3D_OPENGL_ES
> 
> I would do just:
> 
> 	depends on BR2_PACKAGE_MESA3D_OPENGL_EGL
> 	depends on BR2_PACKAGE_MESA3D_OPENGL_ES

Actually, it doesn't depend on a specific GLES / EGL implementation,
but it does depend on Mesa for GBM. There are alternative
implementation of GBM as well, so having something like:

depends on BR2_PACKAGE_HAS_LIBEGL
depends on BR2_PACKAGE_HAS_LIBGLES
depends on BR2_PACKAGE_MESA3D # GBM implementation

Seems more natural

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170214/fe5d8720/attachment.asc>

[Buildroot] [PATCH v2] kmscube: Add new package

[Thomas Petazzoni]

Hello,

On Tue, 14 Feb 2017 10:24:00 +0100, Maxime Ripard wrote:

> Actually, it doesn't depend on a specific GLES / EGL implementation,
> but it does depend on Mesa for GBM. There are alternative
> implementation of GBM as well, so having something like:
> 
> depends on BR2_PACKAGE_HAS_LIBEGL
> depends on BR2_PACKAGE_HAS_LIBGLES
> depends on BR2_PACKAGE_MESA3D # GBM implementation
> 
> Seems more natural

In principle, I would agree, but we pass --enable-gbm only when
BR2_PACKAGE_MESA3D_OPENGL_EGL=y. So with your proposal, we could end up
in a situation where libgbm isn't.

Try a build with just BR2_PACKAGE_MESA3D=y, libgm isn't
built/installed. So if we want to do this, perhaps we need a separate
BR2_PACKAGE_MESA3D_LIBGBM option?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v2] kmscube: Add new package

[Fabio Estevam]

Hi Thomas,

On Tue, Feb 14, 2017 at 6:18 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:

> I would do just:
>
>         depends on BR2_PACKAGE_MESA3D_OPENGL_EGL
>         depends on BR2_PACKAGE_MESA3D_OPENGL_ES

Got it. I can do like this.

What about KMSCUBE_DEPENDENCIES? Currently I have:
KMSCUBE_DEPENDENCIES = host-pkgconf libdrm mesa3d

Should this be adjusted to something different?

Thanks

[Buildroot] [PATCH v2] kmscube: Add new package

[Maxime Ripard]

On Tue, Feb 14, 2017 at 11:23:49AM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 14 Feb 2017 10:24:00 +0100, Maxime Ripard wrote:
> 
> > Actually, it doesn't depend on a specific GLES / EGL implementation,
> > but it does depend on Mesa for GBM. There are alternative
> > implementation of GBM as well, so having something like:
> > 
> > depends on BR2_PACKAGE_HAS_LIBEGL
> > depends on BR2_PACKAGE_HAS_LIBGLES
> > depends on BR2_PACKAGE_MESA3D # GBM implementation
> > 
> > Seems more natural
> 
> In principle, I would agree, but we pass --enable-gbm only when
> BR2_PACKAGE_MESA3D_OPENGL_EGL=y. So with your proposal, we could end up
> in a situation where libgbm isn't.

He, I thought this was a bad choice and why I said like :)

Obviously, this isn't a good candidate.

> Try a build with just BR2_PACKAGE_MESA3D=y, libgm isn't
> built/installed. So if we want to do this, perhaps we need a separate
> BR2_PACKAGE_MESA3D_LIBGBM option?

Maybe, we could also just depend on BR2_PACKAGE_MESA3D_OPENGL_EGL for
now (in addition to the virtual packages).

Moving gbm to a virtual package is quite far from now, but still
having something to grep for when we move to that would be great.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170215/b30d45d5/attachment.asc>

[Buildroot] [PATCH v2] kmscube: Add new package

[Thomas Petazzoni]

Hello,

On Wed, 15 Feb 2017 09:55:51 +0100, Maxime Ripard wrote:

> Moving gbm to a virtual package is quite far from now, but still
> having something to grep for when we move to that would be great.

To solve that, I think a simple comment is quite good, like:

package/efl/Config.in:  depends on BR2_PACKAGE_MESA3D_OPENGL_EGL # require libgbm from mesa3d
package/qt5/qt5base/qt5base.mk:# Uses libgbm from mesa3d
package/weston/Config.in:# Uses libgbm from mesa3d
package/x11r7/xdriver_xf86-video-amdgpu/Config.in:      depends on BR2_PACKAGE_MESA3D_OPENGL_EGL # gbm

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com