[Intel-gfx] [PATCH] drm/i915/guc: ADL-N should use the same GuC FW as ADL-S

[Ceraolo Spurio, Daniele]

On 6/21/2022 5:34 PM, Matt Roper wrote:
> On Tue, Jun 21, 2022 at 04:30:05PM -0700, Daniele Ceraolo Spurio wrote:
>> The only difference between the ADL S and P GuC FWs is the HWConfig
>> support. ADL-N does not support HWConfig, so we should use the same
>> binary as ADL-S, otherwise the GuC might attempt to fetch a config
>> table that does not exist. ADL-N is internally identified as an ADL-P,
>> so we need to special-case it in the FW selection code.
>>
>> Fixes: 7e28d0b26759 ("drm/i915/adl-n: Enable ADL-N platform")
>> Cc: John Harrison <John.C.Harrison at Intel.com>
>> Cc: Tejas Upadhyay <tejas.upadhyay at intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Would the config table still get used somehow even though we return
> false for ADL-N in has_table()?

On platforms that support the config table, the GuC does always try to 
fetch and decode the table during the firmware init stage, even if we 
don't query it later due to has_table() being false.

Daniele

>
> Even if it couldn't be used, this change makes the behavior more clear
> and explicit.
>
> Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> 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 d2c5c9367cc4..ef2d10184ee2 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> @@ -162,6 +162,15 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
>>   	u8 rev = INTEL_REVID(i915);
>>   	int i;
>>   
>> +	/*
>> +	 * The only difference between the ADL GuC FWs is the HWConfig support.
>> +	 * ADL-N does not support HWConfig, so we should use the same binary as
>> +	 * ADL-S, otherwise the GuC might attempt to fetch a config table that
>> +	 * does not exist.
>> +	 */
>> +	if (IS_ADLP_N(i915))
>> +		p = INTEL_ALDERLAKE_S;
>> +
>>   	GEM_BUG_ON(uc_fw->type >= ARRAY_SIZE(blobs_all));
>>   	fw_blobs = blobs_all[uc_fw->type].blobs;
>>   	fw_count = blobs_all[uc_fw->type].count;
>> -- 
>> 2.25.1
>>