From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paulo Zanoni Subject: [PATCH 2/3] drm/i915: grab the audio power domain when enabling audio on HSW+ Date: Wed, 21 May 2014 17:29:31 -0300 Message-ID: <1400704171-1920-1-git-send-email-przanoni@gmail.com> References: <1400700202-2905-2-git-send-email-przanoni@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-yk0-f172.google.com (mail-yk0-f172.google.com [209.85.160.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 1D8A66E0A7 for ; Wed, 21 May 2014 13:29:57 -0700 (PDT) Received: by mail-yk0-f172.google.com with SMTP id 79so2037778ykr.17 for ; Wed, 21 May 2014 13:29:56 -0700 (PDT) In-Reply-To: <1400700202-2905-2-git-send-email-przanoni@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: intel-gfx@lists.freedesktop.org Cc: Daniel Vetter , Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org From: Paulo Zanoni With the current code, we unconditionally touch HSW_AUD_PIN_ELD_CP_VLD, which means we can touch it when the power well is off, and that will trigger an "Unclaimed register" message. Just adding the intel_crtc->config.has_audio should already avoid the unclaimed register messsages, but since we actually need the power well to make the Audio code work, it makes sense to also grab the audio power domain reference, and release it when it's not needed anymore. I used IGT's pm_rpm to reproduce this bug, but it can probably be reproduced on other tests that do modesets. I'm using a machine with eDP+HDMI connected. Regression introduced by: commit acfa75b02e72bad7c93564ac379712e29c001432 Author: Daniel Vetter Date: Thu Apr 24 23:54:51 2014 +0200 drm/i915: Simplify audio handling on DDI ports Credits to Daniel for suggesting this implementation. Cc: Daniel Vetter Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_ddi.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 355f569..b17b9c7 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1355,6 +1355,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder) } if (intel_crtc->config.has_audio) { + intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO); tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD); tmp |= ((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) << (pipe * 4)); I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp); @@ -1372,10 +1373,15 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder) struct drm_i915_private *dev_priv = dev->dev_private; uint32_t tmp; - tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD); - tmp &= ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) << - (pipe * 4)); - I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp); + /* We can't touch HSW_AUD_PIN_ELD_CP_VLD uncionditionally because this + * register is part of the power well on Haswell. */ + if (intel_crtc->config.has_audio) { + tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD); + tmp &= ~((AUDIO_OUTPUT_ENABLE_A | AUDIO_ELD_VALID_A) << + (pipe * 4)); + I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp); + intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO); + } if (type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); -- 1.9.0