Re: [PATCH 3/4] drm/xe: Combine common force-wake code into helpers

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

On Mon, Jun 10, 2024 at 08:37:56PM +0200, Michal Wajdeczko wrote:
> The code of 'control' and 'wait' force-wake operations are very
> similar for both 'wake' and 'sleep' cases. Add helpers to maximize
> code reuse.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_force_wake.c | 55 ++++++++++++++++--------------
>  1 file changed, 30 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c
> index a1d97cce3d56..b0baa9094418 100644
> --- a/drivers/gpu/drm/xe/xe_force_wake.c
> +++ b/drivers/gpu/drm/xe/xe_force_wake.c
> @@ -20,6 +20,11 @@ static struct xe_gt *fw_to_gt(struct xe_force_wake *fw)
>  	return fw->gt;
>  }
>  
> +static inline const char *str_wake_sleep(bool v)
> +{
> +	return v ? "wake" : "sleep";

probably better with s/v/wake ?!

but up to you:

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


> +}
> +
>  static void domain_init(struct xe_force_wake_domain *domain,
>  			enum xe_force_wake_domain_id id,
>  			struct xe_reg reg, struct xe_reg ack)
> @@ -94,47 +99,47 @@ void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw)
>  			    FORCEWAKE_ACK_GSC);
>  }
>  
> +static void __domain_ctl(struct xe_gt *gt, struct xe_force_wake_domain *domain, bool wake)
> +{
> +	xe_mmio_write32(gt, domain->reg_ctl, domain->mask | (wake ? domain->val : 0));
> +}
> +
> +static int __domain_wait(struct xe_gt *gt, struct xe_force_wake_domain *domain, bool wake)
> +{
> +	u32 value;
> +	int ret;
> +
> +	ret = xe_mmio_wait32(gt, domain->reg_ack, domain->val, wake ? domain->val : 0,
> +			     XE_FORCE_WAKE_ACK_TIMEOUT_MS * USEC_PER_MSEC,
> +			     &value, true);
> +	if (ret)
> +		xe_gt_notice(gt, "Force wake domain %d failed to ack %s (%pe) reg[%#x] = %#x\n",
> +			     domain->id, str_wake_sleep(wake), ERR_PTR(ret),
> +			     domain->reg_ack.addr, value);
> +
> +	return ret;
> +}
> +
>  static void domain_wake(struct xe_gt *gt, struct xe_force_wake_domain *domain)
>  {
> -	xe_mmio_write32(gt, domain->reg_ctl, domain->mask | domain->val);
> +	__domain_ctl(gt, domain, true);
>  }
>  
>  static int domain_wake_wait(struct xe_gt *gt,
>  			    struct xe_force_wake_domain *domain)
>  {
> -	u32 value;
> -	int ret;
> -
> -	ret = xe_mmio_wait32(gt, domain->reg_ack, domain->val, domain->val,
> -			     XE_FORCE_WAKE_ACK_TIMEOUT_MS * USEC_PER_MSEC,
> -			     &value, true);
> -	if (ret)
> -		xe_gt_notice(gt, "Force wake domain %d failed to ack wake (%pe) reg[%#x] = %#x\n",
> -			     domain->id, ERR_PTR(ret), domain->reg_ack.addr, value);
> -
> -	return ret;
> +	return __domain_wait(gt, domain, true);
>  }
>  
>  static void domain_sleep(struct xe_gt *gt, struct xe_force_wake_domain *domain)
>  {
> -	xe_mmio_write32(gt, domain->reg_ctl, domain->mask);
> +	__domain_ctl(gt, domain, false);
>  }
>  
>  static int domain_sleep_wait(struct xe_gt *gt,
>  			     struct xe_force_wake_domain *domain)
>  {
> -	u32 value;
> -	int ret;
> -
> -	ret = xe_mmio_wait32(gt, domain->reg_ack, domain->val, 0,
> -			     XE_FORCE_WAKE_ACK_TIMEOUT_MS * USEC_PER_MSEC,
> -			     &value, true);
> -
> -	if (ret)
> -		xe_gt_notice(gt, "Force wake domain %d failed to ack sleep (%pe) reg[%#x] = %#x\n",
> -			     domain->id, ERR_PTR(ret), domain->reg_ack.addr, value);
> -
> -	return ret;
> +	return __domain_wait(gt, domain, false);
>  }
>  
>  #define for_each_fw_domain_masked(domain__, mask__, fw__, tmp__) \
> -- 
> 2.43.0
>