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, 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),
-- 
2.46.2

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