Re: [PATCH v1 2/2] ALSA: hda/cirrus: Use CS8409 Equalizer to fix abnormal sounds on Bullseye

[Takashi Iwai <tiwai@suse.de>]

On Sat, 24 Apr 2021 16:32:44 +0200,
Vitaly Rodionov wrote:
> 
> From: Stefan Binding <sbinding@opensource.cirrus.com>
> 
> Tested on DELL Inspiron-3505, DELL Inspiron-3501, DELL Inspiron-3500

Similarly like the previous patch, the description about the problem
itself is missing, so I cannot judge whether to take this or not.
Please clarify at first.

Also, it needs clarification whether applying this EQ to all hardware
models with this codec chip is really safe or not; i.e. it's mandatory
for the codec.


thanks,

Takashi

> 
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
> Signed-off-by: Vitaly Rodionov <vitalyr@opensource.cirrus.com>
> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1924997
> Reported-and-tested-by: You-Sheng Yang <vicamo.yang@canonical.com>
> ---
>  sound/pci/hda/patch_cirrus.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
> index d6cf93b7483c..82c5f0869684 100644
> --- a/sound/pci/hda/patch_cirrus.c
> +++ b/sound/pci/hda/patch_cirrus.c
> @@ -1481,6 +1481,34 @@ static const struct cs8409_cir_param cs8409_cs42l42_hw_cfg[] = {
>  	{} /* Terminator */
>  };
>  
> +static const struct cs8409_cir_param cs8409_cs42l42_bullseye_atn[] = {
> +	{ 0x47, 0x65, 0x4000 }, /* EQ_SEL=1, EQ1/2_EN=0 */
> +	{ 0x47, 0x64, 0x4000 }, /* +EQ_ACC */
> +	{ 0x47, 0x65, 0x4010 }, /* +EQ2_EN */
> +	{ 0x47, 0x63, 0x0647 }, /* EQ_DATA_HI=0x0647 */
> +	{ 0x47, 0x64, 0xc0c7 }, /* +EQ_WRT, +EQ_ACC, EQ_ADR=0, EQ_DATA_LO=0x67 */
> +	{ 0x47, 0x63, 0x0647 }, /* EQ_DATA_HI=0x0647 */
> +	{ 0x47, 0x64, 0xc1c7 }, /* +EQ_WRT, +EQ_ACC, EQ_ADR=1, EQ_DATA_LO=0x67 */
> +	{ 0x47, 0x63, 0xf370 }, /* EQ_DATA_HI=0xf370 */
> +	{ 0x47, 0x64, 0xc271 }, /* +EQ_WRT, +EQ_ACC, EQ_ADR=2, EQ_DATA_LO=0x71 */
> +	{ 0x47, 0x63, 0x1ef8 }, /* EQ_DATA_HI=0x1ef8 */
> +	{ 0x47, 0x64, 0xc348 }, /* +EQ_WRT, +EQ_ACC, EQ_ADR=3, EQ_DATA_LO=0x48 */
> +	{ 0x47, 0x63, 0xc110 }, /* EQ_DATA_HI=0xc110 */
> +	{ 0x47, 0x64, 0xc45a }, /* +EQ_WRT, +EQ_ACC, EQ_ADR=4, EQ_DATA_LO=0x5a */
> +	{ 0x47, 0x63, 0x1f29 }, /* EQ_DATA_HI=0x1f29 */
> +	{ 0x47, 0x64, 0xc574 }, /* +EQ_WRT, +EQ_ACC, EQ_ADR=5, EQ_DATA_LO=0x74 */
> +	{ 0x47, 0x63, 0x1d7a }, /* EQ_DATA_HI=0x1d7a */
> +	{ 0x47, 0x64, 0xc653 }, /* +EQ_WRT, +EQ_ACC, EQ_ADR=6, EQ_DATA_LO=0x53 */
> +	{ 0x47, 0x63, 0xc38c }, /* EQ_DATA_HI=0xc38c */
> +	{ 0x47, 0x64, 0xc714 }, /* +EQ_WRT, +EQ_ACC, EQ_ADR=7, EQ_DATA_LO=0x14 */
> +	{ 0x47, 0x63, 0x1ca3 }, /* EQ_DATA_HI=0x1ca3 */
> +	{ 0x47, 0x64, 0xc8c7 }, /* +EQ_WRT, +EQ_ACC, EQ_ADR=8, EQ_DATA_LO=0xc7 */
> +	{ 0x47, 0x63, 0xc38c }, /* EQ_DATA_HI=0xc38c */
> +	{ 0x47, 0x64, 0xc914 }, /* +EQ_WRT, +EQ_ACC, EQ_ADR=9, EQ_DATA_LO=0x14 */
> +	{ 0x47, 0x64, 0x0000 }, /* -EQ_ACC, -EQ_WRT */
> +	{} /* Terminator */
> +};
> +
>  /**
>   * cs8409_enable_i2c_clock - Enable I2C clocks
>   * @codec: the codec instance
> @@ -2029,6 +2057,7 @@ static void cs8409_enable_ur(struct hda_codec *codec, int flag)
>  static void cs8409_cs42l42_hw_init(struct hda_codec *codec)
>  {
>  	const struct cs8409_cir_param *seq = cs8409_cs42l42_hw_cfg;
> +	const struct cs8409_cir_param *seq_bullseye = cs8409_cs42l42_bullseye_atn;
>  	struct cs_spec *spec = codec->spec;
>  
>  	if (spec->gpio_mask) {
> @@ -2043,6 +2072,10 @@ static void cs8409_cs42l42_hw_init(struct hda_codec *codec)
>  	for (; seq->nid; seq++)
>  		cs_vendor_coef_set(codec, seq->cir, seq->coeff);
>  
> +	if (codec->fixup_id == CS8409_BULLSEYE)
> +		for (; seq_bullseye->nid; seq_bullseye++)
> +			cs_vendor_coef_set(codec, seq_bullseye->cir, seq_bullseye->coeff);
> +
>  	/* Disable Unsolicited Response during boot */
>  	cs8409_enable_ur(codec, 0);
>  
> -- 
> 2.25.1
>