| Message ID | d4797282-d15e-4548-5804-77ab974ae5b7@sonarnerd.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi On Mon, Nov 28, 2016 at 10:55 AM, Jussi Laako <jussi@sonarnerd.net> wrote: > Hi, > >> Can you please sent it inline? Not having access at the hardware now >> but Antonio should have it > > > Sure! I have tested this with my hiFace... > > > - Jussi > > > From f4eec5602d86b5f938abed48a5725f59141d32cd Mon Sep 17 00:00:00 2001 > From: Jussi Laako <jussi@sonarnerd.net> > Date: Mon, 28 Nov 2016 01:11:46 +0200 > Subject: [PATCH] Fix M2Tech hiFace driver sampling rate change > > Sampling rate changes after first set one are not reflected to the > hardware, while driver and ALSA think the rate has been changed. > > Fix the problem by properly stopping the interface at the beginning of > prepare call, allowing new rate to be set to the hardware. This keeps > the hardware in sync with the driver. > > Signed-off-by: Jussi Laako <jussi@sonarnerd.net> > --- > sound/usb/hiface/pcm.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c > index 2c44139..33db205 100644 > --- a/sound/usb/hiface/pcm.c > +++ b/sound/usb/hiface/pcm.c > @@ -445,6 +445,8 @@ static int hiface_pcm_prepare(struct snd_pcm_substream > *alsa_sub) > > mutex_lock(&rt->stream_mutex); > > + hiface_pcm_stream_stop(rt); > + Ok, I was expecting stream already paused on STOP or SUSPEND call. I have the impression that they enqueue 0 urb still and they don't really stop it. Michael > sub->dma_off = 0; > sub->period_off = 0; > > -- > 2.7.4
On Mon, 28 Nov 2016 11:09:25 +0100, Michael Trimarchi wrote: > > Hi > > On Mon, Nov 28, 2016 at 10:55 AM, Jussi Laako <jussi@sonarnerd.net> wrote: > > Hi, > > > >> Can you please sent it inline? Not having access at the hardware now > >> but Antonio should have it > > > > > > Sure! I have tested this with my hiFace... > > > > > > - Jussi > > > > > > From f4eec5602d86b5f938abed48a5725f59141d32cd Mon Sep 17 00:00:00 2001 > > From: Jussi Laako <jussi@sonarnerd.net> > > Date: Mon, 28 Nov 2016 01:11:46 +0200 > > Subject: [PATCH] Fix M2Tech hiFace driver sampling rate change > > > > Sampling rate changes after first set one are not reflected to the > > hardware, while driver and ALSA think the rate has been changed. > > > > Fix the problem by properly stopping the interface at the beginning of > > prepare call, allowing new rate to be set to the hardware. This keeps > > the hardware in sync with the driver. > > > > Signed-off-by: Jussi Laako <jussi@sonarnerd.net> > > --- > > sound/usb/hiface/pcm.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c > > index 2c44139..33db205 100644 > > --- a/sound/usb/hiface/pcm.c > > +++ b/sound/usb/hiface/pcm.c > > @@ -445,6 +445,8 @@ static int hiface_pcm_prepare(struct snd_pcm_substream > > *alsa_sub) > > > > mutex_lock(&rt->stream_mutex); > > > > + hiface_pcm_stream_stop(rt); > > + > > Ok, I was expecting stream already paused on STOP or SUSPEND call. I > have the impression that they enqueue 0 urb still and they don't > really > stop it. The trigger callback is atomic, so it can't kill urb by itself, and hiface_pcm_trigger() just flips the sub->active flag. So, yes, calling hiface_pcm_stream_stop() should be correct. One question is whether it should be unconditionally called, or it should be done only when something got changed. I guess it's OK to call it always (it's safer), but it may lead to some glitches or such that wasn't present beforehand, when the player restarts the stream. thanks, Takashi
On Mon, 28 Nov 2016 15:11:25 +0100, Takashi Iwai wrote: > > On Mon, 28 Nov 2016 11:09:25 +0100, > Michael Trimarchi wrote: > > > > Hi > > > > On Mon, Nov 28, 2016 at 10:55 AM, Jussi Laako <jussi@sonarnerd.net> wrote: > > > Hi, > > > > > >> Can you please sent it inline? Not having access at the hardware now > > >> but Antonio should have it > > > > > > > > > Sure! I have tested this with my hiFace... > > > > > > > > > - Jussi > > > > > > > > > From f4eec5602d86b5f938abed48a5725f59141d32cd Mon Sep 17 00:00:00 2001 > > > From: Jussi Laako <jussi@sonarnerd.net> > > > Date: Mon, 28 Nov 2016 01:11:46 +0200 > > > Subject: [PATCH] Fix M2Tech hiFace driver sampling rate change > > > > > > Sampling rate changes after first set one are not reflected to the > > > hardware, while driver and ALSA think the rate has been changed. > > > > > > Fix the problem by properly stopping the interface at the beginning of > > > prepare call, allowing new rate to be set to the hardware. This keeps > > > the hardware in sync with the driver. > > > > > > Signed-off-by: Jussi Laako <jussi@sonarnerd.net> > > > --- > > > sound/usb/hiface/pcm.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c > > > index 2c44139..33db205 100644 > > > --- a/sound/usb/hiface/pcm.c > > > +++ b/sound/usb/hiface/pcm.c > > > @@ -445,6 +445,8 @@ static int hiface_pcm_prepare(struct snd_pcm_substream > > > *alsa_sub) > > > > > > mutex_lock(&rt->stream_mutex); > > > > > > + hiface_pcm_stream_stop(rt); > > > + > > > > Ok, I was expecting stream already paused on STOP or SUSPEND call. I > > have the impression that they enqueue 0 urb still and they don't > > really > > stop it. > > The trigger callback is atomic, so it can't kill urb by itself, and > hiface_pcm_trigger() just flips the sub->active flag. > > So, yes, calling hiface_pcm_stream_stop() should be correct. > One question is whether it should be unconditionally called, or it > should be done only when something got changed. > > I guess it's OK to call it always (it's safer), but it may lead to > some glitches or such that wasn't present beforehand, when the player > restarts the stream. And I forgot this patch until now, sorry. I applied it now for 4.10-rc1. thanks, Takashi
diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c index 2c44139..33db205 100644 --- a/sound/usb/hiface/pcm.c +++ b/sound/usb/hiface/pcm.c @@ -445,6 +445,8 @@ static int hiface_pcm_prepare(struct snd_pcm_substream *alsa_sub) mutex_lock(&rt->stream_mutex); + hiface_pcm_stream_stop(rt); + sub->dma_off = 0; sub->period_off = 0;