RE: [PATCH 31/31] drm/{i915, xe}/display: Consolidade entire runtime pm sequence

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



-----Original Message-----
From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Rodrigo Vivi
Sent: Tuesday, September 24, 2024 1:36 PM
To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel-xe@xxxxxxxxxxxxxxxxxxxxx
Cc: Deak, Imre <imre.deak@xxxxxxxxx>; Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>
Subject: [PATCH 31/31] drm/{i915, xe}/display: Consolidade entire runtime pm sequence
> 
> No functional change.
> 
> Consolidate the entire runtime pm sequences under
> intel_display_driver.
> 
> Simplifications and optimizations around the D3cold sequences are
> likely still possible. But before that can be done, consolidate
> everything at the intel_display_driver side.
> 
> Xe display power management functions are now only a wrapper
> checking for xe's display probe parameter.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>

LGTM.

Reviewed-by: Jonathan Cavitt <jonathan.cavitt@xxxxxxxxx>
-Jonathan Cavitt

> ---
>  .../drm/i915/display/intel_display_driver.c   | 66 ++++++++++++++++---
>  .../drm/i915/display/intel_display_driver.h   | 12 ++--
>  drivers/gpu/drm/i915/i915_driver.c            |  8 +--
>  drivers/gpu/drm/xe/display/xe_display.c       | 56 ++--------------
>  4 files changed, 74 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 62a7aa56f0da..3861fdbefaff 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -856,12 +856,45 @@ void intel_display_driver_resume(struct drm_i915_private *i915)
>  	intel_power_domains_enable(i915);
>  }
>  
> -void intel_display_driver_runtime_suspend(struct drm_i915_private *i915)
> +static void intel_display_to_d3cold(struct drm_i915_private *i915)
>  {
> -	intel_display_power_suspend(i915);
> +	struct intel_display *display = &i915->display;
> +
> +	/* We do a lot of poking in a lot of registers, make sure they work properly. */
> +	intel_power_domains_disable(i915);
> +
> +	intel_hpd_cancel_work(i915);
> +
> +	intel_opregion_suspend(display, PCI_D3cold);
> +
> +	intel_dmc_suspend(display);
> +}
> +
> +static void intel_display_from_d3cold(struct drm_i915_private *i915)
> +{
> +	struct intel_display *display = &i915->display;
> +
> +	intel_dmc_resume(display);
> +
> +	if (HAS_DISPLAY(i915))
> +		drm_mode_config_reset(&i915->drm);
> +
> +	intel_display_driver_init_hw(i915);
> +
> +	intel_opregion_resume(display);
> +
> +	intel_power_domains_enable(i915);
> +}
> +
> +void intel_display_driver_runtime_suspend(struct drm_i915_private *i915, bool d3cold_allowed)
> +{
> +	if (d3cold_allowed)
> +		intel_display_to_d3cold(i915);
> +	else
> +		intel_display_power_suspend(i915);
>  }
>  
> -void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915)
> +static void display_runtime_suspend_notify_opregion(struct drm_i915_private *i915)
>  {
>  	struct intel_display *display = &i915->display;
>  
> @@ -887,20 +920,37 @@ void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915)
>  		 */
>  		intel_opregion_notify_adapter(display, PCI_D1);
>  	}
> +}
> +
> +void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915,
> +					       bool d3cold_allowed)
> +{
> +	if (d3cold_allowed)
> +		intel_display_power_suspend_late(i915, false);
> +	else
> +		display_runtime_suspend_notify_opregion(i915);
>  
>  	if (!IS_VALLEYVIEW(i915) && !IS_CHERRYVIEW(i915))
>  		intel_hpd_poll_enable(i915);
>  }
>  
> -void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915)
> +void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915,
> +					       bool d3cold_allowed)
>  {
> -	intel_opregion_notify_adapter(&i915->display, PCI_D0);
> -
> -	intel_display_power_resume(i915);
> +	if (d3cold_allowed) {
> +		intel_display_power_resume_early(i915);
> +	} else {
> +		intel_opregion_notify_adapter(&i915->display, PCI_D0);
> +		intel_display_power_resume(i915);
> +	}
>  }
>  
> -void intel_display_driver_runtime_resume(struct drm_i915_private *i915)
> +void intel_display_driver_runtime_resume(struct drm_i915_private *i915,
> +					 bool d3cold_allowed)
>  {
> +	if (d3cold_allowed)
> +		intel_display_from_d3cold(i915);
> +
>  	/*
>  	 * On VLV/CHV display interrupts are part of the display
>  	 * power well, so hpd is reinitialized from there. For
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.h b/drivers/gpu/drm/i915/display/intel_display_driver.h
> index b1441a55d72d..21aa0e551898 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.h
> @@ -33,10 +33,14 @@ void intel_display_driver_resume(struct drm_i915_private *i915);
>  void intel_display_driver_resume_noirq(struct drm_i915_private *i915);
>  void intel_display_driver_resume_noirq_legacy(struct drm_i915_private *i915);
>  void intel_display_driver_resume_nogem(struct intel_display *display);
> -void intel_display_driver_runtime_suspend(struct drm_i915_private *i915);
> -void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915);
> -void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915);
> -void intel_display_driver_runtime_resume(struct drm_i915_private *i915);
> +void intel_display_driver_runtime_suspend(struct drm_i915_private *i915,
> +					  bool d3cold_allowed);
> +void intel_display_driver_runtime_suspend_late(struct drm_i915_private *i915,
> +					       bool d3cold_allowed);
> +void intel_display_driver_runtime_resume_early(struct drm_i915_private *i915,
> +					       bool d3cold_allowed);
> +void intel_display_driver_runtime_resume(struct drm_i915_private *i915,
> +					 bool d3cold_allowed);
>  void intel_display_driver_shutdown(struct drm_i915_private *i915);
>  void intel_display_driver_shutdown_noirq(struct drm_i915_private *i915);
>  void intel_display_driver_shutdown_nogem(struct drm_i915_private *i915);
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index b3eaa55ebacb..719b1c21b695 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1402,7 +1402,7 @@ static int intel_runtime_suspend(struct device *kdev)
>  	for_each_gt(gt, dev_priv, i)
>  		intel_uncore_suspend(gt->uncore);
>  
> -	intel_display_driver_runtime_suspend(dev_priv);
> +	intel_display_driver_runtime_suspend(dev_priv, false);
>  
>  	ret = vlv_suspend_complete(dev_priv);
>  	if (ret) {
> @@ -1436,7 +1436,7 @@ static int intel_runtime_suspend(struct device *kdev)
>  	if (root_pdev)
>  		pci_d3cold_disable(root_pdev);
>  
> -	intel_display_driver_runtime_suspend_late(dev_priv);
> +	intel_display_driver_runtime_suspend_late(dev_priv, false);
>  
>  	assert_forcewakes_inactive(&dev_priv->uncore);
>  
> @@ -1469,7 +1469,7 @@ static int intel_runtime_resume(struct device *kdev)
>  		drm_dbg(&dev_priv->drm,
>  			"Unclaimed access during suspend, bios?\n");
>  
> -	intel_display_driver_runtime_resume_early(dev_priv);
> +	intel_display_driver_runtime_resume_early(dev_priv, false);
>  
>  	ret = vlv_resume_prepare(dev_priv, true);
>  
> @@ -1487,7 +1487,7 @@ static int intel_runtime_resume(struct device *kdev)
>  
>  	intel_pxp_runtime_resume(dev_priv->pxp);
>  
> -	intel_display_driver_runtime_resume_early(dev_priv);
> +	intel_display_driver_runtime_resume_early(dev_priv, false);
>  
>  	enable_rpm_wakeref_asserts(rpm);
>  
> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c
> index ab85c7fb217a..9a652292d988 100644
> --- a/drivers/gpu/drm/xe/display/xe_display.c
> +++ b/drivers/gpu/drm/xe/display/xe_display.c
> @@ -283,36 +283,6 @@ static bool suspend_to_idle(void)
>  	return false;
>  }
>  
> -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);
> -
> -	intel_hpd_cancel_work(xe);
> -
> -	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);
> -
> -	intel_display_driver_init_hw(xe);
> -
> -	intel_opregion_resume(display);
> -
> -	intel_power_domains_enable(xe);
> -}
> -
>  void xe_display_pm_suspend(struct xe_device *xe)
>  {
>  	if (!xe->info.probe_display)
> @@ -413,10 +383,7 @@ void xe_display_pm_runtime_suspend(struct xe_device *xe)
>  	if (!xe->info.probe_display)
>  		return;
>  
> -	if (xe->d3cold.allowed)
> -		xe_display_to_d3cold(xe);
> -	else
> -		intel_display_power_suspend(xe);
> +	intel_display_driver_runtime_suspend(xe, xe->d3cold.allowed);
>  }
>  
>  void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
> @@ -424,12 +391,7 @@ void xe_display_pm_runtime_suspend_late(struct xe_device *xe)
>  	if (!xe->info.probe_display)
>  		return;
>  
> -	if (xe->d3cold.allowed)
> -		intel_display_power_suspend_late(xe, false);
> -	else
> -		intel_opregion_notify_adapter(&xe->display, PCI_D1);
> -
> -	intel_hpd_poll_enable(xe);
> +	intel_display_driver_runtime_suspend_late(xe, xe->d3cold.allowed);
>  }
>  
>  void xe_display_pm_runtime_resume_early(struct xe_device *xe)
> @@ -437,12 +399,7 @@ void xe_display_pm_runtime_resume_early(struct xe_device *xe)
>  	if (!xe->info.probe_display)
>  		return;
>  
> -	if (xe->d3cold.allowed) {
> -		intel_display_power_resume_early(xe);
> -	} else {
> -		intel_opregion_notify_adapter(&xe->display, PCI_D0);
> -		intel_display_power_resume(xe);
> -	}
> +	intel_display_driver_runtime_resume_early(xe, xe->d3cold.allowed);
>  }
>  
>  void xe_display_pm_runtime_resume(struct xe_device *xe)
> @@ -450,12 +407,7 @@ void xe_display_pm_runtime_resume(struct xe_device *xe)
>  	if (!xe->info.probe_display)
>  		return;
>  
> -	if (xe->d3cold.allowed)
> -		xe_display_from_d3cold(xe);
> -
> -	intel_hpd_init(xe);
> -	intel_hpd_poll_disable(xe);
> -	skl_watermark_ipc_update(xe);
> +	intel_display_driver_runtime_resume(xe, xe->d3cold.allowed);
>  }
>  
>  static void display_device_remove(struct drm_device *dev, void *arg)
> -- 
> 2.46.0
> 
> 




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux