diff mbox series

[v2,2/4] package/freescale-imx/firmware-imx: Add option for all i.MX FW needs

Message ID 1593419148-26821-3-git-send-email-stephane.viau@oss.nxp.com
State Superseded
Headers show
Series package/freescale-imx: clean-up proposal | expand

Commit Message

Stephane Viau (OSS) June 29, 2020, 8:25 a.m. UTC 
Some SoC need a HDMI FW for their bootloader, some other require EPDC,
SDMA and/or VPU.
Instead of trying to "guess" what firmware images need to be installed
in firmware-imx.mk, let the Config framework do the job and allow each
SoC to pick what firmware they need.

Note that this patch should also help introducing an eventual DP FW, as
Gary mentioned in a separate thread [1].

[1] http://lists.busybox.net/pipermail/buildroot/2020-May/283181.html

Suggested-by: Yann E. MORIN <yann.morin.1998@free.fr>
Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Stephane Viau <stephane.viau@oss.nxp.com>
---
v2:
- Do not embed VPU FW for i.MX 6S/6UL/7 (Fabio)
- Name VPU FW after IP name (Fabio)
- Rename symbols using the "_NEED_" in their name (Thomas)

Signed-off-by: Stephane Viau <stephane.viau@oss.nxp.com>
---
 package/freescale-imx/firmware-imx/Config.in | 43 +++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

Comments

Gary Bisson June 30, 2020, 11:53 a.m. UTC | #1 
Hi Stephane

On Mon, Jun 29, 2020 at 10:25:46AM +0200, Stephane Viau wrote:
> Some SoC need a HDMI FW for their bootloader, some other require EPDC,
> SDMA and/or VPU.
> Instead of trying to "guess" what firmware images need to be installed
> in firmware-imx.mk, let the Config framework do the job and allow each
> SoC to pick what firmware they need.
> 
> Note that this patch should also help introducing an eventual DP FW, as
> Gary mentioned in a separate thread [1].
> 
> [1] http://lists.busybox.net/pipermail/buildroot/2020-May/283181.html
> 
> Suggested-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Stephane Viau <stephane.viau@oss.nxp.com>
> ---
> v2:
> - Do not embed VPU FW for i.MX 6S/6UL/7 (Fabio)
> - Name VPU FW after IP name (Fabio)
> - Rename symbols using the "_NEED_" in their name (Thomas)
> 
> Signed-off-by: Stephane Viau <stephane.viau@oss.nxp.com>
> ---
>  package/freescale-imx/firmware-imx/Config.in | 43 +++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/package/freescale-imx/firmware-imx/Config.in b/package/freescale-imx/firmware-imx/Config.in
> index d6310e6..2ac5dfa 100644
> --- a/package/freescale-imx/firmware-imx/Config.in
> +++ b/package/freescale-imx/firmware-imx/Config.in
> @@ -4,11 +4,52 @@ config BR2_PACKAGE_FIRMWARE_IMX
>  	help
>  	  Firmware blobs for the Freescale i.MX SoCs.
>  
> -	  It contains blobs for SDMA and VPU.
> +	  It contains blobs for SDMA, VPU, etc...
>  
>  	  This library is provided by Freescale as-is and doesn't have
>  	  an upstream.
>  
> +config BR2_PACKAGE_FIRMWARE_IMX_SDMA
> +	bool
> +	depends on BR2_PACKAGE_FIRMWARE_IMX
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX25_3STACK
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX27ADS
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX37_3STACK
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX50
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX51
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX53
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6Q
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6S
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6UL
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX7
> +
> +config BR2_PACKAGE_FIRMWARE_IMX_VPU_CODA
> +	bool
> +	depends on BR2_PACKAGE_FIRMWARE_IMX
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX27ADS
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX37_3STACK
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX50
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX51
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX53
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6Q
> +
> +config BR2_PACKAGE_FIRMWARE_IMX_VPU_MALONE
> +	bool
> +	depends on BR2_PACKAGE_FIRMWARE_IMX
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8
> +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8X

So here you are either copying all the VPU Malone blob or all the VPU
CODA blob no matter the platform being used?

Since we cleaning things up, I'd rather have a generic
BR2_PACKAGE_FIRMWARE_IMX_VPU config for all the platforms that require a
VPU blob (from firmware-imx).
Then in the .mk I'd only copy the VPU blob associated to the platform
selected (mx27, mx6, mx8 etc).
That way we don't embed imx27 blob when we build for imx6. As someone
mentioned in the previous, there's no need to embeds blobs we know are
useless.

What do you think?

Regards,
Gary
Thomas Petazzoni June 30, 2020, 12:08 p.m. UTC | #2 
On Tue, 30 Jun 2020 13:53:41 +0200
Gary Bisson <gary.bisson@boundarydevices.com> wrote:

> > +config BR2_PACKAGE_FIRMWARE_IMX_VPU_CODA
> > +	bool
> > +	depends on BR2_PACKAGE_FIRMWARE_IMX
> > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX27ADS
> > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX37_3STACK
> > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX50
> > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX51
> > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX53
> > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6Q
> > +
> > +config BR2_PACKAGE_FIRMWARE_IMX_VPU_MALONE
> > +	bool
> > +	depends on BR2_PACKAGE_FIRMWARE_IMX
> > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8
> > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8X  
> 
> So here you are either copying all the VPU Malone blob or all the VPU
> CODA blob no matter the platform being used?
> 
> Since we cleaning things up, I'd rather have a generic
> BR2_PACKAGE_FIRMWARE_IMX_VPU config for all the platforms that require a
> VPU blob (from firmware-imx).
> Then in the .mk I'd only copy the VPU blob associated to the platform
> selected (mx27, mx6, mx8 etc).
> That way we don't embed imx27 blob when we build for imx6. As someone
> mentioned in the previous, there's no need to embeds blobs we know are
> useless.

Could you list which blobs are relevant for each platform, so we get a
sense of what is needed and what is not ?

Thomas
Gary Bisson June 30, 2020, 12:18 p.m. UTC | #3 
Hi Thomas,

On Tue, Jun 30, 2020 at 02:08:29PM +0200, Thomas Petazzoni wrote:
> On Tue, 30 Jun 2020 13:53:41 +0200
> Gary Bisson <gary.bisson@boundarydevices.com> wrote:
> 
> > > +config BR2_PACKAGE_FIRMWARE_IMX_VPU_CODA
> > > +	bool
> > > +	depends on BR2_PACKAGE_FIRMWARE_IMX
> > > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX27ADS
> > > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX37_3STACK
> > > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX50
> > > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX51
> > > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX53
> > > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6Q
> > > +
> > > +config BR2_PACKAGE_FIRMWARE_IMX_VPU_MALONE
> > > +	bool
> > > +	depends on BR2_PACKAGE_FIRMWARE_IMX
> > > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8
> > > +	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8X  
> > 
> > So here you are either copying all the VPU Malone blob or all the VPU
> > CODA blob no matter the platform being used?
> > 
> > Since we cleaning things up, I'd rather have a generic
> > BR2_PACKAGE_FIRMWARE_IMX_VPU config for all the platforms that require a
> > VPU blob (from firmware-imx).
> > Then in the .mk I'd only copy the VPU blob associated to the platform
> > selected (mx27, mx6, mx8 etc).
> > That way we don't embed imx27 blob when we build for imx6. As someone
> > mentioned in the previous, there's no need to embeds blobs we know are
> > useless.
> 
> Could you list which blobs are relevant for each platform, so we get a
> sense of what is needed and what is not ?

Well listing the files from the package gives a pretty good overview:
$ find firmware/vpu/vpu*
firmware/vpu/vpu_fw_imx27_TO1.bin
firmware/vpu/vpu_fw_imx27_TO2.bin
firmware/vpu/vpu_fw_imx51.bin
firmware/vpu/vpu_fw_imx53.bin
firmware/vpu/vpu_fw_imx6d.bin
firmware/vpu/vpu_fw_imx6q.bin
firmware/vpu/vpu_fw_imx8_dec.bin
firmware/vpu/vpu_fw_imx8_enc.bin

Also, with this approach we eliminate the possible errors from selecting
the wrong package for your platforms.

I don't know many customers that actually know the VPU IP is CODA for
i.MX6 or Malone for i.MX8.

Regards,
Gary
Thomas Petazzoni June 30, 2020, 12:26 p.m. UTC | #4 
On Tue, 30 Jun 2020 14:18:11 +0200
Gary Bisson <gary.bisson@boundarydevices.com> wrote:

> > Could you list which blobs are relevant for each platform, so we get a
> > sense of what is needed and what is not ?  
> 
> Well listing the files from the package gives a pretty good overview:
> $ find firmware/vpu/vpu*
> firmware/vpu/vpu_fw_imx27_TO1.bin
> firmware/vpu/vpu_fw_imx27_TO2.bin
> firmware/vpu/vpu_fw_imx51.bin
> firmware/vpu/vpu_fw_imx53.bin
> firmware/vpu/vpu_fw_imx6d.bin
> firmware/vpu/vpu_fw_imx6q.bin
> firmware/vpu/vpu_fw_imx8_dec.bin
> firmware/vpu/vpu_fw_imx8_enc.bin
> 
> Also, with this approach we eliminate the possible errors from selecting
> the wrong package for your platforms.
> 
> I don't know many customers that actually know the VPU IP is CODA for
> i.MX6 or Malone for i.MX8.

I see. Indeed, it makes sense to only copy the files needed for the
particular platform.

We had another comment on this series: what if I need some i.MX
firmware (such as the SDMA ones) but not others (such as the VPU ones).
Do we want explicit options ? Or do we leave that sort of clean up to a
post-build script ?

Thomas
Gary Bisson June 30, 2020, 12:54 p.m. UTC | #5 
Hi,

On Tue, Jun 30, 2020 at 02:26:46PM +0200, Thomas Petazzoni wrote:
> On Tue, 30 Jun 2020 14:18:11 +0200
> Gary Bisson <gary.bisson@boundarydevices.com> wrote:
> 
> > > Could you list which blobs are relevant for each platform, so we get a
> > > sense of what is needed and what is not ?  
> > 
> > Well listing the files from the package gives a pretty good overview:
> > $ find firmware/vpu/vpu*
> > firmware/vpu/vpu_fw_imx27_TO1.bin
> > firmware/vpu/vpu_fw_imx27_TO2.bin
> > firmware/vpu/vpu_fw_imx51.bin
> > firmware/vpu/vpu_fw_imx53.bin
> > firmware/vpu/vpu_fw_imx6d.bin
> > firmware/vpu/vpu_fw_imx6q.bin
> > firmware/vpu/vpu_fw_imx8_dec.bin
> > firmware/vpu/vpu_fw_imx8_enc.bin
> > 
> > Also, with this approach we eliminate the possible errors from selecting
> > the wrong package for your platforms.
> > 
> > I don't know many customers that actually know the VPU IP is CODA for
> > i.MX6 or Malone for i.MX8.
> 
> I see. Indeed, it makes sense to only copy the files needed for the
> particular platform.
> 
> We had another comment on this series: what if I need some i.MX
> firmware (such as the SDMA ones) but not others (such as the VPU ones).
> Do we want explicit options ? Or do we leave that sort of clean up to a
> post-build script ?

We could add some values to the bool part of each option for them to
appear in the menu for people to select/unselect.
Maybe even turn the FIRMWARE_IMX into a menuconfig.
I guess it would require to change imx-vpu though, as it currently
select FIRMWARE_IMX, it would need to select the VPU option.

Regards,
Gary
diff mbox series

Patch

diff --git a/package/freescale-imx/firmware-imx/Config.in b/package/freescale-imx/firmware-imx/Config.in
index d6310e6..2ac5dfa 100644
--- a/package/freescale-imx/firmware-imx/Config.in
+++ b/package/freescale-imx/firmware-imx/Config.in
@@ -4,11 +4,52 @@  config BR2_PACKAGE_FIRMWARE_IMX
 	help
 	  Firmware blobs for the Freescale i.MX SoCs.
 
-	  It contains blobs for SDMA and VPU.
+	  It contains blobs for SDMA, VPU, etc...
 
 	  This library is provided by Freescale as-is and doesn't have
 	  an upstream.
 
+config BR2_PACKAGE_FIRMWARE_IMX_SDMA
+	bool
+	depends on BR2_PACKAGE_FIRMWARE_IMX
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX25_3STACK
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX27ADS
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX37_3STACK
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX50
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX51
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX53
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6Q
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6S
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6UL
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX7
+
+config BR2_PACKAGE_FIRMWARE_IMX_VPU_CODA
+	bool
+	depends on BR2_PACKAGE_FIRMWARE_IMX
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX27ADS
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX37_3STACK
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX50
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX51
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX53
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6Q
+
+config BR2_PACKAGE_FIRMWARE_IMX_VPU_MALONE
+	bool
+	depends on BR2_PACKAGE_FIRMWARE_IMX
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8X
+
+config BR2_PACKAGE_FIRMWARE_IMX_HDMI
+	bool
+	depends on BR2_PACKAGE_FIRMWARE_IMX
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX8M
+
+config BR2_PACKAGE_FIRMWARE_IMX_EPDC
+	bool
+	depends on BR2_PACKAGE_FIRMWARE_IMX
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6Q
+	default y if BR2_PACKAGE_FREESCALE_IMX_PLATFORM_IMX6S
+
 config BR2_PACKAGE_FIRMWARE_IMX_NEEDS_DDR_FW
 	bool
 	depends on BR2_PACKAGE_FIRMWARE_IMX