diff mbox series

[5/7] ALSA: core: Implement compress page allocation and free routines

Message ID 20191217095851.19629-6-cezary.rojewski@intel.com (mailing list archive)
State New, archived
Headers show
Series ALSA: hda: Enable HDAudio compress | expand

Commit Message

Cezary Rojewski Dec. 17, 2019, 9:58 a.m. UTC 
Add simple malloc and free methods for memory management for compress
streams. Based on snd_pcm_lib_malloc_pages and snd_pcm_lib_free_pages
implementation.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Divya Prakash <divya1.prakash@intel.com>
---
 include/sound/compress_driver.h |  5 ++++
 sound/core/compress_offload.c   | 42 +++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

Comments

Takashi Iwai Dec. 17, 2019, 10:24 a.m. UTC | #1 
On Tue, 17 Dec 2019 10:58:49 +0100,
Cezary Rojewski wrote:
> 
> Add simple malloc and free methods for memory management for compress
> streams. Based on snd_pcm_lib_malloc_pages and snd_pcm_lib_free_pages
> implementation.

I see no user of these functions in the series.  How these are
supposed to be used?


Takashi

> 
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Signed-off-by: Divya Prakash <divya1.prakash@intel.com>
> ---
>  include/sound/compress_driver.h |  5 ++++
>  sound/core/compress_offload.c   | 42 +++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
> index 00f633c0c3ba..6ce8effa0b12 100644
> --- a/include/sound/compress_driver.h
> +++ b/include/sound/compress_driver.h
> @@ -67,6 +67,7 @@ struct snd_compr_runtime {
>   * @metadata_set: metadata set flag, true when set
>   * @next_track: has userspace signal next track transition, true when set
>   * @private_data: pointer to DSP private data
> + * @dma_buffer: allocated buffer if any
>   */
>  struct snd_compr_stream {
>  	const char *name;
> @@ -78,6 +79,7 @@ struct snd_compr_stream {
>  	bool metadata_set;
>  	bool next_track;
>  	void *private_data;
> +	struct snd_dma_buffer dma_buffer;
>  };
>  
>  /**
> @@ -212,6 +214,9 @@ snd_compr_set_runtime_buffer(struct snd_compr_stream *stream,
>  	}
>  }
>  
> +int snd_compr_malloc_pages(struct snd_compr_stream *stream, size_t size);
> +int snd_compr_free_pages(struct snd_compr_stream *stream);
> +
>  int snd_compr_stop_error(struct snd_compr_stream *stream,
>  			 snd_pcm_state_t state);
>  
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index f34ce564d92c..dfb20ceb2d30 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -488,6 +488,48 @@ snd_compr_get_codec_caps(struct snd_compr_stream *stream, unsigned long arg)
>  }
>  #endif /* !COMPR_CODEC_CAPS_OVERFLOW */
>  
> +int snd_compr_malloc_pages(struct snd_compr_stream *stream, size_t size)
> +{
> +	struct snd_dma_buffer *dmab;
> +	int ret;
> +
> +	if (snd_BUG_ON(!(stream) || !(stream)->runtime))
> +		return -EINVAL;
> +	dmab = kzalloc(sizeof(*dmab), GFP_KERNEL);
> +	if (!dmab)
> +		return -ENOMEM;
> +	dmab->dev = stream->dma_buffer.dev;
> +	ret = snd_dma_alloc_pages(dmab->dev.type, dmab->dev.dev, size, dmab);
> +	if (ret < 0) {
> +		kfree(dmab);
> +		return ret;
> +	}
> +
> +	snd_compr_set_runtime_buffer(stream, dmab);
> +	stream->runtime->dma_bytes = size;
> +	return 1;
> +}
> +EXPORT_SYMBOL(snd_compr_malloc_pages);
> +
> +int snd_compr_free_pages(struct snd_compr_stream *stream)
> +{
> +	struct snd_compr_runtime *runtime = stream->runtime;
> +
> +	if (snd_BUG_ON(!(stream) || !(stream)->runtime))
> +		return -EINVAL;
> +	if (!runtime->dma_area)
> +		return 0;
> +	if (runtime->dma_buffer_p != &stream->dma_buffer) {
> +		/* It's a newly allocated buffer. Release it now. */
> +		snd_dma_free_pages(runtime->dma_buffer_p);
> +		kfree(runtime->dma_buffer_p);
> +	}
> +
> +	snd_compr_set_runtime_buffer(stream, NULL);
> +	return 0;
> +}
> +EXPORT_SYMBOL(snd_compr_free_pages);
> +
>  /* revisit this with snd_pcm_preallocate_xxx */
>  static int snd_compr_allocate_buffer(struct snd_compr_stream *stream,
>  		struct snd_compr_params *params)
> -- 
> 2.17.1
>
Cezary Rojewski Dec. 17, 2019, 12:22 p.m. UTC | #2 
On 2019-12-17 11:24, Takashi Iwai wrote:
> On Tue, 17 Dec 2019 10:58:49 +0100,
> Cezary Rojewski wrote:
>>
>> Add simple malloc and free methods for memory management for compress
>> streams. Based on snd_pcm_lib_malloc_pages and snd_pcm_lib_free_pages
>> implementation.
> 
> I see no user of these functions in the series.  How these are
> supposed to be used?
> 
> 
> Takashi
> 

I've given github links in the cover letter although I could have been 
more descriptive here too..

Probing pull request link:
https://github.com/thesofproject/linux/pull/1276/

Commits implementing compr flow:
- ASoC: SOF: Generic probe compress operations
https://github.com/thesofproject/linux/pull/1276/commits/217025f0fdad7523645a141ad2965dff4923a229

- ASoC: SOF: Intel: Probe compress operations
https://github.com/thesofproject/linux/pull/1276/commits/fb3e724a54bf859f039b2b0b03503a52e1740375

Methods: sof_probe_compr_set_params and sof_probe_compr_free of "ASoC: 
SOF: Generic probe compress operations" commit make use of these.

Basically it shares the concept of simple HDA PCM stream setup. During 
the development process, we decided to split the "introduction" and 
"usage" parts and thus this set of 7patches had spawned - covers the 
introduction. Actual probing patches take care of the "usage".

Czarek

>>
>> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
>> Signed-off-by: Divya Prakash <divya1.prakash@intel.com>
>> ---
>>   include/sound/compress_driver.h |  5 ++++
>>   sound/core/compress_offload.c   | 42 +++++++++++++++++++++++++++++++++
>>   2 files changed, 47 insertions(+)
>>
>> diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
>> index 00f633c0c3ba..6ce8effa0b12 100644
>> --- a/include/sound/compress_driver.h
>> +++ b/include/sound/compress_driver.h
>> @@ -67,6 +67,7 @@ struct snd_compr_runtime {
>>    * @metadata_set: metadata set flag, true when set
>>    * @next_track: has userspace signal next track transition, true when set
>>    * @private_data: pointer to DSP private data
>> + * @dma_buffer: allocated buffer if any
>>    */
>>   struct snd_compr_stream {
>>   	const char *name;
>> @@ -78,6 +79,7 @@ struct snd_compr_stream {
>>   	bool metadata_set;
>>   	bool next_track;
>>   	void *private_data;
>> +	struct snd_dma_buffer dma_buffer;
>>   };
>>   
>>   /**
>> @@ -212,6 +214,9 @@ snd_compr_set_runtime_buffer(struct snd_compr_stream *stream,
>>   	}
>>   }
>>   
>> +int snd_compr_malloc_pages(struct snd_compr_stream *stream, size_t size);
>> +int snd_compr_free_pages(struct snd_compr_stream *stream);
>> +
>>   int snd_compr_stop_error(struct snd_compr_stream *stream,
>>   			 snd_pcm_state_t state);
>>   
>> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
>> index f34ce564d92c..dfb20ceb2d30 100644
>> --- a/sound/core/compress_offload.c
>> +++ b/sound/core/compress_offload.c
>> @@ -488,6 +488,48 @@ snd_compr_get_codec_caps(struct snd_compr_stream *stream, unsigned long arg)
>>   }
>>   #endif /* !COMPR_CODEC_CAPS_OVERFLOW */
>>   
>> +int snd_compr_malloc_pages(struct snd_compr_stream *stream, size_t size)
>> +{
>> +	struct snd_dma_buffer *dmab;
>> +	int ret;
>> +
>> +	if (snd_BUG_ON(!(stream) || !(stream)->runtime))
>> +		return -EINVAL;
>> +	dmab = kzalloc(sizeof(*dmab), GFP_KERNEL);
>> +	if (!dmab)
>> +		return -ENOMEM;
>> +	dmab->dev = stream->dma_buffer.dev;
>> +	ret = snd_dma_alloc_pages(dmab->dev.type, dmab->dev.dev, size, dmab);
>> +	if (ret < 0) {
>> +		kfree(dmab);
>> +		return ret;
>> +	}
>> +
>> +	snd_compr_set_runtime_buffer(stream, dmab);
>> +	stream->runtime->dma_bytes = size;
>> +	return 1;
>> +}
>> +EXPORT_SYMBOL(snd_compr_malloc_pages);
>> +
>> +int snd_compr_free_pages(struct snd_compr_stream *stream)
>> +{
>> +	struct snd_compr_runtime *runtime = stream->runtime;
>> +
>> +	if (snd_BUG_ON(!(stream) || !(stream)->runtime))
>> +		return -EINVAL;
>> +	if (!runtime->dma_area)
>> +		return 0;
>> +	if (runtime->dma_buffer_p != &stream->dma_buffer) {
>> +		/* It's a newly allocated buffer. Release it now. */
>> +		snd_dma_free_pages(runtime->dma_buffer_p);
>> +		kfree(runtime->dma_buffer_p);
>> +	}
>> +
>> +	snd_compr_set_runtime_buffer(stream, NULL);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(snd_compr_free_pages);
>> +
>>   /* revisit this with snd_pcm_preallocate_xxx */
>>   static int snd_compr_allocate_buffer(struct snd_compr_stream *stream,
>>   		struct snd_compr_params *params)
>> -- 
>> 2.17.1
>>
Pierre-Louis Bossart Dec. 17, 2019, 2:06 p.m. UTC | #3 
On 12/17/19 4:24 AM, Takashi Iwai wrote:
> On Tue, 17 Dec 2019 10:58:49 +0100,
> Cezary Rojewski wrote:
>>
>> Add simple malloc and free methods for memory management for compress
>> streams. Based on snd_pcm_lib_malloc_pages and snd_pcm_lib_free_pages
>> implementation.
> 
> I see no user of these functions in the series.  How these are
> supposed to be used?

I asked Cezary to post those patches on alsa-devel to make sure there is 
no disagreement on the initial solution.

The next steps would be probes (data injection/extraction in the DSP 
firmware) and compressed audio support for i.MX and Intel platforms.
Both would be coming in early Q1 next year.
diff mbox series

Patch

diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
index 00f633c0c3ba..6ce8effa0b12 100644
--- a/include/sound/compress_driver.h
+++ b/include/sound/compress_driver.h
@@ -67,6 +67,7 @@  struct snd_compr_runtime {
  * @metadata_set: metadata set flag, true when set
  * @next_track: has userspace signal next track transition, true when set
  * @private_data: pointer to DSP private data
+ * @dma_buffer: allocated buffer if any
  */
 struct snd_compr_stream {
 	const char *name;
@@ -78,6 +79,7 @@  struct snd_compr_stream {
 	bool metadata_set;
 	bool next_track;
 	void *private_data;
+	struct snd_dma_buffer dma_buffer;
 };
 
 /**
@@ -212,6 +214,9 @@  snd_compr_set_runtime_buffer(struct snd_compr_stream *stream,
 	}
 }
 
+int snd_compr_malloc_pages(struct snd_compr_stream *stream, size_t size);
+int snd_compr_free_pages(struct snd_compr_stream *stream);
+
 int snd_compr_stop_error(struct snd_compr_stream *stream,
 			 snd_pcm_state_t state);
 
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index f34ce564d92c..dfb20ceb2d30 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -488,6 +488,48 @@  snd_compr_get_codec_caps(struct snd_compr_stream *stream, unsigned long arg)
 }
 #endif /* !COMPR_CODEC_CAPS_OVERFLOW */
 
+int snd_compr_malloc_pages(struct snd_compr_stream *stream, size_t size)
+{
+	struct snd_dma_buffer *dmab;
+	int ret;
+
+	if (snd_BUG_ON(!(stream) || !(stream)->runtime))
+		return -EINVAL;
+	dmab = kzalloc(sizeof(*dmab), GFP_KERNEL);
+	if (!dmab)
+		return -ENOMEM;
+	dmab->dev = stream->dma_buffer.dev;
+	ret = snd_dma_alloc_pages(dmab->dev.type, dmab->dev.dev, size, dmab);
+	if (ret < 0) {
+		kfree(dmab);
+		return ret;
+	}
+
+	snd_compr_set_runtime_buffer(stream, dmab);
+	stream->runtime->dma_bytes = size;
+	return 1;
+}
+EXPORT_SYMBOL(snd_compr_malloc_pages);
+
+int snd_compr_free_pages(struct snd_compr_stream *stream)
+{
+	struct snd_compr_runtime *runtime = stream->runtime;
+
+	if (snd_BUG_ON(!(stream) || !(stream)->runtime))
+		return -EINVAL;
+	if (!runtime->dma_area)
+		return 0;
+	if (runtime->dma_buffer_p != &stream->dma_buffer) {
+		/* It's a newly allocated buffer. Release it now. */
+		snd_dma_free_pages(runtime->dma_buffer_p);
+		kfree(runtime->dma_buffer_p);
+	}
+
+	snd_compr_set_runtime_buffer(stream, NULL);
+	return 0;
+}
+EXPORT_SYMBOL(snd_compr_free_pages);
+
 /* revisit this with snd_pcm_preallocate_xxx */
 static int snd_compr_allocate_buffer(struct snd_compr_stream *stream,
 		struct snd_compr_params *params)