Re: [PATCH 1/7] drm/xe: Fix xe_pm_runtime_get_if_active return

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Fri, 2024-05-03 at 15:13 -0400, Rodrigo Vivi wrote:
> Current callers of this function are already taking the result
> to a boolean and using in an if. It might be a problem because
> current function might return negative error codes on failure,
> without increasing the reference counter.
> 
> In this scenario we could end up with extra 'put' call ending
> in unbalanced scenarios.
> 
> Let's fix it, while aligning with the current xe_pm_get_if_in_use
> style.
> 
> Cc: Francois Dugast <francois.dugast@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pm.c | 8 ++++----
>  drivers/gpu/drm/xe/xe_pm.h | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index c1831106ea4b..eca977d1e1cb 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -505,12 +505,12 @@ int xe_pm_runtime_get_ioctl(struct xe_device
> *xe)
>   * xe_pm_runtime_get_if_active - Get a runtime_pm reference if
> device active
>   * @xe: xe device instance
>   *
> - * Returns: Any number greater than or equal to 0 for success,
> negative error
> - * code otherwise.
> + * Returns: True if device is awake (regardless the previous number
> of references)
Nit: Athough both "Returns:" and "Return:" work, the latter seems to be
the preferred.

Otherwise LGTM.
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>



> + * and a new reference was taken, false otherwise.
>   */
> -int xe_pm_runtime_get_if_active(struct xe_device *xe)
> +bool xe_pm_runtime_get_if_active(struct xe_device *xe)
>  {
> -	return pm_runtime_get_if_active(xe->drm.dev);
> +	return pm_runtime_get_if_active(xe->drm.dev) > 0;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
> index 18b0613fe57b..f694005db278 100644
> --- a/drivers/gpu/drm/xe/xe_pm.h
> +++ b/drivers/gpu/drm/xe/xe_pm.h
> @@ -29,7 +29,7 @@ int xe_pm_runtime_resume(struct xe_device *xe);
>  void xe_pm_runtime_get(struct xe_device *xe);
>  int xe_pm_runtime_get_ioctl(struct xe_device *xe);
>  void xe_pm_runtime_put(struct xe_device *xe);
> -int xe_pm_runtime_get_if_active(struct xe_device *xe);
> +bool xe_pm_runtime_get_if_active(struct xe_device *xe);
>  bool xe_pm_runtime_get_if_in_use(struct xe_device *xe);
>  void xe_pm_runtime_get_noresume(struct xe_device *xe);
>  bool xe_pm_runtime_resume_and_get(struct xe_device *xe);