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

On Tue, 19 Nov 2019 09:24:33 +0100,
Takashi Iwai wrote:
> 
> On Tue, 19 Nov 2019 08:40:25 +0100,
> Ranjani Sridharan wrote:
> > 
> > > > 
> > > > Hi Takashi,
> > > > 
> > > > I just realized that In the SOF driver, we only set the component
> > > > driver ops. The pcm ops are set when creating the new pcm. So,
> > > > should I
> > > > also add the sync_stop op in the component driver and set the pcm
> > > > sync_stop op to point to the component sync_stop op? Just wanted to
> > > > confirm if I am on the right track.
> > > 
> > > Yes, I didn't touch this yet, but that's the way to go I suppose.
> > > One caveat is that this ops is optional and needs NULL as default,
> > > hence you'd need to set only when defined, like copy_user, page or
> > > mmap ops, at least.
> > Hi Takashi,
> > 
> > This is what I tried in the SOF driver:
> > https://github.com/thesofproject/linux/pull/1513/commits
> > 
> > And it seems to cause the system to hang when I stop the stream and I
> > have no meaningful logs to pinpoint to the problem. Could you please
> > have a look at the 4 commits that I have added to your series and let
> > me know what I could be missing? 
> 
> I couldn't find anything obvious.  Could you try without changing
> snd_sof_pcm_period_elapsed(), i.e. only adding the stuff and calling
> sync_stop, in order to see whether the additional stuff broke
> anything?

... and looking at the code again, I don't think the stop_sync ops
would help in your case.  The PCM ops is called *after* trigger-STOP
is issued, while your case requires the serialization of trigger call
itself.

But I'm still wondering whether the current implementation is safe,
too.  Basically the scheduled work may be executed immediately after
queuing, so if it's about the timing issue, it's not solid solution.
Pushing to work helps if it's about the locking issue, though.


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 [this message]
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
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=s5himngxl1a.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