diff mbox series

[alsa-ucm-conf,1/2] sof-soundwire: Add basic support for cs42l43

Message ID 20231205142420.1256042-1-ckeepax@opensource.cirrus.com
State Superseded
Headers show
Series [alsa-ucm-conf,1/2] sof-soundwire: Add basic support for cs42l43 | expand

Commit Message

Charles Keepax Dec. 5, 2023, 2:24 p.m. UTC 
cs42l43 is a codec device, add basic support for it. Including a dual
channel DMIC input, stereo headphones, and a mono headset microphone.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 ucm2/sof-soundwire/cs42l43-dmic.conf | 28 +++++++++++++++++
 ucm2/sof-soundwire/cs42l43.conf      | 47 ++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 ucm2/sof-soundwire/cs42l43-dmic.conf
 create mode 100644 ucm2/sof-soundwire/cs42l43.conf

Comments

Pierre-Louis Bossart Dec. 5, 2023, 3:25 p.m. UTC | #1 
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"
> +	}
> +}
Charles Keepax Dec. 6, 2023, 9:47 a.m. UTC | #2 
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
Jaroslav Kysela Dec. 6, 2023, 10:09 a.m. UTC | #3 
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
Charles Keepax Dec. 6, 2023, 10:31 a.m. UTC | #4 
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
Pierre-Louis Bossart Dec. 6, 2023, 3:39 p.m. UTC | #5 
>>>>> +	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 mbox series

Patch

diff --git a/ucm2/sof-soundwire/cs42l43-dmic.conf b/ucm2/sof-soundwire/cs42l43-dmic.conf
new file mode 100644
index 0000000..bf59eca
--- /dev/null
+++ b/ucm2/sof-soundwire/cs42l43-dmic.conf
@@ -0,0 +1,28 @@ 
+# Use case Configuration for sof-soundwire card
+
+SectionDevice."Mic" {
+	Comment "Microphones"
+
+	ConflictingDevice [
+		"Headset"
+	]
+
+	EnableSequence [
+		cset "name='cs42l43 DP1TX1 Input' 'Decimator 3'"
+		cset "name='cs42l43 DP1TX2 Input' 'Decimator 4'"
+		cset "name='cs42l43 Decimator 3 Switch' on"
+		cset "name='cs42l43 Decimator 4 Switch' on"
+	]
+
+	DisableSequence [
+		cset "name='cs42l43 Decimator 3 Switch' off"
+		cset "name='cs42l43 Decimator 4 Switch' off"
+		cset "name='cs42l43 DP1TX1 Input' 'None'"
+		cset "name='cs42l43 DP1TX2 Input' 'None'"
+	]
+
+	Value {
+		CapturePriority 100
+		CapturePCM "hw:${CardId},4"
+	}
+}
diff --git a/ucm2/sof-soundwire/cs42l43.conf b/ucm2/sof-soundwire/cs42l43.conf
new file mode 100644
index 0000000..378dbb2
--- /dev/null
+++ b/ucm2/sof-soundwire/cs42l43.conf
@@ -0,0 +1,47 @@ 
+# Use case Configuration for sof-soundwire card
+
+SectionDevice."Headphones" {
+	Comment "Headphones"
+
+	EnableSequence [
+		cset "name='cs42l43 Headphone L Input 1' 'DP5RX1'"
+		cset "name='cs42l43 Headphone R Input 1' 'DP5RX2'"
+	]
+
+	DisableSequence [
+		cset "name='cs42l43 Headphone L Input 1' 'None'"
+		cset "name='cs42l43 Headphone R Input 1' 'None'"
+	]
+
+	Value {
+		PlaybackPriority 200
+		PlaybackPCM "hw:${CardId},0"
+		PlaybackVolume "cs42l43 Headphone Digital Volume"
+		JackControl "Headphone Jack"
+	}
+}
+
+SectionDevice."Headset" {
+	Comment "Headset Microphone"
+
+	EnableSequence [
+		cset "name='cs42l43 ADC1 Input' 'IN1'"
+		cset "name='cs42l43 Decimator 1 Mode' 'ADC'"
+
+		cset "name='cs42l43 DP1TX1 Input' 'Decimator 1'"
+		cset "name='cs42l43 DP1TX2 Input' 'Decimator 1'"
+	]
+
+	DisableSequence [
+		cset "name='cs42l43 DP1TX1 Input' 'None'"
+		cset "name='cs42l43 DP1TX2 Input' 'None'"
+	]
+
+	Value {
+		CapturePriority 200
+		CapturePCM "hw:${CardId},4"
+		CaptureVolume "cs42l43 Decimator 1 Volume"
+		CaptureSwitch "cs42l43 Decimator 1 Switch"
+		JackControl "Headset Mic Jack"
+	}
+}