fixup! ASoC: sof_sdw_rt1308/sof_sdw_rt1316/rt1308-sdw/rt1316-sdw: fix distorted sound #3945

shumingfan · 2022-10-21T06:35:43Z

Add BQ params into the device property at the machine driver level, then the codec driver will get the BQ params and apply them.
The Dell XPS 17 9710 model needs the BQ params for the tweeter/woofer.
Issue #3896

plbossart

thanks @shumingfan, I have a couple of nit-picks but also concerns on the error flows, see below.

sound/soc/codecs/rt1308-sdw.c

plbossart · 2022-10-21T14:53:56Z

sound/soc/codecs/rt1308-sdw.h

sound/soc/intel/boards/sof_sdw_rt1308.c

shumingfan · 2022-10-24T06:40:19Z

thanks @shumingfan, I have a couple of nit-picks but also concerns on the error flows, see below.

@plbossart Thanks for your review, I will update the PR.

sound/soc/codecs/rt1308-sdw.c

plbossart

looks good @shumingfan, now we need to do this for all other SKUs with 4 speakers...

plbossart · 2022-10-24T14:36:49Z

sound/soc/intel/boards/sof_sdw_rt1308.c

nit-pick: I would have used u8 for consistency with the device property definition but that works.

also maybe move this table to a new file which will only contain coefficients. The C file should be more about the programming sequences, the binary data should be in a separate file IMHO.

Sure, will move it to a separate .h file.

shumingfan · 2022-10-25T02:19:42Z

looks good @shumingfan, now we need to do this for all other SKUs with 4 speakers...

@plbossart I will collect the information from our hardware engineer. If other models need to set BQ parameters, I will add them to PR.

shumingfan · 2022-10-27T10:21:39Z

@plbossart I got the settings of all 4-speakers cases from the Windows team. Some models don't have BQ parameters.
However, the rest of the models of rt1308 having BQ parameters have the same parameters. The same situation happened to rt1316. The hardware engineer said those models use the same speaker design.

plbossart · 2022-10-27T12:58:44Z

@plbossart I got the settings of all 4-speakers cases from the Windows team. Some models don't have BQ parameters. However, the rest of the models of rt1308 having BQ parameters have the same parameters. The same situation happened to rt1316. The hardware engineer said those models use the same speaker design.

Makes sense, and that makes our life simpler, we just need to add the quirks for the relevant skus and point to the same table. Thanks for looking into this!

sound/soc/intel/boards/sof_sdw_rt1316.c

plbossart

Missing SKUs @shumingfan ?

plbossart · 2022-10-31T19:45:13Z

sound/soc/intel/boards/sof_sdw_rt1316.c

this list needs to be aligned with what we already have in sof_sdw.c, e.g. 0AFF is not listed here.

@plbossart I got the list from the agents. The 0AFF is used in the ADL platform and the model name is Stradale ADL.
The 0BDA/0BDB should be new models. They are used in the RPL platform and the model name is Stradale RPL.
If you want to remove those models for now, it will be ok for me.

I don't think that's quite right @shumingfan, we have not added any RPL devices just yet. @gongjun-song is working on RPL support with PR #3978

@plbossart Yeah, I know. I just follow the list information and the settings of the Windows driver to add those models.
The agents give me the latest list and the Windows team goes faster than Linux.

sound/soc/intel/boards/sof_sdw_rt1308.c

sound/soc/intel/boards/sof_sdw_rt1316.c

If the machine driver level sets the BQ params into the device property, the codec driver will get the BQ params and apply them. Signed-off-by: Shuming Fan <shumingf@realtek.com>

The Dell SKU 0A5D/0A5E/0990/098F model needs the BQ params for the tweeter/woofer. Signed-off-by: Shuming Fan <shumingf@realtek.com>

The Dell SKU 0B00/0B01/0AFE/0AFF model needs the BQ params for the tweeter/woofer. Signed-off-by: Shuming Fan <shumingf@realtek.com>

plbossart

LGTM @shumingfan can you send this upstream? Feel free to add my

Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com

plbossart · 2022-11-09T01:13:02Z

closing, this should go upstream now

shumingfan · 2022-11-09T06:59:21Z

LGTM @shumingfan can you send this upstream? Feel free to add my

Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com

@plbossart OK, thanks. I will send this upstream today.

JuanFcoMiranda · 2022-12-12T18:19:38Z

Hi, how could I apply this patch? I tried copying only the modifications and also downloading the modified files but when I compile my kernel, it always fails.

Thanks in advance.

plbossart · 2022-12-12T18:48:30Z

?? The code is upstream now:

1b435e4 ASoC: Intel: sof_sdw_rt1308: add BQ params for the Dell models
3c46b58 ASoC: rt1316-sdw: get BQ params property and apply them
5efb40b ASoC: rt1308-sdw: get BQ params property and apply them
f0c4d9f (tag: v6.1-rc4) Linux 6.1-rc4

JuanFcoMiranda · 2022-12-13T10:59:07Z

Has it already been published? On which version of the kernel?

plbossart · 2022-12-13T15:25:48Z

@JuanFcoMiranda I am not sure what you mean by "published", all patches first go to the ASoC maintainer tree, and they will flow into Linus Torvalds' tree during the merge window. this patch will be in 6.2 which will tentatively be released in 10 weeks or so.

JuanFcoMiranda · 2022-12-13T17:48:19Z

@plbossart I was meant when it would be available in the final release of the kernel. I'll be waiting for that version.

Thank you very much.

shumingfan requested review from RanderWang, ajitkupandey, bardliao, dbaluta, jack-cy-yu, kuanhsuncheng, kv2019i, lgirdwood, oder-chiou, plbossart, ranj063, ujfalusi and yaochunhung as code owners October 21, 2022 06:35

plbossart requested changes Oct 21, 2022

View reviewed changes

shumingfan force-pushed the fix-rt1308-distorted-sound branch from 16aed2d to f94c98f Compare October 24, 2022 07:11

RanderWang reviewed Oct 24, 2022

View reviewed changes

sound/soc/codecs/rt1308-sdw.c Outdated Show resolved Hide resolved

RanderWang reviewed Oct 24, 2022

View reviewed changes

sound/soc/codecs/rt1308-sdw.c Outdated Show resolved Hide resolved

shumingfan force-pushed the fix-rt1308-distorted-sound branch from f94c98f to 7e15a7d Compare October 24, 2022 08:55

plbossart previously approved these changes Oct 24, 2022

View reviewed changes

RanderWang previously approved these changes Oct 25, 2022

View reviewed changes

shumingfan dismissed stale reviews from RanderWang and plbossart via b40a1a7 October 27, 2022 10:09

shumingfan force-pushed the fix-rt1308-distorted-sound branch from 7e15a7d to b40a1a7 Compare October 27, 2022 10:09

shumingfan changed the title ~~fixup! ASoC: sof_sdw_rt1308/rt1308-sdw: fix distorted sound~~ fixup! ASoC: sof_sdw_rt1308/sof_sdw_rt1316/rt1308-sdw/rt1316-sdw: fix distorted sound Oct 27, 2022

RanderWang reviewed Oct 28, 2022

View reviewed changes

sound/soc/intel/boards/sof_sdw_rt1316.c Outdated Show resolved Hide resolved

shumingfan force-pushed the fix-rt1308-distorted-sound branch from b40a1a7 to dec7140 Compare October 28, 2022 03:04

RanderWang previously approved these changes Oct 28, 2022

View reviewed changes

plbossart requested changes Oct 31, 2022

View reviewed changes

shumingfan dismissed RanderWang’s stale review via 3762b34 November 1, 2022 04:32

shumingfan force-pushed the fix-rt1308-distorted-sound branch from dec7140 to 3762b34 Compare November 1, 2022 04:32

plbossart requested changes Nov 2, 2022

View reviewed changes

shumingfan added 4 commits November 3, 2022 13:57

ASoC: rt1308-sdw: get BQ params property and apply them

52607b7

If the machine driver level sets the BQ params into the device property, the codec driver will get the BQ params and apply them. Signed-off-by: Shuming Fan <shumingf@realtek.com>

ASoC: rt1316-sdw: get BQ params property and apply them

cb9b3bf

If the machine driver level sets the BQ params into the device property, the codec driver will get the BQ params and apply them. Signed-off-by: Shuming Fan <shumingf@realtek.com>

ASoC: Intel: sof_sdw_rt1308: add BQ params for the Dell models

4753514

The Dell SKU 0A5D/0A5E/0990/098F model needs the BQ params for the tweeter/woofer. Signed-off-by: Shuming Fan <shumingf@realtek.com>

ASoC: Intel: sof_sdw_rt1316: add BQ params for the Dell models

43a60be

The Dell SKU 0B00/0B01/0AFE/0AFF model needs the BQ params for the tweeter/woofer. Signed-off-by: Shuming Fan <shumingf@realtek.com>

shumingfan force-pushed the fix-rt1308-distorted-sound branch from 3762b34 to 43a60be Compare November 3, 2022 06:07

plbossart approved these changes Nov 9, 2022

View reviewed changes

plbossart closed this Nov 9, 2022

shumingfan deleted the fix-rt1308-distorted-sound branch December 13, 2022 05:36

fixup! ASoC: sof_sdw_rt1308/sof_sdw_rt1316/rt1308-sdw/rt1316-sdw: fix distorted sound #3945

fixup! ASoC: sof_sdw_rt1308/sof_sdw_rt1316/rt1308-sdw/rt1316-sdw: fix distorted sound #3945

Uh oh!

Conversation

shumingfan commented Oct 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

plbossart Oct 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shumingfan commented Oct 24, 2022

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart Oct 24, 2022

Choose a reason for hiding this comment

Uh oh!

plbossart Oct 24, 2022

Choose a reason for hiding this comment

Uh oh!

shumingfan Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

shumingfan commented Oct 25, 2022

Uh oh!

shumingfan commented Oct 27, 2022

Uh oh!

plbossart commented Oct 27, 2022

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart Oct 31, 2022

Choose a reason for hiding this comment

Uh oh!

shumingfan Nov 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart Nov 1, 2022

Choose a reason for hiding this comment

Uh oh!

shumingfan Nov 2, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart commented Nov 9, 2022

Uh oh!

shumingfan commented Nov 9, 2022

Uh oh!

JuanFcoMiranda commented Dec 12, 2022

Uh oh!

plbossart commented Dec 12, 2022

Uh oh!

JuanFcoMiranda commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

plbossart commented Dec 13, 2022

Uh oh!

shumingfan commented Oct 21, 2022 •

edited

Loading

shumingfan Nov 1, 2022 •

edited

Loading

JuanFcoMiranda commented Dec 13, 2022 •

edited

Loading