Re: [PATCH 3/6] davinci: edma: clear events in edma_start()
"Sudhakar Rajashekhara" writes: > Hi, > > On Sat, Jun 05, 2010 at 02:05:50, Brian Niebuhr wrote: >> > >> >> This patch causes that the sound can not work normally >> > on OMAP_L138. >> > >> >> > >> After revert it, the audio works fine. >> > >> >> > > >> > > This patch works fine on DM644x which has McBSP but breaks >> > audio on OMAP L138 >> > > (as you had mentioned) which has McASP. Ideally McBSP/McASP >> > should start after >> > > EDMA is started. If not then this patch clears the EDMA >> > event which is actually >> > > set by McBSP/McASP. As this patch is working fine on >> > DM644x, I think there is >> > > some issue in the audio driver which needs to be debugged. >> > >> > In the mean time, I think it makes sense to revert $SUBJECT patch in >> > davinci git until the audio driver is debugged. >> >> Kevin - >> >> What is the process or timeline for getting this patch reapplied? I am >> working right now on resubmitting my patch for the Davinci SPI driver, >> however that driver won't work as is without this patch. I could >> probably hack something into the SPI driver to compensate, however if >> all are agreed that drivers shouldn't rely on DMA events that occur >> before the DMA is started, then it seems better to handle the issue in >> the DMA driver as I did in the reverted patch. [absentee maintainer re-surfaces after travel and some time off... sorry for the delays.] I'll be adding (back) your original patch as Sudhakar has found and fixed the root cause. > > I debugged this further today and I noticed that the issue of audio not > playing was observed only on DA830 and DA850 but not on DM6467 which also > has McASP. The only difference between McASP present on DM6467 and DA8XX > was hardware FIFO support. So when I disabled hardware FIFO on DA8XX devices, > even with this patch, audio started working correctly. Debugging further, I > found that, after doing FIFO configurations, FIFO was being enabled in > davinci_hw_common_param() function of sound/soc/davinci/davinci-mcasp.c file. > This was causing an extra McASP EDMA event to occur which was getting cleared > during edma_start(). This resulted in a state where EDMA was waiting for an > EDMA event from McASP, but the event which was already there, had got cleared. > Following patch fixes this issue. Sudhakar, Thanks for finding and fixing the root cause. Kevin ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH 3/6] davinci: edma: clear events in edma_start()
Hi,
On Sat, Jun 05, 2010 at 02:05:50, Brian Niebuhr wrote:
> > >> >> This patch causes that the sound can not work normally
> > on OMAP_L138.
> > >>
> > >> After revert it, the audio works fine.
> > >>
> > >
> > > This patch works fine on DM644x which has McBSP but breaks
> > audio on OMAP L138
> > > (as you had mentioned) which has McASP. Ideally McBSP/McASP
> > should start after
> > > EDMA is started. If not then this patch clears the EDMA
> > event which is actually
> > > set by McBSP/McASP. As this patch is working fine on
> > DM644x, I think there is
> > > some issue in the audio driver which needs to be debugged.
> >
> > In the mean time, I think it makes sense to revert $SUBJECT patch in
> > davinci git until the audio driver is debugged.
>
> Kevin -
>
> What is the process or timeline for getting this patch reapplied? I am
> working right now on resubmitting my patch for the Davinci SPI driver,
> however that driver won't work as is without this patch. I could
> probably hack something into the SPI driver to compensate, however if
> all are agreed that drivers shouldn't rely on DMA events that occur
> before the DMA is started, then it seems better to handle the issue in
> the DMA driver as I did in the reverted patch.
>
I debugged this further today and I noticed that the issue of audio not
playing was observed only on DA830 and DA850 but not on DM6467 which also
has McASP. The only difference between McASP present on DM6467 and DA8XX
was hardware FIFO support. So when I disabled hardware FIFO on DA8XX devices,
even with this patch, audio started working correctly. Debugging further, I
found that, after doing FIFO configurations, FIFO was being enabled in
davinci_hw_common_param() function of sound/soc/davinci/davinci-mcasp.c file.
This was causing an extra McASP EDMA event to occur which was getting cleared
during edma_start(). This resulted in a state where EDMA was waiting for an
EDMA event from McASP, but the event which was already there, had got cleared.
Following patch fixes this issue.
===
diff --git a/sound/soc/davinci/davinci-mcasp.c
b/sound/soc/davinci/davinci-mcasp.c
index 79f0f4a..d395509 100644
--- a/sound/soc/davinci/davinci-mcasp.c
+++ b/sound/soc/davinci/davinci-mcasp.c
@@ -612,7 +612,6 @@ static void davinci_hw_common_param(struct
davinci_audio_dev *dev, int stream)
NUMDMA_MASK);
mcasp_mod_bits(dev->base + DAVINCI_MCASP_WFIFOCTL,
((dev->txnumevt * tx_ser) << 8), NUMEVT_MASK);
- mcasp_set_bits(dev->base + DAVINCI_MCASP_WFIFOCTL, FIFO_ENABLE);
}
if (dev->rxnumevt && stream == SNDRV_PCM_STREAM_CAPTURE) {
@@ -623,7 +622,6 @@ static void davinci_hw_common_param(struct
davinci_audio_dev *dev, int stream)
NUMDMA_MASK);
mcasp_mod_bits(dev->base + DAVINCI_MCASP_RFIFOCTL,
((dev->rxnumevt * rx_ser) << 8), NUMEVT_MASK);
- mcasp_set_bits(dev->base + DAVINCI_MCASP_RFIFOCTL, FIFO_ENABLE);
}
}
===
I'll be submitting this patch to the list soon.
Regards,
Sudhakar
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH 3/6] davinci: edma: clear events in edma_start()
> >> >> This patch causes that the sound can not work normally > on OMAP_L138. > >> > >> After revert it, the audio works fine. > >> > > > > This patch works fine on DM644x which has McBSP but breaks > audio on OMAP L138 > > (as you had mentioned) which has McASP. Ideally McBSP/McASP > should start after > > EDMA is started. If not then this patch clears the EDMA > event which is actually > > set by McBSP/McASP. As this patch is working fine on > DM644x, I think there is > > some issue in the audio driver which needs to be debugged. > > In the mean time, I think it makes sense to revert $SUBJECT patch in > davinci git until the audio driver is debugged. Kevin - What is the process or timeline for getting this patch reapplied? I am working right now on resubmitting my patch for the Davinci SPI driver, however that driver won't work as is without this patch. I could probably hack something into the SPI driver to compensate, however if all are agreed that drivers shouldn't rely on DMA events that occur before the DMA is started, then it seems better to handle the issue in the DMA driver as I did in the reverted patch. Thanks! Brian ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 3/6] davinci: edma: clear events in edma_start()
"stanley.miao" writes: > Kevin Hilman wrote: >> >>> This patch causes that the sound can not work normally on OMAP_L138. >>> >>> >> >> Can you describe "can not work normally"? Does that mean simply does >> not work, or works with pops & clicks etc.? >> >> Just to clarify... how did you isolate it to this patch. >> >> If you revert just this patch on current davinci git, do you have >> working sound as you expect again? >> > Below is the test result: > > $/root> aplay audiodump.wav > Playing WAVE 'audiodump.wav' : Signed 16 bit Little Endian, Rate 44100 > Hz, Stereo > aplay: pcm_write:1269: write error: Input/output error > $/root> > > After revert it, the audio works fine. Thanks for clarifying. Kevin ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 3/6] davinci: edma: clear events in edma_start()
"Sudhakar Rajashekhara" writes: > Hi Stanley, > > On Tue, May 25, 2010 at 15:35:52, stanley.miao wrote: >> Kevin Hilman wrote: >> > >> >>> >> >>> >> >> This patch causes that the sound can not work normally on OMAP_L138. >> >> >> >> >> > >> > Can you describe "can not work normally"? Does that mean simply does >> > not work, or works with pops & clicks etc.? >> > >> > Just to clarify... how did you isolate it to this patch. >> > >> > If you revert just this patch on current davinci git, do you have >> > working sound as you expect again? >> > >> Below is the test result: >> >> $/root> aplay audiodump.wav >> Playing WAVE 'audiodump.wav' : Signed 16 bit Little Endian, Rate 44100 >> Hz, Stereo >> aplay: pcm_write:1269: write error: Input/output error >> $/root> >> >> After revert it, the audio works fine. >> > > This patch works fine on DM644x which has McBSP but breaks audio on OMAP L138 > (as you had mentioned) which has McASP. Ideally McBSP/McASP should start after > EDMA is started. If not then this patch clears the EDMA event which is > actually > set by McBSP/McASP. As this patch is working fine on DM644x, I think there is > some issue in the audio driver which needs to be debugged. In the mean time, I think it makes sense to revert $SUBJECT patch in davinci git until the audio driver is debugged. Kevin ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH 3/6] davinci: edma: clear events in edma_start()
Hi Stanley, On Tue, May 25, 2010 at 15:35:52, stanley.miao wrote: > Kevin Hilman wrote: > > > >>> > >>> > >> This patch causes that the sound can not work normally on OMAP_L138. > >> > >> > > > > Can you describe "can not work normally"? Does that mean simply does > > not work, or works with pops & clicks etc.? > > > > Just to clarify... how did you isolate it to this patch. > > > > If you revert just this patch on current davinci git, do you have > > working sound as you expect again? > > > Below is the test result: > > $/root> aplay audiodump.wav > Playing WAVE 'audiodump.wav' : Signed 16 bit Little Endian, Rate 44100 > Hz, Stereo > aplay: pcm_write:1269: write error: Input/output error > $/root> > > After revert it, the audio works fine. > This patch works fine on DM644x which has McBSP but breaks audio on OMAP L138 (as you had mentioned) which has McASP. Ideally McBSP/McASP should start after EDMA is started. If not then this patch clears the EDMA event which is actually set by McBSP/McASP. As this patch is working fine on DM644x, I think there is some issue in the audio driver which needs to be debugged. Regards, Sudhakar ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 3/6] davinci: edma: clear events in edma_start()
Kevin Hilman wrote:
This patch causes that the sound can not work normally on OMAP_L138.
Can you describe "can not work normally"? Does that mean simply does
not work, or works with pops & clicks etc.?
Just to clarify... how did you isolate it to this patch.
If you revert just this patch on current davinci git, do you have
working sound as you expect again?
Below is the test result:
$/root> aplay audiodump.wav
Playing WAVE 'audiodump.wav' : Signed 16 bit Little Endian, Rate 44100
Hz, Stereo
aplay: pcm_write:1269: write error: Input/output error
$/root>
After revert it, the audio works fine.
Hi, Brian,
For the SPI issue, try this patch:
--- a/drivers/spi/davinci_spi.c
+++ b/drivers/spi/davinci_spi.c
@@ -359,8 +359,10 @@ static void davinci_spi_dma_rx_callback(unsigned
lch, u16 ch_status, void *data)
if (ch_status == DMA_COMPLETE)
edma_stop(davinci_spi_dma->dma_rx_channel);
- else
+ else {
+ edma_stop(davinci_spi_dma->dma_rx_channel);
edma_clean_channel(davinci_spi_dma->dma_rx_channel);
+ }
complete(&davinci_spi_dma->dma_rx_completion);
/* We must disable the DMA RX request */
@@ -380,8 +382,10 @@ static void davinci_spi_dma_tx_callback(unsigned
lch, u16 ch_status, void *data)
if (ch_status == DMA_COMPLETE)
edma_stop(davinci_spi_dma->dma_tx_channel);
- else
+ else {
+ edma_stop(davinci_spi_dma->dma_tx_channel);
edma_clean_channel(davinci_spi_dma->dma_tx_channel);
+ }
complete(&davinci_spi_dma->dma_tx_completion);
-
Stanley.
Kevin
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 3/6] davinci: edma: clear events in edma_start()
[please don't top post, moved your reply below original...] "stanley.miao" writes: > Kevin Hilman wrote: >> From: Brian Niebuhr >> >> This patch fixes an issue where a DMA channel can erroneously process an >> event generated by a previous transfer. A failure case is where DMA is >> being used for SPI transmit and receive channels on OMAP L138. In this >> case there is a single bit that controls all event generation from the >> SPI peripheral. Therefore it is possible that between when edma_stop() >> has been called for the transmit channel on a previous transfer and >> edma_start() is called for the transmit channel on a subsequent transfer, >> that a transmit event has been generated. >> >> The fix is to clear events in edma_start(). This prevents false events >> from being processed when events are enabled for that channel. >> > > This patch causes that the sound can not work normally on OMAP_L138. > Can you describe "can not work normally"? Does that mean simply does not work, or works with pops & clicks etc.? Just to clarify... how did you isolate it to this patch. If you revert just this patch on current davinci git, do you have working sound as you expect again? Kevin ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH 3/6] davinci: edma: clear events in edma_start()
This patch causes that the sound can not work normally on OMAP_L138.
Stanley.
Kevin Hilman wrote:
From: Brian Niebuhr
This patch fixes an issue where a DMA channel can erroneously process an
event generated by a previous transfer. A failure case is where DMA is
being used for SPI transmit and receive channels on OMAP L138. In this
case there is a single bit that controls all event generation from the
SPI peripheral. Therefore it is possible that between when edma_stop()
has been called for the transmit channel on a previous transfer and
edma_start() is called for the transmit channel on a subsequent transfer,
that a transmit event has been generated.
The fix is to clear events in edma_start(). This prevents false events
from being processed when events are enabled for that channel.
Signed-off-by: Brian Niebuhr
Signed-off-by: Kevin Hilman
---
arch/arm/mach-davinci/dma.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
index 15dd886..260485c 100644
--- a/arch/arm/mach-davinci/dma.c
+++ b/arch/arm/mach-davinci/dma.c
@@ -1266,7 +1266,8 @@ int edma_start(unsigned channel)
/* EDMA channel with event association */
pr_debug("EDMA: ER%d %08x\n", j,
edma_shadow0_read_array(ctlr, SH_ER, j));
- /* Clear any pending error */
+ /* Clear any pending event or error */
+ edma_write_array(ctlr, EDMA_ECR, j, mask);
edma_write_array(ctlr, EDMA_EMCR, j, mask);
/* Clear any SER */
edma_shadow0_write_array(ctlr, SH_SECR, j, mask);
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
