[Intel-gfx] [PATCH 22/22] hax: drm/i915/audio: Make HSW hardware ELD buffer sort of work

Ville Syrjala ville.syrjala at linux.intel.com
Tue Oct 11 17:00:11 UTC 2022


From: Ville Syrjälä <ville.syrjala at linux.intel.com>

On HSW (at least) the ELD buffer seems to live in the codec, and
when the codec isn't suitably powered up the ELD buffer does not
work at all (address is stuck at 0, data goes nowhere).

So we have this fun chicken and egg problem where we need to
power up the codec before doing the ELD update, but in order
to get the audio driver to do stuff we sort of need the ELD
buffer prefilled. Apparently reordering the .pin_eld_notify()
to happen before .audio_codec_enable() is sufficient to trigger
pulseaudio (lols) to kick the codec out of its slumber and then
we just wait in .audio_codec_enable() until the ELD buffer
operates normally.

Took me a while to figure out what the heck is going on, with
ELD buffer sometimes kinda working, something taking random
amount of time to become operational, etc. Initially didn't
even realize pulseaudio was enabled on this systems and part
of the "solution".

If we really want to poke at the ELD hw buffer, then it seems
we'd need some kind of synchronous codec power up call first
(while avoiding all the fun deadlocks), then do the ELD write,
and finally notify the audio driver about stuff.

Maybe the answer is to just stop poking at the ELD buffer
altogether? For which hw can we do that on? ilk+? Though then
we also don't get the state checker, but does it matter at
that point even? Also I have no idea if the "ELD valid" bit
does something else in hardware besides trigger the unsolicited
event for the audio driver?

Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah at intel.com>
Cc: Kai Vehmanen <kai.vehmanen at linux.intel.com>
Cc: Takashi Iwai <tiwai at suse.de>
Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_audio.c    | 23 +++++++++++++++----
 .../gpu/drm/i915/display/intel_audio_regs.h   |  1 +
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 328c47719fd8..d7c63dd2f2a7 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -655,6 +655,16 @@ static void enable_audio_dsc_wa(struct intel_encoder *encoder,
 	intel_de_write(i915, AUD_CONFIG_BE, val);
 }
 
+static bool hsw_audio_coded_ready(struct drm_i915_private *i915,
+				  enum transcoder cpu_transcoder)
+{
+	intel_de_rmw(i915, HSW_AUD_DIP_ELD_CTRL(cpu_transcoder),
+		     IBX_ELD_ADDRESS_MASK, IBX_ELD_ADDRESS(1));
+
+	return (intel_de_read(i915, HSW_AUD_DIP_ELD_CTRL(cpu_transcoder)) &
+		IBX_ELD_ADDRESS_MASK) == IBX_ELD_ADDRESS(1);
+}
+
 static void hsw_audio_codec_enable(struct intel_encoder *encoder,
 				   const struct intel_crtc_state *crtc_state,
 				   const struct drm_connector_state *conn_state)
@@ -681,6 +691,9 @@ static void hsw_audio_codec_enable(struct intel_encoder *encoder,
 	intel_de_rmw(i915, HSW_AUD_PIN_ELD_CP_VLD,
 		     AUDIO_ELD_VALID(cpu_transcoder), 0);
 
+	if (wait_for(hsw_audio_coded_ready(i915, cpu_transcoder), 100))
+		drm_dbg_kms(&i915->drm, "codec didn't power up\n");
+
 	/* Reset ELD address */
 	intel_de_rmw(i915, HSW_AUD_DIP_ELD_CTRL(cpu_transcoder),
 		     IBX_ELD_ADDRESS_MASK, 0);
@@ -917,11 +930,6 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
 		    encoder->base.base.id, encoder->base.name,
 		    pipe_name(pipe), drm_eld_size(crtc_state->eld));
 
-	if (i915->display.funcs.audio)
-		i915->display.funcs.audio->audio_codec_enable(encoder,
-								  crtc_state,
-								  conn_state);
-
 	mutex_lock(&i915->display.audio.mutex);
 	encoder->audio_connector = connector;
 
@@ -941,6 +949,11 @@ void intel_audio_codec_enable(struct intel_encoder *encoder,
 	intel_lpe_audio_notify(i915, pipe, port, crtc_state->eld,
 			       crtc_state->port_clock,
 			       intel_crtc_has_dp_encoder(crtc_state));
+
+	if (i915->display.funcs.audio)
+		i915->display.funcs.audio->audio_codec_enable(encoder,
+								  crtc_state,
+								  conn_state);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/display/intel_audio_regs.h b/drivers/gpu/drm/i915/display/intel_audio_regs.h
index 4f432c2eb543..69a09017f834 100644
--- a/drivers/gpu/drm/i915/display/intel_audio_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_audio_regs.h
@@ -25,6 +25,7 @@
 						  _IBX_AUD_CNTL_ST_B)
 #define   IBX_ELD_BUFFER_SIZE_MASK	REG_GENMASK(14, 10)
 #define   IBX_ELD_ADDRESS_MASK		REG_GENMASK(9, 5)
+#define   IBX_ELD_ADDRESS(dw)		REG_FIELD_PREP(IBX_ELD_ADDRESS_MASK, (dw))
 #define   IBX_ELD_ACK			REG_BIT(4)
 #define IBX_AUD_CNTL_ST2		_MMIO(0xE20C0)
 #define   IBX_CP_READY(port)		REG_BIT(((port) - 1) * 4 + 1)
-- 
2.35.1



More information about the Intel-gfx mailing list