| Message ID | 20231205142420.1256042-2-ckeepax@opensource.cirrus.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [alsa-ucm-conf,1/2] sof-soundwire: Add basic support for cs42l43 | expand |
On 12/5/23 08:24, Charles Keepax wrote: > cs35l56 is a boosted speaker amp add UCM support for a 4 amp > configuration. > > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> > --- > ucm2/sof-soundwire/cs35l56-4.conf | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > create mode 100644 ucm2/sof-soundwire/cs35l56-4.conf > > diff --git a/ucm2/sof-soundwire/cs35l56-4.conf b/ucm2/sof-soundwire/cs35l56-4.conf > new file mode 100644 > index 0000000..f5af1e4 > --- /dev/null > +++ b/ucm2/sof-soundwire/cs35l56-4.conf > @@ -0,0 +1,24 @@ > +# Use case Configuration for sof-soundwire card > + > +SectionDevice."Speaker" { > + Comment "Speaker" > + > + EnableSequence [ > + cset "name='AMP1 Speaker Switch' 1" > + cset "name='AMP2 Speaker Switch' 1" > + cset "name='AMP3 Speaker Switch' 1" > + cset "name='AMP4 Speaker Switch' 1" > + ] > + > + DisableSequence [ > + cset "name='AMP4 Speaker Switch' 0" > + cset "name='AMP3 Speaker Switch' 0" > + cset "name='AMP2 Speaker Switch' 0" > + cset "name='AMP1 Speaker Switch' 0" > + ] If we only need an on/off switch, I wonder if this can be made conditional, i.e. enable/disable a control if it exists. That would scale to various numbers of amplifiers without a need to add a 2-amp, 6 or 8-amp configuration. > + > + Value { > + PlaybackPriority 100 > + PlaybackPCM "hw:${CardId},2" > + } > +}
On Tue, Dec 05, 2023 at 09:25:27AM -0600, Pierre-Louis Bossart wrote: > On 12/5/23 08:24, Charles Keepax wrote: > > + EnableSequence [ > > + cset "name='AMP1 Speaker Switch' 1" > > + cset "name='AMP2 Speaker Switch' 1" > > + cset "name='AMP3 Speaker Switch' 1" > > + cset "name='AMP4 Speaker Switch' 1" > > + ] > > + > > + DisableSequence [ > > + cset "name='AMP4 Speaker Switch' 0" > > + cset "name='AMP3 Speaker Switch' 0" > > + cset "name='AMP2 Speaker Switch' 0" > > + cset "name='AMP1 Speaker Switch' 0" > > + ] > > If we only need an on/off switch, I wonder if this can be made > conditional, i.e. enable/disable a control if it exists. That would > scale to various numbers of amplifiers without a need to add a 2-amp, 6 > or 8-amp configuration. I think that is possible, would you lean towards modifying HiFi.conf to only include a single file for cs35l56, or would you lean more towards having each cs35l56-x.conf file include a single base file? Thanks, Charles
On 12/5/23 10:28, Charles Keepax wrote: > On Tue, Dec 05, 2023 at 09:25:27AM -0600, Pierre-Louis Bossart wrote: >> On 12/5/23 08:24, Charles Keepax wrote: >>> + EnableSequence [ >>> + cset "name='AMP1 Speaker Switch' 1" >>> + cset "name='AMP2 Speaker Switch' 1" >>> + cset "name='AMP3 Speaker Switch' 1" >>> + cset "name='AMP4 Speaker Switch' 1" >>> + ] >>> + >>> + DisableSequence [ >>> + cset "name='AMP4 Speaker Switch' 0" >>> + cset "name='AMP3 Speaker Switch' 0" >>> + cset "name='AMP2 Speaker Switch' 0" >>> + cset "name='AMP1 Speaker Switch' 0" >>> + ] >> >> If we only need an on/off switch, I wonder if this can be made >> conditional, i.e. enable/disable a control if it exists. That would >> scale to various numbers of amplifiers without a need to add a 2-amp, 6 >> or 8-amp configuration. > > I think that is possible, would you lean towards modifying > HiFi.conf to only include a single file for cs35l56, or would you > lean more towards having each cs35l56-x.conf file include a > single base file? I wasn't referring to partitioning of files, rather the conditional UCM syntax, Condition { Type ControlExists Control "name='AMP4 Speaker Switch'" } e.g. https://github.com/alsa-project/alsa-ucm-conf/blob/master/ucm2/Intel/sof-hda-dsp/HiFi.conf#L37 I am not sure however if this can be part of an Enable/Disable sequence, that was really a question for Jaroslav.
On Tue, Dec 05, 2023 at 11:11:03AM -0600, Pierre-Louis Bossart wrote: > On 12/5/23 10:28, Charles Keepax wrote: > > On Tue, Dec 05, 2023 at 09:25:27AM -0600, Pierre-Louis Bossart wrote: > >> On 12/5/23 08:24, Charles Keepax wrote: > >>> + EnableSequence [ > >>> + cset "name='AMP1 Speaker Switch' 1" > >>> + cset "name='AMP2 Speaker Switch' 1" > >>> + cset "name='AMP3 Speaker Switch' 1" > >>> + cset "name='AMP4 Speaker Switch' 1" > >>> + ] > >> > >> If we only need an on/off switch, I wonder if this can be made > >> conditional, i.e. enable/disable a control if it exists. That would > >> scale to various numbers of amplifiers without a need to add a 2-amp, 6 > >> or 8-amp configuration. > > > > I think that is possible, would you lean towards modifying > > HiFi.conf to only include a single file for cs35l56, or would you > > lean more towards having each cs35l56-x.conf file include a > > single base file? > > I wasn't referring to partitioning of files, rather the conditional UCM > syntax, > > Condition { > Type ControlExists > Control "name='AMP4 Speaker Switch'" > } > I get that, but once you have added those you still have the issue HiFi.conf will load the speaker use-case as follows: False.Include.spkdev.File "/sof-soundwire/${var:SpeakerCodec1}-${var:SpeakerAmps1}.conf" Meaning the number of amps will be part of the file name requested. So my question was how you wanted to deal with that? Personally I would lean towards just having all the cs35l56-8.conf, cs35l56-6.conf etc. include a cs35l56-base.conf. Its slightly more files, but feels a bit less crufty than having a special case for cs35l56 to not include the number of amps in the filename. Thanks, Charles
On 06. 12. 23 10:47, Charles Keepax wrote: > On Tue, Dec 05, 2023 at 11:11:03AM -0600, Pierre-Louis Bossart wrote: >> On 12/5/23 10:28, Charles Keepax wrote: >>> On Tue, Dec 05, 2023 at 09:25:27AM -0600, Pierre-Louis Bossart wrote: >>>> On 12/5/23 08:24, Charles Keepax wrote: >>>>> + EnableSequence [ >>>>> + cset "name='AMP1 Speaker Switch' 1" >>>>> + cset "name='AMP2 Speaker Switch' 1" >>>>> + cset "name='AMP3 Speaker Switch' 1" >>>>> + cset "name='AMP4 Speaker Switch' 1" >>>>> + ] >>>> >>>> If we only need an on/off switch, I wonder if this can be made >>>> conditional, i.e. enable/disable a control if it exists. That would >>>> scale to various numbers of amplifiers without a need to add a 2-amp, 6 >>>> or 8-amp configuration. >>> >>> I think that is possible, would you lean towards modifying >>> HiFi.conf to only include a single file for cs35l56, or would you >>> lean more towards having each cs35l56-x.conf file include a >>> single base file? >> >> I wasn't referring to partitioning of files, rather the conditional UCM >> syntax, >> >> Condition { >> Type ControlExists >> Control "name='AMP4 Speaker Switch'" >> } >> > > I get that, but once you have added those you still have the > issue HiFi.conf will load the speaker use-case as follows: > > False.Include.spkdev.File "/sof-soundwire/${var:SpeakerCodec1}-${var:SpeakerAmps1}.conf" This is a good question. From the maintainer POV, I would prefer to use conditionals (join the common code) also for other codecs, so I vote to remove SpeakerAmps1 substitution from the filename for all codecs. Jaroslav
On Wed, Dec 06, 2023 at 11:09:29AM +0100, Jaroslav Kysela wrote: > On 06. 12. 23 10:47, Charles Keepax wrote: > >On Tue, Dec 05, 2023 at 11:11:03AM -0600, Pierre-Louis Bossart wrote: > >>On 12/5/23 10:28, Charles Keepax wrote: > >>>On Tue, Dec 05, 2023 at 09:25:27AM -0600, Pierre-Louis Bossart wrote: > >>>>On 12/5/23 08:24, Charles Keepax wrote: > >I get that, but once you have added those you still have the > >issue HiFi.conf will load the speaker use-case as follows: > > > >False.Include.spkdev.File "/sof-soundwire/${var:SpeakerCodec1}-${var:SpeakerAmps1}.conf" > > This is a good question. From the maintainer POV, I would prefer to > use conditionals (join the common code) also for other codecs, so I > vote to remove SpeakerAmps1 substitution from the filename for all > codecs. Ok I think for now I will add a conditional to drop the SpeakerAmps1 just for the cs35l56, there are 3 realtek devices that also use this stuff. They look slightly more complex than the Cirrus case, although maybe doable with conditionals. But I will leave those for people with the hardware to look at. Thanks, Charles
>>>>> + EnableSequence [ >>>>> + cset "name='AMP1 Speaker Switch' 1" >>>>> + cset "name='AMP2 Speaker Switch' 1" >>>>> + cset "name='AMP3 Speaker Switch' 1" >>>>> + cset "name='AMP4 Speaker Switch' 1" >>>>> + ] >>>> >>>> If we only need an on/off switch, I wonder if this can be made >>>> conditional, i.e. enable/disable a control if it exists. That would >>>> scale to various numbers of amplifiers without a need to add a 2-amp, 6 >>>> or 8-amp configuration. >>> >>> I think that is possible, would you lean towards modifying >>> HiFi.conf to only include a single file for cs35l56, or would you >>> lean more towards having each cs35l56-x.conf file include a >>> single base file? >> >> I wasn't referring to partitioning of files, rather the conditional UCM >> syntax, >> >> Condition { >> Type ControlExists >> Control "name='AMP4 Speaker Switch'" >> } >> > > I get that, but once you have added those you still have the > issue HiFi.conf will load the speaker use-case as follows: > > False.Include.spkdev.File "/sof-soundwire/${var:SpeakerCodec1}-${var:SpeakerAmps1}.conf" > > Meaning the number of amps will be part of the file name > requested. So my question was how you wanted to deal with that? > Personally I would lean towards just having all the > cs35l56-8.conf, cs35l56-6.conf etc. include a cs35l56-base.conf. > Its slightly more files, but feels a bit less crufty than having > a special case for cs35l56 to not include the number of amps in > the filename. Ah yes, I forgot that part... I must admit I don't recall either what we were trying to achieve in UCM with the number of amplifiers and speakers both added to the components string: "cfg-spk:%d cfg-amp:%d" Probably best to go with your solution and see what can be optimized later...
diff --git a/ucm2/sof-soundwire/cs35l56-4.conf b/ucm2/sof-soundwire/cs35l56-4.conf new file mode 100644 index 0000000..f5af1e4 --- /dev/null +++ b/ucm2/sof-soundwire/cs35l56-4.conf @@ -0,0 +1,24 @@ +# Use case Configuration for sof-soundwire card + +SectionDevice."Speaker" { + Comment "Speaker" + + EnableSequence [ + cset "name='AMP1 Speaker Switch' 1" + cset "name='AMP2 Speaker Switch' 1" + cset "name='AMP3 Speaker Switch' 1" + cset "name='AMP4 Speaker Switch' 1" + ] + + DisableSequence [ + cset "name='AMP4 Speaker Switch' 0" + cset "name='AMP3 Speaker Switch' 0" + cset "name='AMP2 Speaker Switch' 0" + cset "name='AMP1 Speaker Switch' 0" + ] + + Value { + PlaybackPriority 100 + PlaybackPCM "hw:${CardId},2" + } +}
cs35l56 is a boosted speaker amp add UCM support for a 4 amp configuration. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> --- ucm2/sof-soundwire/cs35l56-4.conf | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 ucm2/sof-soundwire/cs35l56-4.conf