All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Enable Realtek ALC671 Codec
@ 2013-04-03 11:29 Rainer Koenig
  2013-04-03 12:25 ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Rainer Koenig @ 2013-04-03 11:29 UTC (permalink / raw)
  To: alsa-devel

Hi,

working on a new mainboard generation with ALC671 codecs the first tests 
showed that the generic codec parser is just configuring the rear mic in 
for audio input, line-in and front-mic are missing.

I tried to understand the code of patch_realtek.c and after compiling 
with CONFIG_SND_DEBUG I saw that this module is not used at all for the 
ALC671 codec because its device-ID was missing.

So what I did is:
- adding the device id and assigned the ALC662 patches for it.
- Added the device id so that it enables 4 ports in
   alc662_parse_auto_config
- changed the conditional in alc_subsystem_id because it looked so
   strange to me when I tried to understand what is happening. :-)

Here is the patch:

Signed-off-by: Rainer Koenig <Rainer.Koenig@ts.fujitsu.com>

--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -1283,7 +1283,7 @@ static int alc_subsystem_id(struct hda_codec *codec,
         }

         ass = codec->subsystem_id & 0xffff;
-       if ((ass != codec->bus->pci->subsystem_device) && (ass & 1))
+       if ((ass == codec->bus->pci->subsystem_device) && (ass & 1))
                 goto do_sku;

         /* invalid SSID, check the special NID pin defcfg instead */
@@ -6663,7 +6663,8 @@ static int alc662_parse_auto_config(struct 
hda_codec *codec)
         const hda_nid_t *ssids;

         if (codec->vendor_id == 0x10ec0272 || codec->vendor_id == 
0x10ec0663 ||
-           codec->vendor_id == 0x10ec0665 || codec->vendor_id == 
0x10ec0670)
+           codec->vendor_id == 0x10ec0665 || codec->vendor_id == 
0x10ec0670 ||
+           codec->vendor_id == 0x10ec0671)
                 ssids = alc663_ssids;
         else
                 ssids = alc662_ssids;
@@ -7115,6 +7116,7 @@ static const struct hda_codec_preset 
snd_hda_preset_realtek[] = {
         { .id = 0x10ec0665, .name = "ALC665", .patch = patch_alc662 },
         { .id = 0x10ec0668, .name = "ALC668", .patch = patch_alc662 },
         { .id = 0x10ec0670, .name = "ALC670", .patch = patch_alc662 },
+       { .id = 0x10ec0671, .name = "ALC671", .patch = patch_alc662 },
         { .id = 0x10ec0680, .name = "ALC680", .patch = patch_alc680 },
         { .id = 0x10ec0880, .name = "ALC880", .patch = patch_alc880 },
         { .id = 0x10ec0882, .name = "ALC882", .patch = patch_alc882 },


-- 
Dipl.-Inf. (FH) Rainer Koenig
Project Manager Linux Clients
Dept. PDG WPS R&D SW OSE

Fujitsu Technology Solutions
Bürgermeister-Ullrich-Str. 100
86199 Augsburg
Germany

Telephone: +49-821-804-3321
Telefax:   +49-821-804-2131
Mail:      mailto:Rainer.Koenig@ts.fujitsu.com

Internet         ts.fujtsu.com
Company Details  ts.fujitsu.com/imprint.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable Realtek ALC671 Codec
  2013-04-03 11:29 [PATCH] Enable Realtek ALC671 Codec Rainer Koenig
@ 2013-04-03 12:25 ` Takashi Iwai
  2013-04-03 13:04   ` Takashi Iwai
  2013-04-03 13:13   ` Rainer Koenig
  0 siblings, 2 replies; 6+ messages in thread
From: Takashi Iwai @ 2013-04-03 12:25 UTC (permalink / raw)
  To: Rainer Koenig; +Cc: alsa-devel

At Wed, 03 Apr 2013 13:29:29 +0200,
Rainer Koenig wrote:
> 
> Hi,
> 
> working on a new mainboard generation with ALC671 codecs the first tests 
> showed that the generic codec parser is just configuring the rear mic in 
> for audio input, line-in and front-mic are missing.
> 
> I tried to understand the code of patch_realtek.c and after compiling 
> with CONFIG_SND_DEBUG I saw that this module is not used at all for the 
> ALC671 codec because its device-ID was missing.
> 
> So what I did is:
> - adding the device id and assigned the ALC662 patches for it.
> - Added the device id so that it enables 4 ports in
>    alc662_parse_auto_config

These are fine to take, but

> - changed the conditional in alc_subsystem_id because it looked so
>    strange to me when I tried to understand what is happening. :-)

... this isn't.  You should try to understand harder :)


Takashi

> Here is the patch:
> 
> Signed-off-by: Rainer Koenig <Rainer.Koenig@ts.fujitsu.com>
> 
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -1283,7 +1283,7 @@ static int alc_subsystem_id(struct hda_codec *codec,
>          }
> 
>          ass = codec->subsystem_id & 0xffff;
> -       if ((ass != codec->bus->pci->subsystem_device) && (ass & 1))
> +       if ((ass == codec->bus->pci->subsystem_device) && (ass & 1))
>                  goto do_sku;
> 
>          /* invalid SSID, check the special NID pin defcfg instead */
> @@ -6663,7 +6663,8 @@ static int alc662_parse_auto_config(struct 
> hda_codec *codec)
>          const hda_nid_t *ssids;
> 
>          if (codec->vendor_id == 0x10ec0272 || codec->vendor_id == 
> 0x10ec0663 ||
> -           codec->vendor_id == 0x10ec0665 || codec->vendor_id == 
> 0x10ec0670)
> +           codec->vendor_id == 0x10ec0665 || codec->vendor_id == 
> 0x10ec0670 ||
> +           codec->vendor_id == 0x10ec0671)
>                  ssids = alc663_ssids;
>          else
>                  ssids = alc662_ssids;
> @@ -7115,6 +7116,7 @@ static const struct hda_codec_preset 
> snd_hda_preset_realtek[] = {
>          { .id = 0x10ec0665, .name = "ALC665", .patch = patch_alc662 },
>          { .id = 0x10ec0668, .name = "ALC668", .patch = patch_alc662 },
>          { .id = 0x10ec0670, .name = "ALC670", .patch = patch_alc662 },
> +       { .id = 0x10ec0671, .name = "ALC671", .patch = patch_alc662 },
>          { .id = 0x10ec0680, .name = "ALC680", .patch = patch_alc680 },
>          { .id = 0x10ec0880, .name = "ALC880", .patch = patch_alc880 },
>          { .id = 0x10ec0882, .name = "ALC882", .patch = patch_alc882 },
> 
> 
> -- 
> Dipl.-Inf. (FH) Rainer Koenig
> Project Manager Linux Clients
> Dept. PDG WPS R&D SW OSE
> 
> Fujitsu Technology Solutions
> Bürgermeister-Ullrich-Str. 100
> 86199 Augsburg
> Germany
> 
> Telephone: +49-821-804-3321
> Telefax:   +49-821-804-2131
> Mail:      mailto:Rainer.Koenig@ts.fujitsu.com
> 
> Internet         ts.fujtsu.com
> Company Details  ts.fujitsu.com/imprint.html
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable Realtek ALC671 Codec
  2013-04-03 12:25 ` Takashi Iwai
@ 2013-04-03 13:04   ` Takashi Iwai
  2013-04-03 13:13   ` Rainer Koenig
  1 sibling, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2013-04-03 13:04 UTC (permalink / raw)
  To: Rainer Koenig; +Cc: alsa-devel

At Wed, 03 Apr 2013 14:25:19 +0200,
Takashi Iwai wrote:
> 
> At Wed, 03 Apr 2013 13:29:29 +0200,
> Rainer Koenig wrote:
> > 
> > Hi,
> > 
> > working on a new mainboard generation with ALC671 codecs the first tests 
> > showed that the generic codec parser is just configuring the rear mic in 
> > for audio input, line-in and front-mic are missing.
> > 
> > I tried to understand the code of patch_realtek.c and after compiling 
> > with CONFIG_SND_DEBUG I saw that this module is not used at all for the 
> > ALC671 codec because its device-ID was missing.
> > 
> > So what I did is:
> > - adding the device id and assigned the ALC662 patches for it.
> > - Added the device id so that it enables 4 ports in
> >    alc662_parse_auto_config
> 
> These are fine to take, but
> 
> > - changed the conditional in alc_subsystem_id because it looked so
> >    strange to me when I tried to understand what is happening. :-)
> 
> ... this isn't.  You should try to understand harder :)

A hint to understand better is that Realtek tries to get its own
config bits given by BIOS.  The first place to check is the lower
16bit of codec SSID, and if this is different from the PCI SSID, it's
supposed to be such a value and taken to retrieve some information.
If it's same as PCI SSID, the pin config of some special pin is
referred as an alternative configuration.

In anyway, could you resend your patch only with the addition of
ALC671?  Also, run scripts/checkpatch.pl and fix the issues before
submission.


thanks,

Takashi

> 
> 
> Takashi
> 
> > Here is the patch:
> > 
> > Signed-off-by: Rainer Koenig <Rainer.Koenig@ts.fujitsu.com>
> > 
> > --- a/sound/pci/hda/patch_realtek.c
> > +++ b/sound/pci/hda/patch_realtek.c
> > @@ -1283,7 +1283,7 @@ static int alc_subsystem_id(struct hda_codec *codec,
> >          }
> > 
> >          ass = codec->subsystem_id & 0xffff;
> > -       if ((ass != codec->bus->pci->subsystem_device) && (ass & 1))
> > +       if ((ass == codec->bus->pci->subsystem_device) && (ass & 1))
> >                  goto do_sku;
> > 
> >          /* invalid SSID, check the special NID pin defcfg instead */
> > @@ -6663,7 +6663,8 @@ static int alc662_parse_auto_config(struct 
> > hda_codec *codec)
> >          const hda_nid_t *ssids;
> > 
> >          if (codec->vendor_id == 0x10ec0272 || codec->vendor_id == 
> > 0x10ec0663 ||
> > -           codec->vendor_id == 0x10ec0665 || codec->vendor_id == 
> > 0x10ec0670)
> > +           codec->vendor_id == 0x10ec0665 || codec->vendor_id == 
> > 0x10ec0670 ||
> > +           codec->vendor_id == 0x10ec0671)
> >                  ssids = alc663_ssids;
> >          else
> >                  ssids = alc662_ssids;
> > @@ -7115,6 +7116,7 @@ static const struct hda_codec_preset 
> > snd_hda_preset_realtek[] = {
> >          { .id = 0x10ec0665, .name = "ALC665", .patch = patch_alc662 },
> >          { .id = 0x10ec0668, .name = "ALC668", .patch = patch_alc662 },
> >          { .id = 0x10ec0670, .name = "ALC670", .patch = patch_alc662 },
> > +       { .id = 0x10ec0671, .name = "ALC671", .patch = patch_alc662 },
> >          { .id = 0x10ec0680, .name = "ALC680", .patch = patch_alc680 },
> >          { .id = 0x10ec0880, .name = "ALC880", .patch = patch_alc880 },
> >          { .id = 0x10ec0882, .name = "ALC882", .patch = patch_alc882 },
> > 
> > 
> > -- 
> > Dipl.-Inf. (FH) Rainer Koenig
> > Project Manager Linux Clients
> > Dept. PDG WPS R&D SW OSE
> > 
> > Fujitsu Technology Solutions
> > Bürgermeister-Ullrich-Str. 100
> > 86199 Augsburg
> > Germany
> > 
> > Telephone: +49-821-804-3321
> > Telefax:   +49-821-804-2131
> > Mail:      mailto:Rainer.Koenig@ts.fujitsu.com
> > 
> > Internet         ts.fujtsu.com
> > Company Details  ts.fujitsu.com/imprint.html
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> > 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable Realtek ALC671 Codec
  2013-04-03 12:25 ` Takashi Iwai
  2013-04-03 13:04   ` Takashi Iwai
@ 2013-04-03 13:13   ` Rainer Koenig
  2013-04-03 13:25     ` Rainer Koenig
  1 sibling, 1 reply; 6+ messages in thread
From: Rainer Koenig @ 2013-04-03 13:13 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On 03.04.2013 14:25, Takashi Iwai wrote:

>> - changed the conditional in alc_subsystem_id because it looked so
>>     strange to me when I tried to understand what is happening. :-)
>
> ... this isn't.  You should try to understand harder :)

I'm really trying hard to understand. The original source says that

	ass = codec->subsystem_id & 0xffff;

Masking out the lower 16bit of codecs subsytem-id in my case ass would 
be 11eb

	if ((ass != codec->bus->pci->subsystem_device) && (ass & 1))

comparing ((11eb !=  11eb) would fail so the whole if-construct fails 
and we don't do the next line that is

		goto do_sku;

Instead we complain about an invalid subsystem-id...?

Really, that is somewhat difficult to understand. Why is the subsystem 
ID that I find in /proc/asound/card0/codec#2 and that matches the 
subsystem device that I can see with 'lspci -vn' invalid???

What stuff do I need to take to understand this? ;-)

Regards
Rainer
-- 
Dipl.-Inf. (FH) Rainer Koenig
Project Manager Linux Clients
Dept. PDG WPS R&D SW OSE

Fujitsu Technology Solutions
Bürgermeister-Ullrich-Str. 100
86199 Augsburg
Germany

Telephone: +49-821-804-3321
Telefax:   +49-821-804-2131
Mail:      mailto:Rainer.Koenig@ts.fujitsu.com

Internet         ts.fujtsu.com
Company Details  ts.fujitsu.com/imprint.html
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable Realtek ALC671 Codec
  2013-04-03 13:13   ` Rainer Koenig
@ 2013-04-03 13:25     ` Rainer Koenig
  2013-04-03 13:31       ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Rainer Koenig @ 2013-04-03 13:25 UTC (permalink / raw)
  To: alsa-devel

Resend of the patch (as asked by Takashi Iwai):

Signed-off-by: Rainer Koenig <Rainer.Koenig@ts.fujitsu.com>

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 7743775..fcbf4c9 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6663,7 +6663,8 @@ static int alc662_parse_auto_config(struct 
hda_codec *codec)
  	const hda_nid_t *ssids;

  	if (codec->vendor_id == 0x10ec0272 || codec->vendor_id == 0x10ec0663 ||
-	    codec->vendor_id == 0x10ec0665 || codec->vendor_id == 0x10ec0670)
+	    codec->vendor_id == 0x10ec0665 || codec->vendor_id == 0x10ec0670 ||
+	    codec->vendor_id == 0x10ec0671)
  		ssids = alc663_ssids;
  	else
  		ssids = alc662_ssids;
@@ -7115,6 +7116,7 @@ static const struct hda_codec_preset 
snd_hda_preset_realtek[] = {
  	{ .id = 0x10ec0665, .name = "ALC665", .patch = patch_alc662 },
  	{ .id = 0x10ec0668, .name = "ALC668", .patch = patch_alc662 },
  	{ .id = 0x10ec0670, .name = "ALC670", .patch = patch_alc662 },
+	{ .id = 0x10ec0671, .name = "ALC671", .patch = patch_alc662 },
  	{ .id = 0x10ec0680, .name = "ALC680", .patch = patch_alc680 },
  	{ .id = 0x10ec0880, .name = "ALC880", .patch = patch_alc880 },
  	{ .id = 0x10ec0882, .name = "ALC882", .patch = patch_alc882 },



-- 
Dipl.-Inf. (FH) Rainer Koenig
Project Manager Linux Clients
Dept. PDG WPS R&D SW OSE

Fujitsu Technology Solutions
Bürgermeister-Ullrich-Str. 100
86199 Augsburg
Germany

Telephone: +49-821-804-3321
Telefax:   +49-821-804-2131
Mail:      mailto:Rainer.Koenig@ts.fujitsu.com

Internet         ts.fujtsu.com
Company Details  ts.fujitsu.com/imprint.html
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Enable Realtek ALC671 Codec
  2013-04-03 13:25     ` Rainer Koenig
@ 2013-04-03 13:31       ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2013-04-03 13:31 UTC (permalink / raw)
  To: Rainer Koenig; +Cc: alsa-devel

At Wed, 03 Apr 2013 15:25:25 +0200,
Rainer Koenig wrote:
> 
> Resend of the patch (as asked by Takashi Iwai):

Keep the patch description.  Resending a patch means to send a
comprehensive patch that can be applied directly.

Also, check how other patches look like.  Usually there are specific
pattern of the subject line depending on the path.

And, last but not least, always give maintainers (in this case, me) to
Cc when you submit a patch.  Maintainers may overlook if it's sent
only to ML.


thanks,

Takashi

> Signed-off-by: Rainer Koenig <Rainer.Koenig@ts.fujitsu.com>
> 
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 7743775..fcbf4c9 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -6663,7 +6663,8 @@ static int alc662_parse_auto_config(struct 
> hda_codec *codec)
>   	const hda_nid_t *ssids;
> 
>   	if (codec->vendor_id == 0x10ec0272 || codec->vendor_id == 0x10ec0663 ||
> -	    codec->vendor_id == 0x10ec0665 || codec->vendor_id == 0x10ec0670)
> +	    codec->vendor_id == 0x10ec0665 || codec->vendor_id == 0x10ec0670 ||
> +	    codec->vendor_id == 0x10ec0671)
>   		ssids = alc663_ssids;
>   	else
>   		ssids = alc662_ssids;
> @@ -7115,6 +7116,7 @@ static const struct hda_codec_preset 
> snd_hda_preset_realtek[] = {
>   	{ .id = 0x10ec0665, .name = "ALC665", .patch = patch_alc662 },
>   	{ .id = 0x10ec0668, .name = "ALC668", .patch = patch_alc662 },
>   	{ .id = 0x10ec0670, .name = "ALC670", .patch = patch_alc662 },
> +	{ .id = 0x10ec0671, .name = "ALC671", .patch = patch_alc662 },
>   	{ .id = 0x10ec0680, .name = "ALC680", .patch = patch_alc680 },
>   	{ .id = 0x10ec0880, .name = "ALC880", .patch = patch_alc880 },
>   	{ .id = 0x10ec0882, .name = "ALC882", .patch = patch_alc882 },
> 
> 
> 
> -- 
> Dipl.-Inf. (FH) Rainer Koenig
> Project Manager Linux Clients
> Dept. PDG WPS R&D SW OSE
> 
> Fujitsu Technology Solutions
> Bürgermeister-Ullrich-Str. 100
> 86199 Augsburg
> Germany
> 
> Telephone: +49-821-804-3321
> Telefax:   +49-821-804-2131
> Mail:      mailto:Rainer.Koenig@ts.fujitsu.com
> 
> Internet         ts.fujtsu.com
> Company Details  ts.fujitsu.com/imprint.html
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-04-03 13:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-03 11:29 [PATCH] Enable Realtek ALC671 Codec Rainer Koenig
2013-04-03 12:25 ` Takashi Iwai
2013-04-03 13:04   ` Takashi Iwai
2013-04-03 13:13   ` Rainer Koenig
2013-04-03 13:25     ` Rainer Koenig
2013-04-03 13:31       ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.