| Message ID | 20241009205800.40570-1-luke@ljones.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ALSA: hda/realtek: fixup ASUS GA605W | expand |
On Wed, Oct 09, 2024, Luke D. Jones wrote: > The GA605W laptop has almost the exact same codec setup as the GA403 > and so the same quirks apply to it. > > Signed-off-by: Luke D. Jones <luke@ljones.dev> > --- > sound/pci/hda/patch_realtek.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c > index 41e1119877b3..19b0bae074b6 100644 > --- a/sound/pci/hda/patch_realtek.c > +++ b/sound/pci/hda/patch_realtek.c > @@ -10507,7 +10507,7 @@ static const struct snd_pci_quirk > alc269_fixup_tbl[] = { ... > SND_PCI_QUIRK(0x1043, 0x1e63, "ASUS H7606W", ALC285_FIXUP_CS35L56_I2C_2), > - SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", ALC285_FIXUP_CS35L56_I2C_2), > + SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", ALC285_FIXUP_ASUS_GA403U_HEADSET_MIC), ... Hi Luke, Thanks! The support code for the CS35L56 has changed a little in Takashi's branches from what that patch was generated against and there is no longer an existing quirk in the fixup table to trigger the component binding (but the general idea seems reasonable to hook the fixup of the headset mic). Cheers, -Simon
On Thu, 10 Oct 2024 11:53:49 +0200, Simon Trimmer wrote: > > On Wed, Oct 09, 2024, Luke D. Jones wrote: > > The GA605W laptop has almost the exact same codec setup as the GA403 > > and so the same quirks apply to it. > > > > Signed-off-by: Luke D. Jones <luke@ljones.dev> > > --- > > sound/pci/hda/patch_realtek.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c > > index 41e1119877b3..19b0bae074b6 100644 > > --- a/sound/pci/hda/patch_realtek.c > > +++ b/sound/pci/hda/patch_realtek.c > > @@ -10507,7 +10507,7 @@ static const struct snd_pci_quirk > > alc269_fixup_tbl[] = { > ... > > SND_PCI_QUIRK(0x1043, 0x1e63, "ASUS H7606W", > ALC285_FIXUP_CS35L56_I2C_2), > > - SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", > ALC285_FIXUP_CS35L56_I2C_2), > > + SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", > ALC285_FIXUP_ASUS_GA403U_HEADSET_MIC), > ... > > Hi Luke, > Thanks! > > The support code for the CS35L56 has changed a little in Takashi's branches > from what that patch was generated against and there is no longer an > existing quirk in the fixup table to trigger the component binding (but the > general idea seems reasonable to hook the fixup of the headset mic). Right, the patch doesn't apply to the latest tree for 6.12-rc. We should start with setting up only the pincfg. The binding with Cirrus is handled via find_cirrus_companion_amps(). The shared DAC of speaker and bass speaker is an open question, whether we still need the workaround by alc285_fixup_speaker2_to_dac1(). Let's see. BTW, rather a question to Cirrus devs: may the call of find_cirrus_companion_amps() conflict with the existing manual setup of the Cirrus codec by a quirk entry? thanks, Takashi
On Thu, 10 Oct 2024 13:15, Takashi Iwai wrote: > On Thu, 10 Oct 2024 11:53:49 +0200, Simon Trimmer wrote: > > On Wed, Oct 09, 2024, Luke D. Jones wrote: > > > The GA605W laptop has almost the exact same codec setup as the GA403 > > > and so the same quirks apply to it. > > > > > > Signed-off-by: Luke D. Jones <luke@ljones.dev> > > > --- > > > sound/pci/hda/patch_realtek.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/sound/pci/hda/patch_realtek.c > b/sound/pci/hda/patch_realtek.c > > > index 41e1119877b3..19b0bae074b6 100644 > > > --- a/sound/pci/hda/patch_realtek.c > > > +++ b/sound/pci/hda/patch_realtek.c > > > @@ -10507,7 +10507,7 @@ static const struct snd_pci_quirk > > > alc269_fixup_tbl[] = { > > ... > > > SND_PCI_QUIRK(0x1043, 0x1e63, "ASUS H7606W", > > ALC285_FIXUP_CS35L56_I2C_2), > > > - SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", > > ALC285_FIXUP_CS35L56_I2C_2), > > > + SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", > > ALC285_FIXUP_ASUS_GA403U_HEADSET_MIC), > > ... > > > > Hi Luke, > > Thanks! > > > > The support code for the CS35L56 has changed a little in Takashi's branches > > from what that patch was generated against and there is no longer an > > existing quirk in the fixup table to trigger the component binding (but the > > general idea seems reasonable to hook the fixup of the headset mic). > > Right, the patch doesn't apply to the latest tree for 6.12-rc. > > We should start with setting up only the pincfg. The binding with > Cirrus is handled via find_cirrus_companion_amps(). The shared DAC > of speaker and bass speaker is an open question, whether we still need > the workaround by alc285_fixup_speaker2_to_dac1(). Let's see. > > BTW, rather a question to Cirrus devs: may the call of > find_cirrus_companion_amps() conflict with the existing manual setup > of the Cirrus codec by a quirk entry? I think we're good (I rarely get to access to the end products so try to stick to the parts of the system that I have the details for which never includes the Realtek codec...) in this general case I'd just expect that the quirk entries wouldn't have the element at the end of the chain that used to do the amp component binding fixup and the remaining tweaks for things like the Realtek routing and mute indicators would be unaffected. Cheers, -Simon
On Thu, 10 Oct 2024 15:20:25 +0200, Simon Trimmer wrote: > > On Thu, 10 Oct 2024 13:15, Takashi Iwai wrote: > > On Thu, 10 Oct 2024 11:53:49 +0200, Simon Trimmer wrote: > > > On Wed, Oct 09, 2024, Luke D. Jones wrote: > > > > The GA605W laptop has almost the exact same codec setup as the GA403 > > > > and so the same quirks apply to it. > > > > > > > > Signed-off-by: Luke D. Jones <luke@ljones.dev> > > > > --- > > > > sound/pci/hda/patch_realtek.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/sound/pci/hda/patch_realtek.c > > b/sound/pci/hda/patch_realtek.c > > > > index 41e1119877b3..19b0bae074b6 100644 > > > > --- a/sound/pci/hda/patch_realtek.c > > > > +++ b/sound/pci/hda/patch_realtek.c > > > > @@ -10507,7 +10507,7 @@ static const struct snd_pci_quirk > > > > alc269_fixup_tbl[] = { > > > ... > > > > SND_PCI_QUIRK(0x1043, 0x1e63, "ASUS H7606W", > > > ALC285_FIXUP_CS35L56_I2C_2), > > > > - SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", > > > ALC285_FIXUP_CS35L56_I2C_2), > > > > + SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", > > > ALC285_FIXUP_ASUS_GA403U_HEADSET_MIC), > > > ... > > > > > > Hi Luke, > > > Thanks! > > > > > > The support code for the CS35L56 has changed a little in Takashi's > branches > > > from what that patch was generated against and there is no longer an > > > existing quirk in the fixup table to trigger the component binding (but > the > > > general idea seems reasonable to hook the fixup of the headset mic). > > > > Right, the patch doesn't apply to the latest tree for 6.12-rc. > > > > We should start with setting up only the pincfg. The binding with > > Cirrus is handled via find_cirrus_companion_amps(). The shared DAC > > of speaker and bass speaker is an open question, whether we still need > > the workaround by alc285_fixup_speaker2_to_dac1(). Let's see. > > > > BTW, rather a question to Cirrus devs: may the call of > > find_cirrus_companion_amps() conflict with the existing manual setup > > of the Cirrus codec by a quirk entry? > > I think we're good (I rarely get to access to the end products so try to > stick to the parts of the system that I have the details for which never > includes the Realtek codec...) in this general case I'd just expect that the > quirk entries wouldn't have the element at the end of the chain that used to > do the amp component binding fixup and the remaining tweaks for things like > the Realtek routing and mute indicators would be unaffected. Well, there are quirks that do call comp_generic_fixup() with fixed arguments, and I'm afraid they'll lead to some spurious error messages. Namely, find_cirrus_companion_amps() is called before snd_hda_apply_fixup(PRE_PROBE), so comp_generic_fixup() might be already invoked there, and the quirk entry invokes comp_generic_fixup() again. This will end up with an error message by the parent->codec check in hda_component_manager_init(). It's just a message and nothing harmful happening, though. Takashi
On 10/10/2024 3:37 pm, Takashi Iwai wrote: > On Thu, 10 Oct 2024 15:20:25 +0200, > Simon Trimmer wrote: >> >> On Thu, 10 Oct 2024 13:15, Takashi Iwai wrote: >>> On Thu, 10 Oct 2024 11:53:49 +0200, Simon Trimmer wrote: >>>> On Wed, Oct 09, 2024, Luke D. Jones wrote: >>>>> The GA605W laptop has almost the exact same codec setup as the GA403 >>>>> and so the same quirks apply to it. >>>>> >>>>> Signed-off-by: Luke D. Jones <luke@ljones.dev> >>>>> --- >>>>> sound/pci/hda/patch_realtek.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/sound/pci/hda/patch_realtek.c >>> b/sound/pci/hda/patch_realtek.c >>>>> index 41e1119877b3..19b0bae074b6 100644 >>>>> --- a/sound/pci/hda/patch_realtek.c >>>>> +++ b/sound/pci/hda/patch_realtek.c >>>>> @@ -10507,7 +10507,7 @@ static const struct snd_pci_quirk >>>>> alc269_fixup_tbl[] = { >>>> ... >>>>> SND_PCI_QUIRK(0x1043, 0x1e63, "ASUS H7606W", >>>> ALC285_FIXUP_CS35L56_I2C_2), >>>>> - SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", >>>> ALC285_FIXUP_CS35L56_I2C_2), >>>>> + SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", >>>> ALC285_FIXUP_ASUS_GA403U_HEADSET_MIC), >>>> ... >>>> >>>> Hi Luke, >>>> Thanks! >>>> >>>> The support code for the CS35L56 has changed a little in Takashi's >> branches >>>> from what that patch was generated against and there is no longer an >>>> existing quirk in the fixup table to trigger the component binding (but >> the >>>> general idea seems reasonable to hook the fixup of the headset mic). >>> >>> Right, the patch doesn't apply to the latest tree for 6.12-rc. >>> >>> We should start with setting up only the pincfg. The binding with >>> Cirrus is handled via find_cirrus_companion_amps(). The shared DAC >>> of speaker and bass speaker is an open question, whether we still need >>> the workaround by alc285_fixup_speaker2_to_dac1(). Let's see. >>> >>> BTW, rather a question to Cirrus devs: may the call of >>> find_cirrus_companion_amps() conflict with the existing manual setup >>> of the Cirrus codec by a quirk entry? >> >> I think we're good (I rarely get to access to the end products so try to >> stick to the parts of the system that I have the details for which never >> includes the Realtek codec...) in this general case I'd just expect that the >> quirk entries wouldn't have the element at the end of the chain that used to >> do the amp component binding fixup and the remaining tweaks for things like >> the Realtek routing and mute indicators would be unaffected. > > Well, there are quirks that do call comp_generic_fixup() with fixed > arguments, and I'm afraid they'll lead to some spurious error > messages. Namely, find_cirrus_companion_amps() is called before > snd_hda_apply_fixup(PRE_PROBE), so comp_generic_fixup() might be > already invoked there, and the quirk entry invokes > comp_generic_fixup() again. This will end up with an error message by > the parent->codec check in hda_component_manager_init(). > It's just a message and nothing harmful happening, though. > find_cirrus_companion_amps() currently only handles CS35L54/56/57, and when that patch was sent it also converted all quirks using those devices so they _don't_ call comp_generic_fixup(). The purpose of the warning was to detect people adding a quirk that tried to do the old way of manual component binding setup from the quirk. The remaining quirks that call comp_generic_fixup() are for CS35L41/51, which we haven't converted yet because there are many more models, some with strange ACPI and some that are old models we can't easily get hold of to re-test.
On Thu, 10 Oct 2024 17:07:24 +0200, Richard Fitzgerald wrote: > > On 10/10/2024 3:37 pm, Takashi Iwai wrote: > > On Thu, 10 Oct 2024 15:20:25 +0200, > > Simon Trimmer wrote: > >> > >> On Thu, 10 Oct 2024 13:15, Takashi Iwai wrote: > >>> On Thu, 10 Oct 2024 11:53:49 +0200, Simon Trimmer wrote: > >>>> On Wed, Oct 09, 2024, Luke D. Jones wrote: > >>>>> The GA605W laptop has almost the exact same codec setup as the GA403 > >>>>> and so the same quirks apply to it. > >>>>> > >>>>> Signed-off-by: Luke D. Jones <luke@ljones.dev> > >>>>> --- > >>>>> sound/pci/hda/patch_realtek.c | 2 +- > >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/sound/pci/hda/patch_realtek.c > >>> b/sound/pci/hda/patch_realtek.c > >>>>> index 41e1119877b3..19b0bae074b6 100644 > >>>>> --- a/sound/pci/hda/patch_realtek.c > >>>>> +++ b/sound/pci/hda/patch_realtek.c > >>>>> @@ -10507,7 +10507,7 @@ static const struct snd_pci_quirk > >>>>> alc269_fixup_tbl[] = { > >>>> ... > >>>>> SND_PCI_QUIRK(0x1043, 0x1e63, "ASUS H7606W", > >>>> ALC285_FIXUP_CS35L56_I2C_2), > >>>>> - SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", > >>>> ALC285_FIXUP_CS35L56_I2C_2), > >>>>> + SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", > >>>> ALC285_FIXUP_ASUS_GA403U_HEADSET_MIC), > >>>> ... > >>>> > >>>> Hi Luke, > >>>> Thanks! > >>>> > >>>> The support code for the CS35L56 has changed a little in Takashi's > >> branches > >>>> from what that patch was generated against and there is no longer an > >>>> existing quirk in the fixup table to trigger the component binding (but > >> the > >>>> general idea seems reasonable to hook the fixup of the headset mic). > >>> > >>> Right, the patch doesn't apply to the latest tree for 6.12-rc. > >>> > >>> We should start with setting up only the pincfg. The binding with > >>> Cirrus is handled via find_cirrus_companion_amps(). The shared DAC > >>> of speaker and bass speaker is an open question, whether we still need > >>> the workaround by alc285_fixup_speaker2_to_dac1(). Let's see. > >>> > >>> BTW, rather a question to Cirrus devs: may the call of > >>> find_cirrus_companion_amps() conflict with the existing manual setup > >>> of the Cirrus codec by a quirk entry? > >> > >> I think we're good (I rarely get to access to the end products so try to > >> stick to the parts of the system that I have the details for which never > >> includes the Realtek codec...) in this general case I'd just expect that the > >> quirk entries wouldn't have the element at the end of the chain that used to > >> do the amp component binding fixup and the remaining tweaks for things like > >> the Realtek routing and mute indicators would be unaffected. > > > > Well, there are quirks that do call comp_generic_fixup() with fixed > > arguments, and I'm afraid they'll lead to some spurious error > > messages. Namely, find_cirrus_companion_amps() is called before > > snd_hda_apply_fixup(PRE_PROBE), so comp_generic_fixup() might be > > already invoked there, and the quirk entry invokes > > comp_generic_fixup() again. This will end up with an error message by > > the parent->codec check in hda_component_manager_init(). > > It's just a message and nothing harmful happening, though. > > > find_cirrus_companion_amps() currently only handles CS35L54/56/57, and > when that patch was sent it also converted all quirks using those > devices so they _don't_ call comp_generic_fixup(). Ah thanks, I completely overlooked that! > The purpose of the > warning was to detect people adding a quirk that tried to do the old > way of manual component binding setup from the quirk. > > The remaining quirks that call comp_generic_fixup() are for CS35L41/51, > which we haven't converted yet because there are many more models, some > with strange ACPI and some that are old models we can't easily get hold > of to re-test. OK, so we are fine :) And, even if the conflict happens, it's an error message and it indicates that we can get rid of that quirk. Takashi
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 41e1119877b3..19b0bae074b6 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -10507,7 +10507,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x1043, 0x1e51, "ASUS Zephyrus M15", ALC294_FIXUP_ASUS_GU502_PINS), SND_PCI_QUIRK(0x1043, 0x1e5e, "ASUS ROG Strix G513", ALC294_FIXUP_ASUS_G513_PINS), SND_PCI_QUIRK(0x1043, 0x1e63, "ASUS H7606W", ALC285_FIXUP_CS35L56_I2C_2), - SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", ALC285_FIXUP_CS35L56_I2C_2), + SND_PCI_QUIRK(0x1043, 0x1e83, "ASUS GA605W", ALC285_FIXUP_ASUS_GA403U_HEADSET_MIC), SND_PCI_QUIRK(0x1043, 0x1e8e, "ASUS Zephyrus G15", ALC289_FIXUP_ASUS_GA401), SND_PCI_QUIRK(0x1043, 0x1ed3, "ASUS HN7306W", ALC287_FIXUP_CS35L41_I2C_2), SND_PCI_QUIRK(0x1043, 0x1ee2, "ASUS UM6702RA/RC", ALC287_FIXUP_CS35L41_I2C_2),
The GA605W laptop has almost the exact same codec setup as the GA403 and so the same quirks apply to it. Signed-off-by: Luke D. Jones <luke@ljones.dev> --- sound/pci/hda/patch_realtek.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)