Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: "Sridharan, Ranjani" <ranjani.sridharan@intel.com>
Cc: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	Linux-ALSA <alsa-devel@alsa-project.org>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Subject: Re: [alsa-devel] [PATCH 7/8] ALSA: pcm: Add card sync_irq field
Date: Thu, 21 Nov 2019 22:13:27 +0100
Message-ID: <s5hftihrkzs.wl-tiwai@suse.de> (raw)
In-Reply-To: <CAFQqKeUimt32z1myzL+JOXoCUhn7jU1=Mj8e6egQKs+c5CY0pA@mail.gmail.com>

On Thu, 21 Nov 2019 21:46:17 +0100,
Sridharan, Ranjani wrote:
> 
>     >
>     > Hi Takashi,
>     >
>     > Sorry the stress tests took a while. 
>     > As we discussed earlier, adding the sync_stop() op didnt quite help the
>     SOF
>     > driver in removing the delayed work for snd_pcm_period_elapsed(). 
>    
>     Yeah, that's understandable.  If the stop operation itself needs some
>     serialization, sync_stop() won't influence at all.
>    
>     However, now after these discussions, I have some concerns in the
>     current code:
>    
>     - The async work started by schedule_work() may be executed
>       (literally) immediately.  So if the timing or the serialization
>       matters, it doesn't guarantee at all.  The same level of concurrency
>       can happen at any time.
>    
>     - The period_elapsed work might be pending at prepare or other
>       operation;
>       the async work means also that it doesn't guarantee its execution in
>       time, and it might be delayed much, and the PCM core might go to
>       prepare or other state even before the work is executed.
>    
>     The second point can be fixed easily now with sync_stop.  You can just
>     put flush_work() in sync_stop in addition to synchronize_irq().
>    
>     But the first point is still unclear.  More exactly, which operation
>     does it conflict?  Does it the playback drain?  Then it might take
>     very long (up to seconds) to block the next operation?
> 
> Hi Takashi,
> 
> As I understand the original intention for adding the period_elapsed_work()
> was  that snd_pcm_period_elapsed() could cause a STOP trigger while the
> current IPC interrupt is still being handled.
> In this case, the STOP trigger generates an IPC to the DSP but the host never
> misses the IPC response from the DSP because it is still handling the previous
> interrupt.

OK, that makes sense.  So the issue is that the trigger stop itself
requires the ack via the interrupt and it can't be caught because it's
being called from the irq handler itself.

In that case, though, another solution would be to make the trigger-
stop an async work (but conditionally) while processing the normal
period_elapsed in the irq handler.  That is, set some flag before
calling snd_pcm_period_elapsed(), and in the trigger-stop, check the
flag.  If the flag is set, schedule the work and return.  And, you'll
sync this async work with sync_stop().  In that way, the period
handling is processed without any delay more lightly.


thanks,

Takashi

> Adding Keyon who added this change to add more and clarify your concerns.
> 
> Thanks,
> Ranjani
> 
>     thanks,
>    
>     Takashi
> 
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

  reply index

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-17  8:53 [alsa-devel] [PATCH 0/8] ALSA: pcm: API cleanups and extensions Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 1/8] ALSA: pcm: Introduce managed buffer allocation mode Takashi Iwai
2019-11-18 16:24   ` Pierre-Louis Bossart
2019-11-18 18:46     ` Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 2/8] ALSA: docs: Update for " Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 3/8] ALSA: pcm: Allow NULL ioctl ops Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 4/8] ALSA: docs: Update document about the default PCM " Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 5/8] ALSA: pcm: Move PCM_RUNTIME_CHECK() macro into local header Takashi Iwai
2019-11-17  9:42   ` kbuild test robot
2019-11-17 10:05     ` Takashi Iwai
2019-11-17 10:28   ` kbuild test robot
2019-11-17  8:53 ` [alsa-devel] [PATCH 6/8] ALSA: pcm: Add the support for sync-stop operation Takashi Iwai
2019-11-18 16:33   ` Pierre-Louis Bossart
2019-11-18 18:47     ` Takashi Iwai
2019-11-17  8:53 ` [alsa-devel] [PATCH 7/8] ALSA: pcm: Add card sync_irq field Takashi Iwai
2019-11-18 16:38   ` Pierre-Louis Bossart
2019-11-18 18:52     ` Takashi Iwai
2019-11-18 19:20       ` Sridharan, Ranjani
2019-11-18 19:49         ` Takashi Iwai
2019-11-18 19:55           ` Sridharan, Ranjani
2019-11-18 20:40             ` Takashi Iwai
2019-11-18 23:47               ` Ranjani Sridharan
2019-11-19  6:44                 ` Takashi Iwai
2019-11-19  7:40                   ` Ranjani Sridharan
2019-11-19  8:24                     ` Takashi Iwai
2019-11-19  9:39                       ` Takashi Iwai
2019-11-19 16:36                       ` Ranjani Sridharan
2019-11-19 21:27                         ` Takashi Iwai
2019-11-19 21:43                           ` Sridharan, Ranjani
2019-11-21 19:22                             ` Sridharan, Ranjani
2019-11-21 20:34                               ` Takashi Iwai
2019-11-21 20:46                                 ` Sridharan, Ranjani
2019-11-21 21:13                                   ` Takashi Iwai [this message]
2019-11-21 21:17                                     ` Sridharan, Ranjani
2019-11-21 21:28                                       ` Takashi Iwai
2019-11-21 21:45                                         ` Sridharan, Ranjani
2019-11-22  4:08                                     ` Jie, Yang
2019-11-17  8:53 ` [alsa-devel] [PATCH 8/8] ALSA: docs: Update about the new PCM sync_stop ops Takashi Iwai

Reply instructions:

You may reply publically 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=s5hftihrkzs.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=ranjani.sridharan@intel.com \
    --cc=ranjani.sridharan@linux.intel.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

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git