All of lore.kernel.org
 help / color / mirror / Atom feed
* PandaBoard ES Audio Problems
@ 2013-01-31 16:51 Brent Weatherall
  2013-01-31 17:28 ` Clemens Ladisch
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Brent Weatherall @ 2013-01-31 16:51 UTC (permalink / raw)
  To: alsa-devel

Hello.

     While developing with OpenMax and ALSA on the PandaBoard ES (OMAP 4460
armhf), I ran into a strange problem.

At a high level, when configuring the built in audio device (3.5 mm jack),
the device accepts ANY rate requested (via snd_pcm_set_rate_near).  it
accepts unsupported rates without alteration.  If the rate is actually not
supported, then 'snd_pcm_hw_params' fails with error '-22'.  After this
happens, it is not possible to reconfigure the rate without closing the
device and starting over to completely re-configure from the start (calling
snd_pcm_open, etc...).  It is not possible to simply try another rate.
 Technical details below:

When 'snd_pcm_set_rate_near' is called, during the min and max rate
probing, the code reaches

interval_inline.h:71      INTERVAL_INLINE int snd_interval_min(const
snd_interval_t *i)

During this call, line '74              return i->min;' executes.  This
function ALWAYS returns the same rate value requested in
'snd_pcm_set_rate_near' as the minimum possible rate.  If I request a rate
near 44100, the minimum rates comes back as 44100.  If I request a rate
near 8000, the minimum is returned as 8000 and so forth.

To recap, the 'snd_pcm_set_rate_near' call ALWAYS succeeds without altering
the rate passed to it the first time.  The only way to find out if the rate
is actually supported is to call 'snd_pcm_hw_params' and then see if that
fails with err == -22 (invalid argument).  Furthermore, once the failure
occurs, another rate value cannot be tried unless the PCM device is closed
and the entire configuration process is repeated.  After the failure,
subsequent calls to 'snd_pcm_set_rate_near' with ANY different value always
get set back to the original bad value.  Example, I request 44100 and it
fails.  So I try 8000 and it returns 44100 (and of course fails again if I
try 'snd_pcm_hw_params').

I am not sure yet, but is the value at 'interval_inline.h:74
 return i->min;' determined directly by the hardware?  It seems like it is,
but I don't know the code well enough yet to make that assertion.

I also haven't yet examined the 'snd_interval_max' command, but my gut
tells me it is behaving the same way, since after a bad rate is set, any
higher or lower rate setting attempts ALWAYS come back as the original
rate.  How would i determine if it is hardware or maybe something bad with
the hardware.software interface?

I checked the PCM state value and it was correct the entire time.

Here are short snippets of two gdb sessions where I found the issue and
below that is test program output showing the state values as I try to
configure two different rates:

Code snippet in ALSA lib file interval_inline.h:

71      INTERVAL_INLINE int snd_interval_min(const snd_interval_t *i)
72      {
73              assert(!snd_interval_empty(i));
74              return i->min;
75      }

Run #1 with 44100 'snd_pcm_set_rate_near' call:
(gdb) s
74              return i->min;
(gdb) print i->min
$6 = 44100

Run #2 with 8000 'snd_pcm_set_rate_near' call:
(gdb) s
74              return i->min;
(gdb) print i->min
$8 = 8000

Here is a log of the high level test program:

snd_pcm_hw_params_set_rate_near succeeded.
Rate set to 44100.
snd_pcm_hw_params_set_channels succeeded.
cannot set buffer time (Invalid argument)
cannot set buffer period time (Invalid argument)
Before commit, state is: SND_PCM_STATE_OPEN
cannot set parameters (Invalid argument)
After failure, state is: SND_PCM_STATE_OPEN
Trying to re-configure rate param.
Before calling 'snd_pcm_drop', state is: SND_PCM_STATE_OPEN
cannot drop (Input/output error)
After calling 'snd_pcm_drop', state is: SND_PCM_STATE_OPEN
Trying rate 8000.
snd_pcm_hw_params_set_rate_near succeeded.
Rate set to 44100.
snd_pcm_hw_params_set_buffer_time_near succeeded.
Buffer size actually set to 22050.
cannot set buffer period time (Invalid argument)
cannot set parameters (Invalid argument)
cannot prepare audio interface for use (Input/output error)
Rate 44100 supported: FAIL

Please let me know your thoughts on this and if there is a good way to
prove if this is a hardware error or a software error.  I am running 'Linux
version 3.4.0-1487-omap4' and using the standard repositories with apt-get
to install ALSA.  I used 'apt-get source' to obtain 'alsa-lib-1.0.25'
source files and I am running 'libasound2-dbg' to enable GDB debugging.

Regards,

Brent

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 16:51 PandaBoard ES Audio Problems Brent Weatherall
@ 2013-01-31 17:28 ` Clemens Ladisch
  2013-01-31 18:23   ` Liam Girdwood
  2013-02-01 12:53 ` Peter Ujfalusi
  2013-02-06 15:10 ` [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard Peter Ujfalusi
  2 siblings, 1 reply; 18+ messages in thread
From: Clemens Ladisch @ 2013-01-31 17:28 UTC (permalink / raw)
  To: Brent Weatherall; +Cc: alsa-devel

Brent Weatherall wrote:
> At a high level, when configuring the built in audio device (3.5 mm jack),
> the device accepts ANY rate requested (via snd_pcm_set_rate_near).  it
> accepts unsupported rates without alteration.  If the rate is actually not
> supported, then 'snd_pcm_hw_params' fails with error '-22'.

This sounds as if the driver published incorrect constraints.

Do you have a pointer to the driver's source code?


Regards,
Clemens

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 17:28 ` Clemens Ladisch
@ 2013-01-31 18:23   ` Liam Girdwood
  2013-01-31 19:24     ` Brent Weatherall
  0 siblings, 1 reply; 18+ messages in thread
From: Liam Girdwood @ 2013-01-31 18:23 UTC (permalink / raw)
  To: Brent Weatherall; +Cc: Peter Ujfalusi, alsa-devel, Clemens Ladisch

On Thu, 2013-01-31 at 18:28 +0100, Clemens Ladisch wrote:
> Brent Weatherall wrote:
> > At a high level, when configuring the built in audio device (3.5 mm jack),
> > the device accepts ANY rate requested (via snd_pcm_set_rate_near).  it
> > accepts unsupported rates without alteration.  If the rate is actually not
> > supported, then 'snd_pcm_hw_params' fails with error '-22'.
> 
> This sounds as if the driver published incorrect constraints.
> 
> Do you have a pointer to the driver's source code?
> 

The OMAP4 ABE driver doesn't actually know the exact constraints if
there is not a valid playback path between source PCM and sink (it will
still throw out anything insane though). This is because it can route
audio from most of it's PCMs to most of it's components, e.g. HS, HF,
BT, MODEM, Earpiece, etc where some components have different
constraints. It's possible that you dont have a path in your case.

Can you check you have enabled a valid playback path with
alsaucm/alsamixer and then test with aplay. This should help narrow down
the issue.

I've CC'ed Peter since 3.4 is a little old and he is working on the
latest OMAP4 codebase.

Regards

Liam

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 18:23   ` Liam Girdwood
@ 2013-01-31 19:24     ` Brent Weatherall
  2013-01-31 19:44       ` Clemens Ladisch
                         ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Brent Weatherall @ 2013-01-31 19:24 UTC (permalink / raw)
  To: Liam Girdwood; +Cc: Peter Ujfalusi, alsa-devel, Clemens Ladisch

Clemens/Liam,

     Liam, I address your question below.  Clemens, stepping through the
ALSA code, it seems the error *might* be in the ALSA code, but I am
probably reading the code incorrectly:

pcm/interval.c:

105 int snd_interval_refine_min(snd_interval_t *i, unsigned int min, int
openmin)
106 {
107         int changed = 0;
108         if (snd_interval_empty(i))
109                 return -ENOENT;
110         if (i->min < min) {
111                 i->min = min;  <-- Is this statement reversed or do I
not understand the code well enough?
112                 i->openmin = openmin;
113                 changed = 1;
114         } else if (i->min == min && !i->openmin && openmin) {
115                 i->openmin = 1;
116                 changed = 1;
117         }
118         if (i->integer) {
119                 if (i->openmin) {
120                         i->min++;
121                         i->openmin = 0;
122                 }
123         }
124         if (snd_interval_checkempty(i)) {
125                 snd_interval_none(i);
126
127         }
128         return changed;
129 }
130

Lines 110 - 113 appear to check if the hw params returned structure's
minimum value (i->min) is less than the current min value (which is set to
the requested rate in pcm.c:827  min = max = best).  Then if true, it
appears to set the hardware reported min value to the current min, instead
of the other way around.  Maybe I am misunderstanding the code, but that
appears to be what is happening.  I haven't stepped past the
'snd_interval_refine_min' function just yet.  Another GDB session snippet.
 I left out GDB portions that were not important:

pcm_params.c:'snd_pcm_hw_param_set_near':815:
 825         if (best > INT_MAX)
 826                 best = INT_MAX;
 827         min = max = best;

GDB session:
_snd_pcm_hw_param_set_min (params=0x13248, var=SNDRV_PCM_HW_PARAM_RATE,
val=44100, dir=0) at pcm_params.c:412
412             else if (hw_is_interval(var))
_snd_pcm_hw_param_set_min (params=0x13248, var=SNDRV_PCM_HW_PARAM_RATE,
val=44100, dir=0) at pcm_params.c:412
413                     changed =
snd_interval_refine_min(hw_param_interval(params,
var), val, openmin);
snd1_interval_refine_min (i=0x13370, min=44100, openmin=0) at interval.c:106
(gdb) p {snd_interval_t} i <-- I interpreted this to be from the device.
 Correct or no?
$24 = {min = 4000, max = 4294967295, openmin = 0, openmax = 1, integer = 0,
empty = 0}
(gdb) s
snd1_interval_refine_min (i=0x13370, min=44100, openmin=0) at interval.c:108
108             if (snd_interval_empty(i))
(gdb) s
110             if (i->min < min) {
(gdb) p min
$25 = 44100
(gdb) s
112                     i->openmin = openmin;
(gdb) p i->min
$26 = 4000
(gdb) p min
$27 = 44100
(gdb) s
111                     i->min = min;
(gdb) s
112                     i->openmin = openmin;
(gdb) p i->min
$28 = 44100
(gdb)

I just seems like it's setting the min value to what was requested instead
of what was in the snd_interval_t structure.  But maybe that is intended.
 Maybe I missed something higher in the calling tree.

Liam,

     Thank you for the information.

>Can you check you have enabled a valid playback path with
>alsaucm/alsamixer and then test with aplay. This should help narrow down
>the issue.

When I started this project, I used the alsamixer to verify the sound
devices and so forth.  There appears to be nothing abnormal as far as I can
tell.  Several devices exist and have changeable values.  There are PandaES
devices and HDMI devices (as also seen with aplay/acrecord -L).  I was able
to record and play back all the following wav files with arecord/aplay:

11025_capt.wav  12000_capt.wav  16000_capt.wav  22050_capt.wav
 24000_capt.wav  32000_capt.wav  44100_capt.wav  48000_capt.wav
 8000_capt.wav with the 'default' device at card 0, device 0.

The number in the file name being the rate it was recorded at.

I found the current issue when I tried to use the Bellagio OpenMax-ALSA
component to playback audio.  I ran the 'omxcapnplay' test routine (from
the libomxalsa library 'test' directory) and it kept failing with 'invalid
argument'.  After digging into the details I found the OMX constructor sets
a default rate of 44100.  I wrote test code to recreate the problem and
found the current issue (asking for 44100 with set_rate_near succeeds, but
when the params are committed it fails).  Then you cannot re-configure
without closing the device.

I also found that 'aplay' can playback any rate I want, BUT when I use code
to open the PCM device I can ONLY open it with rates of 8000, 12000, 16000,
24000 and 48000 AND this is only if use the 'set_buffer_time_near' call
before trying to commit.  Otherwise I can ONLY set 8000 when using the
minimum number of 'snd_pcm*' configuration calls (see OpenMax
ALSA component SetParams function for reference).  Oddly enough
'speaker-test' cannot playback 48000 Hz even though I can open the device
with a rate of 48000 Hz.

Also, alsaloop works without issues.

If it would be helpful, I can post my test program source and the various
outputs I get when running with the rates I mentioned above.
For playback I am using 'default' which is card 0, device 0.

Thank you for taking the time to look this over.  Is there an aplay command
that might uncover if there is a valid path to the device, or does the
ability to play sound with aplay demonstrate the presence of the path?

Regards,

Brent

On Thu, Jan 31, 2013 at 10:23 AM, Liam Girdwood <
liam.r.girdwood@linux.intel.com> wrote:

> On Thu, 2013-01-31 at 18:28 +0100, Clemens Ladisch wrote:
> > Brent Weatherall wrote:
> > > At a high level, when configuring the built in audio device (3.5 mm
> jack),
> > > the device accepts ANY rate requested (via snd_pcm_set_rate_near).  it
> > > accepts unsupported rates without alteration.  If the rate is actually
> not
> > > supported, then 'snd_pcm_hw_params' fails with error '-22'.
> >
> > This sounds as if the driver published incorrect constraints.
> >
> > Do you have a pointer to the driver's source code?
> >
>
> The OMAP4 ABE driver doesn't actually know the exact constraints if
> there is not a valid playback path between source PCM and sink (it will
> still throw out anything insane though). This is because it can route
> audio from most of it's PCMs to most of it's components, e.g. HS, HF,
> BT, MODEM, Earpiece, etc where some components have different
> constraints. It's possible that you dont have a path in your case.
>
> Can you check you have enabled a valid playback path with
> alsaucm/alsamixer and then test with aplay. This should help narrow down
> the issue.
>
> I've CC'ed Peter since 3.4 is a little old and he is working on the
> latest OMAP4 codebase.
>
> Regards
>
> Liam
>
>
>
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 19:24     ` Brent Weatherall
@ 2013-01-31 19:44       ` Clemens Ladisch
  2013-01-31 19:50         ` Brent Weatherall
  2013-01-31 20:00         ` Liam Girdwood
  2013-01-31 19:50       ` Liam Girdwood
  2013-02-01  6:57       ` Takashi Iwai
  2 siblings, 2 replies; 18+ messages in thread
From: Clemens Ladisch @ 2013-01-31 19:44 UTC (permalink / raw)
  To: Brent Weatherall; +Cc: Liam Girdwood, Peter Ujfalusi, alsa-devel

Brent Weatherall wrote:
> stepping through the ALSA code, it seems the error *might* be in
> the ALSA code, but I am probably reading the code incorrectly:
>
> pcm/interval.c:

That code is correct (it's used in a different context).

The problem is that the kernel driver returns wrong min/max values.

> Liam Girdwood wrote:
>> The OMAP4 ABE driver doesn't actually know the exact constraints if
>> there is not a valid playback path between source PCM and sink (it will
>> still throw out anything insane though). This is because it can route
>> audio from most of it's PCMs to most of it's components, e.g. HS, HF,
>> BT, MODEM, Earpiece, etc where some components have different
>> constraints. It's possible that you dont have a path in your case.

In theory, what the driver should do is:
- don't allow the PCM device to be opened if there is no valid route
  configuration (or just never allow a completely invalid
  configuration); and
- while the PCM device is open, lock the route configuration, or
- if changing the route during playback is necessary, allow to set
  only those routes that are possible with the current PCM
  configuration.


Regards,
Clemens

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 19:44       ` Clemens Ladisch
@ 2013-01-31 19:50         ` Brent Weatherall
  2013-01-31 20:00         ` Liam Girdwood
  1 sibling, 0 replies; 18+ messages in thread
From: Brent Weatherall @ 2013-01-31 19:50 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: Liam Girdwood, Peter Ujfalusi, alsa-devel

Clemens,

     OK.  It sounds like I need to go further down into the hardware driver
to ferret out the cause.  I agree with you it looks like a bad return from
hardware.  I will see if I can figure out how to get the correct code.  it
sounds to me like it should be in one of the TI OMAP branches then.  Thanks!

Regards,

Brent


On Thu, Jan 31, 2013 at 11:44 AM, Clemens Ladisch <clemens@ladisch.de>wrote:

> Brent Weatherall wrote:
> > stepping through the ALSA code, it seems the error *might* be in
> > the ALSA code, but I am probably reading the code incorrectly:
> >
> > pcm/interval.c:
>
> That code is correct (it's used in a different context).
>
> The problem is that the kernel driver returns wrong min/max values.
>
> > Liam Girdwood wrote:
> >> The OMAP4 ABE driver doesn't actually know the exact constraints if
> >> there is not a valid playback path between source PCM and sink (it will
> >> still throw out anything insane though). This is because it can route
> >> audio from most of it's PCMs to most of it's components, e.g. HS, HF,
> >> BT, MODEM, Earpiece, etc where some components have different
> >> constraints. It's possible that you dont have a path in your case.
>
> In theory, what the driver should do is:
> - don't allow the PCM device to be opened if there is no valid route
>   configuration (or just never allow a completely invalid
>   configuration); and
> - while the PCM device is open, lock the route configuration, or
> - if changing the route during playback is necessary, allow to set
>   only those routes that are possible with the current PCM
>   configuration.
>
>
> Regards,
> Clemens
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 19:24     ` Brent Weatherall
  2013-01-31 19:44       ` Clemens Ladisch
@ 2013-01-31 19:50       ` Liam Girdwood
  2013-02-01  6:57       ` Takashi Iwai
  2 siblings, 0 replies; 18+ messages in thread
From: Liam Girdwood @ 2013-01-31 19:50 UTC (permalink / raw)
  To: Brent Weatherall; +Cc: Peter Ujfalusi, alsa-devel, Clemens Ladisch



> Thank you for taking the time to look this over.  Is there an aplay
> command that might uncover if there is a valid path to the device, or
> does the ability to play sound with aplay demonstrate the presence of
> the path?
> 
Working aplay/arecord will show the path is valid.

Liam 
>         
>         
>         
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 19:44       ` Clemens Ladisch
  2013-01-31 19:50         ` Brent Weatherall
@ 2013-01-31 20:00         ` Liam Girdwood
  2013-01-31 22:13           ` Brent Weatherall
  1 sibling, 1 reply; 18+ messages in thread
From: Liam Girdwood @ 2013-01-31 20:00 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: Peter Ujfalusi, alsa-devel, Brent Weatherall

On Thu, 2013-01-31 at 20:44 +0100, Clemens Ladisch wrote:
> 
> The problem is that the kernel driver returns wrong min/max values.
> 
> > Liam Girdwood wrote:
> >> The OMAP4 ABE driver doesn't actually know the exact constraints if
> >> there is not a valid playback path between source PCM and sink (it will
> >> still throw out anything insane though). This is because it can route
> >> audio from most of it's PCMs to most of it's components, e.g. HS, HF,
> >> BT, MODEM, Earpiece, etc where some components have different
> >> constraints. It's possible that you dont have a path in your case.
> 
> In theory, what the driver should do is:
> - don't allow the PCM device to be opened if there is no valid route
>   configuration (or just never allow a completely invalid
>   configuration); and

Ah, this was my initial design too :)

Some userspace software will actually and validly open a PCM, configure
some mixers and then perform a hw_params() -> trigger(). So locking out
the PCM at open() for invalid paths breaks some userspace code. 

> - while the PCM device is open, lock the route configuration, or

We cant do this either as we have to support runtime switching between
different use cases (that use different back end components) e.g. MP3
playback to Phonecall

> - if changing the route during playback is necessary, allow to set
>   only those routes that are possible with the current PCM
>   configuration.

We do this already, but will first try and fixup any of the
configuration differences in the DSP so that the userspace operation
succeeds.

I guess Peter/Brent will have to look at the min/max values in more
detail.

Liam

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 20:00         ` Liam Girdwood
@ 2013-01-31 22:13           ` Brent Weatherall
  2013-02-01  8:29             ` Liam Girdwood
  0 siblings, 1 reply; 18+ messages in thread
From: Brent Weatherall @ 2013-01-31 22:13 UTC (permalink / raw)
  To: Liam Girdwood; +Cc: Peter Ujfalusi, alsa-devel, Clemens Ladisch

Liam,

     Before I begin to dig around, do you know if there is the possibility
a newer kernel (like 3.7 or something) will have fixed this issue?

Regards,

Brent


On Thu, Jan 31, 2013 at 12:00 PM, Liam Girdwood <
liam.r.girdwood@linux.intel.com> wrote:

> On Thu, 2013-01-31 at 20:44 +0100, Clemens Ladisch wrote:
> >
> > The problem is that the kernel driver returns wrong min/max values.
> >
> > > Liam Girdwood wrote:
> > >> The OMAP4 ABE driver doesn't actually know the exact constraints if
> > >> there is not a valid playback path between source PCM and sink (it
> will
> > >> still throw out anything insane though). This is because it can route
> > >> audio from most of it's PCMs to most of it's components, e.g. HS, HF,
> > >> BT, MODEM, Earpiece, etc where some components have different
> > >> constraints. It's possible that you dont have a path in your case.
> >
> > In theory, what the driver should do is:
> > - don't allow the PCM device to be opened if there is no valid route
> >   configuration (or just never allow a completely invalid
> >   configuration); and
>
> Ah, this was my initial design too :)
>
> Some userspace software will actually and validly open a PCM, configure
> some mixers and then perform a hw_params() -> trigger(). So locking out
> the PCM at open() for invalid paths breaks some userspace code.
>
> > - while the PCM device is open, lock the route configuration, or
>
> We cant do this either as we have to support runtime switching between
> different use cases (that use different back end components) e.g. MP3
> playback to Phonecall
>
> > - if changing the route during playback is necessary, allow to set
> >   only those routes that are possible with the current PCM
> >   configuration.
>
> We do this already, but will first try and fixup any of the
> configuration differences in the DSP so that the userspace operation
> succeeds.
>
> I guess Peter/Brent will have to look at the min/max values in more
> detail.
>
> Liam
>
>
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 19:24     ` Brent Weatherall
  2013-01-31 19:44       ` Clemens Ladisch
  2013-01-31 19:50       ` Liam Girdwood
@ 2013-02-01  6:57       ` Takashi Iwai
  2 siblings, 0 replies; 18+ messages in thread
From: Takashi Iwai @ 2013-02-01  6:57 UTC (permalink / raw)
  To: Brent Weatherall
  Cc: Liam Girdwood, Peter Ujfalusi, alsa-devel, Clemens Ladisch

At Thu, 31 Jan 2013 11:24:04 -0800,
Brent Weatherall wrote:
> 
> Clemens/Liam,
> 
>      Liam, I address your question below.  Clemens, stepping through the
> ALSA code, it seems the error *might* be in the ALSA code, but I am
> probably reading the code incorrectly:
> 
> pcm/interval.c:
> 
> 105 int snd_interval_refine_min(snd_interval_t *i, unsigned int min, int
> openmin)
> 106 {
> 107         int changed = 0;
> 108         if (snd_interval_empty(i))
> 109                 return -ENOENT;
> 110         if (i->min < min) {
> 111                 i->min = min;  <-- Is this statement reversed or do I
> not understand the code well enough?
> 112                 i->openmin = openmin;
> 113                 changed = 1;
> 114         } else if (i->min == min && !i->openmin && openmin) {
> 115                 i->openmin = 1;
> 116                 changed = 1;
> 117         }
> 118         if (i->integer) {
> 119                 if (i->openmin) {
> 120                         i->min++;
> 121                         i->openmin = 0;
> 122                 }
> 123         }
> 124         if (snd_interval_checkempty(i)) {
> 125                 snd_interval_none(i);
> 126
> 127         }
> 128         return changed;
> 129 }
> 130
> 
> Lines 110 - 113 appear to check if the hw params returned structure's
> minimum value (i->min) is less than the current min value (which is set to
> the requested rate in pcm.c:827  min = max = best).  Then if true, it
> appears to set the hardware reported min value to the current min, instead
> of the other way around.

The code is correct.  In general, hw_refine() code works like the
following:

- The configuration space begins with the full space

- When a hw constraint is given, it tries to shrink the configuration
  space to fit within the given constraint; it never expands in
  general (exceptions allowed, though, like in usb-audio's special
  code).

  This is the point you suggested in the  above.
  snd_internval_refine_min() tries to shrink the config space to fit
  wit with the condition (min <= i->min).

- Loop until all hw constraints are satisfied

- Then pick up either max or min value from the configuration space as
  the final value; it depends on the parameter type whether to pick
  max or min value


Takashi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 22:13           ` Brent Weatherall
@ 2013-02-01  8:29             ` Liam Girdwood
  2013-02-01 10:51               ` Peter Ujfalusi
  0 siblings, 1 reply; 18+ messages in thread
From: Liam Girdwood @ 2013-02-01  8:29 UTC (permalink / raw)
  To: Brent Weatherall; +Cc: Peter Ujfalusi, alsa-devel, Clemens Ladisch, sguiriec

On Thu, 2013-01-31 at 14:13 -0800, Brent Weatherall wrote:
> Liam,

>      Before I begin to dig around, do you know if there is the
> possibility a newer kernel (like 3.7 or something) will have fixed
> this issue?
> 
I think Peter will probably have an 3.8rc development branch on
gitorious here :-

git@gitorious.org:omap-audio/linux-audio.git next/ti-audio-next


Liam

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-02-01  8:29             ` Liam Girdwood
@ 2013-02-01 10:51               ` Peter Ujfalusi
  2013-02-01 10:56                 ` Michael Trimarchi
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Ujfalusi @ 2013-02-01 10:51 UTC (permalink / raw)
  To: Liam Girdwood
  Cc: Clemens Ladisch, alsa-devel, Brent Weatherall, Guiriec, Sebastien

On 02/01/2013 09:29 AM, Liam Girdwood wrote:
> On Thu, 2013-01-31 at 14:13 -0800, Brent Weatherall wrote:
>> Liam,
> 
>>      Before I begin to dig around, do you know if there is the
>> possibility a newer kernel (like 3.7 or something) will have fixed
>> this issue?
>>
> I think Peter will probably have an 3.8rc development branch on
> gitorious here :-
> 
> git@gitorious.org:omap-audio/linux-audio.git next/ti-audio-next

Yes, we have that but it is with dynamic FW which demands different ABE
firmware...
I'll go through the thread and try to give some pointers.
Do we know where to get the sources for the '3.4.0-1487-omap4'?

-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-02-01 10:51               ` Peter Ujfalusi
@ 2013-02-01 10:56                 ` Michael Trimarchi
  0 siblings, 0 replies; 18+ messages in thread
From: Michael Trimarchi @ 2013-02-01 10:56 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Liam Girdwood, Brent Weatherall, alsa-devel, Clemens Ladisch,
	Guiriec, Sebastien

Hi

On 02/01/2013 11:51 AM, Peter Ujfalusi wrote:
> On 02/01/2013 09:29 AM, Liam Girdwood wrote:
>> On Thu, 2013-01-31 at 14:13 -0800, Brent Weatherall wrote:
>>> Liam,
>>
>>>      Before I begin to dig around, do you know if there is the
>>> possibility a newer kernel (like 3.7 or something) will have fixed
>>> this issue?
>>>
>> I think Peter will probably have an 3.8rc development branch on
>> gitorious here :-
>>
>> git@gitorious.org:omap-audio/linux-audio.git next/ti-audio-next
> 
> Yes, we have that but it is with dynamic FW which demands different ABE
> firmware...
> I'll go through the thread and try to give some pointers.
> Do we know where to get the sources for the '3.4.0-1487-omap4'?
> 

git://dev.omapzoom.org/pub/scm/integration/kernel-ubuntu.git

Michael

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: PandaBoard ES Audio Problems
  2013-01-31 16:51 PandaBoard ES Audio Problems Brent Weatherall
  2013-01-31 17:28 ` Clemens Ladisch
@ 2013-02-01 12:53 ` Peter Ujfalusi
  2013-02-06 15:10 ` [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard Peter Ujfalusi
  2 siblings, 0 replies; 18+ messages in thread
From: Peter Ujfalusi @ 2013-02-01 12:53 UTC (permalink / raw)
  To: Brent Weatherall; +Cc: Liam Girdwood, alsa-devel, Guiriec, Sebastien

Hi,

On 01/31/2013 05:51 PM, Brent Weatherall wrote:

> Here is a log of the high level test program:
> 
> snd_pcm_hw_params_set_rate_near succeeded.
> Rate set to 44100.
> snd_pcm_hw_params_set_channels succeeded.
> cannot set buffer time (Invalid argument)
> cannot set buffer period time (Invalid argument)
> Before commit, state is: SND_PCM_STATE_OPEN
> cannot set parameters (Invalid argument)
> After failure, state is: SND_PCM_STATE_OPEN
> Trying to re-configure rate param.
> Before calling 'snd_pcm_drop', state is: SND_PCM_STATE_OPEN
> cannot drop (Input/output error)
> After calling 'snd_pcm_drop', state is: SND_PCM_STATE_OPEN
> Trying rate 8000.
> snd_pcm_hw_params_set_rate_near succeeded.
> Rate set to 44100.
> snd_pcm_hw_params_set_buffer_time_near succeeded.
> Buffer size actually set to 22050.
> cannot set buffer period time (Invalid argument)
> cannot set parameters (Invalid argument)
> cannot prepare audio interface for use (Input/output error)
> Rate 44100 supported: FAIL

Which PCM you are opening? My guess is pcm0 which supports 44.1 and 48. You
seams to be failing with the period size here.
I know we have constraint for the period size, but can not recall what it is
and why it has not been applied to the stream.
Sebastien: can you comment?

-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard
  2013-01-31 16:51 PandaBoard ES Audio Problems Brent Weatherall
  2013-01-31 17:28 ` Clemens Ladisch
  2013-02-01 12:53 ` Peter Ujfalusi
@ 2013-02-06 15:10 ` Peter Ujfalusi
  2013-02-06 15:10   ` [PATCH 1/2] ASoC: omap-abe-mmap: Make the hwrule function to be more generic Peter Ujfalusi
                     ` (2 more replies)
  2 siblings, 3 replies; 18+ messages in thread
From: Peter Ujfalusi @ 2013-02-06 15:10 UTC (permalink / raw)
  To: brentweatherall; +Cc: liam.r.girdwood, alsa-devel, michael, clemens, s-guiriec

Hi Brent,

Could you try the following two patch on top of your ubuntu kernel:
git://dev.omapzoom.org/pub/scm/integration/kernel-ubuntu.git ubuntu/ti-ubuntu-3.4-1487

It fixes similar issue on my board when using mplayer with ABE.

Regards,
Peter
---
Peter Ujfalusi (2):
  ASoC: omap-abe-mmap: Make the hwrule function to be more generic
  ASoC: omap-abe-mmap: Place constraint to buffer size as well

 sound/soc/omap/omap-abe-mmap.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

-- 
1.8.1.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] ASoC: omap-abe-mmap: Make the hwrule function to be more generic
  2013-02-06 15:10 ` [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard Peter Ujfalusi
@ 2013-02-06 15:10   ` Peter Ujfalusi
  2013-02-06 15:10   ` [PATCH 2/2] ASoC: omap-abe-mmap: Place constraint to buffer size as well Peter Ujfalusi
  2013-02-06 15:37   ` [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard Brent Weatherall
  2 siblings, 0 replies; 18+ messages in thread
From: Peter Ujfalusi @ 2013-02-06 15:10 UTC (permalink / raw)
  To: brentweatherall; +Cc: liam.r.girdwood, alsa-devel, michael, clemens, s-guiriec

Avoid using defines to get the snd_interval we are about to constrain.
rule->var holds the correct ID.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/omap/omap-abe-mmap.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/sound/soc/omap/omap-abe-mmap.c b/sound/soc/omap/omap-abe-mmap.c
index 92c240b..e5654c9 100644
--- a/sound/soc/omap/omap-abe-mmap.c
+++ b/sound/soc/omap/omap-abe-mmap.c
@@ -87,18 +87,17 @@ irqreturn_t abe_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int omap_abe_hwrule_period_step(struct snd_pcm_hw_params *params,
+static int omap_abe_hwrule_size_step(struct snd_pcm_hw_params *params,
 					struct snd_pcm_hw_rule *rule)
 {
-	struct snd_interval *period_size = hw_param_interval(params,
-				     SNDRV_PCM_HW_PARAM_PERIOD_SIZE);
 	unsigned int rate = params_rate(params);
 
 	/* 44.1kHz has the same iteration number as 48kHz */
 	rate = (rate == 44100) ? 48000 : rate;
 
 	/* ABE requires chunks of 250us worth of data */
-	return snd_interval_step(period_size, 0, rate / 4000);
+	return snd_interval_step(hw_param_interval(params, rule->var), 0,
+				 rate / 4000);
 }
 
 static int aess_open(struct snd_pcm_substream *substream)
@@ -128,7 +127,7 @@ static int aess_open(struct snd_pcm_substream *substream)
 		 */
 		ret = snd_pcm_hw_rule_add(substream->runtime, 0,
 					SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
-					omap_abe_hwrule_period_step,
+					omap_abe_hwrule_size_step,
 					NULL,
 					SNDRV_PCM_HW_PARAM_PERIOD_SIZE, -1);
 		break;
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] ASoC: omap-abe-mmap: Place constraint to buffer size as well
  2013-02-06 15:10 ` [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard Peter Ujfalusi
  2013-02-06 15:10   ` [PATCH 1/2] ASoC: omap-abe-mmap: Make the hwrule function to be more generic Peter Ujfalusi
@ 2013-02-06 15:10   ` Peter Ujfalusi
  2013-02-06 15:37   ` [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard Brent Weatherall
  2 siblings, 0 replies; 18+ messages in thread
From: Peter Ujfalusi @ 2013-02-06 15:10 UTC (permalink / raw)
  To: brentweatherall; +Cc: liam.r.girdwood, alsa-devel, michael, clemens, s-guiriec

In this we can make sure that applications will figure out the correct
combination of period size and buffer size.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 sound/soc/omap/omap-abe-mmap.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sound/soc/omap/omap-abe-mmap.c b/sound/soc/omap/omap-abe-mmap.c
index e5654c9..0aea7ec 100644
--- a/sound/soc/omap/omap-abe-mmap.c
+++ b/sound/soc/omap/omap-abe-mmap.c
@@ -122,7 +122,7 @@ static int aess_open(struct snd_pcm_substream *substream)
 		break;
 	default:
 		/*
-		 * Period size must be aligned with the Audio Engine
+		 * Period and buffer size must be aligned with the Audio Engine
 		 * processing loop which is 250 us long
 		 */
 		ret = snd_pcm_hw_rule_add(substream->runtime, 0,
@@ -130,6 +130,11 @@ static int aess_open(struct snd_pcm_substream *substream)
 					omap_abe_hwrule_size_step,
 					NULL,
 					SNDRV_PCM_HW_PARAM_PERIOD_SIZE, -1);
+		ret = snd_pcm_hw_rule_add(substream->runtime, 0,
+					SNDRV_PCM_HW_PARAM_BUFFER_SIZE,
+					omap_abe_hwrule_size_step,
+					NULL,
+					SNDRV_PCM_HW_PARAM_BUFFER_SIZE, -1);
 		break;
 	}
 
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard
  2013-02-06 15:10 ` [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard Peter Ujfalusi
  2013-02-06 15:10   ` [PATCH 1/2] ASoC: omap-abe-mmap: Make the hwrule function to be more generic Peter Ujfalusi
  2013-02-06 15:10   ` [PATCH 2/2] ASoC: omap-abe-mmap: Place constraint to buffer size as well Peter Ujfalusi
@ 2013-02-06 15:37   ` Brent Weatherall
  2 siblings, 0 replies; 18+ messages in thread
From: Brent Weatherall @ 2013-02-06 15:37 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Liam Girdwood, alsa-devel, michael, Clemens Ladisch, s-guiriec

Hi Peter,

     I apologize if you have sent me previous messages.  I think my e-mail
filters are off and this one almost got trashed.  I will try to get time on
this in the next couple of days as I can work it in.  Thanks for looking at
this.

Regards,

Brent


On Wed, Feb 6, 2013 at 7:10 AM, Peter Ujfalusi <peter.ujfalusi@ti.com>wrote:

> Hi Brent,
>
> Could you try the following two patch on top of your ubuntu kernel:
> git://dev.omapzoom.org/pub/scm/integration/kernel-ubuntu.gitubuntu/ti-ubuntu-3.4-1487
>
> It fixes similar issue on my board when using mplayer with ABE.
>
> Regards,
> Peter
> ---
> Peter Ujfalusi (2):
>   ASoC: omap-abe-mmap: Make the hwrule function to be more generic
>   ASoC: omap-abe-mmap: Place constraint to buffer size as well
>
>  sound/soc/omap/omap-abe-mmap.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> --
> 1.8.1.1
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2013-02-06 15:37 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-31 16:51 PandaBoard ES Audio Problems Brent Weatherall
2013-01-31 17:28 ` Clemens Ladisch
2013-01-31 18:23   ` Liam Girdwood
2013-01-31 19:24     ` Brent Weatherall
2013-01-31 19:44       ` Clemens Ladisch
2013-01-31 19:50         ` Brent Weatherall
2013-01-31 20:00         ` Liam Girdwood
2013-01-31 22:13           ` Brent Weatherall
2013-02-01  8:29             ` Liam Girdwood
2013-02-01 10:51               ` Peter Ujfalusi
2013-02-01 10:56                 ` Michael Trimarchi
2013-01-31 19:50       ` Liam Girdwood
2013-02-01  6:57       ` Takashi Iwai
2013-02-01 12:53 ` Peter Ujfalusi
2013-02-06 15:10 ` [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard Peter Ujfalusi
2013-02-06 15:10   ` [PATCH 1/2] ASoC: omap-abe-mmap: Make the hwrule function to be more generic Peter Ujfalusi
2013-02-06 15:10   ` [PATCH 2/2] ASoC: omap-abe-mmap: Place constraint to buffer size as well Peter Ujfalusi
2013-02-06 15:37   ` [PATCH 0/2] ASoC: OMAP4+ABE (ubuntu): Might fix OpenMax on PandaBoard Brent Weatherall

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.