[Intel-gfx] [PATCH 2/3] drm/i915/uc: Move uc firmware layout definitions to dedicated file

Michal Wajdeczko michal.wajdeczko at intel.com
Wed Jul 24 20:01:36 UTC 2019


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

>
> 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