Daniel Drake drake@endlessm.com 於 2019年6月3日 週一 下午4:47寫道：
On Mon, Jun 3, 2019 at 4:27 PM Jian-Hong Pan jian-hong@endlessm.com 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