On Thu, May 09, 2019 at 09:03:04AM +0100, Chris Wilson wrote:
> 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@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > 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.
Ok.
> There's a few repetitions in include/linux so maybe worth
> centralising.
For reference, the corresponding spatch, found a few tens of uses across
the tree (not sure if 'identifier I;' is the best way to denote a struct
member):
@nc@
type T;
identifier I;
@@
- ((T *)NULL)->I
+ struct_member(T, I)
>
> > +#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?
Yep. One day I may go through the architecture maze/header magic I
suspect that involves, or if someone could add it I'd be happy to use
that instead.
>
> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
