| Message ID | CAEj2-1OQaPzpLwrrzN-42WDUiRk-eWd9=GPUtUYmJXdWQp124Q@mail.gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | package/linux-firmware: Add Qualcomm video firmware option | expand |
Hello Andre, Thanks for this contribution! See below for some comments. On Sun, 11 Mar 2018 21:57:15 +0000, Andre Renaud wrote: > Added Qualcomm video firmware option. > > Signed-off-by: Andre Renaud <andre@ignavus.net> > --- > package/linux-firmware/Config.in | 5 +++++ > package/linux-firmware/linux-firmware.mk | 5 +++++ > 2 files changed, 10 insertions(+) > > diff --git a/package/linux-firmware/Config.in > b/package/linux-firmware/Config.in > index 4e77a3f9a3..61164e101c 100644 > --- a/package/linux-firmware/Config.in > +++ b/package/linux-firmware/Config.in > @@ -29,6 +29,11 @@ config BR2_PACKAGE_LINUX_FIRMWARE_RADEON > help > Firmware files for AMD Radeon video cards. > > +config BR2_PACKAGE_LINUX_FIRMWARE_QCOM > + bool "Qualcomm video card firmware" > + help > + Firmware files for Qualcomm/Snapdragon video cards. "video cards" doesn't mean much. Are you talking about GPU, VPU? And in fact, if you look at the "WHENCE" file in the linux-firmware project, you'll see: -------------------------------------------------------------------------- Driver: venus - Qualcomm Venus video codec accelerator File: qcom/venus-1.8/venus.mdt File: qcom/venus-1.8/venus.b00 File: qcom/venus-1.8/venus.b01 File: qcom/venus-1.8/venus.b02 File: qcom/venus-1.8/venus.b03 File: qcom/venus-1.8/venus.b04 Version: 1.8-00109 File: qcom/venus-4.2/venus.mdt File: qcom/venus-4.2/venus.b00 File: qcom/venus-4.2/venus.b01 File: qcom/venus-4.2/venus.b02 File: qcom/venus-4.2/venus.b03 File: qcom/venus-4.2/venus.b04 Version: 4.2 Licence: Redistributable. See LICENSE.qcom and qcom/NOTICE.txt for details Binary files supplied originally from https://developer.qualcomm.com/hardware/dragonboard-410c/tools -------------------------------------------------------------------------- -------------------------------------------------------------------------- Driver: adreno - Qualcomm Adreno GPU firmware File: qcom/a300_pfp.fw Link: a300_pfp.fw -> qcom/a300_pfp.fw File: qcom/a300_pm4.fw Link: a300_pm4.fw -> qcom/a300_pm4.fw File: qcom/a530_pfp.fw File: qcom/a530_pm4.fw File: qcom/a530v3_gpmu.fw2 File: qcom/a530_zap.b00 File: qcom/a530_zap.b01 File: qcom/a530_zap.b02 File: qcom/a530_zap.mdt Licence: Redistributable. See LICENSE.qcom and qcom/NOTICE.txt for details Binary files supplied originally from https://developer.qualcomm.com/hardware/dragonboard-410c/tools -------------------------------------------------------------------------- So the files in qcom/venus/ are for a video processing unit (doing video encoding/decoding acceleration), while the qcom/a* files are for the Adreno GPUs. So, I believe we need two sub-options: BR2_PACKAGE_LINUX_FIRMWARE_QCOM_VENUS BR2_PACKAGE_LINUX_FIRMWARE_QCOM_ADRENO Could you rework your patch to add those two options instead ? Thanks! Thomas Petazzoni
Added Qualcomm video accelerator and GPU firmware option. Signed-off-by: Andre Renaud <andre@ignavus.net> --- package/linux-firmware/Config.in | 10 ++++++++++ package/linux-firmware/linux-firmware.mk | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/package/linux-firmware/Config.in b/package/linux-firmware/Config.in index 4e77a3f9a3..9d8d8b340b 100644 --- a/package/linux-firmware/Config.in +++ b/package/linux-firmware/Config.in @@ -29,6 +29,16 @@ config BR2_PACKAGE_LINUX_FIRMWARE_RADEON help Firmware files for AMD Radeon video cards. +config BR2_PACKAGE_LINUX_FIRMWARE_QCOM_VENUS + bool "Qualcomm Venus video codec accelerator" + help + Firmware files for Qualcomm Venus video codec accelerator + +config BR2_PACKAGE_LINUX_FIRMWARE_QCOM_ADRENO + bool "Qualcomm Adreno GPU firmware" + help + Firmware files for Qualcomm Adreno GPU firmware + endmenu # Video menu "Bluetooth firmware" diff --git a/package/linux-firmware/linux-firmware.mk b/package/linux-firmware/linux-firmware.mk index 6f0ba53800..03c4b2c576 100644 --- a/package/linux-firmware/linux-firmware.mk +++ b/package/linux-firmware/linux-firmware.mk @@ -24,6 +24,16 @@ LINUX_FIRMWARE_DIRS += radeon LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.radeon endif +ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_QCOM_VENUS),y) +LINUX_FIRMWARE_DIRS += qcom/venus-1.8 qcom/venus-4.2 +LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.qcom +endif + +ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_QCOM_ADRENO),y) +LINUX_FIRMWARE_FILES += qcom/a* +LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.qcom +endif + # Intel Wireless Bluetooth ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_IBT),y) LINUX_FIRMWARE_FILES += intel/ibt-* On Tue, 27 Mar 2018 at 10:33 Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > Hello Andre, > > Thanks for this contribution! See below for some comments. > > On Sun, 11 Mar 2018 21:57:15 +0000, Andre Renaud wrote: > > Added Qualcomm video firmware option. > > > > Signed-off-by: Andre Renaud <andre@ignavus.net> > > --- > > package/linux-firmware/Config.in | 5 +++++ > > package/linux-firmware/linux-firmware.mk | 5 +++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/package/linux-firmware/Config.in > > b/package/linux-firmware/Config.in > > index 4e77a3f9a3..61164e101c 100644 > > --- a/package/linux-firmware/Config.in > > +++ b/package/linux-firmware/Config.in > > @@ -29,6 +29,11 @@ config BR2_PACKAGE_LINUX_FIRMWARE_RADEON > > help > > Firmware files for AMD Radeon video cards. > > > > +config BR2_PACKAGE_LINUX_FIRMWARE_QCOM > > + bool "Qualcomm video card firmware" > > + help > > + Firmware files for Qualcomm/Snapdragon video cards. > > "video cards" doesn't mean much. Are you talking about GPU, VPU? > > And in fact, if you look at the "WHENCE" file in the linux-firmware > project, you'll see: > > -------------------------------------------------------------------------- > > Driver: venus - Qualcomm Venus video codec accelerator > > File: qcom/venus-1.8/venus.mdt > File: qcom/venus-1.8/venus.b00 > File: qcom/venus-1.8/venus.b01 > File: qcom/venus-1.8/venus.b02 > File: qcom/venus-1.8/venus.b03 > File: qcom/venus-1.8/venus.b04 > > Version: 1.8-00109 > > File: qcom/venus-4.2/venus.mdt > File: qcom/venus-4.2/venus.b00 > File: qcom/venus-4.2/venus.b01 > File: qcom/venus-4.2/venus.b02 > File: qcom/venus-4.2/venus.b03 > File: qcom/venus-4.2/venus.b04 > > Version: 4.2 > > Licence: Redistributable. See LICENSE.qcom and qcom/NOTICE.txt for details > > Binary files supplied originally from > https://developer.qualcomm.com/hardware/dragonboard-410c/tools > > -------------------------------------------------------------------------- > > -------------------------------------------------------------------------- > > Driver: adreno - Qualcomm Adreno GPU firmware > > File: qcom/a300_pfp.fw > Link: a300_pfp.fw -> qcom/a300_pfp.fw > File: qcom/a300_pm4.fw > Link: a300_pm4.fw -> qcom/a300_pm4.fw > File: qcom/a530_pfp.fw > File: qcom/a530_pm4.fw > File: qcom/a530v3_gpmu.fw2 > File: qcom/a530_zap.b00 > File: qcom/a530_zap.b01 > File: qcom/a530_zap.b02 > File: qcom/a530_zap.mdt > > Licence: Redistributable. See LICENSE.qcom and qcom/NOTICE.txt for details > > Binary files supplied originally from > https://developer.qualcomm.com/hardware/dragonboard-410c/tools > > -------------------------------------------------------------------------- > > So the files in qcom/venus/ are for a video processing unit (doing > video encoding/decoding acceleration), while the qcom/a* files are for > the Adreno GPUs. > > So, I believe we need two sub-options: > > BR2_PACKAGE_LINUX_FIRMWARE_QCOM_VENUS > BR2_PACKAGE_LINUX_FIRMWARE_QCOM_ADRENO > > Could you rework your patch to add those two options instead ? > > Thanks! > > Thomas Petazzoni > -- > Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com > <div dir="ltr"><div>Added Qualcomm video accelerator and GPU firmware option.</div><div><br></div><div>Signed-off-by: Andre Renaud <<a href="mailto:andre@ignavus.net">andre@ignavus.net</a>></div><div>---</div><div><div> package/linux-firmware/Config.in | 10 ++++++++++</div><div> package/linux-firmware/<a href="http://linux-firmware.mk">linux-firmware.mk</a> | 10 ++++++++++</div><div> 2 files changed, 20 insertions(+)</div><div><br></div><div>diff --git a/package/linux-firmware/Config.in b/package/linux-firmware/Config.in</div><div>index 4e77a3f9a3..9d8d8b340b 100644</div><div>--- a/package/linux-firmware/Config.in</div><div>+++ b/package/linux-firmware/Config.in</div><div>@@ -29,6 +29,16 @@ config BR2_PACKAGE_LINUX_FIRMWARE_RADEON</div><div> <span style="white-space:pre"> </span>help</div><div> <span style="white-space:pre"> </span> Firmware files for AMD Radeon video cards.</div><div> </div><div>+config BR2_PACKAGE_LINUX_FIRMWARE_QCOM_VENUS</div><div>+ bool "Qualcomm Venus video codec accelerator"</div><div>+ help</div><div>+ Firmware files for Qualcomm Venus video codec accelerator</div><div>+</div><div>+config BR2_PACKAGE_LINUX_FIRMWARE_QCOM_ADRENO</div><div>+ bool "Qualcomm Adreno GPU firmware"</div><div>+ help</div><div>+ Firmware files for Qualcomm Adreno GPU firmware</div><div>+</div><div> endmenu # Video</div><div> </div><div> menu "Bluetooth firmware"</div><div>diff --git a/package/linux-firmware/<a href="http://linux-firmware.mk">linux-firmware.mk</a> b/package/linux-firmware/<a href="http://linux-firmware.mk">linux-firmware.mk</a></div><div>index 6f0ba53800..03c4b2c576 100644</div><div>--- a/package/linux-firmware/<a href="http://linux-firmware.mk">linux-firmware.mk</a></div><div>+++ b/package/linux-firmware/<a href="http://linux-firmware.mk">linux-firmware.mk</a></div><div>@@ -24,6 +24,16 @@ LINUX_FIRMWARE_DIRS += radeon</div><div> LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.radeon</div><div> endif</div><div> </div><div>+ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_QCOM_VENUS),y)</div><div>+LINUX_FIRMWARE_DIRS += qcom/venus-1.8 qcom/venus-4.2</div><div>+LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.qcom</div><div>+endif</div><div>+</div><div>+ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_QCOM_ADRENO),y)</div><div>+LINUX_FIRMWARE_FILES += qcom/a*</div><div>+LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.qcom</div><div>+endif</div><div>+</div><div> # Intel Wireless Bluetooth</div><div> ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_IBT),y)</div><div> LINUX_FIRMWARE_FILES += intel/ibt-*</div><div><br></div><br><div class="gmail_quote"><div dir="ltr">On Tue, 27 Mar 2018 at 10:33 Thomas Petazzoni <<a href="mailto:thomas.petazzoni@bootlin.com">thomas.petazzoni@bootlin.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello Andre,<br> <br> Thanks for this contribution! See below for some comments.<br> <br> On Sun, 11 Mar 2018 21:57:15 +0000, Andre Renaud wrote:<br> > Added Qualcomm video firmware option.<br> ><br> > Signed-off-by: Andre Renaud <<a href="mailto:andre@ignavus.net" target="_blank">andre@ignavus.net</a>><br> > ---<br> > package/linux-firmware/Config.in | 5 +++++<br> > package/linux-firmware/<a href="http://linux-firmware.mk" rel="noreferrer" target="_blank">linux-firmware.mk</a> | 5 +++++<br> > 2 files changed, 10 insertions(+)<br> ><br> > diff --git a/package/linux-firmware/Config.in<br> > b/package/linux-firmware/Config.in<br> > index 4e77a3f9a3..61164e101c 100644<br> > --- a/package/linux-firmware/Config.in<br> > +++ b/package/linux-firmware/Config.in<br> > @@ -29,6 +29,11 @@ config BR2_PACKAGE_LINUX_FIRMWARE_RADEON<br> > help<br> > Firmware files for AMD Radeon video cards.<br> ><br> > +config BR2_PACKAGE_LINUX_FIRMWARE_QCOM<br> > + bool "Qualcomm video card firmware"<br> > + help<br> > + Firmware files for Qualcomm/Snapdragon video cards.<br> <br> "video cards" doesn't mean much. Are you talking about GPU, VPU?<br> <br> And in fact, if you look at the "WHENCE" file in the linux-firmware<br> project, you'll see:<br> <br> --------------------------------------------------------------------------<br> <br> Driver: venus - Qualcomm Venus video codec accelerator<br> <br> File: qcom/venus-1.8/venus.mdt<br> File: qcom/venus-1.8/venus.b00<br> File: qcom/venus-1.8/venus.b01<br> File: qcom/venus-1.8/venus.b02<br> File: qcom/venus-1.8/venus.b03<br> File: qcom/venus-1.8/venus.b04<br> <br> Version: 1.8-00109<br> <br> File: qcom/venus-4.2/venus.mdt<br> File: qcom/venus-4.2/venus.b00<br> File: qcom/venus-4.2/venus.b01<br> File: qcom/venus-4.2/venus.b02<br> File: qcom/venus-4.2/venus.b03<br> File: qcom/venus-4.2/venus.b04<br> <br> Version: 4.2<br> <br> Licence: Redistributable. See LICENSE.qcom and qcom/NOTICE.txt for details<br> <br> Binary files supplied originally from<br> <a href="https://developer.qualcomm.com/hardware/dragonboard-410c/tools" rel="noreferrer" target="_blank">https://developer.qualcomm.com/hardware/dragonboard-410c/tools</a><br> <br> --------------------------------------------------------------------------<br> <br> --------------------------------------------------------------------------<br> <br> Driver: adreno - Qualcomm Adreno GPU firmware<br> <br> File: qcom/a300_pfp.fw<br> Link: a300_pfp.fw -> qcom/a300_pfp.fw<br> File: qcom/a300_pm4.fw<br> Link: a300_pm4.fw -> qcom/a300_pm4.fw<br> File: qcom/a530_pfp.fw<br> File: qcom/a530_pm4.fw<br> File: qcom/a530v3_gpmu.fw2<br> File: qcom/a530_zap.b00<br> File: qcom/a530_zap.b01<br> File: qcom/a530_zap.b02<br> File: qcom/a530_zap.mdt<br> <br> Licence: Redistributable. See LICENSE.qcom and qcom/NOTICE.txt for details<br> <br> Binary files supplied originally from<br> <a href="https://developer.qualcomm.com/hardware/dragonboard-410c/tools" rel="noreferrer" target="_blank">https://developer.qualcomm.com/hardware/dragonboard-410c/tools</a><br> <br> --------------------------------------------------------------------------<br> <br> So the files in qcom/venus/ are for a video processing unit (doing<br> video encoding/decoding acceleration), while the qcom/a* files are for<br> the Adreno GPUs.<br> <br> So, I believe we need two sub-options:<br> <br> BR2_PACKAGE_LINUX_FIRMWARE_QCOM_VENUS<br> BR2_PACKAGE_LINUX_FIRMWARE_QCOM_ADRENO<br> <br> Could you rework your patch to add those two options instead ?<br> <br> Thanks!<br> <br> Thomas Petazzoni<br> --<br> Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)<br> Embedded Linux and Kernel engineering<br> <a href="https://bootlin.com" rel="noreferrer" target="_blank">https://bootlin.com</a><br> </blockquote></div></div></div>
Hi Andre, On Wed, Apr 11, 2018 at 02:09:46AM +0000, Andre Renaud wrote: > Added Qualcomm video accelerator and GPU firmware option. > > Signed-off-by: Andre Renaud <andre@ignavus.net> ... > +ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_QCOM_VENUS),y) > +LINUX_FIRMWARE_DIRS += qcom/venus-1.8 qcom/venus-4.2 > +LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.qcom qcom/NOTICE.txt is also applicable, I think. Don't forget to update the .hash file. > +endif > + > +ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_QCOM_ADRENO),y) > +LINUX_FIRMWARE_FILES += qcom/a* > +LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.qcom > +endif baruch
Hello Andre, On Wed, 11 Apr 2018 02:09:46 +0000, Andre Renaud wrote: > Added Qualcomm video accelerator and GPU firmware option. > > Signed-off-by: Andre Renaud <andre@ignavus.net> > --- > package/linux-firmware/Config.in | 10 ++++++++++ > package/linux-firmware/linux-firmware.mk | 10 ++++++++++ > 2 files changed, 20 insertions(+) Thanks for this new version! Could you please sent this as a proper new patch with git send-email, and not pasted above the previous patch? Also, see beloiw. > diff --git a/package/linux-firmware/Config.in > b/package/linux-firmware/Config.in > index 4e77a3f9a3..9d8d8b340b 100644 > --- a/package/linux-firmware/Config.in > +++ b/package/linux-firmware/Config.in > @@ -29,6 +29,16 @@ config BR2_PACKAGE_LINUX_FIRMWARE_RADEON > help > Firmware files for AMD Radeon video cards. > > +config BR2_PACKAGE_LINUX_FIRMWARE_QCOM_VENUS > + bool "Qualcomm Venus video codec accelerator" > + help Indentation should be done with tabs > + Firmware files for Qualcomm Venus video codec accelerator And here one tab + 2 spaces. You can run ./utils/check-package package/linux-firmware/Config.in, which well validate this kind of trivial coding style issues. Thanks! Thomas
diff --git a/package/linux-firmware/Config.in b/package/linux-firmware/Config.in index 4e77a3f9a3..61164e101c 100644 --- a/package/linux-firmware/Config.in +++ b/package/linux-firmware/Config.in @@ -29,6 +29,11 @@ config BR2_PACKAGE_LINUX_FIRMWARE_RADEON help Firmware files for AMD Radeon video cards. +config BR2_PACKAGE_LINUX_FIRMWARE_QCOM + bool "Qualcomm video card firmware" + help + Firmware files for Qualcomm/Snapdragon video cards. + endmenu # Video menu "Bluetooth firmware" diff --git a/package/linux-firmware/linux-firmware.mk b/package/linux-firmware/linux-firmware.mk index a6fd8ebba4..da106a9941 100644 --- a/package/linux-firmware/linux-firmware.mk +++ b/package/linux-firmware/linux-firmware.mk @@ -24,6 +24,11 @@ LINUX_FIRMWARE_DIRS += radeon LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.radeon endif +ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_QCOM),y) +LINUX_FIRMWARE_DIRS += qcom +LINUX_FIRMWARE_ALL_LICENSE_FILES += LICENSE.qcom +endif + # Intel Wireless Bluetooth ifeq ($(BR2_PACKAGE_LINUX_FIRMWARE_IBT),y) LINUX_FIRMWARE_FILES += intel/ibt-*
Added Qualcomm video firmware option. Signed-off-by: Andre Renaud <andre@ignavus.net> --- package/linux-firmware/Config.in | 5 +++++ package/linux-firmware/linux-firmware.mk | 5 +++++ 2 files changed, 10 insertions(+)