Re: [PATCH v9 21/26] drm/xe/xe_reg_sr: Update handling of xe_force_wake_get return

["Nilawar, Badal" <badal.nilawar@intel.com>]

On 14-10-2024 13:25, Himal Prasad Ghimiray wrote:
> With xe_force_wake_get() now returning the refcount-incremented
> domain mask, a non-zero return value in the case of XE_FORCEWAKE_ALL does
> not necessarily indicate success. Use xe_force_wake_ref_has_domain()
> to determine the status of the call.
> 
> Modify the return handling of xe_force_wake_get() accordingly and
> pass the return value to xe_force_wake_put().
> 
> v3
> - return xe_wakeref_t instead of int in xe_force_wake_get()
> - xe_force_wake_put() error doesn't need to be checked. It internally
> WARNS on domain ack failure.
> 
> v5
> - return unsigned int from xe_force_wake_get()
> 
> v6
> - use helper xe_force_wake_ref_has_domain()
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_reg_sr.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c
> index 191cb4121acd..e1a0e27cda14 100644
> --- a/drivers/gpu/drm/xe/xe_reg_sr.c
> +++ b/drivers/gpu/drm/xe/xe_reg_sr.c
> @@ -188,27 +188,27 @@ void xe_reg_sr_apply_mmio(struct xe_reg_sr *sr, struct xe_gt *gt)
>   {
>   	struct xe_reg_sr_entry *entry;
>   	unsigned long reg;
> -	int err;
> +	unsigned int fw_ref;
>   
>   	if (xa_empty(&sr->xa))
>   		return;
>   
>   	xe_gt_dbg(gt, "Applying %s save-restore MMIOs\n", sr->name);
>   
> -	err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> -	if (err)
> +	fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> +	if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL))
>   		goto err_force_wake;
>   
>   	xa_for_each(&sr->xa, reg, entry)
>   		apply_one_mmio(gt, entry);
>   
> -	err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> -	XE_WARN_ON(err);
> +	xe_force_wake_put(gt_to_fw(gt), fw_ref);
>   
>   	return;
>   
>   err_force_wake:
> -	xe_gt_err(gt, "Failed to apply, err=%d\n", err);
> +	xe_force_wake_put(gt_to_fw(gt), fw_ref);
> +	xe_gt_err(gt, "Failed to apply, err=-ETIMEDOUT\n");
>   }
>   
>   void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe)
> @@ -221,15 +221,15 @@ void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe)
>   	u32 mmio_base = hwe->mmio_base;
>   	unsigned long reg;
>   	unsigned int slot = 0;
> -	int err;
> +	unsigned int fw_ref;
>   
>   	if (xa_empty(&sr->xa))
>   		return;
>   
>   	drm_dbg(&xe->drm, "Whitelisting %s registers\n", sr->name);
>   
> -	err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> -	if (err)
> +	fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> +	if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL))
>   		goto err_force_wake;
>   
>   	p = drm_dbg_printer(&xe->drm, DRM_UT_DRIVER, NULL);
> @@ -254,13 +254,13 @@ void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe)
>   		xe_mmio_write32(&gt->mmio, RING_FORCE_TO_NONPRIV(mmio_base, slot), addr);
>   	}
>   
> -	err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> -	XE_WARN_ON(err);
> +	xe_force_wake_put(gt_to_fw(gt), fw_ref);
>   
>   	return;
>   
>   err_force_wake:
> -	drm_err(&xe->drm, "Failed to apply, err=%d\n", err);
> +	xe_force_wake_put(gt_to_fw(gt), fw_ref);
> +	drm_err(&xe->drm, "Failed to apply, err=-ETIMEDOUT\n");

Reviewed-by: Badal Nilawar <badal.nilawar@intel.com>

Regards,
Badal
>   }
>   
>   /**