Thread (24 messages) 24 messages, 2 authors, 2021-01-13

Re: [PATCH 5/6 v2] sound: Add n64 driver

From: Takashi Iwai <hidden>
Date: 2021-01-09 18:18:14

On Sat, 09 Jan 2021 18:46:01 +0100,
Lauri Kasanen wrote:
On Sat, 09 Jan 2021 09:16:08 +0100
Takashi Iwai [off-list ref] wrote:
quoted
quoted
quoted
quoted
+static const struct snd_pcm_hardware n64audio_pcm_hw = {
+	.info = (SNDRV_PCM_INFO_MMAP |
+		 SNDRV_PCM_INFO_MMAP_VALID |
+		 SNDRV_PCM_INFO_INTERLEAVED |
+		 SNDRV_PCM_INFO_BLOCK_TRANSFER),
+	.formats =          SNDRV_PCM_FMTBIT_S16_BE,
+	.rates =            SNDRV_PCM_RATE_8000_48000,
+	.rate_min =         8000,
+	.rate_max =         48000,
+	.channels_min =     2,
+	.channels_max =     2,
+	.buffer_bytes_max = 32768,
+	.period_bytes_min = 1024,
+	.period_bytes_max = 32768,
+	.periods_min =      1,
periods_min=1 makes little sense for this driver.
I have some questions about this.

When I had periods_min = 128, OSS apps were broken. I mean simple ones,
open /dev/dsp, ioctl the format/rate/stereo, write data. They got an IO
error errno IIRC, and no clarifying error in dmesg.

I tried following the error with printks, several levels deep. I gave
up when it got to the constraint resolving function, and there was no
good way to print and track which constraint failed, why, and who set
the constraint.
Did you try to set up the hw constraint for the integer PERIODS like
below at open?
  snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS)

Without this, it'd allow inconsistent buffer/period set up in your
case.
No, not yet. But surely an inconsistent buffer size would still play
something, instead of immediately erroring out?
In some cases, it's possible.  PCM OSS translation has some special
way depending on the period ("fragment" in OSS) setup...
quoted
quoted
Only through blind guessing did I stumble upon periods_min.
The periods_min usually defines the hardware/software limit of the
interrupt transfer.
Why do you say periods_min=1 makes little sense? At 44.1 khz, that'd be
172 interrupts per second, which is a lot but workable? There is no hw
limit against 172 irqs/s.
Well, it's not about the sample rate or the process speed.  You need
to know what periods=1 means.  periods=1 is a VERY special usage.  No
double buffering and the driver has to report the precise accurate
position without period updates; i.e. it's almost for free-wheeling
DMA transfer.  Hence periods_min=1 makes sense if the driver may
behave like that.
quoted
quoted
- why was there no clarifying error in dmesg? Just an errno that means
a million things makes it impossible for the userspace app writer to
know why it's not working
Did you check the debug messages with dyndbg enabled?
No, CONFIG_DYNAMIC_DEBUG, CONFIG_DEBUG_FS and CONFIG_SND_DEBUG are all
disabled because this is a memory-constrained platform. Surely "why my
app is not producing sound" is not something that needs several
different kernel debug options enabled (+ root perms b/c debugfs).
But you are debugging the *kernel* problem, not the application.
I agree that debugfs isn't always needed for hunting application bugs,
yeah.


Takashi
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help