buildroot.busybox.net archive mirror
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: Refik Tuzakli <tuzakli.refik@gmail.com>
Cc: Gary Bisson <bisson.gary@gmail.com>,
	Peter Seiderer <ps.report@gmx.net>,
	buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH v1 3/3] package/freescale-imx/imx-gpu-viv: add options for new feture
Date: Thu, 5 Aug 2021 23:36:15 +0200	[thread overview]
Message-ID: <20210805233615.4e69d63d@windsurf> (raw)
In-Reply-To: <20200209134408.10534-3-tuzakli.refik@gmail.com>

Hello Refik,

On Sun,  9 Feb 2020 16:44:08 +0300
Refik Tuzakli <tuzakli.refik@gmail.com> wrote:

> NXP supply new fetures with new gpu driver. This patch adds options to select
> the library needed and saves space on target.
> 
> Signed-off-by: Refik Tuzakli <tuzakli.refik@gmail.com>

Thanks a lot for this patch. I started looking into it, and realized
that now due to the massive amount of libraries installed by
imx-gpu-viv, the TARGET_DIR/usr/lib folder weights a total of 190 MB!
This is really huge, and we definitely want to make this more
conditional as not everybody needs OpenVX, Vulkan, or the neural
network stuff.

However, I think we would need to do that step by step, in multiple
patches. First we should make conditional what the package already
supports. The package currently provides:

IMX_GPU_VIV_PROVIDES = libegl libgles libopencl libopenvg

I think libegl and libgles should remain unconditional. However, we
should make libopencl and libopenvg optional. When they are not
enabled, then the libraries and headers should not be installed to
STAGING_DIR. Indeed, we really want a hard build/link failure if
somebody tries to build an OpenCL or OpenVG application and the
corresponding options are not enabled. We cannot install "the whole
imx-gpu-viv" to STAGING_DIR and only conditionally install to
TARGET_DIR, because it means someone not enabling OpenCL would be able
to build/link the application and it would only fail at runtime which
is not great.

Then once we have this imx-gpu-viv package that has options for
libopencl and libopenvg, you can create additional patches adding
options for libvulkan, for libopenvx, for the neural network stuff.

Would you be willing to work on something like this ?

Also, one question is: are all these features available for all i.MX
platforms? Vulkan, neural network stuff is available even on the older
i.MX6 platforms that also use imx-gpu-viv ?


> +ifeq ($(BR2_PACKAGE_IMX_GPU_VIV_LIBNN),y)
> +IMX_GPU_VIV_PROVIDES += libnn
> +define IMX_GPU_VIV_INSTALL_TARGET_LIBNN
> +	cp -dpfr $(@D)/gpu-core/usr/lib/libnnrt.so $(TARGET_DIR)/usr/lib
> +	cp -dpfr $(@D)/gpu-core/usr/lib/libneuralnetworks.so $(TARGET_DIR)/usr/lib
> +	cp -dpfr $(@D)/gpu-core/usr/lib/libNNGPUBinary-lite.so $(TARGET_DIR)/usr/lib
> +	cp -dpfr $(@D)/gpu-core/usr/lib/libNNGPUBinary-xsvx.so $(TARGET_DIR)/usr/lib
> +	cp -dpfr $(@D)/gpu-core/usr/lib/libNNVXCBinary-evis.so $(TARGET_DIR)/usr/lib
> +	cp -dpfr $(@D)/gpu-core/usr/lib/libNNGPUBinary-ulite.so $(TARGET_DIR)/usr/lib
> +	cp -dpfr $(@D)/gpu-core/usr/lib/libNNVXCBinary-evis2.so $(TARGET_DIR)/usr/lib

Do we need *all* those libraries in the target? Several of them have
very similar names, they somehow look like alternatives of each other.

>  endef
> +define IMX_GPU_VIV_INSTALL_STAGING_LIBVULKAN

Should have been LIBNN I guess.

> +	cp -dpfr $(@D)/gpu-core/usr/include/nnrt $(STAGING_DIR)/usr/include/
> +endef
> +endif
>  
>  ifeq ($(IMX_GPU_VIV_LIB_TARGET),fb)
>  define IMX_GPU_VIV_FIXUP_FB_HEADERS
> @@ -88,7 +143,10 @@ endef
>  endif
>  
>  define IMX_GPU_VIV_INSTALL_STAGING_CMDS
> -	cp -r $(@D)/gpu-core/usr/* $(STAGING_DIR)/usr
> +	cp -dpfr $(@D)/gpu-core/usr/lib $(STAGING_DIR)/usr

See here? You are installing all libraries to $(STAGING_DIR), even the
ones that are not in TARGET_DIR. It means you won't get link failures,
but runtime failures (as I explained above).

Are you interested in improving your patch series along those lines ?

Also, Peter Seiderer comment on your patch in December, but you never
replied.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

      parent reply	other threads:[~2021-08-05 21:36 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-09 13:44 [Buildroot] [PATCH v1 1/3] package/opengl/libopenvx: new virtual package Refik Tuzakli
2020-02-09 13:44 ` [Buildroot] [PATCH v1 2/3] package/opengl/libvulkan: " Refik Tuzakli
2020-12-30 15:18   ` Peter Seiderer
2020-02-09 13:44 ` [Buildroot] [PATCH v1 3/3] package/freescale-imx/imx-gpu-viv: add options for new feture Refik Tuzakli
2020-12-30 15:41   ` Peter Seiderer
2021-08-05 21:36   ` Thomas Petazzoni [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210805233615.4e69d63d@windsurf \
    --to=thomas.petazzoni@bootlin.com \
    --cc=bisson.gary@gmail.com \
    --cc=buildroot@buildroot.org \
    --cc=ps.report@gmx.net \
    --cc=tuzakli.refik@gmail.com \
    --subject='Re: [Buildroot] [PATCH v1 3/3] package/freescale-imx/imx-gpu-viv: add options for new feture' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).