Linux Sound subsystem development
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Damien <damien.dagorn29@gmail.com>
Cc: perex@perex.cz, linux-sound@vger.kernel.org, alsa-devel@alsa-project.org
Subject: Re: [PATCH 0/1] ALSA: hda/realtek: fix LG Gram Style 14 speakers
Date: Fri, 23 Jan 2026 17:35:27 +0100	[thread overview]
Message-ID: <871pjge180.wl-tiwai@suse.de> (raw)
In-Reply-To: <CAN59QMUSiJjGrpVzW2czHqdbVUf4yshxUFdKQPws2Fe8veSsdQ@mail.gmail.com>

On Wed, 21 Jan 2026 14:12:09 +0100,
Damien wrote:
> 
> 
>   Hi,
> 
>   The LG Gram Style 14 (14Z90RS-G.AD77F, SSID 1854:0490) with Realtek ALC298
>   shows normal routing and volume changes, but internal speakers remain silent
>   unless a userland HDA-verb workaround is applied.
> 
>   This patch adds a dedicated quirk for the LG Gram Style 14 that programs
>   the codec coefficient sequence used by the known workaround and enables
>   the speaker amps only during playback.
> 
>   Tested-by: Dams <damien.dagorn29@gmail.com>
>   Tested: speaker-test -D hw:0 -c 2 -t wav -l 1 (front left/right OK)
> 
>   Please find the patch series attached.
> 
>   Thanks,
>   Damien

Thanks for the patch.  Unfortunately, the patch looks created somehow
in a not ideal way.  First of all, the useful information like:
> 
> From 113b13ace8671a18c8f3820b007d5757df43dd36 Mon Sep 17 00:00:00 2001
> From: Dams <damien.dagorn29@gmail.com>
> Date: Wed, 21 Jan 2026 14:06:38 +0100
> Subject: [PATCH 0/1] ALSA: hda/realtek: fix LG Gram Style 14 speakers
> 
> The LG Gram Style 14 (14Z90RS-G.AD77F, SSID 1854:0490) with Realtek ALC298
> shows normal routing and volume changes, but internal speakers stay silent
> unless a userland HDA-verb workaround is applied.
> 
> This patch adds a dedicated quirk for the LG Gram Style 14 that programs
> the codec coefficient sequence used by the known workaround and enables
> the speaker amps only during playback.
> 
> Tested-by: Dams <damien.dagorn29@gmail.com>
> Tested: speaker-test -D hw:0 -c 2 -t wav -l 1 (front left/right OK)

... those should be put rather in the description of the patch itself.
You've put it in 0/1, that is a cover letter, which won't be included
in the git commit.
(And in general, there is no need for splitting to a cover letter if
it's only a single patch; describe it right in the patch itself or
not.)

Secondly, a valid Signed-off-by tag is mandatory for upstreaming.
It's a legal requirement.  And for the Signed-off-by tag (also the
From line as the author), you should provide a full real name.

Now about the code changes:

> --- a/sound/hda/codecs/realtek/alc269.c
> +++ b/sound/hda/codecs/realtek/alc269.c
> @@ -1814,6 +1814,161 @@ static void alc298_samsung_v2_init_amps(struct hda_codec *codec,
>  	spec->gen.pcm_playback_hook = alc298_samsung_v2_playback_hook;
>  }
>  

Maybe it'd be better to have some comments about the quirk.
It's no trivial code, hence it's worth for explanation.

> +struct alc298_lg_gram_style_seq {
> +	unsigned short verb;
> +	unsigned short idx;
> +	unsigned short val;
> +};
> +
> +static void alc298_lg_gram_style_coef_write(struct hda_codec *codec,
> +					    unsigned int verb,
> +					    unsigned int idx,
> +					    unsigned int val)
> +{
> +	snd_hda_codec_write(codec, 0x20, 0, AC_VERB_SET_COEF_INDEX, 0x23);
> +	snd_hda_codec_write(codec, 0x20, 0, verb, idx);
> +	snd_hda_codec_write(codec, 0x20, 0, AC_VERB_SET_PROC_COEF, 0x00);
> +	snd_hda_codec_write(codec, 0x20, 0, AC_VERB_SET_PROC_COEF, val);
> +	snd_hda_codec_write(codec, 0x20, 0, 0x4b0, 0x11);

Use AC_VERB_SET_PROC_COEF for 0x400.  It's equivalent with
	snd_hda_codec_write(codec, 0x20, 0, AC_VERB_SET_PROC_COEF, 0xb011);

Could you try to resubmit with the corrections above?


thanks,

Takashi

  reply	other threads:[~2026-01-23 16:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-21 13:12 [PATCH 0/1] ALSA: hda/realtek: fix LG Gram Style 14 speakers Damien
2026-01-23 16:35 ` Takashi Iwai [this message]
2026-01-23 17:20   ` Damien
2026-01-24 10:21     ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871pjge180.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=damien.dagorn29@gmail.com \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox