All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] ASoC: codecs: fix ES8326 performance and pop noise
@ 2024-01-19 11:28 Zhu Ning
  2024-01-19 11:28 ` [PATCH 1/5] ASoC: codecs: ES8326: improving crosstalk performance Zhu Ning
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Zhu Ning @ 2024-01-19 11:28 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: tiwai, amadeuszx.slawinski, yangxiaohua, zhuning, zhangyi,
	Zhu Ning

    Hi ,

    5 patches here for the es8326 driver...
    
    We get some issues regarding crosstalk, THD+N performance and pop 
    noise from customer's project.
    Five patches are used to fix these issues. We did tests with the new 
    driver. The test results form test department met our expectations.

    Thanks,
    Michael.

Zhu Ning (5):
  ASoC: codecs: ES8326: improving crosstalk performance
  ASoC: codecs: ES8326: Improving the THD+N performance
  ASoC: codecs: ES8326: Adding new volume kcontrols
  ASoC: codecs: ES8326: Minimize the pop noise on headphone
  ASoC: codecs: ES8326: fix the capture noise issue

 sound/soc/codecs/es8326.c | 216 +++++++++++++++++++++++++++++---------
 sound/soc/codecs/es8326.h |   8 +-
 2 files changed, 175 insertions(+), 49 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread
[parent not found: <lkp@intel.com>]
* RE: [PATCH 3/5] ASoC: codecs: ES8326: Adding new volume kcontrols
@ 2024-01-20  4:57 Zhu Ning
  0 siblings, 0 replies; 11+ messages in thread
From: Zhu Ning @ 2024-01-20  4:57 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: tiwai, amadeuszx.slawinski, yangxiaohua, zhuning, zhangyi

> Volumes should be standard volume controls with TLV information rather
> than enumerations. I see that you can't just use a standard volume
> control here since there's an invalid value that needs to be skipped
> over, and it doesn't help that there's no ordering to the values either.
> I think the best thing would probably be to open code a custom volume
> control - this feels weird enough that it's probably not worrying about
> trying for reuse. Just have put and get functions that map 0..6 into
> the register values for the volumes in ascending order, then you can
> have a control that looks normal to userspace.

I'll try your mentioned method in a new version of the same patch.

^ permalink raw reply	[flat|nested] 11+ messages in thread
* RE: [PATCH 3/5] ASoC: codecs: ES8326: Adding new volume kcontrols
@ 2024-01-24  5:48 Zhu Ning
  0 siblings, 0 replies; 11+ messages in thread
From: Zhu Ning @ 2024-01-24  5:48 UTC (permalink / raw)
  To: alsa-devel, broonie
  Cc: tiwai, amadeuszx.slawinski, yangxiaohua, zhuning, zhangyi

> ES8326 features a headphone volume control register and four DAC
> volume control registers.
> We add new volume Kcontrols for these registers to enhance the
> configurability of the volume settings, providing users with
> greater flexibility.
This is much better integrated than the earlier version, but there's
still a few issues.
> +static int es8326_hplvol_set(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> + struct es8326_priv *es8326 = snd_soc_component_get_drvdata(component);
> + unsigned int hp_vol;
> +
> + if (ucontrol->value.integer.value[0] == 3) {
> > + dev_dbg(component->dev, "HPL_VOL does not change");
> > + return 0;
> > + }
> This will silently ignore attempts to write the invalid value which
> isn't great any might confuse some userspace code, it would be better to
> do something like
> val = ucontrol->value.integer.value[0];
> if (val >= 3)
> val++;
> (with corresponding changes to the other functions) so that we just skip
> over the invalid value without userspace being aware of it. We should
> also be validating that out of range values are rejected or at least
> constained to the relevant bitfield, other than 3 any invalid value will
> just be written straight into the register (rejecting should be more
> robust).
> > + regmap_read(es8326->regmap, ES8326_HP_VOL, &hp_vol);
> > + hp_vol &= 0x8F;
> > + hp_vol |= ucontrol->value.integer.value[0] << 4;
> > + regmap_write(es8326->regmap, ES8326_HP_VOL, hp_vol);
> regmap_update_bits().
> > +
> > + return 0;
> > +}
> Also please run mixer-test on your driver - for this control it'll tell
> you that this function isn't returning 1 when the value changes, meaning
> that events won't be generated when the value changes.
> > + SOC_SINGLE_TLV("HPL Playback Volume", ES8326_DACL_VOL, 0, 0xbf, 0, dac_vol_tlv),
> > + SOC_SINGLE_TLV("HPR Playback Volume", ES8326_DACR_VOL, 0, 0xbf, 0, dac_vol_tlv),
> > + SOC_SINGLE_TLV("SPKL Playback Volume", ES8326_SPKL_VOL, 0, 0xbf, 0, dac_vol_tlv),
> > + SOC_SINGLE_TLV("SPKR Playback Volume", ES8326_SPKR_VOL, 0, 0xbf, 0, dac_vol_tlv),
> It would be *better* if these were stereo controls, but it's not
> essential.
> > +
> > + SOC_ENUM("HPVol SPKVol switch", hpvol_spkvol_switch),
> Switch should have a capital letter (mixer-test should catch this as
> well).

I'll take your advice and submit a separate new patch

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-01-24  5:58 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-19 11:28 [PATCH 0/5] ASoC: codecs: fix ES8326 performance and pop noise Zhu Ning
2024-01-19 11:28 ` [PATCH 1/5] ASoC: codecs: ES8326: improving crosstalk performance Zhu Ning
2024-01-19 11:28 ` [PATCH 2/5] ASoC: codecs: ES8326: Improving the THD+N performance Zhu Ning
2024-01-19 11:28 ` [PATCH 3/5] ASoC: codecs: ES8326: Adding new volume kcontrols Zhu Ning
2024-01-19 13:53   ` Mark Brown
2024-01-20  3:03   ` kernel test robot
2024-01-19 11:28 ` [PATCH 4/5] ASoC: codecs: ES8326: Minimize the pop noise on headphone Zhu Ning
2024-01-19 11:28 ` [PATCH 5/5] ASoC: codecs: ES8326: fix the capture noise issue Zhu Ning
     [not found] <lkp@intel.com>
2024-01-20  4:53 ` [PATCH 3/5] ASoC: codecs: ES8326: Adding new volume kcontrols Zhu Ning
  -- strict thread matches above, loose matches on Subject: below --
2024-01-20  4:57 Zhu Ning
2024-01-24  5:48 Zhu Ning

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.