[PATCH v5 5/7] accel/ivpu: Implement firmware parsing and booting
Jacek Lawrynowicz
jacek.lawrynowicz at linux.intel.com
Wed Jan 11 08:15:06 UTC 2023
Hi,
On 10.01.2023 16:34, Oded Gabbay wrote:
> On Mon, Jan 9, 2023 at 2:24 PM Jacek Lawrynowicz
> <jacek.lawrynowicz at linux.intel.com> wrote:
>> Read, parse and boot VPU firmware image.
>>
>> Co-developed-by: Andrzej Kacprowski <andrzej.kacprowski at linux.intel.com>
>> Signed-off-by: Andrzej Kacprowski <andrzej.kacprowski at linux.intel.com>
>> Co-developed-by: Krystian Pradzynski <krystian.pradzynski at linux.intel.com>
>> Signed-off-by: Krystian Pradzynski <krystian.pradzynski at linux.intel.com>
>> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz at linux.intel.com>
>> ---
>> drivers/accel/ivpu/Makefile | 1 +
>> drivers/accel/ivpu/ivpu_drv.c | 131 +++++++++-
>> drivers/accel/ivpu/ivpu_drv.h | 11 +
>> drivers/accel/ivpu/ivpu_fw.c | 419 ++++++++++++++++++++++++++++++
>> drivers/accel/ivpu/ivpu_fw.h | 38 +++
>> drivers/accel/ivpu/ivpu_hw_mtl.c | 10 +
>> drivers/accel/ivpu/vpu_boot_api.h | 349 +++++++++++++++++++++++++
>> include/uapi/drm/ivpu_accel.h | 21 ++
>> 8 files changed, 979 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/accel/ivpu/ivpu_fw.c
>> create mode 100644 drivers/accel/ivpu/ivpu_fw.h
>> create mode 100644 drivers/accel/ivpu/vpu_boot_api.h
>>
>> diff --git a/drivers/accel/ivpu/Makefile b/drivers/accel/ivpu/Makefile
>> index 46595f0112e3..9fa6a76e9d79 100644
>> --- a/drivers/accel/ivpu/Makefile
>> +++ b/drivers/accel/ivpu/Makefile
>> @@ -3,6 +3,7 @@
>>
>> intel_vpu-y := \
>> ivpu_drv.o \
>> + ivpu_fw.o \
>> ivpu_gem.o \
>> ivpu_hw_mtl.o \
>> ivpu_ipc.o \
>> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
>> index 6643ae6b5a52..53e103f64832 100644
>> --- a/drivers/accel/ivpu/ivpu_drv.c
>> +++ b/drivers/accel/ivpu/ivpu_drv.c
>> @@ -14,10 +14,13 @@
>> #include <drm/drm_ioctl.h>
>> #include <drm/drm_prime.h>
>>
>> +#include "vpu_boot_api.h"
>> #include "ivpu_drv.h"
>> +#include "ivpu_fw.h"
>> #include "ivpu_gem.h"
>> #include "ivpu_hw.h"
>> #include "ivpu_ipc.h"
>> +#include "ivpu_jsm_msg.h"
>> #include "ivpu_mmu.h"
>> #include "ivpu_mmu_context.h"
>>
>> @@ -32,6 +35,10 @@ int ivpu_dbg_mask;
>> module_param_named(dbg_mask, ivpu_dbg_mask, int, 0644);
>> MODULE_PARM_DESC(dbg_mask, "Driver debug mask. See IVPU_DBG_* macros.");
>>
>> +int ivpu_test_mode;
>> +module_param_named_unsafe(test_mode, ivpu_test_mode, int, 0644);
>> +MODULE_PARM_DESC(test_mode, "Test mode: 0 - normal operation, 1 - fw unit test, 2 - null hw");
>> +
>> u8 ivpu_pll_min_ratio;
>> module_param_named(pll_min_ratio, ivpu_pll_min_ratio, byte, 0644);
>> MODULE_PARM_DESC(pll_min_ratio, "Minimum PLL ratio used to set VPU frequency");
>> @@ -129,6 +136,28 @@ static int ivpu_get_param_ioctl(struct drm_device *dev, void *data, struct drm_f
>> case DRM_IVPU_PARAM_CONTEXT_ID:
>> args->value = file_priv->ctx.id;
>> break;
>> + case DRM_IVPU_PARAM_FW_API_VERSION:
>> + if (args->index < VPU_FW_API_VER_NUM) {
>> + struct vpu_firmware_header *fw_hdr;
>> +
>> + fw_hdr = (struct vpu_firmware_header *)vdev->fw->file->data;
>> + args->value = fw_hdr->api_version[args->index];
>> + } else {
>> + ret = -EINVAL;
>> + }
>> + break;
>> + case DRM_IVPU_PARAM_ENGINE_HEARTBEAT:
>> + ret = ivpu_jsm_get_heartbeat(vdev, args->index, &args->value);
>> + break;
>> + case DRM_IVPU_PARAM_UNIQUE_INFERENCE_ID:
>> + args->value = (u64)atomic64_inc_return(&vdev->unique_id_counter);
>> + break;
>> + case DRM_IVPU_PARAM_TILE_CONFIG:
>> + args->value = vdev->hw->tile_fuse;
>> + break;
>> + case DRM_IVPU_PARAM_SKU:
>> + args->value = vdev->hw->sku;
>> + break;
>> default:
>> ret = -EINVAL;
>> break;
>> @@ -226,11 +255,85 @@ static const struct drm_ioctl_desc ivpu_drm_ioctls[] = {
>> DRM_IOCTL_DEF_DRV(IVPU_BO_USERPTR, ivpu_bo_userptr_ioctl, 0),
>> };
>>
>> +static int ivpu_wait_for_ready(struct ivpu_device *vdev)
>> +{
>> + struct ivpu_ipc_consumer cons;
>> + struct ivpu_ipc_hdr ipc_hdr;
>> + unsigned long timeout;
>> + int ret;
>> +
>> + if (ivpu_test_mode == IVPU_TEST_MODE_FW_TEST)
>> + return 0;
>> +
>> + ivpu_ipc_consumer_add(vdev, &cons, IVPU_IPC_CHAN_BOOT_MSG);
>> +
>> + timeout = jiffies + msecs_to_jiffies(vdev->timeout.boot);
>> + while (1) {
>> + ret = ivpu_ipc_irq_handler(vdev);
>> + if (ret)
>> + break;
>> + ret = ivpu_ipc_receive(vdev, &cons, &ipc_hdr, NULL, 0);
>> + if (ret != -ETIMEDOUT || time_after_eq(jiffies, timeout))
>> + break;
>> +
>> + cond_resched();
>> + }
>> +
>> + ivpu_ipc_consumer_del(vdev, &cons);
>> +
>> + if (!ret && ipc_hdr.data_addr != IVPU_IPC_BOOT_MSG_DATA_ADDR) {
>> + ivpu_err(vdev, "Invalid VPU ready message: 0x%x\n",
>> + ipc_hdr.data_addr);
>> + return -EIO;
>> + }
>> +
>> + if (!ret)
>> + ivpu_info(vdev, "VPU ready message received successfully\n");
>> + else
>> + ivpu_hw_diagnose_failure(vdev);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * ivpu_boot() - Start VPU firmware
>> + * @vdev: VPU device
>> + *
>> + * This function is paired with ivpu_shutdown() but it doesn't power up the
>> + * VPU because power up has to be called very early in ivpu_probe().
>> + */
>> +int ivpu_boot(struct ivpu_device *vdev)
>> +{
>> + int ret;
>> +
>> + /* Update boot params located at first 4KB of FW memory */
>> + ivpu_fw_boot_params_setup(vdev, vdev->fw->mem->kvaddr);
>> +
>> + ret = ivpu_hw_boot_fw(vdev);
>> + if (ret) {
>> + ivpu_err(vdev, "Failed to start the firmware: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = ivpu_wait_for_ready(vdev);
>> + if (ret) {
>> + ivpu_err(vdev, "Failed to boot the firmware: %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ivpu_hw_irq_clear(vdev);
>> + enable_irq(vdev->irq);
>> + ivpu_hw_irq_enable(vdev);
>> + ivpu_ipc_enable(vdev);
>> + return 0;
>> +}
>> +
>> int ivpu_shutdown(struct ivpu_device *vdev)
>> {
>> int ret;
>>
>> ivpu_hw_irq_disable(vdev);
>> + disable_irq(vdev->irq);
>> ivpu_ipc_disable(vdev);
>> ivpu_mmu_disable(vdev);
>>
>> @@ -348,6 +451,10 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
>> if (!vdev->mmu)
>> return -ENOMEM;
>>
>> + vdev->fw = drmm_kzalloc(&vdev->drm, sizeof(*vdev->fw), GFP_KERNEL);
>> + if (!vdev->fw)
>> + return -ENOMEM;
>> +
>> vdev->ipc = drmm_kzalloc(&vdev->drm, sizeof(*vdev->ipc), GFP_KERNEL);
>> if (!vdev->ipc)
>> return -ENOMEM;
>> @@ -356,6 +463,7 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
>> vdev->platform = IVPU_PLATFORM_INVALID;
>> vdev->context_xa_limit.min = IVPU_GLOBAL_CONTEXT_MMU_SSID + 1;
>> vdev->context_xa_limit.max = IVPU_CONTEXT_LIMIT;
>> + atomic64_set(&vdev->unique_id_counter, 0);
>> xa_init_flags(&vdev->context_xa, XA_FLAGS_ALLOC);
>>
>> ret = ivpu_pci_init(vdev);
>> @@ -396,14 +504,34 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
>> goto err_mmu_gctx_fini;
>> }
>>
>> + ret = ivpu_fw_init(vdev);
>> + if (ret) {
>> + ivpu_err(vdev, "Failed to initialize firmware: %d\n", ret);
>> + goto err_mmu_gctx_fini;
>> + }
>> +
>> ret = ivpu_ipc_init(vdev);
>> if (ret) {
>> ivpu_err(vdev, "Failed to initialize IPC: %d\n", ret);
>> - goto err_mmu_gctx_fini;
>> + goto err_fw_fini;
>> + }
>> +
>> + ret = ivpu_fw_load(vdev);
>> + if (ret) {
>> + ivpu_err(vdev, "Failed to load firmware: %d\n", ret);
>> + goto err_fw_fini;
>> + }
>> +
>> + ret = ivpu_boot(vdev);
>> + if (ret) {
>> + ivpu_err(vdev, "Failed to boot: %d\n", ret);
>> + goto err_fw_fini;
>> }
>>
>> return 0;
>>
>> +err_fw_fini:
>> + ivpu_fw_fini(vdev);
>> err_mmu_gctx_fini:
>> ivpu_mmu_global_context_fini(vdev);
>> err_power_down:
>> @@ -417,6 +545,7 @@ static void ivpu_dev_fini(struct ivpu_device *vdev)
>> {
>> ivpu_shutdown(vdev);
>> ivpu_ipc_fini(vdev);
>> + ivpu_fw_fini(vdev);
>> ivpu_mmu_global_context_fini(vdev);
>>
>> drm_WARN_ON(&vdev->drm, !xa_empty(&vdev->context_xa));
>> diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
>> index c1e76d1fb8ba..317ae61f43bd 100644
>> --- a/drivers/accel/ivpu/ivpu_drv.h
>> +++ b/drivers/accel/ivpu/ivpu_drv.h
>> @@ -74,6 +74,7 @@ struct ivpu_wa_table {
>>
>> struct ivpu_hw_info;
>> struct ivpu_mmu_info;
>> +struct ivpu_fw_info;
>> struct ivpu_ipc_info;
>>
>> struct ivpu_device {
>> @@ -86,12 +87,15 @@ struct ivpu_device {
>> struct ivpu_wa_table wa;
>> struct ivpu_hw_info *hw;
>> struct ivpu_mmu_info *mmu;
>> + struct ivpu_fw_info *fw;
>> struct ivpu_ipc_info *ipc;
>>
>> struct ivpu_mmu_context gctx;
>> struct xarray context_xa;
>> struct xa_limit context_xa_limit;
>>
>> + atomic64_t unique_id_counter;
>> +
>> struct {
>> int boot;
>> int jsm;
>> @@ -116,9 +120,16 @@ extern int ivpu_dbg_mask;
>> extern u8 ivpu_pll_min_ratio;
>> extern u8 ivpu_pll_max_ratio;
>>
>> +#define IVPU_TEST_MODE_DISABLED 0
>> +#define IVPU_TEST_MODE_FW_TEST 1
>> +#define IVPU_TEST_MODE_NULL_HW 2
>> +extern int ivpu_test_mode;
>> +
>> struct ivpu_file_priv *ivpu_file_priv_get(struct ivpu_file_priv *file_priv);
>> struct ivpu_file_priv *ivpu_file_priv_get_by_ctx_id(struct ivpu_device *vdev, unsigned long id);
>> void ivpu_file_priv_put(struct ivpu_file_priv **link);
>> +
>> +int ivpu_boot(struct ivpu_device *vdev);
>> int ivpu_shutdown(struct ivpu_device *vdev);
>>
>> static inline bool ivpu_is_mtl(struct ivpu_device *vdev)
>> diff --git a/drivers/accel/ivpu/ivpu_fw.c b/drivers/accel/ivpu/ivpu_fw.c
>> new file mode 100644
>> index 000000000000..4baa0767a10d
>> --- /dev/null
>> +++ b/drivers/accel/ivpu/ivpu_fw.c
>> @@ -0,0 +1,419 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2020-2023 Intel Corporation
>> + */
>> +
>> +#include <linux/firmware.h>
>> +#include <linux/highmem.h>
>> +#include <linux/moduleparam.h>
>> +#include <linux/pci.h>
>> +
>> +#include "vpu_boot_api.h"
>> +#include "ivpu_drv.h"
>> +#include "ivpu_fw.h"
>> +#include "ivpu_gem.h"
>> +#include "ivpu_hw.h"
>> +#include "ivpu_ipc.h"
>> +
>> +#define FW_GLOBAL_MEM_START (2ull * SZ_1G)
>> +#define FW_GLOBAL_MEM_END (3ull * SZ_1G)
>> +#define FW_SHARED_MEM_SIZE SZ_256M /* Must be aligned to FW_SHARED_MEM_ALIGNMENT */
>> +#define FW_SHARED_MEM_ALIGNMENT SZ_128K /* VPU MTRR limitation */
>> +#define FW_RUNTIME_MAX_SIZE SZ_512M
>> +#define FW_SHAVE_NN_MAX_SIZE SZ_2M
>> +#define FW_RUNTIME_MIN_ADDR (FW_GLOBAL_MEM_START)
>> +#define FW_RUNTIME_MAX_ADDR (FW_GLOBAL_MEM_END - FW_SHARED_MEM_SIZE)
>> +#define FW_VERSION_HEADER_SIZE SZ_4K
>> +#define FW_FILE_IMAGE_OFFSET (VPU_FW_HEADER_SIZE + FW_VERSION_HEADER_SIZE)
>> +
>> +#define WATCHDOG_MSS_REDIRECT 32
>> +#define WATCHDOG_NCE_REDIRECT 33
>> +
>> +#define ADDR_TO_L2_CACHE_CFG(addr) ((addr) >> 31)
>> +
>> +#define IVPU_FW_CHECK_API(vdev, fw_hdr, name) ivpu_fw_check_api(vdev, fw_hdr, #name, \
>> + VPU_##name##_API_VER_INDEX, \
>> + VPU_##name##_API_VER_MAJOR, \
>> + VPU_##name##_API_VER_MINOR)
>> +
>> +static char *ivpu_firmware;
>> +module_param_named_unsafe(firmware, ivpu_firmware, charp, 0644);
>> +MODULE_PARM_DESC(firmware, "VPU firmware binary in /lib/firmware/..");
>> +
>> +static int ivpu_fw_request(struct ivpu_device *vdev)
>> +{
>> + static const char * const fw_names[] = {
>> + "mtl_vpu.bin",
>> + "intel/vpu/mtl_vpu_v0.0.bin"
>> + };
>> + int ret = -ENOENT;
>> + int i;
>> +
>> + if (ivpu_firmware)
>> + return request_firmware(&vdev->fw->file, ivpu_firmware, vdev->drm.dev);
>> +
>> + for (i = 0; i < ARRAY_SIZE(fw_names); i++) {
>> + ret = firmware_request_nowarn(&vdev->fw->file, fw_names[i], vdev->drm.dev);
>> + if (!ret)
>> + return 0;
>> + }
>> +
>> + ivpu_err(vdev, "Failed to request firmware: %d\n", ret);
>> + return ret;
>> +}
>> +
>> +static void
>> +ivpu_fw_check_api(struct ivpu_device *vdev, const struct vpu_firmware_header *fw_hdr,
>> + const char *str, int index, u16 expected_major, u16 expected_minor)
>> +{
>> + u16 major = (u16)(fw_hdr->api_version[index] >> 16);
>> + u16 minor = (u16)(fw_hdr->api_version[index]);
>> +
>> + if (major != expected_major) {
>> + ivpu_warn(vdev, "Incompatible FW %s API version: %d.%d (expected %d.%d)\n",
>> + str, major, minor, expected_major, expected_minor);
>> + }
>> + ivpu_dbg(vdev, FW_BOOT, "FW %s API version: %d.%d (expected %d.%d)\n",
>> + str, major, minor, expected_major, expected_minor);
>> +}
>> +
>> +static int ivpu_fw_parse(struct ivpu_device *vdev)
>> +{
>> + struct ivpu_fw_info *fw = vdev->fw;
>> + const struct vpu_firmware_header *fw_hdr = (const void *)fw->file->data;
>> + u64 runtime_addr, image_load_addr, runtime_size, image_size;
>> +
>> + if (fw->file->size <= FW_FILE_IMAGE_OFFSET) {
>> + ivpu_err(vdev, "Firmware file is too small: %zu\n", fw->file->size);
>> + return -EINVAL;
>> + }
>> +
>> + if (fw_hdr->header_version != VPU_FW_HEADER_VERSION) {
>> + ivpu_err(vdev, "Invalid firmware header version: %u\n", fw_hdr->header_version);
>> + return -EINVAL;
>> + }
>> +
>> + runtime_addr = fw_hdr->boot_params_load_address;
>> + runtime_size = fw_hdr->runtime_size;
>> + image_load_addr = fw_hdr->image_load_address;
>> + image_size = fw_hdr->image_size;
>> +
>> + if (runtime_addr < FW_RUNTIME_MIN_ADDR || runtime_addr > FW_RUNTIME_MAX_ADDR) {
>> + ivpu_err(vdev, "Invalid firmware runtime address: 0x%llx\n", runtime_addr);
>> + return -EINVAL;
>> + }
>> +
>> + if (runtime_size < fw->file->size || runtime_size > FW_RUNTIME_MAX_SIZE) {
>> + ivpu_err(vdev, "Invalid firmware runtime size: %llu\n", runtime_size);
>> + return -EINVAL;
>> + }
>> +
>> + if (FW_FILE_IMAGE_OFFSET + image_size > fw->file->size) {
>> + ivpu_err(vdev, "Invalid image size: %llu\n", image_size);
>> + return -EINVAL;
>> + }
>> +
>> + if (image_load_addr < runtime_addr ||
>> + image_load_addr + image_size > runtime_addr + runtime_size) {
>> + ivpu_err(vdev, "Invalid firmware load address size: 0x%llx and size %llu\n",
>> + image_load_addr, image_size);
>> + return -EINVAL;
>> + }
>> +
>> + if (fw_hdr->shave_nn_fw_size > FW_SHAVE_NN_MAX_SIZE) {
>> + ivpu_err(vdev, "SHAVE NN firmware is too big: %u\n", fw_hdr->shave_nn_fw_size);
>> + return -EINVAL;
>> + }
>> +
>> + if (fw_hdr->entry_point < image_load_addr ||
>> + fw_hdr->entry_point >= image_load_addr + image_size) {
>> + ivpu_err(vdev, "Invalid entry point: 0x%llx\n", fw_hdr->entry_point);
>> + return -EINVAL;
>> + }
>> +
>> + fw->runtime_addr = runtime_addr;
>> + fw->runtime_size = runtime_size;
>> + fw->image_load_offset = image_load_addr - runtime_addr;
>> + fw->image_size = image_size;
>> + fw->shave_nn_size = PAGE_ALIGN(fw_hdr->shave_nn_fw_size);
>> +
>> + fw->cold_boot_entry_point = fw_hdr->entry_point;
>> + fw->entry_point = fw->cold_boot_entry_point;
>> +
>> + ivpu_dbg(vdev, FW_BOOT, "Header version: 0x%x, format 0x%x\n",
>> + fw_hdr->header_version, fw_hdr->image_format);
>> + ivpu_dbg(vdev, FW_BOOT, "Size: file %lu image %u runtime %u shavenn %u\n",
>> + fw->file->size, fw->image_size, fw->runtime_size, fw->shave_nn_size);
>> + ivpu_dbg(vdev, FW_BOOT, "Address: runtime 0x%llx, load 0x%llx, entry point 0x%llx\n",
>> + fw->runtime_addr, image_load_addr, fw->entry_point);
>> + ivpu_dbg(vdev, FW_BOOT, "FW version: %s\n", (char *)fw_hdr + VPU_FW_HEADER_SIZE);
>> +
>> + IVPU_FW_CHECK_API(vdev, fw_hdr, BOOT);
>> + IVPU_FW_CHECK_API(vdev, fw_hdr, JSM);
>> +
>> + return 0;
>> +}
>> +
>> +static void ivpu_fw_release(struct ivpu_device *vdev)
>> +{
>> + release_firmware(vdev->fw->file);
>> +}
>> +
>> +static int ivpu_fw_update_global_range(struct ivpu_device *vdev)
>> +{
>> + struct ivpu_fw_info *fw = vdev->fw;
>> + u64 start = ALIGN(fw->runtime_addr + fw->runtime_size, FW_SHARED_MEM_ALIGNMENT);
>> + u64 size = FW_SHARED_MEM_SIZE;
>> +
>> + if (start + size > FW_GLOBAL_MEM_END) {
>> + ivpu_err(vdev, "No space for shared region, start %lld, size %lld\n", start, size);
>> + return -EINVAL;
>> + }
>> +
>> + ivpu_hw_init_range(&vdev->hw->ranges.global_low, start, size);
>> + return 0;
>> +}
>> +
>> +static int ivpu_fw_mem_init(struct ivpu_device *vdev)
>> +{
>> + struct ivpu_fw_info *fw = vdev->fw;
>> + int ret;
>> +
>> + ret = ivpu_fw_update_global_range(vdev);
>> + if (ret)
>> + return ret;
>> +
>> + fw->mem = ivpu_bo_alloc_internal(vdev, fw->runtime_addr, fw->runtime_size, DRM_IVPU_BO_WC);
>> + if (!fw->mem) {
>> + ivpu_err(vdev, "Failed to allocate firmware runtime memory\n");
>> + return -ENOMEM;
>> + }
>> +
>> + if (fw->shave_nn_size) {
>> + fw->mem_shave_nn = ivpu_bo_alloc_internal(vdev, vdev->hw->ranges.global_high.start,
>> + fw->shave_nn_size, DRM_IVPU_BO_UNCACHED);
>> + if (!fw->mem_shave_nn) {
>> + ivpu_err(vdev, "Failed to allocate shavenn buffer\n");
>> + ivpu_bo_free_internal(fw->mem);
>> + return -ENOMEM;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void ivpu_fw_mem_fini(struct ivpu_device *vdev)
>> +{
>> + struct ivpu_fw_info *fw = vdev->fw;
>> +
>> + if (fw->mem_shave_nn) {
>> + ivpu_bo_free_internal(fw->mem_shave_nn);
>> + fw->mem_shave_nn = NULL;
>> + }
>> +
>> + ivpu_bo_free_internal(fw->mem);
>> + fw->mem = NULL;
>> +}
>> +
>> +int ivpu_fw_init(struct ivpu_device *vdev)
>> +{
>> + int ret;
>> +
>> + ret = ivpu_fw_request(vdev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = ivpu_fw_parse(vdev);
>> + if (ret)
>> + goto err_fw_release;
>> +
>> + ret = ivpu_fw_mem_init(vdev);
>> + if (ret)
>> + goto err_fw_release;
>> +
>> + return 0;
>> +
>> +err_fw_release:
>> + ivpu_fw_release(vdev);
> Why do you release the firmware only on error ?
> Why maintain the hold on the firmware file for as long as the device lives ?
> Can't you release the file once you loaded the firmware to the device's memory ?
Yes, we keep the fw image in memory for the following reasons:
1. It reduces the time of reset, recovery and resume without warmboot, where we need to overwite the fw.
2. It prevents user space from changing the fw binary while the module is loaded.
If a new version of the fw was installed it will be used on the next insmod which prevents hard to detect bugs.
Regards,
Jacek
More information about the dri-devel
mailing list