[PATCH 1/9] drm/xe: Introduce xe_pm_runtime_get_noresume for inner callers

[Matthew Auld]

On 10/04/2024 02:58, Rodrigo Vivi wrote:
> Let's ensure that we have an option for inner callers that will
> raise WARN if device is not active and not protected by outer callers.
> 
> Make this also a void function forcing every caller to unconditionally
> put the reference back afterwards.
> 
> This will be very important for cases where we want to hold the
> reference before scheduling a work in a queue. Then the work job
> will be responsible for putting it back.
> 
> While at this, already convert a case from mem_access_get_ongoing where
> it is not checking for the reference and put it back, what would
> cause the underflow.
> 
> v2: Fix identation.
> v3: Convert equivalent missing put from mem_access towards pm_runtime.
> 
> Reviewed-by: Matthew Auld <matthew.auld at intel.com> #v1
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

r-b still applies to v3.

> ---
>   drivers/gpu/drm/xe/xe_exec_queue.c |  4 ++--
>   drivers/gpu/drm/xe/xe_pm.c         | 20 ++++++++++++++++++++
>   drivers/gpu/drm/xe/xe_pm.h         |  1 +
>   3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index 71bd52dfebcf..50ec661116a2 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -128,7 +128,7 @@ static int __xe_exec_queue_init(struct xe_exec_queue *q)
>   	 * already grabbed the rpm ref outside any sensitive locks.
>   	 */
>   	if (!(q->flags & EXEC_QUEUE_FLAG_PERMANENT) && (q->flags & EXEC_QUEUE_FLAG_VM || !q->vm))
> -		drm_WARN_ON(&xe->drm, !xe_device_mem_access_get_if_ongoing(xe));
> +		xe_pm_runtime_get_noresume(xe);
>   
>   	return 0;
>   
> @@ -217,7 +217,7 @@ void xe_exec_queue_fini(struct xe_exec_queue *q)
>   	for (i = 0; i < q->width; ++i)
>   		xe_lrc_finish(q->lrc + i);
>   	if (!(q->flags & EXEC_QUEUE_FLAG_PERMANENT) && (q->flags & EXEC_QUEUE_FLAG_VM || !q->vm))
> -		xe_device_mem_access_put(gt_to_xe(q->gt));
> +		xe_pm_runtime_put(gt_to_xe(q->gt));
>   	__xe_exec_queue_free(q);
>   }
>   
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 9ef03f23a2d7..1692cf08e950 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -489,6 +489,26 @@ bool xe_pm_runtime_get_if_in_use(struct xe_device *xe)
>   	return pm_runtime_get_if_in_use(xe->drm.dev) > 0;
>   }
>   
> +/**
> + * xe_pm_runtime_get_noresume - Bump runtime PM usage counter without resuming
> + * @xe: xe device instance
> + *
> + * This function should be used in inner places where it is surely already
> + * protected by outer-bound callers of `xe_pm_runtime_get`.
> + * It will warn if not protected.
> + * The reference should be put back after this function regardless, since it
> + * will always bump the usage counter, regardless.
> + */
> +void xe_pm_runtime_get_noresume(struct xe_device *xe)
> +{
> +	bool ref;
> +
> +	ref = xe_pm_runtime_get_if_in_use(xe);
> +
> +	if (drm_WARN(&xe->drm, !ref, "Missing outer runtime PM protection\n"))
> +		pm_runtime_get_noresume(xe->drm.dev);
> +}
> +
>   /**
>    * xe_pm_runtime_resume_and_get - Resume, then get a runtime_pm ref if awake.
>    * @xe: xe device instance
> diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
> index 0cb38ca244fe..119b630ad1d1 100644
> --- a/drivers/gpu/drm/xe/xe_pm.h
> +++ b/drivers/gpu/drm/xe/xe_pm.h
> @@ -31,6 +31,7 @@ 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_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);
>   void xe_pm_assert_unbounded_bridge(struct xe_device *xe);
>   int xe_pm_set_vram_threshold(struct xe_device *xe, u32 threshold);