[Intel-gfx] [PATCH v2 1/1] drm/i915/uc: Make GuC/HuC fw fetch and loading functions/file structure symmetric
Sagar Arun Kamble
sagar.a.kamble at intel.com
Thu Mar 1 15:01:45 UTC 2018
On 3/1/2018 8:21 PM, Michal Wajdeczko wrote:
> On Thu, 01 Mar 2018 15:47:22 +0100, Sagar Arun Kamble
> <sagar.a.kamble at intel.com> wrote:
>
>> GuC load function is named intel_guc_fw_upload() and HuC load
>> function is
>> named intel_huc_init_hw(). Make them consistent intel_*_fw_upload. Also
>> move HuC fw loading functions and declarations to separate files
>> intel_huc_fw.c|h like GuC.
>>
>> While at this, do below changes
>> 1. Update kernel-doc comment for intel_*_fw_upload() functions
>> 2. s/huc_ucode_xfer/huc_fw_xfer
>> 3. Introduce intel_huc_fw_init_early()
>>
>> v2: Changed patch to update HuC functions instead of changing
>> guc_fw_upload and update file structure. (Michal Wajdeczko)
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> Cc: Michal Winiarski <michal.winiarski at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
<snip>
>> diff --git a/drivers/gpu/drm/i915/intel_huc.h
>> b/drivers/gpu/drm/i915/intel_huc.h
>> index 40039db..5d6e804 100644
>> --- a/drivers/gpu/drm/i915/intel_huc.h
>> +++ b/drivers/gpu/drm/i915/intel_huc.h
>> @@ -26,6 +26,7 @@
>> #define _INTEL_HUC_H_
>> #include "intel_uc_fw.h"
>> +#include "intel_huc_fw.h"
>
> hmm, as we don't need anything from this header, maybe we can drop it ?
fw_init_early called from huc.c and fw_upload from uc.c which can access
through intel_huc.h.
Similar to GuC. Not including huc_fw.h directly in huc.c or uc.c
>
>> struct intel_huc {
>> /* Generic uC firmware management */
>> @@ -35,7 +36,6 @@ struct intel_huc {
>> };
>> void intel_huc_init_early(struct intel_huc *huc);
>> -int intel_huc_init_hw(struct intel_huc *huc);
>> int intel_huc_auth(struct intel_huc *huc);
>> #endif
>> diff --git a/drivers/gpu/drm/i915/intel_huc_fw.c
>> b/drivers/gpu/drm/i915/intel_huc_fw.c
>> new file mode 100644
>> index 0000000..d83a63d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_huc_fw.c
>> @@ -0,0 +1,184 @@
>> +/*
>> + * Copyright © 2016-2018 Intel Corporation
>
> I think there was agreement to use SPDX license identifiers only.
>
Ah. right. will update. Thanks
>> + *
>> + * 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 (including
>> the next
>> + * paragraph) 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 "intel_huc_fw.h"
>> +#include "i915_drv.h"
>> +
>> +/**
>> + * DOC: HuC Firmware
>> + *
>> + * Motivation:
>> + * GEN9 introduces a new dedicated firmware for usage in media HEVC
>> (High
>> + * Efficiency Video Coding) operations. Userspace can use the firmware
>> + * capabilities by adding HuC specific commands to batch buffers.
>> + *
>> + * Implementation:
>> + * The same firmware loader is used as the GuC. However, the actual
>> + * loading to HW is deferred until GEM initialization is done.
>> + *
>> + * Note that HuC firmware loading must be done before GuC loading.
>> + */
>> +
>> +#define BXT_HUC_FW_MAJOR 01
>> +#define BXT_HUC_FW_MINOR 07
>> +#define BXT_BLD_NUM 1398
>> +
>> +#define SKL_HUC_FW_MAJOR 01
>> +#define SKL_HUC_FW_MINOR 07
>> +#define SKL_BLD_NUM 1398
>> +
>> +#define KBL_HUC_FW_MAJOR 02
>> +#define KBL_HUC_FW_MINOR 00
>> +#define KBL_BLD_NUM 1810
>> +
>> +#define HUC_FW_PATH(platform, major, minor, bld_num) \
>> + "i915/" __stringify(platform) "_huc_ver" __stringify(major) "_" \
>> + __stringify(minor) "_" __stringify(bld_num) ".bin"
>> +
>> +#define I915_SKL_HUC_UCODE HUC_FW_PATH(skl, SKL_HUC_FW_MAJOR, \
>> + SKL_HUC_FW_MINOR, SKL_BLD_NUM)
>> +MODULE_FIRMWARE(I915_SKL_HUC_UCODE);
>> +
>> +#define I915_BXT_HUC_UCODE HUC_FW_PATH(bxt, BXT_HUC_FW_MAJOR, \
>> + BXT_HUC_FW_MINOR, BXT_BLD_NUM)
>> +MODULE_FIRMWARE(I915_BXT_HUC_UCODE);
>> +
>> +#define I915_KBL_HUC_UCODE HUC_FW_PATH(kbl, KBL_HUC_FW_MAJOR, \
>> + KBL_HUC_FW_MINOR, KBL_BLD_NUM)
>> +MODULE_FIRMWARE(I915_KBL_HUC_UCODE);
>> +
>> +static void huc_fw_select(struct intel_uc_fw *huc_fw)
>> +{
>> + struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
>> + struct drm_i915_private *dev_priv = huc_to_i915(huc);
>> +
>> + GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
>> +
>> + if (!HAS_HUC(dev_priv))
>> + return;
>> +
>> + if (i915_modparams.huc_firmware_path) {
>> + huc_fw->path = i915_modparams.huc_firmware_path;
>> + huc_fw->major_ver_wanted = 0;
>> + huc_fw->minor_ver_wanted = 0;
>> + } else if (IS_SKYLAKE(dev_priv)) {
>> + huc_fw->path = I915_SKL_HUC_UCODE;
>> + huc_fw->major_ver_wanted = SKL_HUC_FW_MAJOR;
>> + huc_fw->minor_ver_wanted = SKL_HUC_FW_MINOR;
>> + } else if (IS_BROXTON(dev_priv)) {
>> + huc_fw->path = I915_BXT_HUC_UCODE;
>> + huc_fw->major_ver_wanted = BXT_HUC_FW_MAJOR;
>> + huc_fw->minor_ver_wanted = BXT_HUC_FW_MINOR;
>> + } else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) {
>> + huc_fw->path = I915_KBL_HUC_UCODE;
>> + huc_fw->major_ver_wanted = KBL_HUC_FW_MAJOR;
>> + huc_fw->minor_ver_wanted = KBL_HUC_FW_MINOR;
>> + } else {
>> + DRM_WARN("%s: No firmware known for this platform!\n",
>> + intel_uc_fw_type_repr(huc_fw->type));
>> + }
>> +}
>> +
>> +/**
>> + * intel_huc_fw_init_early() - initializes HuC firmware struct
>> + * @huc: intel_huc struct
>> + *
>> + * On platforms with HuC selects firmware for uploading
>> + */
>> +void intel_huc_fw_init_early(struct intel_huc *huc)
>> +{
>> + struct intel_uc_fw *huc_fw = &huc->fw;
>> +
>> + intel_uc_fw_init(huc_fw, INTEL_UC_FW_TYPE_HUC);
>> + huc_fw_select(huc_fw);
>> +}
>> +
>> +/**
>> + * huc_fw_xfer() - DMA's the firmware
>> + * @huc_fw: the firmware descriptor
>> + * @vma: the firmware image (bound into the GGTT)
>> + *
>> + * Transfer the firmware image to RAM for execution by the
>> microcontroller.
>> + *
>> + * Return: 0 on success, non-zero on failure
>> + */
>> +static int huc_fw_xfer(struct intel_uc_fw *huc_fw, struct i915_vma
>> *vma)
>> +{
>> + struct intel_huc *huc = container_of(huc_fw, struct intel_huc, fw);
>> + struct drm_i915_private *dev_priv = huc_to_i915(huc);
>> + unsigned long offset = 0;
>> + u32 size;
>> + int ret;
>> +
>> + GEM_BUG_ON(huc_fw->type != INTEL_UC_FW_TYPE_HUC);
>> +
>> + intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>> +
>> + /* Set the source address for the uCode */
>> + offset = guc_ggtt_offset(vma) + huc_fw->header_offset;
>> + I915_WRITE(DMA_ADDR_0_LOW, lower_32_bits(offset));
>> + I915_WRITE(DMA_ADDR_0_HIGH, upper_32_bits(offset) & 0xFFFF);
>> +
>> + /* Hardware doesn't look at destination address for HuC. Set it
>> to 0,
>> + * but still program the correct address space.
>> + */
>> + I915_WRITE(DMA_ADDR_1_LOW, 0);
>> + I915_WRITE(DMA_ADDR_1_HIGH, DMA_ADDRESS_SPACE_WOPCM);
>> +
>> + size = huc_fw->header_size + huc_fw->ucode_size;
>> + I915_WRITE(DMA_COPY_SIZE, size);
>> +
>> + /* Start the DMA */
>> + I915_WRITE(DMA_CTRL, _MASKED_BIT_ENABLE(HUC_UKERNEL | START_DMA));
>> +
>> + /* Wait for DMA to finish */
>> + ret = intel_wait_for_register_fw(dev_priv, DMA_CTRL, START_DMA,
>> 0, 100);
>> +
>> + DRM_DEBUG_DRIVER("HuC DMA transfer wait over with ret %d\n", ret);
>> +
>> + /* Disable the bits once DMA is over */
>> + I915_WRITE(DMA_CTRL, _MASKED_BIT_DISABLE(HUC_UKERNEL));
>> +
>> + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * intel_huc_fw_upload() - load HuC uCode to device
>> + * @huc: intel_huc structure
>> + *
>> + * Called from intel_uc_init_hw() during driver load, resume from
>> sleep and
>> + * after a GPU reset. Note that HuC must be loaded before GuC.
>> + *
>> + * The firmware image should have already been fetched into memory
>> by the
>> + * earlier call to intel_uc_init_fw(), so here we need to only check
>> that
>> + * fetch succeeded, and then transfer the image to the h/w.
>> + *
>> + * Return: non-zero code on error
>> + */
>> +int intel_huc_fw_upload(struct intel_huc *huc)
>> +{
>> + return intel_uc_fw_upload(&huc->fw, huc_fw_xfer);
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_huc_fw.h
>> b/drivers/gpu/drm/i915/intel_huc_fw.h
>> new file mode 100644
>> index 0000000..ab0a0b9
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_huc_fw.h
>> @@ -0,0 +1,33 @@
>> +/*
>> + * Copyright © 2014-2018 Intel Corporation
>
> use SPDX
>
>> + *
>> + * 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 (including
>> the next
>> + * paragraph) 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.
>> + *
>> + */
>> +
>> +#ifndef _INTEL_HUC_FW_H_
>> +#define _INTEL_HUC_FW_H_
>> +
>> +struct intel_huc;
>> +
>> +void intel_huc_fw_init_early(struct intel_huc *huc);
>> +int intel_huc_fw_upload(struct intel_huc *huc);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 9f1bac6..8e25474 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -361,7 +361,7 @@ int intel_uc_init_hw(struct drm_i915_private
>> *dev_priv)
>> goto err_out;
>> if (USES_HUC(dev_priv)) {
>> - ret = intel_huc_init_hw(huc);
>> + ret = intel_huc_fw_upload(huc);
>> if (ret)
>> goto err_out;
>> }
>
> LGTM, and with SPDX fixes,
>
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
Thanks for the review
--
Thanks,
Sagar
More information about the Intel-gfx
mailing list