Nouveau - Jan 2016 - [PATCH 0/5] nouveau: unified firmware loading functions

This patchset centralizes the firmware-loading procedure to one set of functions
instead of having each engine load its firmware as it pleases. This helps ensure
that all firmware comes from the same place, namely nvidia/<chip>/.

This changes where the firmware is fetched from for falcon/xtensa/bios, but
these locations never seemed to have been official anyway. Also for most (all?)
chips supported by Nouveau there is corresponding internal firmware, so
disruption should be minimal/non-existent. If this assumption is wrong, feel
free to drop patches 3-5. At the very least, firmware officially provided by
NVIDIA should be looked up using the new functions for consistency.

These new functions are also used by the secure boot series, which will follow
shortly.

Alexandre Courbot (5):
  core: add firmware handling functions
  gr/gf100: use the nvkm_firmware functions
  falcon: use the nvkm_firmware functions
  xtensa: use the nvkm_firmware functions
  bios: use the nvkm_firmware functions

 drm/nouveau/include/nvkm/core/firmware.h | 11 ++++++
 drm/nouveau/nvkm/core/Kbuild             |  1 +
 drm/nouveau/nvkm/core/firmware.c         | 64 ++++++++++++++++++++++++++++++++
 drm/nouveau/nvkm/engine/falcon.c         | 22 +++++------
 drm/nouveau/nvkm/engine/gr/gf100.c       | 18 ++-------
 drm/nouveau/nvkm/engine/xtensa.c         | 12 +++---
 drm/nouveau/nvkm/subdev/bios/shadow.c    |  7 ++--
 7 files changed, 99 insertions(+), 36 deletions(-)
 create mode 100644 drm/nouveau/include/nvkm/core/firmware.h
 create mode 100644 drm/nouveau/nvkm/core/firmware.c

-- 
2.7.0

Add two functions nvkm_firmware_get() and nvkm_firmware_put() to load a
firmware file and free its resources, respectively. Since firmware files
are becoming a necessity for new GPUs, and their location has been
standardized to nvidia/chip/, this will prevent duplicate and
error-prone name-generation code.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drm/nouveau/include/nvkm/core/firmware.h | 11 ++++++
 drm/nouveau/nvkm/core/Kbuild             |  1 +
 drm/nouveau/nvkm/core/firmware.c         | 64 ++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)
 create mode 100644 drm/nouveau/include/nvkm/core/firmware.h
 create mode 100644 drm/nouveau/nvkm/core/firmware.c

diff --git a/drm/nouveau/include/nvkm/core/firmware.h
b/drm/nouveau/include/nvkm/core/firmware.h
new file mode 100644
index 000000000000..a626ce378f04
--- /dev/null
+++ b/drm/nouveau/include/nvkm/core/firmware.h
@@ -0,0 +1,11 @@
+#ifndef __NVKM_FIRMWARE_H__
+#define __NVKM_FIRMWARE_H__
+
+#include <core/device.h>
+
+int nvkm_firmware_get(struct nvkm_device *device, const char *fwname,
+		      const struct firmware **fw);
+
+void nvkm_firmware_put(const struct firmware *fw);
+
+#endif
diff --git a/drm/nouveau/nvkm/core/Kbuild b/drm/nouveau/nvkm/core/Kbuild
index 7f66963f305c..86a31a8e1e51 100644
--- a/drm/nouveau/nvkm/core/Kbuild
+++ b/drm/nouveau/nvkm/core/Kbuild
@@ -2,6 +2,7 @@ nvkm-y := nvkm/core/client.o
 nvkm-y += nvkm/core/engine.o
 nvkm-y += nvkm/core/enum.o
 nvkm-y += nvkm/core/event.o
+nvkm-y += nvkm/core/firmware.o
 nvkm-y += nvkm/core/gpuobj.o
 nvkm-y += nvkm/core/ioctl.o
 nvkm-y += nvkm/core/memory.o
diff --git a/drm/nouveau/nvkm/core/firmware.c b/drm/nouveau/nvkm/core/firmware.c
new file mode 100644
index 000000000000..4a4b4a5d55a8
--- /dev/null
+++ b/drm/nouveau/nvkm/core/firmware.c
@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
"Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include <core/device.h>
+
+#include <linux/firmware.h>
+
+/**
+ * nvkm_firmware_get - load firmware from the official nvidia/chip/ directory
+ * @device	device that will use that firmware
+ * @fwname	name of firmware file to load
+ * @fw		firmware structure to load to
+ *
+ * Use this function to load firmware files in the form nvidia/chip/fwname.bin.
+ * Firmware files released by NVIDIA will always follow this format.
+ */
+int
+nvkm_firmware_get(struct nvkm_device *device, const char *fwname,
+		  const struct firmware **fw)
+{
+	char f[64];
+	char cname[16];
+	int i;
+
+	/* Convert device name to lowercase */
+	strncpy(cname, device->chip->name, sizeof(cname));
+	cname[sizeof(cname) - 1] = '\0';
+	i = strlen(cname);
+	while (i) {
+		--i;
+		cname[i] = tolower(cname[i]);
+	}
+
+	snprintf(f, sizeof(f), "nvidia/%s/%s.bin", cname, fwname);
+	return request_firmware(fw, f, device->dev);
+}
+
+/**
+ * nvkm_firmware_put - release firmware loaded with nvkm_firmware_get
+ */
+void
+nvkm_firmware_put(const struct firmware *fw)
+{
+	release_firmware(fw);
+}
-- 
2.7.0

Use the nvkm_firmware_* functions when loading external firmware to
avoid duplicate code.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drm/nouveau/nvkm/engine/gr/gf100.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drm/nouveau/nvkm/engine/gr/gf100.c
b/drm/nouveau/nvkm/engine/gr/gf100.c
index 1f81069edc58..f2410aff07cf 100644
--- a/drm/nouveau/nvkm/engine/gr/gf100.c
+++ b/drm/nouveau/nvkm/engine/gr/gf100.c
@@ -27,6 +27,7 @@
 
 #include <core/client.h>
 #include <core/option.h>
+#include <core/firmware.h>
 #include <subdev/fb.h>
 #include <subdev/mc.h>
 #include <subdev/pmu.h>
@@ -1720,22 +1721,9 @@ gf100_gr_ctor_fw(struct gf100_gr *gr, const char *fwname,
 	struct nvkm_subdev *subdev = &gr->base.engine.subdev;
 	struct nvkm_device *device = subdev->device;
 	const struct firmware *fw;
-	char f[64];
-	char cname[16];
 	int ret;
-	int i;
-
-	/* Convert device name to lowercase */
-	strncpy(cname, device->chip->name, sizeof(cname));
-	cname[sizeof(cname) - 1] = '\0';
-	i = strlen(cname);
-	while (i) {
-		--i;
-		cname[i] = tolower(cname[i]);
-	}
 
-	snprintf(f, sizeof(f), "nvidia/%s/%s.bin", cname, fwname);
-	ret = request_firmware(&fw, f, device->dev);
+	ret = nvkm_firmware_get(device, fwname, &fw);
 	if (ret) {
 		nvkm_error(subdev, "failed to load %s\n", fwname);
 		return ret;
@@ -1743,7 +1731,7 @@ gf100_gr_ctor_fw(struct gf100_gr *gr, const char *fwname,
 
 	fuc->size = fw->size;
 	fuc->data = kmemdup(fw->data, fuc->size, GFP_KERNEL);
-	release_firmware(fw);
+	nvkm_firmware_put(fw);
 	return (fuc->data != NULL) ? 0 : -ENOMEM;
 }
 
-- 
2.7.0

Use the newly-introduced nvkm_firmware functions. Note that this will
change the expected location of firmware files.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drm/nouveau/nvkm/engine/falcon.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drm/nouveau/nvkm/engine/falcon.c b/drm/nouveau/nvkm/engine/falcon.c
index 74000602fbb1..028113c6f82d 100644
--- a/drm/nouveau/nvkm/engine/falcon.c
+++ b/drm/nouveau/nvkm/engine/falcon.c
@@ -22,6 +22,7 @@
 #include <engine/falcon.h>
 
 #include <core/gpuobj.h>
+#include <core/firmware.h>
 #include <subdev/timer.h>
 #include <engine/fifo.h>
 
@@ -187,16 +188,15 @@ nvkm_falcon_init(struct nvkm_engine *engine)
 	 * locate a "self-bootstrapping" firmware image for the engine
 	 */
 	if (!falcon->code.data) {
-		snprintf(name, sizeof(name), "nouveau/nv%02x_fuc%03x",
-			 device->chipset, falcon->addr >> 12);
+		snprintf(name, sizeof(name), "fuc%03x", falcon->addr >>
12);
 
-		ret = request_firmware(&fw, name, device->dev);
+		ret = nvkm_firmware_get(device, name, &fw);
 		if (ret == 0) {
 			falcon->code.data = vmemdup(fw->data, fw->size);
 			falcon->code.size = fw->size;
 			falcon->data.data = NULL;
 			falcon->data.size = 0;
-			release_firmware(fw);
+			nvkm_firmware_put(fw);
 		}
 
 		falcon->external = true;
@@ -206,10 +206,9 @@ nvkm_falcon_init(struct nvkm_engine *engine)
 	 * images for the engine
 	 */
 	if (!falcon->code.data) {
-		snprintf(name, sizeof(name), "nouveau/nv%02x_fuc%03xd",
-			 device->chipset, falcon->addr >> 12);
+		snprintf(name, sizeof(name), "fuc%03xd", falcon->addr >>
12);
 
-		ret = request_firmware(&fw, name, device->dev);
+		ret = nvkm_firmware_get(device, name, &fw);
 		if (ret) {
 			nvkm_error(subdev, "unable to load firmware data\n");
 			return -ENODEV;
@@ -217,14 +216,13 @@ nvkm_falcon_init(struct nvkm_engine *engine)
 
 		falcon->data.data = vmemdup(fw->data, fw->size);
 		falcon->data.size = fw->size;
-		release_firmware(fw);
+		nvkm_firmware_put(fw);
 		if (!falcon->data.data)
 			return -ENOMEM;
 
-		snprintf(name, sizeof(name), "nouveau/nv%02x_fuc%03xc",
-			 device->chipset, falcon->addr >> 12);
+		snprintf(name, sizeof(name), "fuc%03xc", falcon->addr >>
12);
 
-		ret = request_firmware(&fw, name, device->dev);
+		ret = nvkm_firmware_get(device, name, &fw);
 		if (ret) {
 			nvkm_error(subdev, "unable to load firmware code\n");
 			return -ENODEV;
@@ -232,7 +230,7 @@ nvkm_falcon_init(struct nvkm_engine *engine)
 
 		falcon->code.data = vmemdup(fw->data, fw->size);
 		falcon->code.size = fw->size;
-		release_firmware(fw);
+		nvkm_firmware_put(fw);
 		if (!falcon->code.data)
 			return -ENOMEM;
 	}
-- 
2.7.0

Use the newly-introduced nvkm_firmware functions. Note that this will
change the expected location of firmware files.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drm/nouveau/nvkm/engine/xtensa.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drm/nouveau/nvkm/engine/xtensa.c b/drm/nouveau/nvkm/engine/xtensa.c
index a3d4f5bcec7a..608741a52fed 100644
--- a/drm/nouveau/nvkm/engine/xtensa.c
+++ b/drm/nouveau/nvkm/engine/xtensa.c
@@ -22,6 +22,7 @@
 #include <engine/xtensa.h>
 
 #include <core/gpuobj.h>
+#include <core/firmware.h>
 #include <engine/fifo.h>
 
 static int
@@ -104,10 +105,9 @@ nvkm_xtensa_init(struct nvkm_engine *engine)
 	u32 tmp;
 
 	if (!xtensa->gpu_fw) {
-		snprintf(name, sizeof(name), "nouveau/nv84_xuc%03x",
-			 xtensa->addr >> 12);
+		snprintf(name, sizeof(name), "xuc%03x", xtensa->addr >>
12);
 
-		ret = request_firmware(&fw, name, device->dev);
+		ret = nvkm_firmware_get(device, name, &fw);
 		if (ret) {
 			nvkm_warn(subdev, "unable to load firmware %s\n", name);
 			return ret;
@@ -115,7 +115,7 @@ nvkm_xtensa_init(struct nvkm_engine *engine)
 
 		if (fw->size > 0x40000) {
 			nvkm_warn(subdev, "firmware %s too large\n", name);
-			release_firmware(fw);
+			nvkm_firmware_put(fw);
 			return -EINVAL;
 		}
 
@@ -123,7 +123,7 @@ nvkm_xtensa_init(struct nvkm_engine *engine)
 				      0x40000, 0x1000, false,
 				      &xtensa->gpu_fw);
 		if (ret) {
-			release_firmware(fw);
+			nvkm_firmware_put(fw);
 			return ret;
 		}
 
@@ -131,7 +131,7 @@ nvkm_xtensa_init(struct nvkm_engine *engine)
 		for (i = 0; i < fw->size / 4; i++)
 			nvkm_wo32(xtensa->gpu_fw, i * 4, *((u32 *)fw->data + i));
 		nvkm_done(xtensa->gpu_fw);
-		release_firmware(fw);
+		nvkm_firmware_put(fw);
 	}
 
 	addr = nvkm_memory_addr(xtensa->gpu_fw);
-- 
2.7.0

Use the newly-introduced nvkm_firmware functions. Note that this will
change the expected location of firmware files.

Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
 drm/nouveau/nvkm/subdev/bios/shadow.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drm/nouveau/nvkm/subdev/bios/shadow.c
b/drm/nouveau/nvkm/subdev/bios/shadow.c
index b2557e87afdd..20814fc4dc1a 100644
--- a/drm/nouveau/nvkm/subdev/bios/shadow.c
+++ b/drm/nouveau/nvkm/subdev/bios/shadow.c
@@ -24,6 +24,7 @@
 #include "priv.h"
 
 #include <core/option.h>
+#include <core/firmware.h>
 #include <subdev/bios.h>
 #include <subdev/bios/image.h>
 
@@ -143,9 +144,9 @@ shadow_fw_read(void *data, u32 offset, u32 length, struct
nvkm_bios *bios)
 static void *
 shadow_fw_init(struct nvkm_bios *bios, const char *name)
 {
-	struct device *dev = bios->subdev.device->dev;
+	struct nvkm_device *device = bios->subdev.device;
 	const struct firmware *fw;
-	int ret = request_firmware(&fw, name, dev);
+	int ret = nvkm_firmware_get(device, name, &fw);
 	if (ret)
 		return ERR_PTR(-ENOENT);
 	return (void *)fw;
@@ -155,7 +156,7 @@ static const struct nvbios_source
 shadow_fw = {
 	.name = "firmware",
 	.init = shadow_fw_init,
-	.fini = (void(*)(void *))release_firmware,
+	.fini = (void(*)(void *))nvkm_firmware_put,
 	.read = shadow_fw_read,
 	.rw = false,
 };
-- 
2.7.0

All the video decoding firmware was *pretty* standard... I suspect
~everyone used my extraction script and it's sitting in
/lib/firmware/nouveau for everyone who uses VDPAU. There are packages
in several distributions which grab the nvidia blob, my script, and
run the extraction on the end user's computer to avoid licensing
complications. Please support the existing locations.

Thanks,

  -ilia

On Mon, Jan 18, 2016 at 1:07 AM, Alexandre Courbot <acourbot at
nvidia.com> wrote:
> This patchset centralizes the firmware-loading procedure to one set of
functions
> instead of having each engine load its firmware as it pleases. This helps
ensure
> that all firmware comes from the same place, namely nvidia/<chip>/.
>
> This changes where the firmware is fetched from for falcon/xtensa/bios, but
> these locations never seemed to have been official anyway. Also for most
(all?)
> chips supported by Nouveau there is corresponding internal firmware, so
> disruption should be minimal/non-existent. If this assumption is wrong,
feel
> free to drop patches 3-5. At the very least, firmware officially provided
by
> NVIDIA should be looked up using the new functions for consistency.
>
> These new functions are also used by the secure boot series, which will
follow
> shortly.
>
> Alexandre Courbot (5):
>   core: add firmware handling functions
>   gr/gf100: use the nvkm_firmware functions
>   falcon: use the nvkm_firmware functions
>   xtensa: use the nvkm_firmware functions
>   bios: use the nvkm_firmware functions
>
>  drm/nouveau/include/nvkm/core/firmware.h | 11 ++++++
>  drm/nouveau/nvkm/core/Kbuild             |  1 +
>  drm/nouveau/nvkm/core/firmware.c         | 64
++++++++++++++++++++++++++++++++
>  drm/nouveau/nvkm/engine/falcon.c         | 22 +++++------
>  drm/nouveau/nvkm/engine/gr/gf100.c       | 18 ++-------
>  drm/nouveau/nvkm/engine/xtensa.c         | 12 +++---
>  drm/nouveau/nvkm/subdev/bios/shadow.c    |  7 ++--
>  7 files changed, 99 insertions(+), 36 deletions(-)
>  create mode 100644 drm/nouveau/include/nvkm/core/firmware.h
>  create mode 100644 drm/nouveau/nvkm/core/firmware.c
>
> --
> 2.7.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/nouveau

On Mon, Jan 18, 2016 at 3:11 PM, Ilia Mirkin <imirkin at alum.mit.edu>
wrote:
> All the video decoding firmware was *pretty* standard... I suspect
> ~everyone used my extraction script and it's sitting in
> /lib/firmware/nouveau for everyone who uses VDPAU. There are packages
> in several distributions which grab the nvidia blob, my script, and
> run the extraction on the end user's computer to avoid licensing
> complications. Please support the existing locations.
Ok, so there *are* some end-users (not developers) relying on these
external firmwares?

In that case probably the best thing to do is to not use these new
functions for video - at least for chips that do not require signed
firmware. All I'd like to do is reduce the number of occurences of the
"build path - load firmware" logic.

Hi Alexandre,

On 18 January 2016 at 06:07, Alexandre Courbot <acourbot at nvidia.com>
wrote:
> Add two functions nvkm_firmware_get() and nvkm_firmware_put() to load a
> firmware file and free its resources, respectively. Since firmware files
> are becoming a necessity for new GPUs, and their location has been
> standardized to nvidia/chip/, this will prevent duplicate and
> error-prone name-generation code.
>
Strictly speaking this will break video decoding (and maybe others)
although I'm not sure how much one could care. Then again pinging
people to update the scripts [1] will be good :)

[1] http://nouveau.freedesktop.org/wiki/VideoAcceleration/#firmware
> Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
> ---
>  drm/nouveau/include/nvkm/core/firmware.h | 11 ++++++
>  drm/nouveau/nvkm/core/Kbuild             |  1 +
>  drm/nouveau/nvkm/core/firmware.c         | 64
++++++++++++++++++++++++++++++++
>  3 files changed, 76 insertions(+)
>  create mode 100644 drm/nouveau/include/nvkm/core/firmware.h
>  create mode 100644 drm/nouveau/nvkm/core/firmware.c
>
> diff --git a/drm/nouveau/include/nvkm/core/firmware.h
b/drm/nouveau/include/nvkm/core/firmware.h
> new file mode 100644
> index 000000000000..a626ce378f04
> --- /dev/null
> +++ b/drm/nouveau/include/nvkm/core/firmware.h
> @@ -0,0 +1,11 @@
> +#ifndef __NVKM_FIRMWARE_H__
> +#define __NVKM_FIRMWARE_H__
> +
> +#include <core/device.h>
> +
> +int nvkm_firmware_get(struct nvkm_device *device, const char *fwname,
> +                     const struct firmware **fw);
> +
> +void nvkm_firmware_put(const struct firmware *fw);
> +
> +#endif
> diff --git a/drm/nouveau/nvkm/core/Kbuild b/drm/nouveau/nvkm/core/Kbuild
> index 7f66963f305c..86a31a8e1e51 100644
> --- a/drm/nouveau/nvkm/core/Kbuild
> +++ b/drm/nouveau/nvkm/core/Kbuild
> @@ -2,6 +2,7 @@ nvkm-y := nvkm/core/client.o
>  nvkm-y += nvkm/core/engine.o
>  nvkm-y += nvkm/core/enum.o
>  nvkm-y += nvkm/core/event.o
> +nvkm-y += nvkm/core/firmware.o
>  nvkm-y += nvkm/core/gpuobj.o
>  nvkm-y += nvkm/core/ioctl.o
>  nvkm-y += nvkm/core/memory.o
> diff --git a/drm/nouveau/nvkm/core/firmware.c
b/drm/nouveau/nvkm/core/firmware.c
> new file mode 100644
> index 000000000000..4a4b4a5d55a8
> --- /dev/null
> +++ b/drm/nouveau/nvkm/core/firmware.c
> @@ -0,0 +1,64 @@
> +/*
> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
"Software"),
> + * to deal in the Software without restriction, including without
limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included
in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <core/device.h>
> +
> +#include <linux/firmware.h>
> +
> +/**
> + * nvkm_firmware_get - load firmware from the official nvidia/chip/
directory
> + * @device     device that will use that firmware
> + * @fwname     name of firmware file to load
> + * @fw         firmware structure to load to
> + *
> + * Use this function to load firmware files in the form
nvidia/chip/fwname.bin.
> + * Firmware files released by NVIDIA will always follow this format.
> + */
> +int
> +nvkm_firmware_get(struct nvkm_device *device, const char *fwname,
> +                 const struct firmware **fw)
> +{
> +       char f[64];
> +       char cname[16];
> +       int i;
> +
> +       /* Convert device name to lowercase */
> +       strncpy(cname, device->chip->name, sizeof(cname));
> +       cname[sizeof(cname) - 1] = '\0';
> +       i = strlen(cname);
> +       while (i) {
> +               --i;
> +               cname[i] = tolower(cname[i]);
> +       }
> +
Considering there's only one user that might need this, is it worth
adding it here or just keeping it where needed ?
> +       snprintf(f, sizeof(f), "nvidia/%s/%s.bin", cname,
fwname);
> +       return request_firmware(fw, f, device->dev);
> +}
> +
> +/**
> + * nvkm_firmware_put - release firmware loaded with nvkm_firmware_get
> + */
> +void
> +nvkm_firmware_put(const struct firmware *fw)
> +{
> +       release_firmware(fw);
If the above suggestion makes sense, then it might be better to keep
these two in the header as static inlines ?

Not that either of the two is a hot path, then again having a one/two
liner as separate function feels odd.

Cheers,
Emil