[alsa-devel] ASoC: fsi: Add FSI DMAEngine support
Hi Mark, Liam, and Paul
These are DMAEngine support for FSI driver
I created a lot of patches since FSI DMAEngine needed much modification. I hope it was good for reviewing. But some patches are just cleanup/tidyup/rename. Last patch is main DMAEngine patch. it support Tx case only at this point.
FSI will use PIO version if CPU/platform doesn't have DMAEngine support.
Paul
Could you please check #16 patch which modify arch/arm/mach-shmobile and arch/sh
Kuninori Morimoto (19): ASoC: fsi: reduce runtime calculation by using pre-setting ASoC: fsi: tidyup: fsi_stream_xx() functions were gathered ASoC: fsi: data push/pop calculation part was divided ASoC: fsi: rename fsi_dma_soft_xxx() to fsi_pio_xxx() ASoC: fsi: tidyup: move fsi_fifo_init() onto fsi_hw_startup() ASoC: fsi: remove unnecessary parameter from fsi_hw_shutdown() ASoC: fsi: rename fsi_stream_push/pop() to fsi_stream_init/quit() ASoC: fsi: modify fsi_pio_get_area() parameter and using position ASoC: fsi: re-define fsi_is_play() and fsi_stream_is_play() ASoC: fsi: use fsi_stream in fsi_get_current_fifo_samples() parameter ASoC: fsi: add fsi_stream_handler and PIO handler ASoC: fsi: tidyup: fsi_pio_xxx() are gathered ASoC: fsi: don't use is_play as a parameter of fsi functions ASoC: fsi: add .start_stop handler to fsi_stream_handler ASoC: fsi: fsi_stream_is_working() care substream->runtime ASoC: fsi: PortA/B information was controlled by sh_fsi_port_info ASoC: fsi: add .init/.quit handler support ASoC: fsi: fixup fsi_pointer() calculation method ASoC: fsi: Add DMAEngine support
Best regards --- Kuninori Morimoto
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 11 +++++------ 1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 3241e5b..0d78740 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -167,6 +167,7 @@ struct fsi_stream { int buff_sample_pos; /* sample position of ALSA buffer */ int period_samples; /* sample number / 1 period */ int period_pos; /* current period position */ + int sample_width; /* sample width */
int uerr_num; int oerr_num; @@ -406,6 +407,7 @@ static void fsi_stream_push(struct fsi_priv *fsi, io->buff_sample_pos = 0; io->period_samples = fsi_frame2sample(fsi, runtime->period_size); io->period_pos = 0; + io->sample_width = samples_to_bytes(runtime, 1); io->oerr_num = -1; /* ignore 1st err */ io->uerr_num = -1; /* ignore 1st err */ spin_unlock_irqrestore(&master->lock, flags); @@ -431,6 +433,7 @@ static void fsi_stream_pop(struct fsi_priv *fsi, int is_play) io->buff_sample_pos = 0; io->period_samples = 0; io->period_pos = 0; + io->sample_width = 0; io->oerr_num = 0; io->uerr_num = 0; spin_unlock_irqrestore(&master->lock, flags); @@ -752,7 +755,6 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream) int is_play = fsi_stream_is_play(stream); struct fsi_stream *io = fsi_get_stream(fsi, is_play); int sample_residues; - int sample_width; int samples; int samples_max; int over_period; @@ -780,9 +782,6 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream) io->buff_sample_pos = 0; }
- /* get 1 sample data width */ - sample_width = samples_to_bytes(runtime, 1); - /* get number of residue samples */ sample_residues = io->buff_sample_capa - io->buff_sample_pos;
@@ -798,7 +797,7 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
samples = sample_residues;
- switch (sample_width) { + switch (io->sample_width) { case 2: fn = fsi_dma_soft_push16; break; @@ -818,7 +817,7 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream) samples_max = sample_residues; samples = fsi_get_current_fifo_samples(fsi, is_play);
- switch (sample_width) { + switch (io->sample_width) { case 2: fn = fsi_dma_soft_pop16; break;
This patch gathered fsi_stream_xxx() functions in order to make it readable.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 115 ++++++++++++++++++++++++++-------------------------- 1 files changed, 57 insertions(+), 58 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 0d78740..162416e 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -338,22 +338,6 @@ static u32 fsi_get_info_flags(struct fsi_priv *fsi) master->info->portb_flags; }
-static inline int fsi_stream_is_play(int stream) -{ - return stream == SNDRV_PCM_STREAM_PLAYBACK; -} - -static inline int fsi_is_play(struct snd_pcm_substream *substream) -{ - return fsi_stream_is_play(substream->stream); -} - -static inline struct fsi_stream *fsi_get_stream(struct fsi_priv *fsi, - int is_play) -{ - return is_play ? &fsi->playback : &fsi->capture; -} - static u32 fsi_get_port_shift(struct fsi_priv *fsi, int is_play) { int is_porta = fsi_is_port_a(fsi); @@ -377,10 +361,60 @@ static int fsi_sample2frame(struct fsi_priv *fsi, int samples) return samples / fsi->chan_num; }
+static int fsi_get_current_fifo_samples(struct fsi_priv *fsi, int is_play) +{ + u32 status; + int frames; + + status = is_play ? + fsi_reg_read(fsi, DOFF_ST) : + fsi_reg_read(fsi, DIFF_ST); + + frames = 0x1ff & (status >> 8); + + return fsi_frame2sample(fsi, frames); +} + +static void fsi_count_fifo_err(struct fsi_priv *fsi) +{ + u32 ostatus = fsi_reg_read(fsi, DOFF_ST); + u32 istatus = fsi_reg_read(fsi, DIFF_ST); + + if (ostatus & ERR_OVER) + fsi->playback.oerr_num++; + + if (ostatus & ERR_UNDER) + fsi->playback.uerr_num++; + + if (istatus & ERR_OVER) + fsi->capture.oerr_num++; + + if (istatus & ERR_UNDER) + fsi->capture.uerr_num++; + + fsi_reg_write(fsi, DOFF_ST, 0); + fsi_reg_write(fsi, DIFF_ST, 0); +} + +/* + * fsi_stream_xx() function + */ +#define fsi_is_play(substream) fsi_stream_is_play(substream->stream) +static inline int fsi_stream_is_play(int stream) +{ + return stream == SNDRV_PCM_STREAM_PLAYBACK; +} + +static inline struct fsi_stream *fsi_stream_get(struct fsi_priv *fsi, + int is_play) +{ + return is_play ? &fsi->playback : &fsi->capture; +} + static int fsi_stream_is_working(struct fsi_priv *fsi, int is_play) { - struct fsi_stream *io = fsi_get_stream(fsi, is_play); + struct fsi_stream *io = fsi_stream_get(fsi, is_play); struct fsi_master *master = fsi_get_master(fsi); unsigned long flags; int ret; @@ -396,7 +430,7 @@ static void fsi_stream_push(struct fsi_priv *fsi, int is_play, struct snd_pcm_substream *substream) { - struct fsi_stream *io = fsi_get_stream(fsi, is_play); + struct fsi_stream *io = fsi_stream_get(fsi, is_play); struct snd_pcm_runtime *runtime = substream->runtime; struct fsi_master *master = fsi_get_master(fsi); unsigned long flags; @@ -415,7 +449,7 @@ static void fsi_stream_push(struct fsi_priv *fsi,
static void fsi_stream_pop(struct fsi_priv *fsi, int is_play) { - struct fsi_stream *io = fsi_get_stream(fsi, is_play); + struct fsi_stream *io = fsi_stream_get(fsi, is_play); struct snd_soc_dai *dai = fsi_get_dai(io->substream); struct fsi_master *master = fsi_get_master(fsi); unsigned long flags; @@ -439,41 +473,6 @@ static void fsi_stream_pop(struct fsi_priv *fsi, int is_play) spin_unlock_irqrestore(&master->lock, flags); }
-static int fsi_get_current_fifo_samples(struct fsi_priv *fsi, int is_play) -{ - u32 status; - int frames; - - status = is_play ? - fsi_reg_read(fsi, DOFF_ST) : - fsi_reg_read(fsi, DIFF_ST); - - frames = 0x1ff & (status >> 8); - - return fsi_frame2sample(fsi, frames); -} - -static void fsi_count_fifo_err(struct fsi_priv *fsi) -{ - u32 ostatus = fsi_reg_read(fsi, DOFF_ST); - u32 istatus = fsi_reg_read(fsi, DIFF_ST); - - if (ostatus & ERR_OVER) - fsi->playback.oerr_num++; - - if (ostatus & ERR_UNDER) - fsi->playback.uerr_num++; - - if (istatus & ERR_OVER) - fsi->capture.oerr_num++; - - if (istatus & ERR_UNDER) - fsi->capture.uerr_num++; - - fsi_reg_write(fsi, DOFF_ST, 0); - fsi_reg_write(fsi, DIFF_ST, 0); -} - /* * dma function */ @@ -481,7 +480,7 @@ static void fsi_count_fifo_err(struct fsi_priv *fsi) static u8 *fsi_dma_get_area(struct fsi_priv *fsi, int stream) { int is_play = fsi_stream_is_play(stream); - struct fsi_stream *io = fsi_get_stream(fsi, is_play); + struct fsi_stream *io = fsi_stream_get(fsi, is_play); struct snd_pcm_runtime *runtime = io->substream->runtime;
return runtime->dma_area + @@ -698,7 +697,7 @@ static void fsi_fifo_init(struct fsi_priv *fsi, struct device *dev) { struct fsi_master *master = fsi_get_master(fsi); - struct fsi_stream *io = fsi_get_stream(fsi, is_play); + struct fsi_stream *io = fsi_stream_get(fsi, is_play); u32 shift, i; int frame_capa;
@@ -753,7 +752,7 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream) struct snd_pcm_runtime *runtime; struct snd_pcm_substream *substream = NULL; int is_play = fsi_stream_is_play(stream); - struct fsi_stream *io = fsi_get_stream(fsi, is_play); + struct fsi_stream *io = fsi_stream_get(fsi, is_play); int sample_residues; int samples; int samples_max; @@ -1150,7 +1149,7 @@ static int fsi_hw_free(struct snd_pcm_substream *substream) static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream) { struct fsi_priv *fsi = fsi_get_priv(substream); - struct fsi_stream *io = fsi_get_stream(fsi, fsi_is_play(substream)); + struct fsi_stream *io = fsi_stream_get(fsi, fsi_is_play(substream)); int samples_pos = io->buff_sample_pos - 1;
if (samples_pos < 0)
On Fri, Feb 03, 2012 at 12:50:35AM -0800, Kuninori Morimoto wrote:
+/*
fsi_stream_xx() function- */
+#define fsi_is_play(substream) fsi_stream_is_play(substream->stream) +static inline int fsi_stream_is_play(int stream) +{
- return stream == SNDRV_PCM_STREAM_PLAYBACK;
+}
That #define should be an inline function. I'll apply this but please fix this incrementally.
Next transfer data size of "push" and "pop" had calculated on shared function. But it was not readable code. This patch divided it into for push, and for pop.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 104 ++++++++++++++++++++++----------------------------- 1 files changed, 45 insertions(+), 59 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 162416e..cbb5643 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -747,17 +747,14 @@ static void fsi_fifo_init(struct fsi_priv *fsi, } }
-static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream) +static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io, + void (*run16)(struct fsi_priv *fsi, int size), + void (*run32)(struct fsi_priv *fsi, int size), + int samples) { struct snd_pcm_runtime *runtime; - struct snd_pcm_substream *substream = NULL; - int is_play = fsi_stream_is_play(stream); - struct fsi_stream *io = fsi_stream_get(fsi, is_play); - int sample_residues; - int samples; - int samples_max; + struct snd_pcm_substream *substream; int over_period; - void (*fn)(struct fsi_priv *fsi, int size);
if (!fsi || !io->substream || @@ -781,57 +778,17 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream) io->buff_sample_pos = 0; }
- /* get number of residue samples */ - sample_residues = io->buff_sample_capa - io->buff_sample_pos; - - if (is_play) { - /* - * for play-back - * - * samples_max : number of FSI fifo free samples space - * samples : number of ALSA residue samples - */ - samples_max = io->fifo_sample_capa; - samples_max -= fsi_get_current_fifo_samples(fsi, is_play); - - samples = sample_residues; - - switch (io->sample_width) { - case 2: - fn = fsi_dma_soft_push16; - break; - case 4: - fn = fsi_dma_soft_push32; - break; - default: - return -EINVAL; - } - } else { - /* - * for capture - * - * samples_max : number of ALSA free samples space - * samples : number of samples in FSI fifo - */ - samples_max = sample_residues; - samples = fsi_get_current_fifo_samples(fsi, is_play); - - switch (io->sample_width) { - case 2: - fn = fsi_dma_soft_pop16; - break; - case 4: - fn = fsi_dma_soft_pop32; - break; - default: - return -EINVAL; - } + switch (io->sample_width) { + case 2: + run16(fsi, samples); + break; + case 4: + run32(fsi, samples); + break; + default: + return -EINVAL; }
- samples = min(samples, samples_max); - - fn(fsi, samples); - /* update buff_sample_pos */ io->buff_sample_pos += samples;
@@ -843,12 +800,41 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, int stream)
static int fsi_data_pop(struct fsi_priv *fsi) { - return fsi_fifo_data_ctrl(fsi, SNDRV_PCM_STREAM_CAPTURE); + int is_play = fsi_stream_is_play(SNDRV_PCM_STREAM_CAPTURE); + int sample_residues; /* samples in FSI fifo */ + int sample_space; /* ALSA free samples space */ + int samples; + struct fsi_stream *io = fsi_stream_get(fsi, is_play); + + sample_residues = fsi_get_current_fifo_samples(fsi, is_play); + sample_space = io->buff_sample_capa - io->buff_sample_pos; + + samples = min(sample_residues, sample_space); + + return fsi_fifo_data_ctrl(fsi, io, + fsi_dma_soft_pop16, + fsi_dma_soft_pop32, + samples); }
static int fsi_data_push(struct fsi_priv *fsi) { - return fsi_fifo_data_ctrl(fsi, SNDRV_PCM_STREAM_PLAYBACK); + int is_play = fsi_stream_is_play(SNDRV_PCM_STREAM_PLAYBACK); + int sample_residues; /* ALSA residue samples */ + int sample_space; /* FSI fifo free samples space */ + int samples; + struct fsi_stream *io = fsi_stream_get(fsi, is_play); + + sample_residues = io->buff_sample_capa - io->buff_sample_pos; + sample_space = io->fifo_sample_capa - + fsi_get_current_fifo_samples(fsi, is_play); + + samples = min(sample_residues, sample_space); + + return fsi_fifo_data_ctrl(fsi, io, + fsi_dma_soft_push16, + fsi_dma_soft_push32, + samples); }
static irqreturn_t fsi_interrupt(int irq, void *data)
This is preparation for DMAEngine support
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index cbb5643..05307ac 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -474,10 +474,10 @@ static void fsi_stream_pop(struct fsi_priv *fsi, int is_play) }
/* - * dma function + * pio function */
-static u8 *fsi_dma_get_area(struct fsi_priv *fsi, int stream) +static u8 *fsi_pio_get_area(struct fsi_priv *fsi, int stream) { int is_play = fsi_stream_is_play(stream); struct fsi_stream *io = fsi_stream_get(fsi, is_play); @@ -487,47 +487,47 @@ static u8 *fsi_dma_get_area(struct fsi_priv *fsi, int stream) samples_to_bytes(runtime, io->buff_sample_pos); }
-static void fsi_dma_soft_push16(struct fsi_priv *fsi, int num) +static void fsi_pio_push16(struct fsi_priv *fsi, int num) { u16 *start; int i;
- start = (u16 *)fsi_dma_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK); + start = (u16 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK);
for (i = 0; i < num; i++) fsi_reg_write(fsi, DODT, ((u32)*(start + i) << 8)); }
-static void fsi_dma_soft_pop16(struct fsi_priv *fsi, int num) +static void fsi_pio_pop16(struct fsi_priv *fsi, int num) { u16 *start; int i;
- start = (u16 *)fsi_dma_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE); + start = (u16 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE);
for (i = 0; i < num; i++) *(start + i) = (u16)(fsi_reg_read(fsi, DIDT) >> 8); }
-static void fsi_dma_soft_push32(struct fsi_priv *fsi, int num) +static void fsi_pio_push32(struct fsi_priv *fsi, int num) { u32 *start; int i;
- start = (u32 *)fsi_dma_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK); + start = (u32 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK);
for (i = 0; i < num; i++) fsi_reg_write(fsi, DODT, *(start + i)); }
-static void fsi_dma_soft_pop32(struct fsi_priv *fsi, int num) +static void fsi_pio_pop32(struct fsi_priv *fsi, int num) { u32 *start; int i;
- start = (u32 *)fsi_dma_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE); + start = (u32 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE);
for (i = 0; i < num; i++) *(start + i) = fsi_reg_read(fsi, DIDT); @@ -812,8 +812,8 @@ static int fsi_data_pop(struct fsi_priv *fsi) samples = min(sample_residues, sample_space);
return fsi_fifo_data_ctrl(fsi, io, - fsi_dma_soft_pop16, - fsi_dma_soft_pop32, + fsi_pio_pop16, + fsi_pio_pop32, samples); }
@@ -832,8 +832,8 @@ static int fsi_data_push(struct fsi_priv *fsi) samples = min(sample_residues, sample_space);
return fsi_fifo_data_ctrl(fsi, io, - fsi_dma_soft_push16, - fsi_dma_soft_push32, + fsi_pio_push16, + fsi_pio_push32, samples); }
fsi_fifo_init() is called only from fsi_hw_startup()
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 109 ++++++++++++++++++++++++++-------------------------- 1 files changed, 54 insertions(+), 55 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 05307ac..79485ed 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -692,61 +692,6 @@ static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable) /* * ctrl function */ -static void fsi_fifo_init(struct fsi_priv *fsi, - int is_play, - struct device *dev) -{ - struct fsi_master *master = fsi_get_master(fsi); - struct fsi_stream *io = fsi_stream_get(fsi, is_play); - u32 shift, i; - int frame_capa; - - /* get on-chip RAM capacity */ - shift = fsi_master_read(master, FIFO_SZ); - shift >>= fsi_get_port_shift(fsi, is_play); - shift &= FIFO_SZ_MASK; - frame_capa = 256 << shift; - dev_dbg(dev, "fifo = %d words\n", frame_capa); - - /* - * The maximum number of sample data varies depending - * on the number of channels selected for the format. - * - * FIFOs are used in 4-channel units in 3-channel mode - * and in 8-channel units in 5- to 7-channel mode - * meaning that more FIFOs than the required size of DPRAM - * are used. - * - * ex) if 256 words of DP-RAM is connected - * 1 channel: 256 (256 x 1 = 256) - * 2 channels: 128 (128 x 2 = 256) - * 3 channels: 64 ( 64 x 3 = 192) - * 4 channels: 64 ( 64 x 4 = 256) - * 5 channels: 32 ( 32 x 5 = 160) - * 6 channels: 32 ( 32 x 6 = 192) - * 7 channels: 32 ( 32 x 7 = 224) - * 8 channels: 32 ( 32 x 8 = 256) - */ - for (i = 1; i < fsi->chan_num; i <<= 1) - frame_capa >>= 1; - dev_dbg(dev, "%d channel %d store\n", - fsi->chan_num, frame_capa); - - io->fifo_sample_capa = fsi_frame2sample(fsi, frame_capa); - - /* - * set interrupt generation factor - * clear FIFO - */ - if (is_play) { - fsi_reg_write(fsi, DOFF_CTL, IRQ_HALF); - fsi_reg_mask_set(fsi, DOFF_CTL, FIFO_CLR, FIFO_CLR); - } else { - fsi_reg_write(fsi, DIFF_CTL, IRQ_HALF); - fsi_reg_mask_set(fsi, DIFF_CTL, FIFO_CLR, FIFO_CLR); - } -} - static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io, void (*run16)(struct fsi_priv *fsi, int size), void (*run32)(struct fsi_priv *fsi, int size), @@ -867,6 +812,60 @@ static irqreturn_t fsi_interrupt(int irq, void *data) /* * dai ops */ +static void fsi_fifo_init(struct fsi_priv *fsi, + int is_play, + struct device *dev) +{ + struct fsi_master *master = fsi_get_master(fsi); + struct fsi_stream *io = fsi_stream_get(fsi, is_play); + u32 shift, i; + int frame_capa; + + /* get on-chip RAM capacity */ + shift = fsi_master_read(master, FIFO_SZ); + shift >>= fsi_get_port_shift(fsi, is_play); + shift &= FIFO_SZ_MASK; + frame_capa = 256 << shift; + dev_dbg(dev, "fifo = %d words\n", frame_capa); + + /* + * The maximum number of sample data varies depending + * on the number of channels selected for the format. + * + * FIFOs are used in 4-channel units in 3-channel mode + * and in 8-channel units in 5- to 7-channel mode + * meaning that more FIFOs than the required size of DPRAM + * are used. + * + * ex) if 256 words of DP-RAM is connected + * 1 channel: 256 (256 x 1 = 256) + * 2 channels: 128 (128 x 2 = 256) + * 3 channels: 64 ( 64 x 3 = 192) + * 4 channels: 64 ( 64 x 4 = 256) + * 5 channels: 32 ( 32 x 5 = 160) + * 6 channels: 32 ( 32 x 6 = 192) + * 7 channels: 32 ( 32 x 7 = 224) + * 8 channels: 32 ( 32 x 8 = 256) + */ + for (i = 1; i < fsi->chan_num; i <<= 1) + frame_capa >>= 1; + dev_dbg(dev, "%d channel %d store\n", + fsi->chan_num, frame_capa); + + io->fifo_sample_capa = fsi_frame2sample(fsi, frame_capa); + + /* + * set interrupt generation factor + * clear FIFO + */ + if (is_play) { + fsi_reg_write(fsi, DOFF_CTL, IRQ_HALF); + fsi_reg_mask_set(fsi, DOFF_CTL, FIFO_CLR, FIFO_CLR); + } else { + fsi_reg_write(fsi, DIFF_CTL, IRQ_HALF); + fsi_reg_mask_set(fsi, DIFF_CTL, FIFO_CLR, FIFO_CLR); + } +}
static int fsi_hw_startup(struct fsi_priv *fsi, int is_play,
This is preparation for DMAEngine support
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 79485ed..a0a9c36 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -927,7 +927,6 @@ static int fsi_hw_startup(struct fsi_priv *fsi, }
static void fsi_hw_shutdown(struct fsi_priv *fsi, - int is_play, struct device *dev) { if (fsi_is_clk_master(fsi)) @@ -947,9 +946,8 @@ static void fsi_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct fsi_priv *fsi = fsi_get_priv(substream); - int is_play = fsi_is_play(substream);
- fsi_hw_shutdown(fsi, is_play, dai->dev); + fsi_hw_shutdown(fsi, dai->dev); fsi->rate = 0; }
@@ -1342,7 +1340,7 @@ static void __fsi_suspend(struct fsi_priv *fsi, return;
fsi_port_stop(fsi, is_play); - fsi_hw_shutdown(fsi, is_play, dev); + fsi_hw_shutdown(fsi, dev); }
static void __fsi_resume(struct fsi_priv *fsi,
This is preparation for DMAEngine support
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index a0a9c36..2e2663b 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -426,7 +426,7 @@ static int fsi_stream_is_working(struct fsi_priv *fsi, return ret; }
-static void fsi_stream_push(struct fsi_priv *fsi, +static void fsi_stream_init(struct fsi_priv *fsi, int is_play, struct snd_pcm_substream *substream) { @@ -447,7 +447,7 @@ static void fsi_stream_push(struct fsi_priv *fsi, spin_unlock_irqrestore(&master->lock, flags); }
-static void fsi_stream_pop(struct fsi_priv *fsi, int is_play) +static void fsi_stream_quit(struct fsi_priv *fsi, int is_play) { struct fsi_stream *io = fsi_stream_get(fsi, is_play); struct snd_soc_dai *dai = fsi_get_dai(io->substream); @@ -960,13 +960,13 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
switch (cmd) { case SNDRV_PCM_TRIGGER_START: - fsi_stream_push(fsi, is_play, substream); + fsi_stream_init(fsi, is_play, substream); ret = is_play ? fsi_data_push(fsi) : fsi_data_pop(fsi); fsi_port_start(fsi, is_play); break; case SNDRV_PCM_TRIGGER_STOP: fsi_port_stop(fsi, is_play); - fsi_stream_pop(fsi, is_play); + fsi_stream_quit(fsi, is_play); break; }
This patch modify fsi_pio_get_area() parameter to use struct fsi_stream, and used it on fsi_fifo_data_ctrl(). This is just prepare cleanup for DMAEngine support.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 43 +++++++++++++++++-------------------------- 1 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 2e2663b..c814d8a 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -477,58 +477,46 @@ static void fsi_stream_quit(struct fsi_priv *fsi, int is_play) * pio function */
-static u8 *fsi_pio_get_area(struct fsi_priv *fsi, int stream) +static u8 *fsi_pio_get_area(struct fsi_priv *fsi, struct fsi_stream *io) { - int is_play = fsi_stream_is_play(stream); - struct fsi_stream *io = fsi_stream_get(fsi, is_play); struct snd_pcm_runtime *runtime = io->substream->runtime;
return runtime->dma_area + samples_to_bytes(runtime, io->buff_sample_pos); }
-static void fsi_pio_push16(struct fsi_priv *fsi, int num) +static void fsi_pio_push16(struct fsi_priv *fsi, u8 *_buf, int num) { - u16 *start; + u16 *start = (u16 *)_buf; int i;
- start = (u16 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK); - for (i = 0; i < num; i++) fsi_reg_write(fsi, DODT, ((u32)*(start + i) << 8)); }
-static void fsi_pio_pop16(struct fsi_priv *fsi, int num) +static void fsi_pio_pop16(struct fsi_priv *fsi, u8 *_buf, int num) { - u16 *start; + u16 *start = (u16 *)_buf; int i;
- start = (u16 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE); - - for (i = 0; i < num; i++) *(start + i) = (u16)(fsi_reg_read(fsi, DIDT) >> 8); }
-static void fsi_pio_push32(struct fsi_priv *fsi, int num) +static void fsi_pio_push32(struct fsi_priv *fsi, u8 *_buf, int num) { - u32 *start; + u32 *start = (u32 *)_buf; int i;
- start = (u32 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_PLAYBACK); - - for (i = 0; i < num; i++) fsi_reg_write(fsi, DODT, *(start + i)); }
-static void fsi_pio_pop32(struct fsi_priv *fsi, int num) +static void fsi_pio_pop32(struct fsi_priv *fsi, u8 *_buf, int num) { - u32 *start; + u32 *start = (u32 *)_buf; int i;
- start = (u32 *)fsi_pio_get_area(fsi, SNDRV_PCM_STREAM_CAPTURE); - for (i = 0; i < num; i++) *(start + i) = fsi_reg_read(fsi, DIDT); } @@ -693,12 +681,13 @@ static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable) * ctrl function */ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io, - void (*run16)(struct fsi_priv *fsi, int size), - void (*run32)(struct fsi_priv *fsi, int size), - int samples) + void (*run16)(struct fsi_priv *fsi, u8 *buf, int samples), + void (*run32)(struct fsi_priv *fsi, u8 *buf, int samples), + int samples) { struct snd_pcm_runtime *runtime; struct snd_pcm_substream *substream; + u8 *buf; int over_period;
if (!fsi || @@ -723,12 +712,14 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io, io->buff_sample_pos = 0; }
+ buf = fsi_pio_get_area(fsi, io); + switch (io->sample_width) { case 2: - run16(fsi, samples); + run16(fsi, buf, samples); break; case 4: - run32(fsi, samples); + run32(fsi, buf, samples); break; default: return -EINVAL;
This patch re-define fsi_is_play() and fsi_stream_is_play(). fsi_data_pop/push() function keeps direct value of "is_play" at this point, but it will be removed soon. This is just prepare cleanup for DMAEngine support.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 15 ++++++++++----- 1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index c814d8a..1cbe474 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -296,6 +296,11 @@ static int fsi_is_spdif(struct fsi_priv *fsi) return fsi->spdif; }
+static int fsi_is_play(struct snd_pcm_substream *substream) +{ + return substream->stream == SNDRV_PCM_STREAM_PLAYBACK; +} + static struct snd_soc_dai *fsi_get_dai(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = substream->private_data; @@ -399,10 +404,10 @@ static void fsi_count_fifo_err(struct fsi_priv *fsi) /* * fsi_stream_xx() function */ -#define fsi_is_play(substream) fsi_stream_is_play(substream->stream) -static inline int fsi_stream_is_play(int stream) +static inline int fsi_stream_is_play(struct fsi_priv *fsi, + struct fsi_stream *io) { - return stream == SNDRV_PCM_STREAM_PLAYBACK; + return &fsi->playback == io; }
static inline struct fsi_stream *fsi_stream_get(struct fsi_priv *fsi, @@ -736,7 +741,7 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io,
static int fsi_data_pop(struct fsi_priv *fsi) { - int is_play = fsi_stream_is_play(SNDRV_PCM_STREAM_CAPTURE); + int is_play = 0; int sample_residues; /* samples in FSI fifo */ int sample_space; /* ALSA free samples space */ int samples; @@ -755,7 +760,7 @@ static int fsi_data_pop(struct fsi_priv *fsi)
static int fsi_data_push(struct fsi_priv *fsi) { - int is_play = fsi_stream_is_play(SNDRV_PCM_STREAM_PLAYBACK); + int is_play = 1; int sample_residues; /* ALSA residue samples */ int sample_space; /* FSI fifo free samples space */ int samples;
On Fri, Feb 03, 2012 at 12:54:02AM -0800, Kuninori Morimoto wrote:
+static int fsi_is_play(struct snd_pcm_substream *substream) +{
- return substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
+}
Oh, good you fixed that up later.
fsi_get_current_fifo_samples() uses fsi_stream instead of is_play. This is just prepare cleanup for DMAEngine support.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 1cbe474..24dbe16 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -210,6 +210,8 @@ struct fsi_master { spinlock_t lock; };
+static int fsi_stream_is_play(struct fsi_priv *fsi, struct fsi_stream *io); + /* * basic read write function */ @@ -366,8 +368,10 @@ static int fsi_sample2frame(struct fsi_priv *fsi, int samples) return samples / fsi->chan_num; }
-static int fsi_get_current_fifo_samples(struct fsi_priv *fsi, int is_play) +static int fsi_get_current_fifo_samples(struct fsi_priv *fsi, + struct fsi_stream *io) { + int is_play = fsi_stream_is_play(fsi, io); u32 status; int frames;
@@ -747,7 +751,7 @@ static int fsi_data_pop(struct fsi_priv *fsi) int samples; struct fsi_stream *io = fsi_stream_get(fsi, is_play);
- sample_residues = fsi_get_current_fifo_samples(fsi, is_play); + sample_residues = fsi_get_current_fifo_samples(fsi, io); sample_space = io->buff_sample_capa - io->buff_sample_pos;
samples = min(sample_residues, sample_space); @@ -768,7 +772,7 @@ static int fsi_data_push(struct fsi_priv *fsi)
sample_residues = io->buff_sample_capa - io->buff_sample_pos; sample_space = io->fifo_sample_capa - - fsi_get_current_fifo_samples(fsi, is_play); + fsi_get_current_fifo_samples(fsi, io);
samples = min(sample_residues, sample_space);
This patch adds struct fsi_stream_handler and defined fsi_pio_push/pop_handler. these are controled by fsi_steam_xxx() function.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 118 insertions(+), 15 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 24dbe16..b02886a 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -159,18 +159,27 @@ typedef int (*set_rate_func)(struct device *dev, int is_porta, int rate, int ena * struct */
+struct fsi_stream_handler; struct fsi_stream { - struct snd_pcm_substream *substream;
+ /* + * these are initialized by fsi_stream_init() + */ + struct snd_pcm_substream *substream; int fifo_sample_capa; /* sample capacity of FSI FIFO */ int buff_sample_capa; /* sample capacity of ALSA buffer */ int buff_sample_pos; /* sample position of ALSA buffer */ int period_samples; /* sample number / 1 period */ int period_pos; /* current period position */ int sample_width; /* sample width */ - int uerr_num; int oerr_num; + + /* + * thse are initialized by fsi_handler_init() + */ + struct fsi_stream_handler *handler; + struct fsi_priv *priv; };
struct fsi_priv { @@ -190,6 +199,16 @@ struct fsi_priv { long rate; };
+struct fsi_stream_handler { + int (*probe)(struct fsi_priv *fsi, struct fsi_stream *io); + int (*transfer)(struct fsi_priv *fsi, struct fsi_stream *io); + int (*remove)(struct fsi_priv *fsi, struct fsi_stream *io); +}; +#define fsi_stream_handler_call(io, func, args...) \ + (!(io) ? -ENODEV : \ + !((io)->handler->func) ? 0 : \ + (io)->handler->func(args)) + struct fsi_core { int ver;
@@ -435,6 +454,11 @@ static int fsi_stream_is_working(struct fsi_priv *fsi, return ret; }
+static struct fsi_priv *fsi_stream_to_priv(struct fsi_stream *io) +{ + return io->priv; +} + static void fsi_stream_init(struct fsi_priv *fsi, int is_play, struct snd_pcm_substream *substream) @@ -482,6 +506,53 @@ static void fsi_stream_quit(struct fsi_priv *fsi, int is_play) spin_unlock_irqrestore(&master->lock, flags); }
+static int fsi_stream_transfer(struct fsi_stream *io) +{ + struct fsi_priv *fsi = fsi_stream_to_priv(io); + if (!fsi) + return -EIO; + + return fsi_stream_handler_call(io, transfer, fsi, io); +} + +static int fsi_stream_probe(struct fsi_priv *fsi) +{ + struct fsi_stream *io; + int ret1, ret2; + + io = &fsi->playback; + ret1 = fsi_stream_handler_call(io, probe, fsi, io); + + io = &fsi->capture; + ret2 = fsi_stream_handler_call(io, probe, fsi, io); + + if (ret1 < 0) + return ret1; + if (ret2 < 0) + return ret2; + + return 0; +} + +static int fsi_stream_remove(struct fsi_priv *fsi) +{ + struct fsi_stream *io; + int ret1, ret2; + + io = &fsi->playback; + ret1 = fsi_stream_handler_call(io, remove, fsi, io); + + io = &fsi->capture; + ret2 = fsi_stream_handler_call(io, remove, fsi, io); + + if (ret1 < 0) + return ret1; + if (ret2 < 0) + return ret2; + + return 0; +} + /* * pio function */ @@ -743,13 +814,11 @@ static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io, return 0; }
-static int fsi_data_pop(struct fsi_priv *fsi) +static int fsi_pio_pop(struct fsi_priv *fsi, struct fsi_stream *io) { - int is_play = 0; int sample_residues; /* samples in FSI fifo */ int sample_space; /* ALSA free samples space */ int samples; - struct fsi_stream *io = fsi_stream_get(fsi, is_play);
sample_residues = fsi_get_current_fifo_samples(fsi, io); sample_space = io->buff_sample_capa - io->buff_sample_pos; @@ -762,13 +831,11 @@ static int fsi_data_pop(struct fsi_priv *fsi) samples); }
-static int fsi_data_push(struct fsi_priv *fsi) +static int fsi_pio_push(struct fsi_priv *fsi, struct fsi_stream *io) { - int is_play = 1; int sample_residues; /* ALSA residue samples */ int sample_space; /* FSI fifo free samples space */ int samples; - struct fsi_stream *io = fsi_stream_get(fsi, is_play);
sample_residues = io->buff_sample_capa - io->buff_sample_pos; sample_space = io->fifo_sample_capa - @@ -782,6 +849,14 @@ static int fsi_data_push(struct fsi_priv *fsi) samples); }
+static struct fsi_stream_handler fsi_pio_push_handler = { + .transfer = fsi_pio_push, +}; + +static struct fsi_stream_handler fsi_pio_pop_handler = { + .transfer = fsi_pio_pop, +}; + static irqreturn_t fsi_interrupt(int irq, void *data) { struct fsi_master *master = data; @@ -792,13 +867,13 @@ static irqreturn_t fsi_interrupt(int irq, void *data) fsi_master_mask_set(master, SOFT_RST, IR, IR);
if (int_st & AB_IO(1, AO_SHIFT)) - fsi_data_push(&master->fsia); + fsi_stream_transfer(&master->fsia.playback); if (int_st & AB_IO(1, BO_SHIFT)) - fsi_data_push(&master->fsib); + fsi_stream_transfer(&master->fsib.playback); if (int_st & AB_IO(1, AI_SHIFT)) - fsi_data_pop(&master->fsia); + fsi_stream_transfer(&master->fsia.capture); if (int_st & AB_IO(1, BI_SHIFT)) - fsi_data_pop(&master->fsib); + fsi_stream_transfer(&master->fsib.capture);
fsi_count_fifo_err(&master->fsia); fsi_count_fifo_err(&master->fsib); @@ -955,14 +1030,16 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) { struct fsi_priv *fsi = fsi_get_priv(substream); + struct fsi_stream *io = fsi_stream_get(fsi, fsi_is_play(substream)); int is_play = fsi_is_play(substream); int ret = 0;
switch (cmd) { case SNDRV_PCM_TRIGGER_START: fsi_stream_init(fsi, is_play, substream); - ret = is_play ? fsi_data_push(fsi) : fsi_data_pop(fsi); - fsi_port_start(fsi, is_play); + ret = fsi_stream_transfer(io); + if (0 == ret) + fsi_port_start(fsi, is_play); break; case SNDRV_PCM_TRIGGER_STOP: fsi_port_stop(fsi, is_play); @@ -1224,6 +1301,13 @@ static struct snd_soc_platform_driver fsi_soc_platform = { /* * platform function */ +static void fsi_handler_init(struct fsi_priv *fsi) +{ + fsi->playback.handler = &fsi_pio_push_handler; /* default PIO */ + fsi->playback.priv = fsi; + fsi->capture.handler = &fsi_pio_pop_handler; /* default PIO */ + fsi->capture.priv = fsi; +}
static int fsi_probe(struct platform_device *pdev) { @@ -1270,10 +1354,22 @@ static int fsi_probe(struct platform_device *pdev) /* FSI A setting */ master->fsia.base = master->base; master->fsia.master = master; + fsi_handler_init(&master->fsia); + ret = fsi_stream_probe(&master->fsia); + if (ret < 0) { + dev_err(&pdev->dev, "FSIA stream probe failed\n"); + goto exit_iounmap; + }
/* FSI B setting */ master->fsib.base = master->base + 0x40; master->fsib.master = master; + fsi_handler_init(&master->fsib); + ret = fsi_stream_probe(&master->fsib); + if (ret < 0) { + dev_err(&pdev->dev, "FSIB stream probe failed\n"); + goto exit_fsia; + }
pm_runtime_enable(&pdev->dev); dev_set_drvdata(&pdev->dev, master); @@ -1282,7 +1378,7 @@ static int fsi_probe(struct platform_device *pdev) id_entry->name, master); if (ret) { dev_err(&pdev->dev, "irq request err\n"); - goto exit_iounmap; + goto exit_fsib; }
ret = snd_soc_register_platform(&pdev->dev, &fsi_soc_platform); @@ -1304,6 +1400,10 @@ exit_snd_soc: snd_soc_unregister_platform(&pdev->dev); exit_free_irq: free_irq(irq, master); +exit_fsib: + fsi_stream_remove(&master->fsib); +exit_fsia: + fsi_stream_remove(&master->fsia); exit_iounmap: iounmap(master->base); pm_runtime_disable(&pdev->dev); @@ -1326,6 +1426,9 @@ static int fsi_remove(struct platform_device *pdev) snd_soc_unregister_dais(&pdev->dev, ARRAY_SIZE(fsi_soc_dai)); snd_soc_unregister_platform(&pdev->dev);
+ fsi_stream_remove(&master->fsia); + fsi_stream_remove(&master->fsib); + iounmap(master->base); kfree(master);
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 101 +++++++++++++++++++++++++--------------------------- 1 files changed, 49 insertions(+), 52 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index b02886a..7c93b7c 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -554,54 +554,6 @@ static int fsi_stream_remove(struct fsi_priv *fsi) }
/* - * pio function - */ - -static u8 *fsi_pio_get_area(struct fsi_priv *fsi, struct fsi_stream *io) -{ - struct snd_pcm_runtime *runtime = io->substream->runtime; - - return runtime->dma_area + - samples_to_bytes(runtime, io->buff_sample_pos); -} - -static void fsi_pio_push16(struct fsi_priv *fsi, u8 *_buf, int num) -{ - u16 *start = (u16 *)_buf; - int i; - - for (i = 0; i < num; i++) - fsi_reg_write(fsi, DODT, ((u32)*(start + i) << 8)); -} - -static void fsi_pio_pop16(struct fsi_priv *fsi, u8 *_buf, int num) -{ - u16 *start = (u16 *)_buf; - int i; - - for (i = 0; i < num; i++) - *(start + i) = (u16)(fsi_reg_read(fsi, DIDT) >> 8); -} - -static void fsi_pio_push32(struct fsi_priv *fsi, u8 *_buf, int num) -{ - u32 *start = (u32 *)_buf; - int i; - - for (i = 0; i < num; i++) - fsi_reg_write(fsi, DODT, *(start + i)); -} - -static void fsi_pio_pop32(struct fsi_priv *fsi, u8 *_buf, int num) -{ - u32 *start = (u32 *)_buf; - int i; - - for (i = 0; i < num; i++) - *(start + i) = fsi_reg_read(fsi, DIDT); -} - -/* * irq function */
@@ -757,10 +709,55 @@ static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable) fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0); }
+ /* - * ctrl function + * pio data transfer handler */ -static int fsi_fifo_data_ctrl(struct fsi_priv *fsi, struct fsi_stream *io, +static void fsi_pio_push16(struct fsi_priv *fsi, u8 *_buf, int samples) +{ + u16 *buf = (u16 *)_buf; + int i; + + for (i = 0; i < samples; i++) + fsi_reg_write(fsi, DODT, ((u32)*(buf + i) << 8)); +} + +static void fsi_pio_pop16(struct fsi_priv *fsi, u8 *_buf, int samples) +{ + u16 *buf = (u16 *)_buf; + int i; + + for (i = 0; i < samples; i++) + *(buf + i) = (u16)(fsi_reg_read(fsi, DIDT) >> 8); +} + +static void fsi_pio_push32(struct fsi_priv *fsi, u8 *_buf, int samples) +{ + u32 *buf = (u32 *)_buf; + int i; + + for (i = 0; i < samples; i++) + fsi_reg_write(fsi, DODT, *(buf + i)); +} + +static void fsi_pio_pop32(struct fsi_priv *fsi, u8 *_buf, int samples) +{ + u32 *buf = (u32 *)_buf; + int i; + + for (i = 0; i < samples; i++) + *(buf + i) = fsi_reg_read(fsi, DIDT); +} + +static u8 *fsi_pio_get_area(struct fsi_priv *fsi, struct fsi_stream *io) +{ + struct snd_pcm_runtime *runtime = io->substream->runtime; + + return runtime->dma_area + + samples_to_bytes(runtime, io->buff_sample_pos); +} + +static int fsi_pio_transfer(struct fsi_priv *fsi, struct fsi_stream *io, void (*run16)(struct fsi_priv *fsi, u8 *buf, int samples), void (*run32)(struct fsi_priv *fsi, u8 *buf, int samples), int samples) @@ -825,7 +822,7 @@ static int fsi_pio_pop(struct fsi_priv *fsi, struct fsi_stream *io)
samples = min(sample_residues, sample_space);
- return fsi_fifo_data_ctrl(fsi, io, + return fsi_pio_transfer(fsi, io, fsi_pio_pop16, fsi_pio_pop32, samples); @@ -843,7 +840,7 @@ static int fsi_pio_push(struct fsi_priv *fsi, struct fsi_stream *io)
samples = min(sample_residues, sample_space);
- return fsi_fifo_data_ctrl(fsi, io, + return fsi_pio_transfer(fsi, io, fsi_pio_push16, fsi_pio_push32, samples);
is_play should be kept as local valuable. it prepare cleanup for DMAEngine support
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 94 +++++++++++++++++++++++++--------------------------- 1 files changed, 45 insertions(+), 49 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 7c93b7c..7dec144 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -364,8 +364,9 @@ static u32 fsi_get_info_flags(struct fsi_priv *fsi) master->info->portb_flags; }
-static u32 fsi_get_port_shift(struct fsi_priv *fsi, int is_play) +static u32 fsi_get_port_shift(struct fsi_priv *fsi, struct fsi_stream *io) { + int is_play = fsi_stream_is_play(fsi, io); int is_porta = fsi_is_port_a(fsi); u32 shift;
@@ -434,15 +435,14 @@ static inline int fsi_stream_is_play(struct fsi_priv *fsi, }
static inline struct fsi_stream *fsi_stream_get(struct fsi_priv *fsi, - int is_play) + struct snd_pcm_substream *substream) { - return is_play ? &fsi->playback : &fsi->capture; + return fsi_is_play(substream) ? &fsi->playback : &fsi->capture; }
static int fsi_stream_is_working(struct fsi_priv *fsi, - int is_play) + struct fsi_stream *io) { - struct fsi_stream *io = fsi_stream_get(fsi, is_play); struct fsi_master *master = fsi_get_master(fsi); unsigned long flags; int ret; @@ -460,10 +460,9 @@ static struct fsi_priv *fsi_stream_to_priv(struct fsi_stream *io) }
static void fsi_stream_init(struct fsi_priv *fsi, - int is_play, + struct fsi_stream *io, struct snd_pcm_substream *substream) { - struct fsi_stream *io = fsi_stream_get(fsi, is_play); struct snd_pcm_runtime *runtime = substream->runtime; struct fsi_master *master = fsi_get_master(fsi); unsigned long flags; @@ -480,9 +479,8 @@ static void fsi_stream_init(struct fsi_priv *fsi, spin_unlock_irqrestore(&master->lock, flags); }
-static void fsi_stream_quit(struct fsi_priv *fsi, int is_play) +static void fsi_stream_quit(struct fsi_priv *fsi, struct fsi_stream *io) { - struct fsi_stream *io = fsi_stream_get(fsi, is_play); struct snd_soc_dai *dai = fsi_get_dai(io->substream); struct fsi_master *master = fsi_get_master(fsi); unsigned long flags; @@ -557,18 +555,18 @@ static int fsi_stream_remove(struct fsi_priv *fsi) * irq function */
-static void fsi_irq_enable(struct fsi_priv *fsi, int is_play) +static void fsi_irq_enable(struct fsi_priv *fsi, struct fsi_stream *io) { - u32 data = AB_IO(1, fsi_get_port_shift(fsi, is_play)); + u32 data = AB_IO(1, fsi_get_port_shift(fsi, io)); struct fsi_master *master = fsi_get_master(fsi);
fsi_core_mask_set(master, imsk, data, data); fsi_core_mask_set(master, iemsk, data, data); }
-static void fsi_irq_disable(struct fsi_priv *fsi, int is_play) +static void fsi_irq_disable(struct fsi_priv *fsi, struct fsi_stream *io) { - u32 data = AB_IO(1, fsi_get_port_shift(fsi, is_play)); + u32 data = AB_IO(1, fsi_get_port_shift(fsi, io)); struct fsi_master *master = fsi_get_master(fsi);
fsi_core_mask_set(master, imsk, data, 0); @@ -585,8 +583,8 @@ static void fsi_irq_clear_status(struct fsi_priv *fsi) u32 data = 0; struct fsi_master *master = fsi_get_master(fsi);
- data |= AB_IO(1, fsi_get_port_shift(fsi, 0)); - data |= AB_IO(1, fsi_get_port_shift(fsi, 1)); + data |= AB_IO(1, fsi_get_port_shift(fsi, &fsi->playback)); + data |= AB_IO(1, fsi_get_port_shift(fsi, &fsi->capture));
/* clear interrupt factor */ fsi_core_mask_set(master, int_st, data, 0); @@ -695,15 +693,16 @@ static int fsi_set_master_clk(struct device *dev, struct fsi_priv *fsi,
#define fsi_port_start(f, i) __fsi_port_clk_ctrl(f, i, 1) #define fsi_port_stop(f, i) __fsi_port_clk_ctrl(f, i, 0) -static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, int is_play, int enable) +static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, struct fsi_stream *io, + int enable) { struct fsi_master *master = fsi_get_master(fsi); u32 clk = fsi_is_port_a(fsi) ? CRA : CRB;
if (enable) - fsi_irq_enable(fsi, is_play); + fsi_irq_enable(fsi, io); else - fsi_irq_disable(fsi, is_play); + fsi_irq_disable(fsi, io);
if (fsi_is_clk_master(fsi)) fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0); @@ -885,17 +884,17 @@ static irqreturn_t fsi_interrupt(int irq, void *data) * dai ops */ static void fsi_fifo_init(struct fsi_priv *fsi, - int is_play, + struct fsi_stream *io, struct device *dev) { struct fsi_master *master = fsi_get_master(fsi); - struct fsi_stream *io = fsi_stream_get(fsi, is_play); + int is_play = fsi_stream_is_play(fsi, io); u32 shift, i; int frame_capa;
/* get on-chip RAM capacity */ shift = fsi_master_read(master, FIFO_SZ); - shift >>= fsi_get_port_shift(fsi, is_play); + shift >>= fsi_get_port_shift(fsi, io); shift &= FIFO_SZ_MASK; frame_capa = 256 << shift; dev_dbg(dev, "fifo = %d words\n", frame_capa); @@ -940,7 +939,7 @@ static void fsi_fifo_init(struct fsi_priv *fsi, }
static int fsi_hw_startup(struct fsi_priv *fsi, - int is_play, + struct fsi_stream *io, struct device *dev) { struct fsi_master *master = fsi_get_master(fsi); @@ -989,11 +988,11 @@ static int fsi_hw_startup(struct fsi_priv *fsi, }
/* irq clear */ - fsi_irq_disable(fsi, is_play); + fsi_irq_disable(fsi, io); fsi_irq_clear_status(fsi);
/* fifo init */ - fsi_fifo_init(fsi, is_play, dev); + fsi_fifo_init(fsi, io, dev);
return 0; } @@ -1009,9 +1008,8 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct fsi_priv *fsi = fsi_get_priv(substream); - int is_play = fsi_is_play(substream);
- return fsi_hw_startup(fsi, is_play, dai->dev); + return fsi_hw_startup(fsi, fsi_stream_get(fsi, substream), dai->dev); }
static void fsi_dai_shutdown(struct snd_pcm_substream *substream, @@ -1027,20 +1025,19 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) { struct fsi_priv *fsi = fsi_get_priv(substream); - struct fsi_stream *io = fsi_stream_get(fsi, fsi_is_play(substream)); - int is_play = fsi_is_play(substream); + struct fsi_stream *io = fsi_stream_get(fsi, substream); int ret = 0;
switch (cmd) { case SNDRV_PCM_TRIGGER_START: - fsi_stream_init(fsi, is_play, substream); + fsi_stream_init(fsi, io, substream); ret = fsi_stream_transfer(io); if (0 == ret) - fsi_port_start(fsi, is_play); + fsi_port_start(fsi, io); break; case SNDRV_PCM_TRIGGER_STOP: - fsi_port_stop(fsi, is_play); - fsi_stream_quit(fsi, is_play); + fsi_port_stop(fsi, io); + fsi_stream_quit(fsi, io); break; }
@@ -1206,7 +1203,7 @@ static int fsi_hw_free(struct snd_pcm_substream *substream) static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream) { struct fsi_priv *fsi = fsi_get_priv(substream); - struct fsi_stream *io = fsi_stream_get(fsi, fsi_is_play(substream)); + struct fsi_stream *io = fsi_stream_get(fsi, substream); int samples_pos = io->buff_sample_pos - 1;
if (samples_pos < 0) @@ -1433,30 +1430,29 @@ static int fsi_remove(struct platform_device *pdev) }
static void __fsi_suspend(struct fsi_priv *fsi, - int is_play, + struct fsi_stream *io, struct device *dev) { - if (!fsi_stream_is_working(fsi, is_play)) + if (!fsi_stream_is_working(fsi, io)) return;
- fsi_port_stop(fsi, is_play); + fsi_port_stop(fsi, io); fsi_hw_shutdown(fsi, dev); }
static void __fsi_resume(struct fsi_priv *fsi, - int is_play, + struct fsi_stream *io, struct device *dev) { - if (!fsi_stream_is_working(fsi, is_play)) + if (!fsi_stream_is_working(fsi, io)) return;
- fsi_hw_startup(fsi, is_play, dev); + fsi_hw_startup(fsi, io, dev);
if (fsi_is_clk_master(fsi) && fsi->rate) fsi_set_master_clk(dev, fsi, fsi->rate, 1);
- fsi_port_start(fsi, is_play); - + fsi_port_start(fsi, io); }
static int fsi_suspend(struct device *dev) @@ -1465,11 +1461,11 @@ static int fsi_suspend(struct device *dev) struct fsi_priv *fsia = &master->fsia; struct fsi_priv *fsib = &master->fsib;
- __fsi_suspend(fsia, 1, dev); - __fsi_suspend(fsia, 0, dev); + __fsi_suspend(fsia, &fsia->playback, dev); + __fsi_suspend(fsia, &fsia->capture, dev);
- __fsi_suspend(fsib, 1, dev); - __fsi_suspend(fsib, 0, dev); + __fsi_suspend(fsib, &fsib->playback, dev); + __fsi_suspend(fsib, &fsib->capture, dev);
return 0; } @@ -1480,11 +1476,11 @@ static int fsi_resume(struct device *dev) struct fsi_priv *fsia = &master->fsia; struct fsi_priv *fsib = &master->fsib;
- __fsi_resume(fsia, 1, dev); - __fsi_resume(fsia, 0, dev); + __fsi_resume(fsia, &fsia->playback, dev); + __fsi_resume(fsia, &fsia->capture, dev);
- __fsi_resume(fsib, 1, dev); - __fsi_resume(fsib, 0, dev); + __fsi_resume(fsib, &fsib->playback, dev); + __fsi_resume(fsib, &fsib->capture, dev);
return 0; }
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 51 +++++++++++++++++++++++++++++---------------------- 1 files changed, 29 insertions(+), 22 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 7dec144..8d05e59 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -203,6 +203,8 @@ struct fsi_stream_handler { int (*probe)(struct fsi_priv *fsi, struct fsi_stream *io); int (*transfer)(struct fsi_priv *fsi, struct fsi_stream *io); int (*remove)(struct fsi_priv *fsi, struct fsi_stream *io); + void (*start_stop)(struct fsi_priv *fsi, struct fsi_stream *io, + int enable); }; #define fsi_stream_handler_call(io, func, args...) \ (!(io) ? -ENODEV : \ @@ -513,6 +515,12 @@ static int fsi_stream_transfer(struct fsi_stream *io) return fsi_stream_handler_call(io, transfer, fsi, io); }
+#define fsi_stream_start(fsi, io)\ + fsi_stream_handler_call(io, start_stop, fsi, io, 1) + +#define fsi_stream_stop(fsi, io)\ + fsi_stream_handler_call(io, start_stop, fsi, io, 0) + static int fsi_stream_probe(struct fsi_priv *fsi) { struct fsi_stream *io; @@ -691,24 +699,6 @@ static int fsi_set_master_clk(struct device *dev, struct fsi_priv *fsi, return ret; }
-#define fsi_port_start(f, i) __fsi_port_clk_ctrl(f, i, 1) -#define fsi_port_stop(f, i) __fsi_port_clk_ctrl(f, i, 0) -static void __fsi_port_clk_ctrl(struct fsi_priv *fsi, struct fsi_stream *io, - int enable) -{ - struct fsi_master *master = fsi_get_master(fsi); - u32 clk = fsi_is_port_a(fsi) ? CRA : CRB; - - if (enable) - fsi_irq_enable(fsi, io); - else - fsi_irq_disable(fsi, io); - - if (fsi_is_clk_master(fsi)) - fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0); -} - - /* * pio data transfer handler */ @@ -845,12 +835,29 @@ static int fsi_pio_push(struct fsi_priv *fsi, struct fsi_stream *io) samples); }
+static void fsi_pio_start_stop(struct fsi_priv *fsi, struct fsi_stream *io, + int enable) +{ + struct fsi_master *master = fsi_get_master(fsi); + u32 clk = fsi_is_port_a(fsi) ? CRA : CRB; + + if (enable) + fsi_irq_enable(fsi, io); + else + fsi_irq_disable(fsi, io); + + if (fsi_is_clk_master(fsi)) + fsi_master_mask_set(master, CLK_RST, clk, (enable) ? clk : 0); +} + static struct fsi_stream_handler fsi_pio_push_handler = { .transfer = fsi_pio_push, + .start_stop = fsi_pio_start_stop, };
static struct fsi_stream_handler fsi_pio_pop_handler = { .transfer = fsi_pio_pop, + .start_stop = fsi_pio_start_stop, };
static irqreturn_t fsi_interrupt(int irq, void *data) @@ -1033,10 +1040,10 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd, fsi_stream_init(fsi, io, substream); ret = fsi_stream_transfer(io); if (0 == ret) - fsi_port_start(fsi, io); + fsi_stream_start(fsi, io); break; case SNDRV_PCM_TRIGGER_STOP: - fsi_port_stop(fsi, io); + fsi_stream_stop(fsi, io); fsi_stream_quit(fsi, io); break; } @@ -1436,7 +1443,7 @@ static void __fsi_suspend(struct fsi_priv *fsi, if (!fsi_stream_is_working(fsi, io)) return;
- fsi_port_stop(fsi, io); + fsi_stream_stop(fsi, io); fsi_hw_shutdown(fsi, dev); }
@@ -1452,7 +1459,7 @@ static void __fsi_resume(struct fsi_priv *fsi, if (fsi_is_clk_master(fsi) && fsi->rate) fsi_set_master_clk(dev, fsi, fsi->rate, 1);
- fsi_port_start(fsi, io); + fsi_stream_start(fsi, io); }
static int fsi_suspend(struct device *dev)
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 8d05e59..1e10184 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -450,7 +450,7 @@ static int fsi_stream_is_working(struct fsi_priv *fsi, int ret;
spin_lock_irqsave(&master->lock, flags); - ret = !!io->substream; + ret = !!(io->substream && io->substream->runtime); spin_unlock_irqrestore(&master->lock, flags);
return ret; @@ -756,9 +756,7 @@ static int fsi_pio_transfer(struct fsi_priv *fsi, struct fsi_stream *io, u8 *buf; int over_period;
- if (!fsi || - !io->substream || - !io->substream->runtime) + if (!fsi_stream_is_working(fsi, io)) return -EINVAL;
over_period = 0;
Current FSI got each PortA/B parameter by porta_flags/portb_flags from platform. And .set_rate function was shared for PortA/B. This structure was not readable and not flexible. This patch adds sh_fsi_port_info, and its own settings was added on each platform. it is preparation for DMAEngine support
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com ---
Paul
Could you please check this patch ?
arch/arm/mach-shmobile/board-ap4evb.c | 30 ++++++++++------------------ arch/arm/mach-shmobile/board-mackerel.c | 18 +++++++--------- arch/sh/boards/mach-ecovec24/setup.c | 4 ++- arch/sh/boards/mach-se/7724/setup.c | 4 ++- include/sound/sh_fsi.h | 10 ++++++-- sound/soc/sh/fsi.c | 32 +++++++++++++++--------------- 6 files changed, 48 insertions(+), 50 deletions(-)
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c index aab0a34..2ac08b2 100644 --- a/arch/arm/mach-shmobile/board-ap4evb.c +++ b/arch/arm/mach-shmobile/board-ap4evb.c @@ -745,26 +745,18 @@ fsi_set_rate_end: return ret; }
-static int fsi_set_rate(struct device *dev, int is_porta, int rate, int enable) -{ - int ret; - - if (is_porta) - ret = fsi_ak4642_set_rate(dev, rate, enable); - else - ret = fsi_hdmi_set_rate(dev, rate, enable); - - return ret; -} - static struct sh_fsi_platform_info fsi_info = { - .porta_flags = SH_FSI_BRS_INV, - - .portb_flags = SH_FSI_BRS_INV | - SH_FSI_BRM_INV | - SH_FSI_LRS_INV | - SH_FSI_FMT_SPDIF, - .set_rate = fsi_set_rate, + .port_a = { + .flags = SH_FSI_BRS_INV, + .set_rate = fsi_ak4642_set_rate, + }, + .port_b = { + .flags = SH_FSI_BRS_INV | + SH_FSI_BRM_INV | + SH_FSI_LRS_INV | + SH_FSI_FMT_SPDIF, + .set_rate = fsi_hdmi_set_rate, + }, };
static struct resource fsi_resources[] = { diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c index 9b42fbd..28d6d1f 100644 --- a/arch/arm/mach-shmobile/board-mackerel.c +++ b/arch/arm/mach-shmobile/board-mackerel.c @@ -901,7 +901,7 @@ static int __fsi_set_round_rate(struct clk *clk, long rate, int enable) return clk_enable(clk); }
-static int fsi_set_rate(struct device *dev, int is_porta, int rate, int enable) +static int fsi_b_set_rate(struct device *dev, int rate, int enable) { struct clk *fsib_clk; struct clk *fdiv_clk = &sh7372_fsidivb_clk; @@ -910,10 +910,6 @@ static int fsi_set_rate(struct device *dev, int is_porta, int rate, int enable) int ackmd_bpfmd; int ret;
- /* FSIA is slave mode. nothing to do here */ - if (is_porta) - return 0; - /* clock start */ switch (rate) { case 44100: @@ -957,14 +953,16 @@ fsi_set_rate_end: }
static struct sh_fsi_platform_info fsi_info = { - .porta_flags = SH_FSI_BRS_INV, - - .portb_flags = SH_FSI_BRS_INV | + .port_a = { + .flags = SH_FSI_BRS_INV, + }, + .port_b = { + .flags = SH_FSI_BRS_INV | SH_FSI_BRM_INV | SH_FSI_LRS_INV | SH_FSI_FMT_SPDIF, - - .set_rate = fsi_set_rate, + .set_rate = fsi_b_set_rate, + } };
static struct resource fsi_resources[] = { diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c index 033ef2b..1e146fc 100644 --- a/arch/sh/boards/mach-ecovec24/setup.c +++ b/arch/sh/boards/mach-ecovec24/setup.c @@ -767,7 +767,9 @@ static struct platform_device camera_devices[] = {
/* FSI */ static struct sh_fsi_platform_info fsi_info = { - .portb_flags = SH_FSI_BRS_INV, + .port_b = { + .flags = SH_FSI_BRS_INV, + }, };
static struct resource fsi_resources[] = { diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c index 036fe1a..1fc5d47 100644 --- a/arch/sh/boards/mach-se/7724/setup.c +++ b/arch/sh/boards/mach-se/7724/setup.c @@ -277,7 +277,9 @@ static struct platform_device ceu1_device = { /* FSI */ /* change J20, J21, J22 pin to 1-2 connection to use slave mode */ static struct sh_fsi_platform_info fsi_info = { - .porta_flags = SH_FSI_BRS_INV, + .port_a = { + .flags = SH_FSI_BRS_INV, + }, };
static struct resource fsi_resources[] = { diff --git a/include/sound/sh_fsi.h b/include/sound/sh_fsi.h index 9b1aaca..78cd77a 100644 --- a/include/sound/sh_fsi.h +++ b/include/sound/sh_fsi.h @@ -72,10 +72,14 @@ #define SH_FSI_BPFMD_32 (5 << 4) #define SH_FSI_BPFMD_16 (6 << 4)
+struct sh_fsi_port_info { + unsigned long flags; + int (*set_rate)(struct device *dev, int rate, int enable); +}; + struct sh_fsi_platform_info { - unsigned long porta_flags; - unsigned long portb_flags; - int (*set_rate)(struct device *dev, int is_porta, int rate, int enable); + struct sh_fsi_port_info port_a; + struct sh_fsi_port_info port_b; };
/* diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 1e10184..75d0cda 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -116,7 +116,7 @@
#define FSI_FMTS (SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE)
-typedef int (*set_rate_func)(struct device *dev, int is_porta, int rate, int enable); +typedef int (*set_rate_func)(struct device *dev, int rate, int enable);
/* * FSI driver use below type name for variable @@ -185,6 +185,7 @@ struct fsi_stream { struct fsi_priv { void __iomem *base; struct fsi_master *master; + struct sh_fsi_port_info *info;
struct fsi_stream playback; struct fsi_stream capture; @@ -227,7 +228,6 @@ struct fsi_master { struct fsi_priv fsia; struct fsi_priv fsib; struct fsi_core *core; - struct sh_fsi_platform_info *info; spinlock_t lock; };
@@ -346,24 +346,20 @@ static struct fsi_priv *fsi_get_priv(struct snd_pcm_substream *substream) return fsi_get_priv_frm_dai(fsi_get_dai(substream)); }
-static set_rate_func fsi_get_info_set_rate(struct fsi_master *master) +static set_rate_func fsi_get_info_set_rate(struct fsi_priv *fsi) { - if (!master->info) + if (!fsi->info) return NULL;
- return master->info->set_rate; + return fsi->info->set_rate; }
static u32 fsi_get_info_flags(struct fsi_priv *fsi) { - int is_porta = fsi_is_port_a(fsi); - struct fsi_master *master = fsi_get_master(fsi); - - if (!master->info) + if (!fsi->info) return 0;
- return is_porta ? master->info->porta_flags : - master->info->portb_flags; + return fsi->info->flags; }
static u32 fsi_get_port_shift(struct fsi_priv *fsi, struct fsi_stream *io) @@ -628,11 +624,14 @@ static int fsi_set_master_clk(struct device *dev, struct fsi_priv *fsi, long rate, int enable) { struct fsi_master *master = fsi_get_master(fsi); - set_rate_func set_rate = fsi_get_info_set_rate(master); + set_rate_func set_rate = fsi_get_info_set_rate(fsi); int fsi_ver = master->core->ver; int ret;
- ret = set_rate(dev, fsi_is_port_a(fsi), rate, enable); + if (!set_rate) + return 0; + + ret = set_rate(dev, rate, enable); if (ret < 0) /* error */ return ret;
@@ -1093,8 +1092,7 @@ static int fsi_set_fmt_spdif(struct fsi_priv *fsi) static int fsi_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) { struct fsi_priv *fsi = fsi_get_priv_frm_dai(dai); - struct fsi_master *master = fsi_get_master(fsi); - set_rate_func set_rate = fsi_get_info_set_rate(master); + set_rate_func set_rate = fsi_get_info_set_rate(fsi); u32 flags = fsi_get_info_flags(fsi); int ret;
@@ -1312,6 +1310,7 @@ static int fsi_probe(struct platform_device *pdev) { struct fsi_master *master; const struct platform_device_id *id_entry; + struct sh_fsi_platform_info *info = pdev->dev.platform_data; struct resource *res; unsigned int irq; int ret; @@ -1346,13 +1345,13 @@ static int fsi_probe(struct platform_device *pdev)
/* master setting */ master->irq = irq; - master->info = pdev->dev.platform_data; master->core = (struct fsi_core *)id_entry->driver_data; spin_lock_init(&master->lock);
/* FSI A setting */ master->fsia.base = master->base; master->fsia.master = master; + master->fsia.info = &info->port_a; fsi_handler_init(&master->fsia); ret = fsi_stream_probe(&master->fsia); if (ret < 0) { @@ -1363,6 +1362,7 @@ static int fsi_probe(struct platform_device *pdev) /* FSI B setting */ master->fsib.base = master->base + 0x40; master->fsib.master = master; + master->fsib.info = &info->port_b; fsi_handler_init(&master->fsib); ret = fsi_stream_probe(&master->fsib); if (ret < 0) {
This is preparation for DMAEngine support
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 75d0cda..79a0afb 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -201,6 +201,8 @@ struct fsi_priv { };
struct fsi_stream_handler { + int (*init)(struct fsi_priv *fsi, struct fsi_stream *io); + int (*quit)(struct fsi_priv *fsi, struct fsi_stream *io); int (*probe)(struct fsi_priv *fsi, struct fsi_stream *io); int (*transfer)(struct fsi_priv *fsi, struct fsi_stream *io); int (*remove)(struct fsi_priv *fsi, struct fsi_stream *io); @@ -474,6 +476,7 @@ static void fsi_stream_init(struct fsi_priv *fsi, io->sample_width = samples_to_bytes(runtime, 1); io->oerr_num = -1; /* ignore 1st err */ io->uerr_num = -1; /* ignore 1st err */ + fsi_stream_handler_call(io, init, fsi, io); spin_unlock_irqrestore(&master->lock, flags); }
@@ -491,6 +494,7 @@ static void fsi_stream_quit(struct fsi_priv *fsi, struct fsi_stream *io) if (io->uerr_num > 0) dev_err(dai->dev, "under_run = %d\n", io->uerr_num);
+ fsi_stream_handler_call(io, quit, fsi, io); io->substream = NULL; io->buff_sample_capa = 0; io->buff_sample_pos = 0;
current fsi_pointer() calculation was not correct for FSI driver. This patch fix it up.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/fsi.c | 6 +----- 1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 79a0afb..1374680 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -1211,12 +1211,8 @@ static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream) { struct fsi_priv *fsi = fsi_get_priv(substream); struct fsi_stream *io = fsi_stream_get(fsi, substream); - int samples_pos = io->buff_sample_pos - 1;
- if (samples_pos < 0) - samples_pos = 0; - - return fsi_sample2frame(fsi, samples_pos); + return fsi_sample2frame(fsi, io->buff_sample_pos); }
static struct snd_pcm_ops fsi_pcm_ops = {
On Fri, Feb 03, 2012 at 12:59:15AM -0800, Kuninori Morimoto wrote:
current fsi_pointer() calculation was not correct for FSI driver. This patch fix it up.
This looks like a bug fix which we should send for version 3.3 but it's patch 18/19. Is there any dependency? If there is a dependency can you provide a version that applies against version 3.3 so that we can get the fix into the next release please.
Hi Mark
Thank you for checking patch
On Fri, Feb 03, 2012 at 12:59:15AM -0800, Kuninori Morimoto wrote:
current fsi_pointer() calculation was not correct for FSI driver. This patch fix it up.
This looks like a bug fix which we should send for version 3.3 but it's patch 18/19. Is there any dependency? If there is a dependency can you provide a version that applies against version 3.3 so that we can get the fix into the next release please.
It depend on DMAEngine series. This is bugfix patch for PIO/DMA, but PIO version FSI driver never got bug since it was sleeping bug.
But this comment seems not understandable. maybe it should be
"current fsi_pointer() calculation was not good much for fsi_stream_handler. This patch fix it up."
I can send v2 patch if you want it. Thank you.
Best regards --- Kuninori Morimoto
On Sun, Feb 05, 2012 at 04:36:28PM -0800, Kuninori Morimoto wrote:
It depend on DMAEngine series. This is bugfix patch for PIO/DMA, but PIO version FSI driver never got bug since it was sleeping bug.
I'm sorry but I'm having a hard time understanding what this means in the context of your patch. What is a "sleeping bug"?
Hi Mark
It depend on DMAEngine series. This is bugfix patch for PIO/DMA, but PIO version FSI driver never got bug since it was sleeping bug.
I'm sorry but I'm having a hard time understanding what this means in the context of your patch. What is a "sleeping bug"?
Sorry for my Engilish. It mean "(Maybe) old fsi_pointer() has bug, but this bug doesn't run a mischief"
Best regards --- Kuninori Morimoto
On Mon, Feb 06, 2012 at 04:13:20PM -0800, Kuninori Morimoto wrote:
I'm sorry but I'm having a hard time understanding what this means in the context of your patch. What is a "sleeping bug"?
Sorry for my Engilish. It mean "(Maybe) old fsi_pointer() has bug, but this bug doesn't run a mischief"
Ah, I see - so it probably was an existing bug but nobody reported it? I suspect it's worth fixing in 3.3 then, there's often issues where userspace applications don't work that don't get reported well or at all, often these are due to issues in the pointer implementations. People end up coming up with configurations that work around them and never think that there might be a driver issue.
Hi Mark
Thank you for your comment
I'm sorry but I'm having a hard time understanding what this means in the context of your patch. What is a "sleeping bug"?
Sorry for my Engilish. It mean "(Maybe) old fsi_pointer() has bug, but this bug doesn't run a mischief"
Ah, I see - so it probably was an existing bug but nobody reported it?
Yes
I suspect it's worth fixing in 3.3 then, there's often issues where userspace applications don't work that don't get reported well or at all, often these are due to issues in the pointer implementations. People end up coming up with configurations that work around them and never think that there might be a driver issue.
I'm not good at detail of user side applications, but aplay was working correctly on old/PIO FSI. So, I didn't notice it.
Best regards --- Kuninori Morimoto
On Tue, Feb 07, 2012 at 04:08:52PM -0800, Kuninori Morimoto wrote:
I suspect it's worth fixing in 3.3 then, there's often issues where userspace applications don't work that don't get reported well or at all, often these are due to issues in the pointer implementations. People end up coming up with configurations that work around them and never think that there might be a driver issue.
I'm not good at detail of user side applications, but aplay was working correctly on old/PIO FSI. So, I didn't notice it.
aplay is (un)fortunately pretty forgiving here - PulseAudio is the most difficult test, though some other applications do run into issues too. Anyway, can you please generate a version of this for 3.3 (or confirm if the current patch is OK for there) so we can integrate the change there?
The change itself seems good, it's just that it'd be better done as a fix.
current fsi_pointer() calculation was not correct for FSI driver. This patch fix it up.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
- this patch is based on latest linus/master (=v3.3.0-rc2)
Mark
I guess you will got some conflict around this fsi_pointer() when you merge.
sound/soc/sh/fsi.c | 6 +----- 1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index db6c89a..ea4a82d0 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -1152,12 +1152,8 @@ static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream) { struct fsi_priv *fsi = fsi_get_priv(substream); struct fsi_stream *io = fsi_get_stream(fsi, fsi_is_play(substream)); - int samples_pos = io->buff_sample_pos - 1;
- if (samples_pos < 0) - samples_pos = 0; - - return fsi_sample2frame(fsi, samples_pos); + return fsi_sample2frame(fsi, io->buff_sample_pos); }
static struct snd_pcm_ops fsi_pcm_ops = {
On Wed, Feb 08, 2012 at 04:57:29PM -0800, Kuninori Morimoto wrote:
I guess you will got some conflict around this fsi_pointer() when you merge.
Applied, thanks. I'll try to make sure that the conflict resolution looks like your original patch whenever I merge up from Linus' tree so hopefully shouldn't be too much hassle.
This patch supports DMAEngine to FSI driver. It supports only Tx case at this point. If platform/cpu doesn't support DMAEngine, FSI driver will use PIO transfer.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/sh_fsi.h | 2 + sound/soc/sh/fsi.c | 232 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 234 insertions(+), 0 deletions(-)
diff --git a/include/sound/sh_fsi.h b/include/sound/sh_fsi.h index 78cd77a..b457e87 100644 --- a/include/sound/sh_fsi.h +++ b/include/sound/sh_fsi.h @@ -74,6 +74,8 @@
struct sh_fsi_port_info { unsigned long flags; + int tx_id; + int rx_id; int (*set_rate)(struct device *dev, int rate, int enable); };
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 1374680..378cc5b 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -13,8 +13,11 @@ */
#include <linux/delay.h> +#include <linux/dma-mapping.h> #include <linux/pm_runtime.h> #include <linux/io.h> +#include <linux/scatterlist.h> +#include <linux/sh_dma.h> #include <linux/slab.h> #include <linux/module.h> #include <sound/soc.h> @@ -53,6 +56,7 @@
/* DO_FMT */ /* DI_FMT */ +#define CR_BWS_MASK (0x3 << 20) /* FSI2 */ #define CR_BWS_24 (0x0 << 20) /* FSI2 */ #define CR_BWS_16 (0x1 << 20) /* FSI2 */ #define CR_BWS_20 (0x2 << 20) /* FSI2 */ @@ -68,6 +72,15 @@ #define CR_TDM (0x4 << 4) #define CR_TDM_D (0x5 << 4)
+/* OUT_DMAC */ +/* IN_DMAC */ +#define VDMD_MASK (0x3 << 4) +#define VDMD_FRONT (0x0 << 4) /* Package in front */ +#define VDMD_BACK (0x1 << 4) /* Package in back */ +#define VDMD_STREAM (0x2 << 4) /* Stream mode(16bit * 2) */ + +#define DMA_ON (0x1 << 0) + /* DOFF_CTL */ /* DIFF_CTL */ #define IRQ_HALF 0x00100000 @@ -180,6 +193,14 @@ struct fsi_stream { */ struct fsi_stream_handler *handler; struct fsi_priv *priv; + + /* + * these are for DMAEngine + */ + struct dma_chan *chan; + struct sh_dmae_slave slave; /* see fsi_handler_init() */ + struct tasklet_struct tasklet; + dma_addr_t dma; };
struct fsi_priv { @@ -889,6 +910,212 @@ static irqreturn_t fsi_interrupt(int irq, void *data) }
/* + * dma data transfer handler + */ +static int fsi_dma_init(struct fsi_priv *fsi, struct fsi_stream *io) +{ + struct snd_pcm_runtime *runtime = io->substream->runtime; + struct snd_soc_dai *dai = fsi_get_dai(io->substream); + enum dma_data_direction dir = fsi_stream_is_play(fsi, io) ? + DMA_TO_DEVICE : DMA_FROM_DEVICE; + + io->dma = dma_map_single(dai->dev, runtime->dma_area, + snd_pcm_lib_buffer_bytes(io->substream), dir); + return 0; +} + +static int fsi_dma_quit(struct fsi_priv *fsi, struct fsi_stream *io) +{ + struct snd_soc_dai *dai = fsi_get_dai(io->substream); + enum dma_data_direction dir = fsi_stream_is_play(fsi, io) ? + DMA_TO_DEVICE : DMA_FROM_DEVICE; + + dma_unmap_single(dai->dev, io->dma, + snd_pcm_lib_buffer_bytes(io->substream), dir); + return 0; +} + +static void fsi_dma_complete(void *data) +{ + struct fsi_stream *io = (struct fsi_stream *)data; + struct fsi_priv *fsi = fsi_stream_to_priv(io); + struct snd_pcm_runtime *runtime = io->substream->runtime; + struct snd_soc_dai *dai = fsi_get_dai(io->substream); + enum dma_data_direction dir = fsi_stream_is_play(fsi, io) ? + DMA_TO_DEVICE : DMA_FROM_DEVICE; + + dma_sync_single_for_cpu(dai->dev, io->dma, + samples_to_bytes(runtime, io->period_samples), dir); + + io->buff_sample_pos += io->period_samples; + io->period_pos++; + + if (io->period_pos >= runtime->periods) { + io->period_pos = 0; + io->buff_sample_pos = 0; + } + + fsi_count_fifo_err(fsi); + fsi_stream_transfer(io); + + snd_pcm_period_elapsed(io->substream); +} + +static dma_addr_t fsi_dma_get_area(struct fsi_stream *io) +{ + struct snd_pcm_runtime *runtime = io->substream->runtime; + + return io->dma + samples_to_bytes(runtime, io->buff_sample_pos); +} + +static void fsi_dma_do_tasklet(unsigned long data) +{ + struct fsi_stream *io = (struct fsi_stream *)data; + struct fsi_priv *fsi = fsi_stream_to_priv(io); + struct dma_chan *chan; + struct snd_soc_dai *dai; + struct dma_async_tx_descriptor *desc; + struct scatterlist sg; + struct snd_pcm_runtime *runtime; + enum dma_data_direction dir; + dma_cookie_t cookie; + int is_play = fsi_stream_is_play(fsi, io); + int len; + dma_addr_t buf; + + if (!fsi_stream_is_working(fsi, io)) + return; + + dai = fsi_get_dai(io->substream); + chan = io->chan; + runtime = io->substream->runtime; + dir = is_play ? DMA_TO_DEVICE : DMA_FROM_DEVICE; + len = samples_to_bytes(runtime, io->period_samples); + buf = fsi_dma_get_area(io); + + dma_sync_single_for_device(dai->dev, io->dma, len, dir); + + sg_init_table(&sg, 1); + sg_set_page(&sg, pfn_to_page(PFN_DOWN(buf)), + len , offset_in_page(buf)); + sg_dma_address(&sg) = buf; + sg_dma_len(&sg) = len; + + desc = chan->device->device_prep_slave_sg(chan, &sg, 1, dir, + DMA_PREP_INTERRUPT | + DMA_CTRL_ACK); + if (!desc) { + dev_err(dai->dev, "device_prep_slave_sg() fail\n"); + return; + } + + desc->callback = fsi_dma_complete; + desc->callback_param = io; + + cookie = desc->tx_submit(desc); + if (cookie < 0) { + dev_err(dai->dev, "tx_submit() fail\n"); + return; + } + + dma_async_issue_pending(chan); + + /* + * FIXME + * + * In DMAEngine case, codec and FSI cannot be started simultaneously + * since FSI is using tasklet. + * Therefore, in capture case, probably FSI FIFO will have got + * overflow error in this point. + * in that case, DMA cannot start transfer until error was cleared. + */ + if (!is_play) { + if (ERR_OVER & fsi_reg_read(fsi, DIFF_ST)) { + fsi_reg_mask_set(fsi, DIFF_CTL, FIFO_CLR, FIFO_CLR); + fsi_reg_write(fsi, DIFF_ST, 0); + } + } +} + +static bool fsi_dma_filter(struct dma_chan *chan, void *param) +{ + struct sh_dmae_slave *slave = param; + + chan->private = slave; + + return true; +} + +static int fsi_dma_transfer(struct fsi_priv *fsi, struct fsi_stream *io) +{ + tasklet_schedule(&io->tasklet); + + return 0; +} + +static void fsi_dma_push_start_stop(struct fsi_priv *fsi, struct fsi_stream *io, + int start) +{ + u32 bws; + u32 dma; + + switch (io->sample_width * start) { + case 2: + bws = CR_BWS_16; + dma = VDMD_STREAM | DMA_ON; + break; + case 4: + bws = CR_BWS_24; + dma = VDMD_BACK | DMA_ON; + break; + default: + bws = 0; + dma = 0; + } + + fsi_reg_mask_set(fsi, DO_FMT, CR_BWS_MASK, bws); + fsi_reg_write(fsi, OUT_DMAC, dma); +} + +static int fsi_dma_probe(struct fsi_priv *fsi, struct fsi_stream *io) +{ + dma_cap_mask_t mask; + + dma_cap_zero(mask); + dma_cap_set(DMA_SLAVE, mask); + + io->chan = dma_request_channel(mask, fsi_dma_filter, &io->slave); + if (!io->chan) + return -EIO; + + tasklet_init(&io->tasklet, fsi_dma_do_tasklet, (unsigned long)io); + + return 0; +} + +static int fsi_dma_remove(struct fsi_priv *fsi, struct fsi_stream *io) +{ + tasklet_kill(&io->tasklet); + + fsi_stream_stop(fsi, io); + + if (io->chan) + dma_release_channel(io->chan); + + io->chan = NULL; + return 0; +} + +static struct fsi_stream_handler fsi_dma_push_handler = { + .init = fsi_dma_init, + .quit = fsi_dma_quit, + .probe = fsi_dma_probe, + .transfer = fsi_dma_transfer, + .remove = fsi_dma_remove, + .start_stop = fsi_dma_push_start_stop, +}; + +/* * dai ops */ static void fsi_fifo_init(struct fsi_priv *fsi, @@ -1304,6 +1531,11 @@ static void fsi_handler_init(struct fsi_priv *fsi) fsi->playback.priv = fsi; fsi->capture.handler = &fsi_pio_pop_handler; /* default PIO */ fsi->capture.priv = fsi; + + if (fsi->info->tx_id) { + fsi->playback.slave.slave_id = fsi->info->tx_id; + fsi->playback.handler = &fsi_dma_push_handler; + } }
static int fsi_probe(struct platform_device *pdev)
On Fri, Feb 03, 2012 at 12:59:33AM -0800, Kuninori Morimoto wrote:
+static void fsi_dma_do_tasklet(unsigned long data) +{
So, we're getting more and more drivers converted over to dmaengine (I've CCed a random selection of people working on relevant drivers and Vinod). This means we're getting a bunch of drivers that talk to the ALSA API on the top and the dmaengine API on the bottom and seem to need to implement very similar code patterns (like this tasklet to push the DMA through). This feels like we need some sort of abstraction to factor out some code here, either library code or perhaps a framework driver with callbacks that fill in the platform specifics - if the APIs are providing useful abstractions then presumably a large part of this is just translating between them.
If that's not doable then what are the problems?
On 02/03/2012 02:48 PM, Mark Brown wrote:
So, we're getting more and more drivers converted over to dmaengine (I've CCed a random selection of people working on relevant drivers and Vinod). This means we're getting a bunch of drivers that talk to the ALSA API on the top and the dmaengine API on the bottom and seem to need to implement very similar code patterns (like this tasklet to push the DMA through). This feels like we need some sort of abstraction to factor out some code here, either library code or perhaps a framework driver with callbacks that fill in the platform specifics - if the APIs are providing useful abstractions then presumably a large part of this is just translating between them.
If that's not doable then what are the problems?
Yes, I was faced with the task of writing yet another dmaengine based ASoC PCM driver last week. At first I thought it might be a good idea to come up with a common ASoC PCM driver which could be used by all platforms using DMA engine. Unfortunately this turned out to be not feasible due to how dmaengine works. Each dmaengine driver has it's custom API for configuring the DMA controller. :/
So what I have come up with so far is a set of helper functions which can be used to implement a dmaengine based ASoC PCM driver. Right now I'm in the process of converting imx, mxs and ep93xx to this set of functions.
These platforms have in common that their dmaengine driver implements device_prep_dma_cyclic function, which prepares a continuous transfer for cyclic buffer. The other ASoC platforms using dmaengine for their PCM driver (including the one in this patch series) don't have this function implemented and so the PCM drivers manually have to setup the cyclic SG lists. So while we could add a common set up functions for this to ASoC or ALSA I think the better place for this is in the dmaengine framework and implement device_prep_dma_cyclic for those drivers which don't do yet.
I'll try to send patches and some further explanations next week.
- Lars
On Fri, Feb 03, 2012 at 03:05:12PM +0100, Lars-Peter Clausen wrote:
Yes, I was faced with the task of writing yet another dmaengine based ASoC PCM driver last week. At first I thought it might be a good idea to come up with a common ASoC PCM driver which could be used by all platforms using DMA engine. Unfortunately this turned out to be not feasible due to how dmaengine works. Each dmaengine driver has it's custom API for configuring the DMA controller. :/
That doesn't sound insurmountible - you need a callback to allow the individual drivers to set the parameters but hopefully the data flow is more generic? There will probably be some other setup type stuff that needs doing.
The other ASoC platforms using dmaengine for their PCM driver (including the one in this patch series) don't have this function implemented and so the PCM drivers manually have to setup the cyclic SG lists. So while we could add a common set up functions for this to ASoC or ALSA I think the better place for this is in the dmaengine framework and implement device_prep_dma_cyclic for those drivers which don't do yet.
That seems like a good plan, that way other users of cyclic DMA will also benefit.
On Fri, 2012-02-03 at 14:14 +0000, Mark Brown wrote:
On Fri, Feb 03, 2012 at 03:05:12PM +0100, Lars-Peter Clausen wrote:
Yes, I was faced with the task of writing yet another dmaengine based ASoC PCM driver last week. At first I thought it might be a good idea to come up with a common ASoC PCM driver which could be used by all platforms using DMA engine. Unfortunately this turned out to be not feasible due to how dmaengine works. Each dmaengine driver has it's custom API for configuring the DMA controller. :/
Not really, if the driver is written properly its doable..
That doesn't sound insurmountible - you need a callback to allow the individual drivers to set the parameters but hopefully the data flow is more generic? There will probably be some other setup type stuff that needs doing.
I had though about this sometime back, and if we add dmaengine support into asoc, we wouldn't need to add this kind of support in every driver.
Typically, finding the right channel involves some handshaking between client and dma driver, so this is something the PCM driver should do as part of setup. The dma_chan can be part of the DAI, including the dma_add_t of the peripheral. Any required dmac parametrs should be set using dma_slave_config at setup. Once the stream is opened, asoc core on finding dma_chan in DAI should call the dmaengine APIs to get cyclic descriptor (in prepare) and then submit. The trigger START should invoke issue_pending callback of DMA. Similarly the trigger PAUSE, RESUME maps to dmanegine PAUSE and RESUME respectively (if implemented by dmac).
This way PCM driver would just need to setup the dma and not worry about data transfer as long as dma driver is properly done.
The other ASoC platforms using dmaengine for their PCM driver (including the one in this patch series) don't have this function implemented and so the PCM drivers manually have to setup the cyclic SG lists. So while we could add a common set up functions for this to ASoC or ALSA I think the better place for this is in the dmaengine framework and implement device_prep_dma_cyclic for those drivers which don't do yet.
That seems like a good plan, that way other users of cyclic DMA will also benefit.
On 02/04/2012 05:31 PM, Vinod Koul wrote:
On Fri, 2012-02-03 at 14:14 +0000, Mark Brown wrote:
On Fri, Feb 03, 2012 at 03:05:12PM +0100, Lars-Peter Clausen wrote:
Yes, I was faced with the task of writing yet another dmaengine based ASoC PCM driver last week. At first I thought it might be a good idea to come up with a common ASoC PCM driver which could be used by all platforms using DMA engine. Unfortunately this turned out to be not feasible due to how dmaengine works. Each dmaengine driver has it's custom API for configuring the DMA controller. :/
Not really, if the driver is written properly its doable..
yes, _if_. Though with the current state of the dmaengine drivers you need at least custom code for configuring the DMA controller and for matching the channel.
That doesn't sound insurmountible - you need a callback to allow the individual drivers to set the parameters but hopefully the data flow is more generic? There will probably be some other setup type stuff that needs doing.
I had though about this sometime back, and if we add dmaengine support into asoc, we wouldn't need to add this kind of support in every driver.
Typically, finding the right channel involves some handshaking between client and dma driver, so this is something the PCM driver should do as part of setup. The dma_chan can be part of the DAI, including the dma_add_t of the peripheral. Any required dmac parametrs should be set using dma_slave_config at setup. Once the stream is opened, asoc core on finding dma_chan in DAI should call the dmaengine APIs to get cyclic descriptor (in prepare) and then submit. The trigger START should invoke issue_pending callback of DMA. Similarly the trigger PAUSE, RESUME maps to dmanegine PAUSE and RESUME respectively (if implemented by dmac).
This is sort of what I have implemented right now. Although I also prepare and submit the dma descriptor in trigger START instead of prepare. Mainly beacuse I mapped trigger STOP to dmaengine_terminate_all. Or is issue_pending supposed to just restart the transfer if it is circular, even though terminate_all has been called?
I also have a helper function which maps a hw_params to a dma_slave_config. Individual drivers still have to implement their own hw_params callback since not all fields of a dma_slave_config can be deduced from a hw_params (e.g. the burst size).
- Lars
On Sat, 2012-02-04 at 18:00 +0100, Lars-Peter Clausen wrote:
yes, _if_. Though with the current state of the dmaengine drivers you need at least custom code for configuring the DMA controller and for matching the channel.
And that is why the dma_chan allocation should be done in setup code by PCM driver. Rest all the work in runtime should be moved into soc-pcm.
On Sat, 2012-02-04 at 18:00 +0100, Lars-Peter Clausen wrote:
This is sort of what I have implemented right now. Although I also prepare and submit the dma descriptor in trigger START instead of prepare. Mainly beacuse I mapped trigger STOP to dmaengine_terminate_all. Or is issue_pending supposed to just restart the transfer if it is circular, even though terminate_all has been called?
trigger START should not be used for preparing descriptors. You want to start to actually start the descriptor (which is issue_pending and NOT submit!!). You should do this is prepare callback.
I also have a helper function which maps a hw_params to a dma_slave_config. Individual drivers still have to implement their own hw_params callback since not all fields of a dma_slave_config can be deduced from a hw_params (e.g. the burst size).
yes, since dma_slave_config is dependent upon what you want to DMA and match that with peripheral, it can be done by PCM drivers only. Core cannot take care of this. That would also help, as channel is set rightly before core calls dmaengine APIs in prepare :-)
On 02/04/2012 07:06 PM, Vinod Koul wrote:
On Sat, 2012-02-04 at 18:00 +0100, Lars-Peter Clausen wrote:
This is sort of what I have implemented right now. Although I also prepare and submit the dma descriptor in trigger START instead of prepare. Mainly beacuse I mapped trigger STOP to dmaengine_terminate_all. Or is issue_pending supposed to just restart the transfer if it is circular, even though terminate_all has been called?
trigger START should not be used for preparing descriptors. You want to start to actually start the descriptor (which is issue_pending and NOT submit!!). You should do this is prepare callback.
I'm doing both, prepare+submit and issue_pending, because of what I wrote above. Or can we assume that each call to the ALSA/ASoC prepare callback will only be followed by at maximum one call to trigger START?
On Sat, 2012-02-04 at 19:25 +0100, Lars-Peter Clausen wrote:
On 02/04/2012 07:06 PM, Vinod Koul wrote:
On Sat, 2012-02-04 at 18:00 +0100, Lars-Peter Clausen wrote:
This is sort of what I have implemented right now. Although I also prepare and submit the dma descriptor in trigger START instead of prepare. Mainly beacuse I mapped trigger STOP to dmaengine_terminate_all. Or is issue_pending supposed to just restart the transfer if it is circular, even though terminate_all has been called?
trigger START should not be used for preparing descriptors. You want to start to actually start the descriptor (which is issue_pending and NOT submit!!). You should do this is prepare callback.
I'm doing both, prepare+submit and issue_pending, because of what I wrote above. Or can we assume that each call to the ALSA/ASoC prepare callback will only be followed by at maximum one call to trigger START?
terminate_all is designed to abort all transfers, so that would mean the dmac will move the descriptors to free_list. So that way your implementation is the right one.
And while at it, I believe for this use, terminate_all is too strong an API to be used, perhaps we should introduce a STOP call, which just aborts the current descriptor but doesn't free it, or possibly move to submitted state/queue. And yes terminate_all should used in scenarios like free callback. That way it should map properly to ALSA PCM operations.
On 02/04/2012 07:40 PM, Vinod Koul wrote:
[...]
And while at it, I believe for this use, terminate_all is too strong an API to be used, perhaps we should introduce a STOP call, which just aborts the current descriptor but doesn't free it, or possibly move to submitted state/queue. And yes terminate_all should used in scenarios like free callback. That way it should map properly to ALSA PCM operations.
Yes, a call that would allow us to reset/restart a transfer would be nice. While we are at it: Another nice addition, which would be good for sound drivers is a callback, which reports the transfer's current position in the datastream. This allows us to implement a pointer callback with sub-period granularity and also allow us to implement the 'no period wakeup' feature. A lot of DMA controllers support reporting this, but as far as I can see there is currently no way to export this information using dmaengine.
- Lars
On Mon, 2012-02-06 at 09:47 +0100, Lars-Peter Clausen wrote:
On 02/04/2012 07:40 PM, Vinod Koul wrote:
[...]
And while at it, I believe for this use, terminate_all is too strong an API to be used, perhaps we should introduce a STOP call, which just aborts the current descriptor but doesn't free it, or possibly move to submitted state/queue. And yes terminate_all should used in scenarios like free callback. That way it should map properly to ALSA PCM operations.
Yes, a call that would allow us to reset/restart a transfer would be nice. While we are at it: Another nice addition, which would be good for sound drivers is a callback, which reports the transfer's current position in the datastream. This allows us to implement a pointer callback with sub-period granularity and also allow us to implement the 'no period wakeup' feature. A lot of DMA controllers support reporting this, but as far as I can see there is currently no way to export this information using dmaengine.
What makes you think it's missing :-)
Pls see device_tx_status callback. You can return the descriptor position using this and enable "no wake mode"
- Lars
On 02/06/2012 03:46 PM, Vinod Koul wrote:
On Mon, 2012-02-06 at 09:47 +0100, Lars-Peter Clausen wrote:
On 02/04/2012 07:40 PM, Vinod Koul wrote:
[...]
And while at it, I believe for this use, terminate_all is too strong an API to be used, perhaps we should introduce a STOP call, which just aborts the current descriptor but doesn't free it, or possibly move to submitted state/queue. And yes terminate_all should used in scenarios like free callback. That way it should map properly to ALSA PCM operations.
Yes, a call that would allow us to reset/restart a transfer would be nice. While we are at it: Another nice addition, which would be good for sound drivers is a callback, which reports the transfer's current position in the datastream. This allows us to implement a pointer callback with sub-period granularity and also allow us to implement the 'no period wakeup' feature. A lot of DMA controllers support reporting this, but as far as I can see there is currently no way to export this information using dmaengine.
What makes you think it's missing :-)
Pls see device_tx_status callback. You can return the descriptor position using this and enable "no wake mode"
Ah nice, I somehow missed it, thanks.
- Lars
On Sat, Feb 04, 2012 at 10:01:32PM +0530, Vinod Koul wrote:
On Fri, 2012-02-03 at 14:14 +0000, Mark Brown wrote:
That doesn't sound insurmountible - you need a callback to allow the individual drivers to set the parameters but hopefully the data flow is more generic? There will probably be some other setup type stuff that needs doing.
I had though about this sometime back, and if we add dmaengine support into asoc, we wouldn't need to add this kind of support in every driver.
OK, this all sounds good and it seems like Lars-Peter has already been working on this. It doesn't seem like it's worth holding Morimoto-san's patch up for the infrastructure, though so once we work out what's going on with the bugfix patch I'll go ahead with that and we can convert it over to use the framework later.
On Fri, Feb 03, 2012 at 12:59:33AM -0800, Kuninori Morimoto wrote:
This patch supports DMAEngine to FSI driver. It supports only Tx case at this point. If platform/cpu doesn't support DMAEngine, FSI driver will use PIO transfer.
Applied, thanks! I have resolved the conflict with your bugfix patch which is now merged into Linus' tree so please check that I have done this correctly.
Hi Mark
[1 <text/plain; us-ascii (7bit)>] On Fri, Feb 03, 2012 at 12:59:33AM -0800, Kuninori Morimoto wrote:
This patch supports DMAEngine to FSI driver. It supports only Tx case at this point. If platform/cpu doesn't support DMAEngine, FSI driver will use PIO transfer.
Applied, thanks! I have resolved the conflict with your bugfix patch which is now merged into Linus' tree so please check that I have done this correctly.
Thank you for your hardwork. Yes. It seems correct.
Best regards --- Kuninori Morimoto
Hi Mark
Thank you for checking patch
These are DMAEngine support for FSI driver
Applied patches 1-17, thanks.
Thank you.
FSI will use PIO version if CPU/platform doesn't have DMAEngine support.
Is this a regression compared to the existing code?
It mean compatible for old platform (which needs PIO transfer) and new platform (which can use DMA transfer).
Best regards --- Kuninori Morimoto
participants (4)
-
Kuninori Morimoto -
Lars-Peter Clausen -
Mark Brown -
Vinod Koul