Re: [PATCH 1/3] drm/xe: Fix display runtime_pm handling

[Matthew Auld <matthew.auld@intel.com>]

On 27/02/2024 18:37, Rodrigo Vivi wrote:
> i915's intel_runtime_pm_get_if_in_use actually calls the
> pm_runtime_get_if_active() with ign_usage_count = false, but Xe
> was erroneously calling it with true because of the mem_access cases.
> 
> This can lead to unnecessary references getting hold here and device
> never getting into the runtime suspended state.
> 
> Let's use directly the 'if_in_use' function provided by linux/pm_runtime.
> 
> Also, already start this new function protected from the runtime
> recursion, since runtime_pm will need to call for display functions
> for a proper D3Cold flow.
> 
> v2: Update commit message based on Matt's feedback.
>      Fix return condition of pm_runtime_get_if_in_use (Matt)
> 
> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Assuming this is applied on top of: drm/xe: Convert 
xe_pm_runtime_{get,put} to void and protect from recursion,

Reviewed-by: Matthew Auld <matthew.auld@intel.com>

> ---
>   .../gpu/drm/xe/compat-i915-headers/i915_drv.h   |  2 +-
>   drivers/gpu/drm/xe/xe_pm.c                      | 17 +++++++++++++++++
>   drivers/gpu/drm/xe/xe_pm.h                      |  1 +
>   3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
> index 420eba0e4be0..ad5864d1dd74 100644
> --- a/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
> +++ b/drivers/gpu/drm/xe/compat-i915-headers/i915_drv.h
> @@ -177,7 +177,7 @@ static inline intel_wakeref_t intel_runtime_pm_get_if_in_use(struct xe_runtime_p
>   {
>   	struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
>   
> -	return xe_pm_runtime_get_if_active(xe);
> +	return xe_pm_runtime_get_if_in_use(xe);
>   }
>   
>   static inline void intel_runtime_pm_put_unchecked(struct xe_runtime_pm *pm)
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index b5511e3c3153..7d7df78f3a2f 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -457,6 +457,23 @@ int xe_pm_runtime_get_if_active(struct xe_device *xe)
>   	return pm_runtime_get_if_active(xe->drm.dev, true);
>   }
>   
> +/**
> + * xe_pm_runtime_get_if_in_use - Get a runtime_pm reference and resume if needed
> + * @xe: xe device instance
> + *
> + * Returns: True if device is awake and the reference was taken, false otherwise.
> + */
> +bool xe_pm_runtime_get_if_in_use(struct xe_device *xe)
> +{
> +	if (xe_pm_read_callback_task(xe) == current) {
> +		/* The device is awake, grab the ref and move on */
> +		pm_runtime_get_noresume(xe->drm.dev);
> +		return true;
> +	}
> +
> +	return pm_runtime_get_if_in_use(xe->drm.dev) > 0;
> +}
> +
>   /**
>    * xe_pm_assert_unbounded_bridge - Disable PM on unbounded pcie parent bridge
>    * @xe: xe device instance
> diff --git a/drivers/gpu/drm/xe/xe_pm.h b/drivers/gpu/drm/xe/xe_pm.h
> index 7f5884babb29..bb4723a98405 100644
> --- a/drivers/gpu/drm/xe/xe_pm.h
> +++ b/drivers/gpu/drm/xe/xe_pm.h
> @@ -30,6 +30,7 @@ int xe_pm_runtime_get(struct xe_device *xe);
>   int xe_pm_runtime_get_ioctl(struct xe_device *xe);
>   int 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_assert_unbounded_bridge(struct xe_device *xe);
>   int xe_pm_set_vram_threshold(struct xe_device *xe, u32 threshold);
>   void xe_pm_d3cold_allowed_toggle(struct xe_device *xe);