Re: [PATCH v3 09/10] drm/xe/vsec: Support late bind fw information

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

On Mon, Aug 24, 2026 at 09:23:25AM -0700, Michael J. Ruhl wrote:
> CRI FW is loaded on power on.  Because of this, access to
> the FW cannot be done until it is running.
> 
> Update the XE PMT probe and access to check for late bind
> devices, verify, and wait for the appropriate FW state
> before probe or access.
> 
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_device.c       |   4 +-
>  drivers/gpu/drm/xe/xe_device_types.h |   4 +
>  drivers/gpu/drm/xe/xe_vsec.c         | 141 +++++++++++++++++++++++++--
>  drivers/gpu/drm/xe/xe_vsec.h         |   2 +-
>  4 files changed, 143 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 74d566693dfd..bf02f881095f 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -1140,7 +1140,9 @@ int xe_device_probe(struct xe_device *xe)
>  	for_each_gt(gt, xe, id)
>  		xe_gt_sanitize_freq(gt);
>  
> -	xe_vsec_init(xe);
> +	err = xe_vsec_init(xe);
> +	if (err)
> +		goto err_unregister_display;
>  
>  	err = xe_sriov_init_late(xe);
>  	if (err)
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 3f1a70813a99..5d9e6e66c665 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -468,6 +468,10 @@ struct xe_device {
>  		struct mutex lock;
>  		/** @pmt.base_offset: device specific base offset */
>  		u64 base_offset;
> +		/** @pmt.work: support late-bind probe */
> +		struct delayed_work work;
> +		/** @pmt.retry_count: late-bind probe retry */
> +		u32 retry_count;
>  	} pmt;
>  
>  	/** @soc_remapper: SoC remapper object */
> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index 578d59048b39..edc20c24137e 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -3,6 +3,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
>  #include <linux/cleanup.h>
> +#include <linux/delay.h>
>  #include <linux/errno.h>
>  #include <linux/intel_vsec.h>
>  #include <linux/module.h>
> @@ -17,6 +18,7 @@
>  #include "xe_mmio.h"
>  #include "xe_platform_types.h"
>  #include "xe_pm.h"
> +#include "xe_sysctrl.h"
>  #include "xe_vsec.h"
>  
>  #include "regs/xe_pmt.h"
> @@ -162,6 +164,14 @@ enum capability {
>  	WATCHER,
>  };
>  
> +/*
> + * Late bind will delay 100msec for up to 20 seconds
> + */
> +#define VSEC_LATE_BIND_DELAY_MSEC	(100)
> +#define VSEC_LATE_BIND_RETRY		(200)
> +
> +static void cri_late_bind_probe(struct xe_device *xe);
> +
>  static int bmg_guid_decode(u32 guid, int *index, u32 *offset)
>  {
>  	u32 record_id = FIELD_GET(GUID_RECORD_ID, guid);
> @@ -272,6 +282,56 @@ static int xe_guid_decode(u32 guid, int *index, u32 *offset)
>  	return -ENODEV;
>  }
>  
> +#define WAITING_FOR_SYCTLR
> +#ifdef WAITING_FOR_SYCTLR

I'm afraid you forgot to remove this before sending... or what's the goal of these?

> +static bool xe_is_oobmsm_fw_ready(struct xe_device *xe)
> +{
> +	return true;
> +}
> +#endif
> +
> +static void cri_late_bind_probe_work(struct work_struct *work)
> +{
> +	struct xe_device *xe = container_of(work, struct xe_device, pmt.work.work);
> +
> +	if (xe_is_oobmsm_fw_ready(xe)) {
> +		cri_late_bind_probe(xe);
> +		xe_pm_runtime_put(xe);
> +		return;
> +	}
> +
> +	xe->pmt.retry_count++;
> +
> +	/* wait up to 20 seconds */
> +	if (xe->pmt.retry_count == VSEC_LATE_BIND_RETRY) {
> +		drm_warn(&xe->drm, "PMT probe: Late Binding failed to complete\n");
> +		xe_pm_runtime_put(xe);
> +		return;
> +	}
> +
> +	if (!schedule_delayed_work(&xe->pmt.work, msecs_to_jiffies(VSEC_LATE_BIND_DELAY_MSEC)))
> +		xe_pm_runtime_put(xe);
> +}
> +
> +static bool wait_for_fw(struct xe_device *xe)
> +{
> +	int retries = VSEC_LATE_BIND_RETRY;  /* wait up to 20 secs */
> +
> +	if (xe->info.platform != XE_CRESCENTISLAND)
> +		return true;
> +
> +	while (retries--) {
> +		if (xe_is_oobmsm_fw_ready(xe))
> +			return true;
> +
> +		msleep(VSEC_LATE_BIND_DELAY_MSEC);
> +	}
> +
> +	drm_warn(&xe->drm, "Late Binding failed to complete\n");
> +
> +	return false;
> +}
> +
>  /*
>   * xe_pmt_telem_read is a callback API.  I.e this can be accessed external to
>   * XE driver (PMT driver scope).  Because of this, DRM hotplug needs to be
> @@ -318,6 +378,11 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  		goto dev_exit;
>  	}
>  
> +	if (!wait_for_fw(xe)) {
> +		ret = -ENODATA;
> +		goto runtime_exit;
> +	}
> +
>  	mutex_lock(&xe->pmt.lock);
>  
>  	/* set SoC re-mapper index register based on GUID memory region */
> @@ -327,6 +392,7 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  
>  	mutex_unlock(&xe->pmt.lock);
>  
> +runtime_exit:
>  	xe_pm_runtime_put(xe);
>  
>  dev_exit:
> @@ -374,6 +440,10 @@ static int xe_pmt_read_reg(struct device *dev, u32 guid, u32 *reg, u32 offset)
>  	disc_addr += CRI_DISCOVERY_OFFSET + inst + offset;
>  
>  	xe_pm_runtime_get(xe);
> +	if (!wait_for_fw(xe)) {
> +		ret = -ENODATA;
> +		goto runtime_exit;
> +	}
>  	mutex_lock(&xe->pmt.lock);
>  
>  	xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
> @@ -381,6 +451,8 @@ static int xe_pmt_read_reg(struct device *dev, u32 guid, u32 *reg, u32 offset)
>  	memcpy_fromio(reg, disc_addr, sizeof(*reg));
>  
>  	mutex_unlock(&xe->pmt.lock);
> +
> +runtime_exit:
>  	xe_pm_runtime_put(xe);
>  
>  dev_exit:
> @@ -416,6 +488,10 @@ static int xe_pmt_write_reg(struct device *dev, u32 guid, u32 reg, u32 offset)
>  	disc_addr += CRI_DISCOVERY_OFFSET + inst + offset;
>  
>  	xe_pm_runtime_get(xe);
> +	if (!wait_for_fw(xe)) {
> +		ret = -ENODATA;
> +		goto runtime_exit;
> +	}
>  	mutex_lock(&xe->pmt.lock);
>  
>  	xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
> @@ -423,6 +499,8 @@ static int xe_pmt_write_reg(struct device *dev, u32 guid, u32 reg, u32 offset)
>  	memcpy_toio(disc_addr, &reg, sizeof(reg));
>  
>  	mutex_unlock(&xe->pmt.lock);
> +
> +runtime_exit:
>  	xe_pm_runtime_put(xe);
>  
>  dev_exit:
> @@ -454,12 +532,44 @@ static enum xe_vsec get_platform_info(struct xe_device *xe)
>  	return vsec_platforms[xe->info.platform];
>  }
>  
> +static void cri_late_bind_probe(struct xe_device *xe)
> +{
> +	struct intel_vsec_platform_info *info;
> +	struct device *dev = xe->drm.dev;
> +	enum xe_vsec platform;
> +
> +	platform = get_platform_info(xe);
> +	if (platform != XE_VSEC_CRI)
> +		return;
> +
> +	info = &xe_vsec_info[platform];
> +	if (!info->headers)
> +		return;
> +
> +	info->priv_data = &xe_cri_pmt_cb;
> +	xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
> +
> +	intel_vsec_register(dev, info);
> +}
> +
> +static void vsec_disable_late_bind_work(void *arg)
> +{
> +	struct xe_device *xe = arg;
> +
> +	/*
> +	 * If was work was cancelled while it was still pending, we need to
> +	 * take care of releasing the runtime reference
> +	 */
> +	if (disable_delayed_work_sync(&xe->pmt.work))
> +		xe_pm_runtime_put(xe);
> +}
> +
>  /**
>   * xe_vsec_init - Initialize resources and add intel_vsec auxiliary
>   * interface
>   * @xe: valid xe instance
>   */
> -void xe_vsec_init(struct xe_device *xe)
> +int xe_vsec_init(struct xe_device *xe)
>  {
>  	struct intel_vsec_platform_info *info;
>  	struct device *dev = xe->drm.dev;
> @@ -467,30 +577,44 @@ void xe_vsec_init(struct xe_device *xe)
>  
>  	platform = get_platform_info(xe);
>  	if (platform == XE_VSEC_UNKNOWN)
> -		return;
> +		return 0;
>  
>  	info = &xe_vsec_info[platform];
>  	if (!info->headers)
> -		return;
> +		return 0;
>  
>  	switch (platform) {
>  	case XE_VSEC_BMG:
>  		if (!xe->soc_remapper.set_telem_region)
> -			return;
> +			return 0;
>  		xe->pmt.base_offset = BMG_TELEMETRY_OFFSET;
>  		info->priv_data = &xe_bmg_pmt_cb;
>  		break;
>  
>  	case XE_VSEC_CRI:
>  		if (!xe->soc_remapper.set_telem_region)
> -			return;
> +			return 0;
>  		xe->pmt.base_offset = CRI_TELEMETRY_OFFSET;
> +
> +		xe->pmt.retry_count = 0;
> +		INIT_DELAYED_WORK(&xe->pmt.work, cri_late_bind_probe_work);
> +
> +		xe_pm_runtime_get_noresume(xe);
> +		if (!xe_is_oobmsm_fw_ready(xe)) {
> +			schedule_delayed_work(&xe->pmt.work,
> +					      msecs_to_jiffies(VSEC_LATE_BIND_DELAY_MSEC));
> +			return devm_add_action_or_reset(xe->drm.dev,
> +							vsec_disable_late_bind_work,
> +							xe);
> +		}
> +
>  		info->priv_data = &xe_cri_pmt_cb;
>  		xe->soc_remapper.set_telem_region(xe, CRI_IDX_TELEM_DISCOVERY);
>  		break;
>  
>  	default:
> -		break;
> +		drm_err(&xe->drm, "Unsupported platform: %u\n", platform);
> +		return 0;
>  	}
>  
>  	/*
> @@ -498,5 +622,10 @@ void xe_vsec_init(struct xe_device *xe)
>  	 * resources.
>  	 */
>  	intel_vsec_register(dev, info);
> +
> +	if (platform == XE_VSEC_CRI)
> +		xe_pm_runtime_put(xe);
> +
> +	return 0;
>  }
>  MODULE_IMPORT_NS("INTEL_VSEC");
> diff --git a/drivers/gpu/drm/xe/xe_vsec.h b/drivers/gpu/drm/xe/xe_vsec.h
> index a25b4e6e681b..c4a1e2fc67d8 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.h
> +++ b/drivers/gpu/drm/xe/xe_vsec.h
> @@ -9,7 +9,7 @@
>  struct device;
>  struct xe_device;
>  
> -void xe_vsec_init(struct xe_device *xe);
> +int xe_vsec_init(struct xe_device *xe);
>  int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offset, u32 count);
>  
>  #endif
> -- 
> 2.43.0
>