Hi Nicolin,

On Jo, 2019-01-03 at 12:03 -0800, Nicolin Chen wrote:

Hi,

On Thu, Jan 03, 2019 at 03:56:46PM +0000, Viorel Suman wrote:

quoted

quoted

quoted

 sound/soc/fsl/fsl_amix.c                           | 554
+++++++++++++++++++++
 sound/soc/fsl/fsl_amix.h                           | 101 ++++

I aimn't against the naming here, but it seems to be AUDMIX in
RM?

Would it be better to align with that? It's your decision though.

To me "AUDMIX" sounds more like some RTL high level integration
module,
I would prefer to keep it as it is if there is no strong reason to 
rename it.

We had AUDMUX, so "AUDMIX" doesn't sound bad to me at all. The only
reason that we are discussing this is because RM uses more "AUDMIX"s
over "amix"s. I'd have chosen AUDMIX if I were you, yet not strongly
as I said. And it looks like Rob is asking you to use AUDMIX in DT
binding doc.

Ok, I'll change in the next version the compatible string to "audmix"
as Rob requested.

quoted

quoted

quoted

+Device driver configuration example:
+======================================
+  amix: amix@59840000 {
+    compatible = "fsl,imx8qm-amix";
+    reg = <0x0 0x59840000 0x0 0x10000>;
+    clocks = <&clk IMX8QXP_AUD_AMIX_IPG>;
+    clock-names = "ipg";
+    power-domains = <&pd_amix>;
+  };

From the description of DT and RM, I don't see how it connects to
SAIs.

Are they fixed to SAI0 and SAI1 in imx8qm? Wondering if it'd be
better to have such information in the doc.

Please check chapter "16.1.2.2 Audio Mixer" in RM: it has two
dedicated
SAI interfaces, SAI4 and SAI5. Audio Mixer operates on bit clock of
one
of these interfaces.

OK. I am actually more wondering how you connect it with SAI on
the software level: for imx8qm, SAI4/5 are used, but later SoCs
might use other SAI blocks. So it might be necessary to indicate
the connections in DT.

See below.

quoted

quoted

quoted

+static int fsl_amix_dai_set_fmt(struct snd_soc_dai *dai,
unsigned
int fmt)
+{
+       /* For playback the AMIX is slave, and for record is
master
*/
+       switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+       case SND_SOC_DAIFMT_CBM_CFM:
+       case SND_SOC_DAIFMT_CBS_CFS:

So it's used either for playback or capture only, not both at
same
time?

From IP functional perspective AMIX capture is the result of AMIX
playback - AMIX output represents the resulting mixed audio stream
routed to SAI4 RX signals (bit & frame clocks and data). So once we
have playback on either SAI4 or SAI5 (or both) - we can capture the
AMIX output on SAI4.

Ah, it sounds like a looping block then, receiving bclk from SAI4
-- slave, and routing the bclk back to SAI4 -- master; SAI4 works
at ASYNC mode?

Right. And yes, SAI4 works in ASYNC mode.

quoted

I guess it would be nice to send the machine driver as part of this
patchset also - it defines two input SAI interfaces as frontends
and
AMIX - as backend. Userspace sees only two SAI interfaces exposed,
both
of them having playback enabled, and only SAI4 having capture
enabled.

DPCM? So you are having the links in the sound DT nodes, i.e. the
machine driver?

The machine driver DT configuration looks like:
================
sound-amix-sai {
  compatible = "fsl,imx-audio-amix";
  model = "amix-audio-sai";
  dais = <&sai4>, <&sai5>;
  amix-controller = <&amix>;
};

The number of elements in "dais" array is at least two elements, DAPM
routes are generated dynamically in machine driver at probe as function
of number of SAI instances, the capture is enabled only for the first
SAI instance in the list.

Thanks
Nicolin

Regards,
Viorel