From: Imre Deak <imre.deak@intel.com>
To: Jani Nikula <jani.nikula@intel.com>,
intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: [PATCH 1/7] drm/i915/display: simplify conditional compilation on runtime PM debug
Date: Thu, 28 Nov 2024 14:35:47 +0200 [thread overview]
Message-ID: <Z0hjo8xx1D6cwp0Q@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <Z0himvZ-5JqTlSpU@ideak-desk.fi.intel.com>
On Thu, Nov 28, 2024 at 02:31:22PM +0200, Imre Deak wrote:
> 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
sorry, confused up the image names, correctly it is:
$ size ~/i915-opt.ko ~/i915-noopt.ko
text data bss dec hex filename
3345708 325773 7680 3679161 3823b9 /home/imre/i915-opt.ko
3346869 325789 7680 3680338 382852 /home/imre/i915-noopt.ko
> > Cc: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Jani Nikula <jani.nikula@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
> >
next prev parent reply other threads:[~2024-11-28 12:35 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-27 17:06 [PATCH 0/7] drm/i915/display: power conversion to struct intel_display Jani Nikula
2024-11-27 17:06 ` [PATCH 1/7] drm/i915/display: simplify conditional compilation on runtime PM debug Jani Nikula
2024-11-28 12:31 ` Imre Deak
2024-11-28 12:35 ` Imre Deak [this message]
2024-11-28 15:39 ` Jani Nikula
2024-11-27 17:06 ` [PATCH 2/7] drm/i915/display: convert for_each_power_well() to struct intel_display Jani Nikula
2024-11-27 17:06 ` [PATCH 3/7] drm/i915/display: convert for_each_power_domain_well() " Jani Nikula
2024-11-27 17:06 ` [PATCH 4/7] drm/i915/display: convert power wells " Jani Nikula
2024-11-27 17:06 ` [PATCH 5/7] drm/i915/display: convert power domain code internally " Jani Nikula
2024-11-27 17:06 ` [PATCH 6/7] drm/i915/display: convert high level power interfaces " Jani Nikula
2024-11-27 17:06 ` [PATCH 7/7] drm/i915/display: convert power map " Jani Nikula
2024-11-27 17:42 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: power conversion " Patchwork
2024-11-27 17:42 ` ✗ Fi.CI.SPARSE: " Patchwork
2024-11-27 17:54 ` ✗ i915.CI.BAT: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z0hjo8xx1D6cwp0Q@ideak-desk.fi.intel.com \
--to=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox