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

[Daniele Ceraolo Spurio]

On 9/26/19 12:37 AM, Michal Wajdeczko wrote:
> 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>
> 

Thanks for the double check and the review, pushed.

Daniele

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