At Tue, 19 Dec 2006 12:37:46 -0500,
Randy Cushman wrote:
>
> Minor request implemented.
Thanks, now I applied it to ALSA HG repo.
Takashi
>
> Randy Cushman
>
>
> Takashi Iwai wrote:
> > At Tue, 19 Dec 2006 11:51:29 -0500,
> > Randy Cushman wrote:
> >
> >> Attached is the first of my split submissions.
> >>
> >
> > Thanks!
> >
> >
> >> Apparently you are correct. I compared the ALC650 datasheet with the
> >> corresponding code in ac97_patch.c. This analysis indicates that my
> >> change would have broken support for this chipset. Part of my confusion
> >> stemmed from the names of the functions in question, which I find to be
> >> counterintuitive.
> >>
> >
> > Ah yes, the names are confusing.
> >
> >
> >> Since some of your comments suggest to me that changes that improve code
> >> maintainability are sometimes preferable to changes that affect the
> >> fewest lines of code, in the attached patch I have taken the liberty of
> >> renaming functions to names that I find to be more meaningful.
> >>
> >> I'll hold off on creating the remaining patches for now because my other
> >> patches may need to be changed if this patch is not accepted.
> >>
> >> Please let me know what you think.
> >>
> >
> > Then change looks fine. One minor request is not to put a space
> > between '!' and the rest in the new codes. I used this style, but
> > apparently it's no major and kernel people tend to dislike it.
> >
> >
> > [BTW, I added Cc to alsa-devel ML again, supposing that you forgot
> > it.]
> >
> >
> > Takashi
> >
> >
> >
> >> Randy Cushman
> >>
> >>
> >> (From "Re: Patch for AD1985 AC97 CODEC")
> >>
> >> Takashi Iwai wrote:
> >>
> >>> Hi,
> >>>
> >>> thanks for the patch.
> >>> For the ease of review, could you split your change to several
> >>> patches? Please post one patch per mail, then.
> >>>
> >>> Anyway, a quick review through the patch is below.
> >>>
> >>> At Tue, 12 Dec 2006 19:31:33 -0500,
> >>> Randy Cushman wrote:
> >>>
> >>>
> >>>> Summary: fix microphone and line_in selection logic
> >>>>
> >>>> This patch fixes the Microphone and LINE_IN select logic for
> >>>> surround codecs with shared jacks. This issue appears to apply
> >>>> to most, if not all AC'97 surround codecs. The existing code
> >>>> can never utilize the shared jacks for Microphone and LINE_IN
> >>>> due to the reversed jack selection logic. The patched code
> >>>> correctly selects the shared jack for input if the "Channel Mode"
> >>>> selector does not specify that the jack is to be used for output.
> >>>>
> >>>> Specifically, in "2ch" mode the Center/LFE jack is used for
> >>>> microphone input and the Surround jack is used for LINE_IN,
> >>>> in "4ch" mode the Center/LFE jack is used for microphone input
> >>>> and the Surround jack is used for output, and in "6ch" mode
> >>>> both jacks are used for output.
> >>>>
> >>>> Signed-off-by: Randy Cushman <rcushman_linux@xxxxxxxxxxxxx>
> >>>>
> >>>>
> >>> This hunk likely breaks other codec support.
> >>>
> >>> is_shared_linein() returns true if the line-in jack is shared with
> >>> surround output _and_ being used as the surround output. Maybe the AD
> >>> codec support is wrongly implemented.
> >>>
> >>>
> >> (snip)
> >>
> >>> thanks,
> >>>
> >>> Takashi
> >>>
> >>>
> >>>
> >>>
> >> [2 ac97_shared_1.patch <text/plain (7bit)>]
> >> Summary: fix microphone and line_in selection logic
> >>
> >> This patch fixes the Microphone and LINE_IN select logic for
> >> Analog Devices surround codecs with shared jacks. The existing
> >> code can never utilize the shared jacks for Microphone and LINE_IN
> >> due to the reversed jack selection logic. The patched code
> >> correctly selects the shared jack for input if the "Channel Mode"
> >> selector does not specify that the jack is to be used for output.
> >>
> >> Specifically, in "2ch" mode the Center/LFE jack is used for
> >> microphone input and the Surround jack is used for LINE_IN,
> >> in "4ch" mode the Center/LFE jack is used for microphone input
> >> and the Surround jack is used for output, and in "6ch" mode
> >> both jacks are used for output.
> >>
> >> Signed-off-by: Randy Cushman <rcushman_linux@xxxxxxxxxxxxx>
> >>
> >>
> >> diff -r cdbbd773cd9e pci/ac97/ac97_patch.c
> >> --- a/pci/ac97/ac97_patch.c Wed Dec 13 11:21:55 2006 +0000
> >> +++ b/pci/ac97/ac97_patch.c Tue Dec 19 10:03:51 2006 -0500
> >> @@ -190,14 +190,28 @@ static inline int is_clfe_on(struct snd_
> >> return ac97->channel_mode >= 2;
> >> }
> >>
> >> +/* system has shared jacks with surround out enabled */
> >> +static inline int is_shared_surrout(struct snd_ac97 *ac97)
> >> +{
> >> + return ! ac97->indep_surround && is_surround_on(ac97);
> >> +}
> >> +
> >> +/* system has shared jacks with center/lfe out enabled */
> >> +static inline int is_shared_clfeout(struct snd_ac97 *ac97)
> >> +{
> >> + return ! ac97->indep_surround && is_clfe_on(ac97);
> >> +}
> >> +
> >> +/* system has shared jacks with line in enabled */
> >> static inline int is_shared_linein(struct snd_ac97 *ac97)
> >> {
> >> - return ! ac97->indep_surround && is_surround_on(ac97);
> >> -}
> >> -
> >> + return ! ac97->indep_surround && ! is_surround_on(ac97);
> >> +}
> >> +
> >> +/* system has shared jacks with mic in enabled */
> >> static inline int is_shared_micin(struct snd_ac97 *ac97)
> >> {
> >> - return ! ac97->indep_surround && is_clfe_on(ac97);
> >> + return ! ac97->indep_surround && ! is_clfe_on(ac97);
> >> }
> >>
> >>
> >> @@ -2014,12 +2028,12 @@ static void alc650_update_jacks(struct s
> >> {
> >> int shared;
> >>
> >> - /* shared Line-In */
> >> - shared = is_shared_linein(ac97);
> >> + /* shared Line-In / Surround Out */
> >> + shared = is_shared_surrout(ac97);
> >> snd_ac97_update_bits(ac97, AC97_ALC650_MULTICH, 1 << 9,
> >> shared ? (1 << 9) : 0);
> >> - /* update shared Mic */
> >> - shared = is_shared_micin(ac97);
> >> + /* update shared Mic In / Center/LFE Out */
> >> + shared = is_shared_clfeout(ac97);
> >> /* disable/enable vref */
> >> snd_ac97_update_bits(ac97, AC97_ALC650_CLOCK, 1 << 12,
> >> shared ? (1 << 12) : 0);
> >> @@ -2149,12 +2163,12 @@ static void alc655_update_jacks(struct s
> >> {
> >> int shared;
> >>
> >> - /* shared Line-In */
> >> - shared = is_shared_linein(ac97);
> >> + /* shared Line-In / Surround Out */
> >> + shared = is_shared_surrout(ac97);
> >> ac97_update_bits_page(ac97, AC97_ALC650_MULTICH, 1 << 9,
> >> shared ? (1 << 9) : 0, 0);
> >> - /* update shared mic */
> >> - shared = is_shared_micin(ac97);
> >> + /* update shared Mic In / Center/LFE Out */
> >> + shared = is_shared_clfeout(ac97);
> >> /* misc control; vrefout disable */
> >> snd_ac97_update_bits(ac97, AC97_ALC650_CLOCK, 1 << 12,
> >> shared ? (1 << 12) : 0);
> >> @@ -2298,16 +2312,16 @@ static void alc850_update_jacks(struct s
> >> {
> >> int shared;
> >>
> >> - /* shared Line-In */
> >> - shared = is_shared_linein(ac97);
> >> + /* shared Line-In / Surround Out */
> >> + shared = is_shared_surrout(ac97);
> >> /* SURR 1kOhm (bit4), Amp (bit5) */
> >> snd_ac97_update_bits(ac97, AC97_ALC850_MISC1, (1<<4)|(1<<5),
> >> shared ? (1<<5) : (1<<4));
> >> /* LINE-IN = 0, SURROUND = 2 */
> >> snd_ac97_update_bits(ac97, AC97_ALC850_JACK_SELECT, 7 << 12,
> >> shared ? (2<<12) : (0<<12));
> >> - /* update shared mic */
> >> - shared = is_shared_micin(ac97);
> >> + /* update shared Mic In / Center/LFE Out */
> >> + shared = is_shared_clfeout(ac97);
> >> /* Vref disable (bit12), 1kOhm (bit13) */
> >> snd_ac97_update_bits(ac97, AC97_ALC850_MISC1, (1<<12)|(1<<13),
> >> shared ? (1<<12) : (1<<13));
> >> @@ -2380,9 +2394,9 @@ int patch_alc850(struct snd_ac97 *ac97)
> >> */
> >> static void cm9738_update_jacks(struct snd_ac97 *ac97)
> >> {
> >> - /* shared Line-In */
> >> + /* shared Line-In / Surround Out */
> >> snd_ac97_update_bits(ac97, AC97_CM9738_VENDOR_CTRL, 1 << 10,
> >> - is_shared_linein(ac97) ? (1 << 10) : 0);
> >> + is_shared_surrout(ac97) ? (1 << 10) : 0);
> >> }
> >>
> >> static const struct snd_kcontrol_new snd_ac97_cm9738_controls[] = {
> >> @@ -2464,12 +2478,12 @@ static const struct snd_kcontrol_new snd
> >>
> >> static void cm9739_update_jacks(struct snd_ac97 *ac97)
> >> {
> >> - /* shared Line-In */
> >> + /* shared Line-In / Surround Out */
> >> snd_ac97_update_bits(ac97, AC97_CM9739_MULTI_CHAN, 1 << 10,
> >> - is_shared_linein(ac97) ? (1 << 10) : 0);
> >> - /* shared Mic */
> >> + is_shared_surrout(ac97) ? (1 << 10) : 0);
> >> + /* shared Mic In / Center/LFE Out **/
> >> snd_ac97_update_bits(ac97, AC97_CM9739_MULTI_CHAN, 0x3000,
> >> - is_shared_micin(ac97) ? 0x1000 : 0x2000);
> >> + is_shared_clfeout(ac97) ? 0x1000 : 0x2000);
> >> }
> >>
> >> static const struct snd_kcontrol_new snd_ac97_cm9739_controls[] = {
> >> @@ -2581,8 +2595,8 @@ static void cm9761_update_jacks(struct s
> >>
> >> val |= surr_on[ac97->spec.dev_flags][is_surround_on(ac97)];
> >> val |= clfe_on[ac97->spec.dev_flags][is_clfe_on(ac97)];
> >> - val |= surr_shared[ac97->spec.dev_flags][is_shared_linein(ac97)];
> >> - val |= clfe_shared[ac97->spec.dev_flags][is_shared_micin(ac97)];
> >> + val |= surr_shared[ac97->spec.dev_flags][is_shared_surrout(ac97)];
> >> + val |= clfe_shared[ac97->spec.dev_flags][is_shared_clfeout(ac97)];
> >>
> >> snd_ac97_update_bits(ac97, AC97_CM9761_MULTI_CHAN, 0x3c88, val);
> >> }
> >> @@ -2829,12 +2843,12 @@ int patch_vt1617a(struct snd_ac97 * ac97
> >> */
> >> static void it2646_update_jacks(struct snd_ac97 *ac97)
> >> {
> >> - /* shared Line-In */
> >> + /* shared Line-In / Surround Out */
> >> snd_ac97_update_bits(ac97, 0x76, 1 << 9,
> >> - is_shared_linein(ac97) ? (1<<9) : 0);
> >> - /* shared Mic */
> >> + is_shared_surrout(ac97) ? (1<<9) : 0);
> >> + /* shared Mic / Center/LFE Out */
> >> snd_ac97_update_bits(ac97, 0x76, 1 << 10,
> >> - is_shared_micin(ac97) ? (1<<10) : 0);
> >> + is_shared_clfeout(ac97) ? (1<<10) : 0);
> >> }
> >>
> >> static const struct snd_kcontrol_new snd_ac97_controls_it2646[] = {
> >>
> >
> >
> >
>
> [2 ac97_shared_2.patch <text/plain (7bit)>]
> Summary: fix microphone and line_in selection logic
>
> This patch fixes the Microphone and LINE_IN select logic for
> Analog Devices surround codecs with shared jacks. The existing
> code can never utilize the shared jacks for Microphone and LINE_IN
> due to the reversed jack selection logic. The patched code
> correctly selects the shared jack for input if the "Channel Mode"
> selector does not specify that the jack is to be used for output.
>
> Specifically, in "2ch" mode the Center/LFE jack is used for
> microphone input and the Surround jack is used for LINE_IN,
> in "4ch" mode the Center/LFE jack is used for microphone input
> and the Surround jack is used for output, and in "6ch" mode
> both jacks are used for output.
>
> Signed-off-by: Randy Cushman <rcushman_linux@xxxxxxxxxxxxx>
>
>
> diff -r cdbbd773cd9e pci/ac97/ac97_patch.c
> --- a/pci/ac97/ac97_patch.c Wed Dec 13 11:21:55 2006 +0000
> +++ b/pci/ac97/ac97_patch.c Tue Dec 19 12:25:43 2006 -0500
> @@ -190,14 +190,28 @@ static inline int is_clfe_on(struct snd_
> return ac97->channel_mode >= 2;
> }
>
> +/* system has shared jacks with surround out enabled */
> +static inline int is_shared_surrout(struct snd_ac97 *ac97)
> +{
> + return !ac97->indep_surround && is_surround_on(ac97);
> +}
> +
> +/* system has shared jacks with center/lfe out enabled */
> +static inline int is_shared_clfeout(struct snd_ac97 *ac97)
> +{
> + return !ac97->indep_surround && is_clfe_on(ac97);
> +}
> +
> +/* system has shared jacks with line in enabled */
> static inline int is_shared_linein(struct snd_ac97 *ac97)
> {
> - return ! ac97->indep_surround && is_surround_on(ac97);
> -}
> -
> + return !ac97->indep_surround && !is_surround_on(ac97);
> +}
> +
> +/* system has shared jacks with mic in enabled */
> static inline int is_shared_micin(struct snd_ac97 *ac97)
> {
> - return ! ac97->indep_surround && is_clfe_on(ac97);
> + return !ac97->indep_surround && !is_clfe_on(ac97);
> }
>
>
> @@ -2014,12 +2028,12 @@ static void alc650_update_jacks(struct s
> {
> int shared;
>
> - /* shared Line-In */
> - shared = is_shared_linein(ac97);
> + /* shared Line-In / Surround Out */
> + shared = is_shared_surrout(ac97);
> snd_ac97_update_bits(ac97, AC97_ALC650_MULTICH, 1 << 9,
> shared ? (1 << 9) : 0);
> - /* update shared Mic */
> - shared = is_shared_micin(ac97);
> + /* update shared Mic In / Center/LFE Out */
> + shared = is_shared_clfeout(ac97);
> /* disable/enable vref */
> snd_ac97_update_bits(ac97, AC97_ALC650_CLOCK, 1 << 12,
> shared ? (1 << 12) : 0);
> @@ -2149,12 +2163,12 @@ static void alc655_update_jacks(struct s
> {
> int shared;
>
> - /* shared Line-In */
> - shared = is_shared_linein(ac97);
> + /* shared Line-In / Surround Out */
> + shared = is_shared_surrout(ac97);
> ac97_update_bits_page(ac97, AC97_ALC650_MULTICH, 1 << 9,
> shared ? (1 << 9) : 0, 0);
> - /* update shared mic */
> - shared = is_shared_micin(ac97);
> + /* update shared Mic In / Center/LFE Out */
> + shared = is_shared_clfeout(ac97);
> /* misc control; vrefout disable */
> snd_ac97_update_bits(ac97, AC97_ALC650_CLOCK, 1 << 12,
> shared ? (1 << 12) : 0);
> @@ -2298,16 +2312,16 @@ static void alc850_update_jacks(struct s
> {
> int shared;
>
> - /* shared Line-In */
> - shared = is_shared_linein(ac97);
> + /* shared Line-In / Surround Out */
> + shared = is_shared_surrout(ac97);
> /* SURR 1kOhm (bit4), Amp (bit5) */
> snd_ac97_update_bits(ac97, AC97_ALC850_MISC1, (1<<4)|(1<<5),
> shared ? (1<<5) : (1<<4));
> /* LINE-IN = 0, SURROUND = 2 */
> snd_ac97_update_bits(ac97, AC97_ALC850_JACK_SELECT, 7 << 12,
> shared ? (2<<12) : (0<<12));
> - /* update shared mic */
> - shared = is_shared_micin(ac97);
> + /* update shared Mic In / Center/LFE Out */
> + shared = is_shared_clfeout(ac97);
> /* Vref disable (bit12), 1kOhm (bit13) */
> snd_ac97_update_bits(ac97, AC97_ALC850_MISC1, (1<<12)|(1<<13),
> shared ? (1<<12) : (1<<13));
> @@ -2380,9 +2394,9 @@ int patch_alc850(struct snd_ac97 *ac97)
> */
> static void cm9738_update_jacks(struct snd_ac97 *ac97)
> {
> - /* shared Line-In */
> + /* shared Line-In / Surround Out */
> snd_ac97_update_bits(ac97, AC97_CM9738_VENDOR_CTRL, 1 << 10,
> - is_shared_linein(ac97) ? (1 << 10) : 0);
> + is_shared_surrout(ac97) ? (1 << 10) : 0);
> }
>
> static const struct snd_kcontrol_new snd_ac97_cm9738_controls[] = {
> @@ -2464,12 +2478,12 @@ static const struct snd_kcontrol_new snd
>
> static void cm9739_update_jacks(struct snd_ac97 *ac97)
> {
> - /* shared Line-In */
> + /* shared Line-In / Surround Out */
> snd_ac97_update_bits(ac97, AC97_CM9739_MULTI_CHAN, 1 << 10,
> - is_shared_linein(ac97) ? (1 << 10) : 0);
> - /* shared Mic */
> + is_shared_surrout(ac97) ? (1 << 10) : 0);
> + /* shared Mic In / Center/LFE Out **/
> snd_ac97_update_bits(ac97, AC97_CM9739_MULTI_CHAN, 0x3000,
> - is_shared_micin(ac97) ? 0x1000 : 0x2000);
> + is_shared_clfeout(ac97) ? 0x1000 : 0x2000);
> }
>
> static const struct snd_kcontrol_new snd_ac97_cm9739_controls[] = {
> @@ -2581,8 +2595,8 @@ static void cm9761_update_jacks(struct s
>
> val |= surr_on[ac97->spec.dev_flags][is_surround_on(ac97)];
> val |= clfe_on[ac97->spec.dev_flags][is_clfe_on(ac97)];
> - val |= surr_shared[ac97->spec.dev_flags][is_shared_linein(ac97)];
> - val |= clfe_shared[ac97->spec.dev_flags][is_shared_micin(ac97)];
> + val |= surr_shared[ac97->spec.dev_flags][is_shared_surrout(ac97)];
> + val |= clfe_shared[ac97->spec.dev_flags][is_shared_clfeout(ac97)];
>
> snd_ac97_update_bits(ac97, AC97_CM9761_MULTI_CHAN, 0x3c88, val);
> }
> @@ -2829,12 +2843,12 @@ int patch_vt1617a(struct snd_ac97 * ac97
> */
> static void it2646_update_jacks(struct snd_ac97 *ac97)
> {
> - /* shared Line-In */
> + /* shared Line-In / Surround Out */
> snd_ac97_update_bits(ac97, 0x76, 1 << 9,
> - is_shared_linein(ac97) ? (1<<9) : 0);
> - /* shared Mic */
> + is_shared_surrout(ac97) ? (1<<9) : 0);
> + /* shared Mic / Center/LFE Out */
> snd_ac97_update_bits(ac97, 0x76, 1 << 10,
> - is_shared_micin(ac97) ? (1<<10) : 0);
> + is_shared_clfeout(ac97) ? (1<<10) : 0);
> }
>
> static const struct snd_kcontrol_new snd_ac97_controls_it2646[] = {
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/alsa-devel
