All of lore.kernel.org
 help / color / mirror / Atom feed
From: Troy Kisky <troy.kisky@boundarydevices.com>
To: "Ambrose, Martin" <martin@ti.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>
Subject: Re: [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown
Date: Fri, 24 Sep 2010 12:14:32 -0700	[thread overview]
Message-ID: <4C9CF898.7070906@boundarydevices.com> (raw)
In-Reply-To: <A24693684029E5489D1D202277BE89447289CD0A@dlee02.ent.ti.com>

On 9/24/2010 9:48 AM, Ambrose, Martin wrote:
> On Sat, Jul 04, 2009 at 21:29:59, Troy Kisky wrote:
> 
>> If the codec is master then prepare should call
>> mcbsp_start, not trigger.
> 
> For me this change induces audio artifacts when starting or stopping a gstreamer pipeline.
> Take, e.g., the pipeline
> 	gst-launch -v filesrc location=file.mp3 ! mad ! alsasink
> 
> In the setup, before the file is opened, gstreamer places the pipeline into the PAUSE state.
> 	... posting state-changed READY to PAUSED
> 
> This results in a call to the davinci_i2s_prepare function which, when the codec is master,
> stops then starts the mcbsp driver. However at this point in the pipeline no valid data has
> been provided to the DMA layer (davinci-pcm). Consequently there is random noise of somewhat
> random length played out the DAC at the start of file playback. Moreover when the stream changes
> from the PAUSE to the PLAYING state another mcbsp_stop/mcbsp_start sequence is performed which
> seems inefficient. Similar behavior, and noise, occurs when the pipeline is shutdown.
> 
> The aplay utility has a simpler startup/shutdown procedure which doesn't produce any noise.
> 
> There doesn't seem to be any state context, e.g. pause vs start, provided to the prepare
> function so I can't see how to handle such cases in the current driver while preserving
> the use of this patch. If I revert this patch then I don't get any noise but I don't 
> know what the consequences are. 
> 
> Comments/suggestions welcome.
> 

I don't see the problem because my codec waits until trigger to activate its interface.

So the question is, who should be responsible for the final turn on?

My thought was that the device (master) which starts the external wires to wiggling should be last.

If the codec is master, and starts the clocks before the mcbsp is ready that could also cause a pop
or noise.


It would be nice if such rules could be documented, are they???

Thanks
Troy

  reply	other threads:[~2010-09-24 19:14 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-05  2:29 davinci-i2c,pcm updates Troy Kisky
2009-07-05  2:29 ` [PATCH V1 01/11] ASoC: DaVinci: i2s, remove MOD_REG_BIT macro Troy Kisky
2009-07-05  2:29   ` [PATCH V1 02/11] ASoC: DaVinci: i2s toggle clock to complete reset Troy Kisky
2009-07-05  2:29     ` [PATCH V1 03/11] ASoc: DaVinci: i2s, minor cleanup Troy Kisky
2009-07-05  2:29       ` [PATCH V1 04/11] ASoC: DaVinci: i2s cleanup Troy Kisky
2009-07-05  2:29         ` [PATCH V1 05/11] ASoC: DaVinci: i2s, only start sample generator if needed Troy Kisky
2009-07-05  2:29           ` [PATCH V1 06/11] ASoC: DaVinci: i2s, minor cleanup of davinci_i2s_startup Troy Kisky
2009-07-05  2:29             ` [PATCH V1 07/11] ASoC: DaVinci: i2s, fix mcbsp_word_length update Troy Kisky
2009-07-05  2:29               ` [PATCH V1 08/11] ASoc: DaVinci: i2s, Improve underrun, support mono Troy Kisky
2009-07-05  2:29                 ` [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown Troy Kisky
     [not found]                   ` <1246761001-21982-10-git-send-email-troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
2009-07-05  2:30                     ` [PATCH V1 10/11] ASoC: DaVinci: i2s don't limit rates Troy Kisky
2009-07-05  2:30                       ` [PATCH V1 11/11] ASoC: DaVinci: pcm, fix underruns by using sram Troy Kisky
2009-07-05 13:03                         ` Mark Brown
2009-07-07  1:10                           ` Troy Kisky
2009-07-07  9:31                             ` Mark Brown
2009-07-07 19:07                               ` Troy Kisky
2009-07-07 19:14                                 ` Troy Kisky
2009-07-07 19:21                                   ` Troy Kisky
2009-07-05 11:57                       ` [PATCH V1 10/11] ASoC: DaVinci: i2s don't limit rates Mark Brown
2009-07-06 22:01                         ` Troy Kisky
2009-07-06 22:19                           ` Mark Brown
2009-07-06 22:27                             ` Troy Kisky
2009-07-05 12:17                   ` [PATCH V1 09/11] ASoC: DaVinci: i2s, add davinci_i2s_prepare and shutdown Mark Brown
2010-09-24 16:48                   ` Ambrose, Martin
2010-09-24 19:14                     ` Troy Kisky [this message]
2010-09-24 19:43                       ` Ambrose, Martin
2010-09-24 23:13                         ` Troy Kisky
2010-09-27  0:56                         ` Mark Brown
2010-09-27 18:50                           ` Troy Kisky
2010-09-27 20:35                             ` Mark Brown
2010-09-27  0:27                       ` Mark Brown
2009-07-05 12:12                 ` [PATCH V1 08/11] ASoc: DaVinci: i2s, Improve underrun, support mono Mark Brown
     [not found]                 ` <1246761001-21982-9-git-send-email-troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
2009-07-06 11:09                   ` Steve Chen
2009-07-06 11:54                     ` Mark Brown
     [not found]                       ` <20090706115442.GA8925-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2009-07-06 12:45                         ` Steve Chen
2009-07-06 13:26                       ` snd_pcm_format_set_silence parameter Guilherme Longo
2009-07-06 13:52                         ` Clemens Ladisch
2009-07-05 11:41 ` davinci-i2c,pcm updates Mark Brown
2009-07-05 12:03   ` performance between access mothods! Guilherme Longo
2009-07-07 11:26     ` Takashi Iwai
2009-07-06 21:30   ` davinci-i2c,pcm updates Troy Kisky
2009-07-06 21:41     ` Mark Brown
2009-07-06 22:51       ` Kevin Hilman
2009-07-07  9:20         ` Mark Brown
2009-07-06 21:47   ` Kevin Hilman

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=4C9CF898.7070906@boundarydevices.com \
    --to=troy.kisky@boundarydevices.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=martin@ti.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.