public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Paulo Zanoni <przanoni@gmail.com>
To: intel-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Paulo Zanoni <paulo.r.zanoni@intel.com>
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	[thread overview]
Message-ID: <1400704171-1920-1-git-send-email-przanoni@gmail.com> (raw)
In-Reply-To: <1400700202-2905-2-git-send-email-przanoni@gmail.com>

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

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 <daniel.vetter@ffwll.ch>
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 <daniel.vetter@ffwll.ch>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 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

  reply	other threads:[~2014-05-21 20:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-21 19:23 [PATCH 1/3] drm/i915: don't read HSW_AUD_PIN_ELD_CP_VLD when the power well is off Paulo Zanoni
2014-05-21 19:23 ` [PATCH 2/3] drm/i915: don't read HSW_AUD_PIN_ELD_CP_VLD wehn the powe rwell is off (2) Paulo Zanoni
2014-05-21 20:29   ` Paulo Zanoni [this message]
2014-05-21 20:37     ` [PATCH 2/3] drm/i915: grab the audio power domain when enabling audio on HSW+ Daniel Vetter
2014-05-21 19:23 ` [PATCH 3/3] drm/i915: reorganize the unclaimed register detection code Paulo Zanoni
2014-05-21 19:33 ` [PATCH 1/3] drm/i915: don't read HSW_AUD_PIN_ELD_CP_VLD when the power well is off Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1400704171-1920-1-git-send-email-przanoni@gmail.com \
    --to=przanoni@gmail.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox