[PATCH 1/7] drm/i915/display: simplify conditional compilation on runtime PM debug

Imre Deak imre.deak at intel.com
Thu Nov 28 12:31:22 UTC 2024


On Wed, Nov 27, 2024 at 07:06:02PM +0200, Jani Nikula wrote:
> Simplify conditional compilation on CONFIG_DRM_I915_DEBUG_RUNTIME_PM.
> Hide it all inside intel_display_power.c.
> 
> This will unnecessarily pass in the wakeref also when debug is disabled,
> but it should not matter a whole lot.

Ftr: there are a lot of callers of these functions and so this change
removing the optimization increases the code by >1kB in the non-debug
build:

$ size ~/i915-opt.ko  ~/i915-noopt.ko
   text	   data	    bss	    dec	    hex	filename
3346869	 325789	   7680	3680338	 382852	/home/imre/i915-opt.ko
3345708	 325773	   7680	3679161	 3823b9	/home/imre/i915-noopt.ko

> Cc: Imre Deak <imre.deak at intel.com>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    | 49 +++++++++-------
>  .../drm/i915/display/intel_display_power.h    | 56 +++----------------
>  2 files changed, 37 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 59dee2dc0552..fe94ef310f6b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -706,10 +706,10 @@ intel_display_power_put_async_work(struct work_struct *work)
>   * The power down is delayed by @delay_ms if this is >= 0, or by a default
>   * 100 ms otherwise.
>   */
> -void __intel_display_power_put_async(struct drm_i915_private *i915,
> -				     enum intel_display_power_domain domain,
> -				     intel_wakeref_t wakeref,
> -				     int delay_ms)
> +static void __intel_display_power_put_async(struct drm_i915_private *i915,
> +					    enum intel_display_power_domain domain,
> +					    intel_wakeref_t wakeref,
> +					    int delay_ms)
>  {
>  	struct i915_power_domains *power_domains = &i915->display.power.domains;
>  	struct intel_runtime_pm *rpm = &i915->runtime_pm;
> @@ -750,6 +750,27 @@ void __intel_display_power_put_async(struct drm_i915_private *i915,
>  	intel_runtime_pm_put(rpm, wakeref);
>  }
>  
> +void intel_display_power_put_async(struct drm_i915_private *i915,
> +				   enum intel_display_power_domain domain,
> +				   intel_wakeref_t wakeref)
> +{
> +	if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM))
> +		wakeref = INTEL_WAKEREF_DEF;
> +
> +	__intel_display_power_put_async(i915, domain, wakeref, -1);
> +}
> +
> +void intel_display_power_put_async_delay(struct drm_i915_private *i915,
> +					 enum intel_display_power_domain domain,
> +					 intel_wakeref_t wakeref,
> +					 int delay_ms)
> +{
> +	if (!IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM))
> +		wakeref = INTEL_WAKEREF_DEF;
> +
> +	__intel_display_power_put_async(i915, domain, wakeref, delay_ms);
> +}
> +
>  /**
>   * intel_display_power_flush_work - flushes the async display power disabling work
>   * @i915: i915 device instance
> @@ -807,7 +828,6 @@ intel_display_power_flush_work_sync(struct drm_i915_private *i915)
>  	drm_WARN_ON(&i915->drm, power_domains->async_put_wakeref);
>  }
>  
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
>  /**
>   * intel_display_power_put - release a power domain reference
>   * @dev_priv: i915 device instance
> @@ -818,6 +838,7 @@ intel_display_power_flush_work_sync(struct drm_i915_private *i915)
>   * intel_display_power_get() and might power down the corresponding hardware
>   * block right away if this is the last reference.
>   */
> +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
>  void intel_display_power_put(struct drm_i915_private *dev_priv,
>  			     enum intel_display_power_domain domain,
>  			     intel_wakeref_t wakeref)
> @@ -826,21 +847,9 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
>  	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
>  }
>  #else
> -/**
> - * intel_display_power_put_unchecked - release an unchecked power domain reference
> - * @dev_priv: i915 device instance
> - * @domain: power domain to reference
> - *
> - * This function drops the power domain reference obtained by
> - * intel_display_power_get() and might power down the corresponding hardware
> - * block right away if this is the last reference.
> - *
> - * This function is only for the power domain code's internal use to suppress wakeref
> - * tracking when the correspondig debug kconfig option is disabled, should not
> - * be used otherwise.
> - */
> -void intel_display_power_put_unchecked(struct drm_i915_private *dev_priv,
> -				       enum intel_display_power_domain domain)
> +void intel_display_power_put(struct drm_i915_private *dev_priv,
> +			     enum intel_display_power_domain domain,
> +			     intel_wakeref_t wakeref)
>  {
>  	__intel_display_power_put(dev_priv, domain);
>  	intel_runtime_pm_put_unchecked(&dev_priv->runtime_pm);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index 688f3b60b5c5..c6bd4f122487 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -190,60 +190,20 @@ intel_wakeref_t intel_display_power_get(struct drm_i915_private *dev_priv,
>  intel_wakeref_t
>  intel_display_power_get_if_enabled(struct drm_i915_private *dev_priv,
>  				   enum intel_display_power_domain domain);
> -void __intel_display_power_put_async(struct drm_i915_private *i915,
> -				     enum intel_display_power_domain domain,
> -				     intel_wakeref_t wakeref,
> -				     int delay_ms);
>  void intel_display_power_flush_work(struct drm_i915_private *i915);
> -#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RUNTIME_PM)
> +
>  void intel_display_power_put(struct drm_i915_private *dev_priv,
>  			     enum intel_display_power_domain domain,
>  			     intel_wakeref_t wakeref);
> -static inline void
> -intel_display_power_put_async(struct drm_i915_private *i915,
> -			      enum intel_display_power_domain domain,
> -			      intel_wakeref_t wakeref)
> -{
> -	__intel_display_power_put_async(i915, domain, wakeref, -1);
> -}
>  
> -static inline void
> -intel_display_power_put_async_delay(struct drm_i915_private *i915,
> -				    enum intel_display_power_domain domain,
> -				    intel_wakeref_t wakeref,
> -				    int delay_ms)
> -{
> -	__intel_display_power_put_async(i915, domain, wakeref, delay_ms);
> -}
> -#else
> -void intel_display_power_put_unchecked(struct drm_i915_private *dev_priv,
> -				       enum intel_display_power_domain domain);
> +void intel_display_power_put_async(struct drm_i915_private *i915,
> +				   enum intel_display_power_domain domain,
> +				   intel_wakeref_t wakeref);
>  
> -static inline void
> -intel_display_power_put(struct drm_i915_private *i915,
> -			enum intel_display_power_domain domain,
> -			intel_wakeref_t wakeref)
> -{
> -	intel_display_power_put_unchecked(i915, domain);
> -}
> -
> -static inline void
> -intel_display_power_put_async(struct drm_i915_private *i915,
> -			      enum intel_display_power_domain domain,
> -			      intel_wakeref_t wakeref)
> -{
> -	__intel_display_power_put_async(i915, domain, INTEL_WAKEREF_DEF, -1);
> -}
> -
> -static inline void
> -intel_display_power_put_async_delay(struct drm_i915_private *i915,
> -				    enum intel_display_power_domain domain,
> -				    intel_wakeref_t wakeref,
> -				    int delay_ms)
> -{
> -	__intel_display_power_put_async(i915, domain, INTEL_WAKEREF_DEF, delay_ms);
> -}
> -#endif
> +void intel_display_power_put_async_delay(struct drm_i915_private *i915,
> +					 enum intel_display_power_domain domain,
> +					 intel_wakeref_t wakeref,
> +					 int delay_ms);
>  
>  void
>  intel_display_power_get_in_set(struct drm_i915_private *i915,
> -- 
> 2.39.5
> 


More information about the Intel-xe mailing list