[FEATURE] topology1: bundle SSP config and DAI format definitions per codec driver

Is your feature request related to a problem? Please describe.
In present topologies, SSP_CONFIG and DAI format (FMT) are defined separately, while the bitwidth under SSP_CONFIG_DATA and DAI data format for AMP SSP have to be unified. There is no checker for that so it's easy to be misaligned by careless changes (e.g. #6956)

Describe the solution you'd like
Bundle SSP_CONFIG and DAI format definitions together, and use the codec driver name as selection key.
Definitions can be listed in a specific m4 file, e.g. platform/intel/amp-codec-config.m4

ifelse(
CODEC, `MAX98357A', `
    define(`AMP_SSP_CONFIG', `
        SSP_CONFIG(I2S, SSP_CLOCK(mclk, 19200000, codec_mclk_in),
                SSP_CLOCK(bclk, 1536000, codec_slave),
                SSP_CLOCK(fsync, 48000, codec_slave),
                SSP_TDM(2, 16, 3, 3),
                SSP_CONFIG_DATA(SSP, SPK_SSP_INDEX, 16))')
    define(`AMP_DAI_FMT', `s16le')',
CODEC, `MAX98360A', `
......

Describe alternatives you've considered

Implement a checker to assert the alignment.
Deduce the bitwidth under SSP_CONFIG_DATA from DAI format or vice versa.

@juimonen

@juimonen @ranj063 @jsarha are we good here for topology2 ? If not, can we discuss and capture it in the topology2 Feature issue. Thanks.

Feature cufoff week for v2.5 and this is not assigned to anyone, so moving forward.

@jsarha

@jsarha pls confirm this is not issue with topology2.

@juimonen

@juimonen @ranj063 @jsarha are we good here for topology2 ? If not, can we discuss and capture it in the topology2 Feature issue. Thanks.

@lgirdwood as of today we do not have a DAI format in topology2. So I dont see the issue there

@ranj063

@ranj063 we kind of do, the copier has a format and so does the dai. If you have a copier that deals with S16_LE and a DAI that works with S32_LE, something will be off.

Ok, too late to fix for v2.6 intplg2, lets make sure we are covered for v2.7.

@ranj063

@ranj063 we kind of do, the copier has a format and so does the dai. If you have a copier that deals with S16_LE and a DAI that works with S32_LE, something will be off.

@plbossart I'm not sure what you meant by "copier has a format and so does the DAI". Can you please poing me to where we set the formats in topology?

@ranj063

@ranj063 the DAI has configuration parameters which MUST be aligned with the copier setup.

Object.Dai.SSP [
	{
		id 		0
		dai_index	0
		direction	"duplex"
		name		NoCodec-0
		default_hw_conf_id	0
		sample_bits		32
		quirks			"lbm_mode"
		io_clk		$MCLK

		Object.Base.hw_config.1 {
			name	"SSP0"
			id	0
			bclk_freq	3072000
			tdm_slot_width	32
			# TODO: remove this. Needs alsaptlg change.
			Object.Base.link_config.1 {
				clock_source	1
			}
		}
	}

	Object.Widget.copier.1 {
					dai_index 0
					dai_type "SSP"
					copier_type "SSP"
					stream_name "NoCodec-0"
					node_type $I2S_LINK_OUTPUT_CLASS
					num_input_audio_formats 3
					Object.Base.input_audio_format [
						{
							in_bit_depth            16
							in_valid_bit_depth      16
						}
						{
							in_bit_depth            32
							in_valid_bit_depth      24
						}
						{
							in_bit_depth            32
							in_valid_bit_depth      32
						}
					]
					num_output_audio_formats 3
					Object.Base.output_audio_format [
						{
							out_bit_depth           16
							out_valid_bit_depth     16
						}
						{
							out_bit_depth           32
							out_valid_bit_depth     24
						}
						{
							out_bit_depth           32
							out_valid_bit_depth     32
						}
					]
				}

@plbossart

@plbossart & @ranj063 So what should be done here? Check and barf if copier output format has bit depths other than what is configured in the DAI? (No idea yet how accomplish that, but just trying get some understanding about the issue.)

I think a good check would be to verify that the bit clock makes sense, but I don't see where the sampling frequency or fsync are specified. Also we could double-check that the valid_bit_depth and the tdm_slot_width are compatible, the former needs to be smaller than the latter otherwise we will truncate the data.

But I have no idea if we can do those checks...

@ranj063

I guess the easiest would to do some sanity checks in the driver, if the topology makes sense. Its a little bit late in the chain, but then again, nobody should release a topology without testing it, and on the other hand the alsatplg is a compiler that does not and should not understand the semantics of the topology file. Something in the middle would be some intermediate validator... but yes, I think it should be quite Ok for the driver to go through the loaded topology and give an error if its not consistent. @ranj063 , or do you have a better idea?

@jsarha

@jsarha @ranj063 do we have a plan for v2.7 ?

I don't think we have a plan and I vote to close the issue. It's just too much work for limited benefits, topology1 is behind us (I hope)

Navigation Menu

Repository navigation

[FEATURE] topology1: bundle SSP config and DAI format definitions per codec driver #6961

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[FEATURE] topology1: bundle SSP config and DAI format definitions per codec driver #6961

Description

Activity

lgirdwood commented on Jan 18, 2023

kv2019i commented on Mar 6, 2023

lgirdwood commented on May 11, 2023

ranj063 commented on May 25, 2023

plbossart commented on May 25, 2023

lgirdwood commented on May 31, 2023

ranj063 commented on May 31, 2023

plbossart commented on May 31, 2023

jsarha commented on Jun 1, 2023

plbossart commented on Jun 1, 2023

jsarha commented on Jun 1, 2023

lgirdwood commented on Jul 4, 2023

plbossart commented on Jul 4, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions