[PATCH] drm/xe/pxp: Fail the load if PXP fails to initialize

[Lucas De Marchi]

On Mon, Feb 03, 2025 at 03:48:57PM -0800, Daniele Ceraolo Spurio wrote:
>The PXP implementation mimics the i915 approach of allowing the load
>to continue even if PXP init has failed. On Xe however we're taking an
>harder stance on boot error and only allowing the load to complete if
>everything is working, so update the code to fail if anything goes wrong
>during PXP init.
>
>While at it, update the return code in case of PXP not supported to be 0
>instead of EOPNOTSUPP, to follow the standard of functions called by
>xe_device_probe where every non-zero value means failure.
>
>Suggested-by: Lucas De Marchi <lucas.demarchi at intel.com>
>Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: John Harrison <John.C.Harrison at Intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi

>---
> drivers/gpu/drm/xe/xe_device.c |  4 ++--
> drivers/gpu/drm/xe/xe_pxp.c    | 20 ++++++++++++--------
> 2 files changed, 14 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
>index 90f6bcc03037..36d7ffb3b4d9 100644
>--- a/drivers/gpu/drm/xe/xe_device.c
>+++ b/drivers/gpu/drm/xe/xe_device.c
>@@ -864,8 +864,8 @@ int xe_device_probe(struct xe_device *xe)
>
> 	/* A PXP init failure is not fatal */
> 	err = xe_pxp_init(xe);
>-	if (err && err != -EOPNOTSUPP)
>-		drm_err(&xe->drm, "PXP initialization failed: %pe\n", ERR_PTR(err));
>+	if (err)
>+		goto err_fini_display;
>
> 	err = drm_dev_register(&xe->drm, 0);
> 	if (err)
>diff --git a/drivers/gpu/drm/xe/xe_pxp.c b/drivers/gpu/drm/xe/xe_pxp.c
>index 5e7a1688a771..3cd3f83e86b0 100644
>--- a/drivers/gpu/drm/xe/xe_pxp.c
>+++ b/drivers/gpu/drm/xe/xe_pxp.c
>@@ -372,8 +372,8 @@ static void pxp_fini(void *arg)
>  * are performed asynchronously as part of the GSC init. PXP can only be used
>  * after both this function and the async worker have completed.
>  *
>- * Returns -EOPNOTSUPP if PXP is not supported, 0 if PXP initialization is
>- * successful, other errno value if there is an error during the init.
>+ * Returns 0 if PXP is not supported or if PXP initialization is successful,
>+ * other errno value if there is an error during the init.
>  */
> int xe_pxp_init(struct xe_device *xe)
> {
>@@ -382,26 +382,28 @@ int xe_pxp_init(struct xe_device *xe)
> 	int err;
>
> 	if (!xe_pxp_is_supported(xe))
>-		return -EOPNOTSUPP;
>+		return 0;
>
> 	/* we only support PXP on single tile devices with a media GT */
> 	if (xe->info.tile_count > 1 || !gt)
>-		return -EOPNOTSUPP;
>+		return 0;
>
> 	/* The GSCCS is required for submissions to the GSC FW */
> 	if (!(gt->info.engine_mask & BIT(XE_HW_ENGINE_GSCCS0)))
>-		return -EOPNOTSUPP;
>+		return 0;
>
> 	/* PXP requires both GSC and HuC firmwares to be available */
> 	if (!xe_uc_fw_is_loadable(&gt->uc.gsc.fw) ||
> 	    !xe_uc_fw_is_loadable(&gt->uc.huc.fw)) {
> 		drm_info(&xe->drm, "skipping PXP init due to missing FW dependencies");
>-		return -EOPNOTSUPP;
>+		return 0;
> 	}
>
> 	pxp = drmm_kzalloc(&xe->drm, sizeof(struct xe_pxp), GFP_KERNEL);
>-	if (!pxp)
>-		return -ENOMEM;
>+	if (!pxp) {
>+		err = -ENOMEM;
>+		goto out;
>+	}
>
> 	INIT_LIST_HEAD(&pxp->queues.list);
> 	spin_lock_init(&pxp->queues.lock);
>@@ -448,6 +450,8 @@ int xe_pxp_init(struct xe_device *xe)
> 	destroy_workqueue(pxp->irq.wq);
> out_free:
> 	drmm_kfree(&xe->drm, pxp);
>+out:
>+	drm_err(&xe->drm, "PXP initialization failed: %pe\n", ERR_PTR(err));
> 	return err;
> }
>
>-- 
>2.43.0
>