Re: [PATCH 04/33] drm/xe/forcewake: Create dedicated type for forcewake references

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 11/7/2025 7:13 PM, Matt Roper wrote:
> xe_force_wake_get() currently returns an integer mask of power domains
> that were successfully awoken; both this mask and a pointer to the force
> wake collection must be passed to xe_force_wake_put() to release the
> wake reference.
> 
> Create a dedicated structure type to hold both the mask and the
> collection pointer.  While this change does little on its own, it will
> make it easier for us to add scope-based cleanup of forcewake in the
> future.
> 
> FIXME:
>    For ease of review, this patch contains only the manual changes to
>    add the structure and change the get/put function definitions; it
>    does not build on its own since the rest of the driver is still
>    trying to call the get/put functions with the old signature.  The
>    next patch contains the coccinelle-generated changes necessary
>    elsewhere in the driver to adapt to the new interface.  The two
>    patches will be squashed together when applied, remain separate for
>    now to help reviewers.
> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---

...

> diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h
> index 0e3e84bfa51c..86e9bca7cac9 100644
> --- a/drivers/gpu/drm/xe/xe_force_wake.h
> +++ b/drivers/gpu/drm/xe/xe_force_wake.h
> @@ -15,9 +15,9 @@ void xe_force_wake_init_gt(struct xe_gt *gt,
>  			   struct xe_force_wake *fw);
>  void xe_force_wake_init_engines(struct xe_gt *gt,
>  				struct xe_force_wake *fw);
> -unsigned int __must_check xe_force_wake_get(struct xe_force_wake *fw,
> -					    enum xe_force_wake_domains domains);
> -void xe_force_wake_put(struct xe_force_wake *fw, unsigned int fw_ref);
> +struct xe_force_wake_ref __must_check xe_force_wake_get(struct xe_force_wake *fw,
> +							enum xe_force_wake_domains domains);
> +void xe_force_wake_put(struct xe_force_wake_ref fw_ref);

but is it really necessary to change signature of all existing xe_force_wake functions?

in my previous attempt [1] this new helper struct was just initialized inside the CLASS
so we can start use new approach and still use existing xe_fw API without any massive changes

+DEFINE_CLASS(xe_fw, struct xe_force_wake_guard,
+	     xe_force_wake_put(_T.fw, _T.ref),
+	     ({ (struct xe_force_wake_guard){ fw, xe_force_wake_get(fw, domains) }; }),
+	     struct xe_force_wake *fw, enum xe_force_wake_domains domains);

[1] https://patchwork.freedesktop.org/patch/625116/?series=141516&rev=1


>  
>  static inline int
>  xe_force_wake_ref(struct xe_force_wake *fw,
> @@ -56,9 +56,10 @@ xe_force_wake_assert_held(struct xe_force_wake *fw,
>   * Return: true if domain is refcounted.
>   */
>  static inline bool
> -xe_force_wake_ref_has_domain(unsigned int fw_ref, enum xe_force_wake_domains domain)
> +xe_force_wake_ref_has_domain(struct xe_force_wake_ref fw_ref,
> +			     enum xe_force_wake_domains domain)
>  {
> -	return fw_ref & domain;
> +	return fw_ref.domains & domain;
>  }
>  
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_force_wake_types.h b/drivers/gpu/drm/xe/xe_force_wake_types.h
> index 9cfa28faf7bc..26df4adba4c5 100644
> --- a/drivers/gpu/drm/xe/xe_force_wake_types.h
> +++ b/drivers/gpu/drm/xe/xe_force_wake_types.h
> @@ -107,4 +107,19 @@ struct xe_force_wake {
>  	struct xe_force_wake_domain domains[XE_FW_DOMAIN_ID_COUNT];
>  };
>  
> +/**
> + * struct xe_force_wake_ref - Xe force wake reference
> + *
> + * Represents a wakeref for a subset of the power domains belonging to an
> + * xe_force_wake collection.  Returned by xe_force_wake_get() and passed
> + * to xe_force_wake_put().
> + */
> +struct xe_force_wake_ref {
> +	/** @fw: back pointer to force wake collection */
> +	struct xe_force_wake *fw;
> +
> +	/** @domains: mask of individual domains held by this reference */
> +	unsigned int domains;
> +};
> +