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

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Jan 08, 2026 at 08:36:43AM -0800, Daniele Ceraolo Spurio wrote:
> 
> 
> 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.

thanks for checking

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

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