Re: [PATCH] drm/i915: Acquire audio powerwell for HD-Audio registers

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Takashi Iwai <tiwai@suse.de>,
	intel-gfx@lists.freedesktop.org, stable@vger.kernel.org
Subject: Re: [PATCH] drm/i915: Acquire audio powerwell for HD-Audio registers
Date: Thu, 4 Aug 2016 18:44:11 +0200	[thread overview]
Message-ID: <20160804164411.GS6232@phenom.ffwll.local> (raw)
In-Reply-To: <1470240540-29004-1-git-send-email-chris@chris-wilson.co.uk>

On Wed, Aug 03, 2016 at 05:09:00PM +0100, Chris Wilson wrote:
> On Haswell/Broadwell, the HD-Audio block is inside the HDMI/display
> power well and so the sna-hda audio codec acquires the display power
> well while it is operational. However, Skylake separates the powerwells
> again, but yet we still need the audio powerwell to setup the registers.
> (But then the hardware uses those registers even while powered off???)

Yeah feels fishy, but will at least duct-tape over the breakage from the
audio side. Most likely the reg writes go exactly nowhere and there's a
bug on the audio side. And this patch doesn't fix that.

But it does what it says on the tin, and it gets the job done.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> v2: Grab both rpm wakelock and audio wakelock
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96214
> Fixes: 03b135cebc47 "ALSA: hda - remove dependency on i915 power well for SKL")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Libin Yang <libin.yang@intel.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: Marius Vlad <marius.c.vlad@intel.com>
> Tested-by: Hans de Goede <hdegoede@redhat.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_audio.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 5d5f6bc10e85..948a7a52e3f8 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -600,6 +600,8 @@ static void i915_audio_component_codec_wake_override(struct device *dev,
>  	if (!IS_SKYLAKE(dev_priv) && !IS_KABYLAKE(dev_priv))
>  		return;
>  
> +	i915_audio_component_get_power(dev);
> +
>  	/*
>  	 * Enable/disable generating the codec wake signal, overriding the
>  	 * internal logic to generate the codec wake to controller.
> @@ -615,6 +617,8 @@ static void i915_audio_component_codec_wake_override(struct device *dev,
>  		I915_WRITE(HSW_AUD_CHICKENBIT, tmp);
>  		usleep_range(1000, 1500);
>  	}
> +
> +	i915_audio_component_put_power(dev);
>  }
>  
>  /* Get CDCLK in kHz  */
> @@ -648,6 +652,7 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
>  	    !IS_HASWELL(dev_priv))
>  		return 0;
>  
> +	i915_audio_component_get_power(dev);
>  	mutex_lock(&dev_priv->av_mutex);
>  	/* 1. get the pipe */
>  	intel_encoder = dev_priv->dig_port_map[port];
> @@ -698,6 +703,7 @@ static int i915_audio_component_sync_audio_rate(struct device *dev,
>  
>   unlock:
>  	mutex_unlock(&dev_priv->av_mutex);
> +	i915_audio_component_put_power(dev);
>  	return err;
>  }
>  
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx