linux-kernel - Re: [REGRESSION][BISECTED] Audio volume issues since 4178d78cd7a8

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8734lcmd01.wl-tiwai@suse.de>
Date: Fri, 04 Oct 2024 11:30:22 +0200
From: Takashi Iwai <tiwai@...e.de>
To: Christoffer Sandberg <cs@...edo.de>
Cc: Takashi Iwai <tiwai@...e.de>,
	Jerry Luo <jerryluo225@...il.com>,
	christian@...sel.eu,
	linux-kernel@...r.kernel.org,
	linux-sound@...r.kernel.org,
	perex@...ex.cz,
	regressions@...ts.linux.dev,
	wse@...edocomputers.com
Subject: Re: [REGRESSION][BISECTED] Audio volume issues since 4178d78cd7a8

On Fri, 04 Oct 2024 11:25:32 +0200,
Christoffer Sandberg wrote:
> 
> 
> 
> On 4.10.2024 10:25, Takashi Iwai wrote:
> > On Fri, 04 Oct 2024 10:18:10 +0200,
> > Christoffer Sandberg wrote:
> >> 
> >> 
> >> 
> >> On 2.10.2024 23:28, Jerry Luo wrote:
> >> > On 10/2/24 10:00 AM, Takashi Iwai wrote:
> >> >> On Wed, 02 Oct 2024 10:21:22 +0200,
> >> >> Christoffer Sandberg wrote:
> >> >>>
> >> >>>
> >> >>> On 30.9.2024 09:44, Takashi Iwai wrote:
> >> >>>> On Mon, 23 Sep 2024 21:37:42 +0200,
> >> >>>> Jerry Luo wrote:
> >> >>>>>
> >> >>>>> Hi Takashi,
> >> >>>>>
> >> >>>>> On Mon, 16 Sep 2024 19:22:05 +0200,
> >> >>>>>
> >> >>>>> Takashi Iwai wrote:
> >> >>>>>
> >> >>>>>      Could you give alsa-info.sh output from both working and
> >> >>>>> non-working
> >> >>>>>      cases?  Run the script with --no-upload option and attach the
> >> >>>>> outputs.
> >> >>>>>
> >> >>>>>      thanks,
> >> >>>>>
> >> >>>>>      Takashi
> >> >>>>>
> >> >>>>> Issue now reappear, output from alsa-info.sh are attached. If they
> >> >>>>> are still
> >> >>>>> needed.
> >> >>>> Thanks.  The obvious difference seems to be the assignment of two
> >> >>>> DACs
> >> >>>> 0x10 and 0x11 for headphone and speaker outputs.
> >> >>>>
> >> >>>> Christoffer, how are those on your machines?
> >> >>> I attached alsa-info from the Sirius Gen2 device.
> >> >>>
> >> >>> Comparing the working/nonworking of Jerry, yeah, the assignment of
> >> >>> 0x10 and 0x11 looks switched around. I don't see what difference this
> >> >>> would make. Also, node 0x22 has "bass speaker" controls in the
> >> >>> non-working version.
> >> >>>
> >> >>> Comparing the Sirius Gen2 alsa-info with Jerrys, to me it looks like
> >> >>> the non-working version corresponds to our working version.
> >> >>>
> >> >>> I would expect the non-working version to happen all the time though
> >> >>> with regards to the "bass speaker" controls. Why would this only
> >> >>> happen sometimes?
> >> >> Thanks!  The assignment of DACs depend on the pins and topology, so it
> >> >> can be a bit sensitive.
> >> >>
> >> >> Now looking more closely at both outputs, I wonder how the commit
> >> >> breaks pang14.  Maybe it has a PCI SSID 2782:12c5 (or 12c3) while the
> >> >> codec SSID is 2782:12b3?  If so, the patch below should fix.
> >> 
> >> Interesting, you're right, PCI SSID c3/c5 and codec SSID c3/c5 for the
> >> Siriuses.
> >> 
> >> I had a look around. In patch_realtek there are some cases where codec
> >> SSID match is needed as well. Would it be better/safer to directly do
> >> this immediately or keep it as an exception where it breaks or for
> >> known sensitive models/brands?
> > 
> > It needs a better handling, yes.  OTOH, the driver became gigantic and
> > it's very tough to change the basic matching stuff.  That is, we can't
> > flip from PCI SSID to codec SSID out of sudden, as it'll break
> > certainly many other systems.
> > 
> > What I have in mind is to add an extra flag to the matching table to
> > indicate the codec SSID matching, something like:
> > 
> > --- a/sound/pci/hda/hda_local.h
> > +++ b/sound/pci/hda/hda_local.h
> > @@ -282,6 +282,7 @@ struct hda_fixup {
> >  	int type;
> >  	bool chained:1;		/* call the chained fixup(s) after this */
> >  	bool chained_before:1;	/* call the chained fixup(s) before this */
> > +	bool match_codec_ssid:1; /* match with codec SSID instead of
> > PCI SSID */
> >  	int chain_id;
> >  	union {
> >  		const struct hda_pintbl *pins;
> > 
> 
> Sounds reasonable to me. It would mean either-or then though.

Ah yes, it matches primarily with codec SSID instead of fallback.
I guess if this flag is set, we shouldn't check PCI SSID, though.

> > Although this will help in this case, many of existing code do check
> > codec ID in addition to PCI SSID, and this flag won't help for them as
> > is.
> 
> For the fixups where codec SSID is already known for all cases it
> would be possible to represent current logic with matching both SSIDs.
> 
> Otherwise I can not judge whether matching both PCI SSID and codec
> SSID at the same time would be needed.
> 
> In any case, your approach would reduce code size if this is a
> recurring thing.

OK, I'll try to prepare a cleanup patch later.


thanks,

Takashi