alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ALSA: hda - Fix error accessing non-existent node for Realtek codecs
@ 2013-07-15  8:20 David Henningsson
  2013-07-15  8:55 ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: David Henningsson @ 2013-07-15  8:20 UTC (permalink / raw)
  To: alsa-devel, tiwai, mengdong.lin, kailang; +Cc: David Henningsson

A recent commit caused my hda-emu based test tool to complain about
accessing nodes that don't exist. This simple test works around this.

Signed-off-by: David Henningsson <david.henningsson@canonical.com>
---
 sound/pci/hda/patch_realtek.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

I don't have ALC5505 hardware here, no alsa-info to look at, and I don't know
what it is. Therefore it would be good with some confirmation that this patch
does not break such hardware, which could be the case if the Realtek codec
breaks the HDA spec by having "hidden" nodes.

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 8bd2261..ca73f10 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -3844,7 +3844,8 @@ static int patch_alc269(struct hda_codec *codec)
 		break;
 	}
 
-	if (snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 0x10ec5505) {
+	if (codec->num_nodes > 0x51 &&
+	    snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 0x10ec5505) {
 		spec->has_alc5505_dsp = true;
 		spec->init_hook = alc5505_dsp_init;
 	}
-- 
1.7.9.5

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

* Re: [PATCH] ALSA: hda - Fix error accessing non-existent node for Realtek codecs
  2013-07-15  8:20 [PATCH] ALSA: hda - Fix error accessing non-existent node for Realtek codecs David Henningsson
@ 2013-07-15  8:55 ` Takashi Iwai
  2013-07-15  8:58   ` David Henningsson
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2013-07-15  8:55 UTC (permalink / raw)
  To: David Henningsson; +Cc: mengdong.lin, alsa-devel, kailang

At Mon, 15 Jul 2013 10:20:52 +0200,
David Henningsson wrote:
> 
> A recent commit caused my hda-emu based test tool to complain about
> accessing nodes that don't exist. This simple test works around this.
> 
> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> ---
>  sound/pci/hda/patch_realtek.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> I don't have ALC5505 hardware here, no alsa-info to look at, and I don't know
> what it is. Therefore it would be good with some confirmation that this patch
> does not break such hardware, which could be the case if the Realtek codec
> breaks the HDA spec by having "hidden" nodes.

It seems that ALC5505 itself doesn't appear in the codec information
but it appears transparent to the real codec behind it.  So the node
0x51 doesn't exist in the widget list of the codec, and your  patch
will break.

we may need to patch hda-emu instead...


Takashi

> 
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 8bd2261..ca73f10 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -3844,7 +3844,8 @@ static int patch_alc269(struct hda_codec *codec)
>  		break;
>  	}
>  
> -	if (snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 0x10ec5505) {
> +	if (codec->num_nodes > 0x51 &&
> +	    snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 0x10ec5505) {
>  		spec->has_alc5505_dsp = true;
>  		spec->init_hook = alc5505_dsp_init;
>  	}
> -- 
> 1.7.9.5
> 

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

* Re: [PATCH] ALSA: hda - Fix error accessing non-existent node for Realtek codecs
  2013-07-15  8:55 ` Takashi Iwai
@ 2013-07-15  8:58   ` David Henningsson
  2013-07-15  9:26     ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: David Henningsson @ 2013-07-15  8:58 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: mengdong.lin, alsa-devel, kailang

On 07/15/2013 10:55 AM, Takashi Iwai wrote:
> At Mon, 15 Jul 2013 10:20:52 +0200,
> David Henningsson wrote:
>>
>> A recent commit caused my hda-emu based test tool to complain about
>> accessing nodes that don't exist. This simple test works around this.
>>
>> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
>> ---
>>   sound/pci/hda/patch_realtek.c |    3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> I don't have ALC5505 hardware here, no alsa-info to look at, and I don't know
>> what it is. Therefore it would be good with some confirmation that this patch
>> does not break such hardware, which could be the case if the Realtek codec
>> breaks the HDA spec by having "hidden" nodes.
>
> It seems that ALC5505 itself doesn't appear in the codec information
> but it appears transparent to the real codec behind it.  So the node
> 0x51 doesn't exist in the widget list of the codec, and your  patch
> will break.

I suspected that, but without alsa-info there was no way to tell if this 
was the case or not.

> we may need to patch hda-emu instead...

Okay. Also, is there a way to limit the effect here, like, what codec 
(or codec variants) have this secret 0x51 node? Right now it's being 
tested for most of ALC2xx codecs.

>
>
> Takashi
>
>>
>> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
>> index 8bd2261..ca73f10 100644
>> --- a/sound/pci/hda/patch_realtek.c
>> +++ b/sound/pci/hda/patch_realtek.c
>> @@ -3844,7 +3844,8 @@ static int patch_alc269(struct hda_codec *codec)
>>   		break;
>>   	}
>>
>> -	if (snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 0x10ec5505) {
>> +	if (codec->num_nodes > 0x51 &&
>> +	    snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 0x10ec5505) {
>>   		spec->has_alc5505_dsp = true;
>>   		spec->init_hook = alc5505_dsp_init;
>>   	}
>> --
>> 1.7.9.5
>>
>



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

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

* Re: [PATCH] ALSA: hda - Fix error accessing non-existent node for Realtek codecs
  2013-07-15  8:58   ` David Henningsson
@ 2013-07-15  9:26     ` Takashi Iwai
  2013-07-15  9:33       ` Kailang
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2013-07-15  9:26 UTC (permalink / raw)
  To: David Henningsson; +Cc: mengdong.lin, alsa-devel, kailang

At Mon, 15 Jul 2013 10:58:16 +0200,
David Henningsson wrote:
> 
> On 07/15/2013 10:55 AM, Takashi Iwai wrote:
> > At Mon, 15 Jul 2013 10:20:52 +0200,
> > David Henningsson wrote:
> >>
> >> A recent commit caused my hda-emu based test tool to complain about
> >> accessing nodes that don't exist. This simple test works around this.
> >>
> >> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> >> ---
> >>   sound/pci/hda/patch_realtek.c |    3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> I don't have ALC5505 hardware here, no alsa-info to look at, and I don't know
> >> what it is. Therefore it would be good with some confirmation that this patch
> >> does not break such hardware, which could be the case if the Realtek codec
> >> breaks the HDA spec by having "hidden" nodes.
> >
> > It seems that ALC5505 itself doesn't appear in the codec information
> > but it appears transparent to the real codec behind it.  So the node
> > 0x51 doesn't exist in the widget list of the codec, and your  patch
> > will break.
> 
> I suspected that, but without alsa-info there was no way to tell if this 
> was the case or not.
> 
> > we may need to patch hda-emu instead...
> 
> Okay. Also, is there a way to limit the effect here, like, what codec 
> (or codec variants) have this secret 0x51 node? Right now it's being 
> tested for most of ALC2xx codecs.

I guess this won't be limited to any codec type.  The DSP itself seems
transparent, so it could be connected to any codec.

Right now, we patched alc269 variants because they are only real
cases, so far.


Takashi


> 
> >
> >
> > Takashi
> >
> >>
> >> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> >> index 8bd2261..ca73f10 100644
> >> --- a/sound/pci/hda/patch_realtek.c
> >> +++ b/sound/pci/hda/patch_realtek.c
> >> @@ -3844,7 +3844,8 @@ static int patch_alc269(struct hda_codec *codec)
> >>   		break;
> >>   	}
> >>
> >> -	if (snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 0x10ec5505) {
> >> +	if (codec->num_nodes > 0x51 &&
> >> +	    snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 0x10ec5505) {
> >>   		spec->has_alc5505_dsp = true;
> >>   		spec->init_hook = alc5505_dsp_init;
> >>   	}
> >> --
> >> 1.7.9.5
> >>
> >
> 
> 
> 
> -- 
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
> 

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

* Re: [PATCH] ALSA: hda - Fix error accessing non-existent node for Realtek codecs
  2013-07-15  9:26     ` Takashi Iwai
@ 2013-07-15  9:33       ` Kailang
  2013-07-15 18:14         ` David Henningsson
  0 siblings, 1 reply; 6+ messages in thread
From: Kailang @ 2013-07-15  9:33 UTC (permalink / raw)
  To: Takashi Iwai, David Henningsson; +Cc: mengdong.lin, alsa-devel


Yes, DSP could be connected to any codecs.
Because Intel use alc282 codec. So, we patched alc269 variants now.

----- Original Message ----- 
From: "Takashi Iwai" <tiwai@suse.de>
To: "David Henningsson" <david.henningsson@canonical.com>
Cc: <alsa-devel@alsa-project.org>; <mengdong.lin@intel.com>; "Kailang" 
<kailang@realtek.com>
Sent: Monday, July 15, 2013 5:26 PM
Subject: Re: [PATCH] ALSA: hda - Fix error accessing non-existent node for 
Realtek codecs


At Mon, 15 Jul 2013 10:58:16 +0200,
David Henningsson wrote:
>
> On 07/15/2013 10:55 AM, Takashi Iwai wrote:
> > At Mon, 15 Jul 2013 10:20:52 +0200,
> > David Henningsson wrote:
> >>
> >> A recent commit caused my hda-emu based test tool to complain about
> >> accessing nodes that don't exist. This simple test works around this.
> >>
> >> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
> >> ---
> >>   sound/pci/hda/patch_realtek.c |    3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> I don't have ALC5505 hardware here, no alsa-info to look at, and I 
> >> don't know
> >> what it is. Therefore it would be good with some confirmation that this 
> >> patch
> >> does not break such hardware, which could be the case if the Realtek 
> >> codec
> >> breaks the HDA spec by having "hidden" nodes.
> >
> > It seems that ALC5505 itself doesn't appear in the codec information
> > but it appears transparent to the real codec behind it.  So the node
> > 0x51 doesn't exist in the widget list of the codec, and your  patch
> > will break.
>
> I suspected that, but without alsa-info there was no way to tell if this
> was the case or not.
>
> > we may need to patch hda-emu instead...
>
> Okay. Also, is there a way to limit the effect here, like, what codec
> (or codec variants) have this secret 0x51 node? Right now it's being
> tested for most of ALC2xx codecs.

I guess this won't be limited to any codec type.  The DSP itself seems
transparent, so it could be connected to any codec.

Right now, we patched alc269 variants because they are only real
cases, so far.


Takashi


>
> >
> >
> > Takashi
> >
> >>
> >> diff --git a/sound/pci/hda/patch_realtek.c 
> >> b/sound/pci/hda/patch_realtek.c
> >> index 8bd2261..ca73f10 100644
> >> --- a/sound/pci/hda/patch_realtek.c
> >> +++ b/sound/pci/hda/patch_realtek.c
> >> @@ -3844,7 +3844,8 @@ static int patch_alc269(struct hda_codec *codec)
> >>   break;
> >>   }
> >>
> >> - if (snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 
> >> 0x10ec5505) {
> >> + if (codec->num_nodes > 0x51 &&
> >> +     snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) == 
> >> 0x10ec5505) {
> >>   spec->has_alc5505_dsp = true;
> >>   spec->init_hook = alc5505_dsp_init;
> >>   }
> >> --
> >> 1.7.9.5
> >>
> >
>
>
>
> -- 
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
>

------Please consider the environment before printing this e-mail. 

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

* Re: [PATCH] ALSA: hda - Fix error accessing non-existent node for Realtek codecs
  2013-07-15  9:33       ` Kailang
@ 2013-07-15 18:14         ` David Henningsson
  0 siblings, 0 replies; 6+ messages in thread
From: David Henningsson @ 2013-07-15 18:14 UTC (permalink / raw)
  To: Kailang; +Cc: Takashi Iwai, mengdong.lin, alsa-devel

On 07/15/2013 11:33 AM, Kailang wrote:
>
> Yes, DSP could be connected to any codecs.
> Because Intel use alc282 codec. So, we patched alc269 variants now.

Thanks for the clarification. Will the DSP always be connected to node 
0x51 regardless of codec, or can this number vary between codecs too?

>
> ----- Original Message ----- From: "Takashi Iwai" <tiwai@suse.de>
> To: "David Henningsson" <david.henningsson@canonical.com>
> Cc: <alsa-devel@alsa-project.org>; <mengdong.lin@intel.com>; "Kailang"
> <kailang@realtek.com>
> Sent: Monday, July 15, 2013 5:26 PM
> Subject: Re: [PATCH] ALSA: hda - Fix error accessing non-existent node
> for Realtek codecs
>
>
> At Mon, 15 Jul 2013 10:58:16 +0200,
> David Henningsson wrote:
>>
>> On 07/15/2013 10:55 AM, Takashi Iwai wrote:
>> > At Mon, 15 Jul 2013 10:20:52 +0200,
>> > David Henningsson wrote:
>> >>
>> >> A recent commit caused my hda-emu based test tool to complain about
>> >> accessing nodes that don't exist. This simple test works around this.
>> >>
>> >> Signed-off-by: David Henningsson <david.henningsson@canonical.com>
>> >> ---
>> >>   sound/pci/hda/patch_realtek.c |    3 ++-
>> >>   1 file changed, 2 insertions(+), 1 deletion(-)
>> >>
>> >> I don't have ALC5505 hardware here, no alsa-info to look at, and I
>> >> don't know
>> >> what it is. Therefore it would be good with some confirmation that
>> this >> patch
>> >> does not break such hardware, which could be the case if the
>> Realtek >> codec
>> >> breaks the HDA spec by having "hidden" nodes.
>> >
>> > It seems that ALC5505 itself doesn't appear in the codec information
>> > but it appears transparent to the real codec behind it.  So the node
>> > 0x51 doesn't exist in the widget list of the codec, and your  patch
>> > will break.
>>
>> I suspected that, but without alsa-info there was no way to tell if this
>> was the case or not.
>>
>> > we may need to patch hda-emu instead...
>>
>> Okay. Also, is there a way to limit the effect here, like, what codec
>> (or codec variants) have this secret 0x51 node? Right now it's being
>> tested for most of ALC2xx codecs.
>
> I guess this won't be limited to any codec type.  The DSP itself seems
> transparent, so it could be connected to any codec.
>
> Right now, we patched alc269 variants because they are only real
> cases, so far.
>
>
> Takashi
>
>
>>
>> >
>> >
>> > Takashi
>> >
>> >>
>> >> diff --git a/sound/pci/hda/patch_realtek.c >>
>> b/sound/pci/hda/patch_realtek.c
>> >> index 8bd2261..ca73f10 100644
>> >> --- a/sound/pci/hda/patch_realtek.c
>> >> +++ b/sound/pci/hda/patch_realtek.c
>> >> @@ -3844,7 +3844,8 @@ static int patch_alc269(struct hda_codec *codec)
>> >>   break;
>> >>   }
>> >>
>> >> - if (snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) ==
>> >> 0x10ec5505) {
>> >> + if (codec->num_nodes > 0x51 &&
>> >> +     snd_hda_codec_read(codec, 0x51, 0, AC_VERB_PARAMETERS, 0) ==
>> >> 0x10ec5505) {
>> >>   spec->has_alc5505_dsp = true;
>> >>   spec->init_hook = alc5505_dsp_init;
>> >>   }
>> >> --
>> >> 1.7.9.5
>> >>
>> >
>>
>>
>>
>> --
>> David Henningsson, Canonical Ltd.
>> https://launchpad.net/~diwic
>>
>
> ------Please consider the environment before printing this e-mail.



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

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

end of thread, other threads:[~2013-07-15 18:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-15  8:20 [PATCH] ALSA: hda - Fix error accessing non-existent node for Realtek codecs David Henningsson
2013-07-15  8:55 ` Takashi Iwai
2013-07-15  8:58   ` David Henningsson
2013-07-15  9:26     ` Takashi Iwai
2013-07-15  9:33       ` Kailang
2013-07-15 18:14         ` David Henningsson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).