[RFC] tegra_pcm DMA buffers allocation

[RFC] tegra_pcm DMA buffers allocation

[Andrey Smirnov]

Hello,

I'm working with a Harmony-based custom Tegra2 board and I'm writing a
driver for a sound capturing I2S device. At this point, for all intents
and purposes the code I use is almost indistinguishable from the similar
code for WM8782(digital microphone) which means that the the structure
describing the DAI has only .capture data and .playback field is left
untouched. This set up, using the tegra-l4t-r15-alpha kernel* results in
the crash because of NULL pointer dereference in
tegra_pcm_preallocate_dma_buffer().

My humble investigation leads me to the following conclusions:
The way I see this code works(correct me if I missed something) is the
following:

soc_probe_dai_link() -> soc_new_pcm() -> snd_pcm_new() ->
snd_pcm_new_stream()

And the decision to allocate a stream is based on the codec_dai's
playback/capture channel count by this piece of code from soc_new_pcm:  

if (codec_dai->driver->playback.channels_min)
	playback = 1;
if (codec_dai->driver->capture.channels_min)
	capture = 1;

Tegra's DMA buffers allocation on the other hand is guided by the amount
of playback capture channels on the cpu_dai as the following excerpt
from tegrap_pcm_new() shows:

struct snd_soc_dai *dai = rtd->cpu_dai;
.
.
.
if (dai->driver->playback.channels_min) {
	ret = tegra_pcm_preallocate_dma_buffer(pcm,
					SNDRV_PCM_STREAM_PLAYBACK);
	if (ret)
		goto err;
}

if (dai->driver->capture.channels_min) {
	ret = tegra_pcm_preallocate_dma_buffer(pcm,
					SNDRV_PCM_STREAM_CAPTURE);
	if (ret)
		goto err_free_play;
}

And wile not positive(unfortunately I didn't have the tenacity required
to actually trace the exact cause of the crash) I'm rather confident
that this is the reason I experience the problem described above, since 
working it around the following way:

if (rtd->codec_dai->driver->playback.channels_min &&
dai->driver->playback.channels_min) {
	ret = tegra_pcm_preallocate_dma_buffer(pcm,
					SNDRV_PCM_STREAM_PLAYBACK);
	if (ret)
		goto err;
}

if (rtd->codec_dai->driver->playback.channels_min &&
dai->driver->capture.channels_min) {
	ret = tegra_pcm_preallocate_dma_buffer(pcm,
					SNDRV_PCM_STREAM_CAPTURE);
	if (ret)
		goto err_free_play;
}

at least allowed the machine to start. Do I miss some additional setup
bit required for this use-case? If not then, what would be the best way
to solve this problem? Declare a fake playback stream in the driver and
ignore it, since there is no physical connection to the corresponding
pins of the T20?


Thank you for you answers, and I apologize if any of my conclusions are
based on faulty premises an therefore are just noise.

Andrey Smirnov

______________
* I know that the code base is different from
git://git.kernel.org/pub/scm/linux/kernel/git/swarren/linux-tegra.git
for-next but from what I can tell the code corresponding to the issue at
ahnd is pretty similar.

Re: [RFC] tegra_pcm DMA buffers allocation

[Stephen Warren]

On 04/19/2012 10:00 AM, Andrey Smirnov wrote:
> Hello,
> 
> I'm working with a Harmony-based custom Tegra2 board and I'm writing a
> driver for a sound capturing I2S device. At this point, for all intents
> and purposes the code I use is almost indistinguishable from the similar
> code for WM8782(digital microphone) which means that the the structure
> describing the DAI has only .capture data and .playback field is left
> untouched. This set up, using the tegra-l4t-r15-alpha kernel* results in
> the crash because of NULL pointer dereference in
> tegra_pcm_preallocate_dma_buffer().

This mailing list is generally considered to be solely for discussing
the mainline kernel, rather than NVIDIA's downstream/product kernels.
I'm afraid we don't have a good story for external contributions to our
downstream kernels through channels other than OEM product engagements
though.

That said, I think if you replace the following code that you say is in
our L4T kernels:

	if (dai->driver->playback.channels_min) {

with the upstream variant:

	if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) {

I hope you'll find your problem is solved. If not, then there is
probably a bug in the mainline code that we could continue to discuss here.

BTW, did you try running the mainline code on your board. If it's
derived from Harmony, it should be easy to get it going.

Re: [RFC] tegra_pcm DMA buffers allocation

[Andrey Smirnov]

On Thu, 2012-04-19 at 16:07 -0400, Stephen Warren wrote:
> On 04/19/2012 10:00 AM, Andrey Smirnov wrote:
> > Hello,
> > 
> > I'm working with a Harmony-based custom Tegra2 board and I'm writing a
> > driver for a sound capturing I2S device. At this point, for all intents
> > and purposes the code I use is almost indistinguishable from the similar
> > code for WM8782(digital microphone) which means that the the structure
> > describing the DAI has only .capture data and .playback field is left
> > untouched. This set up, using the tegra-l4t-r15-alpha kernel* results in
> > the crash because of NULL pointer dereference in
> > tegra_pcm_preallocate_dma_buffer().
> 
> This mailing list is generally considered to be solely for discussing
> the mainline kernel, rather than NVIDIA's downstream/product kernels.
> I'm afraid we don't have a good story for external contributions to our
> downstream kernels through channels other than OEM product engagements
> though.
> 
> That said, I think if you replace the following code that you say is in
> our L4T kernels:
> 
> 	if (dai->driver->playback.channels_min) {
> 
> with the upstream variant:
> 
> 	if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) {
> 
> I hope you'll find your problem is solved. If not, then there is
> probably a bug in the mainline code that we could continue to discuss here.

Oh, I apologize I think I mixed the two code bases, for some reason I
thought that was the way it was handled in upstream too, that's why I
brought the issue here. I saw the same code used in PXA pcm code(the one
example using capture only drivers I found). And it worked like a charm,
thank your for suggestion.

> 
> BTW, did you try running the mainline code on your board. If it's
> derived from Harmony, it should be easy to get it going.

Last time we did there was no support for Tegra's NAND and display
controllers(I was very excited to read about those DRM DC patches the
other day), but to switch to upstream code is definitely the plan.

Re: [RFC] tegra_pcm DMA buffers allocation

[Stephen Warren]

On 04/19/2012 03:07 PM, Andrey Smirnov wrote:
> On Thu, 2012-04-19 at 16:07 -0400, Stephen Warren wrote:
...
>> BTW, did you try running the mainline code on your board. If it's
>> derived from Harmony, it should be easy to get it going.
> 
> Last time we did there was no support for Tegra's NAND and display
> controllers

What do you need the NAND controllers for? I'm not 100% certain, but I
believe we plan on upstreaming NAND support into U-Boot, but not the
kernel, since we didn't see a need for it there IIRC. (Not that we'd
stop anyone else doing it)

Re: [RFC] tegra_pcm DMA buffers allocation

[Andrey Smirnov]

On Thu, 2012-04-19 at 17:37 -0400, Stephen Warren wrote:
> On 04/19/2012 03:07 PM, Andrey Smirnov wrote:
> > On Thu, 2012-04-19 at 16:07 -0400, Stephen Warren wrote:
> ...
> >> BTW, did you try running the mainline code on your board. If it's
> >> derived from Harmony, it should be easy to get it going.
> > 
> > Last time we did there was no support for Tegra's NAND and display
> > controllers
> 
> What do you need the NAND controllers for? I'm not 100% certain, but I
> believe we plan on upstreaming NAND support into U-Boot, but not the
> kernel, since we didn't see a need for it there IIRC. (Not that we'd
> stop anyone else doing it)

Our design is Harmony based, but unlike the original Harmony board we
have eMMC instead of removable SDMMC card, which means that for now the
only way for us to update the software on the board is to use nvflash.
Nvflash, to the best of my knowledge at least, does not support flashing
of eMMC for boards other than Ventana and Cardhu, and that forces us to
use one of the NAND partitions to store kernel image and rootfs.

Andrey Smirnov

Re: [RFC] tegra_pcm DMA buffers allocation

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]

* Stephen Warren wrote:
> On 04/19/2012 03:07 PM, Andrey Smirnov wrote:
> > On Thu, 2012-04-19 at 16:07 -0400, Stephen Warren wrote:
> ...
> >> BTW, did you try running the mainline code on your board. If it's
> >> derived from Harmony, it should be easy to get it going.
> > 
> > Last time we did there was no support for Tegra's NAND and display
> > controllers
> 
> What do you need the NAND controllers for? I'm not 100% certain, but I
> believe we plan on upstreaming NAND support into U-Boot, but not the
> kernel, since we didn't see a need for it there IIRC.

We use a device that is sort of a set-top box, which currently boots from an
SD card. However the SD card is eventually supposed to allow users to display
custom media (slide show, ...). In that case it is very stupid to keep the OS
on the same card.

While U-Boot NAND support is enough to get a kernel booted, you still need
the root filesystem to be loaded from NAND by the kernel. Kernel support is
also pretty much a requirement for runtime updates.

> (Not that we'd stop anyone else doing it)

I've been trying to make the NAND controller work on Tegra 2. Unfortunately
there's not much of a reference to work off except the driver that Simon
Glass posted to the U-Boot mailing list and written by Jim Lin. There is also
a driver in the NVIDIA downstream kernels but that's pretty much broken.

The U-Boot driver doesn't use interrupts obviously so I've resolved to use it
as a base and add interrupt support. Unfortunately I've hit a snag and
whenever I enable interrupts I see the CPU stormed by the NAND controller for
unknown reasons (no bits in ISR, DMA control or decode status registers set).
I'll have to investigate further. Maybe the controller isn't properly
initialized yet.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]