[PATCH 0/5] Minor default jack pop performance updates
Some small updates to the driver defaults to ensure a good pop performance on jack insert and removal.
Thanks, Charles
Charles Keepax (5): dt-bindings: ASoC: cirrus,cs42l43: Update a couple of default values ASoC: cs42l43: Lower default type detect time ASoC: cs42l43: Enable bias sense by default ASoC: cs42l43: Move headset bias sense enable earlier in process ASoC: cs42l43: Extend timeout on bias sense timeout
.../bindings/sound/cirrus,cs42l43.yaml | 4 +- sound/soc/codecs/cs42l43-jack.c | 38 +++++++++---------- 2 files changed, 21 insertions(+), 21 deletions(-)
The bias sense is being enabled by default in the driver, and the default detect time is being dropped slightly. Update the binding document to match.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml index 7a6de938b11d1..4fa22fa70ace5 100644 --- a/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml +++ b/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml @@ -83,7 +83,7 @@ properties: Current at which the headset micbias sense clamp will engage, 0 to disable. enum: [ 0, 14, 23, 41, 50, 60, 68, 86, 95 ] - default: 0 + default: 14
cirrus,bias-ramp-ms: description: @@ -97,7 +97,7 @@ properties: Time in microseconds the type detection will run for. Long values will cause more audible effects, but give more accurate detection. enum: [ 20, 100, 1000, 10000, 50000, 75000, 100000, 200000 ] - default: 10000 + default: 1000
cirrus,button-automute: type: boolean
On Tue, Sep 19, 2023 at 11:31:12AM +0100, Charles Keepax wrote:
The bias sense is being enabled by default in the driver, and the default detect time is being dropped slightly. Update the binding document to match.
That's not really a compatible change. If I wrote my DT expecting bias sense was disabled by default then the OS changes behavior, my platform behavior would change. Maybe that doesn't matter here? IDK. It's on you if this breaks anyone, so:
Acked-by: Rob Herring robh@kernel.org
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com
Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml index 7a6de938b11d1..4fa22fa70ace5 100644 --- a/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml +++ b/Documentation/devicetree/bindings/sound/cirrus,cs42l43.yaml @@ -83,7 +83,7 @@ properties: Current at which the headset micbias sense clamp will engage, 0 to disable. enum: [ 0, 14, 23, 41, 50, 60, 68, 86, 95 ]
- default: 0
default: 14
cirrus,bias-ramp-ms: description:
@@ -97,7 +97,7 @@ properties: Time in microseconds the type detection will run for. Long values will cause more audible effects, but give more accurate detection. enum: [ 20, 100, 1000, 10000, 50000, 75000, 100000, 200000 ]
- default: 10000
default: 1000
cirrus,button-automute: type: boolean
-- 2.39.2
On Tue, Sep 19, 2023 at 02:23:02PM -0500, Rob Herring wrote:
On Tue, Sep 19, 2023 at 11:31:12AM +0100, Charles Keepax wrote:
The bias sense is being enabled by default in the driver, and the default detect time is being dropped slightly. Update the binding document to match.
That's not really a compatible change. If I wrote my DT expecting bias sense was disabled by default then the OS changes behavior, my platform behavior would change. Maybe that doesn't matter here? IDK. It's on you if this breaks anyone, so:
Acked-by: Rob Herring robh@kernel.org
Yeah I appreciate that, but this should be fine, the part is very very new. The only systems using the part are still in development.
Thanks, Charles
The current default is a little excessive, reduce the pop on insertion by reducing the time a little. The new value of 1000uS is still pretty conservative.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs42l43-jack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c index 92e37bc1df9dc..7008e50eded96 100644 --- a/sound/soc/codecs/cs42l43-jack.c +++ b/sound/soc/codecs/cs42l43-jack.c @@ -110,7 +110,7 @@ int cs42l43_set_jack(struct snd_soc_component *component, priv->buttons[3] = 735; }
- ret = cs42l43_find_index(priv, "cirrus,detect-us", 10000, &priv->detect_us, + ret = cs42l43_find_index(priv, "cirrus,detect-us", 1000, &priv->detect_us, cs42l43_accdet_us, ARRAY_SIZE(cs42l43_accdet_us)); if (ret < 0) goto error;
Improve the default pop performance on jack removal by enabling bias sense on the least sensitive level by default.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs42l43-jack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c index 7008e50eded96..7bd7cc1779506 100644 --- a/sound/soc/codecs/cs42l43-jack.c +++ b/sound/soc/codecs/cs42l43-jack.c @@ -127,7 +127,7 @@ int cs42l43_set_jack(struct snd_soc_component *component,
hs2 |= ret << CS42L43_HSBIAS_RAMP_SHIFT;
- ret = cs42l43_find_index(priv, "cirrus,bias-sense-microamp", 0, + ret = cs42l43_find_index(priv, "cirrus,bias-sense-microamp", 14, &priv->bias_sense_ua, cs42l43_accdet_bias_sense, ARRAY_SIZE(cs42l43_accdet_bias_sense)); if (ret < 0)
Currently the bias sense is enabled along with the button detect, but this has two problems. Firstly, the detections themselves arn't covered by the bias sense, potentially resulting in pops and secondly, the sequence of enabling/disabling looks like:
enable bias enable bias sense disable bias sense disable bias
When the bias sense is disabled but the bias is still on the clamp is removed and a pop results. Fix both of these issues by moving the bias sense enable/disable to be along with the bias itself. With a resulting sequence of:
enable bias sense enable bias disable bias disable bias sense
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs42l43-jack.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c index 7bd7cc1779506..66923cf2fdaff 100644 --- a/sound/soc/codecs/cs42l43-jack.c +++ b/sound/soc/codecs/cs42l43-jack.c @@ -250,6 +250,15 @@ static void cs42l43_start_hs_bias(struct cs42l43_codec *priv, bool force_high) if (!force_high && priv->bias_low) val = 0x2 << CS42L43_HSBIAS_MODE_SHIFT;
+ if (priv->bias_sense_ua) { + regmap_update_bits(cs42l43->regmap, + CS42L43_HS_BIAS_SENSE_AND_CLAMP_AUTOCONTROL, + CS42L43_HSBIAS_SENSE_EN_MASK | + CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK, + CS42L43_HSBIAS_SENSE_EN_MASK | + CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK); + } + regmap_update_bits(cs42l43->regmap, CS42L43_MIC_DETECT_CONTROL_1, CS42L43_HSBIAS_MODE_MASK, val);
@@ -267,6 +276,13 @@ static void cs42l43_stop_hs_bias(struct cs42l43_codec *priv)
regmap_update_bits(cs42l43->regmap, CS42L43_HS2, CS42L43_HS_CLAMP_DISABLE_MASK, 0); + + if (priv->bias_sense_ua) { + regmap_update_bits(cs42l43->regmap, + CS42L43_HS_BIAS_SENSE_AND_CLAMP_AUTOCONTROL, + CS42L43_HSBIAS_SENSE_EN_MASK | + CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK, 0); + } }
irqreturn_t cs42l43_bias_detect_clamp(int irq, void *data) @@ -318,15 +334,6 @@ static void cs42l43_start_button_detect(struct cs42l43_codec *priv) regmap_update_bits(cs42l43->regmap, CS42L43_MIC_DETECT_CONTROL_1, CS42L43_BUTTON_DETECT_MODE_MASK | CS42L43_MIC_LVL_DET_DISABLE_MASK, val); - - if (priv->bias_sense_ua) { - regmap_update_bits(cs42l43->regmap, - CS42L43_HS_BIAS_SENSE_AND_CLAMP_AUTOCONTROL, - CS42L43_HSBIAS_SENSE_EN_MASK | - CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK, - CS42L43_HSBIAS_SENSE_EN_MASK | - CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK); - } }
static void cs42l43_stop_button_detect(struct cs42l43_codec *priv) @@ -335,13 +342,6 @@ static void cs42l43_stop_button_detect(struct cs42l43_codec *priv)
dev_dbg(priv->dev, "Stop button detect\n");
- if (priv->bias_sense_ua) { - regmap_update_bits(cs42l43->regmap, - CS42L43_HS_BIAS_SENSE_AND_CLAMP_AUTOCONTROL, - CS42L43_HSBIAS_SENSE_EN_MASK | - CS42L43_AUTO_HSBIAS_CLAMP_EN_MASK, 0); - } - regmap_update_bits(cs42l43->regmap, CS42L43_MIC_DETECT_CONTROL_1, CS42L43_BUTTON_DETECT_MODE_MASK | CS42L43_MIC_LVL_DET_DISABLE_MASK,
For very slow removals the current bias sense timeout is sometimes too short and unclamps the mic bias before the jack removal is properly detected by the tip detect, causing a pop. As bias sense should be tuned to deliver very few false positives, increase the timeout fairly dramatically to cover all but the most exaggerated removals.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com --- sound/soc/codecs/cs42l43-jack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/cs42l43-jack.c b/sound/soc/codecs/cs42l43-jack.c index 66923cf2fdaff..861f9ee671cdf 100644 --- a/sound/soc/codecs/cs42l43-jack.c +++ b/sound/soc/codecs/cs42l43-jack.c @@ -290,7 +290,7 @@ irqreturn_t cs42l43_bias_detect_clamp(int irq, void *data) struct cs42l43_codec *priv = data;
queue_delayed_work(system_wq, &priv->bias_sense_timeout, - msecs_to_jiffies(250)); + msecs_to_jiffies(1000));
return IRQ_HANDLED; }
On Tue, 19 Sep 2023 11:31:11 +0100, Charles Keepax wrote:
Some small updates to the driver defaults to ensure a good pop performance on jack insert and removal.
Thanks, Charles
Charles Keepax (5): dt-bindings: ASoC: cirrus,cs42l43: Update a couple of default values ASoC: cs42l43: Lower default type detect time ASoC: cs42l43: Enable bias sense by default ASoC: cs42l43: Move headset bias sense enable earlier in process ASoC: cs42l43: Extend timeout on bias sense timeout
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/5] dt-bindings: ASoC: cirrus,cs42l43: Update a couple of default values commit: aa7627111c689f9dc2458c7dd9c1fbb554502664 [2/5] ASoC: cs42l43: Lower default type detect time commit: 686b8f711b990d895d39dee3fab88861917d2dc4 [3/5] ASoC: cs42l43: Enable bias sense by default commit: 9c0ccc9f8e3be79ab44f5f8034ef90c367caf06f [4/5] ASoC: cs42l43: Move headset bias sense enable earlier in process commit: 1e4ce0d5c023e8d8663f6b79b98b9f8026776127 [5/5] ASoC: cs42l43: Extend timeout on bias sense timeout commit: 6388a0619c83625214e98377c32bcefa4fffb9cd
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (3)
-
Charles Keepax -
Mark Brown -
Rob Herring