Re: [PATCH 1/3] drm/xe/gsc: Make GSC FW load optional for newer platforms

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 1/8/2026 6:35 AM, Rodrigo Vivi wrote:
> On Wed, Jan 07, 2026 at 05:13:42PM -0800, Daniele Ceraolo Spurio wrote:
>> On newer platforms GSC FW is only required for content protection
>> features, so the core driver features work perfectly fine without it
>> (and we did in fact not enable it to start with on PTL). Therefore, we
>> can selectively enable the GSC only if the FW is found on disk, without
>> failing if it is not found.
>>
>> Note that this means that the FW can now be enabled (i.e., we're looking
>> for it) but not available (i.e., we haven't found it), so checks on FW
>> support should use the latter state to decide whether to go on or not.
>>
>> As part of the rework, the message for FW not found has been cleaned up
>> to be more readable.
>>
>> While at it, drop the comment about xe_uc_fw_init() since the code has
>> been reworked and the statement no longer applies.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Julia Filipchuk <julia.filipchuk@intel.com>
>> ---
>>   drivers/gpu/drm/xe/display/xe_hdcp_gsc.c |  2 +-
>>   drivers/gpu/drm/xe/xe_gsc.c              | 13 +++++++------
>>   drivers/gpu/drm/xe/xe_uc_fw.c            | 10 +++++++---
>>   3 files changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>> index 07acae121aa7..ed1f65f5ef4d 100644
>> --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>> +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>> @@ -39,7 +39,7 @@ static bool intel_hdcp_gsc_check_status(struct drm_device *drm)
>>   	struct xe_gt *gt = tile->media_gt;
>>   	struct xe_gsc *gsc = &gt->uc.gsc;
>>   
>> -	if (!gsc || !xe_uc_fw_is_enabled(&gsc->fw)) {
>> +	if (!gsc || !xe_uc_fw_is_available(&gsc->fw)) {
>>   		drm_dbg_kms(&xe->drm,
> should this be changed to a warn or info?

I'm not super familiar with the display side of things, but AFAIK this 
gets called when UMD checks the HDCP status, so IMO better to keep it to 
debug level, similar to what we do for messages in ioctl-related code.

>
>>   			    "GSC Components not ready for HDCP2.x\n");
>>   		return false;
>> diff --git a/drivers/gpu/drm/xe/xe_gsc.c b/drivers/gpu/drm/xe/xe_gsc.c
>> index a3157b0fe791..8ad27c884f68 100644
>> --- a/drivers/gpu/drm/xe/xe_gsc.c
>> +++ b/drivers/gpu/drm/xe/xe_gsc.c
>> @@ -414,15 +414,16 @@ int xe_gsc_init(struct xe_gsc *gsc)
>>   	}
>>   
>>   	/*
>> -	 * Some platforms can have GuC but not GSC. That would cause
>> -	 * xe_uc_fw_init(gsc) to return a "not supported" failure code and abort
>> -	 * all firmware loading. So check for GSC being enabled before
>> -	 * propagating the failure back up. That way the higher level will keep
>> -	 * going and load GuC as appropriate.
>> +	 * Starting from BMG the GSC is no longer needed for MC6 entry, so the
>> +	 * only missing features if the FW is lacking would be the content
>> +	 * protection ones. This is acceptable, si we allow the driver load to
>> +	 * continue if the GSC FW is missing.
>>   	 */
>>   	ret = xe_uc_fw_init(&gsc->fw);
>>   	if (!xe_uc_fw_is_enabled(&gsc->fw))
>>   		return 0;
>> +	else if (gt_to_xe(gt)->info.platform >= XE_BATTLEMAGE && !xe_uc_fw_is_available(&gsc->fw))
>> +		return 0;
>>   	else if (ret)
>>   		goto out;
>>   
>> @@ -614,7 +615,7 @@ void xe_gsc_print_info(struct xe_gsc *gsc, struct drm_printer *p)
>>   
>>   	drm_printf(p, "\tfound security version %u\n", gsc->security_version);
>>   
>> -	if (!xe_uc_fw_is_enabled(&gsc->fw))
>> +	if (!xe_uc_fw_is_available(&gsc->fw))
>>   		return;
>>   
>>   	CLASS(xe_force_wake, fw_ref)(gt_to_fw(gt), XE_FW_GSC);
>> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
>> index 50458d3bdc7d..b1a9ef3f6195 100644
>> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
>> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
>> @@ -741,7 +741,7 @@ static int uc_fw_request(struct xe_uc_fw *uc_fw, const struct firmware **firmwar
>>   		return 0;
>>   	}
>>   
>> -	err = request_firmware(&fw, uc_fw->path, dev);
>> +	err = firmware_request_nowarn(&fw, uc_fw->path, dev);
>>   	if (err)
>>   		goto fail;
>>   
>> @@ -770,8 +770,12 @@ static int uc_fw_request(struct xe_uc_fw *uc_fw, const struct firmware **firmwar
>>   			       XE_UC_FIRMWARE_MISSING :
>>   			       XE_UC_FIRMWARE_ERROR);
>>   
>> -	xe_gt_notice(gt, "%s firmware %s: fetch failed with error %pe\n",
>> -		     xe_uc_fw_type_repr(uc_fw->type), uc_fw->path, ERR_PTR(err));
>> +	if (err == -ENOENT)
>> +		xe_gt_info(gt, "%s firmware %s not found\n",
>> +			   xe_uc_fw_type_repr(uc_fw->type), uc_fw->path);
>> +	else
>> +		xe_gt_notice(gt, "%s firmware %s: fetch failed with error %pe\n",
> shouldn't this be an warn or error?

The lack of FW is now a valid case for GSC on PTL, so I think it is 
better to avoid throwing an error for it. Note that there are errors for 
both GuC and HuC being printed at other levels of the stack.

Daniele

>
>> +			     xe_uc_fw_type_repr(uc_fw->type), uc_fw->path, ERR_PTR(err));
>>   	xe_gt_info(gt, "%s firmware(s) can be downloaded from %s\n",
>>   		   xe_uc_fw_type_repr(uc_fw->type), XE_UC_FIRMWARE_URL);
>>   
>> -- 
>> 2.43.0
>>