diff mbox series

drm/i915/audio: set HDA link parameters in driver

Message ID 20210212150838.2568483-1-kai.vehmanen@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/audio: set HDA link parameters in driver | expand

Commit Message

Kai Vehmanen Feb. 12, 2021, 3:08 p.m. UTC 
Update logic to program AUD_FREQ_CNTRL register based on new guidance.
Earlier this register was configured by BIOS and driver discovered the
value at init. This is no longer recommended and instead driver should
set the values based on the hardware revision.

Add the recommended values for all supported hardware. This change applies
for all GEN12+ hardware. For TGL, some special case handling is needed
to not break existing systems.

Extend the debug print to also include values of the register as written
by BIOS. This can help debug rare cases where BIOS has configured the link
settings to incorrect values.

Bspec: 49279
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/display/intel_audio.c | 30 ++++++++++++++++++----
 1 file changed, 25 insertions(+), 5 deletions(-)


base-commit: 9d8fddf8579a2a20d0e8a8b631547069a62b953e

Comments

Kai Vehmanen March 22, 2021, 11:53 a.m. UTC | #1 
Hi,

On Fri, 12 Feb 2021, Kai Vehmanen wrote:

> Update logic to program AUD_FREQ_CNTRL register based on new guidance.
> Earlier this register was configured by BIOS and driver discovered the
> value at init. This is no longer recommended and instead driver should
> set the values based on the hardware revision.

any chance to get this merged? This has a reviewed-by already and all
tests pass:

https://patchwork.freedesktop.org/series/87040/

Br, Kai
Shankar, Uma March 24, 2021, 11:20 a.m. UTC | #2 
> -----Original Message-----
> From: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Sent: Monday, March 22, 2021 5:24 PM
> To: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>
> Subject: Re: [PATCH] drm/i915/audio: set HDA link parameters in driver
> 
> Hi,
> 
> On Fri, 12 Feb 2021, Kai Vehmanen wrote:
> 
> > Update logic to program AUD_FREQ_CNTRL register based on new guidance.
> > Earlier this register was configured by BIOS and driver discovered the
> > value at init. This is no longer recommended and instead driver should
> > set the values based on the hardware revision.
> 
> any chance to get this merged? This has a reviewed-by already and all tests pass:

Hi Kai,
Can you rebase and resend, I will help merge this.

Regards,
Uma Shankar

> https://patchwork.freedesktop.org/series/87040/
> 
> Br, Kai
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index f7de55707746..2c486284396d 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -1266,6 +1266,15 @@  static const struct component_ops i915_audio_component_bind_ops = {
 	.unbind	= i915_audio_component_unbind,
 };
 
+#define AUD_FREQ_TMODE_SHIFT	14
+#define AUD_FREQ_4T		0
+#define AUD_FREQ_8T		(2 << AUD_FREQ_TMODE_SHIFT)
+#define AUD_FREQ_PULLCLKS(x)	(((x) & 0x3) << 11)
+#define AUD_FREQ_BCLK_96M	BIT(4)
+
+#define AUD_FREQ_GEN12          (AUD_FREQ_8T | AUD_FREQ_PULLCLKS(0) | AUD_FREQ_BCLK_96M)
+#define AUD_FREQ_TGL_BROKEN     (AUD_FREQ_8T | AUD_FREQ_PULLCLKS(2) | AUD_FREQ_BCLK_96M)
+
 /**
  * i915_audio_component_init - initialize and register the audio component
  * @dev_priv: i915 device instance
@@ -1284,6 +1293,7 @@  static const struct component_ops i915_audio_component_bind_ops = {
  */
 static void i915_audio_component_init(struct drm_i915_private *dev_priv)
 {
+	u32 aud_freq, aud_freq_init;
 	int ret;
 
 	ret = component_add_typed(dev_priv->drm.dev,
@@ -1297,11 +1307,21 @@  static void i915_audio_component_init(struct drm_i915_private *dev_priv)
 	}
 
 	if (INTEL_GEN(dev_priv) >= 9) {
-		dev_priv->audio_freq_cntrl = intel_de_read(dev_priv,
-							   AUD_FREQ_CNTRL);
-		drm_dbg_kms(&dev_priv->drm,
-			    "init value of AUD_FREQ_CNTRL of 0x%x\n",
-			    dev_priv->audio_freq_cntrl);
+		aud_freq_init = intel_de_read(dev_priv, AUD_FREQ_CNTRL);
+
+		if (INTEL_GEN(dev_priv) >= 12)
+			aud_freq = AUD_FREQ_GEN12;
+		else
+			aud_freq = aud_freq_init;
+
+		/* use BIOS provided value for TGL unless it is a known bad value */
+		if (IS_TIGERLAKE(dev_priv) && aud_freq_init != AUD_FREQ_TGL_BROKEN)
+			aud_freq = aud_freq_init;
+
+		drm_dbg_kms(&dev_priv->drm, "use AUD_FREQ_CNTRL of 0x%x (init value 0x%x)\n",
+			    aud_freq, aud_freq_init);
+
+		dev_priv->audio_freq_cntrl = aud_freq;
 	}
 
 	dev_priv->audio_component_registered = true;