[keyonjie]

Mostly are topology updates, needed for keyword detect feature.

[plbossart]

Thanks @keyonjie. Patches 3,4,5 could be merged directly. For the first two, I'd like to have more information on backwards compatibility since they seem to be a minor ABI change.

[plbossart]

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

[plbossart]

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

[ranj063]

@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]

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

[keyonjie]

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]

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.

[plbossart]

ok, I am going to merge this to realign with firmware. @singalsu please make sure we deal with backwards compatibility issues and that the old firmware-new kernel case is covered. Thanks!

[singalsu]

@plbossart OK, I'll test with this. I have related two other EQ control issues to fix as well.