[Intel-gfx] [PATCH v2 01/11] drm/i915: Add support for tracking wakerefs w/o power-on guarantee

[Chris Wilson]

Quoting Imre Deak (2019-05-09 07:19:44)
> It's useful to track runtime PM refs that don't guarantee a device
> power-on state to the rest of the driver. One such case is holding a
> reference that will be put asynchronously, during which normal users
> without their own reference shouldn't access the HW. A follow-up patch
> will add support for disabling display power domains asynchronously
> which needs this.
> 
> For this we can split wakeref_count into a low half-word tracking
> all references (raw-wakerefs) and a high half-word tracking
> references guaranteeing a power-on state (wakelocks).
> 
> Follow-up patches will make use of the API added here.
> 
> While at it add the missing docbook header for the unchecked
> display-power and runtime_pm put functions.
> 
> No functional changes, except for printing leaked raw-wakerefs
> and wakelocks separately in intel_runtime_pm_cleanup().
> 
> v2:
> - Track raw wakerefs/wakelocks in the low/high half-word of
>   wakeref_count, instead of adding a new counter. (Chris)
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h        |  52 +++++++-
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 150 ++++++++++++++++++------
>  2 files changed, 160 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 247893ed1543..772ed0fedb39 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1619,6 +1619,24 @@ unsigned int i9xx_plane_max_stride(struct intel_plane *plane,
>                                    unsigned int rotation);
>  
>  /* intel_runtime_pm.c */

#define struct_member(T, member) (((T *)0)->member)

and stick that in i915_utils.h. There's a few repetitions in
include/linux so maybe worth centralising.

> +#define BITS_PER_WAKEREF       \
> +       BITS_PER_TYPE(((struct i915_runtime_pm *)NULL)->wakeref_count)
> +#define INTEL_RPM_WAKELOCK_SHIFT       (BITS_PER_WAKEREF / 2)
> +#define INTEL_RPM_WAKELOCK_BIAS                (1 << INTEL_RPM_WAKELOCK_SHIFT)
> +#define INTEL_RPM_RAW_WAKEREF_MASK     (INTEL_RPM_WAKELOCK_BIAS - 1)


> +static void
> +intel_runtime_pm_acquire(struct drm_i915_private *i915, bool wakelock)
> +{
> +       struct i915_runtime_pm *rpm = &i915->runtime_pm;
> +
> +       if (wakelock) {
> +               atomic_add(1 + INTEL_RPM_WAKELOCK_BIAS, &rpm->wakeref_count);
> +               assert_rpm_wakelock_held(i915);
> +       } else {
> +               atomic_inc(&rpm->wakeref_count);
> +               assert_rpm_raw_wakeref_held(i915);
> +       }
> +}
> +
> +static void
> +intel_runtime_pm_release(struct drm_i915_private *i915, int wakelock)
> +{
> +       struct i915_runtime_pm *rpm = &i915->runtime_pm;
> +
> +       if (wakelock) {
> +               assert_rpm_wakelock_held(i915);
> +               atomic_sub(INTEL_RPM_WAKELOCK_BIAS, &rpm->wakeref_count);
> +       } else {
> +               assert_rpm_raw_wakeref_held(i915);
> +       }
> +
> +       __intel_wakeref_dec_and_check_tracking(i915);

Creating a atomic_sub_and_lock_irqsave() would restore the onion?

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris