Re: [PATCH 1/2] drm/i915/uc: Update MAKE_HUC_FW_PATH macro

From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
To: "Srivatsa, Anusha" <anusha.srivatsa@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm/i915/uc: Update MAKE_HUC_FW_PATH macro
Date: Tue, 10 Sep 2019 14:37:13 -0700	[thread overview]
Message-ID: <dfe067cf-9b8a-f41d-70ca-4fc874669622@intel.com> (raw)
In-Reply-To: <83F5C7385F545743AD4FB2A62F75B073481590AC@ORSMSX108.amr.corp.intel.com>


On 9/10/19 2:35 PM, Srivatsa, Anusha wrote:
> 
> 
>> -----Original Message-----
>> From: Ceraolo Spurio, Daniele
>> Sent: Tuesday, September 10, 2019 2:05 PM
>> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Subject: Re: [PATCH 1/2] drm/i915/uc: Update MAKE_HUC_FW_PATH macro
>>
>>
>>
>> On 9/9/19 12:28 PM, Anusha Srivatsa wrote:
>>> Update MAKE_HUC_FW_PATH macro to follow the same convention as the
>>> MAKE_GUC_FW_PATH with the separator changing from "_" to "."
>>> and removing "ver".
>>>
>>> The current convention being:
>>> <platform>_<g/h>uc_<major>.<minor>.patch.bin
>>>
>>> Update the versions of huc being loaded of the platforms.
>>>
>>> SKL - v2.0.0
>>> BXT - v2.0.0
>>> KBL - v4.0.0
>>> GLK - v4.0.0
>>> CFL - KBL v4.0.0
>>> ICL - v9.0.0
>>> CML - v4.0.0
>>>
>>> v2: Remove the separator parameter altogether from
>>> __MAKE_UC_FW_PATH.(Daniele)
>>> - Squash all firmware update patches (Daniele)
>>>
>>> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 25 ++++++++++++------------
>>>    1 file changed, 13 insertions(+), 12 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 296a82603be0..da4bf24368bd 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> @@ -40,25 +40,26 @@ void intel_uc_fw_change_status(struct intel_uc_fw
>> *uc_fw,
>>>     */
>>>    #define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
>>>    	fw_def(ELKHARTLAKE, 0, guc_def(ehl, 33, 0, 4), huc_def(ehl,  9,  0,    0))
>> \
>>> -	fw_def(ICELAKE,     0, guc_def(icl, 33, 0, 0), huc_def(icl,  8,  4, 3238)) \
>>> -	fw_def(COFFEELAKE,  0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00,
>> 1810)) \
>>> -	fw_def(GEMINILAKE,  0, guc_def(glk, 33, 0, 0), huc_def(glk, 03, 01,
>> 2893)) \
>>> -	fw_def(KABYLAKE,    0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 02, 00, 1810))
>> \
>>> -	fw_def(BROXTON,     0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 01,  8,
>> 2893)) \
>>> -	fw_def(SKYLAKE,     0, guc_def(skl, 33, 0, 0), huc_def(skl, 01, 07, 1398))
>>> -
>>> -#define __MAKE_UC_FW_PATH(prefix_, name_, separator_, major_, minor_,
>>> patch_) \
>>> +	fw_def(COFFEELAKE,  0, guc_def(cml, 33, 0, 0), huc_def(cml,  4,  0,    0))
>> \
>>> +	fw_def(ICELAKE,     0, guc_def(icl, 33, 0, 0), huc_def(icl,  9,  0,    0)) \
>>> +	fw_def(COFFEELAKE,  0, guc_def(kbl, 33, 0, 0), huc_def(kbl,  4,  0,    0)) \
>>> +	fw_def(GEMINILAKE,  0, guc_def(glk, 33, 0, 0), huc_def(glk,  4,  0,    0)) \
>>> +	fw_def(KABYLAKE,    0, guc_def(kbl, 33, 0, 0), huc_def(kbl,  4,  0,    0)) \
>>> +	fw_def(BROXTON,     0, guc_def(bxt, 33, 0, 0), huc_def(bxt,  2,  0,    0)) \
>>> +	fw_def(SKYLAKE,     0, guc_def(skl, 33, 0, 0), huc_def(skl,  2,  0,    0))
>>
>> In addition to what Michal pointed out, while we're at it you can also get rid of
>> all the extra spaces in the huc_def() column since we've now standardized the
>> length of the items like we did with GuC. Maybe just leave an extra space for the
>> major, since we're close to hitting v10?
> 
> Regarding that, do we still want 4 spaces for HuC build number?
> And 2 spaces for minor?

No, just leave 1 space for minor and build num (like for guc_def) and 2 
for the major so we don't have to move everything when we get HuC v10

Daniele

> 
> Anusha
>> Daniele
>>
>>> +
>>> +#define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \
>>>    	"i915/" \
>>>    	__stringify(prefix_) name_ \
>>> -	__stringify(major_) separator_ \
>>> -	__stringify(minor_) separator_ \
>>> +	__stringify(major_) "." \
>>> +	__stringify(minor_) "." \
>>>    	__stringify(patch_) ".bin"
>>>
>>>    #define MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_) \
>>> -	__MAKE_UC_FW_PATH(prefix_, "_guc_", ".", major_, minor_, patch_)
>>> +	__MAKE_UC_FW_PATH(prefix_, "_guc_", major_, minor_, patch_)
>>>
>>>    #define MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_) \
>>> -	__MAKE_UC_FW_PATH(prefix_, "_huc_ver", "_", major_, minor_,
>> bld_num_)
>>> +	__MAKE_UC_FW_PATH(prefix_, "_huc_", major_, minor_, bld_num_)
>>>
>>>    /* All blobs need to be declared via MODULE_FIRMWARE() */
>>>    #define INTEL_UC_MODULE_FW(platform_, revid_, guc_, huc_) \
>>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx