Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 55 additions & 0 deletions tools/topology/v2/attributes.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# common attributes and attribute types. Attributes, defined here can then be
# used by any blocks. E.g. the "format" attribute can be used by pipelines,
# DAIs, etc.

# we're using an "availability" parameter in attribute declaration instead of
# adding ".optional" or ".mandatory" to the array name

DefineType.format {
enum [
Copy link
Member

Choose a reason for hiding this comment

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

Can we name space this and use the "" here. e.g.

DefineType."pcm.format" {

Since "format" can be used for multiple items.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is a type definition, not an attribute declaration. PCM and DAI formats are the same, aren't they?

"s16le"
"s24le"
"s32le"
"float"
]
}

DefineType.time_domain {
Copy link
Member

Choose a reason for hiding this comment

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

ditto "pipeline.time_domain"

enum [
"dma"
Copy link
Member

Choose a reason for hiding this comment

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

We should use the # here to comment each field. i.e.
"dma" #pipeline is scheduled via DMA interrupts

"timer"
]
}

DefineAttribute.tokens {
Copy link
Member

Choose a reason for hiding this comment

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

Dont need to define tokens - it's low level implementation detail only.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a "tokens" attribute for cases where higher level configuration files (e.g. machine level) pass their specific token arrays to lower level configuration files (pipelines). This is currently used for example to define pipelines:

dnl W_PIPELINE(stream, period, priority, core, initiator, platform)
							  ^^^^^^^^
define(`W_PIPELINE',
...
`SectionWidget."'N_PIPELINE($1)`" {'
...
`	data ['
`		"'N_PIPELINE($1)`_data"'
`		"'$6`"'
		^^^^^^
`	]'
`}')

which then is called as

W_PIPELINE(N_DAI_OUT, SCHEDULE_PERIOD, SCHEDULE_PRIORITY, SCHEDULE_CORE, SCHEDULE_TIME_DOMAIN, pipe_dai_schedule_plat)
											       ^^^^^^^^^^^^^^^^^^^^^^

and pipe_dai_schedule_plat is an array of tokens, defined in bxt.m4 or similar. So far I don't have a good idea how to avoid such token array passing. The only thing we can make easier is make arrays of word- and string-tokens a built-in type, without this defining them directly as a structure of an array of string tokens and an array of word tokens becomes really painful.

Copy link
Member

Choose a reason for hiding this comment

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

But we are not using SectionWidget or token array directly - we abstract all tokens with Attributes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lgirdwood we do that where possible, yes. E.g. for volume, where it's known in advance which tokens must be supplied. But here it can be any tokens that the respective platform decides to apply. So I don't see how we can replace them with fixed attributes?

type array.tuple
}

DefineAttribute.format {
Copy link
Member

Choose a reason for hiding this comment

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

This overlaps with DefineType. I much rather reduce the number of new keywords so

DefineAttribute."pcm.format" {
    type enum
    values [
        s16le
        s24le
   [
    default s16le
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

but we will have several attributes of type "format." Without defining the type "format" we'll have to repeat the same enum every time. Even worse with much longer enums like widget types.

Copy link
Member

Choose a reason for hiding this comment

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

This will be global and in a global pcm.conf header. PCM format will be used in lots of places so why would we have to repeat. Can you provide an example ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lgirdwood Even in minimal examples we have two formats: a pipeline format and a DAI format, so we need at least two attributes of type "format" - see pipeline_format and dai_format in this PR

type format
availability mandatory
}

DefineAttribute.template {
type string
availability mandatory
Copy link
Member

Choose a reason for hiding this comment

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

Lets not put the availability in the attribute definitions, since each attribute may be optional for some and mandatory for other components.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, I remember that was your proposal. And I understand the improvement, that it offers, but it would make the code look less readable IMHO. Also, I don't think you can make this work 100%. The same attribute within the same component can be mandatory in some configurations and optional in others, can it not? I don't think you can catch all such errors at compile-time, some can only be really verified at run-time? We could omit availability altogether for now and see later, if you feel strongly about it?

}

DefineAttribute.pipeline_id {
type integer
availability mandatory
}

DefineAttribute.periods_sink {
type integer
availability optional
default 2
min 1
}

DefineAttribute.periods_source {
type integer
availability optional
default 2
min 1
}
17 changes: 17 additions & 0 deletions tools/topology/v2/buffer.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Buffer block and related attributes

DefineAttribute.capabilities {
type array.integer
}

# A buffer block, most attributes inherited from "Widget"
DefineWidget."buffer" {
attributes [
Copy link
Member

Choose a reason for hiding this comment

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

can we make these attribute mandatory and optional in the widget definition. Some widgets will only have mandatory but some will have both e.g. for buffer "size" is mandatory but "ignore_underrun" is optional and false by default.

name
format
periods
capabilities
]

type buffer
}
17 changes: 17 additions & 0 deletions tools/topology/v2/bxt.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# BXT specific topology elements

# BXT host memory capability flags
platform_host_mem_cap [
MEM_CAP_RAM
MEM_CAP_DMA
MEM_CAP_CACHE
MEM_CAP_HP
]

# BXT DAI memory capability flags
platform_dai_mem_cap [
MEM_CAP_RAM
MEM_CAP_DMA
MEM_CAP_CACHE
MEM_CAP_HP
]
47 changes: 47 additions & 0 deletions tools/topology/v2/common.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Common topology defines

# All SOF widget types
DefineType.widget_type {
enum [
"aif_in"
"aif_out"
"asrc"
"buffer"
"dai_in"
"dai_out"
"effect"
"input"
"mixer"
"output"
"pga"
"scheduler"
"siggen"
"src"
]
}
Copy link
Member

Choose a reason for hiding this comment

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

Dont need this, it's low level and encoded in the CC.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

don't we add new widget types from time to time? Do we really want to patch the compiler every time we do that?


# Attributes, common to all widgets
DefineAttribute.type {
type widget_type
availability optional
}

DefineAttribute.index {
type integer
availability optional
}

DefineAttribute.no_pm {
type bool
availability optional
default true
}

# All widgets inherit these attributes
DefineWidget {
attributes [
type
index
no_pm
]
}
114 changes: 114 additions & 0 deletions tools/topology/v2/dai.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# DAI attributes and component declarations

# These have to be globally defined, so they are using a "dai_" prefix
DefineAttribute.dai_format {
type format
# could make it optional with s32le as default?
availability mandatory
}

DefineAttribute.dai_periods {
type integer
availability optional
default 2
min 1
}

DefineAttribute.dai_index {
type integer
availability mandatory
}

DefineType.dai_type {
enum [
"ALH"
"DMIC"
"ESAI"
"HDA"
"SAI"
"SSP"
]
}

DefineAttribute.dai_type {
type dai_type
availability mandatory
}

# The DAI widget
DefineWidget."dai" {
DefineAttribute.backend {
type string
availability mandatory
}

attributes [
template
backend
periods_sink
periods_source
dai_format
dai_type
dai_index
schedule_time_domain
]
}

# DAI_CONFIG adds 2 components: SectionBE, SectionHWConfig, but we want a block
# for each component, so we need two defines

# A back end / SectionBE
DefineBE {
DefineAttribute.index {
type integer
availability mandatory
}

DefineAttribute.id {
type integer
availability mandatory
}

DefineAttribute.default_hw_conf_id {
type integer
availability mandatory
}

DefineAttribute.hw_configs {
type array.string
availability mandatory
}

# index is always \"0\", default_hw_conf_id is always identical
attributes [
id
index
default_hw_conf_id
hw_configs
]

index 0
default_hw_conf_id $id

# FIXME we need concatenation
hw_configs [
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to concat this ?

hw_configs [
    config1
    config2
]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

those aren't multiple configurations, that's just one configuration, generated by concatenating those two strings:

`SectionBE."'$4`" {'
...
`	hw_configs ['
`		"'$1$2`"'
`	]'

@func concat
strings [
$dai_type
$dai_index
]
]
}

# SectionHWConfig
DefineHWConfig {
Copy link
Member

Choose a reason for hiding this comment

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

DefineHWConfig."hw_cfg.ssp" etc would support per DAI type configs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure I understand correctly what you mean. This is dai.conf, so this is a generic definition of a SectionHWConfig component. "hw_cfg." seems to be unneeded - it just repeats DefineHWConfig. As for "ssp" - that would mean, that we have to instantiate SectionHWConfig components in machine configuration files. In fact this is exactly what my latest version does. But then we also have to define them per DAI type - as you say. What do we gain that way? So far we don't seem to have to do that? But that can be done, I think, let's see if we need it.

DefineAttribute.id {
type integer
availability mandatory
}

attributes [
id
# we also need to define attributes for SSP, SAI, ESAI
]
}
80 changes: 80 additions & 0 deletions tools/topology/v2/machine.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# Instantiate a subtree - all the pipelines, connected to one DAI
subtree {
Copy link
Member

Choose a reason for hiding this comment

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

I would expect the machine file to contain

DeclarePipeline."pipe-name1" {
}

DeclarePipeline."pipe-name2" {
}

DeclareDAI."dai-name1" {
}

It should be representing the same data as contained in the machine M4 today.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"Declare" in your code above means instantiation? I use "new." for it now. But yes, that's also what I do.
As far as I understand, the whole DSP audio topology can be divided into "DAI groups." I.e. each DAI can have a group of pipelines associated with it, but different DAIs never link. This seems to be what I see in most pipelines ATM but in fact I suspect this isn't a general rule? There can be some pass-through paths from a DMIC to a playback DAI, right? Do we have any such examples or any other examples, where different DAIs link together?
Currently in my notation a "subtree" is such an isolated part of the topology graph and it can include several pipelines, but only one DAI. The difference to the current m4 situation, where machine files just contain collections of pipelines, DAIs, backends, etc. is, that to make that approach work our m4 files make a heavy use of string-matching, i.e. concatenation. That's how DAIs, their configurations, graphs and pipelines are matched. To avoid that and to make explicit separation of graph parts I use "subtrees." E.g. each such subtree defines global parameters like dai_format etc. Without separating subtrees this would be impossible.

# Subtree-level attributes

# the DAI will use pipeline_id from the first pipeline in the array
# We put all the DAI attributes in the global scope, but in principle
# the compiler can also be taught to take them from the DAI scope.
dai_format s32le
dai_periods 2
dai_type HDA
dai_index 4
schedule_time_domain timer

# the below could also be written as "pipelines.0 {...}" especially in
# cases with only one pipeline in the subtree

# Each element of this array instantiates a block, as defined per
# DefineWidget."pipeline"
# TODO: this has to be made explicit
pipelines [
{
template "volume-playback"

pipeline_id 7
pipeline_channels 2
pipeline_format s24le
pipeline_rate 48000

schedule_period 1000
schedule_priority 0
# optional: 0 core is the default
schedule_core 0

pcm_id 3
pcm_rate {
min 48000
max 48000
}

tokens [
{
# from bxt.m4, cnl.m4
sched_mips 5000
}
]

# Not every pipeline has a PCM, some pipelines might
# just form an alternative path in a graph, so a PCM in
# a pipeline is optional, but common
# PCM_PLAYBACK_ADD(HDMI1, 3, PIPELINE_PCM_7)
# adds a "SectionPCM" block
pcm {
name "HDMI1"
}
}
]

# Instantiate a block, as defined per
# DefineWidget."dai"
# TODO: this has to be made explicit
dai {
# DAI-specific attributes
template "playback"
backend "iDisp1"
}

# The fourth parameter is the BE name and it seems to always be equal to
# the fifth parameter of the DAI_ADD() macro, however, this hasn't been
# verified in all topologies.
# DAI_CONFIG(HDA, 4, 1, iDisp1)
# Possibly this can be merged with the "dai" block above
dai_config {
id 1
}

# Need to instantiate SectionHWConfig separately
hw_config {
id 1
}
}
Loading