[master/kirkstone,v2,2/2] ti-sgx-ddk-um: use udev for userspace initialization

The ti-sgx-ddk driver requires an additional userspace initialization
step after the kernel module has probed the device. Without this
initialization, no EGL context can be created and Weston etc. will fail to
start.

The driver package contains an init script, but this does not work on pure
systemd-based systems without sysvinit compat support (as provided by the
systemd-compat-units package). Introduce an enabled-by-default
PACKAGECONFIG that installs a udev rule instead to run the init command
automatically when the driver is loaded, solving the issue without
depending on a specific init system. On builds without udev, this
PACKAGECONFIG can be disabled, reverting to the old init script-based
solution.

udev reports several events when the pvrsrvkm module is loaded:

- add event for the kernel module
- add events for two DRM devices
- bind event for the GPU platform device

The DRM devices aren't nice to match on, and the kernel module add is
too early to run `pvrsrvctl --start`, so we trigger on the platform
device bind.

Tested with Weston 9.0.0 on the AM65x-based TQ-Systems MBa65xx.

Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
---

v2: applied review suggestions:
- Removed 'local' shell directive from recipe
- Improved commit description

 .../libgles/ti-sgx-ddk-um/pvrsrvkm.rules      |  1 +
 .../libgles/ti-sgx-ddk-um_1.17.4948957.bb     | 23 ++++++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um/pvrsrvkm.rules

On Thu, Jan 19, 2023 at 11:29:52AM +0100, Matthias Schiffer wrote:
> The ti-sgx-ddk driver requires an additional userspace initialization
> step after the kernel module has probed the device. Without this
> initialization, no EGL context can be created and Weston etc. will fail to
> start.
> 
> The driver package contains an init script, but this does not work on pure
> systemd-based systems without sysvinit compat support (as provided by the
> systemd-compat-units package). Introduce an enabled-by-default
> PACKAGECONFIG that installs a udev rule instead to run the init command
> automatically when the driver is loaded, solving the issue without
> depending on a specific init system. On builds without udev, this
> PACKAGECONFIG can be disabled, reverting to the old init script-based
> solution.
> 
> udev reports several events when the pvrsrvkm module is loaded:
> 
> - add event for the kernel module
> - add events for two DRM devices
> - bind event for the GPU platform device
> 
> The DRM devices aren't nice to match on, and the kernel module add is
> too early to run `pvrsrvctl --start`, so we trigger on the platform
> device bind.
> 
> Tested with Weston 9.0.0 on the AM65x-based TQ-Systems MBa65xx.

Quick question - does this also apply to Rogue graphics recipe in 
ti-img-rogue-umlibs recipe?


> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> ---
> 
> v2: applied review suggestions:
> - Removed 'local' shell directive from recipe
> - Improved commit description
> 
>  .../libgles/ti-sgx-ddk-um/pvrsrvkm.rules      |  1 +
>  .../libgles/ti-sgx-ddk-um_1.17.4948957.bb     | 23 ++++++++++++++++++-
>  2 files changed, 23 insertions(+), 1 deletion(-)
>  create mode 100644 meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um/pvrsrvkm.rules
> 
> diff --git a/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um/pvrsrvkm.rules b/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um/pvrsrvkm.rules
> new file mode 100644
> index 00000000..e49fd9b8
> --- /dev/null
> +++ b/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um/pvrsrvkm.rules
> @@ -0,0 +1 @@
> +SUBSYSTEM=="platform", ACTION=="bind", ENV{DRIVER}=="pvrsrvkm", RUN+="/usr/bin/pvrsrvctl --start --no-module"
> diff --git a/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um_1.17.4948957.bb b/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um_1.17.4948957.bb
> index bd88d14d..bc56e705 100644
> --- a/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um_1.17.4948957.bb
> +++ b/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um_1.17.4948957.bb
> @@ -14,7 +14,10 @@ PR = "r37"
>  
>  BRANCH = "ti-img-sgx/dunfell/${PV}"
>  
> -SRC_URI = "git://git.ti.com/git/graphics/omap5-sgx-ddk-um-linux.git;protocol=https;branch=${BRANCH}"
> +SRC_URI = " \
> +    git://git.ti.com/git/graphics/omap5-sgx-ddk-um-linux.git;protocol=https;branch=${BRANCH} \
> +    file://pvrsrvkm.rules \
> +"
>  SRCREV = "742cf38aba13e1ba1a910cf1f036a1a212c263b6"
>  
>  TARGET_PRODUCT:omap-a15 = "jacinto6evm"
> @@ -27,6 +30,9 @@ INITSCRIPT_PARAMS = "defaults 8"
>  
>  inherit update-rc.d
>  
> +PACKAGECONFIG ??= "udev"
> +PACKAGECONFIG[udev] = ",,,udev"
> +
>  PROVIDES += "virtual/egl virtual/libgles1 virtual/libgles2 virtual/libgbm"
>  
>  DEPENDS += "libdrm udev wayland wayland-protocols libffi expat"
> @@ -56,6 +62,20 @@ do_install () {
>      oe_runmake install DESTDIR=${D} TARGET_PRODUCT=${TARGET_PRODUCT}
>      ln -sf libGLESv2.so.2 ${D}${libdir}/libGLESv2.so.1
>  
> +    without_sysvinit=${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'false', 'true', d)}
> +    with_udev=${@bb.utils.contains('PACKAGECONFIG', 'udev', 'true', 'false', d)}
> +
> +    # Delete initscript if it is not needed or would conflict with the udev rules
> +    if $without_sysvinit || $with_udev; then
> +        rm -rf ${D}${sysconfdir}/init.d
> +        rmdir --ignore-fail-on-non-empty ${D}${sysconfdir}
> +    fi
> +
> +    if $with_udev; then
> +        install -m644 -D ${WORKDIR}/pvrsrvkm.rules \
> +            ${D}${nonarch_base_libdir}/udev/rules.d/80-pvrsrvkm.rules
> +    fi
> +
>      chown -R root:root ${D}
>  }
>  
> @@ -63,6 +83,7 @@ FILES:${PN} =  "${bindir}/*"
>  FILES:${PN} += " ${libdir}/*"
>  FILES:${PN} +=  "${includedir}/*"
>  FILES:${PN} +=  "${sysconfdir}/*"
> +FILES:${PN} += "${nonarch_base_libdir}/udev/rules.d"
>  
>  INSANE_SKIP:${PN} += "dev-so ldflags useless-rpaths"
>  INSANE_SKIP:${PN} += "already-stripped dev-deps"
> -- 
> 2.34.1
>

On Thu, 2023-01-19 at 17:17 -0500, Denys Dmytriyenko wrote:
> On Thu, Jan 19, 2023 at 11:29:52AM +0100, Matthias Schiffer wrote:
> > The ti-sgx-ddk driver requires an additional userspace initialization
> > step after the kernel module has probed the device. Without this
> > initialization, no EGL context can be created and Weston etc. will fail to
> > start.
> > 
> > The driver package contains an init script, but this does not work on pure
> > systemd-based systems without sysvinit compat support (as provided by the
> > systemd-compat-units package). Introduce an enabled-by-default
> > PACKAGECONFIG that installs a udev rule instead to run the init command
> > automatically when the driver is loaded, solving the issue without
> > depending on a specific init system. On builds without udev, this
> > PACKAGECONFIG can be disabled, reverting to the old init script-based
> > solution.
> > 
> > udev reports several events when the pvrsrvkm module is loaded:
> > 
> > - add event for the kernel module
> > - add events for two DRM devices
> > - bind event for the GPU platform device
> > 
> > The DRM devices aren't nice to match on, and the kernel module add is
> > too early to run `pvrsrvctl --start`, so we trigger on the platform
> > device bind.
> > 
> > Tested with Weston 9.0.0 on the AM65x-based TQ-Systems MBa65xx.
> 
> Quick question - does this also apply to Rogue graphics recipe in 
> ti-img-rogue-umlibs recipe?

We don't have any boards that use ti-img-rogue-umlibs for now at TQ,
but looking at the initscript, there are a few differences - in
particular, nothing in the initscript calls a driver-specific userspace
tool.

What the initscript does differs between the dunfell and kirkstone
branches - on dunfell, it doesn't only load the kernel module, but also
starts Weston.

On the recently added kirkstone branch, this was reduced to only load
the kernel module. So if the kernel modules correctly specifies its
aliases (which I think it does), udev should already load it by itself,
making the initscript entirely redundant on kirkstone-based systems
with udev.

In conclusion, ti-img-rogue-umlibs should already work out-of-the-box
on pure systemd systems. As a minor optimization, the recipe could be
adjusted to remove the initscript when DISTRO_FEATURES doesn't have
sysvinit, to avoid installing the unused script.


> 
> 
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > ---
> > 
> > v2: applied review suggestions:
> > - Removed 'local' shell directive from recipe
> > - Improved commit description
> > 
> >  .../libgles/ti-sgx-ddk-um/pvrsrvkm.rules      |  1 +
> >  .../libgles/ti-sgx-ddk-um_1.17.4948957.bb     | 23 ++++++++++++++++++-
> >  2 files changed, 23 insertions(+), 1 deletion(-)
> >  create mode 100644 meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um/pvrsrvkm.rules
> > 
> > diff --git a/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um/pvrsrvkm.rules b/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um/pvrsrvkm.rules
> > new file mode 100644
> > index 00000000..e49fd9b8
> > --- /dev/null
> > +++ b/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um/pvrsrvkm.rules
> > @@ -0,0 +1 @@
> > +SUBSYSTEM=="platform", ACTION=="bind", ENV{DRIVER}=="pvrsrvkm", RUN+="/usr/bin/pvrsrvctl --start --no-module"
> > diff --git a/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um_1.17.4948957.bb b/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um_1.17.4948957.bb
> > index bd88d14d..bc56e705 100644
> > --- a/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um_1.17.4948957.bb
> > +++ b/meta-ti-bsp/recipes-graphics/libgles/ti-sgx-ddk-um_1.17.4948957.bb
> > @@ -14,7 +14,10 @@ PR = "r37"
> >  
> >  BRANCH = "ti-img-sgx/dunfell/${PV}"
> >  
> > -SRC_URI = "git://git.ti.com/git/graphics/omap5-sgx-ddk-um-linux.git;protocol=https;branch=${BRANCH}"
> > +SRC_URI = " \
> > +    git://git.ti.com/git/graphics/omap5-sgx-ddk-um-linux.git;protocol=https;branch=${BRANCH} \
> > +    file://pvrsrvkm.rules \
> > +"
> >  SRCREV = "742cf38aba13e1ba1a910cf1f036a1a212c263b6"
> >  
> >  TARGET_PRODUCT:omap-a15 = "jacinto6evm"
> > @@ -27,6 +30,9 @@ INITSCRIPT_PARAMS = "defaults 8"
> >  
> >  inherit update-rc.d
> >  
> > +PACKAGECONFIG ??= "udev"
> > +PACKAGECONFIG[udev] = ",,,udev"
> > +
> >  PROVIDES += "virtual/egl virtual/libgles1 virtual/libgles2 virtual/libgbm"
> >  
> >  DEPENDS += "libdrm udev wayland wayland-protocols libffi expat"
> > @@ -56,6 +62,20 @@ do_install () {
> >      oe_runmake install DESTDIR=${D} TARGET_PRODUCT=${TARGET_PRODUCT}
> >      ln -sf libGLESv2.so.2 ${D}${libdir}/libGLESv2.so.1
> >  
> > +    without_sysvinit=${@bb.utils.contains('DISTRO_FEATURES', 'sysvinit', 'false', 'true', d)}
> > +    with_udev=${@bb.utils.contains('PACKAGECONFIG', 'udev', 'true', 'false', d)}
> > +
> > +    # Delete initscript if it is not needed or would conflict with the udev rules
> > +    if $without_sysvinit || $with_udev; then
> > +        rm -rf ${D}${sysconfdir}/init.d
> > +        rmdir --ignore-fail-on-non-empty ${D}${sysconfdir}
> > +    fi
> > +
> > +    if $with_udev; then
> > +        install -m644 -D ${WORKDIR}/pvrsrvkm.rules \
> > +            ${D}${nonarch_base_libdir}/udev/rules.d/80-pvrsrvkm.rules
> > +    fi
> > +
> >      chown -R root:root ${D}
> >  }
> >  
> > @@ -63,6 +83,7 @@ FILES:${PN} =  "${bindir}/*"
> >  FILES:${PN} += " ${libdir}/*"
> >  FILES:${PN} +=  "${includedir}/*"
> >  FILES:${PN} +=  "${sysconfdir}/*"
> > +FILES:${PN} += "${nonarch_base_libdir}/udev/rules.d"
> >  
> >  INSANE_SKIP:${PN} += "dev-so ldflags useless-rpaths"
> >  INSANE_SKIP:${PN} += "already-stripped dev-deps"
> > -- 
> > 2.34.1
> >