Quoting Mika Kuoppala (2019-01-14 15:01:59)
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> > unsigned long i915_chipset_val(struct drm_i915_private *dev_priv)
> > {
> > - unsigned long val;
> > + intel_wakeref_t wakeref;
> > + unsigned long val = 0;
> >
> > if (!IS_GEN(dev_priv, 5))
> > return 0;
> >
> > - spin_lock_irq(&mchdev_lock);
> > -
> > - val = __i915_chipset_val(dev_priv);
> > -
> > - spin_unlock_irq(&mchdev_lock);
> > + with_intel_runtime_pm(dev_priv, wakeref) {
> > + spin_lock_irq(&mchdev_lock);
>
> This lock is now much more ips lock than mchdev_lock.
> Name should reflect that, so ips_lock?
ips is a user, the mch is the device. The interface is yucky, so best
forget and move on, honestly.
[snip]
> > - return ret;
> > + drm_dev_put(&i915->drm);
>
> mchdev_put() would read better.
Nah. mchdev_get() returns a new reference to the underlying device. From
that moment on, it's just a regular device reference. We are not
operating on the singleton itself.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
