Re: [PATCH 08/18] ASoC: SOF: Intel: hda-mlink: introduce helpers for 'extended links' PM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/27/2023 1:29 PM, Peter Ujfalusi wrote:
From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>

Add helpers to program SPA/CPA bits, using a mutex to access the
shared LCTL register if required.

All links are managed with the same LCTLx.SPA bits. However there are
quite a few implementation details to be aware of:

Legacy HDaudio multi-links are powered-up when exiting reset, which
requires the ref_count to be manually set to one when initializing the
link.

Alternate links for SoundWire/DMIC/SSP need to be explicitly
powered-up before accessing the SHIM/IP/Vendor-Specific SHIM space for
each sublink. DMIC/SSP/SoundWire are all different cases with a
different device/dai/hlink relationship.

SoundWire will handle power management with the auxiliary device
resume/suspend routine. The ref_count is not necessary in this case.

The DMIC/SSP will by contrast handle the power management from DAI
.startup and .shutdown callbacks.

The SSP has a 1:1 mapping between sublink and DAI, but it's
bidirectional so the ref_count will help avoid turning off the sublink
when one of the two directions is still in use.

The DMIC has a single link but two DAIs for data generated at
different sampling frequencies, again the ref_count will make sure the
two DAIs can be used concurrently.

And last the SoundWire Intel require power-up/down and bank switch to
be handled with a lock already taken, so the 'eml_lock' is made
optional with the _unlocked versions of the helpers.

Note that the _check_power_active() implementation is similar to
previous helpers in sound/hda/ext, with sleep duration and timeout
aligned with hardware recommendations. If desired, this helper could
be modified in a second step with .e.g. readl_poll_timeout()

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
Reviewed-by: Rander Wang <rander.wang@xxxxxxxxx>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@xxxxxxxxxxxxxxx>
---
  include/sound/hda-mlink.h       |  32 +++++++
  sound/soc/sof/intel/hda-mlink.c | 163 ++++++++++++++++++++++++++++++++
  2 files changed, 195 insertions(+)


...

diff --git a/sound/soc/sof/intel/hda-mlink.c b/sound/soc/sof/intel/hda-mlink.c
index 90b68ae2564c..4cfef4007d0c 100644
--- a/sound/soc/sof/intel/hda-mlink.c
+++ b/sound/soc/sof/intel/hda-mlink.c
@@ -170,6 +170,68 @@ static int hdaml_lnk_enum(struct device *dev, struct hdac_ext2_link *h2link,
  	return 0;
  }
+/*
+ * Hardware recommendations are to wait ~10us before checking any hardware transition
+ * reported by bits changing status.
+ * This value does not need to be super-precise, a slack of 5us is perfectly acceptable.
+ * The worst-case is about 1ms before reporting an issue
+ */
+#define HDAML_POLL_DELAY_MIN_US 10
+#define HDAML_POLL_DELAY_SLACK_US 5
+#define HDAML_POLL_DELAY_RETRY  100
+
+static int check_power_active(u32 __iomem *lctl, int sublink, bool enable)

Should last argument be named 'active' instead of 'enable'? It would make more sense to me.

+{
+	int mask = BIT(sublink) << AZX_ML_LCTL_CPA_SHIFT;
+	int retry = HDAML_POLL_DELAY_RETRY;
+	u32 val;
+
+	usleep_range(HDAML_POLL_DELAY_MIN_US,
+		     HDAML_POLL_DELAY_MIN_US + HDAML_POLL_DELAY_SLACK_US);
+	do {
+		val = readl(lctl);
+		if (enable) {
+			if (val & mask)
+				return 0;
+		} else {
+			if (!(val & mask))
+				return 0;
+		}
+		usleep_range(HDAML_POLL_DELAY_MIN_US,
+			     HDAML_POLL_DELAY_MIN_US + HDAML_POLL_DELAY_SLACK_US);
+
+	} while (--retry);
+
+	return -EIO;
+}
+

...



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux