| Message ID | 1452788982-11583-1-git-send-email-caleb@crome.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jan 14, 2016 at 08:29:42AM -0800, Caleb Crome wrote: > Tuning the SSI fifo watermark & maxburst settings needs to be > optimized differently depending on the demands on the system. The > current default of 2 is too low for high data-rate systems. This > patch maintains exactly the same behavior by default (i.e defaults to > 2), but adds device tree options to set maxburst & fifo depth from the > device tree. This is necessary because a setting of 2 simply doesn't > work at higher data rates. > @@ -61,6 +61,16 @@ Optional properties: > - fsl,mode: The operating mode for the AC97 interface only. > "ac97-slave" - AC97 mode, SSI is clock slave > "ac97-master" - AC97 mode, SSI is clock master > +- fsl,fifo-watermark: Sets the fifo watermark. The default is > + fifo_depth-2 words, meaning 'initiate dma transfer > + when 2 words are left in the fifo'. At higher > + data rates (48kHz, 16-channels for example), this > + causes silent but deadly DMA xruns and channel > + slips. For 15 word FIFOs (like on MX5, MX6) 8 is > + a good value when running at high data rates > +- fsl,dma-maxburst: sets the max number of words to transfer in DMA. > + This defaults to the same value as > + fsl,fifo-watermark. I think DT maintainers may not give a consent towards these two properties as they are not to describe the hardware but to hack software configurations. (And it seems you haven't CCed them.) I forgot which values you've figured out for these two properties, but I think those two values should work for normal cases as well: as SSI only has limited FIFO depth, it won't hurt (increasing too much latency) even if using a higher watermark configuration imo. So it could be a good idea to use optimized settings for all use cases and let other users test it. Nicolin
On Thu, Jan 14, 2016 at 12:18 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote: > On Thu, Jan 14, 2016 at 08:29:42AM -0800, Caleb Crome wrote: >> Tuning the SSI fifo watermark & maxburst settings needs to be >> optimized differently depending on the demands on the system. The >> current default of 2 is too low for high data-rate systems. This >> patch maintains exactly the same behavior by default (i.e defaults to >> 2), but adds device tree options to set maxburst & fifo depth from the >> device tree. This is necessary because a setting of 2 simply doesn't >> work at higher data rates. > >> @@ -61,6 +61,16 @@ Optional properties: >> - fsl,mode: The operating mode for the AC97 interface only. >> "ac97-slave" - AC97 mode, SSI is clock slave >> "ac97-master" - AC97 mode, SSI is clock master >> +- fsl,fifo-watermark: Sets the fifo watermark. The default is >> + fifo_depth-2 words, meaning 'initiate dma transfer >> + when 2 words are left in the fifo'. At higher >> + data rates (48kHz, 16-channels for example), this >> + causes silent but deadly DMA xruns and channel >> + slips. For 15 word FIFOs (like on MX5, MX6) 8 is >> + a good value when running at high data rates >> +- fsl,dma-maxburst: sets the max number of words to transfer in DMA. >> + This defaults to the same value as >> + fsl,fifo-watermark. > > I think DT maintainers may not give a consent towards these two > properties as they are not to describe the hardware but to hack > software configurations. (And it seems you haven't CCed them.) > Yeah, I thought I'd just ask alsa first, rather than send to DT maintainers. Is it preferable to just send to everybody that get_maintainers spits out even for an RFC? > I forgot which values you've figured out for these two properties, > but I think those two values should work for normal cases as well: > as SSI only has limited FIFO depth, it won't hurt (increasing too > much latency) even if using a higher watermark configuration imo. > So it could be a good idea to use optimized settings for all use > cases and let other users test it. > > Nicolin As for optimal settings, I finally came to a setting of 4 for depth & maxburst, which will result in more DMA requests, but it's the only way that works at 48kHz for me. The default settings is 13 (15 - 2) for the ones of the 15 item fifo, which is a pretty dramatic difference. I just don't know if other chips will behave badly in that case. I'd be happy to just submit a patch that sets it to 4 if we think that's the right way to go. -Caleb
Nicolin Chen wrote: > I think DT maintainers may not give a consent towards these two > properties as they are not to describe the hardware but to hack > software configurations. (And it seems you haven't CCed them.) I admit it's a grey area, but the hardware doesn't work if you use the wrong value, and it is a fixed value per device. A p1022ds would use a different value than in in i.MX6, and once you pick a value, it's the same no matter which sample rate, buffer size, etc you choose.
On Thu, Jan 14, 2016 at 07:31:19PM -0600, Timur Tabi wrote: > Nicolin Chen wrote: > >I think DT maintainers may not give a consent towards these two > >properties as they are not to describe the hardware but to hack > >software configurations. (And it seems you haven't CCed them.) > I admit it's a grey area, but the hardware doesn't work if you use > the wrong value, and it is a fixed value per device. A p1022ds > would use a different value than in in i.MX6, and once you pick a > value, it's the same no matter which sample rate, buffer size, etc > you choose. Wish we could settle down a common solution for each case. If that doesn't work for PPC, we may confine the modifications to i.MX only by overriding those settings in the fsl_ssi_imx_probe() for safety.
On Thu, Jan 14, 2016 at 01:26:24PM -0800, Caleb Crome wrote: > As for optimal settings, I finally came to a setting of 4 for depth & > maxburst, which will result in more DMA requests, but it's the only > way that works at 48kHz for me. The default settings is 13 (15 - 2) > for the ones of the 15 item fifo, which is a pretty dramatic > difference. I just don't know if other chips will behave badly in > that case. What's your final configuration for TFWM0 bits, 4?
On Thu, Jan 14, 2016 at 07:31:19PM -0600, Timur Tabi wrote: > Nicolin Chen wrote: > >I think DT maintainers may not give a consent towards these two > >properties as they are not to describe the hardware but to hack > >software configurations. (And it seems you haven't CCed them.) > > I admit it's a grey area, but the hardware doesn't work if you use the wrong > value, and it is a fixed value per device. A p1022ds would use a different > value than in in i.MX6, and once you pick a value, it's the same no matter > which sample rate, buffer size, etc you choose. We've allowed similar properties for other things like SPI controllers. It really depends on the frequency you expect to change it. The more often it changes, the higher up the s/w stack it should be controlled (firmware/DT, kernel, or userspace). A function of the SOC or codec rate, then yes DT is fine. Every user needs to tune it on the same platform, then no, don't put it in DT. My recollection from i.MX is this would be the former case. Rob
On Thu, Jan 14, 2016 at 6:45 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote: > On Thu, Jan 14, 2016 at 01:26:24PM -0800, Caleb Crome wrote: > >> As for optimal settings, I finally came to a setting of 4 for depth & >> maxburst, which will result in more DMA requests, but it's the only >> way that works at 48kHz for me. The default settings is 13 (15 - 2) >> for the ones of the 15 item fifo, which is a pretty dramatic >> difference. I just don't know if other chips will behave badly in >> that case. > > What's your final configuration for TFWM0 bits, 4? Yes, a value of 4 for my use case: i.MX6 @ 768000 words/second (48khz * 16 channels). Also, works at 8kHz, 16kHz 32 kHz. A setting of 8 does not work reliably at 48kHz but does work at 8, 16 and 32. -caleb
On Thu, Jan 14, 2016 at 07:31:19PM -0600, Timur Tabi wrote: > Nicolin Chen wrote: > >I think DT maintainers may not give a consent towards these two > >properties as they are not to describe the hardware but to hack > >software configurations. (And it seems you haven't CCed them.) > I admit it's a grey area, but the hardware doesn't work if you use the wrong > value, and it is a fixed value per device. A p1022ds would use a different > value than in in i.MX6, and once you pick a value, it's the same no matter > which sample rate, buffer size, etc you choose. Caleb's original message suggested this was rate dependant.
Mark Brown wrote: >> >I admit it's a grey area, but the hardware doesn't work if you use the wrong >> >value, and it is a fixed value per device. A p1022ds would use a different >> >value than in in i.MX6, and once you pick a value, it's the same no matter >> >which sample rate, buffer size, etc you choose. > Caleb's original message suggested this was rate dependant. Yeah, I just noticed that. In that case, I agree that a device tree property is inappropriate, unless it's an array that contains tuples of sample rates and watermark/maxburst settings. That would get unwieldy very easily, though.
On Fri, Jan 15, 2016 at 5:46 AM, Timur Tabi <timur@tabi.org> wrote: > Mark Brown wrote: >>> >>> >I admit it's a grey area, but the hardware doesn't work if you use the >>> > wrong >>> >value, and it is a fixed value per device. A p1022ds would use a >>> > different >>> >value than in in i.MX6, and once you pick a value, it's the same no >>> > matter >>> >which sample rate, buffer size, etc you choose. > > >> Caleb's original message suggested this was rate dependant. > > > Yeah, I just noticed that. In that case, I agree that a device tree > property is inappropriate, unless it's an array that contains tuples of > sample rates and watermark/maxburst settings. That would get unwieldy very > easily, though. The rate dependance is only a *potential* issue. I suspect that a value of 4 should be functional for all rates and chips. The only trade off is more DMA requests/bursts. In a typical 15 word fifo, 48kHz, stereo, single fifo DMA system, the old value was 15-2 = 13, which would mean 7385 13-word DMA bursts/second. A new value of 4 would mean 24,000 4-word DMA bursts/second. Is that consequential for anybody? It's about the same total bandwidth on the system, but just broken up into smaller chunks (I don't know what the overhead is for a DMA burst) In a high channel count system (16 channels @ 48kHz), the old value doesn't work, and the new value would mean 192,000 4-word DMA bursts/second, which works on my MX6. So given that 192000 works fine, I'm not sure that the difference in a typical system would matter at all. If nobody objects, we can just set the value to 4 and be done with it. Another question: is the watermark ever going to be different than maxburst? Is there any reason to have them different? -Caleb
On Thu, Jan 14, 2016 at 08:56:31PM -0800, Caleb Crome wrote: > On Thu, Jan 14, 2016 at 6:45 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote: > > On Thu, Jan 14, 2016 at 01:26:24PM -0800, Caleb Crome wrote: > > > >> As for optimal settings, I finally came to a setting of 4 for depth & > >> maxburst, which will result in more DMA requests, but it's the only > >> way that works at 48kHz for me. The default settings is 13 (15 - 2) > >> for the ones of the 15 item fifo, which is a pretty dramatic > >> difference. I just don't know if other chips will behave badly in > >> that case. > > > > What's your final configuration for TFWM0 bits, 4? > > Yes, a value of 4 for my use case: i.MX6 @ 768000 words/second (48khz > * 16 channels). 4 means there are >= 4 empty slots in the FIFO, so there are no more than 11 remaining data. This makes sense. IIRC, the Freescale official BSP release for i.MX is used to set 6 to TFWM0/1 in the old day, not sure about recent ones though. So I think setting 4 to TFWM0/1 should work for most of cases. We may also let others test it before merging it. Actually a setting of 13 is much more risky in my opinion. It means only two empty slots in the FIFO, so it might be easily to get under/ overflow if a DMA transaction gets delay somehow. The only benefit is that DMA requests and interrupt (FIQ) can be reduced.
On Fri, Jan 15, 2016 at 09:03:28AM -0800, Caleb Crome wrote: > If nobody objects, we can just set the value to 4 and be done with it. I agree. And we may apply it only to i.MX platforms with DMA if other platform owners feel comfortable with the previous settings. > Another question: is the watermark ever going to be different than > maxburst? Is there any reason to have them different? The watermark is merely a threshold to trigger a DMA request. The only relationship with the burst size is that each burst transfer should not carry more data than the number of empty slots; FIFO under/overflow occurs otherwise. So it's just more efficient and safer to set an identical value to both of them. I don't think it will cause functional problems to set TFWM to 4 and burst size to 1 -- It just lets DMA operate in a single data transfer mode.
On Fri, Jan 15, 2016 at 10:38 AM, Nicolin Chen <nicoleotsuka@gmail.com> wrote: > On Fri, Jan 15, 2016 at 09:03:28AM -0800, Caleb Crome wrote: > >> If nobody objects, we can just set the value to 4 and be done with it. > > I agree. And we may apply it only to i.MX platforms with DMA if > other platform owners feel comfortable with the previous settings. > >> Another question: is the watermark ever going to be different than >> maxburst? Is there any reason to have them different? > > The watermark is merely a threshold to trigger a DMA request. The > only relationship with the burst size is that each burst transfer > should not carry more data than the number of empty slots; FIFO > under/overflow occurs otherwise. So it's just more efficient and > safer to set an identical value to both of them. I don't think > it will cause functional problems to set TFWM to 4 and burst size > to 1 -- It just lets DMA operate in a single data transfer mode. If there is no penalty for setting maxburst to 1 (or 2 in the case of dual fifo I think), then should we just set both the watermark and maxburst to 1? I guess the real difference would be when you're in FIQ mode. In FIQ mode, the penalty of an interrupt per word would be pretty bad, but in DMA mode, if we just set both to 1, we should be fine, right?
On Fri, Jan 15, 2016 at 10:49:04AM -0800, Caleb Crome wrote: > > The watermark is merely a threshold to trigger a DMA request. The > > only relationship with the burst size is that each burst transfer > > should not carry more data than the number of empty slots; FIFO > > under/overflow occurs otherwise. So it's just more efficient and > > safer to set an identical value to both of them. I don't think > > it will cause functional problems to set TFWM to 4 and burst size > > to 1 -- It just lets DMA operate in a single data transfer mode. > > If there is no penalty for setting maxburst to 1 (or 2 in the case of > dual fifo I think), then should we just set both the watermark and > maxburst to 1? > > I guess the real difference would be when you're in FIQ mode. In FIQ > mode, the penalty of an interrupt per word would be pretty bad, but in > DMA mode, if we just set both to 1, we should be fine, right? There will be much more overhead drawn by frequent DMA transfers. I believe you understand the idea -- less burst size then more DMA request. Each DMA transfer contains a pair of handshaking overhead according to the bus protocol. Apparently the bus will be wasted with lots of handshaking section instead of keep dedicated to data transfer truly. It might work if SSI is the only user of the bus, which we shouldn't assume.
On Fri, Jan 15, 2016 at 10:57 AM, Nicolin Chen <nicoleotsuka@gmail.com> wrote: > On Fri, Jan 15, 2016 at 10:49:04AM -0800, Caleb Crome wrote: > >> > The watermark is merely a threshold to trigger a DMA request. The >> > only relationship with the burst size is that each burst transfer >> > should not carry more data than the number of empty slots; FIFO >> > under/overflow occurs otherwise. So it's just more efficient and >> > safer to set an identical value to both of them. I don't think >> > it will cause functional problems to set TFWM to 4 and burst size >> > to 1 -- It just lets DMA operate in a single data transfer mode. >> >> If there is no penalty for setting maxburst to 1 (or 2 in the case of >> dual fifo I think), then should we just set both the watermark and >> maxburst to 1? >> >> I guess the real difference would be when you're in FIQ mode. In FIQ >> mode, the penalty of an interrupt per word would be pretty bad, but in >> DMA mode, if we just set both to 1, we should be fine, right? > > There will be much more overhead drawn by frequent DMA transfers. > I believe you understand the idea -- less burst size then more DMA > request. Each DMA transfer contains a pair of handshaking overhead > according to the bus protocol. Apparently the bus will be wasted > with lots of handshaking section instead of keep dedicated to data > transfer truly. It might work if SSI is the only user of the bus, > which we shouldn't assume. Right, I knew there must be a trade off. So, how about I submit a patch with a fixed value of 4 for whichever platforms you think is correct. I see the 4 compatibles are: fsl,mpc8610-ssi, fsl,imx51-ssi, fsl,imx35-ssi, and fsl,imx21-ssi. Shall I submit a patch that sets the value to 4 for for any/all of the above? Should it be different based on whether fiq is enabled. Maybe set it to 4 for whenever DMA is used, and to 13 whenever FIQ is used? -Caleb
On Fri, Jan 15, 2016 at 11:10:29AM -0800, Caleb Crome wrote: > Shall I submit a patch that sets the value to 4 for for any/all of the > above? Should it be different based on whether fiq is enabled. Maybe > set it to 4 for whenever DMA is used, and to 13 whenever FIQ is used? A possible solution: Use the original settings (depth - 2) as a common configuration in the main probe() and overwrite it in the fsl_ssi_imx_probe() if detecting a use_dma flag.
On Fri, Jan 15, 2016 at 1:10 PM, Caleb Crome <caleb@crome.org> wrote: > > I see the 4 compatibles are: fsl,mpc8610-ssi, fsl,imx51-ssi, > fsl,imx35-ssi, and fsl,imx21-ssi. > > Shall I submit a patch that sets the value to 4 for for any/all of the > above? Should it be different based on whether fiq is enabled. Maybe > set it to 4 for whenever DMA is used, and to 13 whenever FIQ is used? I don't like the idea that we'll need a new device tree to make a new kernel work. The driver should continue to work with old device trees as-is.
On Fri, Jan 15, 2016 at 12:38 PM, Nicolin Chen <nicoleotsuka@gmail.com> wrote: > >> If nobody objects, we can just set the value to 4 and be done with it. > > I agree. And we may apply it only to i.MX platforms with DMA if > other platform owners feel comfortable with the previous settings. It's imperative that the PowerPC chips continue to use whatever setting they're using now. It took a long time for me to find values that actually work on those chips, and I don't want to have to re-test the driver on hardware that I don't have any more.
Caleb Crome wrote: > I see the 4 compatibles are: fsl,mpc8610-ssi, fsl,imx51-ssi, > fsl,imx35-ssi, and fsl,imx21-ssi. > > Shall I submit a patch that sets the value to 4 for for any/all of the > above? Should it be different based on whether fiq is enabled. Maybe > set it to 4 for whenever DMA is used, and to 13 whenever FIQ is used? Do NOT change the watermark value for fsl,mpc8610-ssi from whatever it is today. You would need to retest your code on an mpc8610 and p1022ds, and I don't think you have either of those boards.
Okay, I'll submit a patch that does the following: fsl,mpc8610: using DMA=fifo_depth-2 (unchanged from current), using FIQ=fifo_depth (unchanged from current). all others that are marked with 'imx': watermark&maxburst = 4 for DMA (new setting). watermark = fifo_depth for FIQ (unchanged from previous). -Caleb On Sat, Jan 16, 2016 at 6:15 AM, Timur Tabi <timur@tabi.org> wrote: > Caleb Crome wrote: >> >> I see the 4 compatibles are: fsl,mpc8610-ssi, fsl,imx51-ssi, >> fsl,imx35-ssi, and fsl,imx21-ssi. >> >> Shall I submit a patch that sets the value to 4 for for any/all of the >> above? Should it be different based on whether fiq is enabled. Maybe >> set it to 4 for whenever DMA is used, and to 13 whenever FIQ is used? > > > Do NOT change the watermark value for fsl,mpc8610-ssi from whatever it is > today. You would need to retest your code on an mpc8610 and p1022ds, and I > don't think you have either of those boards.
diff --git a/Documentation/devicetree/bindings/sound/fsl,ssi.txt b/Documentation/devicetree/bindings/sound/fsl,ssi.txt index 5b76be4..7af62b9 100644 --- a/Documentation/devicetree/bindings/sound/fsl,ssi.txt +++ b/Documentation/devicetree/bindings/sound/fsl,ssi.txt @@ -61,6 +61,16 @@ Optional properties: - fsl,mode: The operating mode for the AC97 interface only. "ac97-slave" - AC97 mode, SSI is clock slave "ac97-master" - AC97 mode, SSI is clock master +- fsl,fifo-watermark: Sets the fifo watermark. The default is + fifo_depth-2 words, meaning 'initiate dma transfer + when 2 words are left in the fifo'. At higher + data rates (48kHz, 16-channels for example), this + causes silent but deadly DMA xruns and channel + slips. For 15 word FIFOs (like on MX5, MX6) 8 is + a good value when running at high data rates +- fsl,dma-maxburst: sets the max number of words to transfer in DMA. + This defaults to the same value as + fsl,fifo-watermark. Child 'codec' node required properties: - compatible: Compatible list, contains the name of the codec diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 5cfc540..94d8b5d7 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -221,6 +221,12 @@ struct fsl_ssi_soc_data { * @dbg_stats: Debugging statistics * * @soc: SoC specific data + * + * @fifo_watermark: the FIFO watermark setting. Notifies DMA when + * there are @fifo_watermark or fewer words in TX fifo or + * @fifo_watermark or more empty words in RX fifo. + * @dma_maxburst: max number of words to transfer in one go. So far, + * this is always the same as fifo_watermark. */ struct fsl_ssi_private { struct regmap *regs; @@ -259,6 +265,9 @@ struct fsl_ssi_private { const struct fsl_ssi_soc_data *soc; struct device *dev; + + u32 fifo_watermark; + u32 dma_maxburst; }; /* @@ -1037,21 +1046,7 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev, regmap_write(regs, CCSR_SSI_SRCR, srcr); regmap_write(regs, CCSR_SSI_SCR, scr); - /* - * Set the watermark for transmit FIFI 0 and receive FIFO 0. We don't - * use FIFO 1. We program the transmit water to signal a DMA transfer - * if there are only two (or fewer) elements left in the FIFO. Two - * elements equals one frame (left channel, right channel). This value, - * however, depends on the depth of the transmit buffer. - * - * We set the watermark on the same level as the DMA burstsize. For - * fiq it is probably better to use the biggest possible watermark - * size. - */ - if (ssi_private->use_dma) - wm = ssi_private->fifo_depth - 2; - else - wm = ssi_private->fifo_depth; + wm = ssi_private->fifo_watermark; regmap_write(regs, CCSR_SSI_SFCSR, CCSR_SSI_SFCSR_TFWM0(wm) | CCSR_SSI_SFCSR_RFWM0(wm) | @@ -1359,12 +1354,8 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev, dev_dbg(&pdev->dev, "could not get baud clock: %ld\n", PTR_ERR(ssi_private->baudclk)); - /* - * We have burstsize be "fifo_depth - 2" to match the SSI - * watermark setting in fsl_ssi_startup(). - */ - ssi_private->dma_params_tx.maxburst = ssi_private->fifo_depth - 2; - ssi_private->dma_params_rx.maxburst = ssi_private->fifo_depth - 2; + ssi_private->dma_params_tx.maxburst = ssi_private->dma_maxburst; + ssi_private->dma_params_rx.maxburst = ssi_private->dma_maxburst; ssi_private->dma_params_tx.addr = ssi_private->ssi_phys + CCSR_SSI_STX0; ssi_private->dma_params_rx.addr = ssi_private->ssi_phys + CCSR_SSI_SRX0; @@ -1421,6 +1412,51 @@ static void fsl_ssi_imx_clean(struct platform_device *pdev, clk_disable_unprepare(ssi_private->clk); } +static void fsl_ssi_set_fifo_settings( + struct device_node *np, + struct fsl_ssi_private *ssi_private) +{ + const uint32_t *iprop; + /* + * Determine the FIFO depth. & watermark + * + * Set the watermark for transmit FIFO 0/1 and receive FIFO + * 0/1. We program the transmit water to signal a DMA transfer + * when this many words are left to be transmitted (or + * received). Previous setting was 2 elements, which was + * assumed to be one frame. Such a small value causes silent + * DMA xruns at high data rates. 8 seems to be a good + * tradeoff between xruns & number of DMA transfers. + * + * We set the watermark on the same level as the DMA burstsize. For + * fiq it is probably better to use the biggest possible watermark + * size. + */ + iprop = of_get_property(np, "fsl,fifo-depth", NULL); + if (iprop) + ssi_private->fifo_depth = be32_to_cpup(iprop); + else + /* Older 8610 DTs didn't have the fifo-depth property */ + ssi_private->fifo_depth = 8; + + iprop = of_get_property(np, "fsl,fifo-watermark", NULL); + if (iprop) + ssi_private->fifo_watermark = be32_to_cpup(iprop); + else + /* Default to old value of 2, which is too + * small at high data rates, but it's the + * default that's been there for years. + */ + ssi_private->fifo_watermark = ssi_private->fifo_depth - 2; + + iprop = of_get_property(np, "fsl,dma-maxburst", NULL); + + if (iprop) + ssi_private->dma_maxburst = be32_to_cpup(iprop); + else + ssi_private->dma_maxburst = ssi_private->fifo_watermark; +} + static int fsl_ssi_probe(struct platform_device *pdev) { struct fsl_ssi_private *ssi_private; @@ -1428,7 +1464,6 @@ static int fsl_ssi_probe(struct platform_device *pdev) struct device_node *np = pdev->dev.of_node; const struct of_device_id *of_id; const char *p, *sprop; - const uint32_t *iprop; struct resource *res; void __iomem *iomem; char name[64]; @@ -1510,13 +1545,7 @@ static int fsl_ssi_probe(struct platform_device *pdev) ssi_private->cpu_dai_drv.symmetric_samplebits = 1; } - /* Determine the FIFO depth. */ - iprop = of_get_property(np, "fsl,fifo-depth", NULL); - if (iprop) - ssi_private->fifo_depth = be32_to_cpup(iprop); - else - /* Older 8610 DTs didn't have the fifo-depth property */ - ssi_private->fifo_depth = 8; + fsl_ssi_set_fifo_settings(np, ssi_private); dev_set_drvdata(&pdev->dev, ssi_private);
Tuning the SSI fifo watermark & maxburst settings needs to be optimized differently depending on the demands on the system. The current default of 2 is too low for high data-rate systems. This patch maintains exactly the same behavior by default (i.e defaults to 2), but adds device tree options to set maxburst & fifo depth from the device tree. This is necessary because a setting of 2 simply doesn't work at higher data rates. Signed-off-by: Caleb Crome <caleb@crome.org> --- .../devicetree/bindings/sound/fsl,ssi.txt | 10 +++ sound/soc/fsl/fsl_ssi.c | 87 ++++++++++++++-------- 2 files changed, 68 insertions(+), 29 deletions(-)