[Sound-open-firmware] [PATCH] configure.ac: add CONFIG_HOST_PTABLE flag for platforms which need handle it
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@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]) 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);
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@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")
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);
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@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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On Wed, 2017-12-06 at 15:42 +0800, Keyon Jie wrote:
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@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.
ok, so it looks like we need to do both here, but dont do AM_CONTIDIONAL inside the switch (confuses automake).
Liam
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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmwar e
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On Wed, 2017-12-06 at 08:43 +0000, Liam Girdwood wrote:
On Wed, 2017-12-06 at 15:42 +0800, Keyon Jie wrote:
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@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-trac which does not apply e.c then don't compile it.
ok, so it looks like we need to do both here, but dont do AM_CONTIDIONAL inside the switch (confuses automake).
Sorry, I mean for the DMA trace patch, not this one which does not apply. Can you redo.
Thanks
Liam
--------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
participants (3)
-
Keyon Jie -
Liam Girdwood -
Liam Girdwood