All of lore.kernel.org
 help / color / mirror / Atom feed
From: Caleb Crome <caleb@crome.org>
To: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Arnaud Mouiche <arnaud.mouiche@invoxia.com>,
	Timur Tabi <timur@tabi.org>, Mark Brown <broonie@kernel.org>,
	Max Krummenacher <max.krummenacher@toradex.com>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	Fabio Estevam <festevam@gmail.com>,
	Sascha Hauer <kernel@pengutronix.de>
Subject: Re: [PATCH v2] ASoC: fsl_ssi: Fix channel swap on playback start
Date: Mon, 3 Apr 2017 16:31:59 -0700	[thread overview]
Message-ID: <CAG5mAdxDiH4_FVqT5fC+PFGZ108GQCZiLLhja8xGRK0RdWo3oQ@mail.gmail.com> (raw)
In-Reply-To: <20170403220811.GA21156@Asurada-Nvidia>

On Mon, Apr 3, 2017 at 3:08 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote:
> On Mon, Apr 03, 2017 at 01:32:42PM -0700, Caleb Crome wrote:
>
>> This patch definitely breaks the i.mx6 channel alignment.  In fact it
>> breaks it so that the channels are never aligned properly.
>>
>> My test setup is as follows:
>> * Get vanilla kernel, tag v4.11-rc5
>> * apply a couple patches to allow AUD4 on the wandboard external
>> connectors (and disable internal audio)
>> * Test results:  v4.11-rc5 works flawlessly using Arnaud's atest
>> program.  No channel slips, no issues at all.
>> * Apply this patch, recompile, build.
>> * Channel alignment fails.  The channels never get aligned properly.
>
> What's your test case for the alignment?

I'm not sure what you are asking.  The test case I'm testing is:
connect SSI to AUD4 on wandboard & physically connect TX -> RX.  (as
per https://github.com/ccrome/linux-caleb-dev/wiki), then use atest to
verify bit-perfection of TX->RX transmission.

Also, you must use a scope on the pins to verify that TX is also in
the right location (i.e. it's possible that bot TX and RX are rotated
by the same number of samples, thus you could be fooled by a
software-only test).

> IIRC, you only needed
> the FIFO to be pre-filled with data input so that SSI will not
> encounter any FIFO underrun after enabling TE bit. That's why
> there is a for-loop before this regmap_update_bits().

Well, there was more to it than that IIRC.  There were several patches
that made it all hang together.

>
>> Am I right that the *only* change is this one-liner, and ignore the
>> previous non V2 version of this patch?
>> > -               regmap_update_bits(regs, CCSR_SSI_SCR, vals->scr, vals->scr);
>> > +               regmap_update_bits(regs, CCSR_SSI_SCR,
>> > +                       CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE,
>> > +                       CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_TE | CCSR_SSI_SCR_RE);
>
> However, this patch seems to merely set the RE bit. It shouldn't
> affect that test case since the SSIEN bit is still set prior to
> the TE bit.

Heh, well, this patch causes audio to be utterly broken on
multi-channel audio :-/

-Caleb

  reply	other threads:[~2017-04-03 23:32 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-01 14:48 Fabio Estevam
2017-04-01 15:13 ` Arnaud Mouiche
2017-04-01 16:03   ` Fabio Estevam
2017-04-03  8:19 ` Max Krummenacher
2017-04-03 20:32 ` Caleb Crome
2017-04-03 20:50   ` Caleb Crome
2017-04-03 21:53   ` Fabio Estevam
2017-04-03 22:05     ` Arnaud Mouiche
2017-04-03 22:20       ` Nicolin Chen
2017-04-03 23:22     ` Caleb Crome
2017-04-03 23:40       ` Fabio Estevam
2017-04-03 23:42         ` Caleb Crome
2017-04-04  8:59           ` Arnaud Mouiche
2017-04-04  9:03             ` Arnaud Mouiche
2017-04-04 11:38             ` Fabio Estevam
2017-04-04 17:12               ` Fabio Estevam
2017-04-04 20:09                 ` Arnaud Mouiche
2017-04-04 20:28                   ` Fabio Estevam
2017-04-05  7:54                     ` Arnaud Mouiche
2017-04-05 13:43                       ` Fabio Estevam
2017-04-05 14:04                       ` Max Krummenacher
2017-04-03 22:08   ` Nicolin Chen
2017-04-03 23:31     ` Caleb Crome [this message]
2017-04-03 23:55       ` Nicolin Chen
2017-04-04  0:07         ` Timur Tabi
2017-04-04  0:39         ` Caleb Crome
2017-04-04  1:25           ` Nicolin Chen
2017-04-03 22:36 ` Nicolin Chen
2017-04-03 22:54   ` Fabio Estevam
2017-04-04  0:08     ` Nicolin Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAG5mAdxDiH4_FVqT5fC+PFGZ108GQCZiLLhja8xGRK0RdWo3oQ@mail.gmail.com \
    --to=caleb@crome.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=arnaud.mouiche@invoxia.com \
    --cc=broonie@kernel.org \
    --cc=fabio.estevam@nxp.com \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=max.krummenacher@toradex.com \
    --cc=nicoleotsuka@gmail.com \
    --cc=timur@tabi.org \
    --subject='Re: [PATCH v2] ASoC: fsl_ssi: Fix channel swap on playback start' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.