[Buildroot] [PATCH 1/2] package/cog: add option for platform DRM.

[Buildroot] [PATCH 1/2] package/cog: add option for platform DRM.

[Charlie Turner]

Signed-off-by: Charlie Turner <cturner@igalia.com>
---
 package/cog/Config.in | 6 ++++++
 package/cog/cog.mk    | 8 ++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/package/cog/Config.in b/package/cog/Config.in
index b25991d4ae..b260fa259c 100644
--- a/package/cog/Config.in
+++ b/package/cog/Config.in
@@ -26,4 +26,10 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
 	  string is used, there is no default and the URI to open
 	  must be always specified in the command line.
 
+config BR2_PACKAGE_COG_PLATFORM_DRM
+       bool "DRM backend"
+       depends on BR2_PACKAGE_LIBDRM
+       help
+         Enable the DRM platform backend. This allows Cog to run
+         without a compositor like Weston.
 endif
diff --git a/package/cog/cog.mk b/package/cog/cog.mk
index d0e5b79c38..0bbc684436 100644
--- a/package/cog/cog.mk
+++ b/package/cog/cog.mk
@@ -8,13 +8,17 @@ COG_VERSION = 0.4.0
 COG_SITE = https://wpewebkit.org/releases
 COG_SOURCE = cog-$(COG_VERSION).tar.xz
 COG_INSTALL_STAGING = YES
-COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
+COG_DEPENDENCIES = dbus wayland wpewebkit wpebackend-fdo
 COG_LICENSE = MIT
 COG_LICENSE_FILES = COPYING
 COG_CONF_OPTS = \
 	-DCOG_BUILD_PROGRAMS=ON \
 	-DCOG_PLATFORM_FDO=ON \
-	-DCOG_PLATFORM_DRM=OFF \
 	-DCOG_HOME_URI='$(call qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
 
+ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
+	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
+	COG_DEPENDENCIES += libdrm libinput
+endif
+
 $(eval $(cmake-package))
-- 
2.20.1

[Buildroot] [PATCH 2/2] board/raspberrypi: add post-image option for VC4 overlay.

[Charlie Turner]

It is often necessary to add a device tree overlay for the VC4 V3D
driver. On a Raspberry Pi 4 for example, if you enable the Gallium VC4
Mesa driver, you must add dtoverlay=vc4-fkms-v3d to the kernel command
line for it to work correctly. There are times when adding vc4-kms-v3d
is also required. This option allows post-image scripts to add either
option conveniently depending on their specific configuration.

Signed-off-by: Charlie Turner <cturner@igalia.com>
---
 board/raspberrypi/post-image.sh | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/board/raspberrypi/post-image.sh b/board/raspberrypi/post-image.sh
index 9dbd98ef9b..4ffda4bf49 100755
--- a/board/raspberrypi/post-image.sh
+++ b/board/raspberrypi/post-image.sh
@@ -11,7 +11,7 @@ for arg in "$@"
 do
 	case "${arg}" in
 		--add-miniuart-bt-overlay)
-		if ! grep -qE '^dtoverlay=' "${BINARIES_DIR}/rpi-firmware/config.txt"; then
+		if ! grep -qE '^dtoverlay=miniuart-bt' "${BINARIES_DIR}/rpi-firmware/config.txt"; then
 			echo "Adding 'dtoverlay=miniuart-bt' to config.txt (fixes ttyAMA0 serial console)."
 			cat << __EOF__ >> "${BINARIES_DIR}/rpi-firmware/config.txt"
 
@@ -36,6 +36,18 @@ __EOF__
 		gpu_mem="${arg:2}"
 		sed -e "/^${gpu_mem%=*}=/s,=.*,=${gpu_mem##*=}," -i "${BINARIES_DIR}/rpi-firmware/config.txt"
 		;;
+		--vc4-modesetting-overlay=*)
+		overlay=${arg##*=}
+		if ! grep -qE "^dtoverlay=vc4-.*-v3d" "${BINARIES_DIR}/rpi-firmware/config.txt"; then
+			echo "Adding 'dtoverlay=vc4-${overlay}-v3d' to config.txt."
+			cat << __EOF__ >> "${BINARIES_DIR}/rpi-firmware/config.txt"
+
+dtoverlay=vc4-${overlay}-v3d
+__EOF__
+		else
+			sed -e "/^dtoverlay=vc4-.*-v3d/s,=.*,=vc4-${overlay}-v3d," -i "${BINARIES_DIR}/rpi-firmware/config.txt"
+		fi
+		;;
 	esac
 
 done
-- 
2.20.1

[Buildroot] [PATCH 2/2] board/raspberrypi: add post-image option for VC4 overlay.

[Peter Seiderer]

Hello Charlie,

On Tue, 10 Mar 2020 11:22:27 +0000, Charlie Turner <cturner@igalia.com> wrote:

> It is often necessary to add a device tree overlay for the VC4 V3D
> driver. On a Raspberry Pi 4 for example, if you enable the Gallium VC4
> Mesa driver, you must add dtoverlay=vc4-fkms-v3d to the kernel command
> line for it to work correctly. There are times when adding vc4-kms-v3d
> is also required. This option allows post-image scripts to add either
> option conveniently depending on their specific configuration.

There where already some attempts for this (or similar) feature enhancement
(see [1], [2]), but maybe it is more stuff for an real custom setup (to keep
the buildroot minimal approach)?

>
> Signed-off-by: Charlie Turner <cturner@igalia.com>
> ---
>  board/raspberrypi/post-image.sh | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/board/raspberrypi/post-image.sh b/board/raspberrypi/post-image.sh
> index 9dbd98ef9b..4ffda4bf49 100755
> --- a/board/raspberrypi/post-image.sh
> +++ b/board/raspberrypi/post-image.sh
> @@ -11,7 +11,7 @@ for arg in "$@"
>  do
>  	case "${arg}" in
>  		--add-miniuart-bt-overlay)
> -		if ! grep -qE '^dtoverlay=' "${BINARIES_DIR}/rpi-firmware/config.txt"; then
> +		if ! grep -qE '^dtoverlay=miniuart-bt' "${BINARIES_DIR}/rpi-firmware/config.txt"; then
>  			echo "Adding 'dtoverlay=miniuart-bt' to config.txt (fixes ttyAMA0 serial console)."
>  			cat << __EOF__ >> "${BINARIES_DIR}/rpi-firmware/config.txt"
>
> @@ -36,6 +36,18 @@ __EOF__
>  		gpu_mem="${arg:2}"
>  		sed -e "/^${gpu_mem%=*}=/s,=.*,=${gpu_mem##*=}," -i "${BINARIES_DIR}/rpi-firmware/config.txt"
>  		;;
> +		--vc4-modesetting-overlay=*)

Call the option simple add-vc4-fkms-v3d-overlay for 'vc4-fkms-v3d.dtb', or make
it completely generic?

> +		overlay=${arg##*=}
> +		if ! grep -qE "^dtoverlay=vc4-.*-v3d" "${BINARIES_DIR}/rpi-firmware/config.txt"; then
> +			echo "Adding 'dtoverlay=vc4-${overlay}-v3d' to config.txt."
> +			cat << __EOF__ >> "${BINARIES_DIR}/rpi-firmware/config.txt"
> +
> +dtoverlay=vc4-${overlay}-v3d
> +__EOF__
> +		else
> +			sed -e "/^dtoverlay=vc4-.*-v3d/s,=.*,=vc4-${overlay}-v3d," -i "${BINARIES_DIR}/rpi-firmware/config.txt"
> +		fi
> +		;;
>  	esac
>
>  done

Regards,
Peter

[1] https://patchwork.ozlabs.org/patch/1007728
[2] http://lists.busybox.net/pipermail/buildroot/2019-February/243941.html

[Buildroot] [PATCH 2/2] board/raspberrypi: add post-image option for VC4 overlay.

[Charles Turner]

Hi Peter,

> On Tue, 10 Mar 2020 11:22:27 +0000, Charlie Turner <
> cturner at igalia.com> wrote:
> 
> > It is often necessary to add a device tree overlay for the VC4 V3D
> > driver. On a Raspberry Pi 4 for example, if you enable the Gallium
> > VC4
> > Mesa driver, you must add dtoverlay=vc4-fkms-v3d to the kernel
> > command
> > line for it to work correctly. There are times when adding vc4-kms-
> > v3d
> > is also required. This option allows post-image scripts to add
> > either
> > option conveniently depending on their specific configuration.
> 
> There where already some attempts for this (or similar) feature
> enhancement
> (see [1], [2]), but maybe it is more stuff for an real custom setup
> (to keep
> the buildroot minimal approach)?

[2] references [a] and [b]. [a] is a series implementing a lot of what
I considered good ideas, and [b] is the review mail which concludes
with,

"Again, my position is that, if one's project/hobby/... needs a custom
linux/uboot/firmware/etc... config file, then one should handle it
locally, using the files in Buildroot as inspiration."
   -- Yann E. MORIN.

In my specific case, I felt buildroot could feasibly deduce from my
selection of the VC4 gallium driver, that on the pi 4 I needed the FKMS
overlay. It took me a lot of research to figure that out, and while I'm
grateful to have learned new things, it does seem a bit unnecessary to
omit its addition.

OTOH, doing this correctly in all possible configurations is an
overwhelming thought for me, so I do sympathise with the approach of
not trying to ship a half-baked solution and instead letting users with
specific needs decide their configuration, which obviously is going to
require expertise with their target platform. The problem is we've
already kinda sorta got defaults logic in the post-image script. If we
took Yann's philosophy more seriously perhaps there should be no
config.txt provided at all...

From the "partial solutions are confusing" line of reason I'm inclined
to withdraw this patch.

[a] 
http://lists.busybox.net/pipermail/buildroot/2019-January/241809.html
[b] 
http://lists.busybox.net/pipermail/buildroot/2019-February/242704.html

> > @@ -36,6 +36,18 @@ __EOF__
> >  		gpu_mem="${arg:2}"
> >  		sed -e "/^${gpu_mem%=*}=/s,=.*,=${gpu_mem##*=}," -i
> > "${BINARIES_DIR}/rpi-firmware/config.txt"
> >  		;;
> > +		--vc4-modesetting-overlay=*)
> 
> Call the option simple add-vc4-fkms-v3d-overlay for 'vc4-fkms-
> v3d.dtb', or make
> it completely generic?

As above I'm inclined to withdraw the patch. If I were to have
continued, after reading your references I would take the approach of
providing a general --config-txt-option=foo=bar option instead of
special casing what I needed in the moment.

B.R
	Charlie.

[Buildroot] [PATCH 1/2] package/cog: add option for platform DRM.

[Peter Seiderer]

Hello Charlie,

On Tue, 10 Mar 2020 11:22:26 +0000, Charlie Turner <cturner@igalia.com> wrote:

> Signed-off-by: Charlie Turner <cturner@igalia.com>
> ---
>  package/cog/Config.in | 6 ++++++
>  package/cog/cog.mk    | 8 ++++++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/package/cog/Config.in b/package/cog/Config.in
> index b25991d4ae..b260fa259c 100644
> --- a/package/cog/Config.in
> +++ b/package/cog/Config.in
> @@ -26,4 +26,10 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
>  	  string is used, there is no default and the URI to open
>  	  must be always specified in the command line.
>
> +config BR2_PACKAGE_COG_PLATFORM_DRM
> +       bool "DRM backend"
> +       depends on BR2_PACKAGE_LIBDRM
> +       help
> +         Enable the DRM platform backend. This allows Cog to run
> +         without a compositor like Weston.
>  endif
> diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> index d0e5b79c38..0bbc684436 100644
> --- a/package/cog/cog.mk
> +++ b/package/cog/cog.mk
> @@ -8,13 +8,17 @@ COG_VERSION = 0.4.0
>  COG_SITE = https://wpewebkit.org/releases
>  COG_SOURCE = cog-$(COG_VERSION).tar.xz
>  COG_INSTALL_STAGING = YES
> -COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
> +COG_DEPENDENCIES = dbus wayland wpewebkit wpebackend-fdo

New option, unconditionally added new dependency on wayland?

>  COG_LICENSE = MIT
>  COG_LICENSE_FILES = COPYING
>  COG_CONF_OPTS = \
>  	-DCOG_BUILD_PROGRAMS=ON \
>  	-DCOG_PLATFORM_FDO=ON \
> -	-DCOG_PLATFORM_DRM=OFF \
>  	-DCOG_HOME_URI='$(call qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
>
> +ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
> +	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
> +	COG_DEPENDENCIES += libdrm libinput

missing:

#else
	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF

Regards,
Peter

> +endif
> +
>  $(eval $(cmake-package))

[Buildroot] [PATCH 1/2] package/cog: add option for platform DRM.

[Charles Turner]

Hi Peter,

On Tue, 2020-03-10 at 13:36 +0100, Peter Seiderer wrote:
> > diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> > index d0e5b79c38..0bbc684436 100644
> > --- a/package/cog/cog.mk
> > +++ b/package/cog/cog.mk
> > @@ -8,13 +8,17 @@ COG_VERSION = 0.4.0
> >  COG_SITE = https://wpewebkit.org/releases
> >  COG_SOURCE = cog-$(COG_VERSION).tar.xz
> >  COG_INSTALL_STAGING = YES
> > -COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
> > +COG_DEPENDENCIES = dbus wayland wpewebkit wpebackend-fdo
> 
> New option, unconditionally added new dependency on wayland?

Perhaps I should place that in a separate patch. Cog does depend on
wayland with wpewebkit, which is why I reflex-added it to the deps.
It's already depended on from the wpewebkit dependency, but I didn't
like relying on a separate packages dependency list for this package,
especially when Cog can be build with webkitgtk instead, in which case
it would not need the wayland dep (although I wonder if anyone has
tested that in buildroot yet :-))

From this reasoning, I will remove this added dependency on wayland.

> 
> >  COG_LICENSE = MIT
> >  COG_LICENSE_FILES = COPYING
> >  COG_CONF_OPTS = \
> >  	-DCOG_BUILD_PROGRAMS=ON \
> >  	-DCOG_PLATFORM_FDO=ON \
> > -	-DCOG_PLATFORM_DRM=OFF \
> >  	-DCOG_HOME_URI='$(call
> > qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
> > 
> > +ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
> > +	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
> > +	COG_DEPENDENCIES += libdrm libinput
> 
> missing:
> 
> #else
> 	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF

This is default off in CMake, which is why I removed it from the .mk
file here.

B.R
	Charlie.

[Buildroot] [PATCH 1/2] package/cog: add option for platform DRM.

[Peter Seiderer]

Hello Charles,

On Tue, 10 Mar 2020 13:49:42 +0000, Charles Turner <cturner@igalia.com> wrote:

> Hi Peter,
>
> On Tue, 2020-03-10 at 13:36 +0100, Peter Seiderer wrote:
> > > diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> > > index d0e5b79c38..0bbc684436 100644
> > > --- a/package/cog/cog.mk
> > > +++ b/package/cog/cog.mk
> > > @@ -8,13 +8,17 @@ COG_VERSION = 0.4.0
> > >  COG_SITE = https://wpewebkit.org/releases
> > >  COG_SOURCE = cog-$(COG_VERSION).tar.xz
> > >  COG_INSTALL_STAGING = YES
> > > -COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
> > > +COG_DEPENDENCIES = dbus wayland wpewebkit wpebackend-fdo
> >
> > New option, unconditionally added new dependency on wayland?
>
> Perhaps I should place that in a separate patch. Cog does depend on
> wayland with wpewebkit, which is why I reflex-added it to the deps.
> It's already depended on from the wpewebkit dependency, but I didn't
> like relying on a separate packages dependency list for this package,
> especially when Cog can be build with webkitgtk instead, in which case
> it would not need the wayland dep (although I wonder if anyone has
> tested that in buildroot yet :-))

Thanks for explanation...

>
> From this reasoning, I will remove this added dependency on wayland.
>
> >
> > >  COG_LICENSE = MIT
> > >  COG_LICENSE_FILES = COPYING
> > >  COG_CONF_OPTS = \
> > >  	-DCOG_BUILD_PROGRAMS=ON \
> > >  	-DCOG_PLATFORM_FDO=ON \
> > > -	-DCOG_PLATFORM_DRM=OFF \
> > >  	-DCOG_HOME_URI='$(call
> > > qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
> > >
> > > +ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
> > > +	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
> > > +	COG_DEPENDENCIES += libdrm libinput
> >
> > missing:
> >
> > #else
> > 	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF
>
> This is default off in CMake, which is why I removed it from the .mk
> file here.

Defaults can change, explicit setting preferred ;-)

Regards,
Peter

>
> B.R
> 	Charlie.
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH v2 1/1] package/cog: add option for platform DRM.

[Charlie Turner]

Signed-off-by: Charlie Turner <cturner@igalia.com>
---
 package/cog/Config.in | 6 ++++++
 package/cog/cog.mk    | 8 +++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/package/cog/Config.in b/package/cog/Config.in
index b25991d4ae..b260fa259c 100644
--- a/package/cog/Config.in
+++ b/package/cog/Config.in
@@ -26,4 +26,10 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
 	  string is used, there is no default and the URI to open
 	  must be always specified in the command line.
 
+config BR2_PACKAGE_COG_PLATFORM_DRM
+       bool "DRM backend"
+       depends on BR2_PACKAGE_LIBDRM
+       help
+         Enable the DRM platform backend. This allows Cog to run
+         without a compositor like Weston.
 endif
diff --git a/package/cog/cog.mk b/package/cog/cog.mk
index d0e5b79c38..4697fdf6ed 100644
--- a/package/cog/cog.mk
+++ b/package/cog/cog.mk
@@ -14,7 +14,13 @@ COG_LICENSE_FILES = COPYING
 COG_CONF_OPTS = \
 	-DCOG_BUILD_PROGRAMS=ON \
 	-DCOG_PLATFORM_FDO=ON \
-	-DCOG_PLATFORM_DRM=OFF \
 	-DCOG_HOME_URI='$(call qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
 
+ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
+	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
+	COG_DEPENDENCIES += libdrm libinput
+else
+	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF
+endif
+
 $(eval $(cmake-package))
-- 
2.20.1

[Buildroot] [PATCH v2 1/1] package/cog: add option for platform DRM.

[Adrian Perez de Castro]

On Wed, 11 Mar 2020 10:30:44 +0000, Charlie Turner <cturner@igalia.com> wrote:
> Signed-off-by: Charlie Turner <cturner@igalia.com>

Reviewed-by: Adrian Perez de Castro <aperez@igalia.com>

> ---
>  package/cog/Config.in | 6 ++++++
>  package/cog/cog.mk    | 8 +++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/package/cog/Config.in b/package/cog/Config.in
> index b25991d4ae..b260fa259c 100644
> --- a/package/cog/Config.in
> +++ b/package/cog/Config.in
> @@ -26,4 +26,10 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
>  	  string is used, there is no default and the URI to open
>  	  must be always specified in the command line.
>  
> +config BR2_PACKAGE_COG_PLATFORM_DRM
> +       bool "DRM backend"
> +       depends on BR2_PACKAGE_LIBDRM
> +       help
> +         Enable the DRM platform backend. This allows Cog to run
> +         without a compositor like Weston.
>  endif
> diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> index d0e5b79c38..4697fdf6ed 100644
> --- a/package/cog/cog.mk
> +++ b/package/cog/cog.mk
> @@ -14,7 +14,13 @@ COG_LICENSE_FILES = COPYING
>  COG_CONF_OPTS = \
>  	-DCOG_BUILD_PROGRAMS=ON \
>  	-DCOG_PLATFORM_FDO=ON \
> -	-DCOG_PLATFORM_DRM=OFF \
>  	-DCOG_HOME_URI='$(call qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
>  
> +ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
> +	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
> +	COG_DEPENDENCIES += libdrm libinput
> +else
> +	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF
> +endif
> +
>  $(eval $(cmake-package))
> -- 
> 2.20.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20200311/72c6a483/attachment.asc>

[Buildroot] [PATCH v2 1/1] package/cog: add option for platform DRM.

[Thomas Petazzoni]

Hello Charlie,

Thanks for your patch. A couple of comments/questions below. Most are
trivial stuff, but one question requires some cog knowledge that I
don't have.

On Wed, 11 Mar 2020 10:30:44 +0000
Charlie Turner <cturner@igalia.com> wrote:

> Signed-off-by: Charlie Turner <cturner@igalia.com>
> ---
>  package/cog/Config.in | 6 ++++++
>  package/cog/cog.mk    | 8 +++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/package/cog/Config.in b/package/cog/Config.in
> index b25991d4ae..b260fa259c 100644
> --- a/package/cog/Config.in
> +++ b/package/cog/Config.in
> @@ -26,4 +26,10 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
>  	  string is used, there is no default and the URI to open
>  	  must be always specified in the command line.
>  
> +config BR2_PACKAGE_COG_PLATFORM_DRM
> +       bool "DRM backend"
> +       depends on BR2_PACKAGE_LIBDRM

We typically don't use "depends on" for such dependencies, but a
"select". That will require however that you replicate the "depends on"
dependencies of BR2_PACKAGE_LIBDRM here.

Also, your .mk file adds a dependency on libinput, so you need a
"select BR2_PACKAGE_LIBINPUT" here.

> diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> index d0e5b79c38..4697fdf6ed 100644
> --- a/package/cog/cog.mk
> +++ b/package/cog/cog.mk
> @@ -14,7 +14,13 @@ COG_LICENSE_FILES = COPYING
>  COG_CONF_OPTS = \
>  	-DCOG_BUILD_PROGRAMS=ON \
>  	-DCOG_PLATFORM_FDO=ON \
> -	-DCOG_PLATFORM_DRM=OFF \

So, now that a second "platform" is supported, do we want the "fdo"
platform to be always unconditionally enabled ? What is the "fdo"
platform compared to the "drm" platform ?

>  	-DCOG_HOME_URI='$(call qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
>  
> +ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
> +	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
> +	COG_DEPENDENCIES += libdrm libinput

Those lines should not be indented with one tab.

> +else
> +	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF

Ditto.

> +endif
> +
>  $(eval $(cmake-package))

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 1/1] package/cog: add option for platform DRM.

[Charles Turner]

Hi Thomas, thanks for your review.

> > diff --git a/package/cog/Config.in b/package/cog/Config.in
> > index b25991d4ae..b260fa259c 100644
> > --- a/package/cog/Config.in
> > +++ b/package/cog/Config.in
> > @@ -26,4 +26,10 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
> >  	  string is used, there is no default and the URI to open
> >  	  must be always specified in the command line.
> >  
> > +config BR2_PACKAGE_COG_PLATFORM_DRM
> > +       bool "DRM backend"
> > +       depends on BR2_PACKAGE_LIBDRM
> 
> We typically don't use "depends on" for such dependencies, but a
> "select". That will require however that you replicate the "depends
> on"
> dependencies of BR2_PACKAGE_LIBDRM here.
> 

The libdrm package heavily depends on the target hardware for what
dependencies it has. In my case the dependencies of libdrm are simply
BR2_aarch64 || BR2_arm because I'm using the VC4 driver on the
Raspberry Pi, but in general how can I replicate the "depends on" in
cog, and should I really do that?

Cog also selects dbus, but I note that dbus' dependencies are not
replicated in Cog's .mk file either (e.g expat). Could select on not
auto-update the dependencies of the selecting package with the selected
package?

It looks like the bug I would be fixing by moving to select from
depends on is compilation order. I didn't realise that its possible cog
could get built before libdrm with only depends on (though unlikely for
other reasons), but I think select forces compilation order? I can run
some experiments later but its a bit of a costly test in terms of time,
couldn't convince myself either way from the Kconfig docs.

> Also, your .mk file adds a dependency on libinput, so you need a
> "select BR2_PACKAGE_LIBINPUT" here.

Done.

> 
> > diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> > index d0e5b79c38..4697fdf6ed 100644
> > --- a/package/cog/cog.mk
> > +++ b/package/cog/cog.mk
> > @@ -14,7 +14,13 @@ COG_LICENSE_FILES = COPYING
> >  COG_CONF_OPTS = \
> >  	-DCOG_BUILD_PROGRAMS=ON \
> >  	-DCOG_PLATFORM_FDO=ON \
> > -	-DCOG_PLATFORM_DRM=OFF \
> 
> So, now that a second "platform" is supported, do we want the "fdo"
> platform to be always unconditionally enabled ? What is the "fdo"
> platform compared to the "drm" platform ?

I didn't think too hard about keeping it default enabled. When I
started this work the DRM platform was experimental and FDO was very
much a default. I am not a developer of Cog nor an expert in the
graphics stack, but how I think about FDO is that all the logic of how
an image is painted to the screen is handled inside Cog using
components from the FreeDesktop.org stack (hence the name FDO). With
DRM Cog just hands painting off to the framebuffer with DRM/KMS.
However, the FDO "platform" does seem to be required by the DRM
"platform", there appears to be dmabuf related functionality provided
by the FDO backend that is needed in the DRM platform as well. The
naming and organization here is very unfortunate.

> 
> >  	-DCOG_HOME_URI='$(call
> > qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
> >  
> > +ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
> > +	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
> > +	COG_DEPENDENCIES += libdrm libinput
> 
> Those lines should not be indented with one tab.

I removed the indent.

> 
> > +else
> > +	COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF
> 
> Ditto.

Done.

Best,
	Charlie.

[Buildroot] [PATCH v2 1/1] package/cog: add option for platform DRM.

[Thomas Petazzoni]

Hello Charles,

On Thu, 12 Mar 2020 13:11:49 +0000
Charles Turner <cturner@igalia.com> wrote:

> The libdrm package heavily depends on the target hardware for what
> dependencies it has. In my case the dependencies of libdrm are simply
> BR2_aarch64 || BR2_arm because I'm using the VC4 driver on the
> Raspberry Pi, but in general how can I replicate the "depends on" in
> cog, and should I really do that?

cog only needs libdrm itself to build, which specific DRM driver is
needed is up to the user to figure out and we will certainly not
replicate all the DRM drivers dependencies to the cog package.

The other packages that use libdrm mostly "select BR2_PACKAGE_LIBDRM".

> Cog also selects dbus, but I note that dbus' dependencies are not
> replicated in Cog's .mk file either (e.g expat). Could select on not
> auto-update the dependencies of the selecting package with the selected
> package?

BR2_PACKAGE_EXPAT is selected by BR2_PACKAGE_DBUS, so this is not
something you need to propagate to cog.

What needs to be propagated to cog are the "depends on" that
BR2_PACKAGE_DBUS has on other options. And this is already done
correctly. See BR2_PACKAGE_DBUS:

config BR2_PACKAGE_DBUS
        bool "dbus"
        depends on BR2_TOOLCHAIN_HAS_THREADS
        # uses fork()
        depends on BR2_USE_MMU

We have depends on BR2_USE_MMU and depends on
BR2_TOOLCHAIN_HAS_THREADS. And cog has:

config BR2_PACKAGE_COG
        bool "cog"
        depends on BR2_PACKAGE_WPEWEBKIT
        depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
        depends on BR2_USE_MMU # dbus

So we're good.

It seems like you're not familiar with how kconfig behaves. A "select"
is stronger than a "depends on", so for example if you have:

config A
	bool

config B
	bool
	depends on A

config C
	bool
	select B

Then it is possible to enable C, which will select B, without A being
enabled, which of course is incorrect. This is known bug/limitation of
kconfig, which we work around in Buildroot by propagating "depends on"
dependencies this way:

config A
	bool

config B
	bool
	depends on A

config C
	bool
	depends on A # b
	select B


> It looks like the bug I would be fixing by moving to select from
> depends on is compilation order.

No. Whatever you put in Config.in file has absolutely no impact
whatsoever on compilation order. The build order is defined by the
<pkg>_DEPENDENCIES variable in the .mk file.


> > > diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> > > index d0e5b79c38..4697fdf6ed 100644
> > > --- a/package/cog/cog.mk
> > > +++ b/package/cog/cog.mk
> > > @@ -14,7 +14,13 @@ COG_LICENSE_FILES = COPYING
> > >  COG_CONF_OPTS = \
> > >  	-DCOG_BUILD_PROGRAMS=ON \
> > >  	-DCOG_PLATFORM_FDO=ON \
> > > -	-DCOG_PLATFORM_DRM=OFF \  
> > 
> > So, now that a second "platform" is supported, do we want the "fdo"
> > platform to be always unconditionally enabled ? What is the "fdo"
> > platform compared to the "drm" platform ?  
> 
> I didn't think too hard about keeping it default enabled. When I
> started this work the DRM platform was experimental and FDO was very
> much a default. I am not a developer of Cog nor an expert in the
> graphics stack, but how I think about FDO is that all the logic of how
> an image is painted to the screen is handled inside Cog using
> components from the FreeDesktop.org stack (hence the name FDO). With
> DRM Cog just hands painting off to the framebuffer with DRM/KMS.
> However, the FDO "platform" does seem to be required by the DRM
> "platform", there appears to be dmabuf related functionality provided
> by the FDO backend that is needed in the DRM platform as well. The
> naming and organization here is very unfortunate.

If the DRM platform does need the FDO platform, then indeed what you
have done in keeping the FDO platform unconditionally enabled is fine.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2 1/1] package/cog: add option for platform DRM.

[Charles Turner]

Hi Thomas,

On Thu, 2020-03-12 at 14:36 +0100, Thomas Petazzoni wrote:
> On Thu, 12 Mar 2020 13:11:49 +0000
> Charles Turner <cturner@igalia.com> wrote:
> 
> > The libdrm package heavily depends on the target hardware for what
> > dependencies it has. In my case the dependencies of libdrm are
> > simply
> > BR2_aarch64 || BR2_arm because I'm using the VC4 driver on the
> > Raspberry Pi, but in general how can I replicate the "depends on"
> > in
> > cog, and should I really do that?
> 
> cog only needs libdrm itself to build, which specific DRM driver is
> needed is up to the user to figure out and we will certainly not
> replicate all the DRM drivers dependencies to the cog package.

In other words, the replication is only needed for the non-configurable 
parts of the dependent package?

> 
> The other packages that use libdrm mostly "select
> BR2_PACKAGE_LIBDRM".
> 
> > Cog also selects dbus, but I note that dbus' dependencies are not
> > replicated in Cog's .mk file either (e.g expat). Could select on
> > not
> > auto-update the dependencies of the selecting package with the
> > selected
> > package?
> 
> BR2_PACKAGE_EXPAT is selected by BR2_PACKAGE_DBUS, so this is not
> something you need to propagate to cog.

OK, *selected*, not depended on. Think I have it now.

> If the DRM platform does need the FDO platform, then indeed what you
> have done in keeping the FDO platform unconditionally enabled is
> fine.

What I mean (and why the naming is unfortunate) is that I can have one
of DRM or FDO "platform", but both rely on the `wpebackend-fdo`
package.

Another thing, the DRM platform needs libgbm, I'm not sure how to
encode this dependency. There could be multiple providers of GBM, so
selecting one like `select BR2_PACKAGE_MESA3D_GBM` is wrong. Cog also
needs EGL, which can be encoded regardless of the implementor using
`depends on BR2_PACKAGE_HAS_LIBEGL`. Do I need to add
`BR2_PACKAGE_HAS_LIBGBM` to encode the gbm dependency properly? Other
packages just `depends on BR2_PACKAGE_MESA3D_OPENGL_EGL` to declare
their dependency on GBM, because that transitively depends on
BR2_PACKAGE_MESA3D_GBM. However, it looks like we have at least one
other GBM implementation in the TI related targets
(BR2_PACKAGE_TI_SGX_LIBGBM), so I am unsure about this approach.

Coordination: I will be away till next Monday.

Thanks for your explanations, hopefully the dependency logic will be
tighter in the next iteration of the patch.

Best,
	Charlie.

[Buildroot] [PATCH v3 1/1] package/cog: add option for platform DRM.

[Charlie Turner]

Now that there's two platforms, the default-on behaviour for FDO has
been made configurable, so that you can choose DRM only platform for FDO
platform. Don't be confused that in both cases the *wpebackend-fdo*
package is required. This is an unfortunate naming issue.

Signed-off-by: Charlie Turner <cturner@igalia.com>
---
 package/cog/Config.in | 27 +++++++++++++++++++++++++++
 package/cog/cog.mk    | 18 +++++++++++++++---
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/package/cog/Config.in b/package/cog/Config.in
index b25991d4ae..664e055104 100644
--- a/package/cog/Config.in
+++ b/package/cog/Config.in
@@ -7,7 +7,11 @@ config BR2_PACKAGE_COG
 	depends on BR2_PACKAGE_WPEWEBKIT
 	depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
 	depends on BR2_USE_MMU # dbus
+	# For egl across all platforms, and gbm in the DRM platform.
+	depends on BR2_PACKAGE_MESA3D_OPENGL_EGL
+
 	select BR2_PACKAGE_DBUS
+
 	help
 	  Single "window" launcher for the WebKit WPE port, and
 	  helper library for implementing WPE launcher. It does
@@ -26,4 +30,27 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
 	  string is used, there is no default and the URI to open
 	  must be always specified in the command line.
 
+config BR2_PACKAGE_COG_PLATFORM_FDO
+	bool "FreeDesktop.org backend"
+
+	select BR2_PACKAGE_LIBXKBCOMMON
+
+	help
+	  Enable the FreeDesktop.org backend.
+
+config BR2_PACKAGE_COG_PLATFORM_DRM
+	bool "DRM backend"
+
+	depends on BR2_PACKAGE_HAS_UDEV # libinput
+
+	select BR2_PACKAGE_LIBDRM
+	select BR2_PACKAGE_LIBINPUT
+
+	help
+	  Enable the DRM platform backend. This allows Cog to run
+	  without a compositor like Weston.
+
+comment "DRM platform needs mesa3d w/ EGL driver and GBM"
+	depends on !BR2_PACKAGE_MESA3D_OPENGL_EGL
+
 endif
diff --git a/package/cog/cog.mk b/package/cog/cog.mk
index d0e5b79c38..339187126a 100644
--- a/package/cog/cog.mk
+++ b/package/cog/cog.mk
@@ -8,13 +8,25 @@ COG_VERSION = 0.4.0
 COG_SITE = https://wpewebkit.org/releases
 COG_SOURCE = cog-$(COG_VERSION).tar.xz
 COG_INSTALL_STAGING = YES
-COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
+COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo wayland wayland-protocols
 COG_LICENSE = MIT
 COG_LICENSE_FILES = COPYING
 COG_CONF_OPTS = \
 	-DCOG_BUILD_PROGRAMS=ON \
-	-DCOG_PLATFORM_FDO=ON \
-	-DCOG_PLATFORM_DRM=OFF \
 	-DCOG_HOME_URI='$(call qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
 
+ifeq ($(BR2_PACKAGE_COG_PLATFORM_FDO),y)
+COG_CONF_OPTS += -DCOG_PLATFORM_FDO=ON
+COG_DEPENDENCIES += libxkbcommon
+else
+COG_CONF_OPTS += -DCOG_PLATFORM_FDO=OFF
+endif
+
+ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
+COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
+COG_DEPENDENCIES += libdrm libinput
+else
+COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF
+endif
+
 $(eval $(cmake-package))
-- 
2.20.1

[Buildroot] [PATCH v3 1/1] package/cog: add option for platform DRM.

[Charles Turner]

Gentle ping...

On Thu, 2020-03-12 at 19:47 +0000, Charlie Turner wrote:
> Now that there's two platforms, the default-on behaviour for FDO has
> been made configurable, so that you can choose DRM only platform for
> FDO
> platform. Don't be confused that in both cases the *wpebackend-fdo*
> package is required. This is an unfortunate naming issue.
> 
> Signed-off-by: Charlie Turner <cturner@igalia.com>
> ---
>  package/cog/Config.in | 27 +++++++++++++++++++++++++++
>  package/cog/cog.mk    | 18 +++++++++++++++---
>  2 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/package/cog/Config.in b/package/cog/Config.in
> index b25991d4ae..664e055104 100644
> --- a/package/cog/Config.in
> +++ b/package/cog/Config.in
> @@ -7,7 +7,11 @@ config BR2_PACKAGE_COG
>  	depends on BR2_PACKAGE_WPEWEBKIT
>  	depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
>  	depends on BR2_USE_MMU # dbus
> +	# For egl across all platforms, and gbm in the DRM platform.
> +	depends on BR2_PACKAGE_MESA3D_OPENGL_EGL
> +
>  	select BR2_PACKAGE_DBUS
> +
>  	help
>  	  Single "window" launcher for the WebKit WPE port, and
>  	  helper library for implementing WPE launcher. It does
> @@ -26,4 +30,27 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
>  	  string is used, there is no default and the URI to open
>  	  must be always specified in the command line.
>  
> +config BR2_PACKAGE_COG_PLATFORM_FDO
> +	bool "FreeDesktop.org backend"
> +
> +	select BR2_PACKAGE_LIBXKBCOMMON
> +
> +	help
> +	  Enable the FreeDesktop.org backend.
> +
> +config BR2_PACKAGE_COG_PLATFORM_DRM
> +	bool "DRM backend"
> +
> +	depends on BR2_PACKAGE_HAS_UDEV # libinput
> +
> +	select BR2_PACKAGE_LIBDRM
> +	select BR2_PACKAGE_LIBINPUT
> +
> +	help
> +	  Enable the DRM platform backend. This allows Cog to run
> +	  without a compositor like Weston.
> +
> +comment "DRM platform needs mesa3d w/ EGL driver and GBM"
> +	depends on !BR2_PACKAGE_MESA3D_OPENGL_EGL
> +
>  endif
> diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> index d0e5b79c38..339187126a 100644
> --- a/package/cog/cog.mk
> +++ b/package/cog/cog.mk
> @@ -8,13 +8,25 @@ COG_VERSION = 0.4.0
>  COG_SITE = https://wpewebkit.org/releases
>  COG_SOURCE = cog-$(COG_VERSION).tar.xz
>  COG_INSTALL_STAGING = YES
> -COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
> +COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo wayland wayland-
> protocols
>  COG_LICENSE = MIT
>  COG_LICENSE_FILES = COPYING
>  COG_CONF_OPTS = \
>  	-DCOG_BUILD_PROGRAMS=ON \
> -	-DCOG_PLATFORM_FDO=ON \
> -	-DCOG_PLATFORM_DRM=OFF \
>  	-DCOG_HOME_URI='$(call
> qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
>  
> +ifeq ($(BR2_PACKAGE_COG_PLATFORM_FDO),y)
> +COG_CONF_OPTS += -DCOG_PLATFORM_FDO=ON
> +COG_DEPENDENCIES += libxkbcommon
> +else
> +COG_CONF_OPTS += -DCOG_PLATFORM_FDO=OFF
> +endif
> +
> +ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
> +COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
> +COG_DEPENDENCIES += libdrm libinput
> +else
> +COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF
> +endif
> +
>  $(eval $(cmake-package))

[Buildroot] [PATCH v3 1/1] package/cog: add option for platform DRM.

[Adrian Perez de Castro]

Hello Charlie,

There was earlier a patch to update to Cog 0.6; I think it should not have
conflicts with this one, but you may want to keep an eye on it:

    https://patchwork.ozlabs.org/patch/1264879/


On Thu, 12 Mar 2020 19:47:00 +0000, Charlie Turner <cturner@igalia.com> wrote:

> Now that there's two platforms, the default-on behaviour for FDO has
> been made configurable, so that you can choose DRM only platform for FDO
> platform. Don't be confused that in both cases the *wpebackend-fdo*
> package is required. This is an unfortunate naming issue.

Whoops! You just made me realize that the naming of WPEBackend-fdo is
not great. Unfortunately, it's a bit late to change it at this point O:-)

> Signed-off-by: Charlie Turner <cturner@igalia.com>
> ---
>  package/cog/Config.in | 27 +++++++++++++++++++++++++++
>  package/cog/cog.mk    | 18 +++++++++++++++---
>  2 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/package/cog/Config.in b/package/cog/Config.in
> index b25991d4ae..664e055104 100644
> --- a/package/cog/Config.in
> +++ b/package/cog/Config.in
> @@ -7,7 +7,11 @@ config BR2_PACKAGE_COG
>  	depends on BR2_PACKAGE_WPEWEBKIT
>  	depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
>  	depends on BR2_USE_MMU # dbus
> +	# For egl across all platforms, and gbm in the DRM platform.
> +	depends on BR2_PACKAGE_MESA3D_OPENGL_EGL

Cog's FDO plug-in works with drivers other than Mesa (for example the
Vivante proprietary driver on i.MX hardware). This dependency should
be under BR2_PACKAGE_COG_PLATFORM_DRM instead.

> +
>  	select BR2_PACKAGE_DBUS
> +
>  	help
>  	  Single "window" launcher for the WebKit WPE port, and
>  	  helper library for implementing WPE launcher. It does
> @@ -26,4 +30,27 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
>  	  string is used, there is no default and the URI to open
>  	  must be always specified in the command line.
>  
> +config BR2_PACKAGE_COG_PLATFORM_FDO
> +	bool "FreeDesktop.org backend"
> +
> +	select BR2_PACKAGE_LIBXKBCOMMON
> +
> +	help
> +	  Enable the FreeDesktop.org backend.

I would add in the help text that it uses Wayland and needs a compositor.

> +
> +config BR2_PACKAGE_COG_PLATFORM_DRM
> +	bool "DRM backend"
> +
> +	depends on BR2_PACKAGE_HAS_UDEV # libinput
> +
> +	select BR2_PACKAGE_LIBDRM
> +	select BR2_PACKAGE_LIBINPUT
> +
> +	help
> +	  Enable the DRM platform backend. This allows Cog to run
> +	  without a compositor like Weston.

Instead of telling what it does not need to run, wouldn't it be better to
write here that it runs directly on video drivers which support kernel mode
setting (KMS) and Direct Rendering Manager?

> +
> +comment "DRM platform needs mesa3d w/ EGL driver and GBM"
> +	depends on !BR2_PACKAGE_MESA3D_OPENGL_EGL
                                              || !BR2_PACKAGE_HAS_UDEV

> +
>  endif
> diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> index d0e5b79c38..339187126a 100644
> --- a/package/cog/cog.mk
> +++ b/package/cog/cog.mk
> @@ -8,13 +8,25 @@ COG_VERSION = 0.4.0
>  COG_SITE = https://wpewebkit.org/releases
>  COG_SOURCE = cog-$(COG_VERSION).tar.xz
>  COG_INSTALL_STAGING = YES
> -COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
> +COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo wayland wayland-protocols
>  COG_LICENSE = MIT
>  COG_LICENSE_FILES = COPYING
>  COG_CONF_OPTS = \
>  	-DCOG_BUILD_PROGRAMS=ON \
> -	-DCOG_PLATFORM_FDO=ON \
> -	-DCOG_PLATFORM_DRM=OFF \
>  	-DCOG_HOME_URI='$(call qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
>  
> +ifeq ($(BR2_PACKAGE_COG_PLATFORM_FDO),y)
> +COG_CONF_OPTS += -DCOG_PLATFORM_FDO=ON
> +COG_DEPENDENCIES += libxkbcommon
> +else
> +COG_CONF_OPTS += -DCOG_PLATFORM_FDO=OFF
> +endif
> +
> +ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
> +COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
> +COG_DEPENDENCIES += libdrm libinput
> +else
> +COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF
> +endif
> +
>  $(eval $(cmake-package))
> -- 
> 2.20.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20200401/1277f9d3/attachment.asc>

[Buildroot] [PATCH v4] package/cog: add option for platform DRM.

[Charlie Turner]

Now that there's two platforms, the default-on behaviour for FDO has
been made configurable, so that you can choose DRM only platform for FDO
platform. Don't be confused that in both cases the *wpebackend-fdo*
package is required. This is an unfortunate naming issue.

Signed-off-by: Charlie Turner <cturner@igalia.com>
---
Changes v3 -> v4:
  - Depend on BR2_PACKAGE_MESA3D_OPENGL_EGL only for the DRM platform,
    rather than FDO and DRM. Not clear how to "require EGL" when it
    can come from Mesa or a board-specific provider.
  - Fixup documentation as per Adri?n's suggestions.

 package/cog/Config.in | 28 ++++++++++++++++++++++++++++
 package/cog/cog.mk    | 18 +++++++++++++++---
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/package/cog/Config.in b/package/cog/Config.in
index b25991d4ae..6a7c5668a5 100644
--- a/package/cog/Config.in
+++ b/package/cog/Config.in
@@ -7,7 +7,9 @@ config BR2_PACKAGE_COG
 	depends on BR2_PACKAGE_WPEWEBKIT
 	depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
 	depends on BR2_USE_MMU # dbus
+
 	select BR2_PACKAGE_DBUS
+
 	help
 	  Single "window" launcher for the WebKit WPE port, and
 	  helper library for implementing WPE launcher. It does
@@ -26,4 +28,30 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
 	  string is used, there is no default and the URI to open
 	  must be always specified in the command line.
 
+config BR2_PACKAGE_COG_PLATFORM_FDO
+	bool "FreeDesktop.org backend"
+
+	select BR2_PACKAGE_LIBXKBCOMMON
+
+	help
+	  Enable the FreeDesktop.org backend. Cog will interface with
+	  a compositor over the Wayland protocol.
+
+config BR2_PACKAGE_COG_PLATFORM_DRM
+	bool "DRM backend"
+
+	depends on BR2_PACKAGE_HAS_UDEV # libinput
+	depends on BR2_PACKAGE_MESA3D_OPENGL_EGL # gbm
+
+	select BR2_PACKAGE_LIBDRM
+	select BR2_PACKAGE_LIBINPUT
+
+	help
+	  Enable the DRM platform backend. Cog will interface directly
+	  with video drivers that support kernel mode-setting (KMS)
+	  via the DRM user-space API.
+
+comment "DRM platform needs mesa3d w/ EGL driver and GBM"
+	depends on !BR2_PACKAGE_MESA3D_OPENGL_EGL
+
 endif
diff --git a/package/cog/cog.mk b/package/cog/cog.mk
index d0e5b79c38..339187126a 100644
--- a/package/cog/cog.mk
+++ b/package/cog/cog.mk
@@ -8,13 +8,25 @@ COG_VERSION = 0.4.0
 COG_SITE = https://wpewebkit.org/releases
 COG_SOURCE = cog-$(COG_VERSION).tar.xz
 COG_INSTALL_STAGING = YES
-COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
+COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo wayland wayland-protocols
 COG_LICENSE = MIT
 COG_LICENSE_FILES = COPYING
 COG_CONF_OPTS = \
 	-DCOG_BUILD_PROGRAMS=ON \
-	-DCOG_PLATFORM_FDO=ON \
-	-DCOG_PLATFORM_DRM=OFF \
 	-DCOG_HOME_URI='$(call qstrip,$(BR2_PACKAGE_COG_PROGRAMS_HOME_URI))'
 
+ifeq ($(BR2_PACKAGE_COG_PLATFORM_FDO),y)
+COG_CONF_OPTS += -DCOG_PLATFORM_FDO=ON
+COG_DEPENDENCIES += libxkbcommon
+else
+COG_CONF_OPTS += -DCOG_PLATFORM_FDO=OFF
+endif
+
+ifeq ($(BR2_PACKAGE_COG_PLATFORM_DRM),y)
+COG_CONF_OPTS += -DCOG_PLATFORM_DRM=ON
+COG_DEPENDENCIES += libdrm libinput
+else
+COG_CONF_OPTS += -DCOG_PLATFORM_DRM=OFF
+endif
+
 $(eval $(cmake-package))
-- 
2.20.1

[Buildroot] [PATCH v4] package/cog: add option for platform DRM.

[Thomas Petazzoni]

Hello,

On Thu,  2 Apr 2020 12:34:55 +0100
Charlie Turner <cturner@igalia.com> wrote:

> diff --git a/package/cog/Config.in b/package/cog/Config.in
> index b25991d4ae..6a7c5668a5 100644
> --- a/package/cog/Config.in
> +++ b/package/cog/Config.in
> @@ -7,7 +7,9 @@ config BR2_PACKAGE_COG
>  	depends on BR2_PACKAGE_WPEWEBKIT
>  	depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
>  	depends on BR2_USE_MMU # dbus
> +

Why this new empty line?

>  	select BR2_PACKAGE_DBUS
> +

Ditto?

>  	help
>  	  Single "window" launcher for the WebKit WPE port, and
>  	  helper library for implementing WPE launcher. It does
> @@ -26,4 +28,30 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
>  	  string is used, there is no default and the URI to open
>  	  must be always specified in the command line.
>  
> +config BR2_PACKAGE_COG_PLATFORM_FDO
> +	bool "FreeDesktop.org backend"
> +

Don't add these empty lines.

> +	select BR2_PACKAGE_LIBXKBCOMMON

Why do we have this new dependency? I thought so far the FDO backend
was the default, so I assume it was the one used until now, and it
build with libxkbcommon.

Also, if the FDO backend is currently enabled by default, and you make
it optional, you should have a "default y" on this option to keep
backward compatibility.

Another question: what happens if neither FDO nor DRM backends are
enabled?

> +

No empty line please.

> +	help
> +	  Enable the FreeDesktop.org backend. Cog will interface with
> +	  a compositor over the Wayland protocol.
> +
> +config BR2_PACKAGE_COG_PLATFORM_DRM
> +	bool "DRM backend"
> +

Not empty line please.

> +	depends on BR2_PACKAGE_HAS_UDEV # libinput
> +	depends on BR2_PACKAGE_MESA3D_OPENGL_EGL # gbm
> +

No empty line please.

> +	select BR2_PACKAGE_LIBDRM
> +	select BR2_PACKAGE_LIBINPUT
> +
> +	help
> +	  Enable the DRM platform backend. Cog will interface directly
> +	  with video drivers that support kernel mode-setting (KMS)
> +	  via the DRM user-space API.
> +
> +comment "DRM platform needs mesa3d w/ EGL driver and GBM"
> +	depends on !BR2_PACKAGE_MESA3D_OPENGL_EGL
> +
>  endif
> diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> index d0e5b79c38..339187126a 100644
> --- a/package/cog/cog.mk
> +++ b/package/cog/cog.mk
> @@ -8,13 +8,25 @@ COG_VERSION = 0.4.0
>  COG_SITE = https://wpewebkit.org/releases
>  COG_SOURCE = cog-$(COG_VERSION).tar.xz
>  COG_INSTALL_STAGING = YES
> -COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
> +COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo wayland wayland-protocols

Why are you dropping the wayland and wayland-protocols dependencies?
Overall, it makes sense because cog does not depends/select wayland or
wayland-protocols. Ditto for wpebackend-fdo.

But dropping these should be done as a separate preliminary patch.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v4] package/cog: add option for platform DRM.

[Adrian Perez de Castro]

Hello,

I am adding a few comments to Thomas' questions below.

On Sat, 4 Apr 2020 23:46:30 +0200, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
 
> On Thu,  2 Apr 2020 12:34:55 +0100
> Charlie Turner <cturner@igalia.com> wrote:
> 
> > diff --git a/package/cog/Config.in b/package/cog/Config.in
> > index b25991d4ae..6a7c5668a5 100644
> > --- a/package/cog/Config.in
> > +++ b/package/cog/Config.in
> > @@ -7,7 +7,9 @@ config BR2_PACKAGE_COG
> >  	depends on BR2_PACKAGE_WPEWEBKIT
> >  	depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
> >  	depends on BR2_USE_MMU # dbus
> > +
> 
> Why this new empty line?
> 
> >  	select BR2_PACKAGE_DBUS
> > +
> 
> Ditto?
> 
> >  	help
> >  	  Single "window" launcher for the WebKit WPE port, and
> >  	  helper library for implementing WPE launcher. It does
> > @@ -26,4 +28,30 @@ config BR2_PACKAGE_COG_PROGRAMS_HOME_URI
> >  	  string is used, there is no default and the URI to open
> >  	  must be always specified in the command line.
> >  
> > +config BR2_PACKAGE_COG_PLATFORM_FDO
> > +	bool "FreeDesktop.org backend"
> > +
> 
> Don't add these empty lines.
> 
> > +	select BR2_PACKAGE_LIBXKBCOMMON
> 
> Why do we have this new dependency? I thought so far the FDO backend
> was the default, so I assume it was the one used until now, and it
> build with libxkbcommon.

The dependency was needed already before, but I think we were just being
lucky that it was being pulled in by some other dependency (probably by the
Wayland package) and that's why things have been working without listing it
here explicitly.

> Also, if the FDO backend is currently enabled by default, and you make
> it optional, you should have a "default y" on this option to keep
> backward compatibility.

Sounds sensible.

> Another question: what happens if neither FDO nor DRM backends are
> enabled?

There is a fallback mode in Cog which is used for simple WPE backends like
WPEBackend-rdk [1] if the ?--platform=?? option is not passed in the command
line.
 
> > diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> > index d0e5b79c38..339187126a 100644
> > --- a/package/cog/cog.mk
> > +++ b/package/cog/cog.mk
> > @@ -8,13 +8,25 @@ COG_VERSION = 0.4.0
> >  COG_SITE = https://wpewebkit.org/releases
> >  COG_SOURCE = cog-$(COG_VERSION).tar.xz
> >  COG_INSTALL_STAGING = YES
> > -COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
> > +COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo wayland wayland-protocols
> 
> Why are you dropping the wayland and wayland-protocols dependencies?
> Overall, it makes sense because cog does not depends/select wayland or
> wayland-protocols. Ditto for wpebackend-fdo.

We need those, but only when BR2_PACKAGE_COG_PLATFORM_FDO is enabled.

Cheers,
?Adri?n

---
[1] https://github.com/WebPlatformForEmbedded/WPEBackend-rdk
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20200405/5671cbc6/attachment.asc>

[Buildroot] [PATCH v4] package/cog: add option for platform DRM.

[Thomas Petazzoni]

Hello Adrian,

Thanks for the feedback.

On Sun, 5 Apr 2020 01:49:59 +0300
Adrian Perez de Castro <aperez@igalia.com> wrote:

> > > +	select BR2_PACKAGE_LIBXKBCOMMON  
> > 
> > Why do we have this new dependency? I thought so far the FDO backend
> > was the default, so I assume it was the one used until now, and it
> > build with libxkbcommon.  
> 
> The dependency was needed already before, but I think we were just being
> lucky that it was being pulled in by some other dependency (probably by the
> Wayland package) and that's why things have been working without listing it
> here explicitly.

If the dependency was already required, then it would be good to have a
preliminary patch that adds it, before adding the DRM platform option.

> > Also, if the FDO backend is currently enabled by default, and you make
> > it optional, you should have a "default y" on this option to keep
> > backward compatibility.  
> 
> Sounds sensible.
> 
> > Another question: what happens if neither FDO nor DRM backends are
> > enabled?  
> 
> There is a fallback mode in Cog which is used for simple WPE backends like
> WPEBackend-rdk [1] if the ?--platform=?? option is not passed in the command
> line.

OK, so we don't need to enforce that one of the two backends is enabled.

> > > diff --git a/package/cog/cog.mk b/package/cog/cog.mk
> > > index d0e5b79c38..339187126a 100644
> > > --- a/package/cog/cog.mk
> > > +++ b/package/cog/cog.mk
> > > @@ -8,13 +8,25 @@ COG_VERSION = 0.4.0
> > >  COG_SITE = https://wpewebkit.org/releases
> > >  COG_SOURCE = cog-$(COG_VERSION).tar.xz
> > >  COG_INSTALL_STAGING = YES
> > > -COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo
> > > +COG_DEPENDENCIES = dbus wpewebkit wpebackend-fdo wayland wayland-protocols  
> > 
> > Why are you dropping the wayland and wayland-protocols dependencies?
> > Overall, it makes sense because cog does not depends/select wayland or
> > wayland-protocols. Ditto for wpebackend-fdo.  
> 
> We need those, but only when BR2_PACKAGE_COG_PLATFORM_FDO is enabled.

Then the Config.in should have a "select" for those, and they should be
added to <pkg>_DEPENDENCIES only when BR_PACKAGE_COG_PLATFORM_FDO is
enabled. Should also be a separate preliminary patch.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v4] package/cog: add option for platform DRM.

[Charles Turner]

Hello,

Thank you for your reviews.

I have commented below. As an aside, I note my patch has been removed
from the Patchwork interface[1]. My process so far was to find it
there, copy the message ID, and continue the version bumps. However,
now that I can't find it there, and that this has become a patch
*series*, I will be sending new mails.

On Sun, 2020-04-05 at 01:49 +0300, Adrian Perez de Castro wrote:
> Hello,
> 
> I am adding a few comments to Thomas' questions below.
> 
> On Sat, 4 Apr 2020 23:46:30 +0200, Thomas Petazzoni <
> thomas.petazzoni at bootlin.com> wrote:
>  
> > On Thu,  2 Apr 2020 12:34:55 +0100
> > Charlie Turner <cturner@igalia.com> wrote:
> > 
> > > diff --git a/package/cog/Config.in b/package/cog/Config.in
> > > index b25991d4ae..6a7c5668a5 100644
> > > --- a/package/cog/Config.in
> > > +++ b/package/cog/Config.in
> > > @@ -7,7 +7,9 @@ config BR2_PACKAGE_COG
> > >  	depends on BR2_PACKAGE_WPEWEBKIT
> > >  	depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
> > >  	depends on BR2_USE_MMU # dbus
> > > +
> > 
> > Why this new empty line?

It was visually pleasing at the time, sorry for introducing an
inconsistency, I should have checked.

> > Why are you dropping the wayland and wayland-protocols
> > dependencies?
> > Overall, it makes sense because cog does not depends/select wayland
> > or
> > wayland-protocols. Ditto for wpebackend-fdo.
> 
> We need those, but only when BR2_PACKAGE_COG_PLATFORM_FDO is enabled.

I slightly diverted here, wayland is needed for both FDO and DRM (FDO
is a client and DRM is a server). Only FDO registers new protocols, so
that is platform-specific.

Best,
	Charlie.

[1] https://patchwork.ozlabs.org/project/buildroot/list/

[Buildroot] [PATCH v4] package/cog: add option for platform DRM.

[Baruch Siach]

Hi Charles,

On Mon, Apr 06 2020, Charles Turner wrote:
> Hello,
>
> Thank you for your reviews.
>
> I have commented below. As an aside, I note my patch has been removed
> from the Patchwork interface[1]. My process so far was to find it
> there, copy the message ID, and continue the version bumps. However,
> now that I can't find it there, and that this has become a patch
> *series*, I will be sending new mails.

Your patch is still there:

  http://patchwork.ozlabs.org/patch/1265532/

It state has changed to "Changes Requested" so it does not appear in the
default view. You can remove the "State = Action Required" filter to see
your patch listed. You can also search patches by submitter:

  http://patchwork.ozlabs.org/project/buildroot/list/?submitter=76614&series=&q=&delegate=&archive=&state=*

You can also do all that from command line using the pwclient utility:

  http://patchwork.ozlabs.org/pwclient/

baruch

> On Sun, 2020-04-05 at 01:49 +0300, Adrian Perez de Castro wrote:
>> Hello,
>> 
>> I am adding a few comments to Thomas' questions below.
>> 
>> On Sat, 4 Apr 2020 23:46:30 +0200, Thomas Petazzoni <
>> thomas.petazzoni at bootlin.com> wrote:
>>  
>> > On Thu,  2 Apr 2020 12:34:55 +0100
>> > Charlie Turner <cturner@igalia.com> wrote:
>> > 
>> > > diff --git a/package/cog/Config.in b/package/cog/Config.in
>> > > index b25991d4ae..6a7c5668a5 100644
>> > > --- a/package/cog/Config.in
>> > > +++ b/package/cog/Config.in
>> > > @@ -7,7 +7,9 @@ config BR2_PACKAGE_COG
>> > >  	depends on BR2_PACKAGE_WPEWEBKIT
>> > >  	depends on BR2_TOOLCHAIN_HAS_THREADS # dbus
>> > >  	depends on BR2_USE_MMU # dbus
>> > > +
>> > 
>> > Why this new empty line?
>
> It was visually pleasing at the time, sorry for introducing an
> inconsistency, I should have checked.
>
>> > Why are you dropping the wayland and wayland-protocols
>> > dependencies?
>> > Overall, it makes sense because cog does not depends/select wayland
>> > or
>> > wayland-protocols. Ditto for wpebackend-fdo.
>> 
>> We need those, but only when BR2_PACKAGE_COG_PLATFORM_FDO is enabled.
>
> I slightly diverted here, wayland is needed for both FDO and DRM (FDO
> is a client and DRM is a server). Only FDO registers new protocols, so
> that is platform-specific.
>
> Best,
> 	Charlie.
>
> [1] https://patchwork.ozlabs.org/project/buildroot/list/
>

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -