[PATCH] drm/xe: Split MCR initialization

[Matt Roper]

On Wed, May 29, 2024 at 06:29:45PM +0200, Michal Wajdeczko wrote:
> We can't follow the same initialization order of GT topology, MCR,
> PAT and GuC HWconfig, as done today on native/PF driver, on the VF
> driver, since fuse registers used in GT topology discovery will be
> obtained by the VF driver from the GuC.
> 
> While we need to program the HW PAT table prior to loading the GuC,
> this requires only basic functionality of the MCR code, which could
> be done separately from the full MCR initialization that may need
> the GT topology.

I'd elaborate here that "basic functionality" is specifically "multicast
writes" and is something that can be performed without initializing the
steering tables, which are only needed for read and unicast write
operations.

In general I'm a little nervous about how hard it is to know during
various parts of initialization whether it's safe or not to do general
MCR access and it seems unfortunate that in VF's there's no way to get
the initialization done earlier.  Loading the GuC is a bunch of
complicated code and I guess we just have to trust that there will never
be a need to do any MCR reads, rmw's, or unicast writes during that part
of the initialization.  We'll also need to keep an eye on error paths
going forward as well (e.g., if GuC initialization fails, we don't want
to do anything that would read from MCR registers in the error handling
path where MCR never got fully initialized).

Anyway, this should work and I don't really see any more future-proof
ways of solving the chicken-and-egg problem, so

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>


Matt

> 
> Split MCR initialization into two steps to avoid introducing VF
> specific code paths. This also fixes duplicated spin_lock inits.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Zhanjun Dong <zhanjun.dong at intel.com>
> ---
> This is alternate solution than proposed in [1] that VF needs.
> 
> [1] https://patchwork.freedesktop.org/series/134157/
> ---
>  drivers/gpu/drm/xe/xe_gt.c     |  6 ++++--
>  drivers/gpu/drm/xe/xe_gt_mcr.c | 29 +++++++++++++++++++++++------
>  drivers/gpu/drm/xe/xe_gt_mcr.h |  1 +
>  3 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 34c1896807e9..7c87edc06ffc 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -502,8 +502,7 @@ int xe_gt_init_hwconfig(struct xe_gt *gt)
>  	if (err)
>  		goto out;
>  
> -	xe_gt_topology_init(gt);
> -	xe_gt_mcr_init(gt);
> +	xe_gt_mcr_init_early(gt);
>  	xe_pat_init(gt);
>  
>  	err = xe_uc_init(&gt->uc);
> @@ -514,6 +513,9 @@ int xe_gt_init_hwconfig(struct xe_gt *gt)
>  	if (err)
>  		goto out_fw;
>  
> +	xe_gt_topology_init(gt);
> +	xe_gt_mcr_init(gt);
> +
>  out_fw:
>  	xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
>  out:
> diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c
> index 577bd7043740..386ac3269909 100644
> --- a/drivers/gpu/drm/xe/xe_gt_mcr.c
> +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
> @@ -375,17 +375,34 @@ static const struct {
>  	[IMPLICIT_STEERING] = { "IMPLICIT", NULL },
>  };
>  
> -void xe_gt_mcr_init(struct xe_gt *gt)
> +/**
> + * xe_gt_mcr_init_early - Early initialization of the MCR support
> + * @gt: GT structure
> + *
> + * Perform early software only initialization of the MCR lock to allow
> + * the synchronization on accessing the STEER_SEMAPHORE register and
> + * use the xe_gt_mcr_multicast_write() function.
> + */
> +void xe_gt_mcr_init_early(struct xe_gt *gt)
>  {
> -	struct xe_device *xe = gt_to_xe(gt);
> -
>  	BUILD_BUG_ON(IMPLICIT_STEERING + 1 != NUM_STEERING_TYPES);
>  	BUILD_BUG_ON(ARRAY_SIZE(xe_steering_types) != NUM_STEERING_TYPES);
>  
> -	if (IS_SRIOV_VF(xe))
> -		return;
> -
>  	spin_lock_init(&gt->mcr_lock);
> +}
> +
> +/**
> + * xe_gt_mcr_init - Normal initialization of the MCR support
> + * @gt: GT structure
> + *
> + * Perform normal initialization of the MCR for all usages.
> + */
> +void xe_gt_mcr_init(struct xe_gt *gt)
> +{
> +	struct xe_device *xe = gt_to_xe(gt);
> +
> +	if (IS_SRIOV_VF(xe))
> +		return;
>  
>  	if (gt->info.type == XE_GT_TYPE_MEDIA) {
>  		drm_WARN_ON(&xe->drm, MEDIA_VER(xe) < 13);
> diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.h b/drivers/gpu/drm/xe/xe_gt_mcr.h
> index e7d03e001a49..8d119a0d5493 100644
> --- a/drivers/gpu/drm/xe/xe_gt_mcr.h
> +++ b/drivers/gpu/drm/xe/xe_gt_mcr.h
> @@ -12,6 +12,7 @@
>  struct drm_printer;
>  struct xe_gt;
>  
> +void xe_gt_mcr_init_early(struct xe_gt *gt);
>  void xe_gt_mcr_init(struct xe_gt *gt);
>  
>  void xe_gt_mcr_set_implicit_defaults(struct xe_gt *gt);
> -- 
> 2.43.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation