[PATCH 10/31] drm/xe/display: Spin-off xe_display runtime/d3cold sequences

[Imre Deak]

On Tue, Sep 24, 2024 at 04:35:31PM -0400, Rodrigo Vivi wrote:
> No functional change. This patch only splits the xe_display_pm
> suspend/resume functions in the regular suspend/resume from the
> runtime/d3cold ones.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/xe/display/xe_display.c | 68 ++++++++++++++++---------
>  1 file changed, 45 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index be431d9907df..a4705a452adb 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -308,8 +308,41 @@ static void xe_display_flush_cleanup_work(struct xe_device *xe)
>  	}
>  }
>  
> -/* TODO: System and runtime suspend/resume sequences will be sanitized as a follow-up. */
> -static void __xe_display_pm_suspend(struct xe_device *xe, bool runtime)
> +static void xe_display_to_d3cold(struct xe_device *xe)
> +{
> +	struct intel_display *display = &xe->display;
> +
> +	/* We do a lot of poking in a lot of registers, make sure they work properly. */
> +	intel_power_domains_disable(xe);
> +
> +	xe_display_flush_cleanup_work(xe);
> +
> +	intel_hpd_cancel_work(xe);

This cancels a work synchronously that takes a runtime PM reference, so
it could dead-lock. I know that xe adds a way to get an RPM reference in
the runtime suspend/resume handlers, not sure if that's a good approach
in general. Also, not sure if it's ok to to drop all interrupts - in
intel_hpd_cancel_work() - here that got raised before runtime suspend,
instead of handling them as expected. So I think intel_hpd_cancel_work()
shouldn't be called here.

> +
> +	intel_opregion_suspend(display, PCI_D3cold);
> +
> +	intel_dmc_suspend(display);
> +}
> +
> +static void xe_display_from_d3cold(struct xe_device *xe)
> +{
> +	struct intel_display *display = &xe->display;
> +
> +	intel_dmc_resume(display);
> +
> +	if (has_display(xe))
> +		drm_mode_config_reset(&xe->drm);

The above is the counterpart of intel_encoder_suspend_all() which is
only called during system suspend.

> +
> +	intel_display_driver_init_hw(xe);
> +
> +	intel_hpd_init(xe);
> +
> +	intel_opregion_resume(display);
> +
> +	intel_power_domains_enable(xe);
> +}
> +
> +void xe_display_pm_suspend(struct xe_device *xe)
>  {
>  	struct intel_display *display = &xe->display;
>  	bool s2idle = suspend_to_idle();
> @@ -321,10 +354,10 @@ static void __xe_display_pm_suspend(struct xe_device *xe, bool runtime)
>  	 * properly.
>  	 */
>  	intel_power_domains_disable(xe);
> -	if (!runtime)
> -		intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true);
>  
> -	if (!runtime && has_display(xe)) {
> +	intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true);
> +
> +	if (has_display(xe)) {
>  		drm_kms_helper_poll_disable(&xe->drm);
>  		intel_display_driver_disable_user_access(xe);
>  		intel_display_driver_suspend(xe);
> @@ -334,7 +367,7 @@ static void __xe_display_pm_suspend(struct xe_device *xe, bool runtime)
>  
>  	intel_hpd_cancel_work(xe);
>  
> -	if (!runtime && has_display(xe)) {
> +	if (has_display(xe)) {
>  		intel_display_driver_suspend_access(xe);
>  		intel_encoder_suspend_all(&xe->display);
>  	}
> @@ -344,11 +377,6 @@ static void __xe_display_pm_suspend(struct xe_device *xe, bool runtime)
>  	intel_dmc_suspend(display);
>  }
>  
> -void xe_display_pm_suspend(struct xe_device *xe)
> -{
> -	__xe_display_pm_suspend(xe, false);
> -}
> -
>  void xe_display_pm_shutdown(struct xe_device *xe)
>  {
>  	if (!xe->info.probe_display)
> @@ -379,7 +407,7 @@ void xe_display_pm_runtime_suspend(struct xe_device *xe)
>  		return;
>  
>  	if (xe->d3cold.allowed)
> -		__xe_display_pm_suspend(xe, true);
> +		xe_display_to_d3cold(xe);
>  
>  	intel_hpd_poll_enable(xe);
>  }
> @@ -405,7 +433,7 @@ void xe_display_pm_resume_early(struct xe_device *xe)
>  	intel_power_domains_resume(xe);
>  }
>  
> -static void __xe_display_pm_resume(struct xe_device *xe, bool runtime)
> +void xe_display_pm_resume(struct xe_device *xe)
>  {
>  	struct intel_display *display = &xe->display;
>  
> @@ -419,12 +447,12 @@ static void __xe_display_pm_resume(struct xe_device *xe, bool runtime)
>  
>  	intel_display_driver_init_hw(xe);
>  
> -	if (!runtime && has_display(xe))
> +	if (has_display(xe))
>  		intel_display_driver_resume_access(xe);
>  
>  	intel_hpd_init(xe);
>  
> -	if (!runtime && has_display(xe)) {
> +	if (has_display(xe)) {
>  		intel_display_driver_resume(xe);
>  		drm_kms_helper_poll_enable(&xe->drm);
>  		intel_display_driver_enable_user_access(xe);
> @@ -433,17 +461,11 @@ static void __xe_display_pm_resume(struct xe_device *xe, bool runtime)
>  
>  	intel_opregion_resume(display);
>  
> -	if (!runtime)
> -		intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_RUNNING, false);
> +	intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_RUNNING, false);
>  
>  	intel_power_domains_enable(xe);
>  }
>  
> -void xe_display_pm_resume(struct xe_device *xe)
> -{
> -	__xe_display_pm_resume(xe, false);
> -}
> -
>  void xe_display_pm_runtime_resume(struct xe_device *xe)
>  {
>  	if (!xe->info.probe_display)
> @@ -452,7 +474,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe)
>  	intel_hpd_poll_disable(xe);
>  
>  	if (xe->d3cold.allowed)
> -		__xe_display_pm_resume(xe, true);
> +		xe_display_from_d3cold(xe);
>  }
>  
>  
> -- 
> 2.46.0
>