Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bcm2835-sdhost: Only claim one DMA channel #1329

Merged
merged 2 commits into from
Mar 8, 2016

Conversation

pelwell
Copy link
Contributor

@pelwell pelwell commented Mar 7, 2016

With both MMC controllers enabled there are few DMA channels left. The
bcm2835-sdhost driver only uses DMA in one direction at a time, so it
doesn't need to claim two channels.

See: #132

@msperl
Copy link
Contributor

msperl commented Mar 7, 2016

looks good

@pelwell
Copy link
Contributor Author

pelwell commented Mar 7, 2016

Any comments, @HiassofT ?

@msperl
Copy link
Contributor

msperl commented Mar 7, 2016

Same exercise also for sdhci?

@HiassofT
Copy link
Contributor

HiassofT commented Mar 7, 2016

Looks good to me as well, thanks a lot!

Ultra-minor nitpicking: you have a typo in the referenced issue (132 instead of 1327)

@msperl
Copy link
Contributor

msperl commented Mar 7, 2016

OK - I meant bcm2835-mmc.c not sdhci (which is this one)...

@pelwell
Copy link
Contributor Author

pelwell commented Mar 7, 2016

Thanks - the cut and paste was a character short - I'll fix before pushing.

With both MMC controllers enabled there are few DMA channels left. The
bcm2835-sdhost driver only uses DMA in one direction at a time, so it
doesn't need to claim two channels.

See: raspberrypi#1327

Signed-off-by: Phil Elwell <[email protected]>
@pelwell
Copy link
Contributor Author

pelwell commented Mar 8, 2016

I think these commits are good to go, but I'd appreciate a second opinion or some testing.

@clivem
Copy link

clivem commented Mar 8, 2016

I've been using a rpi-4.4.y build plus this patch for the last 12 hours on 4x Pi3B, 1x Pi2B and a Zero.

@popcornmix
Copy link
Collaborator

sdcard still working for me after applying this patch

@pelwell
Copy link
Contributor Author

pelwell commented Mar 8, 2016

Cool. I've added an equivalent patch for bcm2835-mmc.

@HiassofT
Copy link
Contributor

HiassofT commented Mar 8, 2016

I did some tests on my RPi2 with a Samsung EVO 32GB and everything works as expected.

Only a very minor cosmetic issue in bcm2835-mmc.c, "channels" instead of "channel" (same for the error cause, "Unable to initialise DMA channels").

[    2.761890] mmc-bcm2835 3f300000.mmc: DMA channels allocated

I tested the performance with iozone -e -I -a -s 50M -r 4k -r 512k -r 16M -i 0 -i 1 -i 2 and also verified that the number of allocated DMAs is correct (I had the Cirrus driver loaded, so SPI and I2S DMA were used as well). No overclocking, BTW.

sdhost without PR:

root@rpi2:~# cat /sys/class/dma/*/in_use | grep 1 | wc -l
6
                                                 random  random
   KB  reclen   write rewrite    read    reread    read   write
51200       4    2727    3125     7975     7989    7814    2880
51200     512   15685   16407    21076    21046   21055   16500
51200   16384   16504   16415    21434    21425   21451   16640

sdhost with PR:

root@rpi2:~# cat /sys/class/dma/*/in_use | grep 1 | wc -l
5
                                                 random  random
   KB  reclen   write rewrite    read    reread    read   write
51200       4    2738    3024     7920     7910    7881    2669
51200     512   14972   16418    21058    21058   21060   16446
51200   16384   16469   16585    21455    21432   21439   16657

mmc without PR:

root@rpi2:~# cat /sys/class/dma/*/in_use | grep 1 | wc -l
6
                                                 random  random
   KB  reclen   write rewrite    read    reread    read   write
51200       4    2678    2998     7380     7386    7279    2703
51200     512   13365   13784    17900    17921   17893   13813
51200   16384   13542   11450    18123    18221   18209   10167

mmc with PR:

root@rpi2:~# cat /sys/class/dma/*/in_use | grep 1 | wc -l
5
                                                 random  random
   KB  reclen   write rewrite    read    reread    read   write
51200       4    2713    3013     7303     7305    7205    2925
51200     512   13252   13779    17622    17642   17612   13701
51200   16384   13845   13681    18120    18131   18134   12274

With both MMC controllers enabled there are few DMA channels left. The
bcm2835-mmc driver only uses DMA in one direction at a time, so it
doesn't need to claim two channels.

See: raspberrypi#1327

Signed-off-by: Phil Elwell <[email protected]>
@pelwell
Copy link
Contributor Author

pelwell commented Mar 8, 2016

Thanks, everyone.

pelwell added a commit that referenced this pull request Mar 8, 2016
bcm2835-sdhost: Only claim one DMA channel
@pelwell pelwell merged commit 4f7b097 into raspberrypi:rpi-4.4.y Mar 8, 2016
@msperl
Copy link
Contributor

msperl commented Mar 8, 2016

thanks to all...
Now what is left is upstreaming those (including the initial DMA) as soon as the sg_slave patch goes in...

@clivem
Copy link

clivem commented Mar 8, 2016

Just so you guys know, call it feedback.... My 4.4.y builds are running on 15x Pi's locally (B, B+, Zero, CM, 2B and 3B) and out in the wild, by approx. 30 users. All have an I2S DAC (of some description) attached. So from a dma point of view, sdcard and cyclic for I2S have been well tested. Won't have seen much (any) testing for SPI dma..... Although I think one guy has an ethernet port attached to a CM via SPI.

@msperl Your 15x patch upstream backport dma series has been patched into my rpi-4.4.y builds since Jan 06.... This code is now well tested, especially cyclic dma.

@HiassofT I added the 3x patches yesterday to enable extra channels.

And obviously, @pelwell, latest to use single dma channel for sdhost and mmc drivers is now in 4.4.y, so now included in my builds without patching.

@clivem
Copy link

clivem commented Mar 8, 2016

Both have I2S DAC attached....

[PI3B dma]$ for f in dma0chan*; do echo -n "$f: in_use: " && cat $f/in_use; done;
dma0chan0: in_use: 1
dma0chan1: in_use: 1
dma0chan2: in_use: 1
dma0chan3: in_use: 1
dma0chan4: in_use: 0
dma0chan5: in_use: 0
dma0chan6: in_use: 0
dma0chan7: in_use: 0
dma0chan8: in_use: 0
dma0chan9: in_use: 0

[ZERO dma]$ for f in dma0chan*; do echo -n "$f: in_use: " && cat $f/in_use; done;
dma0chan0: in_use: 1
dma0chan1: in_use: 1
dma0chan2: in_use: 1
dma0chan3: in_use: 0
dma0chan4: in_use: 0
dma0chan5: in_use: 0
dma0chan6: in_use: 0
dma0chan7: in_use: 0
dma0chan8: in_use: 0
dma0chan9: in_use: 0

@pelwell
Copy link
Contributor Author

pelwell commented Mar 8, 2016

Looks good. BTW, a little trick I learnt from @popcornmix:

$ grep . /sys/class/dma/*/in_use
/sys/class/dma/dma0chan0/in_use:1
/sys/class/dma/dma0chan1/in_use:1
/sys/class/dma/dma0chan2/in_use:0
/sys/class/dma/dma0chan3/in_use:0
/sys/class/dma/dma0chan4/in_use:0
/sys/class/dma/dma0chan5/in_use:0
/sys/class/dma/dma0chan6/in_use:0

@msperl
Copy link
Contributor

msperl commented Mar 8, 2016

or simpler still:
head /sys/class/dma/*/in_use
but with a slightly different layout...

@clivem: the lates upstream patches are slightly different - Eric wanted the LITE channel handling in a separate patch, Vinod wanted some values configurable in dt and some more things (like peeking into the dma channel registers via debugfs)

@clivem
Copy link

clivem commented Mar 8, 2016

$ grep . /sys/class/dma//in_use
head /sys/class/dma/
/in_use

LOL. Thanks guys. If there is a long-winded way of doing anything, you can bet your last dollar, I'll be doing it that way....

the lates upstream patches are slightly different

@msperl Thanks for the info. I did notice a flurry of stuff from you to lk-arm ML in the last couple of weeks, but haven't looked too closely. My company has settled on using UDOO Neo for a SBC based commercial product, so my "work" attention is now on all things IMX related rather than Pi.

@pelwell pelwell deleted the sdhost4.4 branch September 15, 2021 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants