Re: [PATCH] conf: topology: Add topolgy for skylake i2s configuration

[Takashi Iwai <tiwai@suse.de>]

On Thu, 11 Feb 2016 14:56:03 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Feb 11 2016 17:38, Takashi Iwai wrote:
> > On Thu, 11 Feb 2016 04:52:58 +0100,
> > Vinod Koul wrote:
> >>
> >> On Tue, Feb 09, 2016 at 02:48:36PM +0100, Takashi Iwai wrote:
> >>>>> 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.
> >>
> >> Okay so we will add a intel-topology.c file to alsa-lib, this will also
> >> include a file which will contain the above structure values for each module
> >> in C style.
> >>
> >> This way anyone can edit it easily and we can build blobs from alsa lib and
> >> then run topology tool on it.
> > 
> > It's much better, indeed.
> > 
> >> Do you have recommendation for location of these two files in alsa-lib?
> > 
> > Just put in the same directory?
> 
> Could I ask your opinion about device-dependent codes in common library?
> 
> The alsa-lib is designed for generic ALSA applications. So codes in
> alsa-lib can be used by the applications. On the other hand, your codes
> are not completely common. Even if Intel SoCs are widely used in this
> world, I think there's rest of consideration about merging it.

The code isn't about inclusion into the library itself, but it's a
code (a la helper binary) to generate a card-specific external data.
So it's rather a kind of config data, just represented in a binary
form.


Takashi