[Sound-open-firmware] [PATCH] configure.ac: add CONFIG_HOST_PTABLE flag for platforms which need handle it

[Keyon Jie]

On 2017年12月06日 15:20, Liam Girdwood wrote:
> On Wed, 2017-12-06 at 15:05 +0800, Keyon Jie wrote:
>> We only need handle host page tables on platforms that we
>> program DMA host buffer(addr/size) inside firmware, for
>> other platforms, host driver will program these settings
>> and won't pass in page tables.
>>
>> So here add frag CONFIG_HOST_PTABLE to configure this for
>> different platforms, on Baytrail, Cherrytrail, we need
>> CONFIG_HOST_PTABLE to be selected.
>>
>> Signed-off-by: Keyon Jie <yang.jie at linux.intel.com>
>> ---
>>   configure.ac        | 2 ++
>>   src/ipc/intel-ipc.c | 6 ++++++
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 093d0b4..e437d06 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -82,6 +82,7 @@ case "$with_platform" in
>>   
>>   	AC_DEFINE([CONFIG_BAYTRAIL], [1], [Configure for Baytrail])
>>   	AC_DEFINE([CONFIG_DMA_TRACE], [1], [Configure DMA trace])
>> +	AC_DEFINE([CONFIG_HOST_PTABLE], [1], [Configure handling
>> host page table])
>>       ;;
>>       cherrytrail*)
>>   
>> @@ -99,6 +100,7 @@ case "$with_platform" in
>>   
>>   	AC_DEFINE([CONFIG_CHERRYTRAIL], [1], [Configure for
>> Cherrytrail])
>>   	AC_DEFINE([CONFIG_DMA_TRACE], [1], [Configure DMA trace])
>> +	AC_DEFINE([CONFIG_HOST_PTABLE], [1], [Configure handling
>> host page table])
>>       ;;
>>       *)
>>           AC_MSG_ERROR([Host platform not specified])
> 
> 
> Best to do this outside switch() with AM_CONDITIONAL
> 
> AM_CONDITIONAL(BUILD_BLAH,  test "$FW_NAME" = "byt" -o "$FW_NAME" = "apl")

Per my understanding, AM_CONDITIONAL() is used to define the condition 
which can be used in Makefile.am, to decide if we need to compile some 
files.

But here it is only used as a configure item which the macro will be 
used in source code directly, so maybe this is better way?
At the same time, with this format, it is very easy to check all 
settings/configs for a specific platform -- they are all gathered together.

I do want to use this to remove some unnecessary files for specific 
platform, which will lead to small .ri files. e.g. if we don't use 
dma-trace.c then don't compile it.

Thanks,
~Keyon

> 
> Look at "man test" for conditional logic details.
> 
>> diff --git a/src/ipc/intel-ipc.c b/src/ipc/intel-ipc.c
>> index 4c310b6..e39951a 100644
>> --- a/src/ipc/intel-ipc.c
>> +++ b/src/ipc/intel-ipc.c
>> @@ -82,6 +82,7 @@ static inline struct sof_ipc_hdr
>> *mailbox_validate(void)
>>   	return hdr;
>>   }
>>   
>> +#ifdef CONFIG_HOST_PTABLE
>>   static void dma_complete(void *data, uint32_t type, struct
>> dma_sg_elem *next)
>>   {
>>   	struct intel_ipc_data *iipc = (struct intel_ipc_data *)data;
>> @@ -219,6 +220,7 @@ static int parse_page_descriptors(struct
>> intel_ipc_data *iipc,
>>   
>>   	return 0;
>>   }
>> +#endif
>>   
>>   /*
>>    * Stream IPC Operations.
>> @@ -227,7 +229,9 @@ static int parse_page_descriptors(struct
>> intel_ipc_data *iipc,
>>   /* allocate a new stream */
>>   static int ipc_stream_pcm_params(uint32_t stream)
>>   {
>> +#ifdef CONFIG_HOST_PTABLE
>>   	struct intel_ipc_data *iipc = ipc_get_drvdata(_ipc);
>> +#endif
>>   	struct sof_ipc_pcm_params *pcm_params = _ipc->comp_data;
>>   	struct sof_ipc_pcm_params_reply reply;
>>   	struct ipc_comp_dev *pcm_dev;
>> @@ -255,6 +259,7 @@ static int ipc_stream_pcm_params(uint32_t stream)
>>   	cd = pcm_dev->cd;
>>   	cd->params = pcm_params->params;
>>   
>> +#ifdef CONFIG_HOST_PTABLE
>>   	/* use DMA to read in compressed page table ringbuffer from
>> host */
>>   	err = get_page_descriptors(iipc, &pcm_params-
>>> params.buffer);
>>   	if (err < 0) {
>> @@ -269,6 +274,7 @@ static int ipc_stream_pcm_params(uint32_t stream)
>>   		trace_ipc_error("eAP");
>>   		goto error;
>>   	}
>> +#endif
>>   
>>   	/* configure pipeline audio params */
>>   	err = pipeline_params(pcm_dev->cd->pipeline, pcm_dev->cd,
>> pcm_params);
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>