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

[Takashi Iwai <tiwai@xxxxxxx>]

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@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().

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