Repository navigation
topology: clean up platform specific macro - #2461
Conversation
plbossart
left a comment
plbossart
left a comment
There was a problem hiding this comment.
Not really in agreement here, sorry, see comments below
There was a problem hiding this comment.
sorry, it makes no sense to move this to a common code. This can only be used for cml-rt5682 based topology. The SSP_INDEX is really tied to a specific hw, it can't be reused blindly. NAK on this.
There was a problem hiding this comment.
Thanks for your comment. I will review again, any way I can make this simple and generic. I will think about belwo comments as well.
There was a problem hiding this comment.
meh. that looks like testing the limits of M4 nested ifs...
There was a problem hiding this comment.
also all.m4 is not really a great name...
There was a problem hiding this comment.
NAK on this, see other comment.
There was a problem hiding this comment.
same here, this is either too limited or too broad. And it's not really good since on WHK/CML the mclk is fixed but on APL/ICL/TGL it's programmable
There was a problem hiding this comment.
What I mean is the we have a cml-sku.m4, whl-sku.m4, byt-sku.m4 etc (one for each platform + sku) and this file is included viainclude(PLATFORM) where PLATFORM is passed in during compilation. We probably have more than one SKU per platform too hence we need an optional -sku suffix.
Each platform.m4 (i.e. whl.m4, cml.m4) will contain the SSP mappings and settings that platform + sku only.
474224e to
bc1670d
Compare
bc1670d to
c25b273
Compare
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
Looks better, but the goal is more simplification that should be applied to all differentiation. @macchian fyi. i.e. I do not want to see lots of if else if else if else conditional logic in the topology to parse the arguments passed in my calling script.
There was a problem hiding this comment.
The naming for SKU should be according to the DAI differentiation (if any) e.g.
- If all CML boards use same SSP, DMIC then just call it cml.m4
- If some CML boards use SSP2 and other use SSP3 then create cml-ssp2.m4 and cml-ssp3.m4
- Same as 2 but for DMICs.
There was a problem hiding this comment.
Why not just do
include(`platform/intel/PLATFORM.m4`)
There was a problem hiding this comment.
@lgirdwood PLATFORM is set from makefile. and sof-cml-demux-rt5682.m4 is shared between whl and cml. if/else is needed in somewhere doesn't it?
"sof-cml-demux-rt5682\;sof-cml-demux-rt5682\;-DPLATFORM=cml"
"sof-cml-demux-rt5682\;sof-whl-demux-rt5682\;-DPLATFORM=whl"
There was a problem hiding this comment.
@lgirdwood now I got it. PLATFORM.m4. will update again.
There was a problem hiding this comment.
@lgirdwood I updated again, could you review agian?
icl, cml whl are sharing some toplogy files. Create platform specific files including DSP configuration, SSP setting and DMIC to simplify platform settings. Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
c25b273 to
4902f36
Compare
|
@fredoh9 btw, please feel free to looks through all the board/machine topologies for where we have conditional logic and do a similar fixes. |
|
SOFCI TEST |
|
Rerun CI as internal CI reported only partial results. @wwittbrx fyi. |
@lgirdwood Sure I will review board/machine topologies after this. Once I touch one it cause chain effect to other files. I'd like to create new one. Touching m4 file is a bit tricky, I need to make sure all conf/tplg files are same after the change also. |
All platform specific changes are moved to new file, platform/intel/all.m4.
Signed-off-by: Fred Oh fred.oh@linux.intel.com
Got a review comment from #2444
Compared all conf and tplg files before and after this change.