[Intel-gfx] [PATCH 15/36] drm/i915: Mark up Ironlake ips with rpm wakerefs

[Chris Wilson]

Quoting Sagar Arun Kamble (2018-03-16 04:58:22)
> 
> 
> On 3/14/2018 3:07 PM, Chris Wilson wrote:
> > Currently Ironlake operates under the assumption that rpm awake (and its
> > error checking is disabled). As such, we have missed a few places where we
> > access registers without taking the rpm wakeref and thus trigger
> > warnings. intel_ips being one culprit.
> >
> > As this involved adding a potentially sleeping rpm_get, we have to
> > rearrange the spinlocks slightly and so switch to acquiring a device-ref
> > under the spinlock rather than hold the spinlock for the whole
> > operation. To be consistent, we make the change in pattern common to the
> > intel_ips interface even though this adds a few more atomic operations
> > than necessary in a few cases.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/i915_drv.c |   3 +
> >   drivers/gpu/drm/i915/intel_pm.c | 138 ++++++++++++++++++++--------------------
> >   2 files changed, 73 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 3d0b7353fb09..5c28990aab7f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1440,6 +1440,9 @@ void i915_driver_unload(struct drm_device *dev)
> >   
> >       i915_driver_unregister(dev_priv);
> >   
> > +     /* Flush any external code that still may be under the RCU lock */
> > +     synchronize_rcu();
> > +
> Hi Chris,
> 
> Will this rcu change be equivalent to
> 
> rcu_assign_pointer(i915_mch_dev, dev_priv) in gpu_ips_init
> rcu_assign_pointer(i915_mch_dev, NULL) in gpu_ips_teardown
> 
> eliminating smp_store_mb from init/teardown and synchronize_rcu here.

We still have to go through the RCU period on teardown to be sure we
flush all readers, but yes, the store_mb can be reduce to
RCU_INIT_POINTER() and the mb are overkill as all we really need is the
ordering on init, and the explicit rcu sync on teardown.
-Chris