[Intel-gfx] [PATCH] drm/i915/huc: fix version parsing from CSS header

[Michal Wajdeczko]

On Thu, 26 Sep 2019 01:03:20 +0200, Summers, Stuart  
<stuart.summers at intel.com> wrote:

> On Wed, 2019-09-25 at 15:21 -0700, Daniele Ceraolo Spurio wrote:
>> The HuC FW has silently switched to encoding the version the same way
>> as
>> the GuC FW does, i.e. major.minor.patch instead of just major.minor.
>> All
>> the current blobs follow the new scheme, but since minor and patch
>> are
>> both zero there is no difference in the end results and we happily
>> load
>> them. New binaries, however, will have non-zero values in there, so
>> we
>> need to make sure to parse them correctly.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <
>> daniele.ceraolospurio at intel.com>
>
> I don't have insight into the HuC change, so just taking your word
> here. The code below looks sane and is an obvious improvement.
>
> It might be interesting to get a look from someone a little closer to
> this for a HuC perspective. With that disclaimer:
> Reviewed-by: Stuart Summers <stuart.summers at intel.com>

Double checked offline with HuC team, so

Acked-by: Michal Wajdeczko <michal.wajdeczko at intel.com>

>
>> Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c     | 23 ++++------------
>> ----
>>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h |  8 +++----
>>  2 files changed, 7 insertions(+), 24 deletions(-)
>>
>> 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 ea9a807abd4f..bb878119f06c 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> @@ -339,25 +339,10 @@ int intel_uc_fw_fetch(struct intel_uc_fw
>> *uc_fw, struct drm_i915_private *i915)
>>  	}
>>
>>  	/* Get version numbers from the CSS header */
>> -	switch (uc_fw->type) {
>> -	case INTEL_UC_FW_TYPE_GUC:
>> -		uc_fw->major_ver_found =
>> FIELD_GET(CSS_SW_VERSION_GUC_MAJOR,
>> -						   css->sw_version);
>> -		uc_fw->minor_ver_found =
>> FIELD_GET(CSS_SW_VERSION_GUC_MINOR,
>> -						   css->sw_version);
>> -		break;
>> -
>> -	case INTEL_UC_FW_TYPE_HUC:
>> -		uc_fw->major_ver_found =
>> FIELD_GET(CSS_SW_VERSION_HUC_MAJOR,
>> -						   css->sw_version);
>> -		uc_fw->minor_ver_found =
>> FIELD_GET(CSS_SW_VERSION_HUC_MINOR,
>> -						   css->sw_version);
>> -		break;
>> -
>> -	default:
>> -		MISSING_CASE(uc_fw->type);
>> -		break;
>> -	}
>> +	uc_fw->major_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MAJOR,
>> +					   css->sw_version);
>> +	uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR,
>> +					   css->sw_version);
>>
>>  	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
>>  	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
>> 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
>> index ae58e8a8c53b..f8f6c91a0df6 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>> @@ -69,11 +69,9 @@ struct uc_css_header {
>>  	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)
>> +#define CSS_SW_VERSION_UC_MAJOR		(0xFF << 16)
>> +#define CSS_SW_VERSION_UC_MINOR		(0xFF << 8)
>> +#define CSS_SW_VERSION_UC_PATCH		(0xFF << 0)
>>  	u32 reserved[14];
>>  	u32 header_info;
>>  } __packed;