Re: [alsa-devel] [PATCH 2/3] ASoC: SOF: Intel: hda: fix ordering bug in resume flow

From: Takashi Iwai <tiwai@suse.de>
To: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Cc: alsa-devel@alsa-project.org, broonie@kernel.org,
	pierre-louis.bossart@linux.intel.com,
	ranjani.sridharan@linux.intel.com
Subject: Re: [alsa-devel] [PATCH 2/3] ASoC: SOF: Intel: hda: fix ordering bug in resume flow
Date: Fri, 07 Feb 2020 09:21:36 +0100	[thread overview]
Message-ID: <s5hr1z6vmcv.wl-tiwai@suse.de> (raw)
In-Reply-To: <20200206200223.7715-2-kai.vehmanen@linux.intel.com>

On Thu, 06 Feb 2020 21:02:22 +0100,
Kai Vehmanen wrote:
> 
> When HDA controller is resumed from suspend, i915 HDMI/DP
> codec requires that following order of actions is kept:
> 
>  - i915 display power up and configuration of link params
>  - hda link reset and setup
> 
> Current SOF HDA code delegates display codec power control
> to the codec driver. This works most of the time, but in
> runtime PM sequences, the above constraint may be violated.
> On platforms where BIOS values for HDA link parameters do
> not match hardware reset defaults, this may lead to errors
> in HDA verb transactions after resume.
> 
> Fix the issue by explicitly powering the display codec
> in the HDA controller resume/suspend calls, thus ensuring
> correct ordering. Special handling is needed for the D0i3
> flow, where display power must be turned off even though
> DSP is left powered.
> 
> Now that we have more invocations of the display power helper
> functions, the conditional checks surrounding each call have
> been moved inside hda_codec_i915_display_power(). The two
> special cases of display powering at initial probe are handled
> separately. The intent is to avoid powering the display whenever
> no display codecs are used.
> 
> Note that early powering of display was removed in
> commit 687ae9e287b3 ("ASoC: intel: skl: Fix display power regression").
> This change was also copied to the SOF driver. No failures
> have resulted as hardware default values for link parameters
> have worked out of the box. However with recent i915 driver
> changes like done in commit 87c1694533c9 ("drm/i915: save
> AUD_FREQ_CNTRL state at audio domain suspend"), this does not
> hold anymore and errors are hit.
> 
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel