| Message ID | 20230927223758.18870-1-sven.frotscher@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1948fa64727685ac3f6584755212e2e738b6b051 |
| Headers | show |
| Series | [v4] ASoC: amd: yc: Fix non-functional mic on Lenovo 82YM | expand |
[adding Takashi to this, in case he wants to chime in] Sven, first off, thx for taking care of this. On 28.09.23 00:36, Sven Frotscher wrote: > Like the Lenovo 82TL, 82V2, 82QF and 82UG, the 82YM (Yoga 7 14ARP8) > requires an entry in the quirk list to enable the internal microphone. > > The latter two received similar fixes in commit 1263cc0f414d > ("ASoC: amd: yc: Fix non-functional mic on Lenovo 82QF and 82UG"). >> Fixes: c008323fe361 ("ASoC: amd: yc: Fix a non-functional mic on Lenovo 82SJ") FWIW, apparently the Leonovo Yoga Slim 7 Pro 82UU needs an quirk entry as well: https://bugzilla.kernel.org/show_bug.cgi?id=217063#c23 Makes me wonder: How many more such quirk entries will be needed? Will we have all machines listed soon, or do we expect that future Lenovo hardware will need entries as well? If it's the latter: are quirks really the right solution here, or do they just hide some bug or then need for code that automatically handles things? Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. > Cc: stable@vger.kernel.org > Signed-off-by: Sven Frotscher <sven.frotscher@gmail.com> > > [...] > > diff --git a/sound/soc/amd/yc/acp6x-mach.c b/sound/soc/amd/yc/acp6x-mach.c > index 94e9eb8e73f2..15a864dcd7bd 100644 > --- a/sound/soc/amd/yc/acp6x-mach.c > +++ b/sound/soc/amd/yc/acp6x-mach.c > @@ -241,6 +241,13 @@ static const struct dmi_system_id yc_acp_quirk_table[] = { > DMI_MATCH(DMI_PRODUCT_NAME, "82V2"), > } > }, > + { > + .driver_data = &acp6x_card, > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), > + DMI_MATCH(DMI_PRODUCT_NAME, "82YM"), > + } > + }, > { > .driver_data = &acp6x_card, > .matches = {
On Mon, Oct 02, 2023 at 11:32:48AM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > Makes me wonder: How many more such quirk entries will be needed? Will > we have all machines listed soon, or do we expect that future Lenovo > hardware will need entries as well? If it's the latter: are quirks > really the right solution here, or do they just hide some bug or then > need for code that automatically handles things? x86 firmware descriptions are terrible, it's just an endless procession of quirks. The model for ACPI is not to describe key information in the kernel and instead on Windows load device specific information from separately supplied tables. On Linux that translates into these endless quirks, on Windows it's platform specific drivers for otherwise generic audio hardware.
On 02.10.23 13:52, Mark Brown wrote: > On Mon, Oct 02, 2023 at 11:32:48AM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > >> Makes me wonder: How many more such quirk entries will be needed? Will >> we have all machines listed soon, or do we expect that future Lenovo >> hardware will need entries as well? If it's the latter: are quirks >> really the right solution here, or do they just hide some bug or then >> need for code that automatically handles things? > > x86 firmware descriptions are terrible, it's just an endless procession > of quirks. The model for ACPI is not to describe key information in the > kernel and instead on Windows load device specific information from > separately supplied tables. On Linux that translates into these endless > quirks, on Windows it's platform specific drivers for otherwise generic > audio hardware. I know all of that, but from the many recent regression reports and patches it seems quirks were not needed for a bunch of Lenovo machines before c008323fe361bd ("ASoC: amd: yc: Fix a non-functional mic on Lenovo 82SJ") [v6.5]. That made me wonder if that commit really did the right thing or if there is a underlying bug somewhere that the newly added quirks hide, as I had a few such situations during the past few months. If you or others the experts in this area say that this is not the case here then I'm totally fine with that, it was just a question. Ciao, Thorsten
On Mon, Oct 02, 2023 at 02:28:47PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > On 02.10.23 13:52, Mark Brown wrote: > > x86 firmware descriptions are terrible, it's just an endless procession > > of quirks. The model for ACPI is not to describe key information in the > > kernel and instead on Windows load device specific information from > > separately supplied tables. On Linux that translates into these endless > > quirks, on Windows it's platform specific drivers for otherwise generic > > audio hardware. > I know all of that, but from the many recent regression reports and > patches it seems quirks were not needed for a bunch of Lenovo machines > before c008323fe361bd ("ASoC: amd: yc: Fix a non-functional mic on > Lenovo 82SJ") [v6.5]. That made me wonder if that commit really did the > right thing or if there is a underlying bug somewhere that the newly > added quirks hide, as I had a few such situations during the past few > months. If you or others the experts in this area say that this is not > the case here then I'm totally fine with that, it was just a question. Until someone tests or otherwise provides specific information on a given machine we're just guessing about how it's wired up.
Am Mo, 2. Okt 2023 um 13:54:59 +01:00:00 schrieb Mark Brown <broonie@kernel.org>: > Until someone tests or otherwise provides specific information on a > given machine we're just guessing about how it's wired up. What specific information are we talking about here? I have the 82YM in front of me and could help investigate.
On Mon, Oct 02, 2023 at 03:00:42PM +0200, Sven Frotscher wrote: > Am Mo, 2. Okt 2023 um 13:54:59 +01:00:00 schrieb Mark Brown > > Until someone tests or otherwise provides specific information on a > > given machine we're just guessing about how it's wired up. > What specific information are we talking about here? I have the 82YM in > front of me and could help investigate. We need to know what magic set of quirks makes the thing work. Are you saying that your patch doesn't actually do that?
>> > Until someone tests or otherwise provides specific information on >> a >> > given machine we're just guessing about how it's wired up. > >> What specific information are we talking about here? I have the >> 82YM in >> front of me and could help investigate. > > We need to know what magic set of quirks makes the thing work. Are > you > saying that your patch doesn't actually do that? It does. Maybe the non-quirk check (ll. 395-403, seems to be using ACPI) is too specific? But I'm a bit out of my depth here, so I can't investigate that by myself.
On 10/2/2023 06:52, Mark Brown wrote: > On Mon, Oct 02, 2023 at 11:32:48AM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > >> Makes me wonder: How many more such quirk entries will be needed? Will >> we have all machines listed soon, or do we expect that future Lenovo >> hardware will need entries as well? If it's the latter: are quirks >> really the right solution here, or do they just hide some bug or then >> need for code that automatically handles things? > > x86 firmware descriptions are terrible, it's just an endless procession > of quirks. The model for ACPI is not to describe key information in the > kernel and instead on Windows load device specific information from > separately supplied tables. On Linux that translates into these endless > quirks, on Windows it's platform specific drivers for otherwise generic > audio hardware. I knew there was a TON of "82" prefix systems from Lenovo so it was an educated guess that all of them needed DMIC support. This was incorrect because one of them didn't have DMIC and that caused a no mic support problem on that system. So in the case of this seemingly endless list of systems being added to enable DMIC support Mark is right, Windows does it differently. With the "next" generation of hardware (Phoenix) both Windows and Linux *should* be using the same _DSD, so hopefully we don't need more quirks like this for those.
On Mon, Oct 02, 2023 at 03:17:33PM +0200, Sven Frotscher wrote: > > We need to know what magic set of quirks makes the thing work. Are you > > saying that your patch doesn't actually do that? > It does. > Maybe the non-quirk check (ll. 395-403, seems to be using ACPI) is too > specific? But I'm a bit out of my depth here, so I can't investigate that by > myself. Like I say it's all just guesses without someone providing information.
On 02.10.23 15:47, Mario Limonciello wrote: > On 10/2/2023 06:52, Mark Brown wrote: >> On Mon, Oct 02, 2023 at 11:32:48AM +0200, Linux regression tracking >> (Thorsten Leemhuis) wrote: >> >>> Makes me wonder: How many more such quirk entries will be needed? Will >>> we have all machines listed soon, or do we expect that future Lenovo >>> hardware will need entries as well? If it's the latter: are quirks >>> really the right solution here, or do they just hide some bug or then >>> need for code that automatically handles things? >> >> x86 firmware descriptions are terrible, it's just an endless procession >> of quirks. The model for ACPI is not to describe key information in the >> kernel and instead on Windows load device specific information from >> separately supplied tables. On Linux that translates into these endless >> quirks, on Windows it's platform specific drivers for otherwise generic >> audio hardware. > > I knew there was a TON of "82" prefix systems from Lenovo so it was an > educated guess that all of them needed DMIC support. This was incorrect > because one of them didn't have DMIC and that caused a no mic support > problem on that system. > > So in the case of this seemingly endless list of systems being added to > enable DMIC support Mark is right, Windows does it differently. Now I understand things better, many thx. But please allow me one more question from the cheap seats: Seems before c008323fe361 things worked for a lot of systems for about one year thx to 2232b2dd8cd4 (which added the wide "82" prefix quirk). We then made that one machine work with c008323fe361, but broke a lot of others with it that now need to be fixed with additional quirks; that "TON of 82 prefix systems" sounds like we might not be close to the end of that journey. So can't we just do it the other way around and assume DMIC support on Lenovo 82* machines, except on those where we know it to cause trouble? Again: you are the experts here. If you are positive that we soon got all machines covered where c008323fe361 causes a regression, then I guess it's best to continue the patch we're on. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page.
On 10/2/2023 09:13, Linux regression tracking (Thorsten Leemhuis) wrote: > On 02.10.23 15:47, Mario Limonciello wrote: >> On 10/2/2023 06:52, Mark Brown wrote: >>> On Mon, Oct 02, 2023 at 11:32:48AM +0200, Linux regression tracking >>> (Thorsten Leemhuis) wrote: >>> >>>> Makes me wonder: How many more such quirk entries will be needed? Will >>>> we have all machines listed soon, or do we expect that future Lenovo >>>> hardware will need entries as well? If it's the latter: are quirks >>>> really the right solution here, or do they just hide some bug or then >>>> need for code that automatically handles things? >>> >>> x86 firmware descriptions are terrible, it's just an endless procession >>> of quirks. The model for ACPI is not to describe key information in the >>> kernel and instead on Windows load device specific information from >>> separately supplied tables. On Linux that translates into these endless >>> quirks, on Windows it's platform specific drivers for otherwise generic >>> audio hardware. >> >> I knew there was a TON of "82" prefix systems from Lenovo so it was an >> educated guess that all of them needed DMIC support. This was incorrect >> because one of them didn't have DMIC and that caused a no mic support >> problem on that system. >> >> So in the case of this seemingly endless list of systems being added to >> enable DMIC support Mark is right, Windows does it differently. > > Now I understand things better, many thx. But please allow me one more > question from the cheap seats: > > Seems before c008323fe361 things worked for a lot of systems for about > one year thx to 2232b2dd8cd4 (which added the wide "82" prefix quirk). > We then made that one machine work with c008323fe361, but broke a lot of > others with it that now need to be fixed with additional quirks; that > "TON of 82 prefix systems" sounds like we might not be close to the end > of that journey. > > So can't we just do it the other way around and assume DMIC support on > Lenovo 82* machines, except on those where we know it to cause trouble? > > Again: you are the experts here. If you are positive that we soon got > all machines covered where c008323fe361 causes a regression, then I > guess it's best to continue the patch we're on. I don't like lists that enable something for a ton of systems and then lists that disable something for a subset of them. This becomes difficult to maintain. I'm not positive, but the only way we get a full list is from Lenovo. > > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > If I did something stupid, please tell me, as explained on that page.
On 02.10.23 16:20, Mario Limonciello wrote: > On 10/2/2023 09:13, Linux regression tracking (Thorsten Leemhuis) wrote: >> On 02.10.23 15:47, Mario Limonciello wrote: >>> On 10/2/2023 06:52, Mark Brown wrote: >>>> On Mon, Oct 02, 2023 at 11:32:48AM +0200, Linux regression tracking >>>> (Thorsten Leemhuis) wrote: >>>> >>>>> Makes me wonder: How many more such quirk entries will be needed? Will >>>>> we have all machines listed soon, or do we expect that future Lenovo >>>>> hardware will need entries as well? If it's the latter: are quirks >>>>> really the right solution here, or do they just hide some bug or then >>>>> need for code that automatically handles things? >>>> >>>> x86 firmware descriptions are terrible, it's just an endless procession >>>> of quirks. The model for ACPI is not to describe key information in >>>> the >>>> kernel and instead on Windows load device specific information from >>>> separately supplied tables. On Linux that translates into these >>>> endless >>>> quirks, on Windows it's platform specific drivers for otherwise generic >>>> audio hardware. >>> >>> I knew there was a TON of "82" prefix systems from Lenovo so it was an >>> educated guess that all of them needed DMIC support. This was incorrect >>> because one of them didn't have DMIC and that caused a no mic support >>> problem on that system. >>> >>> So in the case of this seemingly endless list of systems being added to >>> enable DMIC support Mark is right, Windows does it differently. >> >> Now I understand things better, many thx. But please allow me one more >> question from the cheap seats: >> >> Seems before c008323fe361 things worked for a lot of systems for about >> one year thx to 2232b2dd8cd4 (which added the wide "82" prefix quirk). >> We then made that one machine work with c008323fe361, but broke a lot of >> others with it that now need to be fixed with additional quirks; that >> "TON of 82 prefix systems" sounds like we might not be close to the end >> of that journey. >> >> So can't we just do it the other way around and assume DMIC support on >> Lenovo 82* machines, except on those where we know it to cause trouble? >> >> Again: you are the experts here. If you are positive that we soon got >> all machines covered where c008323fe361 causes a regression, then I >> guess it's best to continue the patch we're on. > > I don't like lists And I don't like if we let people run into regressions knowingly. ;) > that enable something for a ton of systems and then > lists that disable something for a subset of them. This becomes > difficult to maintain. Well, I had more thought along the lines of "do enable DMIC on Lenovo 82*, unless the following dmi (the one from c008323fe361) matches". But I assume that's not easy to realize with the quirks table, so I guess that is out. Whatever. Well, I rest my case. But I guess I might come back to this if multiple additional regressions reports come it due to c008323fe361. Ciao, Thorsten
Am Mo, 2. Okt 2023 um 09:20:04 -05:00:00 schrieb Mario Limonciello
<mario.limonciello@amd.com>:
> I'm not positive, but the only way we get a full list is from Lenovo.
In order to be totally certain, yes.
But maybe the non-quirk check can be expanded to also cover (some of)
the affected Lenovo models. As I said, I'm open to provide some
relevant info from mine if someone tells me what "relevant info" means
in this case.
On Thu, 28 Sep 2023 00:36:07 +0200, Sven Frotscher wrote: > Like the Lenovo 82TL, 82V2, 82QF and 82UG, the 82YM (Yoga 7 14ARP8) > requires an entry in the quirk list to enable the internal microphone. > The latter two received similar fixes in commit 1263cc0f414d > ("ASoC: amd: yc: Fix non-functional mic on Lenovo 82QF and 82UG"). > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: amd: yc: Fix non-functional mic on Lenovo 82YM commit: 1948fa64727685ac3f6584755212e2e738b6b051 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
diff --git a/sound/soc/amd/yc/acp6x-mach.c b/sound/soc/amd/yc/acp6x-mach.c index 94e9eb8e73f2..15a864dcd7bd 100644 --- a/sound/soc/amd/yc/acp6x-mach.c +++ b/sound/soc/amd/yc/acp6x-mach.c @@ -241,6 +241,13 @@ static const struct dmi_system_id yc_acp_quirk_table[] = { DMI_MATCH(DMI_PRODUCT_NAME, "82V2"), } }, + { + .driver_data = &acp6x_card, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_NAME, "82YM"), + } + }, { .driver_data = &acp6x_card, .matches = {
Like the Lenovo 82TL, 82V2, 82QF and 82UG, the 82YM (Yoga 7 14ARP8) requires an entry in the quirk list to enable the internal microphone. The latter two received similar fixes in commit 1263cc0f414d ("ASoC: amd: yc: Fix non-functional mic on Lenovo 82QF and 82UG"). Fixes: c008323fe361 ("ASoC: amd: yc: Fix a non-functional mic on Lenovo 82SJ") Cc: stable@vger.kernel.org Signed-off-by: Sven Frotscher <sven.frotscher@gmail.com> --- v3->v4 changes: * re-add blank line between commit message title and body --- v2->v3 changes: * add message title of referenced commit to commit message * make whitespace consistent with surrounding code * use a patch-friendly e-mail client --- v1->v2 changes: * add Fixes and Cc tags to commit message * remove redundant LKML link from commit message * fix mangled diff --- sound/soc/amd/yc/acp6x-mach.c | 7 +++++++ 1 file changed, 7 insertions(+)