Skip to content

updates for keyword detect #771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
plbossart merged 5 commits into from
Apr 3, 2019
Merged

updates for keyword detect #771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d30ba64
ASoC: SOF: topology: Improve handling of processing components
ranj063 Feb 13, 2019
464d9ef
ASoC: SOF: add new process comp types for hotwording
ranj063 Feb 13, 2019
25321db
ASoC: SOF: add support for binding widgets with external events
ranj063 Mar 6, 2019
7698c85
ASoC: SOF: do not send routes to/from output/out_drv widgets to the DSP
ranj063 Mar 6, 2019
59f40b8
ASoC: SOF: fix bug in snd_sof_find_spcm_name()
ranj063 Mar 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 25 additions & 29 deletions include/sound/sof/topology.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,12 @@ enum sof_comp_type {
SOF_COMP_BUFFER,
SOF_COMP_EQ_IIR,
SOF_COMP_EQ_FIR,
SOF_COMP_FILEREAD, /**< host test based file IO */
SOF_COMP_FILEWRITE, /**< host test based file IO */
SOF_COMP_KEYWORD_DETECT,
SOF_COMP_KPB, /* A key phrase buffer component */
SOF_COMP_SELECTOR, /**< channel selector component */
/* keep FILEREAD/FILEWRITE as the last ones */
SOF_COMP_FILEREAD = 10000, /**< host test based file IO */
SOF_COMP_FILEWRITE = 10001, /**< host test based file IO */
};

/* XRUN action for component */
Expand Down Expand Up @@ -163,39 +167,25 @@ struct sof_ipc_comp_tone {
int32_t ramp_step;
} __packed;

/** \brief Types of EFFECT */
enum sof_ipc_effect_type {
SOF_EFFECT_NONE = 0, /**< None */
SOF_EFFECT_INTEL_EQFIR, /**< Intel FIR */
SOF_EFFECT_INTEL_EQIIR, /**< Intel IIR */
/** \brief Types of processing components */
enum sof_ipc_process_type {
SOF_PROCESS_NONE = 0, /**< None */
SOF_PROCESS_EQFIR, /**< Intel FIR */
SOF_PROCESS_EQIIR, /**< Intel IIR */
SOF_PROCESS_KEYWORD_DETECT, /**< Keyword Detection */
SOF_PROCESS_KPB, /**< KeyPhrase Buffer Manager */
SOF_PROCESS_CHAN_SELECTOR, /**< Channel Selector */
};

/* general purpose EFFECT configuration */
struct sof_ipc_comp_effect {
struct sof_ipc_hdr hdr;
uint32_t type; /** sof_ipc_effect_type */
} __packed;

/* FIR equalizer component */
struct sof_ipc_comp_eq_fir {
struct sof_ipc_comp comp;
struct sof_ipc_comp_config config;
uint32_t size;

/* reserved for future use */
uint32_t reserved[8];

unsigned char data[0];
} __packed;

/* IIR equalizer component */
struct sof_ipc_comp_eq_iir {
/* generic "effect", "codec" or proprietary processing component */
struct sof_ipc_comp_process {
struct sof_ipc_comp comp;
struct sof_ipc_comp_config config;
uint32_t size;
uint32_t size; /**< size of bespoke data section in bytes */
uint32_t type; /**< sof_ipc_process_type */

/* reserved for future use */
uint32_t reserved[8];
uint32_t reserved[7];

unsigned char data[0];
} __packed;
Expand Down Expand Up @@ -257,4 +247,10 @@ struct sof_ipc_pipe_comp_connect {
uint32_t sink_id;
} __packed;

/* external events */
enum sof_event_types {
SOF_EVENT_NONE = 0,
SOF_KEYWORD_DETECT_DAPM_EVENT,
};

#endif
2 changes: 1 addition & 1 deletion include/uapi/sound/sof/abi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

/* SOF ABI version major, minor and patch numbers */
#define SOF_ABI_MAJOR 3
#define SOF_ABI_MINOR 3
#define SOF_ABI_MINOR 4

@plbossart plbossart Apr 2, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there really an ABI change and is this aligned with firmware?

@plbossart plbossart Apr 2, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also have you tested backwards compatibility (new kernel, old firmware)?

@ranj063 ranj063 Apr 2, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keyonjie @plbossart the ABI change is probably the most confusing bit of all for me. The same change was merged in the FW a few weeks ago and the ABI minor version was bumped up to 2. But since then, there have been other changes merged into the FW and the kernel which have updated the ABI too.

So I'd say we probably dont need an ABI change for this. But then we do update the ipc structure so maybe we should?

@plbossart plbossart Apr 2, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgirdwood need your help on this one. it seems all of this was merged in the firmware some time back?

@keyonjie keyonjie Apr 3, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just went though uapi headers of FW, we are actually somewhat mismatched already with the latest codes of both side(maybe it can still work for EQ components), I think that is a reason of why @lgirdwood is planning for a marjor version bump?

back to question from @plbossart, I think we are good as here the new struct sof_ipc_comp_process is compatible with the old struct sof_ipc_comp_eq_fir and struct sof_ipc_comp_eq_iir, that means, though the driver sending with new struct, FW can still parse them in format of old ones, and get the correct settings. Furthermore, there is no de facto EQ topology used by any releases yet so we are good with this update? @singalsu can you help confirm?

some minor mismatched for abi of processing components e.g.
FW:

/** \brief Types of processing components */
enum sof_ipc_process_type {
	SOF_PROCESS_NONE = 0,		/**< None */
	SOF_PROCESS_EQFIR,		/**< Intel FIR */
	SOF_PROCESS_EQIIR,		/**< Intel IIR */
};

/* generic "effect", "codec" or proprietary processing component */
struct sof_ipc_comp_process {
	struct sof_ipc_comp comp;
	struct sof_ipc_comp_config config;
	uint32_t size;	/**< size of bespoke data section in bytes */
	uint32_t type;	/**< sof_ipc_effect_type */

	/* reserved for future use */
	uint32_t reserved[7];

	unsigned char data[0];
} __attribute__((packed));

driver:

/** \brief Types of EFFECT */
enum sof_ipc_effect_type {
	SOF_EFFECT_NONE = 0,		/**< None */
	SOF_EFFECT_INTEL_EQFIR,		/**< Intel FIR */
	SOF_EFFECT_INTEL_EQIIR,		/**< Intel IIR */
};

/* general purpose EFFECT configuration */
struct sof_ipc_comp_effect {
	struct sof_ipc_hdr hdr;
	uint32_t type;			/** sof_ipc_effect_type */
} __packed;

/* FIR equalizer component */
struct sof_ipc_comp_eq_fir {
	struct sof_ipc_comp comp;
	struct sof_ipc_comp_config config;
	uint32_t size;

	/* reserved for future use */
	uint32_t reserved[8];

	unsigned char data[0];
} __packed;

/* IIR equalizer component */
struct sof_ipc_comp_eq_iir {
	struct sof_ipc_comp comp;
	struct sof_ipc_comp_config config;
	uint32_t size;

	/* reserved for future use */
	uint32_t reserved[8];

	unsigned char data[0];
} __packed;

@keyonjie keyonjie Apr 3, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do backwards compatibility test if @singalsu or @ranj063 can help point out which EQ topology file I should run on what platform which is verified work with latest code base.

#define SOF_ABI_PATCH 0

/* SOF ABI version number. Format within 32bit word is MMmmmppp */
Expand Down
6 changes: 5 additions & 1 deletion include/uapi/sound/sof/tokens.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@
/* Tone */
#define SOF_TKN_TONE_SAMPLE_RATE 800

#define SOF_TKN_EFFECT_TYPE 900
/* Processing Components */
#define SOF_TKN_PROCESS_TYPE 900

/* for backward compatibility */
#define SOF_TKN_EFFECT_TYPE SOF_TKN_PROCESS_TYPE

#endif
11 changes: 8 additions & 3 deletions sound/soc/sof/core.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,23 @@
*/

struct snd_sof_pcm *snd_sof_find_spcm_name(struct snd_sof_dev *sdev,
char *name)
const char *name)
{
struct snd_sof_pcm *spcm = NULL;

list_for_each_entry(spcm, &sdev->pcm_list, list) {
/* match with PCM dai name */
if (strcmp(spcm->pcm.dai_name, name) == 0)
return spcm;

if (strcmp(spcm->pcm.caps[0].name, name) == 0)
/* match with playback caps name if set */
if (*spcm->pcm.caps[0].name &&
!strcmp(spcm->pcm.caps[0].name, name))
return spcm;

if (strcmp(spcm->pcm.caps[1].name, name) == 0)
/* match with capture caps name if set */
if (*spcm->pcm.caps[1].name &&
!strcmp(spcm->pcm.caps[1].name, name))
return spcm;
}

Expand Down
2 changes: 1 addition & 1 deletion sound/soc/sof/sof-priv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ struct snd_sof_pcm *snd_sof_find_spcm_dai(struct snd_sof_dev *sdev,
}

struct snd_sof_pcm *snd_sof_find_spcm_name(struct snd_sof_dev *sdev,
char *name);
const char *name);
struct snd_sof_pcm *snd_sof_find_spcm_comp(struct snd_sof_dev *sdev,
unsigned int comp_id,
int *direction);
Expand Down
Loading