Daniel Drake <drake@xxxxxxxxxxxx> 於 2019年6月3日 週一 下午4:47寫道：
>
> On Mon, Jun 3, 2019 at 4:27 PM Jian-Hong Pan <jian-hong@xxxxxxxxxxxx> wrote:
> > static void i915_audio_component_get_power(struct device *kdev)
> > {
> > - intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
> > + struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> > +
> > + intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> > +
> > + /* Force CDCLK to 2*BCLK as long as we need audio to be powered. */
> > + if (dev_priv->audio_power_refcount++ == 0)
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > + glk_force_audio_cdclk(dev_priv, true);
> > }
> >
> > static void i915_audio_component_put_power(struct device *kdev)
> > {
> > - intel_display_power_put_unchecked(kdev_to_i915(kdev),
> > - POWER_DOMAIN_AUDIO);
> > + struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> > + intel_wakeref_t cookie;
> > +
> > + /* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
> > + if (--dev_priv->audio_power_refcount == 0)
> > + if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > + glk_force_audio_cdclk(dev_priv, false);
> > +
> > + cookie = intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> > + intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO, cookie);
> > }
>
> The code above is the rediffed part of the patch. I think the last 2
> lines here are not quite right.
>
> Here, get means "increment reference count" and put means "decrease
> reference count".
>
> Before your patch, i915_audio_component_get_power() does
> intel_display_power_get(), and i915_audio_component_put_power() does
> intel_display_power_put_unchecked(). You can see the symmetry.
>
> After your patch, i915_audio_component_get_power() does
> intel_display_power_get() as before (good), but
> i915_audio_component_put_power() does a 2nd get and then a single put.
> So the reference count is now unbalanced.
>
> I think the last 2 lines of this function should just be left the same
> as they were before:
> intel_display_power_put_unchecked(kdev_to_i915(kdev),
> POWER_DOMAIN_AUDIO);
Hi Daniel,
Thanks for your reviewing.
I'm going to prepare new Rediff.
Jian-Hong Pan
