diff mbox series

Enable SPDIF output on Intel Hades Canyon

Message ID 5d5924ee-a52a-04f0-5080-2b8d91bce5ba@bakke.com
State New
Headers show
Series Enable SPDIF output on Intel Hades Canyon | expand

Commit Message

Dag B May 23, 2022, 8:20 p.m. UTC 
Without the attached patch, the s/pdif output on the Hades Canyon NUC 
does not work.

"Well known" issue, less known fix. As far as I can tell, there is no 
risk of any averse side-effects. But a bonus fix is enabling on-wire 
headset microphone, by chaining the right 'model' choice for the hardware.

If I should Cc: someone directly for this patch to be picked up, please 
let me know.


I have been running with this patch for half a year or so.

Patch passes checkpatch.pl

Patch is based on what 'cyber4o' posted on the insanelymac forum [1]. 
All the glory to this person, any error is likely mine.

Some other users with issues: [2] [3]


Dag Bakke


[1] 
https://www.insanelymac.com/forum/topic/339291-guide-hac-mini-osx-mojave-on-intel-hades-canyon-nuc8i7hvknuc8i7hnk/page/8/

2] https://bbs.archlinux.org/viewtopic.php?id=270917

[3] 
https://www.reddit.com/r/intelnuc/comments/9ft9x8/any_linux_users_got_the_spdif_toslink_to_work_on/

Comments

Pierre-Louis Bossart May 23, 2022, 8:30 p.m. UTC | #1 
On 5/23/22 15:20, Dag B wrote:
> Without the attached patch, the s/pdif output on the Hades Canyon NUC
> does not work.
> 
> "Well known" issue, less known fix. As far as I can tell, there is no
> risk of any averse side-effects. But a bonus fix is enabling on-wire
> headset microphone, by chaining the right 'model' choice for the hardware.
> 
> If I should Cc: someone directly for this patch to be picked up, please
> let me know.
> 
> 
> I have been running with this patch for half a year or so.
> 
> Patch passes checkpatch.pl
> 
> Patch is based on what 'cyber4o' posted on the insanelymac forum [1].
> All the glory to this person, any error is likely mine.

Thanks for the patch.

You would need a Signed-off-by tag for this patch, and CC: Takashi (added)

+	[ALC700_FIXUP_NUC_SPDIF] = {

Maybe use HC_NUC, there are multiple versions of those devices?

+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc700_fixup_nuc_spdif,

alc700_fixup_hc_nuc_spdif ?

+		.chained = true,
+		.chain_id = ALC269_FIXUP_DELL1_MIC_NO_PRESENCE,

that chain_id doesn't seem quite right?


> 
> Some other users with issues: [2] [3]
> 
> 
> Dag Bakke
> 
> 
> [1]
> https://www.insanelymac.com/forum/topic/339291-guide-hac-mini-osx-mojave-on-intel-hades-canyon-nuc8i7hvknuc8i7hnk/page/8/
> 
> 
> 2] https://bbs.archlinux.org/viewtopic.php?id=270917
> 
> [3]
> https://www.reddit.com/r/intelnuc/comments/9ft9x8/any_linux_users_got_the_spdif_toslink_to_work_on/
> 
> 
>
Pierre-Louis Bossart May 23, 2022, 9:53 p.m. UTC | #2 
On 5/23/22 16:29, Dag B wrote:
> On 23.05.2022 22:30, Pierre-Louis Bossart wrote:
>> On 5/23/22 15:20, Dag B wrote:
>>> Without the attached patch, the s/pdif output on the Hades Canyon NUC
>>> does not work.
>>>
>>> "Well known" issue, less known fix. As far as I can tell, there is no
>>> risk of any averse side-effects. But a bonus fix is enabling on-wire
>>> headset microphone, by chaining the right 'model' choice for the
>>> hardware.
>>>
>>> If I should Cc: someone directly for this patch to be picked up, please
>>> let me know.
>>>
>>>
>>> I have been running with this patch for half a year or so.
>>>
>>> Patch passes checkpatch.pl
>>>
>>> Patch is based on what 'cyber4o' posted on the insanelymac forum [1].
>>> All the glory to this person, any error is likely mine.
>> Thanks for the patch.
>>
>> You would need a Signed-off-by tag for this patch, and CC: Takashi
>> (added)
>>
>> +    [ALC700_FIXUP_NUC_SPDIF] = {
>>
>> Maybe use HC_NUC, there are multiple versions of those devices?
>>
>> +        .type = HDA_FIXUP_FUNC,
>> +        .v.func = alc700_fixup_nuc_spdif,
>>
>> alc700_fixup_hc_nuc_spdif ?
>>
>> +        .chained = true,
>> +        .chain_id = ALC269_FIXUP_DELL1_MIC_NO_PRESENCE,
>>
>> that chain_id doesn't seem quite right?
>>
> Thank you for reviewing this. I have made adjustments as suggested.
> Revised patch attached.
>  
> The chain_id may appear odd, I can assure you that it does the job. I
> picked up the suggestion from the excellent Arch Linux wiki:
> 
> https://wiki.archlinux.org/title/Intel_NUC#Hades_Canyon_NUC_-_No_External_Microphones

Parts of my comment was the reference to ALC269 when this is an ALC700,
but it seems the two parts are identical if I understand this
definition well:	

ALC269_TYPE_ALC700,

The other point was the reference to Dell when this isn't a Dell
platform. ALC269_FIXUP_DELL1_MIC_NO_PRESENCE may do the job but so far
it's only used for Dell platforms, so it's a bit confusing.

> So my patch equates to:
> 
> a) creating a fixup for enabling spdif
> 
> and
> 
> b) making the new "model=nuc-hc" equate to the spdif fix +
> "model=dell-headset-multi"
> 
> I hope this is acceptable. As stated, I have used this solution for half
> a year or so.
> 
> |Signed-off-by: Dag Bakke <dag@bakke.com>|

usually this comes inside the patch with a git commit title and message.

> 
> Dag B
> 
> 
> 
>>> Some other users with issues: [2] [3]
>>>
>>>
>>> Dag Bakke
>>>
>>>
>>> [1]
>>> https://www.insanelymac.com/forum/topic/339291-guide-hac-mini-osx-mojave-on-intel-hades-canyon-nuc8i7hvknuc8i7hnk/page/8/
>>>
>>>
>>>
>>> 2]https://bbs.archlinux.org/viewtopic.php?id=270917
>>>
>>> [3]
>>> https://www.reddit.com/r/intelnuc/comments/9ft9x8/any_linux_users_got_the_spdif_toslink_to_work_on/
>>>
>>>
>>>
>>>
Takashi Iwai May 27, 2022, 2:50 p.m. UTC | #3 
On Tue, 24 May 2022 08:58:30 +0200,
Dag B wrote:
> From fe562e391b522dca09f00a5f8c280ab43136ef1f Mon Sep 17 00:00:00 2001
> From: Dag B <dag@bakke.com>
> Date: Tue, 24 May 2022 08:38:42 +0200
> Subject: [PATCH] Enable Intel Hades Canyon SPDIF
> 
> Signed-off-by: Dag B <dag@bakke.com>

Please give more description, especially why this patch is needed and
what actually does.

About the code change:


> ---
>  sound/pci/hda/patch_realtek.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index ad292df7d805..cd6a2cb4c381 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -4752,6 +4752,12 @@ static void alc280_fixup_hp_gpio2_mic_hotkey(struct hda_codec *codec,
>  	}
>  }
>  
> +static void alc700_fixup_hc_nuc_spdif(struct hda_codec *codec,
> +				const struct hda_fixup *fix, int action)
> +{
> +	snd_hda_override_wcaps(codec, 0x6, 0x611);

Better to be the values with AC_WCAP_*.


> @@ -5792,7 +5798,7 @@ static void alc_fixup_headset_mode_alc255_no_hp_mic(struct hda_codec *codec,
>  		struct alc_spec *spec = codec->spec;
>  		spec->parse_flags |= HDA_PINCFG_HEADSET_MIC;
>  		alc255_set_default_jack_type(codec);
> -	} 
> +	}
>  	else
>  		alc_fixup_headset_mode(codec, fix, action);
>  }

Avoid unnecessary changes like this.


> @@ -9159,6 +9172,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
>  	SND_PCI_QUIRK(0x10cf, 0x1757, "Lifebook E752", ALC269_FIXUP_LIFEBOOK_HP_PIN),
>  	SND_PCI_QUIRK(0x10cf, 0x1845, "Lifebook U904", ALC269_FIXUP_LIFEBOOK_EXTMIC),
>  	SND_PCI_QUIRK(0x10ec, 0x10f2, "Intel Reference board", ALC700_FIXUP_INTEL_REFERENCE),
> +	SND_PCI_QUIRK(0x10ec, 0x2073, "Intel NUC8 Hades Canyon", ALC700_FIXUP_HC_NUC_SPDIF),

So this is about the generic ID (with Realtek vendor-id).
It's most likely OK, as such an ID is used only for some reference
board or such, but it should be mentioned in the changelog.

> @@ -9445,6 +9459,7 @@ static const struct hda_model_fixup alc269_fixup_models[] = {
>  	{.id = ALC298_FIXUP_TPT470_DOCK, .name = "tpt470-dock"},
>  	{.id = ALC233_FIXUP_LENOVO_MULTI_CODECS, .name = "dual-codecs"},
>  	{.id = ALC700_FIXUP_INTEL_REFERENCE, .name = "alc700-ref"},
> +	{.id = ALC700_FIXUP_HC_NUC_SPDIF, .name = "nuc-hc"},

Ditto, better to mention the available model name in the changelog,
too.

Once after all those are fixed / improved, please resubmit the patch.


thanks,

Takashi
diff mbox series

Patch

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index ad292df7d805..1c4c7435b705 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -4752,6 +4752,12 @@  static void alc280_fixup_hp_gpio2_mic_hotkey(struct hda_codec *codec,
 	}
 }
 
+static void alc700_fixup_nuc_spdif(struct hda_codec *codec,
+				const struct hda_fixup *fix, int action)
+{
+	snd_hda_override_wcaps(codec, 0x6, 0x611);
+}
+
 /* Line2 = mic mute hotkey
  * GPIO2 = mic mute LED
  */
@@ -5792,7 +5798,7 @@  static void alc_fixup_headset_mode_alc255_no_hp_mic(struct hda_codec *codec,
 		struct alc_spec *spec = codec->spec;
 		spec->parse_flags |= HDA_PINCFG_HEADSET_MIC;
 		alc255_set_default_jack_type(codec);
-	} 
+	}
 	else
 		alc_fixup_headset_mode(codec, fix, action);
 }
@@ -6939,6 +6945,7 @@  enum {
 	ALC225_FIXUP_DELL_WYSE_MIC_NO_PRESENCE,
 	ALC225_FIXUP_S3_POP_NOISE,
 	ALC700_FIXUP_INTEL_REFERENCE,
+	ALC700_FIXUP_NUC_SPDIF,
 	ALC274_FIXUP_DELL_BIND_DACS,
 	ALC274_FIXUP_DELL_AIO_LINEOUT_VERB,
 	ALC298_FIXUP_TPT470_DOCK_FIX,
@@ -7260,6 +7267,12 @@  static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
 	},
+	[ALC700_FIXUP_NUC_SPDIF] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = alc700_fixup_nuc_spdif,
+		.chained = true,
+		.chain_id = ALC269_FIXUP_DELL1_MIC_NO_PRESENCE,
+	},
 	[ALC269_FIXUP_DELL1_MIC_NO_PRESENCE] = {
 		.type = HDA_FIXUP_PINS,
 		.v.pins = (const struct hda_pintbl[]) {
@@ -9159,6 +9172,7 @@  static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x10cf, 0x1757, "Lifebook E752", ALC269_FIXUP_LIFEBOOK_HP_PIN),
 	SND_PCI_QUIRK(0x10cf, 0x1845, "Lifebook U904", ALC269_FIXUP_LIFEBOOK_EXTMIC),
 	SND_PCI_QUIRK(0x10ec, 0x10f2, "Intel Reference board", ALC700_FIXUP_INTEL_REFERENCE),
+	SND_PCI_QUIRK(0x10ec, 0x2073, "Intel NUC8 Hades Canyon", ALC700_FIXUP_NUC_SPDIF),
 	SND_PCI_QUIRK(0x10ec, 0x118c, "Medion EE4254 MD62100", ALC256_FIXUP_MEDION_HEADSET_NO_PRESENCE),
 	SND_PCI_QUIRK(0x10ec, 0x1230, "Intel Reference board", ALC295_FIXUP_CHROME_BOOK),
 	SND_PCI_QUIRK(0x10ec, 0x1252, "Intel Reference board", ALC295_FIXUP_CHROME_BOOK),
@@ -9445,6 +9459,7 @@  static const struct hda_model_fixup alc269_fixup_models[] = {
 	{.id = ALC298_FIXUP_TPT470_DOCK, .name = "tpt470-dock"},
 	{.id = ALC233_FIXUP_LENOVO_MULTI_CODECS, .name = "dual-codecs"},
 	{.id = ALC700_FIXUP_INTEL_REFERENCE, .name = "alc700-ref"},
+	{.id = ALC700_FIXUP_NUC_SPDIF, .name = "nuc-hc"},
 	{.id = ALC269_FIXUP_SONY_VAIO, .name = "vaio"},
 	{.id = ALC269_FIXUP_DELL_M101Z, .name = "dell-m101z"},
 	{.id = ALC269_FIXUP_ASUS_G73JW, .name = "asus-g73jw"},