[Intel-gfx] [PATCH 2/3] drm/i915/uc: Move uc firmware layout definitions to dedicated file
Daniele Ceraolo Spurio
daniele.ceraolospurio at intel.com
Wed Jul 24 20:05:15 UTC 2019
On 7/24/19 1:01 PM, Michal Wajdeczko wrote:
> On Wed, 24 Jul 2019 19:50:37 +0200, Daniele Ceraolo Spurio
> <daniele.ceraolospurio at intel.com> wrote:
>
>>
>>
>> On 7/24/19 10:34 AM, Michal Wajdeczko wrote:
>>> Generic uc firmware layout definitions are unlikely to change and
>>> are separate to other GuC specific definitions.
>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>
>> Keeping things that apply to HuC as well in a generic file seems
>> sensible to me.
>>
>>> ---
>>> Documentation/gpu/i915.rst | 2 +-
>>> drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 70 -----------------
>>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 1 +
>>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 81 ++++++++++++++++++++
>>> 4 files changed, 83 insertions(+), 71 deletions(-)
>>> create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>>> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
>>> index c2173d120492..366cb7f46d17 100644
>>> --- a/Documentation/gpu/i915.rst
>>> +++ b/Documentation/gpu/i915.rst
>>> @@ -448,7 +448,7 @@ GuC-based command submission
>>> GuC Firmware Layout
>>> -------------------
>>> -.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>>> +.. kernel-doc:: drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>>> :doc: GuC Firmware Layout
>>
>> This is now generic uC firmware layout
>>
>>> GuC Address Space
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>>> index 30cca3a29323..06a9bdfb0faf 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>>> @@ -121,76 +121,6 @@
>>> #define GUC_CTL_MAX_DWORDS (SOFT_SCRATCH_COUNT - 2) /*
>>> [1..14] */
>>> -/**
>>> - * DOC: GuC Firmware Layout
>>> - *
>>> - * The GuC firmware layout looks like this:
>>> - *
>>> - * +-------------------------------+
>>> - * | uc_css_header |
>>> - * | |
>>> - * | contains major/minor version |
>>> - * +-------------------------------+
>>> - * | uCode |
>>> - * +-------------------------------+
>>> - * | RSA signature |
>>> - * +-------------------------------+
>>> - * | modulus key |
>>> - * +-------------------------------+
>>> - * | exponent val |
>>> - * +-------------------------------+
>>> - *
>>> - * The firmware may or may not have modulus key and exponent data.
>>> The header,
>>> - * uCode and RSA signature are must-have components that will be
>>> used by driver.
>>> - * Length of each components, which is all in dwords, can be found
>>> in header.
>>> - * In the case that modulus and exponent are not present in fw,
>>> a.k.a truncated
>>> - * image, the length value still appears in header.
>>> - *
>>> - * Driver will do some basic fw size validation based on the
>>> following rules:
>>> - *
>>> - * 1. Header, uCode and RSA are must-have components.
>>> - * 2. All firmware components, if they present, are in the sequence
>>> illustrated
>>> - * in the layout table above.
>>> - * 3. Length info of each component can be found in header, in dwords.
>>> - * 4. Modulus and exponent key are not required by driver. They may
>>> not appear
>>> - * in fw. So driver will load a truncated firmware in this case.
>>> - *
>>> - * HuC firmware layout is same as GuC firmware.
>>> - * Only HuC version information is saved in a different way.
>>> - */
>>> -
>>> -struct uc_css_header {
>>> - u32 module_type;
>>> - /* header_size includes all non-uCode bits, including
>>> css_header, rsa
>>> - * key, modulus key and exponent data. */
>>> - u32 header_size_dw;
>>> - u32 header_version;
>>> - u32 module_id;
>>> - u32 module_vendor;
>>> - u32 date;
>>> -#define CSS_DATE_DAY (0xFF << 0)
>>> -#define CSS_DATE_MONTH (0xFF << 8)
>>> -#define CSS_DATE_YEAR (0xFFFF << 16)
>>> - u32 size_dw; /* uCode plus header_size_dw */
>>> - u32 key_size_dw;
>>> - u32 modulus_size_dw;
>>> - u32 exponent_size_dw;
>>> - u32 time;
>>> -#define CSS_TIME_HOUR (0xFF << 0)
>>> -#define CSS_DATE_MIN (0xFF << 8)
>>> -#define CSS_DATE_SEC (0xFFFF << 16)
>>> - char username[8];
>>> - char buildnumber[12];
>>> - u32 sw_version;
>>> -#define CSS_SW_VERSION_GUC_MAJOR (0xFF << 16)
>>> -#define CSS_SW_VERSION_GUC_MINOR (0xFF << 8)
>>> -#define CSS_SW_VERSION_GUC_PATCH (0xFF << 0)
>>> -#define CSS_SW_VERSION_HUC_MAJOR (0xFFFF << 16)
>>> -#define CSS_SW_VERSION_HUC_MINOR (0xFFFF << 0)
>>> - u32 reserved[14];
>>> - u32 header_info;
>>> -} __packed;
>>> -
>>> /* Work item for submitting workloads into work queue of GuC. */
>>> struct guc_wq_item {
>>> u32 header;
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> index 8ce7210907c0..d5cb19b4e5c1 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> @@ -27,6 +27,7 @@
>>> #include <drm/drm_print.h>
>>> #include "intel_uc_fw.h"
>>> +#include "intel_uc_fw_abi.h"
>>> #include "i915_drv.h"
>>> /**
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>>> new file mode 100644
>>> index 000000000000..3ca535534151
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>>> @@ -0,0 +1,81 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2019 Intel Corporation
>>> + */
>>> +
>>> +#ifndef _INTEL_UC_FW_ABI_H
>>> +#define _INTEL_UC_FW_ABI_H
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +/**
>>> + * DOC: GuC Firmware Layout
>>> + *
>>> + * The GuC firmware layout looks like this:
>>
>> same here, s/GuC/uC.
>>
>>> + *
>>> + * +-------------------------------+
>>> + * | uc_css_header |
>>> + * | |
>>> + * | contains major/minor version |
>>> + * +-------------------------------+
>>> + * | uCode |
>>> + * +-------------------------------+
>>> + * | RSA signature |
>>> + * +-------------------------------+
>>> + * | modulus key |
>>> + * +-------------------------------+
>>> + * | exponent val |
>>> + * +-------------------------------+
>>> + *
>>> + * The firmware may or may not have modulus key and exponent data.
>>> The header,
>>> + * uCode and RSA signature are must-have components that will be
>>> used by driver.
>>> + * Length of each components, which is all in dwords, can be found
>>> in header.
>>> + * In the case that modulus and exponent are not present in fw,
>>> a.k.a truncated
>>> + * image, the length value still appears in header.
>>> + *
>>> + * Driver will do some basic fw size validation based on the
>>> following rules:
>>> + *
>>> + * 1. Header, uCode and RSA are must-have components.
>>> + * 2. All firmware components, if they present, are in the sequence
>>> illustrated
>>> + * in the layout table above.
>>> + * 3. Length info of each component can be found in header, in dwords.
>>> + * 4. Modulus and exponent key are not required by driver. They may
>>> not appear
>>> + * in fw. So driver will load a truncated firmware in this case.
>>> + *
>>> + * HuC firmware layout is same as GuC firmware.
>>> + * Only HuC version information is saved in a different way.
>>
>> And reword this as something like: "The only difference between GuC
>> and HuC firmwares is how the version information is saved"
>>
>> With that:
>
> As aim of this patch was just to move definition as-is from one place
> to the other, can I make all above fixups in other patch ?
>
> Note that changing "DOC" tag will require corresponding changes in .rst
> and I don't want to that in here
Having the changes in a new patch in the series works for me. I'd prefer
to have them before the move but not a blocker.
Daniele
>
>>
>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>>
>> Daniele
>>
>>> + */
>>> +
>>> +struct uc_css_header {
>>> + u32 module_type;
>>> + /* header_size includes all non-uCode bits, including
>>> css_header, rsa
>>> + * key, modulus key and exponent data. */
>>> + u32 header_size_dw;
>>> + u32 header_version;
>>> + u32 module_id;
>>> + u32 module_vendor;
>>> + u32 date;
>>> +#define CSS_DATE_DAY (0xFF << 0)
>>> +#define CSS_DATE_MONTH (0xFF << 8)
>>> +#define CSS_DATE_YEAR (0xFFFF << 16)
>>> + u32 size_dw; /* uCode plus header_size_dw */
>>> + u32 key_size_dw;
>>> + u32 modulus_size_dw;
>>> + u32 exponent_size_dw;
>>> + u32 time;
>>> +#define CSS_TIME_HOUR (0xFF << 0)
>>> +#define CSS_DATE_MIN (0xFF << 8)
>>> +#define CSS_DATE_SEC (0xFFFF << 16)
>>> + char username[8];
>>> + char buildnumber[12];
>>> + u32 sw_version;
>>> +#define CSS_SW_VERSION_GUC_MAJOR (0xFF << 16)
>>> +#define CSS_SW_VERSION_GUC_MINOR (0xFF << 8)
>>> +#define CSS_SW_VERSION_GUC_PATCH (0xFF << 0)
>>> +#define CSS_SW_VERSION_HUC_MAJOR (0xFFFF << 16)
>>> +#define CSS_SW_VERSION_HUC_MINOR (0xFFFF << 0)
>>> + u32 reserved[14];
>>> + u32 header_info;
>>> +} __packed;
>>> +
>>> +#endif /* _INTEL_UC_FW_ABI_H */
More information about the Intel-gfx
mailing list