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

[Liam Girdwood]

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 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.
> 

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 at alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmwar
> > e
> > 
> 
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware