| Message ID | 1346417459-30042-3-git-send-email-gururaja.hebbar@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Aug 31, 2012 at 18:20:58, Hebbar, Gururaja wrote: > OMAP2+ platforms (like TI81xx, AM33xx) uses davinci-pcm driver. SRAM is > not needed/used in EDMA transfers for audio on such platforms. However > they do not provide generic SRAM APIs (sram_alloc() and sram_free()). In > such cases the pcm driver cannot be used directly as it results in > compilation failure on OMAP2 platforms. > > Fix this by providing a config option to indicate missing SRAM API > support. This config is enabled by default on Davinci platform so as to > not break existing platforms. For OMAP2+ platforms, the config is > disabled. Gentle ping. Is there any comments/review for this patch? If not, can this be pulled in? > > Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com> > --- > :100644 100644 9e11a14... 328b463... M sound/soc/davinci/Kconfig > :100644 100644 93ea3bf... 7ac5a19... M sound/soc/davinci/davinci-pcm.c > sound/soc/davinci/Kconfig | 5 +++++ > sound/soc/davinci/davinci-pcm.c | 8 ++++++++ > 2 files changed, 13 insertions(+), 0 deletions(-) > > diff --git a/sound/soc/davinci/Kconfig b/sound/soc/davinci/Kconfig > index 9e11a14..328b463 100644 > --- a/sound/soc/davinci/Kconfig > +++ b/sound/soc/davinci/Kconfig > @@ -15,6 +15,11 @@ config SND_DAVINCI_SOC_MCASP > config SND_DAVINCI_SOC_VCIF > tristate > > +config SND_DAVINCI_HAVE_SRAM > + bool > + default y if ARCH_DAVINCI=y > + default n if ARCH_OMAP=y > + > config SND_DAVINCI_SOC_EVM > tristate "SoC Audio support for DaVinci DM6446, DM355 or DM365 EVM" > depends on SND_DAVINCI_SOC > diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c > index 93ea3bf..7ac5a19 100644 > --- a/sound/soc/davinci/davinci-pcm.c > +++ b/sound/soc/davinci/davinci-pcm.c > @@ -23,7 +23,9 @@ > #include <sound/soc.h> > > #include <asm/dma.h> > +#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) > #include <mach/sram.h> > +#endif > > #include "davinci-pcm.h" > > @@ -259,6 +261,7 @@ static void davinci_pcm_dma_irq(unsigned link, u16 ch_status, void *data) > } > } > > +#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) > static int allocate_sram(struct snd_pcm_substream *substream, unsigned size, > struct snd_pcm_hardware *ppcm) > { > @@ -289,6 +292,7 @@ exit2: > exit1: > return -ENOMEM; > } > +#endif > > /* > * Only used with ping/pong. > @@ -676,7 +680,9 @@ static int davinci_pcm_open(struct snd_pcm_substream *substream) > > ppcm = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ? > &pcm_hardware_playback : &pcm_hardware_capture; > +#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) > allocate_sram(substream, params->sram_size, ppcm); > +#endif > snd_soc_set_runtime_hwparams(substream, ppcm); > /* ensure that buffer size is a multiple of period size */ > ret = snd_pcm_hw_constraint_integer(runtime, > @@ -817,11 +823,13 @@ static void davinci_pcm_free(struct snd_pcm *pcm) > dma_free_writecombine(pcm->card->dev, buf->bytes, > buf->area, buf->addr); > buf->area = NULL; > +#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) > iram_dma = buf->private_data; > if (iram_dma) { > sram_free(iram_dma->area, iram_dma->bytes); > kfree(iram_dma); > } > +#endif > } > } > > -- > 1.7.1 > > Regards, Gururaja
On Wed, Sep 12, 2012 at 07:50:31AM +0000, Hebbar, Gururaja wrote: > Gentle ping. Is there any comments/review for this patch? > If not, can this be pulled in? Don't send contentless quoted pings.
On Fri, Aug 31, 2012 at 06:20:58PM +0530, Hebbar, Gururaja wrote: > +config SND_DAVINCI_HAVE_SRAM > + bool > + default y if ARCH_DAVINCI=y > + default n if ARCH_OMAP=y > + I've been sitting on this mostly since it seems like a step back from multi-platform kernels (which is where we're trying to get to) and I've been trying to decide what the best approach is. I'm thinking that we do want a generic API for allocating this stuff, it's a fairly generic feature (there's TCMs as well). Adding ifdefs like this does just doesn't seem good.
On Sat, Sep 22, 2012 at 21:03:14, Mark Brown wrote: > On Fri, Aug 31, 2012 at 06:20:58PM +0530, Hebbar, Gururaja wrote: > > > +config SND_DAVINCI_HAVE_SRAM > > + bool > > + default y if ARCH_DAVINCI=y > > + default n if ARCH_OMAP=y > > + > > I've been sitting on this mostly since it seems like a step back from > multi-platform kernels (which is where we're trying to get to) and I've > been trying to decide what the best approach is. I'm thinking that we > do want a generic API for allocating this stuff, it's a fairly generic > feature (there's TCMs as well). > > Adding ifdefs like this does just doesn't seem good. > I have no knowledge about multi-platform builds yet. Currently this driver is shared between OMAP & DaVinci and both of them doesn't exist in single image build. There was a effort done for this SRAM Consolidation [1] but it didn't progress. Hence I took this approach as a time-being/workaround. This was we can get affected platforms (like AM335x) get going/working. [1]. http://patchwork.ozlabs.org/patch/104059/ Regards, Gururaja
On Thu, Sep 27, 2012 at 06:57:49AM +0000, Hebbar, Gururaja wrote: > I have no knowledge about multi-platform builds yet. > Currently this driver is shared between OMAP & DaVinci and both of them > doesn't exist in single image build. ...due to issues like having compile time things selecting between the platforms. In other words what these ifdefs would do is move us further away from that goal rather than towards it as we should be doing. > There was a effort done for this SRAM Consolidation [1] but it didn't progress. > Hence I took this approach as a time-being/workaround. This was we can get > affected platforms (like AM335x) get going/working. Looks like it just needs more people pushing it...
On 09/22/2012 06:33 PM, Mark Brown wrote: > On Fri, Aug 31, 2012 at 06:20:58PM +0530, Hebbar, Gururaja wrote: > >> +config SND_DAVINCI_HAVE_SRAM >> + bool >> + default y if ARCH_DAVINCI=y >> + default n if ARCH_OMAP=y >> + > > I've been sitting on this mostly since it seems like a step back from > multi-platform kernels (which is where we're trying to get to) and I've > been trying to decide what the best approach is. I'm thinking that we > do want a generic API for allocating this stuff, it's a fairly generic > feature (there's TCMs as well). > > Adding ifdefs like this does just doesn't seem good. I also agree that ifdef is not a good solution. It is better to have this information passed as device_data and via DT it can be decided based on the compatible property for the device.
Hi Mark, On 9/22/2012 9:03 PM, Mark Brown wrote: > On Fri, Aug 31, 2012 at 06:20:58PM +0530, Hebbar, Gururaja wrote: > >> +config SND_DAVINCI_HAVE_SRAM >> + bool >> + default y if ARCH_DAVINCI=y >> + default n if ARCH_OMAP=y >> + > > I've been sitting on this mostly since it seems like a step back from > multi-platform kernels (which is where we're trying to get to) and I've > been trying to decide what the best approach is. I'm thinking that we > do want a generic API for allocating this stuff, it's a fairly generic > feature (there's TCMs as well). > > Adding ifdefs like this does just doesn't seem good. How about converting to use genalloc instead of the DaVinci private SRAM API and passing the pool to be used via platform data? Matt Porter just used this approach to make the uio PRUSS driver usable on both DaVinci and AM335x. It suffered from the exact same problem. https://patchwork.kernel.org/patch/1522481/ Thanks, Sekhar
On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > I also agree that ifdef is not a good solution. > It is better to have this information passed as device_data and via DT it can > be decided based on the compatible property for the device. That's not really the problem here - the problem is that the APIs used to get the SRAM are DaVinci only so it's not possible to build on OMAP or other platforms. The SRAM code needs to move to a standard API.
On 02.10.2012 11:37, Mark Brown wrote: > On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > >> I also agree that ifdef is not a good solution. >> It is better to have this information passed as device_data and via DT it can >> be decided based on the compatible property for the device. > > That's not really the problem here - the problem is that the APIs used > to get the SRAM are DaVinci only so it's not possible to build on OMAP > or other platforms. The SRAM code needs to move to a standard API. What about following Matt Porter's idea and ignore the SRAM code entirely and port the entire PCM code to generic dmaengine code first? The EDMA driver needs to learn support for cyclic DMA for that, and I might give that a try in near future. Later on, the SRAM ping-pong code can get added back using genalloc functions, as Sekhar proposed. That needs to be done by someone who has access to a Davinci board though, I only have a AM33xx/OMAP here. Daniel
On 10/2/2012 4:03 PM, Daniel Mack wrote: > On 02.10.2012 11:37, Mark Brown wrote: >> On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: >> >>> I also agree that ifdef is not a good solution. >>> It is better to have this information passed as device_data and via DT it can >>> be decided based on the compatible property for the device. >> >> That's not really the problem here - the problem is that the APIs used >> to get the SRAM are DaVinci only so it's not possible to build on OMAP >> or other platforms. The SRAM code needs to move to a standard API. > > What about following Matt Porter's idea and ignore the SRAM code > entirely and port the entire PCM code to generic dmaengine code first? > The EDMA driver needs to learn support for cyclic DMA for that, and I > might give that a try in near future. > > Later on, the SRAM ping-pong code can get added back using genalloc > functions, as Sekhar proposed. That needs to be done by someone who has > access to a Davinci board though, I only have a AM33xx/OMAP here. We cannot "get rid" of SRAM code and add it back "later". It is required for most DaVinci parts. The SRAM code can be converted to use genalloc (conversion should be straightforward and I can help test it) and the code that uses SRAM can probably keep using the private EDMA API till the dmaengine EDMA driver has cyclic DMA support. Matt has already posted patches to move private EDMA APIs to a common location. That should keep AM335x build from breaking. Is this something that is feasible? Thanks, Sekhar
On 02.10.2012 13:06, Sekhar Nori wrote: > On 10/2/2012 4:03 PM, Daniel Mack wrote: >> On 02.10.2012 11:37, Mark Brown wrote: >>> On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: >>> >>>> I also agree that ifdef is not a good solution. >>>> It is better to have this information passed as device_data and via DT it can >>>> be decided based on the compatible property for the device. >>> >>> That's not really the problem here - the problem is that the APIs used >>> to get the SRAM are DaVinci only so it's not possible to build on OMAP >>> or other platforms. The SRAM code needs to move to a standard API. >> >> What about following Matt Porter's idea and ignore the SRAM code >> entirely and port the entire PCM code to generic dmaengine code first? >> The EDMA driver needs to learn support for cyclic DMA for that, and I >> might give that a try in near future. >> >> Later on, the SRAM ping-pong code can get added back using genalloc >> functions, as Sekhar proposed. That needs to be done by someone who has >> access to a Davinci board though, I only have a AM33xx/OMAP here. > > We cannot "get rid" of SRAM code and add it back "later". It is required > for most DaVinci parts. The SRAM code can be converted to use genalloc > (conversion should be straightforward and I can help test it) and the > code that uses SRAM can probably keep using the private EDMA API till > the dmaengine EDMA driver has cyclic DMA support. Matt has already > posted patches to move private EDMA APIs to a common location. That > should keep AM335x build from breaking. Is this something that is feasible? Yes - by "later" I just meant in a subsequent patch. But you're probably right, we can also do that first. I'm looking at that right now and the problem seems that we don't have a sane way to dynamically look up gen_pools independently of the actual run-time platform. All users of gen_pools seem to know which platform they run on and access a platform-specific pool. So I don't currently see how we could implement multi-platform code, gen_pools are fine but don't solve the problem here. Would it be an idea to add a char* member to gen_pools and a function that can be used to dynamically look it up again? If a buffer with a certain name exists, we can use it and install that ping-pong buffer, otherwise we just don't. While that would be easy to do, it's a tree-wide change. Is there a better way that I miss? Daniel
On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > On 09/22/2012 06:33 PM, Mark Brown wrote: > > On Fri, Aug 31, 2012 at 06:20:58PM +0530, Hebbar, Gururaja wrote: > > > >> +config SND_DAVINCI_HAVE_SRAM > >> + bool > >> + default y if ARCH_DAVINCI=y > >> + default n if ARCH_OMAP=y > >> + > > > > I've been sitting on this mostly since it seems like a step back from > > multi-platform kernels (which is where we're trying to get to) and I've > > been trying to decide what the best approach is. I'm thinking that we > > do want a generic API for allocating this stuff, it's a fairly generic > > feature (there's TCMs as well). > > > > Adding ifdefs like this does just doesn't seem good. > > I also agree that ifdef is not a good solution. > It is better to have this information passed as device_data and via DT it can > be decided based on the compatible property for the device. The driver is going to be used by both !DT and DT only platforms for a while so DT-centric solutions just don't make sense. There's a clean way to do this that was used for uio_pruss.c. When davinci is further along on DT conversion we can make use of the devicetree-based generic sram driver that's progressing along right now [1]. It needs some minor help to allow specifying the gen_pool allocation order, but it will work nicely for getting access to the right sram pool. -Matt [1] https://patchwork.kernel.org/patch/1421961/
On Tue, Oct 02, 2012 at 12:33:39PM +0200, Daniel Mack wrote: > On 02.10.2012 11:37, Mark Brown wrote: > > On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > > > >> I also agree that ifdef is not a good solution. > >> It is better to have this information passed as device_data and via DT it can > >> be decided based on the compatible property for the device. > > > > That's not really the problem here - the problem is that the APIs used > > to get the SRAM are DaVinci only so it's not possible to build on OMAP > > or other platforms. The SRAM code needs to move to a standard API. > > What about following Matt Porter's idea and ignore the SRAM code > entirely and port the entire PCM code to generic dmaengine code first? > The EDMA driver needs to learn support for cyclic DMA for that, and I > might give that a try in near future. > > Later on, the SRAM ping-pong code can get added back using genalloc > functions, as Sekhar proposed. That needs to be done by someone who has > access to a Davinci board though, I only have a AM33xx/OMAP here. I already backed away from that idea since the older SoCs without a FIFO absolutely need ping-pong buffering in SRAM. -Matt
On Tue, Oct 02, 2012 at 03:42:47PM +0200, Daniel Mack wrote: > On 02.10.2012 13:06, Sekhar Nori wrote: > > On 10/2/2012 4:03 PM, Daniel Mack wrote: > >> On 02.10.2012 11:37, Mark Brown wrote: > >>> On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > >>> > >>>> I also agree that ifdef is not a good solution. > >>>> It is better to have this information passed as device_data and via DT it can > >>>> be decided based on the compatible property for the device. > >>> > >>> That's not really the problem here - the problem is that the APIs used > >>> to get the SRAM are DaVinci only so it's not possible to build on OMAP > >>> or other platforms. The SRAM code needs to move to a standard API. > >> > >> What about following Matt Porter's idea and ignore the SRAM code > >> entirely and port the entire PCM code to generic dmaengine code first? > >> The EDMA driver needs to learn support for cyclic DMA for that, and I > >> might give that a try in near future. > >> > >> Later on, the SRAM ping-pong code can get added back using genalloc > >> functions, as Sekhar proposed. That needs to be done by someone who has > >> access to a Davinci board though, I only have a AM33xx/OMAP here. > > > > We cannot "get rid" of SRAM code and add it back "later". It is required > > for most DaVinci parts. The SRAM code can be converted to use genalloc > > (conversion should be straightforward and I can help test it) and the > > code that uses SRAM can probably keep using the private EDMA API till > > the dmaengine EDMA driver has cyclic DMA support. Matt has already > > posted patches to move private EDMA APIs to a common location. That > > should keep AM335x build from breaking. Is this something that is feasible? > > Yes - by "later" I just meant in a subsequent patch. But you're probably > right, we can also do that first. > > I'm looking at that right now and the problem seems that we don't have a > sane way to dynamically look up gen_pools independently of the actual > run-time platform. All users of gen_pools seem to know which platform > they run on and access a platform-specific pool. So I don't currently > see how we could implement multi-platform code, gen_pools are fine but > don't solve the problem here. > > Would it be an idea to add a char* member to gen_pools and a function > that can be used to dynamically look it up again? If a buffer with a > certain name exists, we can use it and install that ping-pong buffer, > otherwise we just don't. While that would be easy to do, it's a > tree-wide change. > > Is there a better way that I miss? At the high level there's two platform models we have to handle, the boot from board file !DT case, and then the boot from DT case. Since Davinci is just starting DT conversion, we mostly care about the !DT base in which the struct gen_pool * is passed in pdata to the ASoC driver. It is then selectable on a per-platform basis where the decision should be made. Given a separate discussion with Sekhar, we're only going to have one SRAM pool on any DaVinci part right now...this was only a question on L138 anyway. But regardless, the created gen_pool will be passed via pdata. Since DT conversion is starting and we need to consider that now, the idea there is to use the DT-based generic sram driver [1] such that when we do boot from DT on Davinci, the genpool is provided via phandle and the pointer extracted with the OF helpers that are part of the series. That's pretty much it. I'm reworking the backend support as discussed with Sekhar wrt to my uio_pruss series. I can post a standalone series that just replaces sram_* with genalloc for davinci ASoC. -Matt
On 02.10.2012 18:41, Matt Porter wrote: > On Tue, Oct 02, 2012 at 03:42:47PM +0200, Daniel Mack wrote: >> On 02.10.2012 13:06, Sekhar Nori wrote: >>> On 10/2/2012 4:03 PM, Daniel Mack wrote: >>>> On 02.10.2012 11:37, Mark Brown wrote: >>>>> On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: >>>>> >>>>>> I also agree that ifdef is not a good solution. >>>>>> It is better to have this information passed as device_data and via DT it can >>>>>> be decided based on the compatible property for the device. >>>>> >>>>> That's not really the problem here - the problem is that the APIs used >>>>> to get the SRAM are DaVinci only so it's not possible to build on OMAP >>>>> or other platforms. The SRAM code needs to move to a standard API. >>>> >>>> What about following Matt Porter's idea and ignore the SRAM code >>>> entirely and port the entire PCM code to generic dmaengine code first? >>>> The EDMA driver needs to learn support for cyclic DMA for that, and I >>>> might give that a try in near future. >>>> >>>> Later on, the SRAM ping-pong code can get added back using genalloc >>>> functions, as Sekhar proposed. That needs to be done by someone who has >>>> access to a Davinci board though, I only have a AM33xx/OMAP here. >>> >>> We cannot "get rid" of SRAM code and add it back "later". It is required >>> for most DaVinci parts. The SRAM code can be converted to use genalloc >>> (conversion should be straightforward and I can help test it) and the >>> code that uses SRAM can probably keep using the private EDMA API till >>> the dmaengine EDMA driver has cyclic DMA support. Matt has already >>> posted patches to move private EDMA APIs to a common location. That >>> should keep AM335x build from breaking. Is this something that is feasible? >> >> Yes - by "later" I just meant in a subsequent patch. But you're probably >> right, we can also do that first. >> >> I'm looking at that right now and the problem seems that we don't have a >> sane way to dynamically look up gen_pools independently of the actual >> run-time platform. All users of gen_pools seem to know which platform >> they run on and access a platform-specific pool. So I don't currently >> see how we could implement multi-platform code, gen_pools are fine but >> don't solve the problem here. >> >> Would it be an idea to add a char* member to gen_pools and a function >> that can be used to dynamically look it up again? If a buffer with a >> certain name exists, we can use it and install that ping-pong buffer, >> otherwise we just don't. While that would be easy to do, it's a >> tree-wide change. >> >> Is there a better way that I miss? > > At the high level there's two platform models we have to handle, the > boot from board file !DT case, and then the boot from DT case. Since > Davinci is just starting DT conversion, we mostly care about the !DT > base in which the struct gen_pool * is passed in pdata to the ASoC > driver. It is then selectable on a per-platform basis where the decision > should be made. > > Given a separate discussion with Sekhar, we're only going to have one > SRAM pool on any DaVinci part right now...this was only a question on > L138 anyway. But regardless, the created gen_pool will be passed via > pdata. I thought about this too, as mmp does it that way. > Since DT conversion is starting and we need to consider that now, > the idea there is to use the DT-based generic sram driver [1] such that > when we do boot from DT on Davinci, the genpool is provided via phandle > and the pointer extracted with the OF helpers that are part of the > series. A phandle is the cleanest way I think, yes. > That's pretty much it. I'm reworking the backend support as discussed > with Sekhar wrt to my uio_pruss series. I can post a standalone series > that just replaces sram_* with genalloc for davinci ASoC. As you can also test it, it would be easiest if you came up with a patch for that, yes. I can have a look at the dma bits laters, once my OMAP board finally works with the code as it currently stands. I'm still fighting with the mcasp driver right now ... Daniel
On Tue, Oct 02, 2012 at 06:50:14PM +0200, Daniel Mack wrote: > On 02.10.2012 18:41, Matt Porter wrote: > > On Tue, Oct 02, 2012 at 03:42:47PM +0200, Daniel Mack wrote: > >> On 02.10.2012 13:06, Sekhar Nori wrote: > >>> On 10/2/2012 4:03 PM, Daniel Mack wrote: > >>>> On 02.10.2012 11:37, Mark Brown wrote: > >>>>> On Tue, Oct 02, 2012 at 10:48:53AM +0300, Peter Ujfalusi wrote: > >>>>> > >>>>>> I also agree that ifdef is not a good solution. > >>>>>> It is better to have this information passed as device_data and via DT it can > >>>>>> be decided based on the compatible property for the device. > >>>>> > >>>>> That's not really the problem here - the problem is that the APIs used > >>>>> to get the SRAM are DaVinci only so it's not possible to build on OMAP > >>>>> or other platforms. The SRAM code needs to move to a standard API. > >>>> > >>>> What about following Matt Porter's idea and ignore the SRAM code > >>>> entirely and port the entire PCM code to generic dmaengine code first? > >>>> The EDMA driver needs to learn support for cyclic DMA for that, and I > >>>> might give that a try in near future. > >>>> > >>>> Later on, the SRAM ping-pong code can get added back using genalloc > >>>> functions, as Sekhar proposed. That needs to be done by someone who has > >>>> access to a Davinci board though, I only have a AM33xx/OMAP here. > >>> > >>> We cannot "get rid" of SRAM code and add it back "later". It is required > >>> for most DaVinci parts. The SRAM code can be converted to use genalloc > >>> (conversion should be straightforward and I can help test it) and the > >>> code that uses SRAM can probably keep using the private EDMA API till > >>> the dmaengine EDMA driver has cyclic DMA support. Matt has already > >>> posted patches to move private EDMA APIs to a common location. That > >>> should keep AM335x build from breaking. Is this something that is feasible? > >> > >> Yes - by "later" I just meant in a subsequent patch. But you're probably > >> right, we can also do that first. > >> > >> I'm looking at that right now and the problem seems that we don't have a > >> sane way to dynamically look up gen_pools independently of the actual > >> run-time platform. All users of gen_pools seem to know which platform > >> they run on and access a platform-specific pool. So I don't currently > >> see how we could implement multi-platform code, gen_pools are fine but > >> don't solve the problem here. > >> > >> Would it be an idea to add a char* member to gen_pools and a function > >> that can be used to dynamically look it up again? If a buffer with a > >> certain name exists, we can use it and install that ping-pong buffer, > >> otherwise we just don't. While that would be easy to do, it's a > >> tree-wide change. > >> > >> Is there a better way that I miss? > > > > At the high level there's two platform models we have to handle, the > > boot from board file !DT case, and then the boot from DT case. Since > > Davinci is just starting DT conversion, we mostly care about the !DT > > base in which the struct gen_pool * is passed in pdata to the ASoC > > driver. It is then selectable on a per-platform basis where the decision > > should be made. > > > > Given a separate discussion with Sekhar, we're only going to have one > > SRAM pool on any DaVinci part right now...this was only a question on > > L138 anyway. But regardless, the created gen_pool will be passed via > > pdata. > > I thought about this too, as mmp does it that way. > > > Since DT conversion is starting and we need to consider that now, > > the idea there is to use the DT-based generic sram driver [1] such that > > when we do boot from DT on Davinci, the genpool is provided via phandle > > and the pointer extracted with the OF helpers that are part of the > > series. > > A phandle is the cleanest way I think, yes. See the of_get_named_gen_pool() helper example in the series http://comments.gmane.org/gmane.linux.kernel/1352210 > > That's pretty much it. I'm reworking the backend support as discussed > > with Sekhar wrt to my uio_pruss series. I can post a standalone series > > that just replaces sram_* with genalloc for davinci ASoC. > > As you can also test it, it would be easiest if you came up with a patch > for that, yes. I can have a look at the dma bits laters, once my OMAP > board finally works with the code as it currently stands. I'm still > fighting with the mcasp driver right now ... Ok. Also, as Sekhar pointed out, dmaengine itself isn't a blocker since we can have AM33xx use the private EDMA API for ASoC until I finish cyclic DMA support. Handling the ping-pong and normal case transparently within the dmaengine driver will require a further look but at least genalloc is our first step there. -Matt
diff --git a/sound/soc/davinci/Kconfig b/sound/soc/davinci/Kconfig index 9e11a14..328b463 100644 --- a/sound/soc/davinci/Kconfig +++ b/sound/soc/davinci/Kconfig @@ -15,6 +15,11 @@ config SND_DAVINCI_SOC_MCASP config SND_DAVINCI_SOC_VCIF tristate +config SND_DAVINCI_HAVE_SRAM + bool + default y if ARCH_DAVINCI=y + default n if ARCH_OMAP=y + config SND_DAVINCI_SOC_EVM tristate "SoC Audio support for DaVinci DM6446, DM355 or DM365 EVM" depends on SND_DAVINCI_SOC diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c index 93ea3bf..7ac5a19 100644 --- a/sound/soc/davinci/davinci-pcm.c +++ b/sound/soc/davinci/davinci-pcm.c @@ -23,7 +23,9 @@ #include <sound/soc.h> #include <asm/dma.h> +#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) #include <mach/sram.h> +#endif #include "davinci-pcm.h" @@ -259,6 +261,7 @@ static void davinci_pcm_dma_irq(unsigned link, u16 ch_status, void *data) } } +#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) static int allocate_sram(struct snd_pcm_substream *substream, unsigned size, struct snd_pcm_hardware *ppcm) { @@ -289,6 +292,7 @@ exit2: exit1: return -ENOMEM; } +#endif /* * Only used with ping/pong. @@ -676,7 +680,9 @@ static int davinci_pcm_open(struct snd_pcm_substream *substream) ppcm = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ? &pcm_hardware_playback : &pcm_hardware_capture; +#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) allocate_sram(substream, params->sram_size, ppcm); +#endif snd_soc_set_runtime_hwparams(substream, ppcm); /* ensure that buffer size is a multiple of period size */ ret = snd_pcm_hw_constraint_integer(runtime, @@ -817,11 +823,13 @@ static void davinci_pcm_free(struct snd_pcm *pcm) dma_free_writecombine(pcm->card->dev, buf->bytes, buf->area, buf->addr); buf->area = NULL; +#if defined(CONFIG_SND_DAVINCI_HAVE_SRAM) iram_dma = buf->private_data; if (iram_dma) { sram_free(iram_dma->area, iram_dma->bytes); kfree(iram_dma); } +#endif } }
OMAP2+ platforms (like TI81xx, AM33xx) uses davinci-pcm driver. SRAM is not needed/used in EDMA transfers for audio on such platforms. However they do not provide generic SRAM APIs (sram_alloc() and sram_free()). In such cases the pcm driver cannot be used directly as it results in compilation failure on OMAP2 platforms. Fix this by providing a config option to indicate missing SRAM API support. This config is enabled by default on Davinci platform so as to not break existing platforms. For OMAP2+ platforms, the config is disabled. Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com> --- :100644 100644 9e11a14... 328b463... M sound/soc/davinci/Kconfig :100644 100644 93ea3bf... 7ac5a19... M sound/soc/davinci/davinci-pcm.c sound/soc/davinci/Kconfig | 5 +++++ sound/soc/davinci/davinci-pcm.c | 8 ++++++++ 2 files changed, 13 insertions(+), 0 deletions(-)