Re: [PATCH] ALSA: hda/realtek: fixup ASUS GA605W

[Richard Fitzgerald]

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@xxxxxxxxxx>
> > > > > ---
> > > > >   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.