[alsa-devel] [PATCH] conf: topology: Add topolgy for skylake i2s configuration

[Takashi Iwai]

On Tue, 09 Feb 2016 14:34:32 +0100,
Vinod Koul wrote:
> 
> On Tue, Feb 09, 2016 at 02:18:58PM +0100, Takashi Iwai wrote:
> > On Tue, 09 Feb 2016 14:14:19 +0100,
> > Subhransu S. Prusty wrote:
> > > 
> > > On Tue, Feb 09, 2016 at 12:54:39PM +0100, Takashi Iwai wrote:
> > > > On Tue, 09 Feb 2016 12:47:53 +0100,
> > > > Subhransu S. Prusty wrote:
> > > > > 
> > > > > On Tue, Feb 09, 2016 at 12:15:45PM +0100, Takashi Iwai wrote:
> > > > > > On Mon, 08 Feb 2016 04:55:56 +0100,
> > > > > > Subhransu S. Prusty wrote:
> > > > > > > 
> > > > > > > This patch adds basic playback/capture support for skylake i2s
> > > > > > > platform. DSP topology module data are passed through the binary
> > > > > > > files. The framework parses these files and puts the data in the
> > > > > > > widget private section for the corresponding widget. This is
> > > > > > > parsed by kernel driver and stored as module config for the DSP.
> > > > > > > Based on usecase these data are sent to the DSP through IPCs for
> > > > > > > further processing.
> > > > > > 
> > > > > > Can we have sources for these binaries, or do they have to be
> > > > > > binary-only?
> > > > > 
> > > > > Hi Takashi,
> > > > > 
> > > > > These are binary only data.
> > > > 
> > > > Then this isn't a good material for merging to alsa-lib.  How is the
> > > > license compatibility?
> > > 
> > > Each binary file here holds config for each module based on skl_dfw_module
> > > structure as expected by Skylake driver. The skl driver formats IPCs parsing
> > > this config.
> > > 
> > > This structure skl_dfw_module is already defined as part of
> > > skl-tplg-interface.h.
> > 
> > Well, the question is whether this IP is a programmed data block, not
> > some simple numbers.  If yes, it's always a question whether it's
> > compatible with GPL.  Although alsa-lib is LGPL, putting the binary
> > blob in the *code tree* doesn't look good to me.
> 
> Hi Takashi,
> 
> This is simple numbers only. Numbers which identify the data for firmware,
> its resources, ids, pipe number, module number and for controls default
> values etc. Basically this struct
> 
> struct skl_dfw_module {
>         char uuid[SKL_UUID_STR_SZ];
> 
>         u16 module_id;
>         u16 instance_id;
>         u32 max_mcps;
>         u32 mem_pages;
>         u32 obs;
>         u32 ibs;
>         u32 vbus_id;
> 
>         u32 max_in_queue:8;
>         u32 max_out_queue:8;
>         u32 time_slot:8;
>         u32 core_id:4;
>         u32 rsvd1:4;
> 
>         u32 module_type:8;
>         u32 conn_type:4;
>         u32 dev_type:4;
>         u32 hw_conn_type:4;
>         u32 rsvd2:12;
> 
>         u32 params_fixup:8;
>         u32 converter:8;
>         u32 input_pin_type:1;
>         u32 output_pin_type:1;
>         u32 is_dynamic_in_pin:1;
>         u32 is_dynamic_out_pin:1;
>         u32 is_loadable:1;
>         u32 rsvd3:11;
> 
>         struct skl_dfw_pipe pipe;
>         struct skl_dfw_module_fmt in_fmt[MAX_IN_QUEUE];
>         struct skl_dfw_module_fmt out_fmt[MAX_OUT_QUEUE];
>         struct skl_dfw_module_pin in_pin[MAX_IN_QUEUE];
>         struct skl_dfw_module_pin out_pin[MAX_OUT_QUEUE];
>         struct skl_dfw_module_caps caps;
> } __packed;

OK, but how did you create it?  Via a hex editor?  If you used some
converter, you'd better provide the readable source, too.

> > IMO, this should go to firmware tree instead, unless you can give the
> > source code to build the binary.
> 
> Okay that should be fine, where do we add the source?

In alsa-lib.  It's not necessarily to be in form as all build-ready
there, but providing the capability is important for future
development.


Takashi