| Message ID | 20191217095851.19629-5-cezary.rojewski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: hda: Enable HDAudio compress | expand |
On Tue, 17 Dec 2019 10:58:48 +0100, Cezary Rojewski wrote: > > Update DMA buffer definition for snd_compr_runtime so it is represented > similarly as in snd_pcm_runtime. While at it, modify > snd_compr_set_runtime_buffer to account for newly added members. > > Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com> > --- > include/sound/compress_driver.h | 35 ++++++++++++++++++++++++--------- > 1 file changed, 26 insertions(+), 9 deletions(-) > > diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h > index bc88d6f964da..00f633c0c3ba 100644 > --- a/include/sound/compress_driver.h > +++ b/include/sound/compress_driver.h > @@ -23,7 +23,6 @@ struct snd_compr_ops; > * struct snd_compr_runtime: runtime stream description > * @state: stream state > * @ops: pointer to DSP callbacks > - * @dma_buffer_p: runtime dma buffer pointer > * @buffer: pointer to kernel buffer, valid only when not in mmap mode or > * DSP doesn't implement copy > * @buffer_size: size of the above buffer > @@ -34,11 +33,14 @@ struct snd_compr_ops; > * @total_bytes_transferred: cumulative bytes transferred by offload DSP > * @sleep: poll sleep > * @private_data: driver private data pointer > + * @dma_area: virtual buffer address > + * @dma_addr: physical buffer address (not accessible from main CPU) > + * @dma_bytes: size of DMA area > + * @dma_buffer_p: runtime dma buffer pointer > */ > struct snd_compr_runtime { > snd_pcm_state_t state; > struct snd_compr_ops *ops; > - struct snd_dma_buffer *dma_buffer_p; > void *buffer; > u64 buffer_size; > u32 fragment_size; > @@ -47,6 +49,11 @@ struct snd_compr_runtime { > u64 total_bytes_transferred; > wait_queue_head_t sleep; > void *private_data; > + > + unsigned char *dma_area; > + dma_addr_t dma_addr; > + size_t dma_bytes; > + struct snd_dma_buffer *dma_buffer_p; Why do we need to have both dma_buffer_p and its values? For consistency with PCM stream? For PCM, dma_area, dma_addr and dma_bytes are the primary data, which aren't necessarily set by the dma_buffer but manually set up by the driver. Just wondering. Takashi
On 2019-12-17 11:23, Takashi Iwai wrote: > On Tue, 17 Dec 2019 10:58:48 +0100, > Cezary Rojewski wrote: >> >> Update DMA buffer definition for snd_compr_runtime so it is represented >> similarly as in snd_pcm_runtime. While at it, modify >> snd_compr_set_runtime_buffer to account for newly added members. >> >> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com> >> --- >> include/sound/compress_driver.h | 35 ++++++++++++++++++++++++--------- >> 1 file changed, 26 insertions(+), 9 deletions(-) >> >> diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h >> index bc88d6f964da..00f633c0c3ba 100644 >> --- a/include/sound/compress_driver.h >> +++ b/include/sound/compress_driver.h >> @@ -23,7 +23,6 @@ struct snd_compr_ops; >> * struct snd_compr_runtime: runtime stream description >> * @state: stream state >> * @ops: pointer to DSP callbacks >> - * @dma_buffer_p: runtime dma buffer pointer >> * @buffer: pointer to kernel buffer, valid only when not in mmap mode or >> * DSP doesn't implement copy >> * @buffer_size: size of the above buffer >> @@ -34,11 +33,14 @@ struct snd_compr_ops; >> * @total_bytes_transferred: cumulative bytes transferred by offload DSP >> * @sleep: poll sleep >> * @private_data: driver private data pointer >> + * @dma_area: virtual buffer address >> + * @dma_addr: physical buffer address (not accessible from main CPU) >> + * @dma_bytes: size of DMA area >> + * @dma_buffer_p: runtime dma buffer pointer >> */ >> struct snd_compr_runtime { >> snd_pcm_state_t state; >> struct snd_compr_ops *ops; >> - struct snd_dma_buffer *dma_buffer_p; >> void *buffer; >> u64 buffer_size; >> u32 fragment_size; >> @@ -47,6 +49,11 @@ struct snd_compr_runtime { >> u64 total_bytes_transferred; >> wait_queue_head_t sleep; >> void *private_data; >> + >> + unsigned char *dma_area; >> + dma_addr_t dma_addr; >> + size_t dma_bytes; >> + struct snd_dma_buffer *dma_buffer_p; > > Why do we need to have both dma_buffer_p and its values? > For consistency with PCM stream? > > For PCM, dma_area, dma_addr and dma_bytes are the primary data, which > aren't necessarily set by the dma_buffer but manually set up by the > driver. > > Just wondering. Yeah, this is simply for consistency reasons. As referred to in previous reply, I'd like to see compr & PCM being tightly coupled in the future so separate page allocation functions are not required. Whether this is entirely possible or not I not know yet, although I'm an optimist when it comes to that subject. If it indeed comes to that, having shared concepts and consistent API makes it much easier for one to refactor. Czarek
diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h index bc88d6f964da..00f633c0c3ba 100644 --- a/include/sound/compress_driver.h +++ b/include/sound/compress_driver.h @@ -23,7 +23,6 @@ struct snd_compr_ops; * struct snd_compr_runtime: runtime stream description * @state: stream state * @ops: pointer to DSP callbacks - * @dma_buffer_p: runtime dma buffer pointer * @buffer: pointer to kernel buffer, valid only when not in mmap mode or * DSP doesn't implement copy * @buffer_size: size of the above buffer @@ -34,11 +33,14 @@ struct snd_compr_ops; * @total_bytes_transferred: cumulative bytes transferred by offload DSP * @sleep: poll sleep * @private_data: driver private data pointer + * @dma_area: virtual buffer address + * @dma_addr: physical buffer address (not accessible from main CPU) + * @dma_bytes: size of DMA area + * @dma_buffer_p: runtime dma buffer pointer */ struct snd_compr_runtime { snd_pcm_state_t state; struct snd_compr_ops *ops; - struct snd_dma_buffer *dma_buffer_p; void *buffer; u64 buffer_size; u32 fragment_size; @@ -47,6 +49,11 @@ struct snd_compr_runtime { u64 total_bytes_transferred; wait_queue_head_t sleep; void *private_data; + + unsigned char *dma_area; + dma_addr_t dma_addr; + size_t dma_bytes; + struct snd_dma_buffer *dma_buffer_p; }; /** @@ -180,19 +187,29 @@ static inline void snd_compr_drain_notify(struct snd_compr_stream *stream) /** * snd_compr_set_runtime_buffer - Set the Compress runtime buffer - * @substream: compress substream to set + * @stream: compress stream to set * @bufp: the buffer information, NULL to clear * * Copy the buffer information to runtime buffer when @bufp is non-NULL. * Otherwise it clears the current buffer information. */ -static inline void snd_compr_set_runtime_buffer( - struct snd_compr_stream *substream, - struct snd_dma_buffer *bufp) +static inline void +snd_compr_set_runtime_buffer(struct snd_compr_stream *stream, + struct snd_dma_buffer *bufp) { - struct snd_compr_runtime *runtime = substream->runtime; - - runtime->dma_buffer_p = bufp; + struct snd_compr_runtime *runtime = stream->runtime; + + if (bufp) { + runtime->dma_buffer_p = bufp; + runtime->dma_area = bufp->area; + runtime->dma_addr = bufp->addr; + runtime->dma_bytes = bufp->bytes; + } else { + runtime->dma_buffer_p = NULL; + runtime->dma_area = NULL; + runtime->dma_addr = 0; + runtime->dma_bytes = 0; + } } int snd_compr_stop_error(struct snd_compr_stream *stream,
Update DMA buffer definition for snd_compr_runtime so it is represented similarly as in snd_pcm_runtime. While at it, modify snd_compr_set_runtime_buffer to account for newly added members. Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com> --- include/sound/compress_driver.h | 35 ++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)